Skip to content

Commit

Permalink
refactor(wallet)!: remove TxOrdering::Bip69Lexicographic
Browse files Browse the repository at this point in the history
BIP 69 proposed a deterministic way to sort transaction inputs and
outputs with the idea of enhancing privacy. It was later discovered
there was no such enhancement but rather a decrement in privacy due to
this sorting.
To avoid the promotion of bad practices, the
TxOrdering::Bip69Lexicographic variant which implemented this BIP for
BDK is removed with this change.
Notice that you can still produce a BIP 69 compliant transaction
defining order functions for TxOrdering::Custom.

Signed-off-by: Steve Myers <[email protected]>
  • Loading branch information
nymius authored and notmandatory committed Jul 5, 2024
1 parent e5cb7b2 commit 3bee563
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 50 deletions.
49 changes: 0 additions & 49 deletions crates/wallet/src/wallet/tx_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,8 +776,6 @@ pub enum TxOrdering {
Shuffle,
/// Unchanged
Untouched,
/// BIP69 / Lexicographic
Bip69Lexicographic,
/// Provide custom comparison functions for sorting
Custom {
/// Transaction inputs sort function
Expand All @@ -792,7 +790,6 @@ impl core::fmt::Debug for TxOrdering {
match self {
TxOrdering::Shuffle => write!(f, "Shuffle"),
TxOrdering::Untouched => write!(f, "Untouched"),
TxOrdering::Bip69Lexicographic => write!(f, "Bip69Lexicographic"),
TxOrdering::Custom { .. } => write!(f, "Custom"),
}
}
Expand All @@ -817,13 +814,6 @@ impl TxOrdering {
shuffle_slice(&mut tx.input, rng);
shuffle_slice(&mut tx.output, rng);
}
TxOrdering::Bip69Lexicographic => {
tx.input.sort_unstable_by_key(|txin| {
(txin.previous_output.txid, txin.previous_output.vout)
});
tx.output
.sort_unstable_by_key(|txout| (txout.value, txout.script_pubkey.clone()));
}
TxOrdering::Custom {
input_sort,
output_sort,
Expand Down Expand Up @@ -939,45 +929,6 @@ mod test {
.expect("it should have moved the outputs at least once");
}

#[test]
fn test_output_ordering_bip69() {
use core::str::FromStr;

let original_tx = ordering_test_tx!();
let mut tx = original_tx;

TxOrdering::Bip69Lexicographic.sort_tx(&mut tx);

assert_eq!(
tx.input[0].previous_output,
bitcoin::OutPoint::from_str(
"0e53ec5dfb2cb8a71fec32dc9a634a35b7e24799295ddd5278217822e0b31f57:5"
)
.unwrap()
);
assert_eq!(
tx.input[1].previous_output,
bitcoin::OutPoint::from_str(
"0f60fdd185542f2c6ea19030b0796051e7772b6026dd5ddccd7a2f93b73e6fc2:0"
)
.unwrap()
);
assert_eq!(
tx.input[2].previous_output,
bitcoin::OutPoint::from_str(
"0f60fdd185542f2c6ea19030b0796051e7772b6026dd5ddccd7a2f93b73e6fc2:1"
)
.unwrap()
);

assert_eq!(tx.output[0].value.to_sat(), 800);
assert_eq!(tx.output[1].script_pubkey, ScriptBuf::from(vec![0xAA]));
assert_eq!(
tx.output[2].script_pubkey,
ScriptBuf::from(vec![0xAA, 0xEE])
);
}

#[test]
fn test_output_ordering_custom_but_bip69() {
use core::str::FromStr;
Expand Down
23 changes: 22 additions & 1 deletion crates/wallet/tests/wallet.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
extern crate alloc;

use std::path::Path;
use std::str::FromStr;

Expand Down Expand Up @@ -966,13 +968,32 @@ fn test_create_tx_drain_to_dust_amount() {

#[test]
fn test_create_tx_ordering_respected() {
use alloc::sync::Arc;

let (mut wallet, _) = get_funded_wallet_wpkh();
let addr = wallet.next_unused_address(KeychainKind::External);

let bip69_txin_cmp = |tx_a: &TxIn, tx_b: &TxIn| {
let project_outpoint = |t: &TxIn| (t.previous_output.txid, t.previous_output.vout);
project_outpoint(tx_a).cmp(&project_outpoint(tx_b))
};

let bip69_txout_cmp = |tx_a: &TxOut, tx_b: &TxOut| {
let project_utxo = |t: &TxOut| (t.value, t.script_pubkey.clone());
project_utxo(tx_a).cmp(&project_utxo(tx_b))
};

let custom_bip69_ordering = bdk_wallet::wallet::tx_builder::TxOrdering::Custom {
input_sort: Arc::new(bip69_txin_cmp),
output_sort: Arc::new(bip69_txout_cmp),
};

let mut builder = wallet.build_tx();
builder
.add_recipient(addr.script_pubkey(), Amount::from_sat(30_000))
.add_recipient(addr.script_pubkey(), Amount::from_sat(10_000))
.ordering(bdk_wallet::wallet::tx_builder::TxOrdering::Bip69Lexicographic);
.ordering(custom_bip69_ordering);

let psbt = builder.finish().unwrap();
let fee = check_fee!(wallet, psbt);

Expand Down

0 comments on commit 3bee563

Please sign in to comment.