From 1f65241b733cd1e962c88909ae66816bc6451fd1 Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 29 Sep 2023 15:20:25 -0300 Subject: [PATCH] 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(-) 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