phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-27 03:33:27 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions

Return SelectionResult from SelectCoinsBnB

Browse source 
Removes coins_out and value_ret has SelectCoinsBnB return a
std::optional<SelectionResult>
This commit is contained in:
Andrew Chow 2020-11-16 14:31:45 -05:00
parent a339add471
commit 60d2ca72e3
5 changed files with 60 additions and 70 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
src/bench/coin_selection.cpp
View file

@ -92,17 +92,14 @@ static void BnBExhaustion(benchmark::Bench& bench)
{ {
// Setup // Setup
std::vector<OutputGroup> utxo_pool; std::vector<OutputGroup> utxo_pool;
CoinSet selection;
CAmount value_ret = 0;
bench.run([&] { bench.run([&] {
// Benchmark // Benchmark
CAmount target = make_hard_case(17, utxo_pool); CAmount target = make_hard_case(17, utxo_pool);
SelectCoinsBnB(utxo_pool, target, 0, selection, value_ret); // Should exhaust SelectCoinsBnB(utxo_pool, target, 0); // Should exhaust
// Cleanup // Cleanup
utxo_pool.clear(); utxo_pool.clear();
selection.clear();
}); });
} }

20
src/wallet/coinselection.cpp
View file

@ -56,17 +56,14 @@ struct {
* bound of the range. * bound of the range.
* @param const CAmount& cost_of_change This is the cost of creating and spending a change output. * @param const CAmount& cost_of_change This is the cost of creating and spending a change output.
* This plus selection_target is the upper bound of the range. * This plus selection_target is the upper bound of the range.
* @param std::set<CInputCoin>& out_set -> This is an output parameter for the set of CInputCoins * @returns The result of this coin selection algorithm, or std::nullopt
* that have been selected.
* @param CAmount& value_ret -> This is an output parameter for the total value of the CInputCoins
* that were selected.
*/ */
static const size_t TOTAL_TRIES = 100000; static const size_t TOTAL_TRIES = 100000;
bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret) std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change)
{ {
out_set.clear(); SelectionResult result(selection_target);
CAmount curr_value = 0; CAmount curr_value = 0;
std::vector<bool> curr_selection; // select the utxo at this index std::vector<bool> curr_selection; // select the utxo at this index
@ -80,7 +77,7 @@ bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selectio
curr_available_value += utxo.GetSelectionAmount(); curr_available_value += utxo.GetSelectionAmount();
} }
if (curr_available_value < selection_target) { if (curr_available_value < selection_target) {
return false; return std::nullopt;
} }
// Sort the utxo_pool // Sort the utxo_pool
@ -156,19 +153,17 @@ bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selectio
// Check for solution // Check for solution
if (best_selection.empty()) { if (best_selection.empty()) {
return false; return std::nullopt;
} }
// Set output set // Set output set
value_ret = 0;
for (size_t i = 0; i < best_selection.size(); ++i) { for (size_t i = 0; i < best_selection.size(); ++i) {
if (best_selection.at(i)) { if (best_selection.at(i)) {
util::insert(out_set, utxo_pool.at(i).m_outputs); result.AddInput(utxo_pool.at(i));
value_ret += utxo_pool.at(i).m_value;
} }
} }
return true; return result;
} }
std::optional<std::pair<std::set<CInputCoin>, CAmount>> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value) std::optional<std::pair<std::set<CInputCoin>, CAmount>> SelectCoinsSRD(const std::vector<OutputGroup>& utxo_pool, CAmount target_value)
@ -421,6 +416,7 @@ void SelectionResult::Clear()
void SelectionResult::AddInput(const OutputGroup& group) void SelectionResult::AddInput(const OutputGroup& group)
{ {
util::insert(m_selected_inputs, group.m_outputs); util::insert(m_selected_inputs, group.m_outputs);
m_use_effective = !group.m_subtract_fee_outputs;
} }
const std::set<CInputCoin>& SelectionResult::GetInputSet() const const std::set<CInputCoin>& SelectionResult::GetInputSet() const

2
src/wallet/coinselection.h
View file

@ -234,7 +234,7 @@ public:
bool operator<(SelectionResult other) const; bool operator<(SelectionResult other) const;
}; };
bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret); std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change);
/** Select coins by Single Random Draw. OutputGroups are selected randomly from the eligible /** Select coins by Single Random Draw. OutputGroups are selected randomly from the eligible
* outputs until the target is satisfied * outputs until the target is satisfied

8
src/wallet/spend.cpp
View file

@ -385,11 +385,9 @@ bool AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const
// Note that unlike KnapsackSolver, we do not include the fee for creating a change output as BnB will not create a change output. // Note that unlike KnapsackSolver, we do not include the fee for creating a change output as BnB will not create a change output.
std::vector<OutputGroup> positive_groups = GroupOutputs(wallet, coins, coin_selection_params, eligibility_filter, true /* positive_only */); std::vector<OutputGroup> positive_groups = GroupOutputs(wallet, coins, coin_selection_params, eligibility_filter, true /* positive_only */);
std::set<CInputCoin> bnb_coins; if (auto bnb_result{SelectCoinsBnB(positive_groups, nTargetValue, coin_selection_params.m_cost_of_change)}) {
CAmount bnb_value; bnb_result->ComputeAndSetWaste(CAmount(0));
if (SelectCoinsBnB(positive_groups, nTargetValue, coin_selection_params.m_cost_of_change, bnb_coins, bnb_value)) { results.emplace_back(std::make_tuple(bnb_result->GetWaste(), bnb_result->GetInputSet(), bnb_result->GetSelectedValue()));
const auto waste = GetSelectionWaste(bnb_coins, /* cost of change */ CAmount(0), nTargetValue, !coin_selection_params.m_subtract_fee_outputs);
results.emplace_back(std::make_tuple(waste, std::move(bnb_coins), bnb_value));
} }
// The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here. // The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here.

95
src/wallet/test/coinselector_tests.cpp
View file

@ -14,6 +14,7 @@
#include <wallet/test/wallet_test_fixture.h> #include <wallet/test/wallet_test_fixture.h>
#include <wallet/wallet.h> #include <wallet/wallet.h>
#include <algorithm>
#include <boost/test/unit_test.hpp> #include <boost/test/unit_test.hpp>
#include <random> #include <random>
@ -95,20 +96,22 @@ static void add_coin(std::vector<COutput>& coins, CWallet& wallet, const CAmount
coins.push_back(output); coins.push_back(output);
} }
static bool equivalent_sets(CoinSet a, CoinSet b) /** Check if SelectionResult a is equivalent to SelectionResult b.
* Equivalent means same input values, but maybe different inputs (i.e. same value, different prevout) */
static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b)
{ {
std::vector<CAmount> a_amts; std::vector<CAmount> a_amts;
std::vector<CAmount> b_amts; std::vector<CAmount> b_amts;
for (const auto& coin : a) { for (const auto& coin : a.GetInputSet()) {
a_amts.push_back(coin.txout.nValue); a_amts.push_back(coin.txout.nValue);
} }
for (const auto& coin : b) { for (const auto& coin : b.GetInputSet()) {
b_amts.push_back(coin.txout.nValue); b_amts.push_back(coin.txout.nValue);
} }
std::sort(a_amts.begin(), a_amts.end()); std::sort(a_amts.begin(), a_amts.end());
std::sort(b_amts.begin(), b_amts.end()); std::sort(b_amts.begin(), b_amts.end());
std::pair<std::vector<CAmount>::iterator, std::vector<CAmount>::iterator> ret = mismatch(a_amts.begin(), a_amts.end(), b_amts.begin()); std::pair<std::vector<CAmount>::iterator, std::vector<CAmount>::iterator> ret = std::mismatch(a_amts.begin(), a_amts.end(), b_amts.begin());
return ret.first == a_amts.end() && ret.second == b_amts.end(); return ret.first == a_amts.end() && ret.second == b_amts.end();
} }
@ -168,17 +171,14 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
{ {
// Setup // Setup
std::vector<CInputCoin> utxo_pool; std::vector<CInputCoin> utxo_pool;
CoinSet selection;
SelectionResult expected_result(CAmount(0)); SelectionResult expected_result(CAmount(0));
CAmount value_ret = 0;
///////////////////////// /////////////////////////
// Known Outcome tests // // Known Outcome tests //
///////////////////////// /////////////////////////
// Empty utxo pool // Empty utxo pool
BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 0.5 * CENT, selection, value_ret)); BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 0.5 * CENT));
selection.clear();
// Add utxos // Add utxos
add_coin(1 * CENT, 1, utxo_pool); add_coin(1 * CENT, 1, utxo_pool);
@ -188,78 +188,77 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
// Select 1 Cent // Select 1 Cent
add_coin(1 * CENT, 1, expected_result); add_coin(1 * CENT, 1, expected_result);
BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 0.5 * CENT, selection, value_ret)); const auto result1 = SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 0.5 * CENT);
BOOST_CHECK(equivalent_sets(selection, expected_result.GetInputSet())); BOOST_CHECK(result1);
BOOST_CHECK_EQUAL(value_ret, 1 * CENT); BOOST_CHECK(EquivalentResult(expected_result, *result1));
BOOST_CHECK_EQUAL(result1->GetSelectedValue(), 1 * CENT);
expected_result.Clear(); expected_result.Clear();
selection.clear();
// Select 2 Cent // Select 2 Cent
add_coin(2 * CENT, 2, expected_result); add_coin(2 * CENT, 2, expected_result);
BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 2 * CENT, 0.5 * CENT, selection, value_ret)); const auto result2 = SelectCoinsBnB(GroupCoins(utxo_pool), 2 * CENT, 0.5 * CENT);
BOOST_CHECK(equivalent_sets(selection, expected_result.GetInputSet())); BOOST_CHECK(result2);
BOOST_CHECK_EQUAL(value_ret, 2 * CENT); BOOST_CHECK(EquivalentResult(expected_result, *result2));
BOOST_CHECK_EQUAL(result2->GetSelectedValue(), 2 * CENT);
expected_result.Clear(); expected_result.Clear();
selection.clear();
// Select 5 Cent // Select 5 Cent
add_coin(4 * CENT, 4, expected_result); add_coin(4 * CENT, 4, expected_result);
add_coin(1 * CENT, 1, expected_result); add_coin(1 * CENT, 1, expected_result);
BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 5 * CENT, 0.5 * CENT, selection, value_ret)); const auto result3 = SelectCoinsBnB(GroupCoins(utxo_pool), 5 * CENT, 0.5 * CENT);
BOOST_CHECK(equivalent_sets(selection, expected_result.GetInputSet())); BOOST_CHECK(result3);
BOOST_CHECK_EQUAL(value_ret, 5 * CENT); BOOST_CHECK(EquivalentResult(expected_result, *result3));
BOOST_CHECK_EQUAL(result3->GetSelectedValue(), 5 * CENT);
expected_result.Clear(); expected_result.Clear();
selection.clear();
// Select 11 Cent, not possible // Select 11 Cent, not possible
BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 11 * CENT, 0.5 * CENT, selection, value_ret)); BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 11 * CENT, 0.5 * CENT));
expected_result.Clear(); expected_result.Clear();
selection.clear();
// Cost of change is greater than the difference between target value and utxo sum // Cost of change is greater than the difference between target value and utxo sum
add_coin(1 * CENT, 1, expected_result); add_coin(1 * CENT, 1, expected_result);
BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 0.9 * CENT, 0.5 * CENT, selection, value_ret)); const auto result4 = SelectCoinsBnB(GroupCoins(utxo_pool), 0.9 * CENT, 0.5 * CENT);
BOOST_CHECK_EQUAL(value_ret, 1 * CENT); BOOST_CHECK(result4);
BOOST_CHECK(equivalent_sets(selection, expected_result.GetInputSet())); BOOST_CHECK_EQUAL(result4->GetSelectedValue(), 1 * CENT);
BOOST_CHECK(EquivalentResult(expected_result, *result4));
expected_result.Clear(); expected_result.Clear();
selection.clear();
// Cost of change is less than the difference between target value and utxo sum // Cost of change is less than the difference between target value and utxo sum
BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.9 * CENT, 0, selection, value_ret)); BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.9 * CENT, 0));
expected_result.Clear(); expected_result.Clear();
selection.clear();
// Select 10 Cent // Select 10 Cent
add_coin(5 * CENT, 5, utxo_pool); add_coin(5 * CENT, 5, utxo_pool);
add_coin(5 * CENT, 5, expected_result); add_coin(5 * CENT, 5, expected_result);
add_coin(4 * CENT, 4, expected_result); add_coin(4 * CENT, 4, expected_result);
add_coin(1 * CENT, 1, expected_result); add_coin(1 * CENT, 1, expected_result);
BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 0.5 * CENT, selection, value_ret)); const auto result5 = SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 0.5 * CENT);
BOOST_CHECK(equivalent_sets(selection, expected_result.GetInputSet())); BOOST_CHECK(result5);
BOOST_CHECK_EQUAL(value_ret, 10 * CENT); BOOST_CHECK(EquivalentResult(expected_result, *result5));
BOOST_CHECK_EQUAL(result5->GetSelectedValue(), 10 * CENT);
expected_result.Clear(); expected_result.Clear();
selection.clear();
// Negative effective value // Negative effective value
// Select 10 Cent but have 1 Cent not be possible because too small // Select 10 Cent but have 1 Cent not be possible because too small
add_coin(5 * CENT, 5, expected_result); add_coin(5 * CENT, 5, expected_result);
add_coin(3 * CENT, 3, expected_result); add_coin(3 * CENT, 3, expected_result);
add_coin(2 * CENT, 2, expected_result); add_coin(2 * CENT, 2, expected_result);
BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 5000, selection, value_ret)); const auto result6 = SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 5000);
BOOST_CHECK_EQUAL(value_ret, 10 * CENT); BOOST_CHECK(result6);
BOOST_CHECK_EQUAL(result6->GetSelectedValue(), 10 * CENT);
// FIXME: this test is redundant with the above, because 1 Cent is selected, not "too small" // FIXME: this test is redundant with the above, because 1 Cent is selected, not "too small"
// BOOST_CHECK(equivalent_sets(selection, expected_result.GetInputSet())); // BOOST_CHECK(EquivalentResult(expected_result, *result));
// Select 0.25 Cent, not possible // Select 0.25 Cent, not possible
BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.25 * CENT, 0.5 * CENT, selection, value_ret)); BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.25 * CENT, 0.5 * CENT));
expected_result.Clear(); expected_result.Clear();
selection.clear();
// Iteration exhaustion test // Iteration exhaustion test
CAmount target = make_hard_case(17, utxo_pool); CAmount target = make_hard_case(17, utxo_pool);
BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), target, 0, selection, value_ret)); // Should exhaust BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), target, 0)); // Should exhaust
target = make_hard_case(14, utxo_pool); target = make_hard_case(14, utxo_pool);
BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), target, 0, selection, value_ret)); // Should not exhaust const auto result7 = SelectCoinsBnB(GroupCoins(utxo_pool), target, 0); // Should not exhaust
BOOST_CHECK(result7);
// Test same value early bailout optimization // Test same value early bailout optimization
utxo_pool.clear(); utxo_pool.clear();
@ -276,9 +275,10 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
for (int i = 0; i < 50000; ++i) { for (int i = 0; i < 50000; ++i) {
add_coin(5 * CENT, 7, utxo_pool); add_coin(5 * CENT, 7, utxo_pool);
} }
BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 30 * CENT, 5000, selection, value_ret)); const auto result8 = SelectCoinsBnB(GroupCoins(utxo_pool), 30 * CENT, 5000);
BOOST_CHECK_EQUAL(value_ret, 30 * CENT); BOOST_CHECK(result8);
BOOST_CHECK(equivalent_sets(selection, expected_result.GetInputSet())); BOOST_CHECK_EQUAL(result8->GetSelectedValue(), 30 * CENT);
BOOST_CHECK(EquivalentResult(expected_result, *result8));
//////////////////// ////////////////////
// Behavior tests // // Behavior tests //
@ -290,7 +290,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
} }
// Run 100 times, to make sure it is never finding a solution // Run 100 times, to make sure it is never finding a solution
for (int i = 0; i < 100; ++i) { for (int i = 0; i < 100; ++i) {
BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 2 * CENT, selection, value_ret)); BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 2 * CENT));
} }
// Make sure that effective value is working in AttemptSelection when BnB is used // Make sure that effective value is working in AttemptSelection when BnB is used
@ -306,20 +306,19 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
wallet->SetupDescriptorScriptPubKeyMans(); wallet->SetupDescriptorScriptPubKeyMans();
std::vector<COutput> coins; std::vector<COutput> coins;
CoinSet setCoinsRet;
CAmount nValueRet;
add_coin(coins, *wallet, 1); add_coin(coins, *wallet, 1);
coins.at(0).nInputBytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail coins.at(0).nInputBytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail
BOOST_CHECK(!SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change, setCoinsRet, nValueRet)); BOOST_CHECK(!SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change));
// Test fees subtracted from output: // Test fees subtracted from output:
coins.clear(); coins.clear();
add_coin(coins, *wallet, 1 * CENT); add_coin(coins, *wallet, 1 * CENT);
coins.at(0).nInputBytes = 40; coins.at(0).nInputBytes = 40;
coin_selection_params_bnb.m_subtract_fee_outputs = true; coin_selection_params_bnb.m_subtract_fee_outputs = true;
BOOST_CHECK(SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change, setCoinsRet, nValueRet)); const auto result9 = SelectCoinsBnB(GroupCoins(coins), 1 * CENT, coin_selection_params_bnb.m_cost_of_change);
BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); BOOST_CHECK(result9);
BOOST_CHECK_EQUAL(result9->GetSelectedValue(), 1 * CENT);
} }
{ {