From 111ea3ab711414236f8678566a7884d48619b2d8 Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 24 May 2022 21:59:54 -0300 Subject: wallet: refactor GetNewDestination, use BResult --- src/bench/wallet_loading.cpp | 7 +++---- src/interfaces/wallet.h | 2 +- src/qt/addresstablemodel.cpp | 14 ++++++-------- src/test/util/wallet.cpp | 7 +++---- src/wallet/interfaces.cpp | 5 ++--- src/wallet/rpc/addresses.cpp | 18 ++++++++--------- src/wallet/scriptpubkeyman.cpp | 35 +++++++++++++++++----------------- src/wallet/scriptpubkeyman.h | 7 ++++--- src/wallet/test/coinselector_tests.cpp | 8 +++----- src/wallet/test/fuzz/notifications.cpp | 11 +++++------ src/wallet/test/wallet_tests.cpp | 8 ++------ src/wallet/wallet.cpp | 29 ++++++++++++++-------------- src/wallet/wallet.h | 5 +++-- 13 files changed, 71 insertions(+), 85 deletions(-) (limited to 'src') diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp index f611383788..d258d7a29e 100644 --- a/src/bench/wallet_loading.cpp +++ b/src/bench/wallet_loading.cpp @@ -47,12 +47,11 @@ static void BenchUnloadWallet(std::shared_ptr&& wallet) static void AddTx(CWallet& wallet) { - bilingual_str error; - CTxDestination dest; - wallet.GetNewDestination(OutputType::BECH32, "", dest, error); + const auto& dest = wallet.GetNewDestination(OutputType::BECH32, ""); + assert(dest.HasRes()); CMutableTransaction mtx; - mtx.vout.push_back({COIN, GetScriptForDestination(dest)}); + mtx.vout.push_back({COIN, GetScriptForDestination(dest.GetObj())}); mtx.vin.push_back(CTxIn()); wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{}); diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 00d238f186..fe198c999b 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -88,7 +88,7 @@ public: virtual std::string getWalletName() = 0; // Get a new address. - virtual bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) = 0; + virtual BResult getNewDestination(const OutputType type, const std::string label) = 0; //! Get public key. virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0; diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index 27ee9509e6..bb50b5384a 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -370,23 +370,21 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con else if(type == Receive) { // Generate a new address to associate with given label - CTxDestination dest; - if(!walletModel->wallet().getNewDestination(address_type, strLabel, dest)) - { + auto op_dest = walletModel->wallet().getNewDestination(address_type, strLabel); + if (!op_dest) { WalletModel::UnlockContext ctx(walletModel->requestUnlock()); - if(!ctx.isValid()) - { + if (!ctx.isValid()) { // Unlock wallet failed or was cancelled editStatus = WALLET_UNLOCK_FAILURE; return QString(); } - if(!walletModel->wallet().getNewDestination(address_type, strLabel, dest)) - { + op_dest = walletModel->wallet().getNewDestination(address_type, strLabel); + if (!op_dest) { editStatus = KEY_GENERATION_FAILURE; return QString(); } } - strAddress = EncodeDestination(dest); + strAddress = EncodeDestination(op_dest.GetObj()); } else { diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp index 52aaeabccf..7a00ac9e1f 100644 --- a/src/test/util/wallet.cpp +++ b/src/test/util/wallet.cpp @@ -20,11 +20,10 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq std::string getnewaddress(CWallet& w) { constexpr auto output_type = OutputType::BECH32; - CTxDestination dest; - bilingual_str error; - if (!w.GetNewDestination(output_type, "", dest, error)) assert(false); + auto op_dest = w.GetNewDestination(output_type, ""); + assert(op_dest.HasRes()); - return EncodeDestination(dest); + return EncodeDestination(op_dest.GetObj()); } #endif // ENABLE_WALLET diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 424d441488..547d972c8d 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -146,11 +146,10 @@ public: void abortRescan() override { m_wallet->AbortRescan(); } bool backupWallet(const std::string& filename) override { return m_wallet->BackupWallet(filename); } std::string getWalletName() override { return m_wallet->GetName(); } - bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) override + BResult getNewDestination(const OutputType type, const std::string label) override { LOCK(m_wallet->cs_wallet); - bilingual_str error; - return m_wallet->GetNewDestination(type, label, dest, error); + return m_wallet->GetNewDestination(type, label); } bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) override { diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index da4cc44ee6..9428d049de 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -58,13 +58,12 @@ RPCHelpMan getnewaddress() output_type = parsed.value(); } - CTxDestination dest; - bilingual_str error; - if (!pwallet->GetNewDestination(output_type, label, dest, error)) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original); + auto op_dest = pwallet->GetNewDestination(output_type, label); + if (!op_dest) { + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original); } - return EncodeDestination(dest); + return EncodeDestination(op_dest.GetObj()); }, }; } @@ -106,12 +105,11 @@ RPCHelpMan getrawchangeaddress() output_type = parsed.value(); } - CTxDestination dest; - bilingual_str error; - if (!pwallet->GetNewChangeDestination(output_type, dest, error)) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original); + auto op_dest = pwallet->GetNewChangeDestination(output_type); + if (!op_dest) { + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original); } - return EncodeDestination(dest); + return EncodeDestination(op_dest.GetObj()); }, }; } diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 1fec82a485..bba31dfe0b 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -21,26 +21,22 @@ namespace wallet { //! Value for the first BIP 32 hardened derivation. Can be used as a bit mask and as a value. See BIP 32 for more details. const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; -bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) +BResult LegacyScriptPubKeyMan::GetNewDestination(const OutputType type) { if (LEGACY_OUTPUT_TYPES.count(type) == 0) { - error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types"); - return false; + return _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types");; } assert(type != OutputType::BECH32M); LOCK(cs_KeyStore); - error.clear(); // Generate a new key that is added to wallet CPubKey new_key; if (!GetKeyFromPool(new_key, type)) { - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return _("Error: Keypool ran out, please call keypoolrefill first"); } LearnRelatedScripts(new_key, type); - dest = GetDestinationForKey(new_key, type); - return true; + return GetDestinationForKey(new_key, type); } typedef std::vector valtype; @@ -1658,12 +1654,11 @@ std::set LegacyScriptPubKeyMan::GetKeys() const return set_address; } -bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) +BResult DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type) { // Returns true if this descriptor supports getting new addresses. Conditions where we may be unable to fetch them (e.g. locked) are caught later if (!CanGetAddresses()) { - error = _("No addresses available"); - return false; + return _("No addresses available"); } { LOCK(cs_desc_man); @@ -1681,15 +1676,14 @@ bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDest std::vector scripts_temp; if (m_wallet_descriptor.range_end <= m_max_cached_index && !TopUp(1)) { // We can't generate anymore keys - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return _("Error: Keypool ran out, please call keypoolrefill first"); } if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.next_index, m_wallet_descriptor.cache, scripts_temp, out_keys)) { // We can't generate anymore keys - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return _("Error: Keypool ran out, please call keypoolrefill first"); } + CTxDestination dest; std::optional out_script_type = m_wallet_descriptor.descriptor->GetOutputType(); if (out_script_type && out_script_type == type) { ExtractDestination(scripts_temp[0], dest); @@ -1698,7 +1692,7 @@ bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDest } m_wallet_descriptor.next_index++; WalletBatch(m_storage.GetDatabase()).WriteDescriptor(GetID(), m_wallet_descriptor); - return true; + return dest; } } @@ -1769,9 +1763,14 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) { LOCK(cs_desc_man); - bool result = GetNewDestination(type, address, error); + auto op_dest = GetNewDestination(type); index = m_wallet_descriptor.next_index - 1; - return result; + if (op_dest) { + address = op_dest.GetObj(); + } else { + error = op_dest.GetError(); + } + return op_dest.HasRes(); } void DescriptorScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index ad924791af..eebc05330f 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -11,6 +11,7 @@ #include