wallet: avoid call bumpfeediscount with negative values #35633

pull polespinasa wants to merge 1 commits into bitcoin:master from polespinasa:2026-07-01-setbumpfeediscount changing 1 files +1 −1

polespinasa commented at 3:19 PM on July 1, 2026: member
in #34232 dergoegge reported an assertion fail in SetBumpFeeDiscount.

Context

The bump-fee discount in the savings that we can have when multiple UTXOs that we selected for our transaction share a common unconfirmed ancestor transaction.

We know we can have some savings because we first calculate the summed_bump_fees and then compare to the combined_bump_fee. For context:
- bump_fees: Extra fees that the new transaction must pay to contribute to get his ancestor confirmed.
- summed_bump_fees: The sum of each input ancestor bump_fee. If we have two inputs with unconfirmed ancestors A and B and both have a bump_fee = 100 then summed_bump_fees = 200.
- combined_bump_fee: Is the total bump_fees summed of all inputs, but taking into account shared ancestors. If A and B share the transaction ancestor then combined_bump_fee = 100 not 200 as the transaction must be bumped only once.
If summed_bumpfees > combined_bump_fee we are overestimating the bump_fee as we are counting multiple times the same ancestors so we can discount it using SetBumpFeeDiscount.

Problem

To calculate summed_bump_fees and combined_bump_fee we use two different fresh MiniMiner snapshots of the mempool. Because they are called in different moments the two snapshots of the mempool might be different. An artificial feerate decrease of an ancestor using prioritizesettransaction can make combined_bump_fee > summed_bump_fees creating a negative discount. This cause calling SetBumpFeeDiscount with a negative vaule triggering an assertion discount >= 0.

Fix

This PR fixes it by ensuring not only that a discount exist but also that is greater the 0.

Test

It is hard to manually trigger this race condition. dergoegge coded a patch and test to trigger it that can be used to test the fix. https://github.com/polespinasa/bitcoin/commit/5320e2fd215734ff3b7c59b07108ca5c5f7e075f

wallet: avoid call bumpfeediscount with negative values

ChooseSelectionResult computes the bump-fee discount as: summed_bump_fees - combined_bump_fee
Where summed_bump_fees is the sum of per-UTXO ancestor bump fees and combined_bump_fee is the
true combined cost taking into account shared ancestors.

Both variables use creates a fresh MiniMiner snapshot of the mempool. Because of that
the two snapshots of the mempool might be different. An artificial feerate decrease
of an ancestor using prioritizesettransaction can make combined_bump_fee > summed_bump_fees.
This cause calling bumpfeediscount with a negative vaule triggering an assertion >= 0.

This commit fixes this by only calling bumpfeediscount when the discount is strictly positive.

Co-authored-by: dergoegge <n.goeggi@gmail.com>

ccfa09e0c3

DrahtBot added the label Wallet on Jul 1, 2026
DrahtBot commented at 3:20 PM on July 1, 2026: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35633.



Reviews

See the guideline for information on the review process. A summary of reviews will appear here.
polespinasa commented at 3:21 PM on July 1, 2026: member

@dergoegge wanted to use your commit for the test but something was failing in the imports :)

Contributors

polespinasa

DrahtBot

Labels

Wallet

Linked (view graph)

#34232 wallet: SetBumpFeeDiscount(const CAmount): Assertion `discount >= 0' failed

wallet: avoid call bumpfeediscount with negative values #35633

Code Coverage & Benchmarks

Reviews