-
Notifications
You must be signed in to change notification settings - Fork 38.7k
bench: Add more realistic Coin Selection Bench #33160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
bench: Add more realistic Coin Selection Bench #33160
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33160. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
6001a67 to
2dfb6d5
Compare
l0rinc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK, the existing test were all quite trivial compared to this new one.
I left a ton of comments, for simplicity here's the final code that I'm suggesting, feel free to pick and choose from it.
Details
// Copyright (c) 2012-2022 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <bench/bench.h>
#include <consensus/amount.h>
#include <interfaces/chain.h>
#include <node/context.h>
#include <outputtype.h>
#include <policy/feerate.h>
#include <policy/policy.h>
#include <primitives/transaction.h>
#include <random.h>
#include <sync.h>
#include <util/result.h>
#include <wallet/coinselection.h>
#include <wallet/spend.h>
#include <wallet/test/util.h>
#include <wallet/transaction.h>
#include <wallet/wallet.h>
#include <cassert>
#include <map>
#include <memory>
#include <set>
#include <utility>
#include <vector>
#include <cmath>
using node::NodeContext;
using wallet::AttemptSelection;
using wallet::CHANGE_LOWER;
using wallet::COutput;
using wallet::CWallet;
using wallet::CWalletTx;
using wallet::CoinEligibilityFilter;
using wallet::CoinSelectionParams;
using wallet::CreateMockableWalletDatabase;
using wallet::OutputGroup;
using wallet::SelectCoinsBnB;
using wallet::TxStateInactive;
namespace {
constexpr CAmount operator""_sat(uint64_t v) { return v; }
constexpr CAmount operator""_ksat(uint64_t v) { return v * 1'000; }
constexpr CAmount operator""_btc(uint64_t v) { return v * COIN; }
constexpr CAmount operator""_btc(long double v) { return std::llround(v * COIN); }
int rand_percentage(FastRandomContext& rng) { return rng.randrange(100); }
CAmount rand_range(FastRandomContext& rng, CAmount lo, CAmount hi) { return lo + rng.randrange(hi - lo + 1); }
} // namespace
static void addCoin(CAmount nValue, std::vector<std::unique_ptr<CWalletTx>>& wtxs)
{
static int nextLockTime = 0;
CMutableTransaction tx;
tx.nLockTime = nextLockTime++; // so all transactions get different hashes
tx.vout.resize(1);
tx.vout[0].nValue = nValue;
wtxs.push_back(std::make_unique<CWalletTx>(MakeTransactionRef(std::move(tx)), TxStateInactive{}));
}
// Simple benchmark for wallet coin selection that exercises a worst-case
// scenario for Knapsack: All UTXOs are necessary, but it is not an exact
// match, so the only eligible input set is only discovered on the second pass
// after all random walks fail to produce a solution.
static void KnapsackWorstCase(benchmark::Bench& bench)
{
NodeContext node;
auto chain{interfaces::MakeChain(node)};
CWallet wallet(chain.get(), "", CreateMockableWalletDatabase());
std::vector<std::unique_ptr<CWalletTx>> wtxs;
LOCK(wallet.cs_wallet);
for (int i{0}; i < 1000; ++i) {
addCoin(1000_btc, wtxs);
}
addCoin(3_btc, wtxs);
// Create coins
wallet::CoinsResult available_coins;
for (const auto& wtx : wtxs) {
const auto txout{wtx->tx->vout.at(0)};
const int input_bytes{CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr)};
const COutput output{COutPoint(wtx->GetHash(), 0), txout, /*depth=*/6 * 24, input_bytes, /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true, /*fees=*/0};
available_coins.coins[OutputType::BECH32].emplace_back(output);
}
const CoinEligibilityFilter filter_standard(1, 6, 0);
FastRandomContext rand{/*fDeterministic=*/true};
const CoinSelectionParams coin_selection_params{
rand,
/*change_output_size=*/34,
/*change_spend_size=*/148,
/*min_change_target=*/CHANGE_LOWER,
/*effective_feerate=*/CFeeRate(20'000),
/*long_term_feerate=*/CFeeRate(10'000),
/*discard_feerate=*/CFeeRate(3000),
/*tx_noinputs_size=*/0,
/*avoid_partial=*/false,
};
auto group{wallet::GroupOutputs(wallet, available_coins, coin_selection_params, {{filter_standard}})[filter_standard]};
bench.run([&] {
auto result{AttemptSelection(wallet.chain(), 1002.99_btc, group, coin_selection_params, /*allow_mixed_output_types=*/true)};
assert(result);
assert(result->GetSelectedValue() == 1003_btc);
assert(result->GetInputSet().size() == 2);
});
}
// This benchmark is based on a UTXO pool composed of 400 UTXOs. The UTXOs are
// pseudorandomly generated to be of the four relevant output types P2PKH,
// P2SH-P2WPKH, P2WPKH, and P2TR UTXOs, and fall in the range of 10'000 sats to
// 1₿ with larger amounts being more likely.
// This UTXO pool is used to run coin selection for 100 pseudorandom selection
// targets from 0.1–2₿. Altogether, this gives us a deterministic benchmark
// with a hopefully somewhat representative coin selection scenario.
static void CoinSelectionOnDiverseWallet(benchmark::Bench& bench)
{
FastRandomContext rng{/*fDeterministic=*/true};
NodeContext node;
auto chain{interfaces::MakeChain(node)};
CWallet wallet(chain.get(), "", CreateMockableWalletDatabase());
LOCK(wallet.cs_wallet);
std::vector<std::unique_ptr<CWalletTx>> wtxs;
wtxs.reserve(400);
for (size_t i{0}; i < wtxs.capacity(); ++i) {
const int p{rand_percentage(rng)};
const auto val{p < 50 ? rand_range(rng, 10_ksat, 100_ksat) :
p < 75 ? rand_range(rng, 100_ksat, 1000_ksat) :
p < 95 ? rand_range(rng, 1000_ksat, 1_btc) :
rand_range(rng, 0.1_btc, 1_btc)};
addCoin(val, wtxs);
}
// Create coins
wallet::CoinsResult available_coins;
for (const auto& wtx : wtxs) {
const int p{rand_percentage(rng)};
auto val{p < 35 ? OutputType::LEGACY :
p < 55 ? OutputType::P2SH_SEGWIT :
p < 90 ? OutputType::BECH32 :
OutputType::BECH32M};
const auto txout{wtx->tx->vout.at(0)};
const int input_bytes{CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr)};
const COutput output{COutPoint{wtx->GetHash(), 0}, txout, /*depth=*/6 * 24, input_bytes, /*solvable=*/true, /*safe=*/true, wtx->GetTxTime(), /*from_me=*/true, /*fees=*/0};
available_coins.coins[val].emplace_back(output);
}
std::vector<CAmount> targets;
targets.reserve(10);
for (size_t i{0}; i < targets.capacity(); ++i) {
targets.push_back(rand_range(rng, 0.1_btc, 1_btc));
}
const CoinSelectionParams coin_selection_params{
rng,
/*change_output_size=*/31,
/*change_spend_size=*/68,
/*min_change_target=*/CHANGE_LOWER,
/*effective_feerate=*/CFeeRate(20'000),
/*long_term_feerate=*/CFeeRate(10'000),
/*discard_feerate=*/CFeeRate(3000),
/*tx_noinputs_size=*/72,
/*avoid_partial=*/false,
};
const CoinEligibilityFilter filter_standard(1, 6, 0);
auto group{wallet::GroupOutputs(wallet, available_coins, coin_selection_params, {{filter_standard}})[filter_standard]};
bench.run([&] {
for (const auto& target : targets) {
auto result{AttemptSelection(wallet.chain(), target, group, coin_selection_params, /*allow_mixed_output_types=*/true)};
assert(result && result->GetSelectedValue() >= target);
}
});
}
static void add_coin(CAmount nValue, uint32_t nInput, std::vector<OutputGroup>& set)
{
CMutableTransaction tx;
tx.vout.resize(nInput + 1);
tx.vout[nInput].nValue = nValue;
COutput output(COutPoint{tx.GetHash(), nInput}, tx.vout.at(nInput), /*depth=*/0, /*input_bytes=*/-1, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/true, /*fees=*/0);
set.emplace_back();
set.back().Insert(std::make_shared<COutput>(output), /*ancestors=*/0, /*descendants=*/0);
}
static CAmount make_hard_case(int utxos, std::vector<OutputGroup>& utxo_pool)
{
utxo_pool.clear();
CAmount target{0};
for (int i{0}; i < utxos; ++i) {
target += 1_sat << (utxos + i);
add_coin(1_sat << (utxos + i), 2 * i, utxo_pool);
add_coin((1_sat << (utxos + i)) + (1_sat << (utxos - 1 - i)), 2 * i + 1, utxo_pool);
}
return target;
}
static void BnBExhaustion(benchmark::Bench& bench)
{
std::vector<OutputGroup> utxo_pool;
auto target{make_hard_case(17, utxo_pool)};
bench.run([&] {
SelectCoinsBnB(utxo_pool, target, 0, MAX_STANDARD_TX_WEIGHT); // Should exhaust
});
}
BENCHMARK(KnapsackWorstCase, benchmark::PriorityLevel::HIGH);
BENCHMARK(CoinSelectionOnDiverseWallet, benchmark::PriorityLevel::HIGH);
BENCHMARK(BnBExhaustion, benchmark::PriorityLevel::HIGH);
| int x{det_rand.randrange(100)}; | ||
| if (x < 50) { | ||
| // 0.0001–0.001 COIN | ||
| addCoin(det_rand.randrange(90'000) + 10'000, wallet, wtxs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't actually need the wallet here, right?
| } | ||
|
|
||
| // Allow actual randomness for selection | ||
| FastRandomContext rand{}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we want deterministic randomness for benchmarks, otherwise it's hard to know what we're actually measuring:
| FastRandomContext rand{}; | |
| FastRandomContext rand{/*fDeterministic=*/true}; |
| } | ||
|
|
||
| std::vector<CAmount> targets; | ||
| for (int i = 0; i < 100; ++i) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could reserve this to avoid the build failure:
std::vector<CAmount> targets;
targets.reserve(10);
for (size_t i{0}; i < targets.capacity(); ++i) {
targets.push_back(rand_range(rng, 0.1_btc, 1_btc));
}Note that I have reduced the target count to 10 since the benchmark was very slow otherwise
| // This benchmark is based on a UTXO pool composed of 400 UTXOs. The UTXOs are | ||
| // pseudorandomly generated to be of the four relevant output types P2PKH, | ||
| // P2SH-P2WPKH, P2WPKH, and P2TR UTXOs, and fall in the range of 10'000 sats to | ||
| // 1 ₿ with larger amounts being more likely. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd avoid the Narrow No-Break Space chars from docs if possible, they're rendered differently on different mediums
| // Use arbitrary static seed for generating a pseudorandom scenario | ||
| uint256 arb_seed = uint256("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"); | ||
| FastRandomContext det_rand{arb_seed}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, seems simpler to just leave it to deterministic instead of hard-coding a confusing seed
| // Use arbitrary static seed for generating a pseudorandom scenario | |
| uint256 arb_seed = uint256("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"); | |
| FastRandomContext det_rand{arb_seed}; | |
| FastRandomContext det_rand{/*fDeterministic=*/true}; |
note: can we reuse this for the random source below as well?
|
|
||
| CAmount target = make_hard_case(17, utxo_pool); | ||
| bench.run([&] { | ||
| // Benchmark |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't have to keep this comment either in 2dfb6d5
| bench.run([&] { | ||
| // Benchmark | ||
| CAmount target = make_hard_case(17, utxo_pool); | ||
| SelectCoinsBnB(utxo_pool, target, 0, MAX_STANDARD_TX_WEIGHT); // Should exhaust |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should exhaust - can we assert that instead of adding a dead comment?
| // P2SH-P2WPKH, P2WPKH, and P2TR UTXOs, and fall in the range of 10'000 sats to | ||
| // 1 ₿ with larger amounts being more likely. | ||
| // This UTXO pool is used to run coin selection for 100 pseudorandom selection | ||
| // targets from 0.1–2 ₿. Altogether, this gives us a deterministic benchmark |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where are we adding 2 btc? Could we avoid values from the comments, they're just adding a maintenance burden when we're forgetting updating them
|
|
||
| bench.run([&] { | ||
| for (const auto& target : targets) { | ||
| auto result = AttemptSelection(wallet.chain(), target, group, coin_selection_params, /*allow_mixed_output_types=*/true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const CoinEligibilityFilter filter_standard(1, 6, 0); | ||
| const CoinSelectionParams coin_selection_params{ | ||
| rand, | ||
| /*change_output_size=*/ 31, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: let's unify the comment styles across the file
|
🐙 This pull request conflicts with the target branch and needs rebase. |

First draft for a Coin Selection Benchmark that doesn’t just test a worst case of one of the algorithms but tries to select inputs for a variety of different targets.