Remove covenant struct desugaring and unify on lowered ABI contract#54
Merged
someone235 merged 19 commits intokaspanet:masterfrom Mar 10, 2026
Merged
Remove covenant struct desugaring and unify on lowered ABI contract#54someone235 merged 19 commits intokaspanet:masterfrom
someone235 merged 19 commits intokaspanet:masterfrom
Conversation
Inline-call debug recording could fail with `CyclicIdentifier("<param>")`
when a function took a struct-array parameter such as `S[]` or `State[]`
and the caller passed the same identifier through.
The regression came from `prepare_inline_call_bindings()`: for array-like
params it could insert a self-alias binding of the form
x -> Identifier("x")
into the inline-call env. Runtime compilation tolerated this because
`resolve_expr_for_runtime()` preserves array identifiers, but debug-info
recording uses `resolve_expr_for_debug()` / `resolve_expr()`, which
recursively resolves env bindings and therefore detected the self-cycle.
Fix this by never inserting self-alias bindings for inline-call params.
This keeps the env semantically clean and fixes debug-info compilation
without changing runtime behavior.
Add compiler coverage showing:
- plain `int[]` inline calls with debug info still compile
- struct-array `S[]` inline calls with debug info now compile
Add focused runtime regressions around the selector-dispatched `validateOutputState` path. The new tests cover three cases: - validating a `State` sourced from `State[]` indexing - validating a `State` sourced from an inline-returned `State[]` - validating a `State` under selector dispatch Together these isolate that the remaining failure is selector-specific: non-selector `validateOutputState` cases succeed, while the selector-dispatched path still rejects what should be valid reflected outputs. Also update the many-to-many covenant security coverage to reflect the same underlying issue in generated covenant wrappers.
Fix `validateOutputState` for selector-dispatched contracts by making state reflection operate on a single shared contract-state segment and correcting the prefix substring extraction. The previous selector layout encoded contract state separately inside each dispatch branch. `validateOutputState` reconstructs the expected redeem script by replacing one contiguous state segment, so selector contracts could not be reflected correctly. This change: - preserves the selector on the alt stack - emits the contract field prolog once before selector dispatch - restores the selector before dispatch execution - fixes the prefix substring in `validateOutputState` to slice `[redeem_start, redeem_start + state_start_offset)` With this, selector-dispatched `validateOutputState` works correctly, including the many-to-many covenant leader path that depends on it.
…ctually use a different script
Add runtime security coverage for N:M covenant transition wrappers that currently accept caller-supplied `prev_states` on the leader path. The new test first proves the honest transition succeeds when the leader passes the real previous states, then attempts to spoof different `prev_states` through the leader sigscript and expects rejection. Today the spoofed case is accepted, so this test captures the regression at the runtime level instead of only through ABI shape assertions.
Fix covenant transition lowering so N:M leader entrypoints do not accept `prev_states` through the ABI. For `binding = cov, mode = transition`, the previous states are committed by the covenant input set and must be sourced with `readInputState(...)` from `OpCovInputIdx(...)`. Exposing them as leader entrypoint arguments allowed callers to spoof the transition inputs through the sigscript. This change: - removes the first policy param (`prev_states`) from cov-transition leader entrypoint params - rebuilds `prev_states` inside the generated leader wrapper from the covenant inputs before calling the policy - updates the expected lowered AST accordingly - updates covenant sigscript coverage to reflect the new leader ABI - adds security coverage that rejects spoofed `prev_states` through the leader ABI This keeps the transition policy shape intact while enforcing that previous states come from the covenant context rather than caller input.
Add a focused runtime regression showing that `readInputState` still fails under selector dispatch. The new test uses the minimal form: - two entrypoints (so selector dispatch is enabled) - `State s = readInputState(this.activeInputIndex);` - `require(s.x == 5);` The same pattern works without selector dispatch, so this isolates the remaining bug to selector-mode `readInputState` handling rather than covenant lowering, struct arrays, or `validateOutputState`. Keep the single-field cov-transition AST example for review of the generated wrapper shape.
Fix `readInputState` when lowering `State` values under selector dispatch.
The remaining selector regression was not in covenant lowering itself, but
in the symbolic `State` lowering path used for expressions like:
State s = readInputState(...)
The runtime `readInputState` helper already used the correct state start
offset, but the symbolic lowering path still assumed contract state
started at offset 0. That worked without selector dispatch and failed
once selector prefixes were present.
This change:
- makes symbolic `readInputState` use the same selector-aware state
offset model as the runtime helper
- narrows the fix to actual `readInputState` lowering instead of pushing
selector offsets through all struct lowering paths
- keeps generic struct lowering unchanged, avoiding unrelated regressions
- updates covenant AST fixtures to match the current generated wrapper
shape
This fixes the selector-mode `readInputState` regression and restores
the honest many-to-many transition happy path.
Add compiler regressions for selector-dispatched `readInputState` and `validateOutputState` using a three-field state with mixed widths: - `int` - `byte[2]` - `byte[32]` These tests extend the selector state-offset coverage beyond the single-field and two-field cases and help guard against future prefix / field-offset regressions in mixed-width state layouts.
Closed
Require `out_count == new_states.length` in generated verification wrappers for both auth and cov bindings. This removes the previous behavior where under-provided `new_states` failed only indirectly and surplus entries were silently ignored. Also update the AST fixtures, security tests, and DECL spec to match the new wrapper invariant.
michaelsutton
approved these changes
Mar 10, 2026
Manyfestation
added a commit
to Manyfestation/silverscript
that referenced
this pull request
Mar 10, 2026
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
2 participants
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.
No description provided.