diff options
author | Ryan Ofsky <ryan@ofsky.org> | 2022-07-21 08:30:39 -0400 |
---|---|---|
committer | Ryan Ofsky <ryan@ofsky.org> | 2022-08-03 07:33:01 -0400 |
commit | a23cca56c0a7f4a267915b4beba3af3454c51603 (patch) | |
tree | 5d85193889f5a4552a568cfd2b472670c2c6d513 /src/wallet | |
parent | 4a4289e2c98cfbc51b05716f21065838afed80f6 (diff) |
refactor: Replace BResult with util::Result
Rename `BResult` class to `util::Result` and update the class interface to be
more compatible with `std::optional` and with a full-featured result class
implemented in https://github.com/bitcoin/bitcoin/pull/25665. Motivation for
this change is to update existing `BResult` usages now so they don't have to
change later when more features are added in #25665.
This change makes the following improvements originally implemented in #25665:
- More explicit API. Drops potentially misleading `BResult` constructor that
treats any bilingual string argument as an error. Adds `util::Error`
constructor so it is never ambiguous when a result is being assigned an error
or non-error value.
- Better type compatibility. Supports `util::Result<bilingual_str>` return
values to hold translated messages which are not errors.
- More standard and consistent API. `util::Result` supports most of the same
operators and methods as `std::optional`. `BResult` had a less familiar
interface with `HasRes`/`GetObj`/`ReleaseObj` methods. The Result/Res/Obj
naming was also not internally consistent.
- Better code organization. Puts `src/util/` code in the `util::` namespace so
naming reflects code organization and it is obvious where the class is coming
from. Drops "B" from name because it is undocumented what it stands for
(bilingual?)
- Has unit tests.
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/feebumper.cpp | 4 | ||||
-rw-r--r-- | src/wallet/interfaces.cpp | 16 | ||||
-rw-r--r-- | src/wallet/rpc/addresses.cpp | 8 | ||||
-rw-r--r-- | src/wallet/rpc/spend.cpp | 6 | ||||
-rw-r--r-- | src/wallet/scriptpubkeyman.cpp | 20 | ||||
-rw-r--r-- | src/wallet/scriptpubkeyman.h | 6 | ||||
-rw-r--r-- | src/wallet/spend.cpp | 50 | ||||
-rw-r--r-- | src/wallet/spend.h | 2 | ||||
-rw-r--r-- | src/wallet/test/availablecoins_tests.cpp | 18 | ||||
-rw-r--r-- | src/wallet/test/coinselector_tests.cpp | 4 | ||||
-rw-r--r-- | src/wallet/test/fuzz/notifications.cpp | 5 | ||||
-rw-r--r-- | src/wallet/test/spend_tests.cpp | 2 | ||||
-rw-r--r-- | src/wallet/test/wallet_tests.cpp | 6 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 10 | ||||
-rw-r--r-- | src/wallet/wallet.h | 4 |
15 files changed, 78 insertions, 83 deletions
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 43fdcee48d..bda4c76bea 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -221,11 +221,11 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo constexpr int RANDOM_CHANGE_POSITION = -1; auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, new_coin_control, false); if (!res) { - errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + res.GetError()); + errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + util::ErrorString(res)); return Result::WALLET_ERROR; } - const auto& txr = res.GetObj(); + const auto& txr = *res; // Write back new fee if successful new_fee = txr.fee; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 4f30060e29..91b188a59c 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -148,7 +148,7 @@ 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(); } - BResult<CTxDestination> getNewDestination(const OutputType type, const std::string label) override + util::Result<CTxDestination> getNewDestination(const OutputType type, const std::string label) override { LOCK(m_wallet->cs_wallet); return m_wallet->GetNewDestination(type, label); @@ -251,17 +251,17 @@ public: LOCK(m_wallet->cs_wallet); return m_wallet->ListLockedCoins(outputs); } - BResult<CTransactionRef> createTransaction(const std::vector<CRecipient>& recipients, + util::Result<CTransactionRef> createTransaction(const std::vector<CRecipient>& recipients, const CCoinControl& coin_control, bool sign, int& change_pos, CAmount& fee) override { LOCK(m_wallet->cs_wallet); - const auto& res = CreateTransaction(*m_wallet, recipients, change_pos, + auto res = CreateTransaction(*m_wallet, recipients, change_pos, coin_control, sign); - if (!res) return res.GetError(); - const auto& txr = res.GetObj(); + if (!res) return util::Error{util::ErrorString(res)}; + const auto& txr = *res; fee = txr.fee; change_pos = txr.change_pos; @@ -571,12 +571,12 @@ public: options.require_existing = true; return MakeWallet(m_context, LoadWallet(m_context, name, true /* load_on_start */, options, status, error, warnings)); } - BResult<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) override + util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) override { DatabaseStatus status; bilingual_str error; - BResult<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings))}; - if (!wallet) return error; + util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings))}; + if (!wallet) return util::Error{error}; return wallet; } std::string getWalletDir() override diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 2a6eb96871..148343a8b0 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -60,10 +60,10 @@ RPCHelpMan getnewaddress() auto op_dest = pwallet->GetNewDestination(output_type, label); if (!op_dest) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original); + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, util::ErrorString(op_dest).original); } - return EncodeDestination(op_dest.GetObj()); + return EncodeDestination(*op_dest); }, }; } @@ -107,9 +107,9 @@ RPCHelpMan getrawchangeaddress() auto op_dest = pwallet->GetNewChangeDestination(output_type); if (!op_dest) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original); + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, util::ErrorString(op_dest).original); } - return EncodeDestination(op_dest.GetObj()); + return EncodeDestination(*op_dest); }, }; } diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index e27cb1139e..628bf3cc99 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -158,14 +158,14 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto constexpr int RANDOM_CHANGE_POSITION = -1; auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, coin_control, true); if (!res) { - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, res.GetError().original); + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, util::ErrorString(res).original); } - const CTransactionRef& tx = res.GetObj().tx; + const CTransactionRef& tx = res->tx; wallet.CommitTransaction(tx, std::move(map_value), {} /* orderForm */); if (verbose) { UniValue entry(UniValue::VOBJ); entry.pushKV("txid", tx->GetHash().GetHex()); - entry.pushKV("fee_reason", StringForFeeReason(res.GetObj().fee_calc.reason)); + entry.pushKV("fee_reason", StringForFeeReason(res->fee_calc.reason)); return entry; } return tx->GetHash().GetHex(); diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index bba31dfe0b..2f242901ab 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -21,10 +21,10 @@ 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; -BResult<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputType type) +util::Result<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputType type) { if (LEGACY_OUTPUT_TYPES.count(type) == 0) { - return _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types");; + return util::Error{_("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types")}; } assert(type != OutputType::BECH32M); @@ -33,7 +33,7 @@ BResult<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputTyp // Generate a new key that is added to wallet CPubKey new_key; if (!GetKeyFromPool(new_key, type)) { - return _("Error: Keypool ran out, please call keypoolrefill first"); + return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}; } LearnRelatedScripts(new_key, type); return GetDestinationForKey(new_key, type); @@ -1654,11 +1654,11 @@ std::set<CKeyID> LegacyScriptPubKeyMan::GetKeys() const return set_address; } -BResult<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type) +util::Result<CTxDestination> 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()) { - return _("No addresses available"); + return util::Error{_("No addresses available")}; } { LOCK(cs_desc_man); @@ -1676,11 +1676,11 @@ BResult<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const Outpu std::vector<CScript> scripts_temp; if (m_wallet_descriptor.range_end <= m_max_cached_index && !TopUp(1)) { // We can't generate anymore keys - return _("Error: Keypool ran out, please call keypoolrefill first"); + return util::Error{_("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 - return _("Error: Keypool ran out, please call keypoolrefill first"); + return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}; } CTxDestination dest; @@ -1766,11 +1766,11 @@ bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bo auto op_dest = GetNewDestination(type); index = m_wallet_descriptor.next_index - 1; if (op_dest) { - address = op_dest.GetObj(); + address = *op_dest; } else { - error = op_dest.GetError(); + error = util::ErrorString(op_dest); } - return op_dest.HasRes(); + return bool(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 eebc05330f..b772dde533 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -172,7 +172,7 @@ protected: public: explicit ScriptPubKeyMan(WalletStorage& storage) : m_storage(storage) {} virtual ~ScriptPubKeyMan() {}; - virtual BResult<CTxDestination> GetNewDestination(const OutputType type) { return Untranslated("Not supported"); } + virtual util::Result<CTxDestination> GetNewDestination(const OutputType type) { return util::Error{Untranslated("Not supported")}; } virtual isminetype IsMine(const CScript& script) const { return ISMINE_NO; } //! Check that the given decryption key is valid for this ScriptPubKeyMan, i.e. it decrypts all of the keys handled by it. @@ -360,7 +360,7 @@ private: public: using ScriptPubKeyMan::ScriptPubKeyMan; - BResult<CTxDestination> GetNewDestination(const OutputType type) override; + util::Result<CTxDestination> GetNewDestination(const OutputType type) override; isminetype IsMine(const CScript& script) const override; bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; @@ -568,7 +568,7 @@ public: mutable RecursiveMutex cs_desc_man; - BResult<CTxDestination> GetNewDestination(const OutputType type) override; + util::Result<CTxDestination> GetNewDestination(const OutputType type) override; isminetype IsMine(const CScript& script) const override; bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index c00a2cd023..5d320feff0 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -758,7 +758,7 @@ static void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng } } -static BResult<CreatedTransactionResult> CreateTransactionInternal( +static util::Result<CreatedTransactionResult> CreateTransactionInternal( CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, @@ -844,11 +844,11 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( // Do not, ever, assume that it's fine to change the fee rate if the user has explicitly // provided one if (coin_control.m_feerate && coin_selection_params.m_effective_feerate > *coin_control.m_feerate) { - return strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB)); + return util::Error{strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB))}; } if (feeCalc.reason == FeeReason::FALLBACK && !wallet.m_allow_fallback_fee) { // eventually allow a fallback fee - return _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); + return util::Error{_("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.")}; } // Calculate the cost of change @@ -874,7 +874,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( } if (IsDust(txout, wallet.chain().relayDustFee())) { - return _("Transaction amount too small"); + return util::Error{_("Transaction amount too small")}; } txNew.vout.push_back(txout); } @@ -895,7 +895,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( // Choose coins to use std::optional<SelectionResult> result = SelectCoins(wallet, available_coins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); if (!result) { - return _("Insufficient funds"); + return util::Error{_("Insufficient funds")}; } TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result->m_algo).c_str(), result->m_target, result->GetWaste(), result->GetSelectedValue()); @@ -910,7 +910,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( nChangePosInOut = rng_fast.randrange(txNew.vout.size() + 1); } else if ((unsigned int)nChangePosInOut > txNew.vout.size()) { - return _("Transaction change output index out of range"); + return util::Error{_("Transaction change output index out of range")}; } assert(nChangePosInOut != -1); @@ -937,7 +937,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( TxSize tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), &wallet, &coin_control); int nBytes = tx_sizes.vsize; if (nBytes == -1) { - return _("Missing solving data for estimating transaction size"); + return util::Error{_("Missing solving data for estimating transaction size")}; } nFeeRet = coin_selection_params.m_effective_feerate.GetFee(nBytes); @@ -997,9 +997,9 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( // Error if this output is reduced to be below dust if (IsDust(txout, wallet.chain().relayDustFee())) { if (txout.nValue < 0) { - return _("The transaction amount is too small to pay the fee"); + return util::Error{_("The transaction amount is too small to pay the fee")}; } else { - return _("The transaction amount is too small to send after the fee has been deducted"); + return util::Error{_("The transaction amount is too small to send after the fee has been deducted")}; } } } @@ -1010,11 +1010,11 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( // Give up if change keypool ran out and change is required if (scriptChange.empty() && nChangePosInOut != -1) { - return error; + return util::Error{error}; } if (sign && !wallet.SignTransaction(txNew)) { - return _("Signing transaction failed"); + return util::Error{_("Signing transaction failed")}; } // Return the constructed transaction data. @@ -1024,17 +1024,17 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( if ((sign && GetTransactionWeight(*tx) > MAX_STANDARD_TX_WEIGHT) || (!sign && tx_sizes.weight > MAX_STANDARD_TX_WEIGHT)) { - return _("Transaction too large"); + return util::Error{_("Transaction too large")}; } if (nFeeRet > wallet.m_default_max_tx_fee) { - return TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED); + return util::Error{TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED)}; } if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits if (!wallet.chain().checkChainLimits(tx)) { - return _("Transaction has too long of a mempool chain"); + return util::Error{_("Transaction has too long of a mempool chain")}; } } @@ -1053,7 +1053,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( return CreatedTransactionResult(tx, nFeeRet, nChangePosInOut, feeCalc); } -BResult<CreatedTransactionResult> CreateTransaction( +util::Result<CreatedTransactionResult> CreateTransaction( CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, @@ -1061,28 +1061,26 @@ BResult<CreatedTransactionResult> CreateTransaction( bool sign) { if (vecSend.empty()) { - return _("Transaction must have at least one recipient"); + return util::Error{_("Transaction must have at least one recipient")}; } if (std::any_of(vecSend.cbegin(), vecSend.cend(), [](const auto& recipient){ return recipient.nAmount < 0; })) { - return _("Transaction amounts must not be negative"); + return util::Error{_("Transaction amounts must not be negative")}; } LOCK(wallet.cs_wallet); auto res = CreateTransactionInternal(wallet, vecSend, change_pos, coin_control, sign); - TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), res.HasRes(), - res ? res.GetObj().fee : 0, res ? res.GetObj().change_pos : 0); + TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), bool(res), + res ? res->fee : 0, res ? res->change_pos : 0); if (!res) return res; - const auto& txr_ungrouped = res.GetObj(); + const auto& txr_ungrouped = *res; // try with avoidpartialspends unless it's enabled already if (txr_ungrouped.fee > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) { TRACE1(coin_selection, attempting_aps_create_tx, wallet.GetName().c_str()); CCoinControl tmp_cc = coin_control; tmp_cc.m_avoid_partial_spends = true; - auto res_tx_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, tmp_cc, sign); - // Helper optional class for now - std::optional<CreatedTransactionResult> txr_grouped{res_tx_grouped.HasRes() ? std::make_optional(res_tx_grouped.GetObj()) : std::nullopt}; + auto txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, tmp_cc, sign); // if fee of this alternative one is within the range of the max fee, we use this one const bool use_aps{txr_grouped.has_value() ? (txr_grouped->fee <= txr_ungrouped.fee + wallet.m_max_aps_fee) : false}; TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, txr_grouped.has_value(), @@ -1090,7 +1088,7 @@ BResult<CreatedTransactionResult> CreateTransaction( if (txr_grouped) { wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", txr_ungrouped.fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped"); - if (use_aps) return res_tx_grouped; + if (use_aps) return txr_grouped; } } return res; @@ -1131,10 +1129,10 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, auto res = CreateTransaction(wallet, vecSend, nChangePosInOut, coinControl, false); if (!res) { - error = res.GetError(); + error = util::ErrorString(res); return false; } - const auto& txr = res.GetObj(); + const auto& txr = *res; CTransactionRef tx_new = txr.tx; nFeeRet = txr.fee; nChangePosInOut = txr.change_pos; diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 96ecd690be..fdb5113ba4 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -155,7 +155,7 @@ struct CreatedTransactionResult * selected by SelectCoins(); Also create the change output, when needed * @note passing change_pos as -1 will result in setting a random position */ -BResult<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, const CCoinControl& coin_control, bool sign = true); +util::Result<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, const CCoinControl& coin_control, bool sign = true); /** * Insert additional inputs into the transaction by diff --git a/src/wallet/test/availablecoins_tests.cpp b/src/wallet/test/availablecoins_tests.cpp index 01d24da981..3356128112 100644 --- a/src/wallet/test/availablecoins_tests.cpp +++ b/src/wallet/test/availablecoins_tests.cpp @@ -33,7 +33,7 @@ public: constexpr int RANDOM_CHANGE_POSITION = -1; auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, dummy); BOOST_CHECK(res); - tx = res.GetObj().tx; + tx = res->tx; } wallet->CommitTransaction(tx, {}, {}); CMutableTransaction blocktx; @@ -57,7 +57,7 @@ public: BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup) { CoinsResult available_coins; - BResult<CTxDestination> dest; + util::Result<CTxDestination> dest{util::Error{}}; LOCK(wallet->cs_wallet); // Verify our wallet has one usable coinbase UTXO before starting @@ -75,28 +75,28 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup) // Bech32m dest = wallet->GetNewDestination(OutputType::BECH32M, ""); - BOOST_ASSERT(dest.HasRes()); - AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 1 * COIN, /*fSubtractFeeFromAmount=*/true}); + BOOST_ASSERT(dest); + AddTx(CRecipient{{GetScriptForDestination(*dest)}, 1 * COIN, /*fSubtractFeeFromAmount=*/true}); available_coins = AvailableCoins(*wallet); BOOST_CHECK_EQUAL(available_coins.bech32m.size(), 2U); // Bech32 dest = wallet->GetNewDestination(OutputType::BECH32, ""); - BOOST_ASSERT(dest.HasRes()); - AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 2 * COIN, /*fSubtractFeeFromAmount=*/true}); + BOOST_ASSERT(dest); + AddTx(CRecipient{{GetScriptForDestination(*dest)}, 2 * COIN, /*fSubtractFeeFromAmount=*/true}); available_coins = AvailableCoins(*wallet); BOOST_CHECK_EQUAL(available_coins.bech32.size(), 2U); // P2SH-SEGWIT dest = wallet->GetNewDestination(OutputType::P2SH_SEGWIT, ""); - AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 3 * COIN, /*fSubtractFeeFromAmount=*/true}); + AddTx(CRecipient{{GetScriptForDestination(*dest)}, 3 * COIN, /*fSubtractFeeFromAmount=*/true}); available_coins = AvailableCoins(*wallet); BOOST_CHECK_EQUAL(available_coins.P2SH_segwit.size(), 2U); // Legacy (P2PKH) dest = wallet->GetNewDestination(OutputType::LEGACY, ""); - BOOST_ASSERT(dest.HasRes()); - AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 4 * COIN, /*fSubtractFeeFromAmount=*/true}); + BOOST_ASSERT(dest); + AddTx(CRecipient{{GetScriptForDestination(*dest)}, 4 * COIN, /*fSubtractFeeFromAmount=*/true}); available_coins = AvailableCoins(*wallet); BOOST_CHECK_EQUAL(available_coins.legacy.size(), 2U); } diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index cd7fd3f4dd..a4a7216436 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -74,9 +74,7 @@ static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmoun tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; if (spendable) { - auto op_dest = wallet.GetNewDestination(OutputType::BECH32, ""); - assert(op_dest.HasRes()); - tx.vout[nInput].scriptPubKey = GetScriptForDestination(op_dest.GetObj()); + tx.vout[nInput].scriptPubKey = GetScriptForDestination(*Assert(wallet.GetNewDestination(OutputType::BECH32, ""))); } uint256 txid = tx.GetHash(); diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp index 5c173773e4..5e9cd4001b 100644 --- a/src/wallet/test/fuzz/notifications.cpp +++ b/src/wallet/test/fuzz/notifications.cpp @@ -69,14 +69,13 @@ struct FuzzedWallet { CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider) { auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)}; - BResult<CTxDestination> op_dest; + util::Result<CTxDestination> op_dest{util::Error{}}; if (fuzzed_data_provider.ConsumeBool()) { op_dest = wallet->GetNewDestination(type, ""); } else { op_dest = wallet->GetNewChangeDestination(type); } - assert(op_dest.HasRes()); - return GetScriptForDestination(op_dest.GetObj()); + return GetScriptForDestination(*Assert(op_dest)); } }; diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index 53c3b5d2ae..dc935a1a04 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -35,7 +35,7 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) coin_control.m_change_type = OutputType::LEGACY; auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, coin_control); BOOST_CHECK(res); - const auto& txr = res.GetObj(); + const auto& txr = *res; BOOST_CHECK_EQUAL(txr.tx->vout.size(), 1); BOOST_CHECK_EQUAL(txr.tx->vout[0].nValue, recipient.nAmount + leftover_input_amount - txr.fee); BOOST_CHECK_GT(txr.fee, 0); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index d3a760742d..1d2235be3d 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -536,7 +536,7 @@ public: constexpr int RANDOM_CHANGE_POSITION = -1; auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, dummy); BOOST_CHECK(res); - tx = res.GetObj().tx; + tx = res->tx; } wallet->CommitTransaction(tx, {}, {}); CMutableTransaction blocktx; @@ -918,8 +918,8 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestingSetup) // Add tx to wallet const auto& op_dest = wallet.GetNewDestination(OutputType::BECH32M, ""); - BOOST_ASSERT(op_dest.HasRes()); - const CTxDestination& dest = op_dest.GetObj(); + BOOST_ASSERT(op_dest); + const CTxDestination& dest = *op_dest; CMutableTransaction mtx; mtx.vout.push_back({COIN, GetScriptForDestination(dest)}); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9a48a1ca21..e750cd5a2c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2330,24 +2330,24 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize) return res; } -BResult<CTxDestination> CWallet::GetNewDestination(const OutputType type, const std::string label) +util::Result<CTxDestination> CWallet::GetNewDestination(const OutputType type, const std::string label) { LOCK(cs_wallet); auto spk_man = GetScriptPubKeyMan(type, false /* internal */); if (!spk_man) { - return strprintf(_("Error: No %s addresses available."), FormatOutputType(type)); + return util::Error{strprintf(_("Error: No %s addresses available."), FormatOutputType(type))}; } spk_man->TopUp(); auto op_dest = spk_man->GetNewDestination(type); if (op_dest) { - SetAddressBook(op_dest.GetObj(), label, "receive"); + SetAddressBook(*op_dest, label, "receive"); } return op_dest; } -BResult<CTxDestination> CWallet::GetNewChangeDestination(const OutputType type) +util::Result<CTxDestination> CWallet::GetNewChangeDestination(const OutputType type) { LOCK(cs_wallet); @@ -2355,7 +2355,7 @@ BResult<CTxDestination> CWallet::GetNewChangeDestination(const OutputType type) bilingual_str error; ReserveDestination reservedest(this, type); if (!reservedest.GetReservedDestination(dest, true, error)) { - return error; + return util::Error{error}; } reservedest.KeepDestination(); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 20f0c3579c..f2b9723840 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -670,8 +670,8 @@ public: */ void MarkDestinationsDirty(const std::set<CTxDestination>& destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - BResult<CTxDestination> GetNewDestination(const OutputType type, const std::string label); - BResult<CTxDestination> GetNewChangeDestination(const OutputType type); + util::Result<CTxDestination> GetNewDestination(const OutputType type, const std::string label); + util::Result<CTxDestination> GetNewChangeDestination(const OutputType type); isminetype IsMine(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); isminetype IsMine(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); |