Skip to content

Commit

Permalink
Merge #1789: Change default tx to version 2
Browse files Browse the repository at this point in the history
2219b7c fix(tx_builder): change default tx to version 2 (benalleng)

Pull request description:

  <!-- You can erase any parts of this template not applicable to your Pull Request. -->

  ### Description

  Changes the default version number to version 2 to enchance privacy for the type of wallet used

  Fixes #1676

  ### Changelog notice

  - Changed the default transaction version number in the transaction builder.
  - Updated the test to test for the default transaction number.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

ACKs for top commit:
  notmandatory:
    ACK 2219b7c

Tree-SHA512: 76a2d394318eea94a99c033c3df14d64dbcf96c0f91dba91d2e337b9ecf78302af3d535ab11a74ed7372a521d3bc14c3455b7c8266d79d2995cc7c5da363b426
  • Loading branch information
notmandatory committed Jan 10, 2025
2 parents b2876d8 + 2219b7c commit a5335a1
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 28 deletions.
15 changes: 7 additions & 8 deletions crates/wallet/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 :)
Expand Down Expand Up @@ -1388,7 +1387,7 @@ impl Wallet {
};

let mut tx = Transaction {
version: transaction::Version::non_standard(version),
version,
lock_time,
input: vec![],
output: vec![],
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -2584,7 +2583,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 {
Expand Down
22 changes: 2 additions & 20 deletions crates/wallet/src/wallet/tx_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
}
1 change: 1 addition & 0 deletions crates/wallet/tests/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit a5335a1

Please sign in to comment.