fix(psbt): return None from fee_amount on overflow or negative fee - #564
Open
shubham5080 wants to merge 1 commit into
Open
shubham5080 wants to merge 1 commit into
shubham5080 wants to merge 1 commit into
Conversation
PsbtUtils::fee_amount summed Amounts with Iterator::sum and then checked_sub(...).expect(...). A PSBT whose outputs exceed its inputs panicked, and summing past u64::MAX overflowed (panic in debug, wrap in release). fee_rate delegates to fee_amount so it hit the same bugs. Use checked_add / checked_sub and return None when the fee cannot be computed. Callers already treat None as "unknown fee". Fixes bitcoindevkit#558 Fixes bitcoindevkit#559
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
PsbtUtils::fee_amountpanics or wraps on hostile/inconsistent PSBTs:checked_sub(..).expect(..)(PsbtUtils::fee_amountpanics when PSBT outputs exceed inputs #558)u64::MAXoverflowsIterator::sum(PsbtUtils::fee_amountoverflows when summing input or output amounts #559)fee_ratedelegates tofee_amount, so it fails the same way. PSBTs often come from another party (coinjoin, payjoin, an external builder), so this is reachable before signing.This change uses
checked_add/checked_suband returnsNonewhen the fee cannot be computed. Callers already treatNoneas "unknown fee".Fixes #558
Fixes #559
Notes to the reviewers
Kept using
get_utxo_forrather than delegating to rust-bitcoin'sPsbt::fee(), so the existing txid / vout checks onnon_witness_utxostill apply.Return type is unchanged (
Option<Amount>). Invalid PSBTs that previously panicked now returnNone.Changelog notice
Fixed
fee_amount/fee_ratereturnNoneinstead of panicking when outputs exceed inputs or when summing amounts overflows (PsbtUtils::fee_amountpanics when PSBT outputs exceed inputs #558,PsbtUtils::fee_amountoverflows when summing input or output amounts #559)Before submitting