From 85fd335fcae0549ec64aff1d5ab987840d279adf Mon Sep 17 00:00:00 2001 From: Cestercian Date: Sun, 6 Sep 2026 07:48:08 +0000 Subject: [PATCH 1/3] fix(wallet): reject txs exceeding MAX_STANDARD_TX_WEIGHT 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 #543 --- src/wallet/error.rs | 36 +++++++++++++- src/wallet/mod.rs | 69 +++++++++++++++++++++++++ tests/create_psbt.rs | 80 ++++++++++++++++++++++++++++- tests/wallet.rs | 116 ++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 297 insertions(+), 4 deletions(-) diff --git a/src/wallet/error.rs b/src/wallet/error.rs index 1eb8fbc3..d97c9cfe 100644 --- a/src/wallet/error.rs +++ b/src/wallet/error.rs @@ -21,7 +21,7 @@ use alloc::{ }; #[cfg(all(bdk_wallet_unstable, feature = "bdk-tx"))] use bdk_tx::bdk_coin_select; -use bitcoin::{Amount, BlockHash, Network, OutPoint, Sequence, Txid, absolute, psbt}; +use bitcoin::{Amount, BlockHash, Network, OutPoint, Sequence, Txid, Weight, absolute, psbt}; use core::fmt; /// The error type when loading a [`Wallet`] from a [`ChangeSet`]. @@ -199,6 +199,17 @@ pub enum CreateTxError { NoUtxosSelected, /// Output created is under the dust limit, 546 satoshis OutputBelowDustLimit(usize), + /// Assembled transaction exceeds [`bitcoin::policy::MAX_STANDARD_TX_WEIGHT`]. + /// + /// The reported `weight` is the estimated signed weight (unsigned transaction plus each + /// input's satisfaction weight). Standard mempools reject transactions above this limit, + /// so building them is treated as an error. + TxWeightLimitExceeded { + /// Estimated signed transaction weight. + weight: Weight, + /// Standardness weight limit that was exceeded. + limit: Weight, + }, /// There was an error with coin selection CoinSelection(coin_selection::InsufficientFunds), /// Cannot build a tx without recipients @@ -269,6 +280,12 @@ impl fmt::Display for CreateTxError { CreateTxError::OutputBelowDustLimit(limit) => { write!(f, "Output below the dust limit: {limit}") } + CreateTxError::TxWeightLimitExceeded { weight, limit } => { + write!( + f, + "Transaction weight {weight} exceeds the standardness limit of {limit}" + ) + } CreateTxError::CoinSelection(e) => e.fmt(f), CreateTxError::NoRecipients => { write!(f, "Cannot build tx without recipients") @@ -387,6 +404,17 @@ pub enum CreatePsbtError { /// After coin selection, all outputs fell below the dust threshold and were /// dropped to fees. AllOutputsBelowDust, + /// Assembled transaction exceeds [`bitcoin::policy::MAX_STANDARD_TX_WEIGHT`]. + /// + /// The reported `weight` is the estimated signed weight (unsigned transaction plus each + /// input's satisfaction weight). Standard mempools reject transactions above this limit, + /// so building them is treated as an error. + TxWeightLimitExceeded { + /// Estimated signed transaction weight. + weight: Weight, + /// Standardness weight limit that was exceeded. + limit: Weight, + }, /// Non-sufficient funds. InsufficientFunds(bdk_coin_select::InsufficientFunds), /// In order to use the [`add_global_xpubs`] option, every extended key in the descriptor must @@ -414,6 +442,12 @@ impl fmt::Display for CreatePsbtError { Self::InsufficientFunds(e) => write!(f, "{e}"), Self::NoRecipients => write!(f, "no output destinations were configured"), Self::AllOutputsBelowDust => write!(f, "all outputs are below the dust threshold",), + Self::TxWeightLimitExceeded { weight, limit } => { + write!( + f, + "transaction weight {weight} exceeds the standardness limit of {limit}" + ) + } Self::MissingKeyOrigin(e) => write!(f, "missing key origin: {e}"), Self::Plan(op) => write!(f, "failed to create a plan for txout with outpoint {op}"), Self::Psbt(e) => write!(f, "{e}"), diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index d559c784..8f93b1a6 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -1505,6 +1505,12 @@ impl Wallet { } }; + let satisfaction_by_outpoint: HashMap = required_utxos + .iter() + .chain(optional_utxos.iter()) + .map(|weighted| (weighted.utxo.outpoint(), weighted.satisfaction_weight)) + .collect(); + // Get drain script. let mut drain_index = Option::<(KeychainKind, u32)>::None; let drain_script = match params.drain_to { @@ -1599,6 +1605,18 @@ impl Wallet { // Sort inputs/outputs according to the chosen algorithm. params.ordering.sort_tx_with_aux_rand(&mut tx, rng); + // Reject transactions that would exceed the standardness weight limit once signed. + // `tx.weight()` is the unsigned weight (empty witnesses); add each input's satisfaction + // weight so we compare against the same limit mempools enforce on the final tx. + let satisfaction_weights = tx.input.iter().map(|txin| { + satisfaction_by_outpoint + .get(&txin.previous_output) + .copied() + .unwrap_or_else(|| self.satisfaction_weight_for_outpoint(txin.previous_output)) + }); + check_max_standard_tx_weight(&tx, satisfaction_weights) + .map_err(|(weight, limit)| CreateTxError::TxWeightLimitExceeded { weight, limit })?; + let psbt = self.complete_transaction(tx, coin_selection.selected, params)?; // Recording changes to the change keychain. @@ -2944,6 +2962,48 @@ impl Wallet { }) .collect() } + + /// Satisfaction weight used to estimate the signed size of spending `outpoint`. + /// + /// Local wallet UTXOs use the descriptor's [`max_weight_to_satisfy`]. Foreign or unknown + /// outpoints return [`Weight::ZERO`], which underestimates; callers that still have the + /// original [`WeightedUtxo`] should prefer that value. + /// + /// [`max_weight_to_satisfy`]: miniscript::Descriptor::max_weight_to_satisfy + fn satisfaction_weight_for_outpoint(&self, outpoint: OutPoint) -> Weight { + self.get_utxo(outpoint) + .and_then(|utxo| { + self.public_descriptor(utxo.keychain) + .max_weight_to_satisfy() + .ok() + }) + .unwrap_or(Weight::ZERO) + } +} + +/// Estimate the signed weight of an assembled unsigned transaction and reject it when it +/// exceeds [`bitcoin::policy::MAX_STANDARD_TX_WEIGHT`]. +/// +/// `tx` is unsigned (empty witnesses). `satisfaction_weights` are the additional scriptSig / +/// witness weights for each input. Empty-witness transactions serialize without the 2-WU +/// segwit marker/flag; that is added once any satisfaction weight is present. +fn check_max_standard_tx_weight( + tx: &Transaction, + satisfaction_weights: impl IntoIterator, +) -> Result<(), (Weight, Weight)> { + let satisfaction: Weight = satisfaction_weights.into_iter().sum(); + let segwit_marker = if satisfaction > Weight::ZERO { + Weight::from_wu(2) + } else { + Weight::ZERO + }; + let weight = tx.weight() + satisfaction + segwit_marker; + let limit = Weight::from_wu(u64::from(bitcoin::policy::MAX_STANDARD_TX_WEIGHT)); + if weight > limit { + Err((weight, limit)) + } else { + Ok(()) + } } /// Methods to construct sync/full-scan requests for spk-based chain sources. @@ -3220,6 +3280,7 @@ impl Wallet { /// - A manually selected input is missing from the wallet, or could not be planned /// - The input value is insufficient to fund the outputs /// - Failure to complete coin selection + /// - The assembled transaction exceeds [`bitcoin::policy::MAX_STANDARD_TX_WEIGHT`] /// - Failure to create or update the PSBT. /// /// # Change address @@ -3424,6 +3485,14 @@ impl Wallet { ) .map_err(CreatePsbtError::Psbt)?; + let satisfaction_weights = psbt + .unsigned_tx + .input + .iter() + .map(|txin| self.satisfaction_weight_for_outpoint(txin.previous_output)); + check_max_standard_tx_weight(&psbt.unsigned_tx, satisfaction_weights) + .map_err(|(weight, limit)| CreatePsbtError::TxWeightLimitExceeded { weight, limit })?; + // Add global xpubs. if params.add_global_xpubs { for xpub in self diff --git a/tests/create_psbt.rs b/tests/create_psbt.rs index cd881d82..b29bd51b 100644 --- a/tests/create_psbt.rs +++ b/tests/create_psbt.rs @@ -1,6 +1,8 @@ //! Integration tests for the unstable `create_psbt` and `replace_by_fee` APIs. #![cfg(all(bdk_wallet_unstable, feature = "bdk-tx"))] +use std::str::FromStr; + use bdk_chain::{BlockId, ConfirmationBlockTime}; use bdk_tx::{ChangeScript, bdk_coin_select}; use bdk_wallet::bitcoin; @@ -9,8 +11,8 @@ use bdk_wallet::{ KeychainKind, PsbtParams, SelectionStrategy, Wallet, error::CreatePsbtError, psbt, }; use bitcoin::{ - Amount, FeeRate, Network, OutPoint, ScriptBuf, Sequence, Transaction, TxIn, TxOut, absolute, - hashes::Hash, + Address, Amount, BlockHash, FeeRate, Network, OutPoint, ScriptBuf, Sequence, Transaction, TxIn, + TxOut, Weight, absolute, hashes::Hash, transaction, }; use miniscript::plan::Assets; @@ -1211,3 +1213,77 @@ fn test_add_planned_psbt_input() -> anyhow::Result<()> { Ok(()) } + +/// Fund `wallet` with `n` confirmed outputs of `value` in a single transaction. +fn fund_wallet_with_n_utxos(wallet: &mut Wallet, n: u32, value: Amount) { + let last_index = n.saturating_sub(1); + let _revealed: Vec<_> = wallet + .reveal_addresses_to(KeychainKind::External, last_index) + .collect(); + + let outputs = (0..n) + .map(|i| TxOut { + script_pubkey: wallet + .peek_address(KeychainKind::External, i) + .script_pubkey(), + value, + }) + .collect(); + + let tx = Transaction { + version: transaction::Version::ONE, + lock_time: absolute::LockTime::ZERO, + input: vec![], + output: outputs, + }; + + let height = wallet.latest_checkpoint().height() + 1; + let mut hash_bytes = [0u8; 32]; + hash_bytes[0] = 0x42; + hash_bytes[1] = (height % 256) as u8; + insert_tx_anchor( + wallet, + tx, + BlockId { + height, + hash: BlockHash::from_byte_array(hash_bytes), + }, + ); +} + +#[test] +fn test_create_psbt_rejects_over_max_standard_tx_weight() { + let (desc, change_desc) = get_test_wpkh_and_change_desc(); + let mut wallet = Wallet::create(desc, change_desc) + .network(Network::Regtest) + .create_wallet_no_persist() + .unwrap(); + + // ~1,500 P2WPKH inputs ≈ 408k WU once satisfied (issue #543). + const N_UTXOS: u32 = 1_500; + fund_wallet_with_n_utxos(&mut wallet, N_UTXOS, Amount::from_sat(1_000)); + assert_eq!(wallet.list_unspent().count(), N_UTXOS as usize); + + let drain_addr = Address::from_str("bcrt1q3qtze4ys45tgdvguj66zrk4fu6hq3a3v9pfly5") + .unwrap() + .assume_checked(); + + let mut params = PsbtParams::default(); + params.fee_rate(FeeRate::BROADCAST_MIN); + params.coin_selection(SelectionStrategy::All); + params.change_script(ChangeScript::from_script( + drain_addr.script_pubkey(), + Weight::from_wu(107), + )); + + let err = wallet.create_psbt(params).expect_err("overweight drain"); + assert!( + matches!( + err, + CreatePsbtError::TxWeightLimitExceeded { weight, limit } + if weight > limit + && limit == Weight::from_wu(u64::from(bitcoin::policy::MAX_STANDARD_TX_WEIGHT)) + ), + "expected TxWeightLimitExceeded, got: {err:?}" + ); +} diff --git a/tests/wallet.rs b/tests/wallet.rs index 557bd2bb..ace8c356 100644 --- a/tests/wallet.rs +++ b/tests/wallet.rs @@ -24,7 +24,7 @@ use bitcoin::sighash::{EcdsaSighashType, TapSighashType}; use bitcoin::taproot::TapNodeHash; use bitcoin::{ Address, Amount, BlockHash, FeeRate, Network, OutPoint, ScriptBuf, Sequence, SignedAmount, - Transaction, TxIn, TxOut, Txid, absolute, transaction, + Transaction, TxIn, TxOut, Txid, WPubkeyHash, Weight, absolute, psbt, transaction, }; use miniscript::descriptor::KeyMapWrapper; use rand::SeedableRng; @@ -3697,3 +3697,117 @@ fn test_create_and_spend_from_truc_tx() -> anyhow::Result<()> { Ok(()) } + +/// Fund `wallet` with `n` confirmed outputs of `value` in a single transaction. +fn fund_wallet_with_n_utxos(wallet: &mut Wallet, n: u32, value: Amount) { + let last_index = n.saturating_sub(1); + let _revealed: Vec<_> = wallet + .reveal_addresses_to(KeychainKind::External, last_index) + .collect(); + + let outputs = (0..n) + .map(|i| TxOut { + script_pubkey: wallet + .peek_address(KeychainKind::External, i) + .script_pubkey(), + value, + }) + .collect(); + + let tx = Transaction { + version: transaction::Version::ONE, + lock_time: absolute::LockTime::ZERO, + input: vec![], + output: outputs, + }; + + let height = wallet.latest_checkpoint().height() + 1; + let mut hash_bytes = [0u8; 32]; + hash_bytes[0] = 0x42; + hash_bytes[1] = (height % 256) as u8; + insert_tx_anchor( + wallet, + tx, + BlockId { + height, + hash: BlockHash::from_byte_array(hash_bytes), + }, + ); +} + +#[test] +fn test_create_tx_rejects_over_max_standard_tx_weight() { + // A drain of many small P2WPKH UTXOs is the reported failure mode: the unsigned + // transaction stays under the limit, but the estimated signed weight does not. + let (desc, change_desc) = get_test_wpkh_and_change_desc(); + let mut wallet = Wallet::create(desc, change_desc) + .network(Network::Regtest) + .create_wallet_no_persist() + .unwrap(); + + // ~1,500 P2WPKH inputs ≈ 408k WU once satisfied (issue #543). + const N_UTXOS: u32 = 1_500; + fund_wallet_with_n_utxos(&mut wallet, N_UTXOS, Amount::from_sat(1_000)); + assert_eq!(wallet.list_unspent().count(), N_UTXOS as usize); + + let drain_addr = Address::from_str("bcrt1q3qtze4ys45tgdvguj66zrk4fu6hq3a3v9pfly5") + .unwrap() + .assume_checked(); + let mut builder = wallet.build_tx(); + builder + .drain_wallet() + .drain_to(drain_addr.script_pubkey()) + .fee_rate(FeeRate::BROADCAST_MIN); + + assert_matches!( + builder.finish(), + Err(CreateTxError::TxWeightLimitExceeded { weight, limit }) + if weight > limit + && limit == Weight::from_wu(u64::from(bitcoin::policy::MAX_STANDARD_TX_WEIGHT)) + ); +} + +#[test] +fn test_create_tx_rejects_over_max_standard_tx_weight_foreign_satisfaction() { + // Unit-level construction: a single foreign input whose satisfaction weight + // alone pushes the estimated signed weight over the standardness limit. + let (mut wallet, _) = get_funded_wallet_wpkh(); + let addr = wallet.next_unused_address(KeychainKind::External); + + let foreign_prev_tx = Transaction { + version: transaction::Version::TWO, + lock_time: absolute::LockTime::ZERO, + input: vec![], + output: vec![TxOut { + value: Amount::from_sat(100_000), + script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()), + }], + }; + let outpoint = OutPoint { + txid: foreign_prev_tx.compute_txid(), + vout: 0, + }; + let psbt_input = psbt::Input { + witness_utxo: Some(foreign_prev_tx.output[0].clone()), + non_witness_utxo: Some(foreign_prev_tx), + ..Default::default() + }; + + let mut builder = wallet.build_tx(); + builder + .add_recipient(addr.script_pubkey(), Amount::from_sat(25_000)) + .only_witness_utxo() + .add_foreign_utxo( + outpoint, + psbt_input, + Weight::from_wu(u64::from(bitcoin::policy::MAX_STANDARD_TX_WEIGHT)), + ) + .unwrap(); + + assert_matches!( + builder.finish(), + Err(CreateTxError::TxWeightLimitExceeded { weight, limit }) + if weight > limit + && limit == Weight::from_wu(u64::from(bitcoin::policy::MAX_STANDARD_TX_WEIGHT)) + ); +} From 6677a0f078def06b11beb4d93e3593d266419282 Mon Sep 17 00:00:00 2001 From: Cestercian Date: Wed, 23 Sep 2026 04:10:05 +0000 Subject: [PATCH 2/3] fix(wallet): correct standardness weight for planned inputs create_psbt ignored satisfaction weights on foreign and planned inputs, the BIP 141 witness overhead was off by a marker or one unit per input, and unchecked Weight addition could panic or wrap past the limit. Count satisfaction from the selected input, add witness overhead only when an input actually has a witness, and use checked addition so an overflow is TxWeightLimitExceeded. --- src/wallet/mod.rs | 277 +++++++++++++++++++++++++++++++++++++------ tests/create_psbt.rs | 65 ++++++++++ tests/wallet.rs | 44 +++++++ 3 files changed, 350 insertions(+), 36 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 8f93b1a6..4c623fa5 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -1505,10 +1505,10 @@ impl Wallet { } }; - let satisfaction_by_outpoint: HashMap = required_utxos + let satisfaction_by_outpoint: HashMap = required_utxos .iter() .chain(optional_utxos.iter()) - .map(|weighted| (weighted.utxo.outpoint(), weighted.satisfaction_weight)) + .map(|weighted| (weighted.utxo.outpoint(), self.input_satisfaction(weighted))) .collect(); // Get drain script. @@ -1606,15 +1606,15 @@ impl Wallet { params.ordering.sort_tx_with_aux_rand(&mut tx, rng); // Reject transactions that would exceed the standardness weight limit once signed. - // `tx.weight()` is the unsigned weight (empty witnesses); add each input's satisfaction - // weight so we compare against the same limit mempools enforce on the final tx. - let satisfaction_weights = tx.input.iter().map(|txin| { + // `tx.weight()` is the unsigned weight (empty witnesses). Each input contributes the + // satisfaction weight tracked on its selection candidate, including foreign UTXOs. + let satisfactions = tx.input.iter().map(|txin| { satisfaction_by_outpoint .get(&txin.previous_output) .copied() - .unwrap_or_else(|| self.satisfaction_weight_for_outpoint(txin.previous_output)) + .unwrap_or_else(|| self.satisfaction_for_outpoint(txin.previous_output)) }); - check_max_standard_tx_weight(&tx, satisfaction_weights) + check_max_standard_tx_weight(&tx, satisfactions) .map_err(|(weight, limit)| CreateTxError::TxWeightLimitExceeded { weight, limit })?; let psbt = self.complete_transaction(tx, coin_selection.selected, params)?; @@ -2963,42 +2963,167 @@ impl Wallet { .collect() } - /// Satisfaction weight used to estimate the signed size of spending `outpoint`. + /// Satisfaction of a coin-selection candidate, including foreign UTXOs. /// - /// Local wallet UTXOs use the descriptor's [`max_weight_to_satisfy`]. Foreign or unknown - /// outpoints return [`Weight::ZERO`], which underestimates; callers that still have the - /// original [`WeightedUtxo`] should prefer that value. + /// Local inputs use the descriptor's [`max_weight_to_satisfy`]. Foreign inputs use the weight + /// supplied with the UTXO. Segwit (native or nested) is recorded separately so the standardness + /// check can apply BIP 141 witness overhead. /// /// [`max_weight_to_satisfy`]: miniscript::Descriptor::max_weight_to_satisfy - fn satisfaction_weight_for_outpoint(&self, outpoint: OutPoint) -> Weight { - self.get_utxo(outpoint) - .and_then(|utxo| { - self.public_descriptor(utxo.keychain) - .max_weight_to_satisfy() - .ok() - }) - .unwrap_or(Weight::ZERO) + fn input_satisfaction(&self, weighted: &WeightedUtxo) -> InputSatisfaction { + let segwit = match &weighted.utxo { + Utxo::Local(local) => self.descriptor_spends_with_witness(local.keychain), + Utxo::Foreign { psbt_input, .. } => spends_with_witness( + weighted.utxo.txout().script_pubkey.as_script(), + Some(psbt_input), + ), + }; + InputSatisfaction { + weight: weighted.satisfaction_weight, + segwit, + } + } + + /// Fallback satisfaction when an outpoint was not part of coin selection. + /// + /// Local wallet UTXOs use the descriptor's [`max_weight_to_satisfy`]. Unknown outpoints + /// contribute nothing. + /// + /// [`max_weight_to_satisfy`]: miniscript::Descriptor::max_weight_to_satisfy + fn satisfaction_for_outpoint(&self, outpoint: OutPoint) -> InputSatisfaction { + let Some(utxo) = self.get_utxo(outpoint) else { + return InputSatisfaction { + weight: Weight::ZERO, + segwit: false, + }; + }; + let weight = self + .public_descriptor(utxo.keychain) + .max_weight_to_satisfy() + .unwrap_or(Weight::ZERO); + InputSatisfaction { + weight, + segwit: self.descriptor_spends_with_witness(utxo.keychain), + } } + + /// Whether spends of this keychain serialize a witness (native or nested segwit, or taproot). + fn descriptor_spends_with_witness(&self, keychain: KeychainKind) -> bool { + let desc = self.public_descriptor(keychain); + desc.is_witness() || desc.is_taproot() + } +} + +/// Extra weight of satisfying one input, and whether that satisfaction is a witness. +/// +/// `weight` is the miniscript [`max_weight_to_satisfy`] delta: the difference between +/// `TxIn::default().segwit_weight()` and the satisfied input. That delta does not include the +/// 1-WU empty witness stack length already assumed by `segwit_weight`. +/// +/// [`max_weight_to_satisfy`]: miniscript::Descriptor::max_weight_to_satisfy +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct InputSatisfaction { + weight: Weight, + /// `true` when satisfying this input serializes a witness (native segwit, nested segwit, or + /// taproot). Pure legacy scriptSig satisfaction is `false`. + segwit: bool, +} + +/// Whether `script_pubkey` (or the PSBT metadata spending it) puts satisfaction in a witness. +/// +/// Native segwit and taproot are witness programs. Nested segwit is a P2SH script pubkey, so it +/// is recognized from the PSBT input: a witness UTXO, witness script, final witness, or a redeem +/// script that is itself a witness program. +fn spends_with_witness(script_pubkey: &bitcoin::Script, psbt_input: Option<&psbt::Input>) -> bool { + if script_pubkey.witness_version().is_some() { + return true; + } + let Some(psbt_input) = psbt_input else { + return false; + }; + if psbt_input.witness_utxo.is_some() + || psbt_input.final_script_witness.is_some() + || psbt_input.witness_script.is_some() + { + return true; + } + psbt_input + .redeem_script + .as_ref() + .is_some_and(|redeem| redeem.witness_version().is_some()) +} + +/// [`bdk_tx::Input`] satisfaction, using the weight already tracked on the selected input. +/// +/// Planned inputs created from a PSBT do not report [`Input::is_segwit`] unless +/// `final_script_witness` is set, so witness-ness also comes from the prevout and PSBT metadata. +#[cfg(all(bdk_wallet_unstable, feature = "bdk-tx"))] +fn selection_input_satisfaction(input: &Input) -> InputSatisfaction { + let segwit = if input.plan().is_some() { + input.is_segwit() + } else { + spends_with_witness( + input.prev_txout().script_pubkey.as_script(), + input.psbt_input(), + ) + }; + InputSatisfaction { + weight: Weight::from_wu(input.satisfaction_weight()), + segwit, + } +} + +/// Signed weight of an unsigned transaction. +/// +/// `tx.weight()` is the legacy serialization (empty witnesses, no BIP 141 marker). Satisfaction +/// weights are miniscript deltas from `TxIn::default().segwit_weight()`. +/// +/// * Pure legacy (no witness satisfaction): overhead is 0. The delta already accounts for the +/// scriptSig. +/// * Any witness satisfaction: overhead is 2 WU (marker and flag) plus 1 WU per input. BIP 141 +/// writes a witness stack length for every input, and the miniscript delta assumed that byte +/// was already present in `segwit_weight`. +/// +/// Returns [`None`] when the sum overflows `u64`. +fn estimated_signed_weight( + tx: &Transaction, + satisfactions: impl IntoIterator, +) -> Option { + let (satisfaction, any_segwit) = + satisfactions + .into_iter() + .try_fold((Weight::ZERO, false), |(acc, any_segwit), sat| { + acc.checked_add(sat.weight) + .map(|sum| (sum, any_segwit || sat.segwit)) + })?; + + let witness_overhead = if any_segwit { + // One stack-length byte per input, plus the 2-byte marker/flag. + // `usize` fits in `u64` on every supported target. + let n_inputs = tx.input.len() as u64; + Weight::from_wu(2).checked_add(Weight::from_wu(n_inputs))? + } else { + Weight::ZERO + }; + + tx.weight() + .checked_add(satisfaction)? + .checked_add(witness_overhead) } /// Estimate the signed weight of an assembled unsigned transaction and reject it when it /// exceeds [`bitcoin::policy::MAX_STANDARD_TX_WEIGHT`]. /// -/// `tx` is unsigned (empty witnesses). `satisfaction_weights` are the additional scriptSig / -/// witness weights for each input. Empty-witness transactions serialize without the 2-WU -/// segwit marker/flag; that is added once any satisfaction weight is present. +/// On overflow the estimated weight is reported as [`Weight::MAX`], which is above the limit, so +/// callers surface [`CreateTxError::TxWeightLimitExceeded`] instead of panicking or wrapping. fn check_max_standard_tx_weight( tx: &Transaction, - satisfaction_weights: impl IntoIterator, + satisfactions: impl IntoIterator, ) -> Result<(), (Weight, Weight)> { - let satisfaction: Weight = satisfaction_weights.into_iter().sum(); - let segwit_marker = if satisfaction > Weight::ZERO { - Weight::from_wu(2) - } else { - Weight::ZERO - }; - let weight = tx.weight() + satisfaction + segwit_marker; let limit = Weight::from_wu(u64::from(bitcoin::policy::MAX_STANDARD_TX_WEIGHT)); + let Some(weight) = estimated_signed_weight(tx, satisfactions) else { + return Err((Weight::MAX, limit)); + }; if weight > limit { Err((weight, limit)) } else { @@ -3485,12 +3610,10 @@ impl Wallet { ) .map_err(CreatePsbtError::Psbt)?; - let satisfaction_weights = psbt - .unsigned_tx - .input - .iter() - .map(|txin| self.satisfaction_weight_for_outpoint(txin.previous_output)); - check_max_standard_tx_weight(&psbt.unsigned_tx, satisfaction_weights) + // Planned and foreign inputs are not in the wallet UTXO set, so their satisfaction + // weights live on the selection candidates rather than on a local descriptor. + let satisfactions = selection.inputs().iter().map(selection_input_satisfaction); + check_max_standard_tx_weight(&psbt.unsigned_tx, satisfactions) .map_err(|(weight, limit)| CreatePsbtError::TxWeightLimitExceeded { weight, limit })?; // Add global xpubs. @@ -4273,4 +4396,86 @@ mod test { assert_eq!(deprecated_finalized, signers_finalized); assert_eq!(deprecated_psbt, signers_psbt); } + + fn p2pkh_script() -> ScriptBuf { + use bitcoin::hashes::Hash; + ScriptBuf::new_p2pkh(&bitcoin::PubkeyHash::all_zeros()) + } + + fn p2wpkh_script() -> ScriptBuf { + use bitcoin::hashes::Hash; + ScriptBuf::new_p2wpkh(&bitcoin::WPubkeyHash::all_zeros()) + } + + /// Unsigned tx: `n_inputs` empty inputs and one output. Witnesses are empty, so + /// [`Transaction::weight`] is the legacy serialization. + fn unsigned_tx(n_inputs: usize, output_script: ScriptBuf) -> Transaction { + Transaction { + version: transaction::Version::TWO, + lock_time: absolute::LockTime::ZERO, + input: (0..n_inputs) + .map(|_| bitcoin::TxIn { + previous_output: OutPoint::null(), + script_sig: ScriptBuf::new(), + sequence: Sequence::MAX, + witness: Witness::new(), + }) + .collect(), + output: vec![TxOut { + value: Amount::from_sat(50_000), + script_pubkey: output_script, + }], + } + } + + fn sat(weight: u64, segwit: bool) -> InputSatisfaction { + InputSatisfaction { + weight: Weight::from_wu(weight), + segwit, + } + } + + #[test] + fn signed_weight_matches_bip141_overhead() { + // Review table: pure legacy must not add the segwit marker. P2PKH satisfaction is the + // scriptSig delta (428 WU); actual signed weight is unsigned + 428. + let legacy = unsigned_tx(1, p2pkh_script()); + assert_eq!(legacy.weight(), Weight::from_wu(340)); + assert_eq!( + estimated_signed_weight(&legacy, [sat(428, false)]).unwrap(), + Weight::from_wu(768) + ); + + // Mixed: 1 P2WPKH (107 WU) + 2 P2PKH (428 WU). Overhead is 2 + K, K = 3. + let mixed = unsigned_tx(3, p2wpkh_script()); + assert_eq!(mixed.weight(), Weight::from_wu(656)); + assert_eq!( + estimated_signed_weight(&mixed, [sat(107, true), sat(428, false), sat(428, false)]) + .unwrap(), + Weight::from_wu(1_624) + ); + + // Pure P2WPKH, K = 100. Overhead is 2 + 100; satisfaction is 107 WU each. + let segwit = unsigned_tx(100, p2wpkh_script()); + assert_eq!(segwit.weight(), Weight::from_wu(16_564)); + let satisfactions = (0..100).map(|_| sat(107, true)); + assert_eq!( + estimated_signed_weight(&segwit, satisfactions).unwrap(), + Weight::from_wu(27_366) + ); + } + + #[test] + fn signed_weight_overflow_is_tx_weight_limit_exceeded() { + let tx = unsigned_tx(1, p2wpkh_script()); + let limit = Weight::from_wu(u64::from(bitcoin::policy::MAX_STANDARD_TX_WEIGHT)); + // Summing the satisfaction itself overflows, before the base tx weight is added. + let overflow = + check_max_standard_tx_weight(&tx, [sat(u64::MAX - 10, false), sat(100, false)]); + assert_eq!(overflow, Err((Weight::MAX, limit))); + + // A single astronomical satisfaction plus the unsigned tx weight overflows. + let astronomical = check_max_standard_tx_weight(&tx, [sat(u64::MAX - 200, true)]); + assert_eq!(astronomical, Err((Weight::MAX, limit))); + } } diff --git a/tests/create_psbt.rs b/tests/create_psbt.rs index b29bd51b..ea96613e 100644 --- a/tests/create_psbt.rs +++ b/tests/create_psbt.rs @@ -1287,3 +1287,68 @@ fn test_create_psbt_rejects_over_max_standard_tx_weight() { "expected TxWeightLimitExceeded, got: {err:?}" ); } + +#[test] +fn test_create_psbt_rejects_planned_input_over_max_standard_tx_weight() { + // Planned/foreign inputs are not wallet UTXOs. Their satisfaction weight is tracked on the + // selection candidate and must still be counted. Satisfaction alone is the standardness limit, + // so the signed tx is over even before the rest of the transaction. + let (desc, change_desc) = get_test_wpkh_and_change_desc(); + let mut wallet = Wallet::create(desc, change_desc) + .network(Network::Regtest) + .create_wallet_no_persist() + .unwrap(); + + let prev_tx = Transaction { + version: transaction::Version::TWO, + lock_time: absolute::LockTime::ZERO, + input: vec![], + output: vec![TxOut { + value: Amount::from_sat(500_000), + script_pubkey: ScriptBuf::new_p2wpkh(&bitcoin::WPubkeyHash::all_zeros()), + }], + }; + let outpoint = OutPoint { + txid: prev_tx.compute_txid(), + vout: 0, + }; + let psbt_input = bitcoin::psbt::Input { + witness_utxo: Some(prev_tx.output[0].clone()), + non_witness_utxo: Some(prev_tx), + ..Default::default() + }; + let planned = bdk_tx::Input::from_psbt_input( + outpoint, + Sequence::ENABLE_RBF_NO_LOCKTIME, + psbt_input, + usize::try_from(bitcoin::policy::MAX_STANDARD_TX_WEIGHT).unwrap(), + None, + false, + None, + ) + .unwrap(); + + let dest = wallet + .reveal_next_address(KeychainKind::External) + .address + .script_pubkey(); + let mut params = PsbtParams::default(); + params + .add_planned_input(planned) + .add_recipients([(dest, Amount::from_sat(10_000))]) + .manually_selected_only() + .fee_rate(FeeRate::ZERO); + + let err = wallet + .create_psbt(params) + .expect_err("planned input satisfaction exceeds the standardness limit"); + assert!( + matches!( + err, + CreatePsbtError::TxWeightLimitExceeded { weight, limit } + if weight > limit + && limit == Weight::from_wu(u64::from(bitcoin::policy::MAX_STANDARD_TX_WEIGHT)) + ), + "expected TxWeightLimitExceeded, got: {err:?}" + ); +} diff --git a/tests/wallet.rs b/tests/wallet.rs index ace8c356..88dc32f5 100644 --- a/tests/wallet.rs +++ b/tests/wallet.rs @@ -3811,3 +3811,47 @@ fn test_create_tx_rejects_over_max_standard_tx_weight_foreign_satisfaction() { && limit == Weight::from_wu(u64::from(bitcoin::policy::MAX_STANDARD_TX_WEIGHT)) ); } + +#[test] +fn test_create_tx_rejects_astronomical_foreign_satisfaction_without_overflow() { + // A caller-supplied satisfaction near u64::MAX must not panic (debug) or wrap (release). + // Either failure mode used to skip TxWeightLimitExceeded. + let (mut wallet, _) = get_funded_wallet_wpkh(); + let addr = wallet.next_unused_address(KeychainKind::External); + + let foreign_prev_tx = Transaction { + version: transaction::Version::TWO, + lock_time: absolute::LockTime::ZERO, + input: vec![], + output: vec![TxOut { + value: Amount::from_sat(100_000), + script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()), + }], + }; + let outpoint = OutPoint { + txid: foreign_prev_tx.compute_txid(), + vout: 0, + }; + let psbt_input = psbt::Input { + witness_utxo: Some(foreign_prev_tx.output[0].clone()), + non_witness_utxo: Some(foreign_prev_tx), + ..Default::default() + }; + + let mut builder = wallet.build_tx(); + builder + .add_foreign_utxo(outpoint, psbt_input, Weight::from_wu(u64::MAX - 200)) + .unwrap(); + builder + .manually_selected_only() + .fee_absolute(Amount::from_sat(1000)) + .drain_wallet() + .drain_to(addr.script_pubkey()); + + assert_matches!( + builder.finish(), + Err(CreateTxError::TxWeightLimitExceeded { weight, limit }) + if weight > limit + && limit == Weight::from_wu(u64::from(bitcoin::policy::MAX_STANDARD_TX_WEIGHT)) + ); +} From d1c06861b43fd019d7ad2fa9dd56d806ccc9a9d4 Mon Sep 17 00:00:00 2001 From: Cestercian Date: Mon, 28 Sep 2026 03:44:28 +0000 Subject: [PATCH 3/3] fix(wallet): classify witness spends from prevout, not witness_utxo spends_with_witness treated a populated witness_utxo as proof of witness serialization. A legacy P2PKH foreign input can carry both UTXO fields; that misclassification adds 3 WU at the standardness boundary and rejects a 400_000 WU legacy spend. Native witness programs come from the prevout. Nested segwit comes from a P2SH redeem script that is itself a witness program. --- src/wallet/mod.rs | 93 +++++++++++++++++++++++++++++++++++++++-------- tests/wallet.rs | 71 ++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 15 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 4c623fa5..272dee98 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -3029,11 +3029,11 @@ struct InputSatisfaction { segwit: bool, } -/// Whether `script_pubkey` (or the PSBT metadata spending it) puts satisfaction in a witness. +/// Whether satisfying `script_pubkey` serializes a witness. /// -/// Native segwit and taproot are witness programs. Nested segwit is a P2SH script pubkey, so it -/// is recognized from the PSBT input: a witness UTXO, witness script, final witness, or a redeem -/// script that is itself a witness program. +/// Native segwit and taproot are witness programs on the prevout. Nested segwit is a P2SH +/// prevout whose redeem script is itself a witness program. A populated [`psbt::Input::witness_utxo`] +/// is not evidence: a legacy prevout can carry both `non_witness_utxo` and `witness_utxo`. fn spends_with_witness(script_pubkey: &bitcoin::Script, psbt_input: Option<&psbt::Input>) -> bool { if script_pubkey.witness_version().is_some() { return true; @@ -3041,22 +3041,20 @@ fn spends_with_witness(script_pubkey: &bitcoin::Script, psbt_input: Option<&psbt let Some(psbt_input) = psbt_input else { return false; }; - if psbt_input.witness_utxo.is_some() - || psbt_input.final_script_witness.is_some() - || psbt_input.witness_script.is_some() - { - return true; - } - psbt_input - .redeem_script - .as_ref() - .is_some_and(|redeem| redeem.witness_version().is_some()) + // Nested segwit only. `witness_utxo` / `witness_script` / `final_script_witness` can be set on + // a legacy spend and must not flip this classification. + script_pubkey.is_p2sh() + && psbt_input + .redeem_script + .as_ref() + .is_some_and(|redeem| redeem.witness_version().is_some()) } /// [`bdk_tx::Input`] satisfaction, using the weight already tracked on the selected input. /// /// Planned inputs created from a PSBT do not report [`Input::is_segwit`] unless -/// `final_script_witness` is set, so witness-ness also comes from the prevout and PSBT metadata. +/// `final_script_witness` is set, so witness-ness comes from the prevout and, for nested segwit, +/// the P2SH redeem script. #[cfg(all(bdk_wallet_unstable, feature = "bdk-tx"))] fn selection_input_satisfaction(input: &Input) -> InputSatisfaction { let segwit = if input.plan().is_some() { @@ -4435,6 +4433,71 @@ mod test { } } + #[test] + fn spends_with_witness_uses_prevout_and_redeem_script() { + // Native witness program, with or without PSBT metadata. + assert!(spends_with_witness(p2wpkh_script().as_script(), None)); + + // Legacy P2PKH that carries both UTXO fields is still a legacy spend. Presence of + // witness_utxo used to classify this as segwit. + let legacy_spk = p2pkh_script(); + let prev_tx = Transaction { + version: transaction::Version::TWO, + lock_time: absolute::LockTime::ZERO, + input: vec![], + output: vec![TxOut { + value: Amount::from_sat(100_000), + script_pubkey: legacy_spk.clone(), + }], + }; + let mut final_witness = Witness::new(); + final_witness.push([0u8; 72]); + final_witness.push([0u8; 33]); + let legacy_input = psbt::Input { + witness_utxo: Some(prev_tx.output[0].clone()), + non_witness_utxo: Some(prev_tx), + witness_script: Some(p2wpkh_script()), + final_script_witness: Some(final_witness), + ..Default::default() + }; + assert!(!spends_with_witness( + legacy_spk.as_script(), + Some(&legacy_input) + )); + + // Nested segwit: P2SH prevout whose redeem script is a witness program. + let redeem = p2wpkh_script(); + let nested_spk = ScriptBuf::new_p2sh(&redeem.script_hash()); + let nested_input = psbt::Input { + redeem_script: Some(redeem), + witness_utxo: Some(TxOut { + value: Amount::from_sat(100_000), + script_pubkey: nested_spk.clone(), + }), + ..Default::default() + }; + assert!(spends_with_witness( + nested_spk.as_script(), + Some(&nested_input) + )); + + // P2SH wrapping a legacy redeem script does not serialize a witness. + let legacy_redeem = p2pkh_script(); + let legacy_p2sh = ScriptBuf::new_p2sh(&legacy_redeem.script_hash()); + let legacy_p2sh_input = psbt::Input { + redeem_script: Some(legacy_redeem), + witness_utxo: Some(TxOut { + value: Amount::from_sat(100_000), + script_pubkey: legacy_p2sh.clone(), + }), + ..Default::default() + }; + assert!(!spends_with_witness( + legacy_p2sh.as_script(), + Some(&legacy_p2sh_input) + )); + } + #[test] fn signed_weight_matches_bip141_overhead() { // Review table: pure legacy must not add the segwit marker. P2PKH satisfaction is the diff --git a/tests/wallet.rs b/tests/wallet.rs index 88dc32f5..e6eb4667 100644 --- a/tests/wallet.rs +++ b/tests/wallet.rs @@ -3812,6 +3812,77 @@ fn test_create_tx_rejects_over_max_standard_tx_weight_foreign_satisfaction() { ); } +#[test] +fn test_create_tx_legacy_foreign_both_utxo_fields_at_standardness_limit() { + // Legacy P2PKH foreign input with both non_witness_utxo and witness_utxo. The spend is + // legacy; witness_utxo must not add the BIP 141 marker, flag, and per-input stack length. + // That misclassification estimates a 400_000 WU transaction as 400_003 WU and rejects it. + let limit = Weight::from_wu(u64::from(bitcoin::policy::MAX_STANDARD_TX_WEIGHT)); + + let probe = finish_legacy_foreign_both_utxos(Weight::from_wu(428)) + .expect("small legacy foreign satisfaction should build"); + let unsigned = probe.unsigned_tx.weight(); + let satisfaction = limit + .checked_sub(unsigned) + .expect("unsigned weight is under the standardness limit"); + // Signed legacy weight is exactly the standardness limit. Classifying this input as segwit + // would add marker + flag + one stack-length byte (3 WU) and reject 400_003 WU. + assert_eq!(unsigned.checked_add(satisfaction).unwrap(), limit); + + let psbt = finish_legacy_foreign_both_utxos(satisfaction).expect( + "legacy spend whose signed weight is exactly MAX_STANDARD_TX_WEIGHT must be accepted", + ); + assert_eq!(psbt.unsigned_tx.input.len(), 1); + assert!(psbt.unsigned_tx.input[0].witness.is_empty()); + assert!(psbt.inputs[0].witness_utxo.is_some()); + assert!(psbt.inputs[0].non_witness_utxo.is_some()); + assert!( + psbt.inputs[0] + .witness_utxo + .as_ref() + .unwrap() + .script_pubkey + .is_p2pkh() + ); + assert_eq!(psbt.unsigned_tx.weight() + satisfaction, limit); +} + +/// Build a one-input drain that spends a legacy P2PKH foreign UTXO carrying both UTXO fields. +fn finish_legacy_foreign_both_utxos(satisfaction: Weight) -> Result { + let (mut wallet, _) = get_funded_wallet_wpkh(); + let addr = wallet.next_unused_address(KeychainKind::External); + + let prev_tx = Transaction { + version: transaction::Version::TWO, + lock_time: absolute::LockTime::ZERO, + input: vec![], + output: vec![TxOut { + value: Amount::from_sat(100_000), + script_pubkey: ScriptBuf::new_p2pkh(&bitcoin::PubkeyHash::all_zeros()), + }], + }; + let outpoint = OutPoint { + txid: prev_tx.compute_txid(), + vout: 0, + }; + let psbt_input = psbt::Input { + witness_utxo: Some(prev_tx.output[0].clone()), + non_witness_utxo: Some(prev_tx), + ..Default::default() + }; + + let mut builder = wallet.build_tx(); + builder + .add_foreign_utxo(outpoint, psbt_input, satisfaction) + .unwrap(); + builder + .manually_selected_only() + .fee_absolute(Amount::from_sat(1_000)) + .drain_wallet() + .drain_to(addr.script_pubkey()); + builder.finish() +} + #[test] fn test_create_tx_rejects_astronomical_foreign_satisfaction_without_overflow() { // A caller-supplied satisfaction near u64::MAX must not panic (debug) or wrap (release).