From 7819da2c1643e9ca892f0fc97ffc2003ac265dac Mon Sep 17 00:00:00 2001 From: stratospher <44024636+stratospher@users.noreply.github.com> Date: Wed, 28 Jan 2026 17:26:20 +0530 Subject: [PATCH] walllet: use CoinsResult instead of PreSelectedInputs PreSelectedInputs is confusing to use. it's `total_amount` might store total amount or effective amount based on SFFO. ex: we might accidentally sum preselected inputs effective amount (named `total_amount`) with automatically selected inputs actual total amount. CoinsResult has a cleaner interface with separate fields for both these amounts. 2 behavioural changes: 1. no more default assert error if effective value is unset - previously PreSelectedInputs::Insert() called COutput::GetEffectiveValue() which assert failed if the optional was unset. - now we don't default assert anymore. * in GUI/getAvailableBalance better not to assert. * SelectCoins's preselected inputs always contain a feerate, so effective amount should be set. explicitly added an assertion to ensure this. 2. FetchSelectedInputs uses OutputType::UNKNOWN as key to populate CoinsResult's coins map. it's discarded later. --- src/wallet/interfaces.cpp | 2 +- src/wallet/spend.cpp | 28 ++++++++++++++++---------- src/wallet/spend.h | 4 ++-- src/wallet/test/coinselector_tests.cpp | 8 ++++---- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 28c4d639323..8e0f240a6fe 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -411,7 +411,7 @@ public: CoinSelectionParams params(rng); // Note: for now, swallow any error. if (auto res = FetchSelectedInputs(*m_wallet, coin_control, params)) { - total_amount += res->total_amount; + total_amount += res->GetTotalAmount(); } } diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 30edd62a49c..675cf623337 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -266,10 +266,10 @@ static OutputType GetOutputType(TxoutType type, bool is_from_p2sh) // Fetch and validate the coin control selected inputs. // Coins could be internal (from the wallet) or external. -util::Result FetchSelectedInputs(const CWallet& wallet, const CCoinControl& coin_control, +util::Result FetchSelectedInputs(const CWallet& wallet, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) { - PreSelectedInputs result; + CoinsResult result; const bool can_grind_r = wallet.CanGrindR(); std::map map_of_bump_fees = wallet.chain().calculateIndividualBumpFees(coin_control.ListSelected(), coin_selection_params.m_effective_feerate); for (const COutPoint& outpoint : coin_control.ListSelected()) { @@ -312,7 +312,7 @@ util::Result FetchSelectedInputs(const CWallet& wallet, const /* Set some defaults for depth, solvable, safe, time, and from_me as these don't matter for preset inputs since no selection is being done. */ COutput output(outpoint, txout, /*depth=*/0, input_bytes, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/false, coin_selection_params.m_effective_feerate); output.ApplyBumpFee(map_of_bump_fees.at(output.outpoint)); - result.Insert(output, coin_selection_params.m_subtract_fee_outputs); + result.Add(OutputType::UNKNOWN, output); } return result; } @@ -812,12 +812,12 @@ util::Result ChooseSelectionResult(interfaces::Chain& chain, co return *std::min_element(results.begin(), results.end()); } -util::Result SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs, +util::Result SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const CoinsResult& pre_set_inputs, const CAmount& nTargetValue, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) { // Deduct preset inputs amount from the search target - CAmount selection_target = nTargetValue - pre_set_inputs.total_amount; + CAmount selection_target = nTargetValue - pre_set_inputs.GetAppropriateTotal(coin_selection_params.m_subtract_fee_outputs).value_or(0); // Return if automatic coin selection is disabled, and we don't cover the selection target if (!coin_control.m_allow_other_inputs && selection_target > 0) { @@ -825,18 +825,22 @@ util::Result SelectCoins(const CWallet& wallet, CoinsResult& av "Please allow other inputs to be automatically selected or include more coins manually")}; } + OutputSet preset_coin_set; + for (const auto& output: pre_set_inputs.All()) { + preset_coin_set.insert(std::make_shared(output)); + } + // Return if we can cover the target only with the preset inputs if (selection_target <= 0) { SelectionResult result(nTargetValue, SelectionAlgorithm::MANUAL); - result.AddInputs(pre_set_inputs.coins, coin_selection_params.m_subtract_fee_outputs); + result.AddInputs(preset_coin_set, coin_selection_params.m_subtract_fee_outputs); result.RecalculateWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); return result; } // Return early if we cannot cover the target with the wallet's UTXO. // We use the total effective value if we are not subtracting fee from outputs and 'available_coins' contains the data. - CAmount available_coins_total_amount = coin_selection_params.m_subtract_fee_outputs ? available_coins.GetTotalAmount() : - (available_coins.GetEffectiveTotalAmount().has_value() ? *available_coins.GetEffectiveTotalAmount() : 0); + CAmount available_coins_total_amount = available_coins.GetAppropriateTotal(coin_selection_params.m_subtract_fee_outputs).value_or(0); if (selection_target > available_coins_total_amount) { return util::Error(); // Insufficient funds } @@ -847,8 +851,10 @@ util::Result SelectCoins(const CWallet& wallet, CoinsResult& av // If needed, add preset inputs to the automatic coin selection result if (!pre_set_inputs.coins.empty()) { - SelectionResult preselected(pre_set_inputs.total_amount, SelectionAlgorithm::MANUAL); - preselected.AddInputs(pre_set_inputs.coins, coin_selection_params.m_subtract_fee_outputs); + auto preset_total = pre_set_inputs.GetAppropriateTotal(coin_selection_params.m_subtract_fee_outputs); + assert(preset_total.has_value()); + SelectionResult preselected(preset_total.value(), SelectionAlgorithm::MANUAL); + preselected.AddInputs(preset_coin_set, coin_selection_params.m_subtract_fee_outputs); op_selection_result->Merge(preselected); op_selection_result->RecalculateWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, @@ -1183,7 +1189,7 @@ static util::Result CreateTransactionInternal( } // Fetch manually selected coins - PreSelectedInputs preset_inputs; + CoinsResult preset_inputs; if (coin_control.HasSelected()) { auto res_fetch_inputs = FetchSelectedInputs(wallet, coin_control, coin_selection_params); if (!res_fetch_inputs) return util::Error{util::ErrorString(res_fetch_inputs)}; diff --git a/src/wallet/spend.h b/src/wallet/spend.h index c1657087e73..5a6ad246491 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -180,7 +180,7 @@ struct PreSelectedInputs * Fetch and validate coin control selected inputs. * Coins could be internal (from the wallet) or external. */ -util::Result FetchSelectedInputs(const CWallet& wallet, const CCoinControl& coin_control, +util::Result FetchSelectedInputs(const CWallet& wallet, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); /** @@ -202,7 +202,7 @@ util::Result AutomaticCoinSelection(const CWallet& wallet, Coin * Select all coins from coin_control, and if coin_control 'm_allow_other_inputs=true', call 'AutomaticCoinSelection' to * select a set of coins such that nTargetValue - pre_set_inputs.total_amount is met. */ -util::Result SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs, +util::Result SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const CoinsResult& pre_set_inputs, const CAmount& nTargetValue, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index b609ce823b2..f113532b6d4 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -222,8 +222,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) coin_control.m_allow_other_inputs = true; COutput select_coin = available_coins.All().at(0); coin_control.Select(select_coin.outpoint); - PreSelectedInputs selected_input; - selected_input.Insert(select_coin, coin_selection_params_bnb.m_subtract_fee_outputs); + CoinsResult selected_input; + selected_input.Add(OutputType::BECH32, select_coin); available_coins.Erase({available_coins.coins[OutputType::BECH32].begin()->outpoint}); LOCK(wallet->cs_wallet); @@ -253,8 +253,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) coin_control.m_allow_other_inputs = true; COutput select_coin = available_coins.All().at(1); // pre select 9 coin coin_control.Select(select_coin.outpoint); - PreSelectedInputs selected_input; - selected_input.Insert(select_coin, coin_selection_params_bnb.m_subtract_fee_outputs); + CoinsResult selected_input; + selected_input.Add(OutputType::BECH32, select_coin); available_coins.Erase({(++available_coins.coins[OutputType::BECH32].begin())->outpoint}); const auto result13 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(EquivalentResult(expected_result, *result13));