feat(wallet)!: enable RBF by default on TxBuilder

crates/wallet/README.md

@@ -174,7 +174,6 @@ println!("Your new receive address is: {}", receive_address.address);
 <!-- let mut builder = wallet.build_tx(); -->
 <!-- builder -->
 <!-- .add_recipient(send_to.script_pubkey(), 50_000) -->
-<!-- .enable_rbf() -->
 <!-- .do_not_spend_change() -->
 <!-- .fee_rate(FeeRate::from_sat_per_vb(5.0)); -->
 <!-- builder.finish()? -->

crates/wallet/src/wallet/error.rs

@@ -65,12 +65,10 @@ pub enum CreateTxError {
  /// Required `LockTime`
  required: absolute::LockTime,
  },
- /// Cannot enable RBF with a `Sequence` >= 0xFFFFFFFE
- RbfSequence,
  /// Cannot enable RBF with `Sequence` given a required OP_CSV
  RbfSequenceCsv {
  /// Given RBF `Sequence`
- rbf: Sequence,
+ sequence: Sequence,
  /// Required OP_CSV `Sequence`
  csv: Sequence,
  },
@@ -131,14 +129,11 @@ impl fmt::Display for CreateTxError {
  } => {
  write!(f, "TxBuilder requested timelock of `{:?}`, but at least `{:?}` is required to spend from this script", required, requested)
  }
- CreateTxError::RbfSequence => {
- write!(f, "Cannot enable RBF with a nSequence >= 0xFFFFFFFE")
- }
- CreateTxError::RbfSequenceCsv { rbf, csv } => {
+ CreateTxError::RbfSequenceCsv { sequence, csv } => {
  write!(
  f,
  "Cannot enable RBF with nSequence `{:?}` given a required OP_CSV of `{:?}`",
- rbf, csv
+ sequence, csv
  )
  }
  CreateTxError::FeeTooLow { required } => {

crates/wallet/src/wallet/mod.rs

@@ -1366,36 +1366,20 @@ impl Wallet {
  }
  };
 
- // The nSequence to be by default for inputs unless an explicit sequence is specified.
- let n_sequence = match (params.rbf, requirements.csv) {
- // No RBF or CSV but there's an nLockTime, so the nSequence cannot be final
- (None, None) if lock_time != absolute::LockTime::ZERO => {
- Sequence::ENABLE_LOCKTIME_NO_RBF
- }
- // No RBF, CSV or nLockTime, make the transaction final
- (None, None) => Sequence::MAX,
-
- // No RBF requested, use the value from CSV. Note that this value is by definition
- // non-final, so even if a timelock is enabled this nSequence is fine, hence why we
- // don't bother checking for it here. The same is true for all the other branches below
+ // nSequence value for inputs
+ // When not explicitly specified, defaults to 0xFFFFFFFD,
+ // meaning RBF signaling is enabled
+ let n_sequence = match (params.sequence, requirements.csv) {
+ // Enable RBF by default
+ (None, None) => Sequence::ENABLE_RBF_NO_LOCKTIME,
+ // None requested, use required
  (None, Some(csv)) => csv,
-
- // RBF with a specific value but that value is too high
- (Some(tx_builder::RbfValue::Value(rbf)), _) if !rbf.is_rbf() => {
- return Err(CreateTxError::RbfSequence)
+ // Requested sequence is incompatible with requirements
+ (Some(sequence), Some(csv)) if !check_nsequence_rbf(sequence, csv) => {
+ return Err(CreateTxError::RbfSequenceCsv { sequence, csv })
  }
- // RBF with a specific value requested, but the value is incompatible with CSV
- (Some(tx_builder::RbfValue::Value(rbf)), Some(csv))
- if !check_nsequence_rbf(rbf, csv) =>
- {
- return Err(CreateTxError::RbfSequenceCsv { rbf, csv })
- }
-
- // RBF enabled with the default value with CSV also enabled. CSV takes precedence
- (Some(tx_builder::RbfValue::Default), Some(csv)) => csv,
- // Valid RBF, either default or with a specific value. We ignore the `CSV` value
- // because we've already checked it before
- (Some(rbf), _) => rbf.get_value(),
+ // Use requested nSequence value
+ (Some(sequence), _) => sequence,
  };
 
  let (fee_rate, mut fee_amount) = match params.fee_policy.unwrap_or_default() {
@@ -1609,8 +1593,7 @@ impl Wallet {
  /// let mut psbt = {
  /// let mut builder = wallet.build_tx();
  /// builder
- /// .add_recipient(to_address.script_pubkey(), Amount::from_sat(50_000))
- /// .enable_rbf();
+ /// .add_recipient(to_address.script_pubkey(), Amount::from_sat(50_000));
  /// builder.finish()?
  /// };
  /// let _ = wallet.sign(&mut psbt, SignOptions::default())?;

crates/wallet/src/wallet/tx_builder.rs

@@ -31,9 +31,7 @@
 //! // With a custom fee rate of 5.0 satoshi/vbyte
 //! .fee_rate(FeeRate::from_sat_per_vb(5).expect("valid feerate"))
 //! // Only spend non-change outputs
-//! .do_not_spend_change()
-//! // Turn on RBF signaling
-//! .enable_rbf();
+//! .do_not_spend_change();
 //! let psbt = tx_builder.finish()?;
 //! # Ok::<(), anyhow::Error>(())
 //! ```
@@ -134,7 +132,7 @@ pub(crate) struct TxParams {
  pub(crate) sighash: Option<psbt::PsbtSighashType>,
  pub(crate) ordering: TxOrdering,
  pub(crate) locktime: Option<absolute::LockTime>,
- pub(crate) rbf: Option<RbfValue>,
+ pub(crate) sequence: Option<Sequence>,
  pub(crate) version: Option<Version>,
  pub(crate) change_policy: ChangeSpendPolicy,
  pub(crate) only_witness_utxo: bool,
@@ -554,23 +552,12 @@ impl<'a, Cs> TxBuilder<'a, Cs> {
  }
  }
 
- /// Enable signaling RBF
+ /// Set an exact nSequence value
  ///
- /// This will use the default nSequence value of `0xFFFFFFFD`.
- pub fn enable_rbf(&mut self) -> &mut Self {
- self.params.rbf = Some(RbfValue::Default);
- self
- }
-
- /// Enable signaling RBF with a specific nSequence value
- ///
- /// This can cause conflicts if the wallet's descriptors contain an "older" (OP_CSV) operator
- /// and the given `nsequence` is lower than the CSV value.
- ///
- /// If the `nsequence` is higher than `0xFFFFFFFD` an error will be thrown, since it would not
- /// be a valid nSequence to signal RBF.
- pub fn enable_rbf_with_sequence(&mut self, nsequence: Sequence) -> &mut Self {
- self.params.rbf = Some(RbfValue::Value(nsequence));
+ /// This can cause conflicts if the wallet's descriptors contain an
+ /// "older" (OP_CSV) operator and the given `nsequence` is lower than the CSV value.
+ pub fn set_exact_sequence(&mut self, n_sequence: Sequence) -> &mut Self {
+ self.params.sequence = Some(n_sequence);
  self
  }
 
@@ -654,8 +641,7 @@ impl<'a, Cs> TxBuilder<'a, Cs> {
  /// .drain_wallet()
  /// // Send the excess (which is all the coins minus the fee) to this address.
  /// .drain_to(to_address.script_pubkey())
- /// .fee_rate(FeeRate::from_sat_per_vb(5).expect("valid feerate"))
- /// .enable_rbf();
+ /// .fee_rate(FeeRate::from_sat_per_vb(5).expect("valid feerate"));
  /// let psbt = tx_builder.finish()?;
  /// # Ok::<(), anyhow::Error>(())
  /// ```
@@ -835,24 +821,6 @@ impl Default for Version {
  }
 }
 
-/// RBF nSequence value
-///
-/// Has a default value of `0xFFFFFFFD`
-#[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Hash, Clone, Copy)]
-pub(crate) enum RbfValue {
- Default,
- Value(Sequence),
-}
-
-impl RbfValue {
- pub(crate) fn get_value(&self) -> Sequence {
- match self {
- RbfValue::Default => Sequence::ENABLE_RBF_NO_LOCKTIME,
- RbfValue::Value(v) => *v,
- }
- }
-}
-
 /// Policy regarding the use of change outputs when creating a transaction
 #[derive(Default, Debug, Ord, PartialOrd, Eq, PartialEq, Hash, Clone, Copy)]
 pub enum ChangeSpendPolicy {

crates/wallet/src/wallet/utils.rs

@@ -52,20 +52,20 @@ impl After {
  }
 }
 
-pub(crate) fn check_nsequence_rbf(rbf: Sequence, csv: Sequence) -> bool {
- // The RBF value must enable relative timelocks
- if !rbf.is_relative_lock_time() {
+pub(crate) fn check_nsequence_rbf(sequence: Sequence, csv: Sequence) -> bool {
+ // The nSequence value must enable relative timelocks
+ if !sequence.is_relative_lock_time() {
  return false;
  }
 
  // Both values should be represented in the same unit (either time-based or
  // block-height based)
- if rbf.is_time_locked() != csv.is_time_locked() {
+ if sequence.is_time_locked() != csv.is_time_locked() {
  return false;
  }
 
  // The value should be at least `csv`
- if rbf < csv {
+ if sequence < csv {
  return false;
  }