From b73c46ac59af0f3902be0f94b771a98045698531 Mon Sep 17 00:00:00 2001 From: Cindy Date: Sat, 25 Jul 2026 21:30:41 -0300 Subject: [PATCH] Shuffle change output on hardware wallet and Bitcoin Cash sends (#3432) * Shuffle change output on hardware wallet sends Hardware wallet BTC/LTC sends passed outputs to the PSBT/device in the given order (change last), a position fingerprint. The outputOrdering param was plumbed but ignored. Add orderOutputs() and apply it in both buildHardwareWalletTransaction paths; set the send call site to shuffle. Refs #3376 (the software send + RBF paths were covered by #3420). * Shuffle change output on Bitcoin Cash sends The BCH send path built via ForkedTransactionBuilder with outputOrdering: none, leaving change deterministically last. The builder shuffles natively and change is found by isChange, not position, so flip it to shuffle (matches the BTC software path from #3420). Refs #3376. * Update cw_bitcoin/lib/electrum_wallet.dart Co-authored-by: Omar Hatem --------- Co-authored-by: Omar Hatem --- cw_bitcoin/lib/bitcoin_wallet.dart | 8 +++- cw_bitcoin/lib/electrum_wallet.dart | 9 +++- cw_bitcoin/lib/litecoin_wallet.dart | 6 ++- cw_bitcoin/lib/output_ordering.dart | 21 +++++++++ cw_bitcoin/test/output_ordering_test.dart | 52 +++++++++++++++++++++++ 5 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 cw_bitcoin/lib/output_ordering.dart create mode 100644 cw_bitcoin/test/output_ordering_test.dart diff --git a/cw_bitcoin/lib/bitcoin_wallet.dart b/cw_bitcoin/lib/bitcoin_wallet.dart index 7e76132a8..b8e53d11b 100644 --- a/cw_bitcoin/lib/bitcoin_wallet.dart +++ b/cw_bitcoin/lib/bitcoin_wallet.dart @@ -17,6 +17,7 @@ import 'package:cw_bitcoin/electrum_wallet_snapshot.dart'; import 'package:cw_bitcoin/hardware/bitcoin_hardware_wallet_service.dart'; import 'package:cw_bitcoin/lightning/lightning_wallet.dart'; import 'package:cw_bitcoin/hardware/bitcoin_ledger_service.dart'; +import 'package:cw_bitcoin/output_ordering.dart'; import 'package:cw_bitcoin/payjoin/manager.dart'; import 'package:cw_bitcoin/payjoin/storage.dart'; import 'package:cw_bitcoin/pending_bitcoin_transaction.dart'; @@ -467,8 +468,10 @@ abstract class BitcoinWalletBase extends ElectrumWallet with Store { final masterFingerprint = await (hardwareWalletService as BitcoinHardwareWalletService).getMasterFingerprint(); + final orderedOutputs = orderOutputs(outputs, outputOrdering); + final psbt = await buildPsbt( - outputs: outputs, + outputs: orderedOutputs, fee: fee, network: network, utxos: utxos, @@ -478,7 +481,8 @@ abstract class BitcoinWalletBase extends ElectrumWallet with Store { memo: memo, enableRBF: enableRBF, inputOrdering: inputOrdering, - outputOrdering: outputOrdering, + // Already applied above; don't reorder again. + outputOrdering: BitcoinOrdering.none, ); final psbtStr = base64Encode(psbt.serialize()); diff --git a/cw_bitcoin/lib/electrum_wallet.dart b/cw_bitcoin/lib/electrum_wallet.dart index 6396abedb..ceba0eccd 100644 --- a/cw_bitcoin/lib/electrum_wallet.dart +++ b/cw_bitcoin/lib/electrum_wallet.dart @@ -1485,7 +1485,9 @@ abstract class ElectrumWalletBase fee: estimatedTx.fee.amount, network: network, memo: estimatedTx.memo, - outputOrdering: BitcoinOrdering.none, + // Shuffle so the change output isn't placed deterministically last + // (privacy fingerprint). Applied by orderOutputs in the builder. + outputOrdering: BitcoinOrdering.shuffle, enableRBF: true, cwOutputs: transactionCredentials.outputs, ); @@ -1518,7 +1520,10 @@ abstract class ElectrumWalletBase fee: estimatedTx.fee.amount, network: network, memo: estimatedTx.memo, - outputOrdering: BitcoinOrdering.none, + // Shuffle so the change output isn't placed deterministically last + // (privacy fingerprint). Change is found by isChange, not position. + inputOrdering: BitcoinOrdering.shuffle, + outputOrdering: BitcoinOrdering.shuffle, enableRBF: !estimatedTx.spendsUnconfirmedTX, ); } else { diff --git a/cw_bitcoin/lib/litecoin_wallet.dart b/cw_bitcoin/lib/litecoin_wallet.dart index 23959374c..aa8fcd330 100644 --- a/cw_bitcoin/lib/litecoin_wallet.dart +++ b/cw_bitcoin/lib/litecoin_wallet.dart @@ -30,6 +30,7 @@ import 'package:cw_bitcoin/electrum_wallet.dart'; import 'package:cw_bitcoin/electrum_wallet_snapshot.dart'; import 'package:cw_bitcoin/hardware/bitcoin_hardware_wallet_service.dart'; import 'package:cw_bitcoin/litecoin_wallet_addresses.dart'; +import 'package:cw_bitcoin/output_ordering.dart'; import 'package:cw_bitcoin/pending_bitcoin_transaction.dart'; import 'package:cw_bitcoin/psbt/transaction_builder.dart'; import 'package:cw_bitcoin/utils.dart'; @@ -1584,8 +1585,11 @@ abstract class LitecoinWalletBase extends ElectrumWallet with Store { )); } + final orderedOutputs = orderOutputs(outputs, outputOrdering); + final rawHex = await (hardwareWalletService as LitecoinHardwareWalletService) - .signLitecoinTransaction(outputs: outputs, inputs: readyInputs, publicKeys: publicKeys); + .signLitecoinTransaction( + outputs: orderedOutputs, inputs: readyInputs, publicKeys: publicKeys); return BtcTransaction.fromRaw(rawHex); } diff --git a/cw_bitcoin/lib/output_ordering.dart b/cw_bitcoin/lib/output_ordering.dart new file mode 100644 index 000000000..36a980e93 --- /dev/null +++ b/cw_bitcoin/lib/output_ordering.dart @@ -0,0 +1,21 @@ +import 'dart:math'; + +import 'package:bitcoin_base/bitcoin_base.dart'; + +/// Applies [ordering] to a transaction's [outputs] before building. +/// +/// Returns a new list; the input is never mutated. Only [BitcoinOrdering.shuffle] +/// reorders (using a secure RNG by default) so the change output is no longer +/// placed deterministically last. Any other value preserves the given order, +/// matching the pre-existing hardware-wallet behavior. +List orderOutputs( + List outputs, + BitcoinOrdering ordering, { + Random? rng, +}) { + final result = List.of(outputs); + if (ordering == BitcoinOrdering.shuffle) { + result.shuffle(rng ?? Random.secure()); + } + return result; +} diff --git a/cw_bitcoin/test/output_ordering_test.dart b/cw_bitcoin/test/output_ordering_test.dart new file mode 100644 index 000000000..d0fc7de05 --- /dev/null +++ b/cw_bitcoin/test/output_ordering_test.dart @@ -0,0 +1,52 @@ +import 'dart:math'; + +import 'package:bitcoin_base/bitcoin_base.dart'; +import 'package:cw_bitcoin/output_ordering.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + group('orderOutputs', () { + test('none preserves the given order', () { + final outputs = [0, 1, 2, 3, 4]; + expect(orderOutputs(outputs, BitcoinOrdering.none), outputs); + }); + + test('does not mutate the input list', () { + final outputs = [0, 1, 2, 3, 4]; + orderOutputs(outputs, BitcoinOrdering.shuffle, rng: Random(1)); + expect(outputs, [0, 1, 2, 3, 4]); + }); + + test('returns a new list instance', () { + final outputs = [0, 1, 2]; + expect(identical(orderOutputs(outputs, BitcoinOrdering.none), outputs), isFalse); + }); + + test('shuffle preserves the multiset of outputs', () { + final outputs = [0, 1, 2, 3, 4, 5, 6, 7]; + final shuffled = orderOutputs(outputs, BitcoinOrdering.shuffle, rng: Random(7)); + expect(shuffled.length, outputs.length); + expect(shuffled.toSet(), outputs.toSet()); + }); + + test('shuffle is deterministic for a given seed', () { + final outputs = [0, 1, 2, 3, 4, 5]; + final a = orderOutputs(outputs, BitcoinOrdering.shuffle, rng: Random(42)); + final b = orderOutputs(outputs, BitcoinOrdering.shuffle, rng: Random(42)); + expect(a, b); + }); + + test('shuffle does not keep the change output deterministically last', () { + // Last element (99) models the change output, appended last today. + final outputs = [0, 1, 2, 3, 99]; + final changePositions = {}; + for (var seed = 0; seed < 50; seed++) { + final shuffled = orderOutputs(outputs, BitcoinOrdering.shuffle, rng: Random(seed)); + changePositions.add(shuffled.indexOf(99)); + } + // Across seeds the change lands in more than one position, and not always last. + expect(changePositions.length, greaterThan(1)); + expect(changePositions, isNot(equals({outputs.length - 1}))); + }); + }); +}