#36563 Cover additional script edge cases with the json fixtures
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36563 Cover additional script edge cases with the json fixtures | Ready | 1 | Crickets | 0 + 196 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Adds JSON script test vectors covering subtle script execution edge cases. These include witness v0 versus tapscript size limits, OP_SUCCESS evaluation before script limits, witness v0 bypassing FindAndDelete, tapscript signature validation weight budgeting, and limit exemptions for unknown tapleaf versions. It also adds mapping for two script error strings in the test runner.
Problem: The shared JSON script test fixtures omit coverage for several script boundary conditions and upgrade rules. Implementers of alternative Bitcoin software and downstream consumers cannot use the standard fixtures to verify conformant consensus behavior for these specific cases.
Evidence of importance:
- PR description: 'During rbitcoin's development, we created some additional script test vectors, some of which seem like they would be valuable additions to bitcoin core's script tests.'
- Commit messages document concrete gaps covered: 'assert v0 and tapscript size limits', 'assert OP_SUCCESS before script limits', 'assert witness v0 scripts skip FindAndDelete', 'assert tapscript signature weight budget', and 'assert unknown tapleaf versions skip limits'.
Scope: Almost all changes (+194 lines) are JSON test fixture data in `src/test/data/script_tests.json`, accompanied by two error string mappings in `src/test/script_tests.cpp` to enable the runner to parse the expected failures.
Risk of a mistake: low (tests). Touches only test fixtures and test error string mappings without changing consensus or validation code.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- reviewer: review of the test fixtures from instagibbs or others 2026-10-19
Agreement
Crickets
no substantive comment either way
Thread notes: Added Yudis-bit's Tested ACK on Windows. The comment is excluded from the support list because approvals with no rationale from accounts with no history in the project do not count as support.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| instagibbs (member) | question, maintenance | Whether the author will update the PR based on review or add further test cases later | answered · author confirmed they will address review feedback and submit any future test cases in separate PRs | answered | yes | 2026-10-19: 'are you looking to throw this over the fence or will you update it on review, or are you still going to add stuff later?' Status from: 2026-10-19: reardencode: 'Happy to update based on review.' |
Participants:
- fanquake (member, neutral, 1 statements 2026-10-19 to 2026-10-19): tagged instagibbs and darosior for review
- instagibbs (member, question, 1 statements 2026-10-19 to 2026-10-19): asked whether author plans to update the PR on review
- Yudis-bit (none, support, 1 statements 2026-10-20 to 2026-10-20): Tested ACK on Windows without rationale (account with no project history)
Files
196 lines under test/bench/ci.
- src/test/data/script_tests.json +194/-0
- src/test/script_tests.cpp +2/-0
Card
The short text the ranking pass compares across PRs.
Adds JSON script test fixtures covering script validation edge cases developed during rbitcoin's implementation. It tests witness v0 versus tapscript size limits, OP_SUCCESS evaluation before script limits, omission of FindAndDelete in witness v0, tapscript signature weight budgets, and limit exemptions on unknown tapleaf versions. The new test vectors help alternative implementations and downstream projects verify consensus compatibility across subtle boundary conditions. No open dependencies.
Data
thread read through 2026-10-20 (4 statements), head 60bb41f494
labels: none