From c986b43f0c45388b87106bcbf2534c75eebe7b72 Mon Sep 17 00:00:00 2001 From: Saikiran Date: Mon, 10 Mar 2025 15:20:55 +0530 Subject: [PATCH] removed duplicate calling of GetDescriptorScriptPubKeyMan Removed duplicate call to GetDescriptorScriptPubKeyMan and Instead of checking linearly I have used find method so time complexity reduced significantly for GetDescriptorScriptPubKeyMan after this fix improved performance of importdescriptor part refs #32013. --- src/wallet/rpc/backup.cpp | 8 ------ src/wallet/scriptpubkeyman.cpp | 2 +- src/wallet/wallet.cpp | 6 +++-- test/functional/wallet_importdescriptors.py | 29 ++++++++++++--------- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index ac23b092d40..3a00dca8d96 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1575,14 +1575,6 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c WalletDescriptor w_desc(std::move(parsed_desc), timestamp, range_start, range_end, next_index); - // Check if the wallet already contains the descriptor - auto existing_spk_manager = wallet.GetDescriptorScriptPubKeyMan(w_desc); - if (existing_spk_manager) { - if (!existing_spk_manager->CanUpdateToWalletDescriptor(w_desc, error)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, error); - } - } - // Add descriptor to the wallet auto spk_manager = wallet.AddWalletDescriptor(w_desc, keys, label, desc_internal); if (spk_manager == nullptr) { diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 04287581f5a..361f7ff6fc2 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -2832,7 +2832,7 @@ void DescriptorScriptPubKeyMan::UpdateWalletDescriptor(WalletDescriptor& descrip LOCK(cs_desc_man); std::string error; if (!CanUpdateToWalletDescriptor(descriptor, error)) { - throw std::runtime_error(std::string(__func__) + ": " + error); + throw std::runtime_error(error); } m_map_pubkeys.clear(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7abd17d31e9..fec8adb8e22 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3906,9 +3906,11 @@ bool CWallet::IsLegacy() const DescriptorScriptPubKeyMan* CWallet::GetDescriptorScriptPubKeyMan(const WalletDescriptor& desc) const { - for (auto& spk_man_pair : m_spk_managers) { + auto spk_man_pair = m_spk_managers.find(desc.id); + + if (spk_man_pair != m_spk_managers.end()) { // Try to downcast to DescriptorScriptPubKeyMan then check if the descriptors match - DescriptorScriptPubKeyMan* spk_manager = dynamic_cast(spk_man_pair.second.get()); + DescriptorScriptPubKeyMan* spk_manager = dynamic_cast(spk_man_pair->second.get()); if (spk_manager != nullptr && spk_manager->HasWalletDescriptor(desc)) { return spk_manager; } diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index 84c07b6a282..74f4ecfd7c5 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -57,15 +57,20 @@ class ImportDescriptorsTest(BitcoinTestFramework): if wallet is not None: wrpc = wallet - result = wrpc.importdescriptors([req]) - observed_warnings = [] - if 'warnings' in result[0]: - observed_warnings = result[0]['warnings'] - assert_equal("\n".join(sorted(warnings)), "\n".join(sorted(observed_warnings))) - assert_equal(result[0]['success'], success) - if error_code is not None: - assert_equal(result[0]['error']['code'], error_code) - assert_equal(result[0]['error']['message'], error_message) + try: + result = wrpc.importdescriptors([req]) + except JSONRPCException as e: + assert_equal(e.error["code"], error_code) + assert_equal(e.error['message'], error_message) + else: + observed_warnings = [] + if 'warnings' in result[0]: + observed_warnings = result[0]['warnings'] + assert_equal("\n".join(sorted(warnings)), "\n".join(sorted(observed_warnings))) + assert_equal(result[0]['success'], success) + if error_code is not None: + assert_equal(result[0]['error']['code'], error_code) + assert_equal(result[0]['error']['message'], error_message) def run_test(self): self.log.info('Setting up wallets') @@ -274,11 +279,11 @@ class ImportDescriptorsTest(BitcoinTestFramework): assert_equal(wpriv.getwalletinfo()['keypoolsize'], 21) self.test_importdesc({**range_request, "range": [5, 10]}, wallet=wpriv, success=False, - error_code=-8, error_message='new range must include current range = [0,20]') + error_code=-1, error_message='new range must include current range = [0,20]') self.test_importdesc({**range_request, "range": [0, 10]}, wallet=wpriv, success=False, - error_code=-8, error_message='new range must include current range = [0,20]') + error_code=-1, error_message='new range must include current range = [0,20]') self.test_importdesc({**range_request, "range": [5, 20]}, wallet=wpriv, success=False, - error_code=-8, error_message='new range must include current range = [0,20]') + error_code=-1, error_message='new range must include current range = [0,20]') assert_equal(wpriv.getwalletinfo()['keypoolsize'], 21) self.log.info("Check we can change descriptor internal flag")