From 2219b7cd55bfb8023e9b426de0ef118b32bfa372 Mon Sep 17 00:00:00 2001
From: benalleng <benalleng@gmail.com>
Date: Thu, 2 Jan 2025 12:09:15 -0500
Subject: [PATCH] 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.
---
 crates/wallet/src/wallet/mod.rs        | 15 +++++++--------
 crates/wallet/src/wallet/tx_builder.rs | 22 ++--------------------
 crates/wallet/tests/wallet.rs          |  1 +
 3 files changed, 10 insertions(+), 28 deletions(-)

diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs
index 2e068a95f..44a2ace0f 100644
--- 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
index 868d51dfa..7699a3b03 100644
--- 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
index dcc8030b5..4fa744ad7 100644
--- 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"