refactor(wallet)!: remove TxOrdering::Bip69Lexicographic

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 <steve@notmandatory.org>

diff --git a/crates/wallet/src/wallet/tx_builder.rs b/crates/wallet/src/wallet/tx_builder.rs
index 7a021f740d1afab8bf648a925d651efa7f91200c..7e6157cfd8ccbf04b916bb07eca89fcb02a9cabd 100644 (file)
--- a/crates/wallet/src/wallet/tx_builder.rs
+++ b/crates/wallet/src/wallet/tx_builder.rs
@@ -776,8 +776,6 @@ pub enum TxOrdering {
     Shuffle,
     /// Unchanged
     Untouched,
-    /// BIP69 / Lexicographic
-    Bip69Lexicographic,
     /// Provide custom comparison functions for sorting
     Custom {
         /// Transaction inputs sort function
@@ -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"),
         }
     }
@@ -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,
@@ -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;

diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs
index 08579396e90ea376d215210c023694897cfed462..e1393fef0fed65c173420b84ba639abe3a3e217e 100644 (file)
--- a/crates/wallet/tests/wallet.rs
+++ b/crates/wallet/tests/wallet.rs
@@ -1,3 +1,5 @@
+extern crate alloc;
+
 use std::path::Path;
 use std::str::FromStr;
 
@@ -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);