fix(wallet): add support for bip-431 rule 2 by oleonardolima · Pull Request #478 · bitcoindevkit/bdk_wallet

oleonardolima · 2026-05-04T20:30:40Z

partially addresses #477
supersedes #442

Description

I've tried this another approach in order to add support for BIP-431 rule 2. I think this way is clearer and more functional than the previously attempted in #442.

It introduces a new private helper method is_truc, it's useful as this validation will be necessary for other BIP-431 rules. Also, it adds a new filter into filter_utxos in order to filter out unconfirmed UTXOs from non-TRUC/TRUC txs.

~~It also introduces the usage of bdk_testenv, as there's some validation that is checked under node policy level.~~

Notes to the reviewers

Let me know what you think about this one.

Changelog notice

### Fixed

- fix(wallet): filter unconfirmed UTXOs by BIP-431 (TRUC) rule-2

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
I ran just p before pushing

New Features:

I've added tests for the new feature
I've added docs for the new feature

Bugfixes:

This pull request breaks the existing API
I've added tests to reproduce the issue which are now passing
I'm linking the issue being fixed by this PR

oleonardolima · 2026-05-04T20:31:27Z

cc @yan-pi as he already reviewed the other PR and has a reproducible test using bdk-cli too.

oleonardolima · 2026-05-04T20:34:23Z

I noticed in CI I need to fix some bdk_testenv/electrs/esplora related issues. However, the fix in bc05f88 is ready for review/discussion.

codecov · 2026-05-04T20:37:10Z

Codecov Report

❌ Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.00%. Comparing base (58fe631) to head (6145afc).

Files with missing lines	Patch %	Lines
src/wallet/mod.rs	90.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #478      +/-   ##
==========================================
+ Coverage   80.96%   81.00%   +0.04%     
==========================================
  Files          24       24              
  Lines        5489     5507      +18     
  Branches      247      249       +2     
==========================================
+ Hits         4444     4461      +17     
  Misses        968      968              
- Partials       77       78       +1

Flag	Coverage Δ
rust	`81.00% <90.00%> (+0.04%)`	⬆️

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.

yan-pi

tACK 1d32117.

IMO The is_truc helper is more functional and reads cleaner than the XNOR in #442, and definitely is a good foundation for the rest of #477.

Tested it locally by checking out test/unconfirmed-truc-txs and rebuilding the rust-repro binaries from https://github.com/yan-pi/bdk-pr442-repro against the PR via [dependencies] bdk_wallet = { path = "../../bdk_wallet" }:

The inverse direction of Rule 2: the non-v3 unconfirmed UTXO was filtered correclty.

One concern not flagged inline: the integration test is great, but maybe the unit-level coverage from #442 is also worth keeping?

Plus two minor nits and typos.

lorenzolfm

ACK 85143c2

- introduces a private method `is_truc` to check if a given tx version is TRUC (e.g `transaction::Version(3)`). - add new step in `filter_utxos` to validate BIP-431 rule 2, which validates the proper usage of unconfirmed TRUC/non-TRUC ancestor in a TRUC/non-TRUC tx.

- introduce `test_create_and_spend_from_tx` to exercise BIP-431 rule-2 filtering in-memory, not relying on `bdk_testenv`/`bdk_electrum`. NOTE: I asked Claude to remove the `bdk_testenv` dependency, keeping the test behavior in-memory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

lorenzolfm

ACK 6145afc

ValuedMammal · 2026-07-09T20:06:13Z

+                    if local_output.chain_position.is_confirmed() {
+                        return true;
+                    }
+
+                    let Some(ancestor_tx) = self.tx_graph().get_tx(local_output.outpoint.txid)
+                    // if we don't have the full tx available we can't assure the ancestor
+                    // tx version it assumes it's a valid candidate.
+                    else {
+                        return true;
+                    };
+
+                    match is_truc(version) {
+                        // if building TRUC; filter out all unconfirmed non-TRUC.
+                        true => is_truc(ancestor_tx.version),
+                        // if building non-TRUC; filter out all unconfirmed TRUC.
+                        false => !is_truc(ancestor_tx.version),
+                    }


I think #442 had the right idea with the bidirectional filter.

Suggested change

if local_output.chain_position.is_confirmed() {

return true;

}

let Some(ancestor_tx) = self.tx_graph().get_tx(local_output.outpoint.txid)

// if we don't have the full tx available we can't assure the ancestor

// tx version it assumes it's a valid candidate.

else {

return true;

};

match is_truc(version) {

// if building TRUC; filter out all unconfirmed non-TRUC.

true => is_truc(ancestor_tx.version),

// if building non-TRUC; filter out all unconfirmed TRUC.

false => !is_truc(ancestor_tx.version),

}

local_output.chain_position.is_confirmed()

|| self

.tx_graph()

.get_tx(local_output.outpoint.txid)

.is_some_and(|tx| (tx.version == Version(3)) == (version == Version(3)))

I don't see the difference, it's filtering the same thing. However, having the predicate makes the code easier to follow.

@ValuedMammal I'd say something like the code below is better, though it's does not read well for users not aware of what's happening.

local_output.chain_position.is_confirmed() || self .tx_graph() .get_tx(local_output.outpoint.txid) .is_some_and(|tx| is_truc(tx.version) && is_truc(version))

ValuedMammal · 2026-07-09T20:14:31Z

+/// Check if the given [`transaction::Version`] is TRUC (Topologically Restricted Until
+/// Confirmation).
+fn is_truc(version: transaction::Version) -> bool {
+    version.eq(&Version(3))
+}


If going for a private helper, I'd argue this is clearer.

Suggested change

/// Check if the given [`transaction::Version`] is TRUC (Topologically Restricted Until

/// Confirmation).

fn is_truc(version: transaction::Version) -> bool {

version.eq(&Version(3))

}

/// The transaction is considered to have opted in to v3 TRUC policy if the transaction version is 3

/// (BIP431).

fn has_opted_into_v3_truc_policy(tx: &Transaction) -> bool {

tx.version == Version(3)

}

The docs looks better, though we don't have the full transaction when creating it, only the version is available.

oleonardolima added this to the Wallet 3.1.0 milestone May 4, 2026

oleonardolima requested a review from ValuedMammal May 4, 2026 20:30

oleonardolima self-assigned this May 4, 2026

oleonardolima added the bug Something isn't working label May 4, 2026

oleonardolima added this to BDK Wallet May 4, 2026

yan-pi reviewed May 6, 2026

View reviewed changes

Comment thread src/wallet/mod.rs Outdated

Comment thread src/wallet/mod.rs Outdated

ValuedMammal moved this to In Progress in BDK Wallet May 8, 2026

EliteCoder18 mentioned this pull request Jun 1, 2026

fix(wallet): exclude unconfirmed v3 outputs from non-v3 coin selection #442

Closed

6 tasks

lorenzolfm reviewed Jun 1, 2026

View reviewed changes

Comment thread src/wallet/mod.rs Outdated

yan-pi mentioned this pull request Jun 1, 2026

feat(wallet): add support for bip-431 rules 4 and 5 #493

Open

8 tasks

luisschwab self-requested a review June 1, 2026 23:39

oleonardolima modified the milestones: Wallet 3.1.0, Wallet 3.2.0 Jun 5, 2026

oleonardolima force-pushed the test/unconfirmed-truc-txs branch 3 times, most recently from b8d9ae1 to 85143c2 Compare June 5, 2026 00:44

lorenzolfm approved these changes Jun 9, 2026

View reviewed changes

ValuedMammal marked this pull request as draft June 10, 2026 15:31

oleonardolima force-pushed the test/unconfirmed-truc-txs branch 2 times, most recently from 47bba04 to d6fca63 Compare June 15, 2026 19:08

oleonardolima requested review from lorenzolfm and yan-pi June 15, 2026 19:12

oleonardolima force-pushed the test/unconfirmed-truc-txs branch from d6fca63 to 6145afc Compare June 15, 2026 19:23

oleonardolima moved this from In Progress to Needs Review in BDK Wallet Jun 15, 2026

oleonardolima marked this pull request as ready for review June 15, 2026 19:25

lorenzolfm approved these changes Jun 16, 2026

View reviewed changes

oleonardolima requested review from notmandatory, nymius and yan-pi and removed request for yan-pi June 17, 2026 01:42

ValuedMammal requested changes Jul 9, 2026

View reviewed changes

oleonardolima requested a review from ValuedMammal July 10, 2026 15:37

Uh oh!

Conversation

oleonardolima commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

New Features:

Bugfixes:

Uh oh!

oleonardolima commented May 4, 2026

Uh oh!

oleonardolima commented May 4, 2026

Uh oh!

codecov Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yan-pi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lorenzolfm left a comment

Choose a reason for hiding this comment

Uh oh!

lorenzolfm left a comment

Choose a reason for hiding this comment

Uh oh!

ValuedMammal Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

oleonardolima Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

oleonardolima Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

ValuedMammal Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

oleonardolima Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

oleonardolima commented May 4, 2026 •

edited

Loading

codecov Bot commented May 4, 2026 •

edited

Loading

yan-pi left a comment •

edited

Loading

oleonardolima Jul 9, 2026 •

edited

Loading