From 05300c14392facf38330eb4fcd8e695a838b76f3 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 21 May 2021 18:55:21 -0400 Subject: Use SelectionResult in SelectCoins Replace setCoinsRet and nValueRet with SelectionResult --- src/wallet/spend.cpp | 46 ++++++++++++++++------------------ src/wallet/spend.h | 17 +++++++------ src/wallet/test/coinselector_tests.cpp | 12 ++++----- 3 files changed, 37 insertions(+), 38 deletions(-) (limited to 'src/wallet') diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 642eb175e8..5d9cc7bf6b 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -414,27 +414,31 @@ std::optional AttemptSelection(const CWallet& wallet, const CAm return best_result; } -bool SelectCoins(const CWallet& wallet, const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params) +std::optional SelectCoins(const CWallet& wallet, const std::vector& vAvailableCoins, const CAmount& nTargetValue, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params) { std::vector vCoins(vAvailableCoins); CAmount value_to_select = nTargetValue; + OutputGroup preset_inputs(coin_selection_params); + // coin control -> return all selected outputs (we want all selected to go into the transaction for sure) if (coin_control.HasSelected() && !coin_control.fAllowOtherInputs) { - for (const COutput& out : vCoins) - { - if (!out.fSpendable) - continue; - nValueRet += out.tx->tx->vout[out.i].nValue; - setCoinsRet.insert(out.GetInputCoin()); + for (const COutput& out : vCoins) { + if (!out.fSpendable) continue; + /* Set depth, from_me, ancestors, and descendants to 0 or false as these don't matter for preset inputs as no actual selection is being done. + * positive_only is set to false because we want to include all preset inputs, even if they are dust. + */ + preset_inputs.Insert(out.GetInputCoin(), 0, false, 0, 0, false); } - return (nValueRet >= nTargetValue); + SelectionResult result(nTargetValue); + result.AddInput(preset_inputs); + if (result.GetSelectedValue() < nTargetValue) return std::nullopt; + return result; } // calculate value from preset inputs and store them std::set setPresetCoins; - OutputGroup preset_inputs(coin_selection_params); std::vector vPresetInputs; coin_control.ListSelected(vPresetInputs); @@ -446,7 +450,7 @@ bool SelectCoins(const CWallet& wallet, const std::vector& vAvailableCo const CWalletTx& wtx = it->second; // Clearly invalid input, fail if (wtx.tx->vout.size() <= outpoint.n) { - return false; + return std::nullopt; } input_bytes = GetTxSpendSize(wallet, wtx, outpoint.n, false); txout = wtx.tx->vout.at(outpoint.n); @@ -455,14 +459,14 @@ bool SelectCoins(const CWallet& wallet, const std::vector& vAvailableCo // The input is external. We either did not find the tx in mapWallet, or we did but couldn't compute the input size with wallet data if (!coin_control.GetExternalOutput(outpoint, txout)) { // Not ours, and we don't have solving data. - return false; + return std::nullopt; } input_bytes = CalculateMaximumSignedInputSize(txout, &coin_control.m_external_provider, /* use_max_sig */ true); } CInputCoin coin(outpoint, txout, input_bytes); if (coin.m_input_bytes == -1) { - return false; // Not solvable, can't estimate size for fee + return std::nullopt; // Not solvable, can't estimate size for fee } coin.effective_value = coin.txout.nValue - coin_selection_params.m_effective_feerate.GetFee(coin.m_input_bytes); if (coin_selection_params.m_subtract_fee_outputs) { @@ -557,15 +561,12 @@ bool SelectCoins(const CWallet& wallet, const std::vector& vAvailableCo return std::optional(); }(); - if (!res) return false; + if (!res) return std::nullopt; // Add preset inputs to result res->AddInput(preset_inputs); - setCoinsRet = res->GetInputSet(); - nValueRet = res->GetSelectedValue(); - - return true; + return res; } static bool IsCurrentForAntiFeeSniping(interfaces::Chain& chain, const uint256& block_hash) @@ -761,17 +762,15 @@ static bool CreateTransactionInternal( AvailableCoins(wallet, vAvailableCoins, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0); // Choose coins to use - CAmount inputs_sum = 0; - std::set setCoins; - if (!SelectCoins(wallet, vAvailableCoins, /* nTargetValue */ selection_target, setCoins, inputs_sum, coin_control, coin_selection_params)) - { + std::optional result = SelectCoins(wallet, vAvailableCoins, /* nTargetValue */ selection_target, coin_control, coin_selection_params); + if (!result) { error = _("Insufficient funds"); return false; } // Always make a change output // We will reduce the fee from this change output later, and remove the output if it is too small. - const CAmount change_and_fee = inputs_sum - recipients_sum; + const CAmount change_and_fee = result->GetSelectedValue() - recipients_sum; assert(change_and_fee >= 0); CTxOut newTxOut(change_and_fee, scriptChange); @@ -790,8 +789,7 @@ static bool CreateTransactionInternal( auto change_position = txNew.vout.insert(txNew.vout.begin() + nChangePosInOut, newTxOut); // Shuffle selected coins and fill in final vin - std::vector selected_coins(setCoins.begin(), setCoins.end()); - Shuffle(selected_coins.begin(), selected_coins.end(), FastRandomContext()); + std::vector selected_coins = result->GetShuffledInputVector(); // Note how the sequence number is set to non-maxint so that // the nLockTime set above actually works. diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 982ba44a37..268bcfc033 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -117,15 +117,18 @@ std::optional AttemptSelection(const CWallet& wallet, const CAm const CoinSelectionParams& coin_selection_params); /** - * Select a set of coins such that nValueRet >= nTargetValue and at least + * Select a set of coins such that nTargetValue is met and at least * all coins from coin_control are selected; never select unconfirmed coins if they are not ours - * param@[out] setCoinsRet Populated with inputs including pre-selected inputs from - * coin_control and Coin Selection if successful. - * param@[out] nValueRet Total value of selected coins including pre-selected ones - * from coin_control and Coin Selection if successful. + * param@[in] wallet The wallet which provides data necessary to spend the selected coins + * param@[in] vAvailableCoins The vector of coins available to be spent + * param@[in] nTargetValue The target value + * param@[in] coin_selection_params Parameters for this coin selection such as feerates, whether to avoid partial spends, + * and whether to subtract the fee from the outputs. + * returns If successful, a SelectionResult containing the selected coins + * If failed, a nullopt. */ -bool SelectCoins(const CWallet& wallet, const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, - const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); +std::optional SelectCoins(const CWallet& wallet, const std::vector& vAvailableCoins, const CAmount& nTargetValue, const CCoinControl& coin_control, + const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); /** * Create a new transaction paying the recipients with a set of coins diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 7d3a022285..38edb61ba7 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -330,8 +330,6 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) wallet->SetupDescriptorScriptPubKeyMans(); std::vector coins; - CoinSet setCoinsRet; - CAmount nValueRet; add_coin(coins, *wallet, 5 * CENT, 6 * 24, false, 0, true); add_coin(coins, *wallet, 3 * CENT, 6 * 24, false, 0, true); @@ -340,7 +338,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) coin_control.fAllowOtherInputs = true; coin_control.Select(COutPoint(coins.at(0).tx->GetHash(), coins.at(0).i)); coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); - BOOST_CHECK(SelectCoins(*wallet, coins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb)); + const auto result10 = SelectCoins(*wallet, coins, 10 * CENT, coin_control, coin_selection_params_bnb); + BOOST_CHECK(result10); } } @@ -713,11 +712,10 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) /* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0), /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), /* tx_noinputs_size= */ 0, /* avoid_partial= */ false); - CoinSet out_set; - CAmount out_value = 0; CCoinControl cc; - BOOST_CHECK(SelectCoins(*wallet, coins, target, out_set, out_value, cc, cs_params)); - BOOST_CHECK_GE(out_value, target); + const auto result = SelectCoins(*wallet, coins, target, cc, cs_params); + BOOST_CHECK(result); + BOOST_CHECK_GE(result->GetSelectedValue(), target); } } -- cgit v1.2.3