From bb4554c81e0d819d74996f89cbb9c00476aedf8c Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 16 Nov 2023 11:15:08 -0300 Subject: bench: add benchmark for wallet creation procedure --- src/Makefile.bench.include | 1 + src/bench/wallet_create.cpp | 55 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 src/bench/wallet_create.cpp (limited to 'src') diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 28b779a5a8..217f123b16 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -84,6 +84,7 @@ endif if ENABLE_WALLET bench_bench_bitcoin_SOURCES += bench/coin_selection.cpp bench_bench_bitcoin_SOURCES += bench/wallet_balance.cpp +bench_bench_bitcoin_SOURCES += bench/wallet_create.cpp bench_bench_bitcoin_SOURCES += bench/wallet_loading.cpp bench_bench_bitcoin_SOURCES += bench/wallet_create_tx.cpp bench_bench_bitcoin_LDADD += $(BDB_LIBS) $(SQLITE_LIBS) diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp new file mode 100644 index 0000000000..ba3c25d25e --- /dev/null +++ b/src/bench/wallet_create.cpp @@ -0,0 +1,55 @@ +// Copyright (c) 2023-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include + +namespace wallet { +static void WalletCreate(benchmark::Bench& bench, bool encrypted) +{ + auto test_setup = MakeNoLogFileContext(); + FastRandomContext random; + + WalletContext context; + context.args = &test_setup->m_args; + context.chain = test_setup->m_node.chain.get(); + + DatabaseOptions options; + options.require_format = DatabaseFormat::SQLITE; + options.require_create = true; + options.create_flags = WALLET_FLAG_DESCRIPTORS; + + if (encrypted) { + options.create_passphrase = random.rand256().ToString(); + } + + DatabaseStatus status; + bilingual_str error_string; + std::vector warnings; + + fs::path wallet_path = test_setup->m_path_root / strprintf("test_wallet_%d", random.rand32()).c_str(); + bench.run([&] { + auto wallet = CreateWallet(context, wallet_path.u8string(), /*load_on_start=*/std::nullopt, options, status, error_string, warnings); + assert(status == DatabaseStatus::SUCCESS); + assert(wallet != nullptr); + + // Cleanup + wallet.reset(); + fs::remove_all(wallet_path); + }); +} + +static void WalletCreatePlain(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/false); } +static void WalletCreateEncrypted(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/true); } + +#ifdef USE_SQLITE +BENCHMARK(WalletCreatePlain, benchmark::PriorityLevel::LOW); +BENCHMARK(WalletCreateEncrypted, benchmark::PriorityLevel::LOW); +#endif + +} // namespace wallet -- cgit v1.2.3 From 075aa44ceba41fa82bb3ce2295e2962e5fd0508e Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 20 Jul 2023 18:20:00 -0300 Subject: wallet: batch descriptor spkm TopUp Instead of performing multiple atomic write operations per descriptor setup call, batch them all within a single atomic db txn. --- src/wallet/scriptpubkeyman.cpp | 16 ++++++++++++++-- src/wallet/scriptpubkeyman.h | 5 ++++- 2 files changed, 18 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index d2b2801aa8..997348d371 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -2135,6 +2135,15 @@ std::map DescriptorScriptPubKeyMan::GetKeys() const } bool DescriptorScriptPubKeyMan::TopUp(unsigned int size) +{ + WalletBatch batch(m_storage.GetDatabase()); + if (!batch.TxnBegin()) return false; + bool res = TopUpWithDB(batch, size); + if (!batch.TxnCommit()) throw std::runtime_error(strprintf("Error during descriptors keypool top up. Cannot commit changes for wallet %s", m_storage.GetDisplayName())); + return res; +} + +bool DescriptorScriptPubKeyMan::TopUpWithDB(WalletBatch& batch, unsigned int size) { LOCK(cs_desc_man); unsigned int target_size; @@ -2157,7 +2166,6 @@ bool DescriptorScriptPubKeyMan::TopUp(unsigned int size) FlatSigningProvider provider; provider.keys = GetKeys(); - WalletBatch batch(m_storage.GetDatabase()); uint256 id = GetID(); for (int32_t i = m_max_cached_index + 1; i < new_range_end; ++i) { FlatSigningProvider out_keys; @@ -2326,6 +2334,8 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_ // Store the master private key, and descriptor WalletBatch batch(m_storage.GetDatabase()); + if (!batch.TxnBegin()) throw std::runtime_error(std::string(__func__) + ": cannot start db transaction"); + if (!AddDescriptorKeyWithDB(batch, master_key.key, master_key.key.GetPubKey())) { throw std::runtime_error(std::string(__func__) + ": writing descriptor master private key failed"); } @@ -2334,9 +2344,11 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_ } // TopUp - TopUp(); + TopUpWithDB(batch); m_storage.UnsetBlankWalletFlag(batch); + + if (!batch.TxnCommit()) throw std::runtime_error(std::string(__func__) + ": error committing db transaction"); return true; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 7c0eca1475..7bdfbf0d34 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -583,7 +583,10 @@ private: std::unique_ptr GetSigningProvider(int32_t index, bool include_private = false) const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); protected: - WalletDescriptor m_wallet_descriptor GUARDED_BY(cs_desc_man); + WalletDescriptor m_wallet_descriptor GUARDED_BY(cs_desc_man); + + //! Same as 'TopUp' but designed for use within a batch transaction context + bool TopUpWithDB(WalletBatch& batch, unsigned int size = 0); public: DescriptorScriptPubKeyMan(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size) -- cgit v1.2.3 From 3eb769f15013873755e482707cad341bc1ce8a8c Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 20 Jul 2023 18:10:41 -0300 Subject: wallet: batch legacy spkm TopUp Instead of performing multiple atomic write operations per legacy spkm setup call, batch them all within a single atomic db txn. --- src/wallet/scriptpubkeyman.cpp | 13 ++++++++----- src/wallet/scriptpubkeyman.h | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 997348d371..ce757a1c6b 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -333,7 +333,8 @@ bool LegacyScriptPubKeyMan::TopUpInactiveHDChain(const CKeyID seed_id, int64_t i chain.m_next_external_index = std::max(chain.m_next_external_index, index + 1); } - TopUpChain(chain, 0); + WalletBatch batch(m_storage.GetDatabase()); + TopUpChain(batch, chain, 0); return true; } @@ -1274,19 +1275,22 @@ bool LegacyScriptPubKeyMan::TopUp(unsigned int kpSize) return false; } - if (!TopUpChain(m_hd_chain, kpSize)) { + WalletBatch batch(m_storage.GetDatabase()); + if (!batch.TxnBegin()) return false; + if (!TopUpChain(batch, m_hd_chain, kpSize)) { return false; } for (auto& [chain_id, chain] : m_inactive_hd_chains) { - if (!TopUpChain(chain, kpSize)) { + if (!TopUpChain(batch, chain, kpSize)) { return false; } } + if (!batch.TxnCommit()) throw std::runtime_error(strprintf("Error during keypool top up. Cannot commit changes for wallet %s", m_storage.GetDisplayName())); NotifyCanGetAddressesChanged(); return true; } -bool LegacyScriptPubKeyMan::TopUpChain(CHDChain& chain, unsigned int kpSize) +bool LegacyScriptPubKeyMan::TopUpChain(WalletBatch& batch, CHDChain& chain, unsigned int kpSize) { LOCK(cs_KeyStore); @@ -1318,7 +1322,6 @@ bool LegacyScriptPubKeyMan::TopUpChain(CHDChain& chain, unsigned int kpSize) missingInternal = 0; } bool internal = false; - WalletBatch batch(m_storage.GetDatabase()); for (int64_t i = missingInternal + missingExternal; i--;) { if (i < missingInternal) { internal = true; diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 7bdfbf0d34..a0e9dbb6c2 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -366,7 +366,7 @@ private: */ bool TopUpInactiveHDChain(const CKeyID seed_id, int64_t index, bool internal); - bool TopUpChain(CHDChain& chain, unsigned int size); + bool TopUpChain(WalletBatch& batch, CHDChain& chain, unsigned int size); public: LegacyScriptPubKeyMan(WalletStorage& storage, int64_t keypool_size) : ScriptPubKeyMan(storage), m_keypool_size(keypool_size) {} -- cgit v1.2.3 From 1f65241b733cd1e962c88909ae66816bc6451fd1 Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 29 Sep 2023 15:20:25 -0300 Subject: wallet: descriptors setup, batch db operations Instead of doing one db transaction per descriptor setup, batch all descriptors' setup writes in a single db txn. Speeding up the process and preventing the wallet from entering an inconsistent state if any of the intermediate transactions fail. --- src/wallet/scriptpubkeyman.cpp | 7 +------ src/wallet/scriptpubkeyman.h | 2 +- src/wallet/wallet.cpp | 18 +++++++++++++++--- src/wallet/wallet.h | 3 +++ 4 files changed, 20 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index ce757a1c6b..0b4800b848 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -2275,7 +2275,7 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const } } -bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type, bool internal) +bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(WalletBatch& batch, const CExtKey& master_key, OutputType addr_type, bool internal) { LOCK(cs_desc_man); assert(m_storage.IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); @@ -2336,9 +2336,6 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_ m_wallet_descriptor = w_desc; // Store the master private key, and descriptor - WalletBatch batch(m_storage.GetDatabase()); - if (!batch.TxnBegin()) throw std::runtime_error(std::string(__func__) + ": cannot start db transaction"); - if (!AddDescriptorKeyWithDB(batch, master_key.key, master_key.key.GetPubKey())) { throw std::runtime_error(std::string(__func__) + ": writing descriptor master private key failed"); } @@ -2350,8 +2347,6 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_ TopUpWithDB(batch); m_storage.UnsetBlankWalletFlag(batch); - - if (!batch.TxnCommit()) throw std::runtime_error(std::string(__func__) + ": error committing db transaction"); return true; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index a0e9dbb6c2..936c76c223 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -621,7 +621,7 @@ public: bool IsHDEnabled() const override; //! Setup descriptors based on the given CExtkey - bool SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type, bool internal); + bool SetupDescriptorGeneration(WalletBatch& batch, const CExtKey& master_key, OutputType addr_type, bool internal); /** Provide a descriptor at setup time * Returns false if already setup or setup fails, true if setup is successful diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ecf18fbe78..4df34923d3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3535,6 +3535,10 @@ void CWallet::SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) { AssertLockHeld(cs_wallet); + // Create single batch txn + WalletBatch batch(GetDatabase()); + if (!batch.TxnBegin()) throw std::runtime_error("Error: cannot create db transaction for descriptors setup"); + for (bool internal : {false, true}) { for (OutputType t : OUTPUT_TYPES) { auto spk_manager = std::unique_ptr(new DescriptorScriptPubKeyMan(*this, m_keypool_size)); @@ -3542,16 +3546,19 @@ void CWallet::SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) if (IsLocked()) { throw std::runtime_error(std::string(__func__) + ": Wallet is locked, cannot setup new descriptors"); } - if (!spk_manager->CheckDecryptionKey(vMasterKey) && !spk_manager->Encrypt(vMasterKey, nullptr)) { + if (!spk_manager->CheckDecryptionKey(vMasterKey) && !spk_manager->Encrypt(vMasterKey, &batch)) { throw std::runtime_error(std::string(__func__) + ": Could not encrypt new descriptors"); } } - spk_manager->SetupDescriptorGeneration(master_key, t, internal); + spk_manager->SetupDescriptorGeneration(batch, master_key, t, internal); uint256 id = spk_manager->GetID(); AddScriptPubKeyMan(id, std::move(spk_manager)); - AddActiveScriptPubKeyMan(id, t, internal); + AddActiveScriptPubKeyManWithDb(batch, id, t, internal); } } + + // Ensure information is committed to disk + if (!batch.TxnCommit()) throw std::runtime_error("Error: cannot commit db transaction for descriptors setup"); } void CWallet::SetupDescriptorScriptPubKeyMans() @@ -3606,6 +3613,11 @@ void CWallet::SetupDescriptorScriptPubKeyMans() void CWallet::AddActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal) { WalletBatch batch(GetDatabase()); + return AddActiveScriptPubKeyManWithDb(batch, id, type, internal); +} + +void CWallet::AddActiveScriptPubKeyManWithDb(WalletBatch& batch, uint256 id, OutputType type, bool internal) +{ if (!batch.WriteActiveScriptPubKeyMan(static_cast(type), id, internal)) { throw std::runtime_error(std::string(__func__) + ": writing active ScriptPubKeyMan id failed"); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index bc45010200..11b7964316 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -419,6 +419,9 @@ private: // Must be the only method adding data to it. void AddScriptPubKeyMan(const uint256& id, std::unique_ptr spkm_man); + // Same as 'AddActiveScriptPubKeyMan' but designed for use within a batch transaction context + void AddActiveScriptPubKeyManWithDb(WalletBatch& batch, uint256 id, OutputType type, bool internal); + /** * Catch wallet up to current chain, scanning new blocks, updating the best * block locator and m_last_block_processed, and registering for -- cgit v1.2.3 From f05302427386fe63f4929a7198652cb1e4ab3bcc Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 21 Nov 2023 23:07:00 -0300 Subject: wallet: batch external signer descriptor import Co-authored-by: furszy --- src/wallet/external_signer_scriptpubkeyman.cpp | 5 ++--- src/wallet/external_signer_scriptpubkeyman.h | 2 +- src/wallet/scriptpubkeyman.h | 5 ----- src/wallet/wallet.cpp | 11 +++++++++-- 4 files changed, 12 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index 6d026d02c1..a71f8f9fbc 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -16,7 +16,7 @@ #include namespace wallet { -bool ExternalSignerScriptPubKeyMan::SetupDescriptor(std::unique_ptr desc) +bool ExternalSignerScriptPubKeyMan::SetupDescriptor(WalletBatch& batch, std::unique_ptr desc) { LOCK(cs_desc_man); assert(m_storage.IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); @@ -29,13 +29,12 @@ bool ExternalSignerScriptPubKeyMan::SetupDescriptor(std::unique_ptr m_wallet_descriptor = w_desc; // Store the descriptor - WalletBatch batch(m_storage.GetDatabase()); if (!batch.WriteDescriptor(GetID(), m_wallet_descriptor)) { throw std::runtime_error(std::string(__func__) + ": writing descriptor failed"); } // TopUp - TopUp(); + TopUpWithDB(batch); m_storage.UnsetBlankWalletFlag(batch); return true; diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index 01dc80b1ca..c052ce6129 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -23,7 +23,7 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan /** Provide a descriptor at setup time * Returns false if already setup or setup fails, true if setup is successful */ - bool SetupDescriptor(std::unique_ptrdesc); + bool SetupDescriptor(WalletBatch& batch, std::unique_ptrdesc); static ExternalSigner GetExternalSigner(); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 936c76c223..dccbf3ced6 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -623,11 +623,6 @@ public: //! Setup descriptors based on the given CExtkey bool SetupDescriptorGeneration(WalletBatch& batch, const CExtKey& master_key, OutputType addr_type, bool internal); - /** Provide a descriptor at setup time - * Returns false if already setup or setup fails, true if setup is successful - */ - bool SetupDescriptor(std::unique_ptrdesc); - bool HavePrivateKeys() const override; std::optional GetOldestKeyPoolTime() const override; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4df34923d3..4ca1c75abf 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3585,6 +3585,10 @@ void CWallet::SetupDescriptorScriptPubKeyMans() UniValue signer_res = signer.GetDescriptors(account); if (!signer_res.isObject()) throw std::runtime_error(std::string(__func__) + ": Unexpected result"); + + WalletBatch batch(GetDatabase()); + if (!batch.TxnBegin()) throw std::runtime_error("Error: cannot create db transaction for descriptors import"); + for (bool internal : {false, true}) { const UniValue& descriptor_vals = signer_res.find_value(internal ? "internal" : "receive"); if (!descriptor_vals.isArray()) throw std::runtime_error(std::string(__func__) + ": Unexpected result"); @@ -3601,12 +3605,15 @@ void CWallet::SetupDescriptorScriptPubKeyMans() } OutputType t = *desc->GetOutputType(); auto spk_manager = std::unique_ptr(new ExternalSignerScriptPubKeyMan(*this, m_keypool_size)); - spk_manager->SetupDescriptor(std::move(desc)); + spk_manager->SetupDescriptor(batch, std::move(desc)); uint256 id = spk_manager->GetID(); AddScriptPubKeyMan(id, std::move(spk_manager)); - AddActiveScriptPubKeyMan(id, t, internal); + AddActiveScriptPubKeyManWithDb(batch, id, t, internal); } } + + // Ensure imported descriptors are committed to disk + if (!batch.TxnCommit()) throw std::runtime_error("Error: cannot commit db transaction for descriptors import"); } } -- cgit v1.2.3