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/test | |
parent | 4a4289e2c98cfbc51b05716f21065838afed80f6 (diff) | |
download | bitcoin-a23cca56c0a7f4a267915b4beba3af3454c51603.tar.xz |
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/test')
-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 |
5 files changed, 16 insertions, 19 deletions
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)}); |