Fix/ecsm address bound air executor - #913
Conversation
|
/bench |
Benchmark — real block (
|
| Metric | main | PR | Δ |
|---|---|---|---|
| Peak heap | 47024 MB | 46860 MB | -164 MB (-0.3%) ⚪ |
| Prove time | 157.521s | 156.874s | -0.647s (-0.4%) ⚪ |
✅ No significant change.
Prove-time spread 0.8% (156.749s / 157.939s / 156.874s)
Commit: d30fee1 · Baseline: cached · Runner: self-hosted bench
|
/ai-review |
Codex Code ReviewNo actionable issues found in the PR diff. The executor and ECSM proving changes appear consistent. |
AI ReviewPR #913 · 9 changed files Findings
Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro). AI-001: Partial write on address overflow in store_u256_le
Claim store_u256_le writes the first 3 doublewords before checking the 4th address; if the 4th overflows, the first 3 have already been committed to memory, violating atomicity of the 32-byte store. Evidence Lines 86-95: the function loops i=0..3, does checked_add and store_doubleword for each. If i=3 fails, i=0,1,2 have already executed store_doubleword. The ECSM syscall calls this as the last step (line 453), so a partial xR write could be left in memory on error. Suggested fix Check all 4 addresses upfront before any stores: add a pre-loop that validates addr.checked_add(31) (last byte), or check each base with checked_add in a separate loop before the store loop. Reviewer Lanes
Verification Lanes
Native Codex and Claude reviews run separately and post their own comments. They are not included in this structured provenance report. Discarded candidates (5) — rejected by the verifier
Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts. |
Give every ECSM operand access its own address column
Closes the divergence reported in #902 by fixing its root cause, so
ecsm_addr_okdisappears rather than moving.What
The AIR built each operand's four doubleword bases as
ADDR_*_0 + 8i— a bare field addition onthe low address limb, high limb passed through, no per-access column, no carry, no range check. So
the circuit only needed the last base canonical (
+24) while the executor guarded+31, and theseven low limbs in between were provable executions the reference VM aborts:
0xFFFF_FFE0(last base0xFFFF_FFF8)0xFFFF_FFE1…0xFFFF_FFE7EcsmAddressOverflow0xFFFF_FFE8(last base2^32)EcsmAddressOverflowThird column = the same trace with the guard bypassed, through the production prove/verify path; an
aborted execution yields nothing to prove. Low severity — the bytes land at the arithmetically
correct addresses, it needs an attacker-authored ELF, and the shipped guest sits at high limb
0xFFFF_FFFFwhere both sides reject — but the accepted set was maintained in two places.Now every access carries its own address, derived and range-checked in-circuit:
addr_{xG,k,xR}[1..=3]asDWordHL; the 12 MEMW senders read them instead ofadding
8i.NUM_COLUMNS667 → 703emit_add_pairadditions, two carries each, plusµ·carry_1 = 0peroperand. 413 → 434
IsHalfwordsends, with matching lookups incollect_bitwise_from_ecsm— the receivemultiplicity has to move with the sends or the bus does not balance
ecsm_addr_okandExecutionError::EcsmAddressOverfloware gone.load_u256_le/store_u256_leusechecked_add, and that is the whole contract: the circuit accepts anoperand iff
addr + 31 < 2^64and the memory path refuses exactly the complement —u64::MAX - 31runs,u64::MAX - 30does not. Worth being precise about which table ownswhich half: ECSM's
µ·carry_1boundsaddr + 24, and the last byte is bounded one table over,where MEMW's per-byte
hi = base_1 + carryis never reduced mod2^32and lands at2^32fora base in
[u64::MAX-30, u64::MAX-24], which no PAGE token supplies. In-circuit, but across-table argument, and not reachable through the executor so no test covers it
call. No prove-time number claimed
The
xG/kdisjointness guard stays, with its comment corrected: overlap does not make thetrace unprovable —
xGis read atTandkatT+1, so an overlapping cell chains throughMEMW normally, and bypassing the guard in a scratch build produced a fully-aliased trace that
verifies. No test here pins that, because the guard is what stops one being built.
Spec
ec:c:range_addr_*andec:c:extrapolate_addr_*onspec/main(c75966ee, #655) specify exactlythis, and
KECCAKalready implements the shape for its 25 lane pointers. Two deliberate deviations:µ·carry_1 = 0is ours. TheADDtemplate constrains its carries only to be bits, soaddr[3] = addr[0] + 24 − 2^64satisfies the spec as written. Test below.range_addr_*coversi = 0..3; we skipi = 0, whoseDWordWLform the spec sanctions("could be
DWordWLs rather thanHLs"). What makes that safe is not the REGISTER tablerange-checking register words — it does not,
register.rshas no constraint set at all. It isthe
i = 0MEMW send putting the pair straight on the Memory bus, where the only tokens comefrom PAGE and REGISTER at canonical
(lo, hi)addresses. Every chip taking an address from aregister leans on the same argument. Worth a second opinion.
Tests
The AIR side of these addresses had no coverage; every existing test was executor-side.
test_prove_ecsm_derived_bases_cross_limb— the positive case for the new columns:xRat0xFFFF_FFFC, so the derived bases carry into the high limb (addr[1] = 0x1_0000_0004). Theold inline derivation would put a non-canonical low limb on the bus, so this guest is
unprovable on
main. Guest reads the bytes back and commits them; must equalx(5·G)extrapolate_addr_rejects_a_wrong_per_access_address— movingaddr_*[i]one byte breaks theADD carry, over all three operands and all three
iaddr_wrapping_past_u64_is_rejected— a wrapped last address satisfies all six carry bits andis caught only by
µ·carry_1; the wrapped address is small and matchable, so what this blocksis a collision with live memory. All three operands
addr_constraints_close_on_padding— every new constraint is µ-gatedis_half_sends_match_the_collector— the send count against the collector that feeds thereceive multiplicity. That invariant is what the MSB16 bus bug broke, and only end-to-end
proving covered it before
test_prove_ecsm_operand_crossing_limb_boundary— byte carries:xRat0xFFFF_FFE1, bytesread back from across
2^32. This pins the executor relaxation, not the new columns — here allfour derived bases have
hi = 0and the sends match the old derivation. Same guest undercontinuations, padded so the epoch boundary falls between the write and the read-back
store and from the load