Skip to content

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

Open
yan-pi wants to merge 2 commits into
bitcoindevkit:masterfrom
yan-pi:feat/truc-vsize-caps
Open

feat(wallet): add support for bip-431 rules 4 and 5#493
yan-pi wants to merge 2 commits into
bitcoindevkit:masterfrom
yan-pi:feat/truc-vsize-caps

Conversation

@yan-pi

@yan-pi yan-pi commented Jun 1, 2026

Copy link
Copy Markdown

partially addresses #477
depends on #478
solves #484
solves #485

Description

Enforces BIP-431 Rules 4 and 5 inside create_tx, before returning a PSBT:

  • Rule 4: rejects any TRUC (nVersion=3) transaction with sigop-adjusted vsize over 10,000 vB
  • Rule 5: tightens the cap to 1,000 vB when the transaction has an unconfirmed TRUC ancestor

Without this, build_tx().version(3).finish() can hand back a PSBT that bitcoind rejects at broadcast.

When the cap is exceeded, create_tx returns the new CreateTxError::TrucSizeExceeded { cap_vb, actual_vb }, so callers can tell which rule fired and by how much.

Adding a variant to the public CreateTxError is a breaking change, this is acceptable since the PR targets the 4.0 release.

This adds estimate_truc_vsize as a private helper and reuses the is_truc helper introduced in #478.

This does not address Rules 1–3 (topology / package limits), that's a separate concern tracked in #477 and #478.

Notes to the reviewers

The check runs after coin_select and before complete_transaction.

CoinSelection::coin_select consumes Vec<WeightedUtxo> and returns Vec<Utxo>, so the per-input satisfaction_weight is gone by the time we'd want it.

We pre-collect a HashMap<OutPoint, Weight> from the WeightedUtxo list before handing it to the selector.
The map is only allocated when version == 3.

For the Rule 5 ancestor check, foreign UTXOs are conservatively treated as non-TRUC.
Same posture as the existing TRUC filter in filter_utxos.

We use plain weight / 4. Bitcoind's TRUC check uses sigop-adjusted vsize (max(weight, sigops * 20) / 4), which matches plain weight/4 for P2WPKH, P2TR and ordinary P2WSH.

#477 should have a follow-up to track proper sigop accounting.

Changelog notice

Added CreateTxError::TrucSizeExceeded { cap_vb, actual_vb }.
`create_tx` now enforces BIP-431 Rules 4 and 5 (vsize caps) for nVersion=3 transactions.

Checklists

All Submissions:

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

@codecov

codecov Bot commented Jun 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.28571% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.91%. Comparing base (7982b6e) to head (eff5481).
⚠️ Report is 30 commits behind head on master.

Files with missing lines Patch % Lines
src/wallet/error.rs 0.00% 4 Missing ⚠️
src/wallet/mod.rs 96.15% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #493      +/-   ##
==========================================
+ Coverage   81.22%   81.91%   +0.68%     
==========================================
  Files          24       25       +1     
  Lines        5565     6529     +964     
  Branches      248      301      +53     
==========================================
+ Hits         4520     5348     +828     
- Misses        969     1081     +112     
- Partials       76      100      +24     
Flag Coverage Δ
rust 81.91% <89.28%> (+0.68%) ⬆️

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.

Comment thread src/wallet/mod.rs Outdated
Comment thread src/wallet/mod.rs Outdated
Comment thread src/wallet/mod.rs Outdated
@oleonardolima oleonardolima added the bug Something isn't working label Jun 5, 2026
@oleonardolima oleonardolima moved this to In Progress in BDK Wallet Jun 5, 2026
@oleonardolima oleonardolima added this to the Wallet 3.2.0 milestone Jun 5, 2026
@yan-pi
yan-pi force-pushed the feat/truc-vsize-caps branch 3 times, most recently from a8c7d37 to 634d476 Compare June 11, 2026 12:14
@oleonardolima

Copy link
Copy Markdown
Contributor

@yan-pi I updated my branch to remove the bdk_testenv dependency, you can rebase this one to get a green CI (though you might need to update your tests).

@yan-pi
yan-pi force-pushed the feat/truc-vsize-caps branch from 634d476 to 1e9f37e Compare June 19, 2026 13:50
@oleonardolima
oleonardolima marked this pull request as ready for review June 19, 2026 14:09
@oleonardolima
oleonardolima marked this pull request as draft June 19, 2026 14:10
@oleonardolima

Copy link
Copy Markdown
Contributor

@yan-pi the CI still failing

@yan-pi
yan-pi force-pushed the feat/truc-vsize-caps branch from 1e9f37e to 1ae1a28 Compare June 19, 2026 17:52
@yan-pi

yan-pi commented Jun 19, 2026

Copy link
Copy Markdown
Author

Rebased this on top of the updated #478 branch.

I dropped the old bdk_testenv e2e test and kept the TRUC Rule 4/5 coverage in-memory.

@oleonardolima
oleonardolima marked this pull request as ready for review June 20, 2026 15:49
@oleonardolima oleonardolima changed the title Add BIP-431 Rules 4 and 5 feat(wallet): add support for bip-431 rules 4 and 5 Jun 20, 2026
Comment thread src/wallet/error.rs
Comment on lines +218 to +227
/// TRUC (BIP-431) virtual size cap exceeded.
///
/// `cap_vb == 10_000` means Rule 4 (any TRUC tx).
/// `cap_vb == 1_000` means Rule 5 (TRUC tx with unconfirmed TRUC ancestor).
TrucSizeExceeded {
/// The cap that was exceeded, in virtual bytes.
cap_vb: u64,
/// The estimated virtual size of the candidate transaction.
actual_vb: u64,
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yan-pi did you manage to do it as a non-breaking change ? I agree that having the error would be the right approach, though it would only land in the next major release.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dropped the new variant and mapped the TRUC size-cap rejection to the existing CreateTxError::CoinSelection(InsufficientFunds { .. }) instead.

This one seems to be the best one to handle the current error without the breaking change.

Comment thread src/wallet/mod.rs Outdated
@yan-pi
yan-pi force-pushed the feat/truc-vsize-caps branch from ebf57d7 to 9b35f7f Compare July 20, 2026 16:21
@yan-pi

yan-pi commented Jul 20, 2026

Copy link
Copy Markdown
Author

rebased on top of 7982b6e

@oleonardolima oleonardolima moved this from In Progress to Needs Review in BDK Wallet Jul 21, 2026
@ValuedMammal ValuedMammal moved this from Needs Review to In Progress in BDK Wallet Jul 28, 2026
@yan-pi

yan-pi commented Aug 4, 2026

Copy link
Copy Markdown
Author

Since this moved to 4.0, I want to check if I understood the direction.

coin-select#51 now has Target::max_weight and MaxWeightExceeded. The current wallet CoinSelectionAlgorithm does not use bdk_coin_select or take a weight limit though, so bringing that here would mean changing the public trait and built-in selectors. bdk-tx#47 seems to be handling something similar in the new path.

@ValuedMammal is that what you had in mind for #493: change the current coin-selection API, or keep the final Rules 4/5 check here and leave max-weight-aware selection for bdk_tx?

I also noticed the cap isn't fixed: it's 10k without an unconfirmed ancestor and 1k when the selection has one, so I think we may need two selection attempts.

btw, I used AI while researching this, mostly to understand how the current coin-selection work fits together across the repos.

@ValuedMammal

Copy link
Copy Markdown
Contributor

@yan-pi I would say bring back the error you had originally designed for and keep the TRUC_MAX_VSIZE_VB validation logic the same.

yan-pi added 2 commits August 5, 2026 21:40
- adds `CreateTxError::TrucSizeExceeded { cap_vb, actual_vb }`.
- rejects v3 txs over 10,000 vB (Rule 4), or over 1,000 vB when the
  tx has an unconfirmed TRUC ancestor (Rule 5).
- adds private helper `estimate_truc_vsize` using plain weight/4.
- covers rejection and acceptance for both rules, plus a v2 case to
  ensure non-TRUC builds aren't affected.
- end-to-end test funds via regtest electrum, broadcasts a v3 parent,
  and exercises both the Rule 5 rejection and the small-child acceptance
  paths through the mempool.
@yan-pi
yan-pi force-pushed the feat/truc-vsize-caps branch from 9b35f7f to eff5481 Compare August 6, 2026 00:44
@yan-pi

yan-pi commented Aug 18, 2026

Copy link
Copy Markdown
Author

Ive restored the dedicated CreateTxError::TrucSizeExceeded { cap_vb, actual_vb } variant. I Think its ready for another review round, or ready to be merged.

@110CodingP 110CodingP left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oleonardolima @ValuedMammal we were planning to remove the TxBuilder in 4.0 right? so these changes should be done to create_psbt instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

5 participants