diff --git a/CHANGELOG.md b/CHANGELOG.md index b3bae91..c374dd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Rename `PsbtParams::version` to `PsbtParams::min_version`. +- Selected inputs that require CSV now raise the unsigned transaction version to `Version::TWO` instead of erroring. +- AFS now raises the transaction version to `Version::TWO` when it chooses the `nSequence` path. + +### Removed + +- Remove `AntiFeeSnipingError::UnsupportedVersion` / the old AFS unsupported-version failure path. + ## [0.2.0] diff --git a/src/afs.rs b/src/afs.rs index 9fd2bb0..85c44e4 100644 --- a/src/afs.rs +++ b/src/afs.rs @@ -13,8 +13,6 @@ use rand_core::RngCore; /// Error returned by `apply_anti_fee_sniping`. #[derive(Debug, Clone, PartialEq)] pub enum AntiFeeSnipingError { - /// Transaction `version` must be >= 2 for AFS to use relative locktimes. - UnsupportedVersion(Version), /// AFS only supports height-based locktimes. The transaction's locktime is /// time-based (MTP), which can originate from either `PsbtParams::min_locktime` /// or an input's time-based CLTV requirement. @@ -24,10 +22,6 @@ pub enum AntiFeeSnipingError { impl core::fmt::Display for AntiFeeSnipingError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { - Self::UnsupportedVersion(version) => write!( - f, - "anti-fee-sniping requires tx.version >= 2 (got {version})" - ), Self::UnsupportedLockTime(locktime) => write!( f, "anti-fee-sniping requires a height-based tx locktime (got time-based {locktime}); \ @@ -53,6 +47,9 @@ impl std::error::Error for AntiFeeSnipingError {} /// - **nLockTime**: Sets the transaction's lock time to approximately the current height /// - **nSequence**: Sets one Taproot input's sequence to approximately its confirmation depth /// +/// If the `nSequence` path is chosen, the transaction version is raised to +/// `Version::TWO` if necessary. +/// /// Random offsets (0-99 blocks) are applied with 10% probability to avoid creating /// a unique fingerprint that could identify transactions from this wallet. /// @@ -71,7 +68,6 @@ impl std::error::Error for AntiFeeSnipingError {} /// is implicitly satisfied. /// /// # Errors -/// - [`AntiFeeSnipingError::UnsupportedVersion`] if `tx.version < 2`. /// - [`AntiFeeSnipingError::UnsupportedLockTime`] if `tx.lock_time` is time-based /// (either from `PsbtParams::min_locktime` or an input's time-based CLTV). /// @@ -89,10 +85,6 @@ pub(crate) fn apply_anti_fee_sniping( const TEN_PERCENT_PROBABILITY_RANGE: u32 = 10; const MAX_RANDOM_OFFSET: u32 = 100; - if tx.version < Version::TWO { - return Err(AntiFeeSnipingError::UnsupportedVersion(tx.version)); - } - if !tx.lock_time.is_block_height() { return Err(AntiFeeSnipingError::UnsupportedLockTime(tx.lock_time)); } @@ -143,6 +135,10 @@ pub(crate) fn apply_anti_fee_sniping( } } else { // Use Sequence + if tx.version < Version::TWO { + tx.version = Version::TWO; + } + let random_index = random_range(rng, taproot_inputs.len() as u32); let (input_index, input) = taproot_inputs[random_index as usize]; let confirmation = input.confirmations(tip_height); diff --git a/src/selection.rs b/src/selection.rs index e1dd410..3e87e34 100644 --- a/src/selection.rs +++ b/src/selection.rs @@ -24,8 +24,11 @@ pub struct Selection { /// Parameters for creating a psbt. #[derive(Debug, Clone)] pub struct PsbtParams { - /// Use a specific [`transaction::Version`]. - pub version: transaction::Version, + /// Minimum [`transaction::Version`] for the resulting transaction. + /// + /// The final version may be raised when selected inputs require CSV or when anti-fee-sniping + /// selects its `nSequence` strategy. + pub min_version: transaction::Version, /// Minimum tx locktime — a floor on the resulting `tx.lock_time`. /// @@ -61,11 +64,9 @@ pub struct PsbtParams { /// /// # Errors /// - /// When `Some(..)`, [`Selection::create_psbt`] returns [`CreatePsbtError::AntiFeeSniping`] if: - /// - the transaction version is less than 2 - /// ([`AntiFeeSnipingError::UnsupportedVersion`]) — v2 is required for relative locktimes; or - /// - a time-based (MTP) locktime is in effect - /// ([`AntiFeeSnipingError::UnsupportedLockTime`]) — AFS only supports height-based locktimes. + /// When `Some(..)`, [`Selection::create_psbt`] returns [`CreatePsbtError::AntiFeeSniping`] if + /// a time-based (MTP) locktime is in effect + /// ([`AntiFeeSnipingError::UnsupportedLockTime`]) — AFS only supports height-based locktimes. /// /// See [BIP326](https://github.com/bitcoin/bips/blob/master/bip-0326.mediawiki) for more details. /// @@ -76,7 +77,7 @@ pub struct PsbtParams { impl Default for PsbtParams { fn default() -> Self { Self { - version: transaction::Version::TWO, + min_version: transaction::Version::TWO, min_locktime: absolute::LockTime::ZERO, mandate_full_tx_for_segwit_v0: true, anti_fee_sniping: None, @@ -131,6 +132,18 @@ impl core::fmt::Display for CreatePsbtError { impl std::error::Error for CreatePsbtError {} impl Selection { + fn effective_min_version(&self, min_version: transaction::Version) -> transaction::Version { + if self + .inputs + .iter() + .any(|input| input.relative_timelock().is_some()) + { + core::cmp::max(min_version, transaction::Version::TWO) + } else { + min_version + } + } + pub(crate) fn new(inputs: Vec, outputs: Vec) -> Self { Self { inputs, outputs } } @@ -250,8 +263,9 @@ impl Selection { params: PsbtParams, rng: &mut impl RngCore, ) -> Result { + let version = self.effective_min_version(params.min_version); let mut tx = bitcoin::Transaction { - version: params.version, + version, lock_time: Self::accumulate_max_locktime( self.inputs .iter() @@ -501,6 +515,80 @@ mod tests { Ok(input) } + fn setup_csv_input(confirmation_height: u32, csv_blocks: u32) -> anyhow::Result { + let secp = Secp256k1::new(); + let desc_str = + format!("tr({TEST_HEX_PK},and_v(v:pk({TEST_DESCRIPTOR_PK}),older({csv_blocks})))"); + let desc = Descriptor::parse_descriptor(&secp, &desc_str)? + .0 + .at_derivation_index(0)?; + let prev_tx = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn::default()], + output: vec![TxOut { + script_pubkey: desc.script_pubkey(), + value: Amount::from_sat(10_000), + }], + }; + let assets = Assets::new() + .add(TEST_DESCRIPTOR_PK.parse::()?) + .older(relative::LockTime::from_height( + csv_blocks.try_into().expect("csv blocks must fit in u16"), + )); + let plan = desc.plan(&assets).expect("script-path plan with CSV"); + let status = crate::ConfirmationStatus { + height: absolute::Height::from_consensus(confirmation_height)?, + prev_mtp: Some(Time::from_consensus(500_000_000)?), + }; + Ok(Input::from_prev_tx(plan, prev_tx, 0, Some(status))?) + } + + #[test] + fn test_min_version_preserves_without_csv() -> anyhow::Result<()> { + let input = setup_test_input(2_000)?; + let output = Output::with_script(ScriptBuf::new(), Amount::from_sat(9_000)); + let selection = Selection::new(vec![input], vec![output]); + + let psbt = selection.create_psbt(PsbtParams { + min_version: Version::ONE, + ..Default::default() + })?; + + assert_eq!(psbt.unsigned_tx.version, Version::ONE); + Ok(()) + } + + #[test] + fn test_min_version_bumps_for_csv() -> anyhow::Result<()> { + let csv_input = setup_csv_input(2_500, 10)?; + let output = Output::with_script(ScriptBuf::new(), Amount::from_sat(9_000)); + let selection = Selection::new(vec![csv_input], vec![output]); + + let psbt = selection.create_psbt(PsbtParams { + min_version: Version::ONE, + ..Default::default() + })?; + + assert_eq!(psbt.unsigned_tx.version, Version::TWO); + Ok(()) + } + + #[test] + fn test_min_version_preserves_higher_version_with_csv() -> anyhow::Result<()> { + let csv_input = setup_csv_input(2_500, 10)?; + let output = Output::with_script(ScriptBuf::new(), Amount::from_sat(9_000)); + let selection = Selection::new(vec![csv_input], vec![output]); + + let psbt = selection.create_psbt(PsbtParams { + min_version: Version::non_standard(3), + ..Default::default() + })?; + + assert_eq!(psbt.unsigned_tx.version, Version::non_standard(3)); + Ok(()) + } + #[test] fn test_anti_fee_sniping_disabled() -> anyhow::Result<()> { let current_height = 2_500; @@ -653,39 +741,12 @@ mod tests { #[test] fn test_anti_fee_sniping_skips_taproot_csv_input() -> anyhow::Result<()> { let tip = absolute::Height::from_consensus(3_000)?; - let csv_blocks = 10; // Input A: regular Taproot, no CSV. let regular_input = setup_test_input(2_500)?; let regular_outpoint = regular_input.prev_outpoint(); - // Input B: Taproot whose script-path requires CSV. The internal key is omitted from - // `assets`, forcing planning to use the script-path leaf (which sets - // `plan.relative_timelock`). - let secp = Secp256k1::new(); - let desc_str = - format!("tr({TEST_HEX_PK},and_v(v:pk({TEST_DESCRIPTOR_PK}),older({csv_blocks})))"); - let desc = Descriptor::parse_descriptor(&secp, &desc_str)? - .0 - .at_derivation_index(0)?; - let prev_tx = Transaction { - version: Version::TWO, - lock_time: LockTime::ZERO, - input: vec![TxIn::default()], - output: vec![TxOut { - script_pubkey: desc.script_pubkey(), - value: Amount::from_sat(10_000), - }], - }; - let assets = Assets::new() - .add(TEST_DESCRIPTOR_PK.parse::()?) - .older(relative::LockTime::from_height(csv_blocks)); - let plan = desc.plan(&assets).expect("script-path plan with CSV"); - let status = crate::ConfirmationStatus { - height: absolute::Height::from_consensus(2_500)?, - prev_mtp: Some(Time::from_consensus(500_000_000)?), - }; - let csv_input = Input::from_prev_tx(plan, prev_tx, 0, Some(status))?; + let csv_input = setup_csv_input(2_500, 10)?; let csv_outpoint = csv_input.prev_outpoint(); let csv_sequence = csv_input.sequence().expect("plan-derived sequence"); @@ -701,11 +762,14 @@ mod tests { vec![output.clone()], ); let psbt = selection.create_psbt(PsbtParams { + min_version: Version::ONE, anti_fee_sniping: Some(tip), ..Default::default() })?; let tx = psbt.unsigned_tx; + assert_eq!(tx.version, Version::TWO); + let csv_txin = tx .input .iter() @@ -766,28 +830,56 @@ mod tests { } #[test] - fn test_anti_fee_sniping_unsupported_version_error() { + fn test_anti_fee_sniping_sequence_path_bumps_version() -> anyhow::Result<()> { + struct ConstantRng; + + impl rand_core::RngCore for ConstantRng { + fn next_u32(&mut self) -> u32 { + 7 + } + + fn next_u64(&mut self) -> u64 { + 7 + } + + fn fill_bytes(&mut self, dest: &mut [u8]) { + for byte in dest { + *byte = 7; + } + } + + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), rand_core::Error> { + self.fill_bytes(dest); + Ok(()) + } + } + let confirmation_height = 800_000; - let input = setup_test_input(confirmation_height).unwrap(); - let inputs = vec![input]; - let current_height = absolute::Height::from_consensus(confirmation_height + 50).unwrap(); + let input = setup_test_input(confirmation_height)?; + let inputs = vec![input.clone()]; + let current_height = absolute::Height::from_consensus(confirmation_height + 50)?; let mut tx = Transaction { version: Version::ONE, lock_time: LockTime::from_height(current_height.to_consensus_u32()).unwrap(), input: vec![TxIn { - previous_output: inputs[0].prev_outpoint(), + previous_output: input.prev_outpoint(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, ..Default::default() }], output: vec![], }; - let result = apply_anti_fee_sniping(&mut tx, &inputs, current_height, &mut OsRng); + let mut rng = ConstantRng; + let result = apply_anti_fee_sniping(&mut tx, &inputs, current_height, &mut rng); - assert!( - matches!(result, Err(AntiFeeSnipingError::UnsupportedVersion(_))), - "should return UnsupportedVersion error for version < 2" + assert!(result.is_ok()); + assert_eq!(tx.version, Version::TWO); + assert_eq!( + tx.input[0].sequence, + Sequence(input.confirmations(current_height)) ); + Ok(()) } #[test]