fix: enforce Core's std tx weight limit when building transactions by tvpeter · Pull Request #361 · bitcoindevkit/bdk-cli

tvpeter · 2026-10-19T19:44:49Z

Description

wallet create_tx could build transactions that are non-standard under Bitcoin Core's default policy, so they failed only at broadcast, after signing:

Nothing limited the transaction to MAX_STANDARD_TX_WEIGHT (400,000 wu / 100,000 vB). As shown in fix transaction creation input validation and fee rate calculation #326, a 99,994 byte --add_string payload produced a 100,150 vB tx that the node rejected with tx-size.
--add_data and --add_string documented "max 80 bytes" but was never enforced, and it stopped being Core's default in v30.

This PR rejects such transactions when they are built, with an error giving the weight and the limit. The check covers create_tx, create_dns_tx, bump_fee (where a fee bump can add inputs and push a tx over), and create_sp_tx.

It also parses --add_data/--add_string with clap value parsers, the same way as --fee_rate. That removes three copies of the same decoding block.

Fixes #339

Notes to the reviewers

Why there is no separate datacarrier check: in Core v30, IsStandardTx checks tx-size before datacarrier, and MAX_OP_RETURN_RELAY is defined as MAX_STANDARD_TX_WEIGHT / WITNESS_SCALE_FACTOR. OP_RETURN bytes are non-witness, so at default settings any tx that passes the weight check also passes datacarrier. A separate check would be dead code.
Accounting for witnesses (from the fix transaction creation input validation and fee rate calculation #326 discussion): All four commands check before signing. Their weight is estimated with each wallet input's max_weight_to_satisfy(), the same worst case coin selection uses. The estimate is an upper bound: a tx under it can't go over once signed. In return, a segwit tx within about 1 wu per input of the limit may be rejected. Inputs are looked up through spk_index() rather than get_utxo(), because a fee bump's inputs are already spent.
Inputs the wallet doesn't own add no witness weight to the estimate.
Testing: the unit tests sign real transactions for tr, wpkh, sh(wpkh), pkh and a fee bump, and check that the estimate bounds the signed weight tightly.
Behaviour change: malformed --add_data base64 now exits 2 at argument parsing instead of 1, consistent with --fee_rate.

Changelog notice

Fixed create_tx, create_dns_tx, create_sp_tx and bump_fee building transactions over Bitcoin Core's 400,000 wu (100,000 vB) standard weight limit; they are now rejected when built, counting the witnesses still to be signed

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
I ran cargo fmt and cargo clippy before committing

Bugfixes:

I've added tests to reproduce the issue which are now passing
I'm linking the issue being fixed by this PR

codecov · 2026-10-19T19:46:44Z

Codecov Report

❌ Patch coverage is 97.16981% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.97%. Comparing base (d2317d2) to head (6858e21).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/utils/common.rs	97.91%	2 Missing ⚠️
src/handlers/dns/mod.rs	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #361      +/-   ##
==========================================
+ Coverage   59.08%   59.97%   +0.88%     
==========================================
  Files          25       25              
  Lines        4646     4734      +88     
==========================================
+ Hits         2745     2839      +94     
+ Misses       1901     1895       -6

Flag	Coverage Δ
rust	`59.97% <97.16%> (+0.88%)`	⬆️

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.

emma31-dev · 2026-10-20T11:56:59Z

since you are already working on this i'll love you to address issue #360 in this same pr

vadim-anfv · 2026-10-29T14:58:17Z

since you are already working on this i'll love you to address issue #360 in this same pr

#360 is a separate fix: wrong-network recipient addresses, not transaction weight. It would land more easily on its own, and I am happy to review it there.

emma31-dev · 2026-10-30T14:39:01Z

@vadim-anfv I have now opened it at #370 . it was quite tricky to pull off so take a look.

- `create_tx`, `create_sp_tx` and `create_dns_tx` each decoded `--add_data`/`--add_string` into a `PushBytesBuf` in the handler, with the same block. Parse both arguments with clap value parsers. - Malformed `--add_data` base64 is now rejected at argument parsing (exit 2) before a wallet is loaded, rather than when building (exit 1).

vadim-anfv

ACK aea4bcc

A few comments inline, none blocking.

vadim-anfv · 2026-11-02T11:43:31Z


-    /// Optionally create an OP_RETURN output containing given String in utf8 encoding (max 80 bytes)
+    /// Optionally create an OP_RETURN output containing given String in utf8 encoding. The transaction must stay within Bitcoin Core's 100,000 vB standard size limit.
+


nit: extra empty line

vadim-anfv · 2026-11-02T11:48:38Z


        let raw_tx = psbt.extract_tx()?;

+        check_standard_weight(raw_tx.weight())?;


create_sp_tx is the only one of the four with an exact weight: the transaction is already signed by the time this runs, so raw_tx.weight() is a fact rather than a bound. NonStandardTxWeight says "can weigh up to {weight} ... once signed", which is wrong on both counts here. Worth either a second variant or wording that holds for both paths.

Thank you for pointing this out. I have moved the check up before signing (similar to others) and used max_signed_weight. So the message stays same.

vadim-anfv · 2026-11-02T11:55:30Z

+/// Parse an `OP_RETURN` payload given as base64.
+pub(crate) fn parse_op_return_base64(s: &str) -> Result<PushBytesBuf, Error> {
+    Ok(PushBytesBuf::try_from(BASE64_STANDARD.decode(s)?)?)
+}


nit: needs a blank line

create_tx` could build transactions over Bitcoin Core's 400,000 wu `MAX_STANDARD_TX_WEIGHT`, which no default node relays. The `--add_data` and `--add_string` docs claimed an 80 byte limit that was never enforced and is no longer Core's default. These changes: - Enforce 400k WU limit across `create_tx`, `create_dns_tx`, `bump_fee`, and `create_sp_tx`. - Estimate PSBT weight using `max_weight_to_satisfy` per input for unsigned flows (`create_tx`, `create_dns_tx`, `bump_fee`, `create_sp_tx`). - Update `--add_data` and `--add_string` docs to remove references to the deprecated 80-byte data limit. Bitcoin Core v30+ evaluates weight limits prior to data carrier limits.

tvpeter · 2026-11-04T15:50:10Z

A few comments inline, none blocking.

Thank you for the review. I have addressed all the comments and updated the PR description to capture the change in the create_sp_tx too.

vadim-anfv

ACK 6858e21

Thanks, nits addressed. Checking create_sp_tx before signing also lines it up with create_tx and bump_fee.

Musab1258 · 2026-11-05T05:56:26Z

tACK 6858e21

I ran cargo test --all-features with all tests passing. I also ran cargo fmt --check, clippy --all-targets -D warnings and cargo check --no-default-features, all clean.

I reproduced the 99,994-byte --add_string through create_tx on the master branch, which which returns a PSBT with exit 0 there and fails with the new error on this PR.

I also did per-commit testing and cargo check --all-features. test_create_tx_rejects_malformed_input passes at both commits.

I tested estimated and signed weight units (wu) for tr, wpkh, sh(wpkh) and each of them overshoots by 3 wu while having three inputs each. pkh with 3 inputs overshoots by 17 wu. All tested estimates are upper bounds on the corresponding signed transactions.

vadim-anfv · 2026-11-05T09:44:43Z

@Musab1258 great, thanks for testing!

I ran cargo test --all-features with all tests passing. I also ran cargo fmt --check, clippy --all-targets -D warnings and cargo check --no-default-features, all clean.

You can use just pre-push for that.

bdk-cli/Justfile

Lines 28 to 34 in 4dbc3e8

    
           pre-push: 
        
               cargo test --features default 
        
               cargo test --no-default-features 
        
               cargo test --all-features -- --test-threads=2 
        
               cargo clippy --no-default-features --all-targets -- -D warnings 
        
               cargo clippy --all-features --all-targets -- -D warnings 
        
               cargo fmt --all -- --check

I think we should add it to the PR template too.

github-project-automation Bot added this to BDK-CLI Oct 19, 2026

github-project-automation Bot moved this to Todo in BDK-CLI Oct 19, 2026

tvpeter changed the title ~~fix: enforce Bitcoin Core's standard tx weight limit when building transactions~~ fix: enforce Core's std tx weight limit when building transactions Oct 19, 2026

tvpeter self-assigned this Oct 19, 2026

tvpeter added this to the CLI 4.1.0 milestone Oct 19, 2026

tvpeter force-pushed the fix/std-tx-weight branch from 70efe32 to 8e88d47 Compare October 27, 2026 09:34

tvpeter force-pushed the fix/std-tx-weight branch from 8e88d47 to aea4bcc Compare November 2, 2026 11:13

vadim-anfv approved these changes Nov 2, 2026

View reviewed changes

tvpeter force-pushed the fix/std-tx-weight branch from aea4bcc to 6858e21 Compare November 4, 2026 15:41

vadim-anfv approved these changes Nov 4, 2026

View reviewed changes

vadim-anfv merged commit 6579837 into bitcoindevkit:master Nov 5, 2026
10 checks passed

github-project-automation Bot moved this from Todo to Done in BDK-CLI Nov 5, 2026


		/// Optionally create an OP_RETURN output containing given String in utf8 encoding (max 80 bytes)
		/// Optionally create an OP_RETURN output containing given String in utf8 encoding. The transaction must stay within Bitcoin Core's 100,000 vB standard size limit.


		let raw_tx = psbt.extract_tx()?;

		check_standard_weight(raw_tx.weight())?;

Conversation

tvpeter commented Oct 19, 2026 • edited by vadim-anfv 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:

Bugfixes:

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

emma31-dev commented Oct 20, 2026

Uh oh!

vadim-anfv commented Oct 29, 2026

Uh oh!

emma31-dev commented Oct 30, 2026

Uh oh!

vadim-anfv left a comment

Choose a reason for hiding this comment

Uh oh!

vadim-anfv Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

vadim-anfv Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

tvpeter Nov 4, 2026

Choose a reason for hiding this comment

Uh oh!

vadim-anfv Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

tvpeter commented Nov 4, 2026

Uh oh!

vadim-anfv left a comment

Choose a reason for hiding this comment

Uh oh!

Musab1258 commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vadim-anfv commented Nov 5, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tvpeter commented Oct 19, 2026 •

edited by vadim-anfv

Loading

codecov Bot commented Oct 19, 2026 •

edited

Loading

Musab1258 commented Nov 5, 2026 •

edited

Loading