highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-06-06 19:23:41 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#34299: wallet: remove PreSelectedInputs and re-activate "AmountWithFeeExceedsBalance" error

Browse Source 
48161f6a05 wallet: introduce "tx amount exceeds balance when fees are included" error (stratospher)
b7fa609ed1 wallet: remove PreSelectedInputs (stratospher)
7819da2c16 walllet: use CoinsResult instead of PreSelectedInputs (stratospher)
e5474079f1 wallet: introduce GetAppropriateTotal() in CoinsResult (stratospher)
d8ea921d01 wallet: correctly reserve in CoinsResult::All() (stratospher)
7072d825e3 wallet: ensure COutput added in set are unique (stratospher)
fefa3be782 wallet: fix, make 'total_effective_amount' optional actually optional (stratospher)

Pull request description:

  picks up https://github.com/bitcoin/bitcoin/pull/25269.

  This PR re-implements the code path so that an error message is thrown when a transaction's total amount (including fees) exceeds the available balance. It also refactors the wallet's coin selection code.

  1. the first 3 commits are unrelated to the code but few small bug fixes which are nice to fix. but also kind of impacts the remaining logic. (could PR separately if reviewers wish)
  1. c467325aaf187d7f056bb1ea1cec6b7c4250af2e: make `total_effective_amount` optional actually optional
  2. 2202ab597596c84fc49f8784e823372b7a9efcbe: ensure `set<shared_ptr<COutput>>` has unique COutput
  3. a5ffbbf122d66fc4ad9b2e7c6d7d1dfa1816388e: Correctly reserve size when flattening `CoinsResult.coins` map to vector

  3. the next 3 commits from 4745d5480ca5c3809edd51140e4d2c0433582844 replace the `PreSelectedInputs` struct with `CoinsResult` and removes `PreSelectedInputs`.

  4. the last commit (e664484a6d34c1795ebb0925ab31faea5d64ab00) deals with the error message - `AmountWithFeeExceedsBalance` error inside `WalletModel::prepareTransaction` is never thrown and remains an unused code path. This is because `createTransaction` does not retrieve the fee when the process fails. The fee return arg is set only at the end of the process, when the transaction is successfully created. Therefore, if the transaction creation fails, the fee is not available inside `WalletModel::prepareTransaction` to trigger the `AmountWithFeeExceedsBalance` error.

  This PR re-implements the feature inside `CreateTransactionInternal` and adds test coverage for it.

  | on master | on PR |
  |-----------|-------|
  | <img src="https://github.com/user-attachments/assets/a903e687-2466-42c7-b898-5dec24bfe515" width="750" alt="Insufficient funds" /> | <img src="https://github.com/user-attachments/assets/74bb3c83-6132-4c09-91f0-0a446618b3c8" width="750" alt="AmountWithFeeExceedsBalance" /> |

  the unreachable code path is removed in https://github.com/bitcoin-core/gui/pull/807 which requires this PR.

ACKs for top commit:
  achow101:
    ACK 48161f6a05
  furszy:
    utACK 48161f6

Tree-SHA512: a963fac8d6714f76571df8cf9aff70601536dc6faa4326fbb5892c3f080dc393f0d7c6e2d21879c7a2c898bf0092adb154376d9b0a8929b31575ce9d1d47dec2
This commit is contained in:
Ava Chow
2026-02-06 14:30:20 -08:00
parent 28160c1e3d 48161f6a05
commit d88997b809
10 changed files with 119 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -30,8 +30,6 @@ BOOST_FIXTURE_TEST_SUITE(coinselector_tests, WalletTestingSetup)
// we repeat those tests this many times and only complain if all iterations of the test fail
#define RANDOM_REPEATS 5
typedef std::set<std::shared_ptr<COutput>> CoinSet;
static const CoinEligibilityFilter filter_standard(1, 6, 0);
static const CoinEligibilityFilter filter_confirmed(1, 1, 0);
static const CoinEligibilityFilter filter_standard_extra(6, 6, 0);
@@ -117,7 +115,7 @@ static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b)
/** Check if this selection is equal to another one. Equal means same inputs (i.e same value and prevout) */
static bool EqualResult(const SelectionResult& a, const SelectionResult& b)
{
std::pair<CoinSet::iterator, CoinSet::iterator> ret = std::mismatch(a.GetInputSet().begin(), a.GetInputSet().end(), b.GetInputSet().begin(),
std::pair<OutputSet::iterator, OutputSet::iterator> ret = std::mismatch(a.GetInputSet().begin(), a.GetInputSet().end(), b.GetInputSet().begin(),
[](const std::shared_ptr<COutput>& a, const std::shared_ptr<COutput>& b) {
return a->outpoint == b->outpoint;
});
@@ -224,8 +222,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
coin_control.m_allow_other_inputs = true;
COutput select_coin = available_coins.All().at(0);
coin_control.Select(select_coin.outpoint);
PreSelectedInputs selected_input;
selected_input.Insert(select_coin, coin_selection_params_bnb.m_subtract_fee_outputs);
CoinsResult selected_input;
selected_input.Add(OutputType::BECH32, select_coin);
available_coins.Erase({available_coins.coins[OutputType::BECH32].begin()->outpoint});
LOCK(wallet->cs_wallet);
@@ -255,8 +253,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
coin_control.m_allow_other_inputs = true;
COutput select_coin = available_coins.All().at(1); // pre select 9 coin
coin_control.Select(select_coin.outpoint);
PreSelectedInputs selected_input;
selected_input.Insert(select_coin, coin_selection_params_bnb.m_subtract_fee_outputs);
CoinsResult selected_input;
selected_input.Add(OutputType::BECH32, select_coin);
available_coins.Erase({(++available_coins.coins[OutputType::BECH32].begin())->outpoint});
const auto result13 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb);
BOOST_CHECK(EquivalentResult(expected_result, *result13));
@@ -1292,7 +1290,7 @@ static util::Result<SelectionResult> select_coins(const CAmount& target, const C
return result;
}
static bool has_coin(const CoinSet& set, CAmount amount)
static bool has_coin(const OutputSet& set, CAmount amount)
{
return std::any_of(set.begin(), set.end(), [&](const auto& coin) { return coin->GetEffectiveValue() == amount; });
}

6
src/wallet/test/fuzz/coinselection.cpp
View File

@@ -68,7 +68,7 @@ static CAmount CreateCoins(FuzzedDataProvider& fuzzed_data_provider, std::vector
static SelectionResult ManualSelection(std::vector<COutput>& utxos, const CAmount& total_amount, const bool& subtract_fee_outputs)
{
SelectionResult result(total_amount, SelectionAlgorithm::MANUAL);
std::set<std::shared_ptr<COutput>> utxo_pool;
OutputSet utxo_pool;
for (const auto& utxo : utxos) {
utxo_pool.insert(std::make_shared<COutput>(utxo));
}
@@ -445,7 +445,7 @@ void FuzzCoinSelectionAlgorithm(std::span<const uint8_t> buffer) {
std::vector<COutput> utxos;
CAmount new_total_balance{CreateCoins(fuzzed_data_provider, utxos, coin_params, next_locktime)};
if (new_total_balance > 0) {
std::set<std::shared_ptr<COutput>> new_utxo_pool;
OutputSet new_utxo_pool;
for (const auto& utxo : utxos) {
new_utxo_pool.insert(std::make_shared<COutput>(utxo));
}
@@ -462,7 +462,7 @@ void FuzzCoinSelectionAlgorithm(std::span<const uint8_t> buffer) {
auto manual_selection{ManualSelection(manual_inputs, manual_balance, coin_params.m_subtract_fee_outputs)};
if (result) {
const CAmount old_target{result->GetTarget()};
const std::set<std::shared_ptr<COutput>> input_set{result->GetInputSet()};
const OutputSet input_set{result->GetInputSet()};
const int old_weight{result->GetWeight()};
result->Merge(manual_selection);
assert(result->GetInputSet().size() == input_set.size() + manual_inputs.size());