highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-11 14:38:29 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#25647: wallet: return change from SelectionResult

Browse Source 
4fef534428 wallet: use GetChange() when computing waste (S3RK)
87e0ef9031 wallet: use GetChange() in tx building (S3RK)
15e97a6886 wallet: add SelectionResult::GetChange (S3RK)
72cad28da0 wallet: calculate and store min_viable_change (S3RK)
e3210a7225 wallet: account for preselected inputs in target (S3RK)
f8e796348b wallet: add SelectionResult::Merge (S3RK)
06f558e4e2 wallet: accurate SelectionResult::m_target (S3RK)
c8cf08ea74 wallet: ensure m_min_change_target always covers change fee (S3RK)

Pull request description:

  Benefits:
  1. more accurate waste calculation for knapsack. Waste calculation is now consistent with tx building code. Before we always assumed change for knapsack even when the solution is changeless4.
  2. simpler tx building code. Only create change output when it's needed
  3. makes it easier to correctly account for fees for CPFP inputs (should be done in a follow up)

  In the first three commits we fix the code to accurately track selection target in `SelectionResult::m_target`
  Then we introduce new variable `min_change` that represents the minimum viable change amount
  Then we introduce `SelectionResult::GetChange()` which incapsulates dropping change for fee logic and uses correct values of `SelectionResult::m_target`
  Then we use `SelectionResult::GetChange()` in both tx building and waste calculation code

  This PR is a refactoring and shouldn't change the behaviour.
  There is only one known small change (arguably a bug fix). Before we dropped change output if it's smaller than `cost_of_change` after paying change fees. This is incorrect as `cost_of_change` already includes `change_fee`.

ACKs for top commit:
  achow101:
    ACK 4fef534428
  Xekyo:
    crACK 4fef534428
  furszy:
    Code review ACK 4fef5344
  w0xlt:
    ACK 4fef534428

Tree-SHA512: 31a7455d4129bc39a444da0f16ad478d690d4d9627b2b8fdb5605facc6488171926bf02f5d7d9a545b2b59efafcf5bb3d404005e4da15c7b44b3f7d441afb941
This commit is contained in:
Andrew Chow
2022-08-22 12:32:33 -04:00
parent 92bb7001d8 4fef534428
commit 2bd9aa5a44
5 changed files with 152 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
src/wallet/test/coinselector_tests.cpp
View File

@@ -248,9 +248,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
// Iteration exhaustion test
CAmount target = make_hard_case(17, utxo_pool);
BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), target, 0)); // Should exhaust
BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), target, 1)); // Should exhaust
target = make_hard_case(14, utxo_pool);
const auto result7 = SelectCoinsBnB(GroupCoins(utxo_pool), target, 0); // Should not exhaust
const auto result7 = SelectCoinsBnB(GroupCoins(utxo_pool), target, 1); // Should not exhaust
BOOST_CHECK(result7);
// Test same value early bailout optimization
@@ -289,8 +289,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
// Make sure that effective value is working in AttemptSelection when BnB is used
CoinSelectionParams coin_selection_params_bnb{
rand,
/*change_output_size=*/ 0,
/*change_spend_size=*/ 0,
/*change_output_size=*/ 31,
/*change_spend_size=*/ 68,
/*min_change_target=*/ 0,
/*effective_feerate=*/ CFeeRate(3000),
/*long_term_feerate=*/ CFeeRate(1000),
@@ -298,6 +298,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
/*tx_noinputs_size=*/ 0,
/*avoid_partial=*/ false,
};
coin_selection_params_bnb.m_change_fee = coin_selection_params_bnb.m_effective_feerate.GetFee(coin_selection_params_bnb.change_output_size);
coin_selection_params_bnb.m_cost_of_change = coin_selection_params_bnb.m_effective_feerate.GetFee(coin_selection_params_bnb.change_spend_size) + coin_selection_params_bnb.m_change_fee;
coin_selection_params_bnb.min_viable_change = coin_selection_params_bnb.m_effective_feerate.GetFee(coin_selection_params_bnb.change_spend_size);
{
std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase());
wallet->LoadWallet();
@@ -777,6 +780,8 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test)
/*tx_noinputs_size=*/ 0,
/*avoid_partial=*/ false,
};
cs_params.m_cost_of_change = 1;
cs_params.min_viable_change = 1;
CCoinControl cc;
const auto result = SelectCoins(*wallet, available_coins, target, cc, cs_params);
BOOST_CHECK(result);