Skip to content

policy: count pubkeys no signature can prove as datacarrier bytes - #422

Open
kwsantiago wants to merge 9 commits into
bitcoinknots:29.x-knotsfrom
privkeyio:policy-unproven-pubkey-datacarrier
Open

kwsantiago wants to merge 9 commits into
bitcoinknots:29.x-knotsfrom
privkeyio:policy-unproven-pubkey-datacarrier

Conversation

@kwsantiago

@kwsantiago kwsantiago commented Sep 16, 2026 •

Copy link
Copy Markdown

DatacarrierBytes counts a multisig script's pubkeys as zero bytes, so a file encoded into fake but valid pubkeys inside 1-of-15 P2WSH multisig relays entirely uncounted, and -datacarriercost multiplies it by zero.

A key in an m-of-n that no signature is offered for authorizes nothing, so its bytes are payload rather than a spending condition. This charges 33 bytes for each such key beyond ten per script, against the existing -datacarriersize budget. Ten is set so no ordinary multisig is charged anything at all, however wide, because the budget is per transaction and any per-input charge accumulates across inputs and rejects real spends. Two things keep it from being a pattern match: where the witness carries the spend, the signatures it actually supplies cap what the script may claim to prove, so naming signatures in a branch that never runs earns nothing; and only a canonical <m> <pubkey>*n <n> OP_CHECKMULTISIG earns the m credit, so padding or reordering the keys raises the charge instead of lowering it.

Replaying the witness scripts of a real published transaction on regtest: relays on v29.4.1, txn-datacarrier-exceeded here, at single input granularity. Publishing the same file at the widest shape that still relays costs 3.4x the vsize and 7x the dust.

Unaffected: 2-of-3, 3-of-5, 2-of-5, 2-of-6, 3-of-9, 11-of-15, 14-of-17 and their P2SH forms, at any input count. The boundary is eleven keys behind one signature, which is free, against twelve, which pays; what is charged is charged per script and adds up over a transaction, and both are pinned in the functional test. The adapted cost is small: measured over two inputs, the 1-of-15 this rejects carries 2.01 payload bytes per vB and the widest shape that still relays, 1-of-11, carries 1.71, so a publisher pays about 15% more rather than being stopped. This is an accounting fix, not a deterrent. A P2SH spend keeps its signatures in the scriptSig where this cannot see them, but MAX_P2SH_SIGOPS caps that shape at nine keys and non-witness bytes cost four weight each, so it measures 0.53 payload bytes per vB against 0.48 for simply complying.

Based on #319, whose four commits are included here; only the last two are new. The helper mirrors that PR's push run counting, so keys balanced by a drop are counted once rather than by both rules.

Testing: script_DataCarrierBytes_unproven_pubkeys covers the counting, mempool_datacarrier.py covers relay behavior, and DatacarrierBytes is added to the script fuzz target, which had no coverage of it.

@LionThunderFingers

Copy link
Copy Markdown

Had a look at this and I think taproot slips through... IsPubkeyPush only matches 33 and 65 byte pushes, but tapscript keys are 32 bytes (x-only) so they never get counted. There's also no OP_CHECKSIGADD handling, which is how tapscript does multisig instead of OP_CHECKMULTISIG.

Tested it by calling DatacarrierBytes directly on two scripts that are basically the same size:

P2WSH 1-of-15, 33-byte keys : 513 bytes -> charged 396
tapscript 1-of-15, 32-byte : 512 bytes -> charged 0

Tapscripts do reach the function (GetScriptForTransactionInput pops the control block and passes the script through), and the tapscript standardness check skips the script itself before the 80 byte stack item limit (SpanPopBack(stack); // Ignore script), so nothing else catches it either.

I had a go at a patch... it counts 32-byte pushes as keys when they're in tapscript key position, and credits the ' OP_CHECKSIG ( OP_CHECKSIGADD)* OP_NUMEQUAL' shape so real tapscript multisig doesn't get charged. After it the tapscript case charges 396, same as the P2WSH one, and 2-of-3 / 3-of-5 still charge 0 on both types. Full test_bitcoin passes (662 cases).

Happy to open it as a PR against your branch or just paste the diff here, whatever's easiest. Still learning this codebase so say if I've got the approach wrong.

@LionThunderFingers

Copy link
Copy Markdown

Here's the tested patch so it's easier to look at than a pasted diff

https://github.com/LionThunderFingers/bitcoin/tree/policy-count-tapscript-unproven-pubkeys

Two commits on top of your 0a32402, should apply cleanly. Let me know if it's useful.

jasonsopko added a commit to jasonsopko/bitcoinknots that referenced this pull request Sep 27, 2026
OLGA hides a payload in P2WSH outputs at the dust threshold, and the
detector keys on its "stamp:" magic. The same layout with "ACME" has put
over 100 transactions on chain since 957497, and changing the magic is
free.

Judge the bytes instead. A hash or x-only key is uniformly random, so
one byte value eight times in a program, a run of six, or a taproot key
off the curve marks it as data, along with its same-type same-value
siblings. Three or more distinct P2WSH outputs sharing one value of 546
sat or less are data on their own: encoders pay every chunk the same,
batch payouts on chain start at 600 sat, Lightning anchors are a pair
and HTLC amounts differ. Small taproot runs are left to the curve check,
since the ones on chain are ordinals postage with no data in the
outputs. The all-zero burn hash is left alone. The detector sits behind
-rejectfakeoutputs, default on and reset by -corepolicy.

Over the 3,018,541 transactions since the fork it newly rejects 45: 14
funding transactions of the fake-key and JXL encoders, 14 swap memos
continued as ASCII in a P2WPKH hash, 11 ACME-style P2WSH payloads and 6
taproot outputs tagged DECODE WITNESS.

Checked on Ubuntu 24.04 against 29.4.2: a gcc 13.3 -Werror build with
wallet and ZMQ has no warnings, and all 147 ctest suites and the full
functional suite pass, alone and merged with bitcoinknots#400, bitcoinknots#319 and bitcoinknots#422. The
clang 18 ASan/UBSan build passes the unit suite and the policy
functional tests.
jasonsopko added a commit to jasonsopko/bitcoinknots that referenced this pull request Sep 27, 2026
The detector counts the OP_FALSE OP_IF envelope but not its mirror, OP_1
OP_NOTIF, or any other branch constants make unreachable. 23e8f946... at
969961 carried a 37 KB JPEG XL image that way, and nothing on the fence
saw it.

Follow the script as far as constants decide it, through stack moves,
arithmetic, comparisons, no-ops and the altstack, and count every
conditional part that can never run, including the dead part of an
OP_ELSE. Anything read from the witness or a signature check is unknown
and never counted, which is how spendable scripts branch. The rule sits
behind -rejectdeadbranches, default on and reset by -corepolicy; with it
off the detector is the old one byte for byte.

Checked on Ubuntu 24.04 against 29.4.2: a gcc 13.3 -Werror build with
wallet and ZMQ has no warnings, and all 147 ctest suites and the full
functional suite pass, alone and merged with bitcoinknots#389, bitcoinknots#319 and bitcoinknots#422. The
clang 18 ASan/UBSan build passes the unit suite and the policy
functional tests. A fuzz target checked that with the switch off the
result matches the original function, and that every part marked dead
leaves the interpreter's verdict unchanged when an OP_RETURN is put in
it, over 4.9 million inputs in 45 minutes. Over all 3,018,541
transactions since the fork it counts one: 23e8f946..., at 37,854 bytes.
@jasonsopko

Copy link
Copy Markdown

Concept ACK. I rewrote the 1-of-15 script three ways to see if the keys would stop being counted: an OP_NOP before OP_CHECKMULTISIG, one 520-byte "key" in place of a real one, and each fake key as OP_0 <key> OP_CHECKSIG OP_DROP. All three were still counted. Tapscript multi_a counts as 0, which LionThunderFingers' patch covers.

I ran 0a32402, merged onto 29.x-knots, over every transaction since the fork, 961640 to 974259 (3,018,541 transactions). It counts at least 6,204 bytes of data in each of the 14 fake-key transactions. It also counts data in 947 others, and at least 507 of those go over the default -datacarriersize of 83 and would be rejected. Most are ordinary multisig spends with several inputs: 2-of-5 (139), 2-of-6 (125), 3-of-7, 3-of-8 and 3-of-9 (76), and 8-of-15 and 11-of-15 federations (66). The two spare keys are allowed per script, but the bytes are added up across inputs, so three 2-of-5 inputs already go over.

Could the allowance be per script only, and higher? At 8 spare keys per script, the 14 fake-key transactions are still rejected and the other hits drop from 947 to 3. Those 3 are vaults with a recovery branch, where every key in the branch not taken is counted: 97f7d78b0b9ad4d47e150659c3091d905c3565fe7e6fd8977e4326b2b7af2270 (11-of-15 with a timelocked 2-of-3, 35 inputs) and 9cb369b0cc44f250367f49be004f99e0710f839ce7e33c5cca8b5960a2c9d065 / 3befc65d7b154f94dab4fd65f0e8251a3583a7880a200cc8994ba6a6f6aa52f1 (2-of-3 decaying to 3-of-6). Counting only the branch the witness takes would clear those too.

nit: if #400 lands, the envelope skip in UnprovenPubkeyBytes should follow -rejectdeadbranches, or keys in a dead OP_NOTIF branch are counted twice.

@MarcanoFilms

Copy link
Copy Markdown

Concept ACK. This is the right layer — folding it into DatacarrierBytes generalises the m == 1 shape I was matching in #435, and it keeps the 2-of-15 migration that a pure shape test loses.

I ran the counting against my own BLAKE2b-fork node (independent of the 29.x-knots replay above): it flags the same bpub reveals, including the witnessScript of be667d76425d48df0ea891176fd8ef0a3a4329775cecc5afaf4558500c9e84e9, and I see the same class of false positives on genuine wide-multisig federations.

On the per-script vs per-tx allowance: jasonsopko's 8-spare-keys-per-script result (947 → 3 hits, the 3 being recovery-branch vaults) looks like the right tradeoff to me — the accumulation across inputs is what catches real wallets, not the shape itself. Happy to help pin that boundary.

Given this subsumes #435, I'll plan to close mine in favour of this once the allowance lands. One thing that may be worth carrying over: my functional test drives the rejection through mempool acceptance (funds a real P2WSH / P2SH-P2WSH output, then submits the reveal), which complements the unit-level DatacarrierBytesStr cases here. I can adapt it to this PR if useful. The tapscript multi_a gap (LionThunderFingers' patch) is the remaining hole and worth landing together.

@kwsantiago
kwsantiago force-pushed the policy-unproven-pubkey-datacarrier branch 2 times, most recently from 845a39a to 95ed47c Compare September 30, 2026 21:50
@jasonsopko

Copy link
Copy Markdown

Updated numbers on 95ed47c (the policy commits are unchanged since 0a32402), merged onto 29.x-knots, over every transaction from 961640 to 975264 (3,278,236):

  • 92 bpub reveals, every multisig input a 1-of-N at a dust value with at least one N of 8 or more: 15,746 inputs, 6.81 MB of file data. As posted, all 92 are rejected.
  • It also rejects 532 other transactions that are standard today: 426 plain multisig spends, mostly 2-of-5 (142) and 2-of-6 (128) with two or more inputs, and 106 wallets with a recovery branch, 45 of them an 11-of-15 with a timelocked 2-of-3 spent with 11 signatures.

At 10 unproven keys per script instead of 2, 90 of the 92 reveals are rejected; the two it misses are single-input 1-of-8 and 1-of-9 reveals, 465 bytes of data between them. One other transaction is rejected: 97f7d78b, the 11-of-15 spent through its timelocked 2-of-3 branch, which leaves 16 keys unsigned. At 8 the two vault spends at 966515 are rejected as well, and no more reveals.

None of the reveals so far use tapscript, where multi_a still counts 0 here; LionThunderFingers' patch covers that.

A patch for 10 with the unit and functional tests updated is in the gist, with the harness. Unit and functional suites pass with it.

I also measured the cost, since DatacarrierBytes runs before the witness size limits. Relaying 400 different 380 kB transactions per script shape from one inbound peer to a regtest node, #319 and #422 together add 0.5 to 1.3 ms per transaction over 29.x-knots (2.6 to 10.4 ms), and RPC latency does not change.

https://gist.github.com/jasonsopko/106c74c2df42963fe99801d8f2c4303f

@jasonsopko

Copy link
Copy Markdown

One correction: of the 45 spends of the 11-of-15 with the timelocked 2-of-3, 44 were spent with 11 signatures. The 45th is 97f7d78b, the branch spend named in the same comment. Nothing else changes.

@dev1ns

This comment was marked as abuse.

@kwsantiago
kwsantiago force-pushed the policy-unproven-pubkey-datacarrier branch from 95ed47c to 7f49d85 Compare October 4, 2026 23:05
@kwsantiago

Copy link
Copy Markdown
Author

Pushed at 10. The largest free shape is now 1-of-11, measured at 1.71 payload bytes/vB against 2.01 for the 1-of-15 it rejects, so the adapted cost is about 15%; at 2 it forces 1-of-3 at 0.55 but rejects your 532. Documented as accounting rather than deterrence.

@kwsantiago
kwsantiago force-pushed the policy-unproven-pubkey-datacarrier branch from 7f49d85 to f2e7e09 Compare October 5, 2026 01:15

@jasonsopko jasonsopko left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ACK f2e7e09, code review and tested.

Since my 2026-10-03 comment the policy code changed in one place: MAX_UNPROVEN_PUBKEYS is 10, with the comment saying why. The unit and functional tests moved to the new boundary (1-of-11 free, 1-of-12 pays 33 bytes, the per-script charge still adds up over inputs), and feature_taproot.py's sigops ratio script 2 dummy push shrank from 220 to 75 bytes so it stays under the data carrier default. I read the diff against 95ed47c and the six commits as they stand.

Tested on this head merged onto 29.x-knots 58398ba: 146 unit tests pass, 291 functional tests pass and 64 are skipped (legacy wallet, USDT, ZMQ, previous releases, routable addresses). Over every transaction from 961640 to 976023 (3,435,048), the rule as written refuses 145 of 147 bpub reveals (12.70 MB of file data); the two it misses are single-input 1-of-8 and 1-of-9 reveals carrying 465 bytes. It refuses one other transaction, 97f7d78b at 963410, an 11-of-15 spent through its timelocked 2-of-3 branch. Since the numbers I posted on 2026-10-03, which ran to 975264, the encoder added 55 reveals, all refused, and nothing else is refused.

Relay cost on this build: 400 different 380 kB hostile transactions per script shape from one inbound peer cost 3.1 to 13.6 ms per transaction, against 2.1 to 10.7 ms on 29.x-knots built the same way; getblockcount median 0.3 ms on the base build and 0.3 to 0.4 ms on this one, peer still connected.

Running on a mainnet node since 2026-10-02 at this allowance (Plumb plumb3 to plumb6).

Still outside this PR, not blocking: tapscript multi_a counts 0 here. LionThunderFingers' two commits from this thread cover it and sit on 0a32402, so they need a rebase; mine onto Plumb's copy of this head is at plumb-node/plumb filter/rejectfakemultisig (f43c9e97dd, f8905c80db) if it saves anyone the work.

@LionThunderFingers

Copy link
Copy Markdown

Updated my tapscript patch for the allowance of 10: https://github.com/LionThunderFingers/bitcoin/tree/policy-count-tapscript-unproven-pubkeys (two commits on f2e7e09).

The code is the same as before... only the test numbers moved.
A 1-of-15 tapscript is now charged 132, the same as the P2WSH one... 1-of-11 is free in both... 1-of-12, 1-of-13 and 5-of-16 also pay the same in both.

The tapscript cases fail without the patch and pass with it.
Unit tests and the functional suite pass... and with the wallet built feature_taproot.py, wallet_taproot.py, rpc_psbt.py and mempool_datacarrier.py pass too.

@booozytheclown

Copy link
Copy Markdown

Concept ACK

@jasonsopko

Copy link
Copy Markdown

Numbers for 717 blocks, 976024 to 976740, on this head.

725 bpub reveals, 193,117 inputs, 83.6 MB of file data, 41.4% of all block weight in the window (145.5M of 351.4M WU, 400 of the 717 blocks). 192,412 of the inputs are still 1-of-15; the encoder has not moved to the 1-of-11 the allowance leaves free. The rule counts data in all 725 (264 to 63,096 bytes each, 25.4 MB in total) and in none of the 6,611 other transactions with a multisig input in the same blocks, 655 of them nested P2SH-P2WSH; the widest real spend there leaves 7 keys unsigned (3-of-10 and 8-of-15). The reveals paid 0.405 BTC, 0.48 sat per byte of file.

A node running this rule at the default -datacarriersize refuses every one of them, so the miners who included them are not running it at that limit.

Survey script and the per-transaction data: https://gist.github.com/jasonsopko/d07b6b8044f2b02a3ebea87711dab9e7

@LionThunderFingers

Copy link
Copy Markdown

Found a hole that gets around the unproven key count (my tapscript patch too)... and had a go at fixing it: https://github.com/LionThunderFingers/bitcoin/tree/fix/unproven-pubkey-verified-signatures (my two tapscript commits plus two new ones, on f2e7e09).

The hole... a 1-of-15 P2WSH with 14 OP_CHECKSIG inside an OP_1 OP_NOTIF ... OP_ENDIF so they never run, then 7 OP_2DROP and the multisig. The witness is [dummy, sig, 14 random 72-byte items]. The checksigs in the script and the signature-sized witness items both count as proven keys... so it looks like all 15 keys are signed for when only 1 is. It gets accepted at about 2.9 payload bytes/vB.

The fix... instead of guessing from the script and witness, it actually checks the signatures. For an input that would go over the allowance with nothing credited (so only wide multisig), the spend gets run with a checker that counts the signatures that really verify, and that's the most it can be credited. It uses the same script flags the mempool does, and a spend that doesn't verify gets no credit. Anything narrower is counted the same as before.

Tests... I added a decoys option to the multisig helper in mempool_datacarrier.py. A padded 1-of-15 is rejected and a padded 11-of-15 still goes through. The 1-of-15 case fails without the fix.
Unit tests and the functional suite pass (with the wallet built: 256 passed, 0 failed, 64 skipped), including feature_taproot.py, wallet_taproot.py and rpc_psbt.py. The policy commit builds and passes on its own too.

Cost is about +0.6-0.7 ms per wide multisig tx on testmempoolaccept... nothing for 2-of-3 since it never does the extra run.

For the release with #357... the checker overrides need the extra SighashRules argument and the mempool needs to pass the unified sighash flag, otherwise a unified signature fails the count. I've got that version tested on rc1 if it helps.

Non-witness P2SH isn't changed since the signatures are in the scriptSig... but from reading the code MAX_P2SH_SIGOPS counts the dead checksigs too, so padding there can't get more unproven keys than a plain multisig.

@kwsantiago
kwsantiago force-pushed the policy-unproven-pubkey-datacarrier branch from d2fd37e to 177e5ca Compare October 12, 2026 01:20
@kwsantiago

Copy link
Copy Markdown
Author

Took the tapscript commits plus a k≥128 fix. Skipped verification: one signature checked repeatedly passes, and uncached reverification costs 282x.

@djkazic

djkazic commented Oct 14, 2026

Copy link
Copy Markdown

Concept ACK, rug the spammers!

@chrisguida

Copy link
Copy Markdown

Assembling the 29.4.3rc1 candidate with this branch at 177e5ca hit two CI failures in the tapscript commits: MSVC rejects size_t{opcode} at script.cpp:389 as a narrowing brace-init (both Win64 native jobs), and the UBSan integer job flags MultiAScript writing 256 into an unsigned char for n above 255. Two one-line fixes as a PR against this branch: privkeyio#2. Fork CI of the assembled candidate without them: https://github.com/chrisguida/bitcoin/actions/runs/38509266693 (15 of 18 green, the 3 failures being these); with them: https://github.com/chrisguida/bitcoin/actions/runs/38609164952 (running).

LionThunderFingers and others added 3 commits October 14, 2026 15:25
… 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
kwsantiago force-pushed the policy-unproven-pubkey-datacarrier branch from 177e5ca to eea2467 Compare October 14, 2026 19:41
jasonsopko added a commit to jasonsopko/bitcoinknots that referenced this pull request Oct 15, 2026
…rrier' into policy-dead-branch-data

bitcoinknots#422 at eea2467 changes DatacarrierBytes too. This branch builds on it so the 29.4.3 assembly can merge it after k422.
jasonsopko added a commit to jasonsopko/bitcoinknots that referenced this pull request Oct 15, 2026
The detector counts the OP_FALSE OP_IF envelope but not its mirror, OP_1
OP_NOTIF, or any other branch constants make unreachable. 23e8f946... at
969961 carried a 37 KB JPEG XL image that way, and nothing on the fence
saw it.

Follow the script as far as constants decide it, through stack moves,
arithmetic, comparisons, no-ops and the altstack, and count every
conditional part that can never run, including the dead part of an
OP_ELSE. Anything read from the witness or a signature check is unknown
and never counted, which is how spendable scripts branch. The rule sits
behind -rejectdeadbranches, default on and reset by -corepolicy; with it
off the detector is the old one byte for byte.

Checked on Ubuntu 24.04 against 29.4.2: a gcc 13.3 -Werror build with
wallet and ZMQ has no warnings, and all 147 ctest suites and the full
functional suite pass, alone and merged with bitcoinknots#389, bitcoinknots#319 and bitcoinknots#422. The
clang 18 ASan/UBSan build passes the unit suite and the policy
functional tests. A fuzz target checked that with the switch off the
result matches the original function, and that every part marked dead
leaves the interpreter's verdict unchanged when an OP_RETURN is put in
it, over 4.9 million inputs in 45 minutes. Over all 3,018,541
transactions since the fork it counts one: 23e8f946..., at 37,854 bytes.
jasonsopko added a commit to jasonsopko/bitcoinknots that referenced this pull request Oct 15, 2026
OLGA hides a payload in P2WSH outputs at the dust threshold, and the
detector keys on its "stamp:" magic. The same layout with "ACME" has put
over 100 transactions on chain since 957497, and changing the magic is
free.

Judge the bytes instead. A hash or x-only key is uniformly random, so
one byte value eight times in a program, a run of six, or a taproot key
off the curve marks it as data, along with its same-type same-value
siblings. Three or more distinct P2WSH outputs sharing one value of 546
sat or less are data on their own: encoders pay every chunk the same,
batch payouts on chain start at 600 sat, Lightning anchors are a pair
and HTLC amounts differ. Small taproot runs are left to the curve check,
since the ones on chain are ordinals postage with no data in the
outputs. The all-zero burn hash is left alone. The detector sits behind
-rejectfakeoutputs, default on and reset by -corepolicy.

Over the 3,018,541 transactions since the fork it newly rejects 45: 14
funding transactions of the fake-key and JXL encoders, 14 swap memos
continued as ASCII in a P2WPKH hash, 11 ACME-style P2WSH payloads and 6
taproot outputs tagged DECODE WITNESS.

Checked on Ubuntu 24.04 against 29.4.2: a gcc 13.3 -Werror build with
wallet and ZMQ has no warnings, and all 147 ctest suites and the full
functional suite pass, alone and merged with bitcoinknots#400, bitcoinknots#319 and bitcoinknots#422. The
clang 18 ASan/UBSan build passes the unit suite and the policy
functional tests.

@jasonsopko jasonsopko left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ACK eea2467, code review and tested. This covers the three commits after my 10-07 ACK on f2e7e09: cd8907e (tapscript counting), 44ec48c (its tests) and eea2467 (k from 128), with the two CI fixes squashed in. The spec credits my ACK with the tapscript part, so this is what that rests on. The commits do what they say, they charge no real spend on this chain, and the evasion Lion posted on 10-11 is open on this head in more than one shape.

What they do. A tapscript k-of-n written the way wallets write it, <key> CHECKSIG (<key> CHECKSIGADD)* <k> NUMEQUAL, is credited k proven keys, as a CHECKMULTISIG k-of-n is. Any other arrangement is credited only the CHECKSIGs it contains. A 32-byte push counts as a key only where a checksig-family opcode claims it, so hash preimages and hashes stay out of the count. A k above 16 is read as a minimal push up to 999, and the last commit fixes 128 to 255, which carry a zero high byte to keep the sign clear. The charge is 33 bytes per unproven key either way, so a 1-of-15 leaf costs what a P2WSH 1-of-15 costs: 132 bytes over the allowance of ten. I found no way to be overcharged. The one imprecision runs the other way: CountWitnessSignatures skips only the last witness item, so a 64 to 73 byte leaf script, or a control block pushed off the end by an annex, counts as a signature and credits one key too many. The comment at the check covers the annex case; the one above the function says the last item is the script or the control block, which for taproot leaves the script in the count. Worth a line, not a fix.

Measured on this chain. Every taproot script-path spend from 961640 through 977475 whose leaf uses CHECKSIGADD: 3,670 inputs in 1,853 transactions, and the widest leaf names 9 keys. None reaches the allowance, so these commits have charged nothing real and could not have. The P2WSH side, over the 3,753,805 transactions in those blocks: the rule refuses 1,983 bpub reveals, 530,898 inputs carrying 229.8 MB of file data at 31 bytes per unproven key, and one other transaction, 97f7d78b at 963410, the same one as on 10-07. Reveals that stay under the allowance are invisible to this build; on 10-07 a build with the allowance at zero found two.

The hole is wider than posted. Lion's evasion from 10-11, a 1-of-15 with 14 OP_CHECKSIGs in an OP_1 OP_NOTIF branch that never runs and 14 signature-sized junk items in the witness, is accepted on this head. So is a shape with nothing dead in it: 14 live OP_CHECKSIG OP_DROP pairs that take a key and an empty signature from the witness and return false without failing, since NULLFAIL fires only for a non-empty signature, with the 14 junk items dropped up front. Both rest on the same two counts: every CHECKSIG in the script is provable whether or not a signature reaches it, and every 64 to 73 byte witness item is a signature whether or not anything checks it. The junk is payload too. Over two inputs the live shape carries 3.14 payload bytes per vB against 1.71 for the 1-of-11 the rule relays, measured the way the description measures and counting 31 bytes per unproven key, the junk whole and 32 per decoy key, so a publisher who adopts it pays less than one who complies. Not crediting a CHECKSIG in a dead branch would not reach the live shape. Crediting signatures that verify does, which is Lion's branch and the cost Kyle declined, and I see no static rule that another reordering would not dodge. The description calls the rule accounting rather than deterrence, and that is the right label while this stands. The tapscript commits inherit the limit; they do not add to it.

Cost. A leaf of 12,000 x-only keys and CHECKSIGADDs, 408,002 bytes, more than a standard transaction can carry, takes 784 microseconds per DatacarrierBytes call on this machine, and 962 with an OP_NOP between every key and its opcode. Linear in the script, and small next to the 3 to 14 ms per transaction the relay test on 10-07 measured for the P2WSH shapes.

Tests. ctest 139 of 139; functional suite 258 passed, 63 skipped, 0 failed, with the wallet built, so feature_taproot.py, wallet_taproot.py and rpc_psbt.py ran. Scan script, harness and the per-input data: https://gist.github.com/jasonsopko/3cd215c8936b2d298e995099b7985807

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants