Repository navigation
Fix the two CI failures in the tapscript commits - #2
Closed
chrisguida wants to merge 5 commits into
Closed
chrisguida wants to merge 5 commits into
chrisguida wants to merge 5 commits into
Conversation
… bytes The rule only recognised a key in a 33 or 65 byte push, so the same payload written as tapscript went free: a 1-of-15 costs 132 datacarrier bytes under P2WSH and nothing at all under a taproot leaf, for 32 bytes of publishable space per key rather than 31. A 32 byte push is not a key on sight, since nothing separates an x-only key from a hash until something spends it. Count it where a checksig-family opcode claims it instead, which leaves the preimage of a hashlock alone. OP_CHECKSIGADD is how tapscript writes an m-of-n, so credit its canonical shape as OP_CHECKMULTISIG's is already credited: a run of <pubkey> OP_CHECKSIG (<pubkey> OP_CHECKSIGADD)* <k> OP_NUMEQUAL credits k, whatever size its keys are. Without it every honest tapscript multisig past eleven keys would be charged for the keys it does not spend, which is the opposite of the point: a 20-of-20 would cost 297 bytes. The OP_CHECKSIG that opens the run has credited one of the k already, so only the remaining k-1 are added there. Padding the run, reordering it or naming a k it cannot satisfy forfeits the credit rather than earning one, so working around the rule raises the charge instead of lowering it. An opcode that leaves the stack as it found it does not hide the key beneath it from the opcode that spends it, so it does not buy the count off either.
Checks the taproot leaf against the P2WSH script that says the same thing, shape by shape, so the two are charged alike rather than only plausibly, including at the tolerance boundary: 1-of-11 is free and 1-of-12, 1-of-13 and 5-of-16 pay the same either way. Covers the hashlock shapes a 32 byte push must not be mistaken for a key in, the padded and inflated runs that forfeit the credit, the OP_CHECKSIG that opens a run crediting one of the k and not two, a k above 16 written as a minimal push, and the key stranded on the stack that neither rule reaches.
kwsantiago
force-pushed
the
policy-unproven-pubkey-datacarrier
branch
from
October 14, 2026 19:41
177e5ca to
eea2467
Compare
Collaborator
|
Thanks, squashed into the commits that introduced them: eea2467. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Assembling the 29.4.3rc1 candidate with bitcoinknots#422 at 177e5ca turned up two CI failures in the new tapscript commits, both one-liners:
src/script/script.cpp:389: MSVC rejectssize_t{opcode}as a narrowing brace-init (C2397), so both Win64 native jobs fail to build. Replaced withstatic_cast<size_t>(opcode).src/test/script_tests.cpp:1667:MultiAScriptfills key bytes withi + 1, which narrows 256 to 0 for n above 255 and fails the UBSan integer job inscript_DataCarrierBytes_tapscript_unproven_pubkeys. Cast tounsigned char; the keys only need to differ.Fork CI run of the assembled candidate with these two commits on top: https://github.com/chrisguida/bitcoin/actions/runs/38609164952 (in progress at the time of writing; the run without them: https://github.com/chrisguida/bitcoin/actions/runs/38509266693, 15 of 18 jobs green, the 3 failures being exactly these).