diff options
author | Ryan Ofsky <ryan@ofsky.org> | 2022-09-15 11:15:06 -0400 |
---|---|---|
committer | Ryan Ofsky <ryan@ofsky.org> | 2024-04-25 16:08:24 -0400 |
commit | 834f65e82405bbed336f98996bc8cef366bbed0f (patch) | |
tree | 2ed28b93420df882aee09cd69464ce3775bfcebc /src/wallet | |
parent | 2eff198f4900c34442439ef2cbd9d82f4903f915 (diff) | |
download | bitcoin-834f65e82405bbed336f98996bc8cef366bbed0f.tar.xz |
refactor: Drop util::Result operator=
`util::Result` objects are aggregates that can hold multiple fields with
different information. Currently Result objects can only hold a success value
of an arbitrary type or a single bilingual_str error message. In followup PR
https://github.com/bitcoin/bitcoin/pull/25722, Result objects may be able to
hold both success and failure values of different types, plus error and warning
messages.
Having a Result::operator= assignment operator that completely erases all
existing Result information before assigning new information is potentially
dangerous in this case. For example, code that looks like it is assigning a
warning value could erase previously-assigned success or failure values.
Conversely, code that looks like it is just assigning a success or failure
value could erase previously assigned error and warning messages.
To prevent potential bugs like this, disable Result::operator= assignment
operator.
It is possible in the future we may want to re-enable operator= in limited
cases (such as when implicit conversions are not used) or add a Replace() or
Reset() method that mimicks default operator= behavior. Followup PR
https://github.com/bitcoin/bitcoin/pull/25722 also adds a Result::Update()
method providing another way to update an existing Result object.
Co-authored-by: stickies-v <stickies-v@protonmail.com>
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/test/fuzz/notifications.cpp | 6 | ||||
-rw-r--r-- | src/wallet/test/spend_tests.cpp | 6 |
2 files changed, 4 insertions, 8 deletions
diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp index 9a515828fe..792079e6c6 100644 --- a/src/wallet/test/fuzz/notifications.cpp +++ b/src/wallet/test/fuzz/notifications.cpp @@ -106,13 +106,11 @@ struct FuzzedWallet { CTxDestination GetDestination(FuzzedDataProvider& fuzzed_data_provider) { auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)}; - util::Result<CTxDestination> op_dest{util::Error{}}; if (fuzzed_data_provider.ConsumeBool()) { - op_dest = wallet->GetNewDestination(type, ""); + return *Assert(wallet->GetNewDestination(type, "")); } else { - op_dest = wallet->GetNewChangeDestination(type); + return *Assert(wallet->GetNewChangeDestination(type)); } - return *Assert(op_dest); } CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider) { return GetScriptForDestination(GetDestination(fuzzed_data_provider)); } void FundTx(FuzzedDataProvider& fuzzed_data_provider, CMutableTransaction tx) diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index 3509bc116f..963c0f838b 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -97,13 +97,11 @@ BOOST_FIXTURE_TEST_CASE(wallet_duplicated_preset_inputs_test, TestChain100Setup) // so that the recipient's amount is no longer equal to the user's selected target of 299 BTC. // First case, use 'subtract_fee_from_outputs=true' - util::Result<CreatedTransactionResult> res_tx = CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control); - BOOST_CHECK(!res_tx.has_value()); + BOOST_CHECK(!CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control)); // Second case, don't use 'subtract_fee_from_outputs'. recipients[0].fSubtractFeeFromAmount = false; - res_tx = CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control); - BOOST_CHECK(!res_tx.has_value()); + BOOST_CHECK(!CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control)); } BOOST_AUTO_TEST_SUITE_END() |