From: merge-script Date: Thu, 5 Nov 2026 10:15:00 +0000 (+0400) Subject: Merge bitcoindevkit/bdk-cli#361: fix: enforce Core's std tx weight limit when buildin... X-Git-Url: http://internal-gitweb-vhost/-script/src/example_cli/scripts/database/taproot/struct.LeafNode.html?a=commitdiff_plain;p=bdk-cli Merge bitcoindevkit/bdk-cli#361: fix: enforce Core's std tx weight limit when building transactions 6858e2188e98c23ec5fad58c9c247b5154b46a5a fix(create_tx,bump_fee): reject txs over the standard weight limit (Vihiga Tyonum) 22304652f99e33f22d97a75b549be59c8bbb8283 refactor(op_return): parse OP_RETURN data at the clap boundary (Vihiga Tyonum) Pull request description: ### 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 #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 #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: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk-cli/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing #### Bugfixes: * [x] I've added tests to reproduce the issue which are now passing * [x] I'm linking the issue being fixed by this PR ACKs for top commit: Musab1258: tACK 6858e2188e98c23ec5fad58c9c247b5154b46a5a vadim-anfv: ACK 6858e2188e98c23ec5fad58c9c247b5154b46a5a Tree-SHA512: 5824e571109e346e02029dc94ecfad368862efb04def3ce52268c307613dc9155b2ef081523df97d791cb5d543661c9d26d10bc442a960b8fd2864f0eca52ce8 --- 6579837b35f1e47063f274d32c3ea7bd47ef5429