fix(tx_builder)/ change default tx to version 2

This change improves privacy since >85% of transactions on the network are version 2. Version 2 is also necessary to eventually implement BIP 326 nSequence-based anti-fee-sniping. Signed-off-by: benalleng <benalleng@gmail.com>
bitcoindevkit · Jan 7, 2025 · c60ec12 · c60ec12
1 parent 4a3675f
commit c60ec12
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 28 deletions.
diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs
@@ -1284,13 +1284,12 @@ impl Wallet {
             external_requirements.merge(&internal_requirements.unwrap_or_default())?;
 
         let version = match params.version {
-            Some(tx_builder::Version(0)) => return Err(CreateTxError::Version0),
-            Some(tx_builder::Version(1)) if requirements.csv.is_some() => {
+            Some(transaction::Version(0)) => return Err(CreateTxError::Version0),
+            Some(transaction::Version::ONE) if requirements.csv.is_some() => {
                 return Err(CreateTxError::Version1Csv)
             }
-            Some(tx_builder::Version(x)) => x,
-            None if requirements.csv.is_some() => 2,
-            None => 1,
+            Some(v) => v,
+            None => transaction::Version::TWO,
         };
 
         // We use a match here instead of a unwrap_or_else as it's way more readable :)
@@ -1388,7 +1387,7 @@ impl Wallet {
         };
 
         let mut tx = Transaction {
-            version: transaction::Version::non_standard(version),
+            version,
             lock_time,
             input: vec![],
             output: vec![],
@@ -1693,7 +1692,7 @@ impl Wallet {
 
         let params = TxParams {
             // TODO: figure out what rbf option should be?
-            version: Some(tx_builder::Version(tx.version.0)),
+            version: Some(tx.version),
             recipients: tx
                 .output
                 .into_iter()
@@ -2581,7 +2580,7 @@ macro_rules! doctest_wallet {
             .unwrap();
         let address = wallet.peek_address(KeychainKind::External, 0).address;
         let tx = Transaction {
-            version: transaction::Version::ONE,
+            version: transaction::Version::TWO,
             lock_time: absolute::LockTime::ZERO,
             input: vec![],
             output: vec![TxOut {

diff --git a/crates/wallet/src/wallet/tx_builder.rs b/crates/wallet/src/wallet/tx_builder.rs
@@ -44,8 +44,8 @@ use alloc::sync::Arc;
 use bitcoin::psbt::{self, Psbt};
 use bitcoin::script::PushBytes;
 use bitcoin::{
-    absolute, Amount, FeeRate, OutPoint, ScriptBuf, Sequence, Transaction, TxIn, TxOut, Txid,
-    Weight,
+    absolute, transaction::Version, Amount, FeeRate, OutPoint, ScriptBuf, Sequence, Transaction,
+    TxIn, TxOut, Txid, Weight,
 };
 use rand_core::RngCore;
 
@@ -796,18 +796,6 @@ impl TxOrdering {
     }
 }
 
-/// Transaction version
-///
-/// Has a default value of `1`
-#[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Hash, Clone, Copy)]
-pub(crate) struct Version(pub(crate) i32);
-
-impl Default for Version {
-    fn default() -> Self {
-        Version(1)
-    }
-}
-
 /// Policy regarding the use of change outputs when creating a transaction
 #[derive(Default, Debug, Ord, PartialOrd, Eq, PartialEq, Hash, Clone, Copy)]
 pub enum ChangeSpendPolicy {
@@ -1064,10 +1052,4 @@ mod test {
         assert_eq!(filtered.len(), 1);
         assert_eq!(filtered[0].keychain, KeychainKind::Internal);
     }
-
-    #[test]
-    fn test_default_tx_version_1() {
-        let version = Version::default();
-        assert_eq!(version.0, 1);
-    }
 }
diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs
@@ -3531,6 +3531,7 @@ fn test_spend_from_wallet(mut wallet: Wallet) {
     builder.add_recipient(addr.script_pubkey(), Amount::from_sat(25_000));
     let mut psbt = builder.finish().unwrap();
 
+    assert_eq!(psbt.unsigned_tx.version.0, 2);
     assert!(
         wallet.sign(&mut psbt, Default::default()).unwrap(),
         "Unable to finalize tx"