diff options
author | Andrew Chow <github@achow101.com> | 2023-01-03 18:43:24 -0500 |
---|---|---|
committer | Andrew Chow <github@achow101.com> | 2023-01-03 18:53:36 -0500 |
commit | 3f8591d46b46cec2c4effc01f8822222087d74c4 (patch) | |
tree | 8851821207fd1c9ac8f3f4649008ce4817351c9f /src/wallet/coinselection.cpp | |
parent | 80fc1af096273c7eebbf0a2a607cb90c7dc5a208 (diff) | |
parent | 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 (diff) | |
download | bitcoin-3f8591d46b46cec2c4effc01f8822222087d74c4.tar.xz |
Merge bitcoin/bitcoin#26661: wallet: Coin Selection, return accurate error messages
76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 gui: create tx, launch error dialog if backend throws runtime_error (furszy)
f4d79477ff0946b0bd340ade9251fa38e3b95dd7 wallet: coin selection, add duplicated inputs checks (furszy)
0aa065b14e67592d5be8f46ebbe5d59a083ff0a5 wallet: return accurate error messages from Coin Selection (furszy)
7e8340ab1a970a14e180b1fcf420b46a5657b062 wallet: make SelectCoins flow return util::Result (furszy)
e5e147fe97f706e82bc51358f8bdc355f355be57 wallet: refactor eight consecutive 'AttemptSelection' calls into a loop (furszy)
Pull request description:
Work decoupled from #25806, which cleanup and improves the Coin Selection flow further.
Adding the capability to propagate specific error messages from the Coin Selection process to the user.
Instead of always returning the general "Insufficient funds" message which is not always accurate to what happened internally.
Letting us instruct the user how to proceed under certain circumstances.
The following error messages were added:
1) If the selection result exceeds the maximum transaction weight,
we now will return:
-> "The inputs size exceeds the maximum weight. Please try sending
a smaller amount or manually consolidating your wallet's UTXOs".
2) If the user pre-selected inputs and disallowed the automatic coin
selection process (no other inputs are allowed), we now will
return:
-> "The preselected coins total amount does not cover the transaction
target. Please allow other inputs to be automatically selected or include
more coins manually".
3) The double-counted preset inputs during Coin Selection error will now
throw an "internal bug detected" message instead of crashing the node.
The essence of this work comes from several comments:
1. https://github.com/bitcoin/bitcoin/pull/26560#discussion_r1037395665
2. https://github.com/bitcoin/bitcoin/pull/25729#discussion_r940619491
3. https://github.com/bitcoin/bitcoin/pull/25269#pullrequestreview-1135240825
4. https://github.com/bitcoin/bitcoin/issues/23144 (which is connected to #24845)
ACKs for top commit:
ishaanam:
crACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15
achow101:
ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15
aureleoules:
ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15
theStack:
ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 :city_sunrise:
Tree-SHA512: 9de30792d7a5849cae77747aa978e70390b66ee9d082779a56088a024f82e725b0af050e6603aece0ac8229f6d73bc471ba97b4ab69dc7eddf419f5f56ae89a5
Diffstat (limited to 'src/wallet/coinselection.cpp')
-rw-r--r-- | src/wallet/coinselection.cpp | 12 |
1 files changed, 6 insertions, 6 deletions
diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 59c6958d6b..e6ba89627c 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -448,7 +448,8 @@ void SelectionResult::Clear() void SelectionResult::AddInput(const OutputGroup& group) { - util::insert(m_selected_inputs, group.m_outputs); + // As it can fail, combine inputs first + InsertInputs(group.m_outputs); m_use_effective = !group.m_subtract_fee_outputs; m_weight += group.m_weight; @@ -456,7 +457,8 @@ void SelectionResult::AddInput(const OutputGroup& group) void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_fee_outputs) { - util::insert(m_selected_inputs, inputs); + // As it can fail, combine inputs first + InsertInputs(inputs); m_use_effective = !subtract_fee_outputs; m_weight += std::accumulate(inputs.cbegin(), inputs.cend(), 0, [](int sum, const auto& coin) { @@ -466,16 +468,14 @@ void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_f void SelectionResult::Merge(const SelectionResult& other) { - // Obtain the expected selected inputs count after the merge (for now, duplicates are not allowed) - const size_t expected_count = m_selected_inputs.size() + other.m_selected_inputs.size(); + // As it can fail, combine inputs first + InsertInputs(other.m_selected_inputs); m_target += other.m_target; m_use_effective |= other.m_use_effective; if (m_algo == SelectionAlgorithm::MANUAL) { m_algo = other.m_algo; } - util::insert(m_selected_inputs, other.m_selected_inputs); - assert(m_selected_inputs.size() == expected_count); m_weight += other.m_weight; } |