Skip to content

Commit fa7deaa

Browse files
author
MarcoFalke
committed
wallet: Pass FastRandomContext& to coin selection
1 parent 77773b0 commit fa7deaa

File tree

5 files changed

+78
-45
lines changed

5 files changed

+78
-45
lines changed

‎src/bench/coin_selection.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,17 @@ static void CoinSelection(benchmark::Bench& bench)
6262
}
6363

6464
const CoinEligibilityFilter filter_standard(1, 6, 0);
65-
const CoinSelectionParams coin_selection_params(/* change_output_size= */ 34,
66-
/* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0),
67-
/* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0),
68-
/* tx_noinputs_size= */ 0, /* avoid_partial= */ false);
65+
FastRandomContext rand{};
66+
const CoinSelectionParams coin_selection_params{
67+
rand,
68+
/* change_output_size= */ 34,
69+
/* change_spend_size= */ 148,
70+
/* effective_feerate= */ CFeeRate(0),
71+
/* long_term_feerate= */ CFeeRate(0),
72+
/* discard_feerate= */ CFeeRate(0),
73+
/* tx_noinputs_size= */ 0,
74+
/* avoid_partial= */ false,
75+
};
6976
bench.run([&] {
7077
auto result = AttemptSelection(wallet, 1003 * COIN, filter_standard, coins, coin_selection_params);
7178
assert(result);

‎src/wallet/coinselection.cpp

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -169,14 +169,14 @@ std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_poo
169169
return result;
170170
}
171171

172-
std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value)
172+
std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, FastRandomContext& rng)
173173
{
174174
SelectionResult result(target_value);
175175

176176
std::vector<size_t> indexes;
177177
indexes.resize(utxo_pool.size());
178178
std::iota(indexes.begin(), indexes.end(), 0);
179-
Shuffle(indexes.begin(), indexes.end(), FastRandomContext());
179+
Shuffle(indexes.begin(), indexes.end(), rng);
180180

181181
CAmount selected_eff_value = 0;
182182
for (const size_t i : indexes) {
@@ -191,16 +191,14 @@ std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& ut
191191
return std::nullopt;
192192
}
193193

194-
static void ApproximateBestSubset(const std::vector<OutputGroup>& groups, const CAmount& nTotalLower, const CAmount& nTargetValue,
194+
static void ApproximateBestSubset(FastRandomContext& insecure_rand, const std::vector<OutputGroup>& groups, const CAmount& nTotalLower, const CAmount& nTargetValue,
195195
std::vector<char>& vfBest, CAmount& nBest, int iterations = 1000)
196196
{
197197
std::vector<char> vfIncluded;
198198

199199
vfBest.assign(groups.size(), true);
200200
nBest = nTotalLower;
201201

202-
FastRandomContext insecure_rand;
203-
204202
for (int nRep = 0; nRep < iterations && nBest != nTargetValue; nRep++)
205203
{
206204
vfIncluded.assign(groups.size(), false);
@@ -237,7 +235,7 @@ static void ApproximateBestSubset(const std::vector<OutputGroup>& groups, const
237235
}
238236
}
239237

240-
std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue)
238+
std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue, FastRandomContext& rng)
241239
{
242240
SelectionResult result(nTargetValue);
243241

@@ -246,7 +244,7 @@ std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups,
246244
std::vector<OutputGroup> applicable_groups;
247245
CAmount nTotalLower = 0;
248246

249-
Shuffle(groups.begin(), groups.end(), FastRandomContext());
247+
Shuffle(groups.begin(), groups.end(), rng);
250248

251249
for (const OutputGroup& group : groups) {
252250
if (group.GetSelectionAmount() == nTargetValue) {
@@ -278,9 +276,9 @@ std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups,
278276
std::vector<char> vfBest;
279277
CAmount nBest;
280278

281-
ApproximateBestSubset(applicable_groups, nTotalLower, nTargetValue, vfBest, nBest);
279+
ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue, vfBest, nBest);
282280
if (nBest != nTargetValue && nTotalLower >= nTargetValue + MIN_CHANGE) {
283-
ApproximateBestSubset(applicable_groups, nTotalLower, nTargetValue + MIN_CHANGE, vfBest, nBest);
281+
ApproximateBestSubset(rng, applicable_groups, nTotalLower, nTargetValue + MIN_CHANGE, vfBest, nBest);
284282
}
285283

286284
// If we have a bigger coin and (either the stochastic approximation didn't find a good solution,

‎src/wallet/coinselection.h

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,9 @@ class CInputCoin {
7373
};
7474

7575
/** Parameters for one iteration of Coin Selection. */
76-
struct CoinSelectionParams
77-
{
76+
struct CoinSelectionParams {
77+
/** Randomness to use in the context of coin selection. */
78+
FastRandomContext& rng_fast;
7879
/** Size of a change output in bytes, determined by the output type. */
7980
size_t change_output_size = 0;
8081
/** Size of the input to spend a change output in virtual bytes. */
@@ -100,17 +101,20 @@ struct CoinSelectionParams
100101
* reuse. Dust outputs are not eligible to be added to output groups and thus not considered. */
101102
bool m_avoid_partial_spends = false;
102103

103-
CoinSelectionParams(size_t change_output_size, size_t change_spend_size, CFeeRate effective_feerate,
104-
CFeeRate long_term_feerate, CFeeRate discard_feerate, size_t tx_noinputs_size, bool avoid_partial) :
105-
change_output_size(change_output_size),
106-
change_spend_size(change_spend_size),
107-
m_effective_feerate(effective_feerate),
108-
m_long_term_feerate(long_term_feerate),
109-
m_discard_feerate(discard_feerate),
110-
tx_noinputs_size(tx_noinputs_size),
111-
m_avoid_partial_spends(avoid_partial)
112-
{}
113-
CoinSelectionParams() {}
104+
CoinSelectionParams(FastRandomContext& rng_fast, size_t change_output_size, size_t change_spend_size, CFeeRate effective_feerate,
105+
CFeeRate long_term_feerate, CFeeRate discard_feerate, size_t tx_noinputs_size, bool avoid_partial)
106+
: rng_fast{rng_fast},
107+
change_output_size(change_output_size),
108+
change_spend_size(change_spend_size),
109+
m_effective_feerate(effective_feerate),
110+
m_long_term_feerate(long_term_feerate),
111+
m_discard_feerate(discard_feerate),
112+
tx_noinputs_size(tx_noinputs_size),
113+
m_avoid_partial_spends(avoid_partial)
114+
{
115+
}
116+
CoinSelectionParams(FastRandomContext& rng_fast)
117+
: rng_fast{rng_fast} {}
114118
};
115119

116120
/** Parameters for filtering which OutputGroups we may use in coin selection.
@@ -246,10 +250,10 @@ std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_poo
246250
* @param[in] target_value The target value to select for
247251
* @returns If successful, a SelectionResult, otherwise, std::nullopt
248252
*/
249-
std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value);
253+
std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value, FastRandomContext& rng);
250254

251255
// Original coin selection algorithm as a fallback
252-
std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue);
256+
std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, const CAmount& nTargetValue, FastRandomContext& rng);
253257
} // namespace wallet
254258

255259
#endif // BITCOIN_WALLET_COINSELECTION_H

‎src/wallet/spend.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -386,15 +386,15 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
386386
std::vector<OutputGroup> all_groups = GroupOutputs(wallet, coins, coin_selection_params, eligibility_filter, false /* positive_only */);
387387
// While nTargetValue includes the transaction fees for non-input things, it does not include the fee for creating a change output.
388388
// So we need to include that for KnapsackSolver as well, as we are expecting to create a change output.
389-
if (auto knapsack_result{KnapsackSolver(all_groups, nTargetValue + coin_selection_params.m_change_fee)}) {
389+
if (auto knapsack_result{KnapsackSolver(all_groups, nTargetValue + coin_selection_params.m_change_fee, coin_selection_params.rng_fast)}) {
390390
knapsack_result->ComputeAndSetWaste(coin_selection_params.m_cost_of_change);
391391
results.push_back(*knapsack_result);
392392
}
393393

394394
// We include the minimum final change for SRD as we do want to avoid making really small change.
395395
// KnapsackSolver does not need this because it includes MIN_CHANGE internally.
396396
const CAmount srd_target = nTargetValue + coin_selection_params.m_change_fee + MIN_FINAL_CHANGE;
397-
if (auto srd_result{SelectCoinsSRD(positive_groups, srd_target)}) {
397+
if (auto srd_result{SelectCoinsSRD(positive_groups, srd_target, coin_selection_params.rng_fast)}) {
398398
srd_result->ComputeAndSetWaste(coin_selection_params.m_cost_of_change);
399399
results.push_back(*srd_result);
400400
}
@@ -501,7 +501,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, const std::vec
501501
// Cases where we have 101+ outputs all pointing to the same destination may result in
502502
// privacy leaks as they will potentially be deterministically sorted. We solve that by
503503
// explicitly shuffling the outputs before processing
504-
Shuffle(vCoins.begin(), vCoins.end(), FastRandomContext());
504+
Shuffle(vCoins.begin(), vCoins.end(), coin_selection_params.rng_fast);
505505
}
506506

507507
// Coin Selection attempts to select inputs from a pool of eligible UTXOs to fund the
@@ -657,7 +657,7 @@ static bool CreateTransactionInternal(
657657
FastRandomContext rng_fast;
658658
CMutableTransaction txNew; // The resulting transaction that we make
659659

660-
CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy
660+
CoinSelectionParams coin_selection_params{rng_fast}; // Parameters for coin selection, init with dummy
661661
coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends;
662662

663663
// Set the long term feerate estimate to the wallet's consolidate feerate

‎src/wallet/test/coinselector_tests.cpp

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,17 @@ inline std::vector<OutputGroup>& GroupCoins(const std::vector<COutput>& coins)
164164

165165
inline std::vector<OutputGroup>& KnapsackGroupOutputs(const std::vector<COutput>& coins, CWallet& wallet, const CoinEligibilityFilter& filter)
166166
{
167-
CoinSelectionParams coin_selection_params(/* change_output_size= */ 0,
168-
/* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(0),
169-
/* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0),
170-
/* tx_noinputs_size= */ 0, /* avoid_partial= */ false);
167+
FastRandomContext rand{};
168+
CoinSelectionParams coin_selection_params{
169+
rand,
170+
/* change_output_size= */ 0,
171+
/* change_spend_size= */ 0,
172+
/* effective_feerate= */ CFeeRate(0),
173+
/* long_term_feerate= */ CFeeRate(0),
174+
/* discard_feerate= */ CFeeRate(0),
175+
/* tx_noinputs_size= */ 0,
176+
/* avoid_partial= */ false,
177+
};
171178
static std::vector<OutputGroup> static_groups;
172179
static_groups = GroupOutputs(wallet, coins, coin_selection_params, filter, /*positive_only=*/false);
173180
return static_groups;
@@ -176,6 +183,7 @@ inline std::vector<OutputGroup>& KnapsackGroupOutputs(const std::vector<COutput>
176183
// Branch and bound coin selection tests
177184
BOOST_AUTO_TEST_CASE(bnb_search_test)
178185
{
186+
FastRandomContext rand{};
179187
// Setup
180188
std::vector<CInputCoin> utxo_pool;
181189
SelectionResult expected_result(CAmount(0));
@@ -301,10 +309,16 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
301309
}
302310

303311
// Make sure that effective value is working in AttemptSelection when BnB is used
304-
CoinSelectionParams coin_selection_params_bnb(/* change_output_size= */ 0,
305-
/* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(3000),
306-
/* long_term_feerate= */ CFeeRate(1000), /* discard_feerate= */ CFeeRate(1000),
307-
/* tx_noinputs_size= */ 0, /* avoid_partial= */ false);
312+
CoinSelectionParams coin_selection_params_bnb{
313+
rand,
314+
/* change_output_size= */ 0,
315+
/* change_spend_size= */ 0,
316+
/* effective_feerate= */ CFeeRate(3000),
317+
/* long_term_feerate= */ CFeeRate(1000),
318+
/* discard_feerate= */ CFeeRate(1000),
319+
/* tx_noinputs_size= */ 0,
320+
/* avoid_partial= */ false,
321+
};
308322
{
309323
std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase());
310324
wallet->LoadWallet();
@@ -351,6 +365,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
351365

352366
BOOST_AUTO_TEST_CASE(knapsack_solver_test)
353367
{
368+
FastRandomContext rand{};
369+
const auto temp1{[&rand](std::vector<OutputGroup>& g, const CAmount& v) { return KnapsackSolver(g, v, rand); }};
370+
const auto KnapsackSolver{temp1};
354371
std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase());
355372
wallet->LoadWallet();
356373
LOCK(wallet->cs_wallet);
@@ -660,6 +677,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
660677

661678
BOOST_AUTO_TEST_CASE(ApproximateBestSubset)
662679
{
680+
FastRandomContext rand{};
663681
std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase());
664682
wallet->LoadWallet();
665683
LOCK(wallet->cs_wallet);
@@ -673,7 +691,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset)
673691
add_coin(coins, *wallet, 1000 * COIN);
674692
add_coin(coins, *wallet, 3 * COIN);
675693

676-
const auto result = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1003 * COIN);
694+
const auto result = KnapsackSolver(KnapsackGroupOutputs(coins, *wallet, filter_standard), 1003 * COIN, rand);
677695
BOOST_CHECK(result);
678696
BOOST_CHECK_EQUAL(result->GetSelectedValue(), 1003 * COIN);
679697
BOOST_CHECK_EQUAL(result->GetInputSet().size(), 2U);
@@ -714,10 +732,16 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test)
714732
CAmount target = rand.randrange(balance - 1000) + 1000;
715733

716734
// Perform selection
717-
CoinSelectionParams cs_params(/* change_output_size= */ 34,
718-
/* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0),
719-
/* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0),
720-
/* tx_noinputs_size= */ 0, /* avoid_partial= */ false);
735+
CoinSelectionParams cs_params{
736+
rand,
737+
/* change_output_size= */ 34,
738+
/* change_spend_size= */ 148,
739+
/* effective_feerate= */ CFeeRate(0),
740+
/* long_term_feerate= */ CFeeRate(0),
741+
/* discard_feerate= */ CFeeRate(0),
742+
/* tx_noinputs_size= */ 0,
743+
/* avoid_partial= */ false,
744+
};
721745
CCoinControl cc;
722746
const auto result = SelectCoins(*wallet, coins, target, cc, cs_params);
723747
BOOST_CHECK(result);

0 commit comments

Comments
 (0)