feat: Updates to foundations, and audit gap resolutions, and improved ergonomics#92
feat: Updates to foundations, and audit gap resolutions, and improved ergonomics#92dtumad wants to merge 87 commits into
Conversation
Move `mul_diff_one_neq` out of the `KoalaBear` namespace and restate it over any
`[Field α]` — the proof goes through unchanged via aesop, and the lemma was only
picked up as an aesop-forward rule so no callers reference it by name. Drop the
13 unused `KoalaBear.{ne_zero_*, val_*}` helpers (referenced only inside
Field.lean); swap their internal uses in `inv_mul_*BB_eq_iff` for `by decide`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sp1-lean into dtumad/milestone-3
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sp1-lean into dtumad/milestone-3
Sail-v4 added two platform-level monadic reads inside control-flow instructions that the SP1 proofs cannot discharge from isInitialized alone: execute_JALR's new update_elp_state call (reads cur_privilege via currentlyEnabled Ext_Zicfilp), and jump_to's new alignment gate (reads misa via currentlyEnabled Ext_Zca). Mirror the pmp_check_machine pattern with three new axioms in SP1Foundations.Assumptions: - update_elp_state_of_isInitialized: SP1 doesn't enable Zicfilp, so update_elp_state reduces to pure (). - jump_to_of_mod4_eq_zero: 4-aligned targets skip the Zca check, making jump_to equivalent to writeReg nextPC target; pure RETIRE_SUCCESS. - jump_to_of_mask_mod4_eq_zero: specialisation for JALR's (rs1 + imm) & ~1 target shape. Plus SailState.isInitialized_insert so the axiom's side condition survives post-write state extensions, and mod4_eq_zero_of_0_1_are_0 (reverse of mul4_means_0_1_are_0) for chips that have bit-level alignment hypotheses instead of mod-4. Apply these to close the stop in JALR_correct. The same pattern is ready to unblock the 6 Branch stops and the Jal stop, pending local tactic plumbing (the ↑↑ vs #' BitVec.ofNat form mismatch). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sp1-lean into dtumad/milestone-3
…sp1-lean into dtumad/milestone-3
Suppress linter.unusedVariables in MemChecks.lean and BranchChip.lean (proofs there are stopped/sorried pending Sail v4 rewrite but keep their final parameter lists). Drop a no-op `repeat rw` in byte_decomp_128 that tripped unusedTactic/unreachableTactic, and mark BoundedKoalaBear.coe_of_le @[reducible] as required for class-valued defs in Lean 4.29. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sp1-lean into dtumad/milestone-3
…sp1-lean into dtumad/milestone-3
…sp1-lean into dtumad/milestone-3
…sp1-lean into dtumad/milestone-3
….com/succinctlabs/sp1-lean into dtumad/update-constraints-is-trusted
| Sail.writeReg Register.nextPC ((← Sail.readReg Register.PC) + 4#64) | ||
| let _ ← execute_JAL imm rd | ||
|
|
||
| set_option debug.skipKernelTC true in |
There was a problem hiding this comment.
how come this is needed (here and in other places)?
There was a problem hiding this comment.
It is sort of hard to diagnose them, tends to have something to do with how sail generates many coercions between bitvec/nat/int. Often times these in theory discharge easily with reflexivity etc., but cause problems when the final proof compiles because we have large constants sitting around. #94 has started to fix some by just not having the koala bear prime in certain expressions, but still other things like Word.toBitVec64 still produce values like 2^48 in their terms, which the kernel can crash over when unifying.
The best MWE is probably something like:
example (b n : ℕ) : -- unifies properly
((b : ℤ) % n).toNat = b % n := by
rfl
example (b n : ℕ) : -- deep recursion
((b * 2^64 : ℤ) % n).toNat = (b * 2^64) % n := by
rflI'd be fine scoping #94 to fix all of these as well, but it is really just hunting down and fixing overzealous unification with large constants, which can be difficult when aesop or simp do those things internally to their execution.
There was a problem hiding this comment.
And also notably, the kernal is not shy about unfolding various things eagerly, so something like def Nat.twoPow (n : ℕ) : ℕ := 2 ^ n can't solve this.
|
|
||
| -- Generated Lean code for chip LoadX0Chip | ||
| @[irreducible] def constraints (Main : Vector (Fin KB) 50) : SP1ConstraintList := | ||
| @[irreducible] def constraints (Main : Vector (Fin KB) 48) : SP1ConstraintList := |
There was a problem hiding this comment.
Is there a theorem for these constraints elsewhere? generally speaking it might be useful to make notes for each constraints file of where the corresponding correctness theorem is located.
There was a problem hiding this comment.
No, I missed LoadX0 when doing the others. I will add that back and plan to gather all the chip_correct statements in a centralized file, making the current correctness statements into helper lemmas in each chip-specific file. I think that's probably the best way to make it easy to both make coverage clear to see and the proofs easy to look for.
| let offset := (BitVec.signExtend 64 (sp1_imm_c Main)).toNat | ||
| reg_val + offset + 1 < 2^64) : | ||
| reg_val + offset + 1 < 2 ^ 64) | ||
| (h_below_clint : |
There was a problem hiding this comment.
Could you document assumptions like this? It shows up in several other places. Since this would be input dependent it's not immediately obvious this is realistic. Maybe it could be placed in SP1Foundations/Assumptions.lean if it stays as is?
There was a problem hiding this comment.
This is actually strictly weaker than the already proven is_u48_sum found here, since SP1 has an even smaller memory region than we need for this. These should definitely be replaced with that, will work on the changes.
| let ram_size := (s.regs.get Register.plat_ram_size (hs _)).toNat | ||
| reg_val + offset + 2 ≤ ram_size) | ||
| reg_val + offset + 2 < 2 ^ 64) | ||
| -- dt: This should eventually come from trusted instruction assumption |
There was a problem hiding this comment.
Any progress on this / reason this might be harder than expected? h_is_aligned shows up in other places.
There was a problem hiding this comment.
See above comment on loadByte, this can come from the same place. Although it might be good to then add (s.regs.get Register.plat_ram_size (hs _)).toNat > 2^48, which would hopefully let us drop pma_check_machine axiom as constraints imply in memory if we assume that much ram size. Will also look at this.
| (state_cstrs : (LoadByte.constraints Main).initialState s) | ||
| (h_is_lb : Main[46] = 1) | ||
| (h_is_lb : Main[45] = 1) | ||
| (h_fits_in_mem : |
There was a problem hiding this comment.
Could this be derived from constraints? (also appears in other places iirc)
There was a problem hiding this comment.
Constraints like these specifically are toggles because the same circuit handles multiple instructions. If this were zero you'd want it to behave like unsigned version so the theorem should fail then. There could I suppose be a combined lemma showing the circuit behavior as a whole. Across cases.
There was a problem hiding this comment.
In a lot of the chips, there are specific predicates like is_sltu, is_divu, etc. that make this type of assumption look cleaner, but they are fundamentally the same sort of assumption
|
Some general comments: |
|
Superceded by #97 |
Updates to Dependencies
4.23 -> 4.29and resolves build errorsLean_RV64Dextraction (as of 4/24 currently, should pin something pre-merge). Uses the main upstream repo now instead of asuccinctlabsfork. Memory proofs in particular had large changes from this, largely for the better.sp1releasev6.1.0and fix breaking changes (nois_trustedor related constraints, some numerical opcode re-ordering, JALR changes)Audit Gaps
stopcalls as a result. Added them there and fixed all the gaps. Should addlake exe mk_allcheck to CI to protect against this in another PR.sorrystubs and so also didn't surface. Added them and fixed all the gaps.sltiproof incorrectly had access to anis_sltuassumption, making the proof trivial. Removed this and resolved the gap.Assumption Changes
pma_checkaxiom mirroring existingpmp_checkaxiom. Needed for the RV64D extraction update.mprotect_disabledassumption, as it's not needed for this release.Register.plat_clint_baseandRegister.plat_clint_size. Not needed after RV64D update.AUIPCandLUItrusted intruction constraints now includeop_b_0.val % 2 ^ 12 = 0instead ofop_b_0 >= 2^12(i.e. the value is properly zero-extended).Register.mseccfghas a zero bit in position 10, i.e. Zicfilp is not enabled in M-mode. Needed for the RV64D extraction update.General Changes
obtainblocks apart, tweaking instance priorities, and extracting some shared helper lemmas.