From 29905c092f15bbbc4d4964c2f99dcf12fefd4111 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 26 Aug 2021 14:39:46 +0200 Subject: [PATCH 1/3] refactor: avoid multiple key->metadata lookups in dumpwallet RPC This also enables working with a const ScriptPubKeyMan which was previously not possible due to std::map::operator[] not being const. --- src/wallet/rpcdump.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 5edd9f8f664..ccfb50c37cf 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -809,6 +809,9 @@ RPCHelpMan dumpwallet() std::string strLabel; CKey key; if (spk_man.GetKey(keyid, key)) { + CKeyMetadata metadata; + const auto it{spk_man.mapKeyMetadata.find(keyid)}; + if (it != spk_man.mapKeyMetadata.end()) metadata = it->second; file << strprintf("%s %s ", EncodeSecret(key), strTime); if (GetWalletAddressesForKey(&spk_man, wallet, keyid, strAddr, strLabel)) { file << strprintf("label=%s", strLabel); @@ -816,12 +819,12 @@ RPCHelpMan dumpwallet() file << "hdseed=1"; } else if (mapKeyPool.count(keyid)) { file << "reserve=1"; - } else if (spk_man.mapKeyMetadata[keyid].hdKeypath == "s") { + } else if (metadata.hdKeypath == "s") { file << "inactivehdseed=1"; } else { file << "change=1"; } - file << strprintf(" # addr=%s%s\n", strAddr, (spk_man.mapKeyMetadata[keyid].has_key_origin ? " hdkeypath="+WriteHDKeypath(spk_man.mapKeyMetadata[keyid].key_origin.path) : "")); + file << strprintf(" # addr=%s%s\n", strAddr, (metadata.has_key_origin ? " hdkeypath="+WriteHDKeypath(metadata.key_origin.path) : "")); } } file << "\n"; From ec2792d1dc3404d749a43556eeb4b63780ee6d94 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 26 Aug 2021 15:11:17 +0200 Subject: [PATCH 2/3] refactor: use const `LegacyScriptPubKeyMan` references in dump{privkey,wallet} RPCs --- src/wallet/rpcdump.cpp | 6 +++--- src/wallet/rpcwallet.cpp | 9 +++++++++ src/wallet/rpcwallet.h | 1 + 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index ccfb50c37cf..2854e52f84a 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -57,7 +57,7 @@ static std::string DecodeDumpString(const std::string &str) { return ret.str(); } -static bool GetWalletAddressesForKey(LegacyScriptPubKeyMan* spk_man, const CWallet& wallet, const CKeyID& keyid, std::string& strAddr, std::string& strLabel) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +static bool GetWalletAddressesForKey(const LegacyScriptPubKeyMan* spk_man, const CWallet& wallet, const CKeyID& keyid, std::string& strAddr, std::string& strLabel) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { bool fLabelFound = false; CKey key; @@ -684,7 +684,7 @@ RPCHelpMan dumpprivkey() std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return NullUniValue; - LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet); + const LegacyScriptPubKeyMan& spk_man = EnsureConstLegacyScriptPubKeyMan(*pwallet); LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); @@ -735,7 +735,7 @@ RPCHelpMan dumpwallet() if (!pwallet) return NullUniValue; CWallet& wallet = *pwallet; - LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(wallet); + const LegacyScriptPubKeyMan& spk_man = EnsureConstLegacyScriptPubKeyMan(wallet); // Make sure the results are valid at least up to the most recent block // the user could have gotten from another RPC command prior to now diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 86bfa10d888..babb61b03ad 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -150,6 +150,15 @@ LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_cr return *spk_man; } +const LegacyScriptPubKeyMan& EnsureConstLegacyScriptPubKeyMan(const CWallet& wallet) +{ + const LegacyScriptPubKeyMan* spk_man = wallet.GetLegacyScriptPubKeyMan(); + if (!spk_man) { + throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command"); + } + return *spk_man; +} + static void WalletTxToJSON(const CWallet& wallet, const CWalletTx& wtx, UniValue& entry) { interfaces::Chain& chain = wallet.chain(); diff --git a/src/wallet/rpcwallet.h b/src/wallet/rpcwallet.h index 8b88ffe8eda..3a85fc0c64a 100644 --- a/src/wallet/rpcwallet.h +++ b/src/wallet/rpcwallet.h @@ -34,6 +34,7 @@ std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& reques void EnsureWalletIsUnlocked(const CWallet&); WalletContext& EnsureWalletContext(const std::any& context); LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create = false); +const LegacyScriptPubKeyMan& EnsureConstLegacyScriptPubKeyMan(const CWallet& wallet); RPCHelpMan getaddressinfo(); RPCHelpMan signrawtransactionwithwallet(); From d150fe3ad5ce181511f2d2fd035a10530eaa4203 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 26 Aug 2021 15:20:12 +0200 Subject: [PATCH 3/3] refactor: use `CWallet` const shared pointers in dump{privkey,wallet} RPCs --- src/wallet/rpcdump.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 2854e52f84a..403c9786807 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -681,7 +681,7 @@ RPCHelpMan dumpprivkey() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + const std::shared_ptr pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return NullUniValue; const LegacyScriptPubKeyMan& spk_man = EnsureConstLegacyScriptPubKeyMan(*pwallet); @@ -731,10 +731,10 @@ RPCHelpMan dumpwallet() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + const std::shared_ptr pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return NullUniValue; - CWallet& wallet = *pwallet; + const CWallet& wallet = *pwallet; const LegacyScriptPubKeyMan& spk_man = EnsureConstLegacyScriptPubKeyMan(wallet); // Make sure the results are valid at least up to the most recent block