feat(wallet): add TxBuilder::avoid_reuse to skip reused-address UTXOs

[tvpeter]

Description

Problem:

An adversary can attack a wallet's privacy through forced address reuse: after observing one of the addresses the user have already spent from, they can send small outputs to it. If the users' wallet later select those coins into a transaction, the adversary learns which inputs the user controls and the destinations they pay to, linking their UTXOs.

The TxBuilder has no way to automatically prevent spending outputs in an address that has already been spent from. This is one of the open privacy items tracked in #28, and mirrors Bitcoin Core's avoid_reuse wallet flag (bitcoin/bitcoin#13756).

Approach:

Add an opt-in transaction builder method:

let mut builder = wallet.build_tx();
builder
    .add_recipient(addr.script_pubkey(), Amount::from_sat(20_000))
    .avoid_reuse();
let psbt = builder.finish()?;

Tradeoff:

• An address that received change and was later used to receive outputs again will be flagged, and its outputs excluded from automatic selection (they remain spendable via add_utxo()). This can lead to InsufficientFunds when the only funds available are on reused addresses.

Notes to the reviewers

• An address (scriptPubKey) is treated as reused when the wallet has indexed more than one output to it. Since each (keychain, derivation_index) maps 1:1 to a scriptPubKey, outputs are counted per index from spk_index().outpoints() (which includes already-spent outputs, so the spend then dust attack is caught).
• Every UTXO on a reused address is added to the existing unspendable set. They can still be spent when selected explicitly via add_utxo().

Changelog notice

• Add TxBuilder::avoid_reuse to prevent TxBuilder from selecting reused address UTXOs

Before submitting

• I followed the contribution guidelines
• This PR breaks the existing API

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.70%. Comparing base (10efa9e) to head (4413067).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #541      +/-   ##
==========================================
+ Coverage   81.66%   81.70%   +0.03%     
==========================================
  Files          25       25              
  Lines        6339     6351      +12     
  Branches      302      303       +1     
==========================================
+ Hits         5177     5189      +12     
  Misses       1055     1055              
  Partials      107      107              

Flag	Coverage Δ
rust	81.70% <100.00%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Dmenec]

cACK, nice feature :)

AFAIK Core only flags an address as used once the wallet has spent from it from then on, any coin sent to it is avoided. The PR description describes it the same way, but the implementation flags any address with more than one received output, even if it was never spent from.

Small test showing it:

let addr = wallet.reveal_next_address(KeychainKind::External).address;

// sent 2 outputs to the same address
receive_output_to_address(&mut wallet, addr.clone(), Amount::from_sat(100_000), ReceiveTo::Mempool(0));
receive_output_to_address(&mut wallet, addr, Amount::from_sat(546), ReceiveTo::Mempool(0));

let mut builder = wallet.build_tx();
builder.add_recipient(recipient.script_pubkey(), Amount::from_sat(10_000)).avoid_reuse();

// the 100k sats are excluded
assert!(matches!(
    builder.finish(),
    Err(CreateTxError::CoinSelection(e)) if e.available == Amount::ZERO
));

Not sure if this was intentional. If it was, I'd document that it differs from Core. I would follow Core's approach as anyone could grief UTXOs knowing this feature.

[Dmenec]

nit: could assert the reused coins are actually selected

Suggested change

	assert!(builder.finish().is_ok());
	let psbt = builder.finish().unwrap();
	let selected: Vec<OutPoint> = psbt
	.unsigned_tx
	.input
	.iter()
	.map(|i| i.previous_output)
	.collect();
	assert!(selected.contains(&reused_outpoint));
	assert!(selected.contains(&reused_outpoint_2));

[Chibey-max]

Should “reused address” mean “received more than one output,” or should it mean “the wallet has already spent from this address, and then later received more coins there”?

[tvpeter]

AFAIK Core only flags an address as used once the wallet has spent from it from then on, any coin sent to it is avoided. The PR description describes it the same way, but the implementation flags any address with more than one received output, even if it was never spent from.

Thank you so much for this review. I have updated the approach to use the wallet's canonical history (Wallet::list_output) to know an address that has been spent from. I have also updated the test cases to show that a never-spent from address is not excluded even if it receives outputs more than once; a spent-from address is excluded (spendable via explicit selection), and a fee-bumped transaction is not counted as a reuse.

[tvpeter]

The CI is failing because of a transitive dependency yoke-derive. The recent updated version not been compatible with rustc 1.85. This was fixed on release/3.x branch by #568. I will duplicate the fix in another PR.

[tvpeter]

The CI is failing because of a transitive dependency yoke-derive. The recent updated version not been compatible with rustc 1.85. This was fixed on release/3.x branch by #568. I will duplicate the fix in another PR.

Minor PR to pin yoke-derive #572

[noahjoeris]

Interesting!

Why did you add it to TxBuilder and not the new create_psbt (bdk-tx) path?

[tvpeter]

Why did you add it to TxBuilder and not the new create_psbt (bdk-tx) path?

I thought that since the feature is non-breaking and tiny, and adds some privacy improvement, shipping it on the next TxBuilder stable release won't be bad. But, I'll certainly add it to create_psbt.

[noahjoeris]

cACK

Maybe we should add a more explicit warning? something like:

WARNING: Coins on spks the wallet already spent from are not used, but still counted in balance(), so it can fail with InsufficientFunds despite enough balance.

[tvpeter]

Maybe we should add a more explicit warning? something like:

WARNING: Coins on spks the wallet already spent from are not used, but still counted in balance(), so it can fail with InsufficientFunds despite enough balance.

I have updated the method docs to include a warning and an example.
Thank you

[Dmenec]

I'm thinking of how we could allow Balance to be aware of it. With the new chain's Balance refactor (bitcoindevkit/bdk#2246) we could create our own fold over Eligibility in the wallet and put the coins sitting on dirty addresses in a separate bucket. Core does something similar, getbalances reports a used balance when avoid_reuse is enabled.

[tvpeter]

Great idea worth exploring. Unlike Core, where avoid_reuse is a persisted wallet flag, this is a per TxBuilder option. So introducing a new balance bucket would therefore require either always reporting the bucket as part of the normal Balance or adding something like balance_with_avoid_reuse() or accepting a boolean parameter in the balance (defaulting to off). Any of these approaches might work but this is definitely worth discussing.

[Dmenec]

tACK 4413067

Looks good!

last nit: Core’s and wallet’s in the doc use ’ instead of ', the rest of the doc and the codebase use the ASCII one.