From 76b3c37fcb93b4bcb047e0500fdaa605160e25d5 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Wed, 20 Jul 2022 19:06:57 +0200 Subject: [PATCH] refactor: wallet: return util::Result from `GetReservedDestination` methods --- src/wallet/scriptpubkeyman.cpp | 23 +++++++---------------- src/wallet/scriptpubkeyman.h | 6 +++--- src/wallet/spend.cpp | 10 ++++++---- src/wallet/wallet.cpp | 25 +++++++++---------------- src/wallet/wallet.h | 2 +- 5 files changed, 26 insertions(+), 40 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 2f242901ab..0cf77b0e87 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -292,26 +292,22 @@ bool LegacyScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, WalletBat return true; } -bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) +util::Result LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) { if (LEGACY_OUTPUT_TYPES.count(type) == 0) { - error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types"); - return false; + return util::Error{_("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types")}; } assert(type != OutputType::BECH32M); LOCK(cs_KeyStore); if (!CanGetAddresses(internal)) { - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}; } if (!ReserveKeyFromKeyPool(index, keypool, internal)) { - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}; } - address = GetDestinationForKey(keypool.vchPubKey, type); - return true; + return GetDestinationForKey(keypool.vchPubKey, type); } bool LegacyScriptPubKeyMan::TopUpInactiveHDChain(const CKeyID seed_id, int64_t index, bool internal) @@ -1760,17 +1756,12 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle return true; } -bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) +util::Result DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) { LOCK(cs_desc_man); auto op_dest = GetNewDestination(type); index = m_wallet_descriptor.next_index - 1; - if (op_dest) { - address = *op_dest; - } else { - error = util::ErrorString(op_dest); - } - return bool(op_dest); + return op_dest; } void DescriptorScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index b772dde533..6c8102062a 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -179,7 +179,7 @@ public: virtual bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) { return false; } virtual bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) { return false; } - virtual bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) { return false; } + virtual util::Result GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) { return util::Error{Untranslated("Not supported")}; } virtual void KeepDestination(int64_t index, const OutputType& type) {} virtual void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) {} @@ -366,7 +366,7 @@ public: bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; - bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) override; + util::Result GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) override; void KeepDestination(int64_t index, const OutputType& type) override; void ReturnDestination(int64_t index, bool internal, const CTxDestination&) override; @@ -574,7 +574,7 @@ public: bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; - bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) override; + util::Result GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) override; void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) override; // Tops up the descriptor cache and m_map_script_pub_keys. The cache is stored in the wallet file diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 5d320feff0..cb8a273dfc 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -812,11 +812,13 @@ static util::Result CreateTransactionInternal( // Reserve a new key pair from key pool. If it fails, provide a dummy // destination in case we don't need change. CTxDestination dest; - bilingual_str dest_err; - if (!reservedest.GetReservedDestination(dest, true, dest_err)) { - error = _("Transaction needs a change address, but we can't generate it.") + Untranslated(" ") + dest_err; + auto op_dest = reservedest.GetReservedDestination(true); + if (!op_dest) { + error = _("Transaction needs a change address, but we can't generate it.") + Untranslated(" ") + util::ErrorString(op_dest); + } else { + dest = *op_dest; + scriptChange = GetScriptForDestination(dest); } - scriptChange = GetScriptForDestination(dest); // A valid destination implies a change script (and // vice-versa). An empty change script will abort later, if the // change keypool ran out, but change is required. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e750cd5a2c..1a7e1684e4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2351,15 +2351,11 @@ util::Result CWallet::GetNewChangeDestination(const OutputType t { LOCK(cs_wallet); - CTxDestination dest; - bilingual_str error; ReserveDestination reservedest(this, type); - if (!reservedest.GetReservedDestination(dest, true, error)) { - return util::Error{error}; - } + auto op_dest = reservedest.GetReservedDestination(true); + if (op_dest) reservedest.KeepDestination(); - reservedest.KeepDestination(); - return dest; + return op_dest; } std::optional CWallet::GetOldestKeyPoolTime() const @@ -2429,27 +2425,24 @@ std::set CWallet::ListAddrBookLabels(const std::string& purpose) co return label_set; } -bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool internal, bilingual_str& error) +util::Result ReserveDestination::GetReservedDestination(bool internal) { m_spk_man = pwallet->GetScriptPubKeyMan(type, internal); if (!m_spk_man) { - error = strprintf(_("Error: No %s addresses available."), FormatOutputType(type)); - return false; + return util::Error{strprintf(_("Error: No %s addresses available."), FormatOutputType(type))}; } - if (nIndex == -1) { m_spk_man->TopUp(); CKeyPool keypool; - if (!m_spk_man->GetReservedDestination(type, internal, address, nIndex, keypool, error)) { - return false; - } + auto op_address = m_spk_man->GetReservedDestination(type, internal, nIndex, keypool); + if (!op_address) return op_address; + address = *op_address; fInternal = keypool.fInternal; } - dest = address; - return true; + return address; } void ReserveDestination::KeepDestination() diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f2b9723840..ae7b9312ae 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -189,7 +189,7 @@ public: } //! Reserve an address - bool GetReservedDestination(CTxDestination& pubkey, bool internal, bilingual_str& error); + util::Result GetReservedDestination(bool internal); //! Return reserved address void ReturnDestination(); //! Keep the address. Do not return it's key to the keypool when this object goes out of scope