feat(wallet): add support for bip-431 rules 4 and 5#493
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #493 +/- ##
==========================================
+ Coverage 80.96% 81.13% +0.17%
==========================================
Files 24 24
Lines 5489 5544 +55
Branches 247 254 +7
==========================================
+ Hits 4444 4498 +54
Misses 968 968
- Partials 77 78 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
yan-pi
force-pushed
the
feat/truc-vsize-caps
branch
3 times, most recently
from
a8c7d37 to
634d476
Compare
- 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>
|
@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
force-pushed
the
feat/truc-vsize-caps
branch
from
634d476 to
1e9f37e
Compare
oleonardolima
requested review from
ValuedMammal and
oleonardolima
as code owners
|
@yan-pi the CI still failing |
yan-pi
force-pushed
the
feat/truc-vsize-caps
branch
from
1e9f37e to
1ae1a28
Compare
|
Rebased this on top of the updated #478 branch. I dropped the old |
oleonardolima
changed the title
| /// 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, | ||
| }, |
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
- 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
force-pushed
the
feat/truc-vsize-caps
branch
from
1ae1a28 to
ebf57d7
Compare
3 participants
partially addresses #477
depends of #478
solves #484
solves #485
Description
Enforces BIP-431 Rules 4 and 5 inside
create_tx, before returning a PSBT:Without this,
build_tx().version(3).finish()hands back a valid PSBT that bitcoind rejects at broadcast.The new
CreateTxError::TrucSizeExceeded { cap_vb, actual_vb }tells callers which rule fired and by how much, so they can react (fewer inputs, split the payment, etc.).It adds
estimate_truc_vsizeas a private helper, and reuses theis_truchelper andbdk_testenvdev-dependency introduced in #478.This does not address Rules 1–3 (topology / package limits), that's a separate concern tracked in #477.
Notes to the reviewers
The check runs after
coin_selectand beforecomplete_transaction. That's the only viable placement that I found.CoinSelection::coin_selectconsumesVec<WeightedUtxo>and returnsVec<Utxo>, so the per-inputsatisfaction_weightis gone by the time we'd want it.We pre-collect a
HashMap<OutPoint, Weight>from theWeightedUtxolist 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 an followup to tracks proper sigop accounting.
Changelog notice
Checklists
All Submissions:
just pbefore pushingNew Features:
Bugfixes: