Repository navigation
feat(verify): #433 — take ordeal 0.33.0 and decide memory/UF goals via Solver::from_sliver - #435
Merged
Merged
Conversation
…a Solver::from_sliver Two asks from ordeal's side, each gated by an oracle observed RED first. 1. ordeal 0.30.0 -> 0.33.0. 0.32.1 fixes a sliver soundness bug (ordeal#247): a query variable named like the sliver's fresh read `$sel:<array>:<index>` was silently unified with it. New test `a_query_variable_cannot_masquerade_as_a_sliver_read` drives the exact counterexample, `select(memory, 5) != $sel:memory:5` (satisfiable), through the production solver. On 0.30.0 it came back `Proven`, with a certificate that rechecked. On 0.33.0 it is `Disproven`. loom's production path was not exposed: all 137 Z3 naming sites in loom-core were checked, and every name begins with a literal letter (`param…`, `pure_call_…`, `mem_after_call_…`); the five pass-throughs re-lower those same names. A collision needs a name starting with `$`. 2. `lower_goal_ne` builds memory/UF goals with `Solver::from_sliver` instead of `sliver::lower` plus a hand-asserted core. From 0.33.0 only such a solver runs the trusted, Lean-proven `check_sliver_lowering` before returning a verdict. New test `memory_obligations_are_decided_on_the_proven_sliver_path` asserts `is_sliver()` for a memory goal and not for a pure one. It was RED on 0.33.0 with the old path and is green after the switch. The refusal stays an `Err` and the deadline still applies, which were the reasons for avoiding the one-shot helpers (#347). Wiring: both tests run in slice 3's verification step (full suite under `both`, count-pinned). Reverting just the `from_sliver` call turned that step [FAIL] by the new test's name (567 passed, 1 failed). Pin 566 -> 568. loom-core with `verification`: 568 passed under default, `both` and `ordeal`; clippy --all-targets -D warnings clean; gate passes slices 3, 4. Fixes: #433 Refs: #313 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RZof1M5HMBSYVZPeoT4MEn
Rivet verification gate✅ 30/38 passed
Filter: Failed artifacts(none) Updated automatically by |
avrabe
added a commit
that referenced
this pull request
Oct 22, 2026
…nd trap_gate Slice 2 left two hand-written copies of the div/rem trap condition: its own `trap_condition` over the neutral RuleTerm, and ordeal's `trap::trap_div`, which trap_gate's constant-fold backstop called. Both had to agree on the div_s/rem_s asymmetry and nothing forced them to. The rule is now written once, `trap_gate::div_trap_condition`, generic over a `TrapTerms` trait that supplies only constructors (constant, eq, and, or). The seam instantiates it over RuleTerm/RuleBool, which it lowers to BOTH engines; trap_gate instantiates it over ordeal's BvTerm/BoolTerm. Neither instantiation decides when an op traps, so they cannot drift. Why loom's formula and not ordeal's: under `both`, the seam's clause must lower to the incumbent too, and an ordeal BoolTerm cannot. This reverses the documented "ordeal supplies the trap clause" boundary for div/rem only; the memory clause (`trap_mem_oob`) stays ordeal's. Module docs updated. Why the backstop stays: the default backend is Z3, and the seam defers to the incumbent before building the trap relation, so in default builds the backstop is the only trap check on const div/rem folds. Evidence: - the new rule is PROVEN equivalent to ordeal's trap_div (the clause it replaced) for all four kinds at widths 1, 8, 32, 64, 128, over symbolic operands; - slice 2's instantiation, sent through the production lowering, is PROVEN equivalent to trap_gate's at the same widths; - both comparisons are shown to discriminate (div_s vs rem_s must differ); - planting ordeal#84 (rem_s given the overflow disjunct) fails 5 tests across both consumers, including the wasm spec table by row; - loom-core with `verification`, rebased on main after #435 (ordeal 0.33): 571 passed under `both`. Slice 3's pinned count moves 568 -> 571 for the 3 new tests; the old pin is checked to no longer match. Slice 4's filtered pin (18 passed) still holds. - Rebase also moves the `a_division_under_a_conditional...` doc comment back onto its own test; #435 had inserted its two tests between them. Refs: #313 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RZof1M5HMBSYVZPeoT4MEn
avrabe
added a commit
that referenced
this pull request
Oct 23, 2026
…nd trap_gate (#434) Slice 2 left two hand-written copies of the div/rem trap condition: its own `trap_condition` over the neutral RuleTerm, and ordeal's `trap::trap_div`, which trap_gate's constant-fold backstop called. Both had to agree on the div_s/rem_s asymmetry and nothing forced them to. The rule is now written once, `trap_gate::div_trap_condition`, generic over a `TrapTerms` trait that supplies only constructors (constant, eq, and, or). The seam instantiates it over RuleTerm/RuleBool, which it lowers to BOTH engines; trap_gate instantiates it over ordeal's BvTerm/BoolTerm. Neither instantiation decides when an op traps, so they cannot drift. Why loom's formula and not ordeal's: under `both`, the seam's clause must lower to the incumbent too, and an ordeal BoolTerm cannot. This reverses the documented "ordeal supplies the trap clause" boundary for div/rem only; the memory clause (`trap_mem_oob`) stays ordeal's. Module docs updated. Why the backstop stays: the default backend is Z3, and the seam defers to the incumbent before building the trap relation, so in default builds the backstop is the only trap check on const div/rem folds. Evidence: - the new rule is PROVEN equivalent to ordeal's trap_div (the clause it replaced) for all four kinds at widths 1, 8, 32, 64, 128, over symbolic operands; - slice 2's instantiation, sent through the production lowering, is PROVEN equivalent to trap_gate's at the same widths; - both comparisons are shown to discriminate (div_s vs rem_s must differ); - planting ordeal#84 (rem_s given the overflow disjunct) fails 5 tests across both consumers, including the wasm spec table by row; - loom-core with `verification`, rebased on main after #435 (ordeal 0.33): 571 passed under `both`. Slice 3's pinned count moves 568 -> 571 for the 3 new tests; the old pin is checked to no longer match. Slice 4's filtered pin (18 passed) still holds. - Rebase also moves the `a_division_under_a_conditional...` doc comment back onto its own test; #435 had inserted its two tests between them. Refs: #313 Claude-Session: https://claude.ai/code/session_01RZof1M5HMBSYVZPeoT4MEn Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
This does both asks in #433. Each has its own oracle, and I watched each one go red before the change.
1. Bump ordeal from 0.30.0 to 0.33.0 (soundness fix ordeal#247)
Oracle:
a_query_variable_cannot_masquerade_as_a_sliver_read. It drives the issue's exact counterexample,select(memory, 5) != $sel:memory:5, throughBoundedOrdealSolver, the productionRuleSolver. The query is satisfiable because the two sides are independent.Proven, with a certificate that rechecks. This is the bug, reproduced through loom's own path.DisprovenWas loom's production path exposed? No, and I checked. I found 137 naming calls (
BV/Bool/Arraynew_const/fresh_const,FuncDecl::new) in non-testloom-corecode:param…,global…,pure_call_…,mem_after_call_…,f32_…).decl_name = "pure_call_{func_idx}", and threerule_solversites that re-lower names the reflector read back from those same Z3 ASTs.A collision needs a name that starts with
$. The sliver's only production input isdecide_bv_equivalence, which receives those ASTs.2.
lower_goal_neusesSolver::from_sliverFrom 0.33.0, only a solver built by
from_sliverruns ordeal's trusted, Lean-provencheck_sliver_loweringbefore it returns a verdict. The old path calledsliver::lowerand asserted the core by hand, which skipped that check.Oracle:
memory_obligations_are_decided_on_the_proven_sliver_path. It assertsSolver::is_sliver()for a memory goal, and not for a pure one. I extracted the construction intosolver_for_goal_newithout changing behavior first. The test was then red on 0.33.0 with the old path, and it went green after the switch. No directsliver::lowercall remains in non-test code.The reasons for avoiding the one-shot helpers (#347) still hold: a refusal is still an
Errthat the seam defers on, and the deadline still governs the check.Wiring
Both tests run inside slice 3's verification step (the full suite under
both, with the count pinned). To prove that by mutation, I reverted only thefrom_slivercall: the wired step went[FAIL]and named the new test (567 passed; 1 failed). The pin moves from 566 to 568.Results
loom-corewithverification: 568 passed underdefault,bothandordeal--all-targets -D warnings: cleantools/run_verification.pypasses slices 3 and 4Cargo.lock, which is what CI buildsKill-criterion
This change is wrong if any memory or UF verdict comes from a solver with
is_sliver() == false, or if the$selimpostor is everProven.One thing would still be false if every check here passed: that an
is_sliver()solver gates every verdict, on the entry point loom uses (check_with_deadline), oncheck_sliver_lowering. That is ordeal's guarantee, documented onSolver::from_sliver, and loom can only observe its precondition. If that guarantee breaks, it should be filed against ordeal.Merge note
#434 also moves the same slice 3 pin, from 566 to 569. Whichever of the two merges second needs the pin set to 571.
Fixes: #433
Refs: #313
🤖 Generated with Claude Code
https://claude.ai/code/session_01RZof1M5HMBSYVZPeoT4MEn