arkscript: Tighten custom policy validation - #1144
Open
sputn1ck wants to merge 2 commits into
Open
Conversation
Opaque condition predicates were assumed to only add restrictions, but OP_SUCCESS makes tapscript succeed before the typed inner clause executes. A partial data push can also consume that inner clause as push data. Require predicates to parse as complete fragments, reject OP_SUCCESS, and compile each leaf during direct policy validation so every admission path fails closed.
sputn1ck
marked this pull request as ready for review
August 13, 2026 14:01
Member
Author
|
@claude please review |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
sputn1ck
marked this pull request as draft
August 13, 2026 14:04
There was a problem hiding this comment.
Pull request overview
This PR hardens the lib/arkscript Condition.Predicate escape hatch so opaque predicate bytes cannot (a) short-circuit tapscript execution via OP_SUCCESSx or (b) change parsing boundaries via incomplete pushes that consume the typed inner clause.
Changes:
- Validate
Condition.Predicateas a complete standalone script fragment and reject any tapscriptOP_SUCCESSxopcodes duringCondition.Script()compilation. - Make
ValidatePolicyfail closed by compiling every leaf upfront (and rejecting nil leaves) before applying structural policy invariants. - Add regression tests for
OP_SUCCESSxopcodes, “OP_SUCCESS byte in pushdata”, and incomplete-push boundary smuggling; update predicate bytes in existing tests to remain valid under the new rules.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| lib/arkscript/validate.go | Compiles/leaves upfront (and rejects nil) so admission fails closed on unsafe/invalid predicate fragments. |
| lib/arkscript/validate_test.go | Adds policy-level regression for OP_SUCCESS predicate rejection; updates predicate bytes to avoid incomplete pushes. |
| lib/arkscript/policy_template_test.go | Updates test predicates to use a valid opcode (OP_NOP) under the new predicate completeness rules. |
| lib/arkscript/node.go | Adds predicate safety validation (complete fragment + reject OP_SUCCESSx) to Condition.Script(). |
| lib/arkscript/node_test.go | Adds comprehensive predicate-safety regressions (all OP_SUCCESSx, pushed bytes, incomplete pushes). |
| docs/policy_arkscript_review_guide.md | Documents the predicate safety boundary and the new bypass prevention guarantees. |
| docs/arkscript_spec.md | Updates spec text to describe predicate validation constraints and removes brittle line references. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
sputn1ck
marked this pull request as ready for review
August 13, 2026 14:05
Policy validation compared raw CSV operands even though consensus masks reserved bits and treats the disable bit as a no-op. Restrict typed CSV locks to canonical non-zero block delays so structural validation and script execution enforce the same value.
sputn1ck
changed the title
sputn1ck
marked this pull request as draft
August 13, 2026 14:51
sputn1ck
changed the title
sputn1ck
changed the title
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
sputn1ck
marked this pull request as ready for review
August 13, 2026 18:31
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
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.
What changed
VTXO, vHTLC, and checkpoint builders
encodings
Why
Policy templates should have one interpretation across semantic validation,
script generation, persistence, and recovery. These changes align compiler and
validator behavior so malformed or ambiguous custom templates fail closed.
Compatibility and rollout
Built-in policy templates and canonical production delays are unchanged.
Persisted custom templates that use unsupported predicate fragments or
noncanonical CSV values will no longer compile and should be audited before
rollout.
Checks
go test ./lib/arkscript ./lib/tx/checkpoint ./lib/tx/oormake lint-changed-localmake buildmake fmt-changedmake tidy-module-checkmake commitmsg-lint range="origin/main..HEAD"git diff --check