Repository navigation
policy: count bare data envelopes (OP_2DROP/pushnum runs) as datacarrier - #319
kwsantiago wants to merge 4 commits into
Conversation
04e6d16 to
3b776a4
Compare
kwsantiago
changed the title
|
tACK 3b776a4 Tested with this transaction (has been mined afterward to be able to share it publicly) on my own signet. |
|
I ran the code changes through a CodeRabbit review, and no issues came up. |
jasonsopko
left a comment
jasonsopko
left a comment
There was a problem hiding this comment.
Tested ACK 3b776a4
Merged onto 29.x-knots at 8c85b15 (v29.4.1) without conflict, built with gcc 13.3 on Ubuntu 24.04. script_tests pass. mempool_datacarrier.py, mempool_accept.py, mempool_packages.py and p2p_segwit.py pass. With a wallet build, feature_taproot.py and wallet_taproot.py pass, so the stricter run counting rejects none of the tapscript shapes that suite sends as standard.
Together with #389: the two touch different code (this one DatacarrierBytes, #389 the output-script tells in policy) and conflict only in mempool_datacarrier.py, where each adds a helper and a run_test call. Keeping both sides, the merged tree builds and both test sets pass, so they compose in either order.
On the counting: a run is charged from its first push to the drop, so a run where an earlier push survives the drop (<pubkey> <tag> OP_DROP OP_CHECKSIG) now counts the key as data too. At the default -datacarriersize of 83 (42 on a node that carried the pre-29.2 value forward in its config) that only matters for a run longer than the limit, and I found no multi-push run ending in a drop at any size: over the last 100 mainnet blocks, 967852 to 967951 (16,546 transactions, 2,683 tapscript, P2WSH and P2SH spend scripts) not one had a run of two or more pushes ending in OP_DROP or OP_2DROP. The shapes that do are the envelopes this PR is after.
3b776a4 to
e133301
Compare
|
Rebased onto the tip of Sorry for the approval reset. The reason for moving it was #422, which builds on this and would otherwise have conflicted here. |
jasonsopko
left a comment
jasonsopko
left a comment
There was a problem hiding this comment.
re-ACK e133301
Range-diff of the old head 3b776a4 against this one shows both commits identical. The diff of the two diffs differs only in the script_tests.cpp hunk offset, which moved from 1650 to 1525 with the older base. The base is 2d00c4c, the datacarrier_opnet-29+knots tip, as stated, and #422 stacks on this head without conflict.
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.
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.
|
Repro, e133301 merged onto The same seed passes on 29.x-knots without these two commits. Under this rule a 30 kB leaf of no-op pushes is a data envelope by policy, so I think the test should say so rather than the rule bend for it: - add_spender(spenders, "tapscript/no10000limit", leaf="t19", **SINGLE_SIG, **common)
+ add_spender(spenders, "tapscript/no10000limit", leaf="t19", standard=False, **SINGLE_SIG, **common)The block path still checks that a script over 10000 bytes is valid. With that line the seed above passes here. |
712eac8 to
6507ed3
Compare
|
Applied, thanks. Reproduced at about one run in six; 20 consecutive seeds pass now. |
jasonsopko
left a comment
jasonsopko
left a comment
There was a problem hiding this comment.
re-ACK 6507ed3
Since e133301 the only change is 6507ed3: standard=False on the tapscript/no10000limit spender, with a comment saying why. Merged onto 29.x-knots, seed 4954157208406648058 fails 4 of 4 times without that line, always with txn-datacarrier-exceeded, and passes with it. Over random seeds the old line failed 2 of 12, so about one run in six as you found, not one in three as I wrote. With the line, 24 random seeds pass. Unit and functional suites pass.
4 participants
DatacarrierBytesrecognizes inscription data in two forms:OP_FALSE OP_IF ... OP_ENDIFenvelopes and<data> OP_DROP. It does not matchOP_2DROP, so a bare envelope of the formpush <marker> <data>... OP_2DROP...OP_DROPcounts as zero datacarrier bytes and evades the size limit.This is the envelope shape ordinals/ord#4545 adopts to remain valid under BIP-110, which disables
OP_IF/OP_NOTIFin tapscript.Changes:
OP_DROP/OP_2DROP, anchoringdata_beganat the start of the run so the whole run is counted on the first drop rather than just the last push.OP_1NEGATE,OP_1..OP_16) as run members. ord's parser accepts pushnums as payload, so a pushnum interleaved between data pushes must not strand the earlier push from the count. This matches the counting to exactly the grammar ord's envelope parser accepts (pushes, pushnums,OP_DROP,OP_2DROP).This makes counting slightly stricter for any push-run ending in a drop, not only the new envelope. Standard scripts that use
OP_DROPdrop a stack item preceded by a non-push opcode (e.g.<locktime> OP_CHECKLOCKTIMEVERIFY OP_DROP), which breaks the run and is not counted.Testing:
script_DataCarrierBytes): a<data> <data> OP_2DROPrun, and a pushnum-interleaved<data> OP_7 <data> OP_2DROPrun.mempool_datacarrier.py): builds P2TR script-path spends with bare envelope tapscripts and assertstxn-datacarrier-exceededunder default policy, including a pushnum-interleaved envelope, mirroring the existing OPNet case. Reverting the counting change alone makes the tests fail (the tx is accepted).