fix(wallet): reject txs exceeding MAX_STANDARD_TX_WEIGHT - #544
Open
cestercian wants to merge 1 commit into
Open
cestercian wants to merge 1 commit into
cestercian wants to merge 1 commit into
Conversation
create_tx and create_psbt assembled transactions without checking bitcoin::policy::MAX_STANDARD_TX_WEIGHT (400_000 WU). A drain of many small UTXOs could therefore produce a fully signed PSBT that every standard mempool rejects, while sign still returned Ok(true). After the unsigned tx is assembled, estimate the signed weight (unsigned weight plus each input's satisfaction weight) and return CreateTxError::TxWeightLimitExceeded / CreatePsbtError::TxWeightLimitExceeded when it exceeds the limit. Fixes bitcoindevkit#543
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
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
Neither
create_txnorcreate_psbtchecked the assembled transaction against Bitcoin's standardness weight limit (bitcoin::policy::MAX_STANDARD_TX_WEIGHT, 400_000 WU). Dust was already rejected (OutputBelowDustLimit); weight was not.A drain of many small UTXOs could therefore produce a fully signed PSBT over 400k WU. Every standardness-enforcing mempool rejects that transaction, but
signstill returnedOk(true)— a misleading success.Root cause: After coin selection the unsigned transaction is assembled with empty witnesses.
Transaction::weight()on that value undercounts the final signed size by each input's satisfaction (witness / scriptSig) weight. No later check compared the estimated signed weight to the standardness limit.Fix: After the unsigned tx is assembled, estimate the signed weight (
tx.weight()plus each input's satisfaction weight, plus the 2-WU segwit marker when witnesses will be present) and reject when it exceedsMAX_STANDARD_TX_WEIGHT.CreateTxError::TxWeightLimitExceeded { weight, limit }(stablecreate_txpath)CreatePsbtError::TxWeightLimitExceeded { weight, limit }(unstablecreate_psbt/create_psbt_from_selectorpath, so RBF is covered too)Tests:
create_psbtdrain regressionFixes #543
Notes to the reviewers
CreateTxErroris exhaustive, so downstreammatches must handle the new variant.CreatePsbtErroris already#[non_exhaustive].tx.weight(). Checking only the unsigned weight would miss the 1,500-P2WPKH-input case (~246k WU unsigned vs ~408k WU signed).create_txcome from theWeightedUtxos used in coin selection (including foreign UTXOs). Thecreate_psbtpath looks up local descriptors viamax_weight_to_satisfy.Changelog notice
create_tx/create_psbtproducing unrelayable transactions overMAX_STANDARD_TX_WEIGHTby returningTxWeightLimitExceeded.Before submitting
Assistance: implementation drafted with Cursor (cloud agent); author is Cestercian. Commits are SSH-signed; GitHub may show Unverified if the cloud-agent SSH signing key is not registered on the account as a signing key.