Conversation
| gnumake | ||
| gnupg # needed for signing commits & codecov/codecov-action | ||
| graphviz | ||
| leanToolchain # formal verification toolchain |
There was a problem hiding this comment.
I would suggest to move it to a separate devshell because most of developers don't need it. And it is quite large (500-800 MB artefacts on the releases page)
There was a problem hiding this comment.
We discussed with Sergey and to make it both easy for people who need lean and everything else we need to add support for .envrc.local which will be gitignored and those who need a non standard devshell could create .envrc.local containing:
use flake .#lean_devshell
This lean devshell will have our regular devshell + lean
|
|
||
| # The release binaries name the FHS loader in PT_INTERP; autoPatchelfHook | ||
| # points them at the Nix one. | ||
| buildInputs = lib.optional stdenv.hostPlatform.isLinux stdenv.cc.cc.lib; |
There was a problem hiding this comment.
@mathbunnyru, is it ok to bring stdlib here? Because I remember we have a custom built one but you probably know this part better
There was a problem hiding this comment.
lean from nix should only be used as a tool, not something linked to, so this should be ok
There was a problem hiding this comment.
My check-tools found this brings another problem: https://github.com/XRPLF/rippled/actions/runs/34207349071/job/102029155206?pr=8186
This downgrades clang from 22 to 19, which we would definitely would like to avoid
mathbunnyru
left a comment
There was a problem hiding this comment.
- Can we maybe use 4.30.0? Not being able to use latest version of a tool is usually a pain in the future, when the gap will become even wider.
- Please, update https://github.com/XRPLF/rippled/blob/develop/bin/check-tools.sh accordingly
Latest Lean4 release is 4.33, there are frequent version bumps. Using 4.30 helps us now as it's the latest in unstable, but it will be updated in the future. That means if there are ever breaking changes in Lean that break the formal verification code or proofs, that will have to be fixed as well. What do you recommend here? |
I would use the version provided by Nix if it works. Note that we don't pin specific versions of any software except for major gcc/clang (and libc version, but that's a different story). So, I would try the simple approach we already use and only start doing something more custom (which we'll have to maintain), only when we get evidence we need to. |
I tested the proofs against all the versions from 4.28 to 4.33, and each version bump adds some additional breakages. It seems most of them are minor, but we still need to deal with them. We are fine with either solution, they have different tradeoffs on long-term maintenance. @Tapanito any opinion on this? Depending on the plans for formal verification in the future, that might push towards one option or another Here is the AI generated report of checking each version: Lean version compatibility reportResult matrix (full
|
| Lean | Mathlib | Result | Broken decls / files |
|---|---|---|---|
| 4.28.0 | v4.28.0 | ✅ clean | 0 |
| 4.29.1 | v4.29.1 | ❌ | 5 decls / 3 files |
| 4.30.0 | v4.30.0 | ❌ | 6 decls / 4 files |
| 4.31.0 | v4.31.0 | ❌ | 30 decls / 16 files |
| 4.32.2 | v4.32.2 | ❌ | 86 decls / 32 files |
| 4.33.1 | v4.33.1 | ❌ | 126 decls / 40 files |
The model (XRPL/Model, 33 files) and FFI (XRPL/FFI, 14 files) build cleanly on every version. All breakage is in XRPL/Properties. To verify completeness I stubbed each failing declaration's proof with sorry and re-ran until green — the counts above are the exact, full breakage sets (no defs were stubbed, so no false downstream failures; statements untouched).
Failure causes (cumulative per version)
- 4.29+ —
omegahits max-recursion-depth on goals mixing big Nat literals (10^15,16^15) andFinsetsums:Rounding/Guard.lean(hex_lt_iff_dec_lt,round_to_nearest_def),Rounding/SmallRangeBound.lean(×2),Rounding/DivQuotient.lean, and on 4.30+STAmount/Add/Common/IOU.lean. RaisingmaxRecDepthto 8192 causes a hard stack overflow — so this needs genuine restructuring (clear/unfold/generalize beforeomega), but only at ~5–6 sites. - 4.31+ —
simp/simp only"made no progress" is now an error (was tolerated in ≤4.30). This is by far the largest category: whole families likeNumber/Add|Mul|Normalize/**/BoundProof.lean,RoundsWithinProofs.lean,Closest/NoInbetween.lean. Plus minorconvert … using 1leftovers inClosest/Normalize.lean(fix: appendnorm_num/rfl). All mechanical, 1-line-ish each — but dozens of sites. - 4.32+ — stricter application matching:
bind_ok_peelno longer applies afterunfold+rwin mostVault/Common/*proofs (hypothesis is only definitionally a>>=chain now). AlsoBurnExits.leanno-opsimp. Fixable per-site withchange/dsimp/simp only [Bind.bind]shims — small but widespread (~50 sites). - 4.33+ — stricter
rw: "motive is not type correct" ×10 andwhnfheartbeat timeouts ×3 in the heavyDoRoundUp/Bounds.lean/ValueChar.leanproofs, plus onelinarithregression (ScaleDown.lean). These are the least mechanical — some proofs will need real reworking (orconv/occs/simpinstead ofrw).
Verdict
- Big fixes? No refactoring of model/statements is needed anywhere — every failure is local to proof bodies, and I confirmed a green build on 4.33.1 with only those 126 proof bodies stubbed.
- Small fixes? Individually yes for ~80% of sites (del no-op
simp, addnorm_num, shufflerw/change), but the volume makes the 4.33.1 upgrade a moderate effort: ~126 proof sites over 40 files, of which a handful (DoRoundUp motive-errors, whnf timeouts, omega recursion) need real attention. - Suggested path if upgrading: go to 4.29.1 first (only 5 sites, all the omega issue), then 4.31 (simp sweep), 4.32 (Vault bind gymnastics), 4.33 (rw/motive + timeouts).
|
Let's use nix-provided 4.30 for now, also let's update when this is merged: NixOS/nixpkgs#545312 |
Sounds good. One remaining question about if [ "${XRPL_DEVSHELL:-}" = "formal-verification" ]; then
echo
echo "Formal verification toolchain:"
check lean
check lake
fi |
hey @mathbunnyru, sorry to ping you again, did you have a chance to see this question? |
Sorry for not answering you before, I was on vacation :) |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The isolated toolchain, environment selection, and validation logic are consistent and introduce no unresolved issues.
Review effort: Balanced
Findings: None
What changed in this PR
Adds an opt-in Nix development shell for Lean 4 formal verification without affecting standard development environments.
Changes:
- Adds the
formal-verificationshell with Lean and Lake. - Extends tool checks for the new shell.
- Supports local direnv shell overrides.
| File | Description |
|---|---|
nix/devshell.nix |
Defines the formal-verification shell. |
bin/check-tools.sh |
Checks Lean and Lake in that shell. |
nix/check-tools/README.md |
Documents shell-specific tool snapshots. |
.envrc |
Loads optional local overrides. |
.gitignore |
Excludes local direnv overrides. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
High Level Overview of Change
Add the lean4 toolchain to the nix development shell. This will be used by the upcoming formal verification work, and is not currently used in the build.
Context of change
We add a new development shell for formal verification containing the
Lean 4toolchain. We use a separate shell as the tool is quite large and won't be used by most users.check-toolswill check depending on the shell it's in using theXRPL_DEVSHELLenv variable.API Impact
No impact, development environment change only.
OLD: previous approach, kept for context on previous approach