Skip to content

Commit

Permalink
Track count of attempts to prevent regressions
Browse files Browse the repository at this point in the history
  • Loading branch information
murchandamus committed Jan 8, 2024
1 parent 05448cf commit c1ccd8b
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 9 deletions.
30 changes: 22 additions & 8 deletions src/wallet/coinselection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,19 +207,21 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
*
* @param std::vector<OutputGroup>& utxo_pool The UTXOs that we are choosing from. These UTXOs will be sorted in
* descending order by effective value, with lower weight preferred as a tie-breaker. (We can think of an output
* group with multiple as a heavier UTXO with the combined amount here.) @param const CAmount& selection_target
* This is the minimum amount that we need for the transaction without considering change.
* @param const CAmount& change_target The minimum budget for creating a change output that we add to the selection_target.
* @param int max_weight The maximum weight available for the input set.
* group with multiple as a heavier UTXO with the combined amount here.)
* @param const CAmount& selection_target This is the minimum amount that we need for the transaction without considering change.
* @param const CAmount& change_target The minimum budget for creating a change output, by which we increase the selection_target.
* @param int max_weight The maximum permitted weight for the input set.
* @returns The result of this coin selection algorithm, or std::nullopt
*/
util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, CAmount change_target, int max_weight)
{
std::sort(utxo_pool.begin(), utxo_pool.end(), descending_effval_weight);
std::vector<CAmount> lookahead(utxo_pool.size()); // The sum of UTXO amounts after this UTXO index
std::vector<int> min_tail_weight(utxo_pool.size()); // The minimum UTXO weight among the remaining UTXOs after this UTXO index
// The sum of UTXO amounts after this UTXO index, e.g. lookahead[5] = Σ(UTXO[6+].amount)
std::vector<CAmount> lookahead(utxo_pool.size());
// The minimum UTXO weight among the remaining UTXOs after this UTXO index, e.g. min_tail_weight[5] = min(UTXO[6+].weight)
std::vector<int> min_tail_weight(utxo_pool.size());

// Calculate lookaheads and check that there are sufficient funds
// Calculate lookahead values, min_tail_weights, and check that there are sufficient funds
CAmount total_available = 0;
int min_group_weight = std::numeric_limits<int>::max();
size_t i = utxo_pool.size();
Expand Down Expand Up @@ -330,6 +332,7 @@ util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, c

if (curr_try >= TOTAL_TRIES) {
// Solution is not guaranteed to be optimal if `curr_try` hit TOTAL_TRIES
result.SetSelectionsEvaluated(curr_try);
result.SetAlgoCompleted(false);
break;
}
Expand All @@ -356,6 +359,7 @@ util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, c
if (curr_selection.empty()) {
// Exhausted search space before running into attempt limit
is_done = true;
result.SetSelectionsEvaluated(curr_try);
result.SetAlgoCompleted(true);
break;
}
Expand Down Expand Up @@ -724,10 +728,20 @@ void SelectionResult::SetAlgoCompleted(bool algo_completed) {
m_algo_completed = algo_completed;
}

bool SelectionResult::GetAlgoCompleted() {
bool SelectionResult::GetAlgoCompleted() const
{
return m_algo_completed;
}

void SelectionResult::SetSelectionsEvaluated(size_t attempts) {
m_selections_evaluated = attempts;
}

size_t SelectionResult::GetSelectionsEvaluated() const
{
return m_selections_evaluated;
}

CAmount SelectionResult::GetWaste() const
{
return *Assert(m_waste);
Expand Down
10 changes: 9 additions & 1 deletion src/wallet/coinselection.h
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,8 @@ struct SelectionResult
std::optional<CAmount> m_waste;
/** False if algorithm was cut short by hitting limit of attempts and solution is non-optimal */
bool m_algo_completed{true};
/** The count of selections that were evaluated by this coin selection attempt */
size_t m_selections_evaluated;
/** Total weight of the selected inputs */
int m_weight{0};
/** How much individual inputs overestimated the bump fees for the shared ancestry */
Expand Down Expand Up @@ -393,7 +395,13 @@ struct SelectionResult
void SetAlgoCompleted(bool algo_completed);

/** Get m_algo_completed */
bool GetAlgoCompleted();
bool GetAlgoCompleted() const;

/** Record the number of selections that were evaluated */
void SetSelectionsEvaluated(size_t attempts);

/** Get selections_evaluated */
size_t GetSelectionsEvaluated() const ;

/**
* Combines the @param[in] other selection result into 'this' selection result.
Expand Down
4 changes: 4 additions & 0 deletions src/wallet/test/coinselector_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1175,6 +1175,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests)
return available_coins;
});
BOOST_CHECK(res);
// If this takes more attempts, the algorithm has regressed
BOOST_CHECK_MESSAGE(res->GetSelectionsEvaluated() == 184, "Expected 184 attempts, but got " + std::to_string(res->GetSelectionsEvaluated()));
}

{
Expand All @@ -1195,6 +1197,8 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests)
add_coin(1 * COIN, 1, expected_result);
add_coin(1 * COIN, 2, expected_result);
BOOST_CHECK(EquivalentResult(expected_result, *res));
// If it takes more attempts, the algorithm has regressed
BOOST_CHECK_MESSAGE(res->GetSelectionsEvaluated() == 3, "Expected 3 attempts, but got " + std::to_string(res->GetSelectionsEvaluated()));
}
}

Expand Down

0 comments on commit c1ccd8b

Please sign in to comment.