Repository navigation
consensus: OP_TUCK obeys the shared stack limit - #1075
Conversation
0eecd69 dropped the after-opcode stack check because every growth went through `push`. OP_TUCK did not: it grew the stack with `insert`, so a TUCK from 1,000 items to 1,001 passed where Core fails with STACK_SIZE. TUCK now pushes the copy and swaps it into place. A witness v0 stack can start above 1,000 items; Core has no initial check there. RIPEMD160 and HASH256 pop then push, so that push can fail. They return the error again instead of panicking on `expect`. Corpus rows pin both, checked against Core v31.1 VerifyScript. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0af76d4 to
3ec3d73
Compare
There was a problem hiding this comment.
Summary
OP_TUCK now grows through push, which applies the shared 1,000-item cap (stack plus altstack), then swaps the copy into Core's (x1 x2 -- x2 x1 x2) position. A tuck from 999 items passes. A tuck from 1,000 fails before the swap. With two items already required, the swap indexes are in range.
OP_RIPEMD160 and OP_HASH256 pop, hash, and push(...)?. A witness v0 stack of 1,001 items pops to 1,000 and the push fails with stack size instead of panicking. OP_SHA1, OP_SHA256, and OP_HASH160 already used ?. No push(...).expect remains in eval_script.
Other growth goes through push with the current altstack length (DUP, IFDUP, 2DUP, 3DUP, OVER, 2OVER, PICK, ROLL, SIZE, DEPTH, numeric ops, CHECKSIG, CHECKSIGADD, CHECKMULTISIG). In-place ops do not change the combined length. FROMALTSTACK pops alt first, then push checks the same total.
Tapscript still rejects an initial stack above 1,000 before the loop. tapscript_has_op_success still returns before that check. Witness v0 still has no initial count check and still caps each initial element at 520. Unexecuted conditionals and disabled opcodes return before 0x7d.
A witness v0 stack that starts above 1,000 can still pass a non-growing opcode here while Core fails that opcode. It cannot become a valid spend: cleanstack needs one true item, and 201 counted ops can drop at most 603 items. From 1,001 that leaves at least 398. Legacy and tapscript cannot enter eval_script above the cap once every growth uses push.
script_corpus_rows_match_our_interpreter walks every object in script_tests.json. The six new ids are present, the P2WSH program hashes match scripts a6 and aa, and the 999 / 1,000 tuck counts are the accept and reject boundary. Deleting the tuck check makes both 1,001 rows succeed. Restoring the two expects panics the corpus test.
changelog.d/tuck-stack-limit.md matches 0eecd692 (2026-10-17), which is after tag v0.8.0 and is the commit that dropped the post-opcode check and switched those two hashes to expect.
One follow-up, not a merge block: every new tuck script uses only OP_1, so a swap of the wrong pair still passes the limit rows. A short legacy OP_1 OP_2 OP_TUCK with equality checks would pin the order. outcome_matches also treats any non-OK as a hit, so the hash rows pin the panic because expect aborts the test; a later cleanstack error would also satisfy ERR.
Issue counts by severity
- bugs: 0
- suggestions: 1
- nits: 1
|
@rearden-grok Thanks. On the two follow-ups:
No new commits. |
Summary
0eecd69 dropped the after-opcode stack check in
eval_scriptbecause "every growth goes throughpush". Two arms didn't hold up:stack size, as in Core (interpreter.cpp:stack.size() + altstack.size() > MAX_STACK_SIZEafter every opcode). On master rbitcoin accepts it in legacy, P2SH, and tapscript.expectpanics instead of rejecting.Not released: 0eecd69 landed after v0.8.0. The
changelog.d/fragment says so.Evidence
Same blocks submitted to Core 31.1 and rbitcoin on regtest:
OP_TUCK, scriptSig 999 ×OP_1OP_TUCK, scriptSig 1,000 ×OP_1Stack size limit exceededOP_RIPEMD160, 1,001 empty witness itemsStack size limit exceededtip-acceptpanics:ripemd160 fitsOP_HASH256, 1,001 empty witness itemsStack size limit exceededexpecttuck-1000-legacy,tuck-1001-legacy,tuck-1000-tapscript,tuck-1001-tapscript,initial-1001-ripemd160-v0,initial-1001-hash256-v0.script-oracle(Core v31.1VerifyScript) agrees with every row.script_corpus_rows_match_our_interpreter:tuck-1001-legacy: expect ERR got Ok(()). With only the TUCK change it panics atripemd160 fits.cargo test --workspace, andci-os-smoke.shpass locally.cargo denywasn't available locally; no dependency changed.Merge Danger
Door: two-way
Blast Radius: consensus
Only scripts whose stack would pass 1,000 items change outcome: TUCK goes from accept to reject, and RIPEMD160 / HASH256 go from panic to reject. Both now match Core. Every other script behaves as before.
🤖 Generated with Claude Code