Skip to content

feat: formalize slot_is_justifiable_after in Lean4#269

Open
MegaRedHand wants to merge 19 commits intomainfrom
lean-formalization
Open

feat: formalize slot_is_justifiable_after in Lean4#269
MegaRedHand wants to merge 19 commits intomainfrom
lean-formalization

Conversation

@MegaRedHand
Copy link
Copy Markdown
Collaborator

@MegaRedHand MegaRedHand commented Apr 9, 2026

Introduction

This PR introduces a Lean4 package under formal/, formalizing slot_is_justifiable_after and proving some properties like "there's always another justifiable slot" and "the distance between justifiable slots increases as the delta increases".

First step of #272

Lean4 package structure

formal/
    EthLambda/
    EthLambdaProofs/
    lean-ffi/

The new Lean4 package has two libraries: EthLambda and EthLambdaProofs. The first one contains the actual implementations called from Rust, using bounded types like UInt64. EthLambdaProofs, on the other hand, contains theorems proving some properties of the implementation.

Apart from those two, there's an additional directory with a Rust crate wrapping the Lean FFI glue.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

🤖 Kimi Code Review

This PR integrates formally verified Lean4 code for the 3SF-mini justifiability check via FFI. The implementation is mathematically sound and the Rust integration follows safe patterns, though there are minor build system and documentation improvements needed.

Critical/Security Findings

None. The consensus logic is mathematically proven (zero axioms/sorrys in the proof tree) and the FFI boundary uses scalar types (u64/u8) avoiding memory safety issues. The isqrt implementation uses bounded iteration (64 steps max) preventing DoS via quadratic convergence.

Code Correctness & Safety

  1. FFI Initialization Safety (formal/lean-ffi/src/lib.rs:29-36)

    • Uses std::sync::Once correctly to ensure single initialization of the Lean runtime.
    • Properly decrements the reference count on the initialization object.
    • Marking as safe since scalar-only FFI calls are thread-safe after initialization.
  2. Overflow Safety in Justifiability Check

    • The Lean isqrt avoids overflow by using division-based correction (r+1 <= n/(r+1) instead of multiplication) and starting Newton's method from n/2+1 (Line 33-34, Justifiability.lean).
    • The bridge theorem (Bridge.lean:428) proves correctness for all UInt64 inputs with the only bound being d < 2^62 to ensure 4*d+1 doesn't overflow, which is enforced by the type system.
  3. Pronic Detection Correctness

    • The algorithm correctly uses the mathematical identity: d is pronic iff 4d+1 is an odd perfect square (PronicDetection.lean:12-32). This is verified in the isPronic_iff_sqrt theorem.

Build System & Maintenance

  1. Path Issue in Build Script (formal/lean-ffi/build.rs:58)

    println!("cargo:rerun-if-changed=../../formal/EthLambda/Justifiability.lean");

    While functionally correct (relative to the workspace root), this path is redundant. Since build.rs is at formal/lean-ffi/build.rs, the path should be ../EthLambda/Justifiability.lean for clarity.

  2. Missing Dependency Documentation
    The build.rs assumes lake and lean are in PATH (Lines 12, 20). Add a check with a helpful error message or document in formal/lean-ffi/README.md that elan (Lean toolchain manager) is required.

  3. Hardcoded Target (formal/lean-ffi/build.rs:14)
    The target EthLambda is hardcoded. If the Lean package name changes, this breaks silently. Consider deriving this from CARGO_PKG_NAME or documenting the coupling.

Rust Best Practices

  1. Documentation Consistency (crates/blockchain/state_transition/src/lib.rs:484-514)
    The #[cfg(feature = "lean-ffi")] version of slot_is_justifiable_after lacks the detailed doc comment present on the #[cfg(not(feature = "lean-ffi"))] version. Add documentation explaining this calls the formally verified implementation.

  2. Feature Flag Propagation
    The feature flag threading through Cargo.toml files (ethlambda-blockchainethlambda-state-transitionethlambda-lean-ffi) is correctly implemented.

Testing

The unit tests in formal/lean-ffi/src/lib.rs (Lines 74-96) provide good coverage for:

  • Small deltas (0-5)
  • Perfect squares
  • Pronic numbers
  • Negative cases
  • Slot API boundary conditions

Recommendations

  1. **Add

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

🤖 Claude Code Review

Now I have enough context. Let me compile the review.


PR Review: feat: formalize slot_is_justifiable_after in Lean4

This is an impressive piece of engineering — formal verification via Lean4 FFI in a consensus client is ambitious. The mathematical proofs are well-structured and the trust chain (Rust → UInt64 justifiable → Nat Justifiable → math definition) is clearly documented. Below are the issues I found, roughly ordered by severity.


Correctness / Potential Bugs

Overflow in justifiable for delta >= 2^62

formal/EthLambda/Justifiability.lean:194-195

|| (let val := 4 * delta + 1
    isqrt val ^ 2 == val && val % 2 == 1)

When delta >= 2^62, 4 * delta + 1 overflows UInt64 and wraps around. For example, delta = 2^62 gives val = 1, and isqrt(1) = 1, so 1^2 == 1 && 1 % 2 == 1 — the pronic check returns true spuriously, misclassifying a non-pronic value as justifiable.

The justifiable_equiv theorem in Bridge.lean explicitly requires d.toNat < 2^62 to guard against this, so the proof is correct, but the runtime FFI path has no such guard. The Rust wrapper in lib.rs passes u64 values through without bounds checking:

pub fn justifiable(delta: u64) -> bool {
    init_lean();
    unsafe { lean_justifiable(delta) != 0 }
}

In practice, delta = slot - finalized_slot would never approach 2^62 in any real network. But since the formal proof explicitly documents this as its trust boundary, the Rust API should either document the precondition with a debug_assert!(delta < (1u64 << 62)), or add a bounds check that falls back to the Rust implementation.


Stale doc-comment path

formal/lean-ffi/src/lib.rs:4

//! The Lean4 code at `formal/ffi/LeanFFI/Justifiability.lean` implements the

The actual path is formal/EthLambda/Justifiability.lean. This was presumably left over from an earlier directory layout.


Build System

Incomplete rerun-if-changed triggers

formal/lean-ffi/build.rs:134-135

println!("cargo:rerun-if-changed=../../formal/EthLambda/Justifiability.lean");
println!("cargo:rerun-if-changed=src/lean_glue.c");

The build also compiles EthLambda.c (the top-level module) which is produced from formal/EthLambda.lean. If that file changes, the compiled C IR changes, but Cargo won't know to re-run the build script. At minimum, add:

println!("cargo:rerun-if-changed=../../formal/EthLambda.lean");
println!("cargo:rerun-if-changed=../../formal/lakefile.toml");
println!("cargo:rerun-if-changed=../../formal/lean-toolchain");

lake build failure gives no diagnostics

formal/lean-ffi/build.rs:91

assert!(status.success(), "lake build failed");

This only prints "lake build failed" with no stderr. Lean compiler errors will be invisible in cargo build output. Use Command::output() instead of status() and print stderr on failure for a better dev experience.

gmp static linking portability

formal/lean-ffi/build.rs:125

println!("cargo:rustc-link-lib=static=gmp");

On many Linux distributions, libgmp is only distributed as a shared library. This will cause a link error if the static archive isn't available. Consider documenting this as a prerequisite, or checking whether a static GMP is actually needed (it may be bundled with the Lean toolchain).


Testing Gaps

No cross-implementation equivalence test

The lean-ffi feature swaps the Rust implementation for the Lean one, but there's no test that runs both and compares outputs across a representative set of inputs. A divergence (e.g., from an off-by-one in the bridge or a future Lean refactor) would silently change consensus behavior. Consider adding a test gated on #[cfg(all(feature = "lean-ffi", test))] that exhaustively checks both implementations agree for delta in 0..=1000 and a handful of large values.

Tests in lib.rs don't cover delta=6

formal/lean-ffi/src/lib.rs:1234-1238 — The test_non_justifiable list doesn't include delta=6, which is the first non-justifiable value above the <= 5 fast path. Worth adding explicitly.


Lean Code Style

Linter suppression in Bridge.lean

formal/EthLambdaProofs/Justifiability/Bridge.lean:242-243

set_option linter.style.whitespace false
set_option linter.style.show false

These suppress the standard style linters for the whole file. If the violations are minor (trailing whitespace, show in tactic proofs), it's better to fix them so the linter remains meaningful.


Architecture Observation (Non-blocking)

The separation between EthLambda (computable, no Mathlib, compiled into binary) and EthLambdaProofs (Mathlib, verification only) is the right design. It ensures the binary doesn't bloat by pulling in Mathlib. The isqrtLoop fuel parameter of 64 is well-chosen — for inputs < 2^64, the starting excess is < 2^63, which halves each Newton step, so 63 steps suffice.

The lakefile.toml defaultTargets includes EthLambdaProofs, which means lake build in the formal/ directory will compile Mathlib and all proofs by default. The build.rs correctly only builds EthLambda (passing it explicitly as an argument), not EthLambdaProofs, so CI and release builds are unaffected.


Summary

Item Severity
justifiable overflow for delta >= 2^62 (no guard in Rust layer) Medium — correctness gap vs. documented proof precondition
Stale formal/ffi/LeanFFI/Justifiability.lean path in doc Low
Missing rerun-if-changed for EthLambda.lean and config files Low — causes stale incremental builds
lake build failure gives no stderr Low — DX issue
gmp static linking may fail on some Linux distros Low — portability
No equivalence test between Lean and Rust implementations Low — regression safety
Linter suppression in Bridge.lean Nit

Automated review by Claude (Anthropic) · sonnet · custom prompt

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

🤖 Codex Code Review

  1. High: enabling lean-ffi changes consensus semantics for large u64 deltas because the Lean path wraps 4 * delta + 1 in UInt64, while the existing Rust path explicitly rejects overflow with checked_mul/checked_add. See lib.rs, lib.rs, and the proof bound at Bridge.lean. A concrete counterexample is slot=4_611_686_018_427_387_906, finalized_slot=0: the current Rust implementation returns false because 4 * delta + 1 overflows, but the Lean implementation returns true because the wrapped value is 9. That result is consensus-critical because it feeds directly into justification/finalization gating at lib.rs and lib.rs, and fork-choice target selection at store.rs. I would block on this until the Lean code mirrors the Rust overflow semantics or the API enforces the proven range before calling FFI.

  2. Medium: the Rust docs currently overclaim the proof coverage. lib.rs says the Lean implementation is “proven correct for all natural numbers”, but the bridge theorem added in Bridge.lean only proves equivalence for d < 2^62. In consensus code that is not just a documentation nit: it hides the exact boundary where Item 1 appears.

I could not run a full cargo check for the feature in this sandbox because lean/lake are not installed and Cargo cannot download toolchains here. Aside from the overflow divergence above, the rest of the PR looks mechanically straightforward.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@MegaRedHand MegaRedHand marked this pull request as ready for review April 10, 2026 21:47
@github-actions
Copy link
Copy Markdown

🤖 Kimi Code Review

This is a substantial and well-structured PR integrating formally verified Lean 4 code into the Rust consensus client. The mathematical proofs and FFI boundaries appear correct, but there are build system fragilities and CI efficiency issues to address.

Critical Issues

Build System Fragility
formal/lean-ffi/build.rs:10 uses a hardcoded relative path:

let lean_project = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../formal");

This breaks if the crate is moved or vendored. Use CARGO_WORKSPACE_DIR (available in recent Cargo) or pass the path via an environment variable set in .cargo/config.toml.

CI Efficiency & Correctness
The lean-ffi-test job (lines 157–269 in .github/workflows/ci.yml) duplicates ~60 lines of fixture generation logic from the test job. Extract this into a reusable composite action or script to prevent drift.

Also, Python 3.14 (line 184) is still pre-release. Use 3.12 or 3.13 for stability.

Missing Error Handling in Build Script
formal/lean-ffi/build.rs:20-27 panics if lean is not installed:

.expect("Failed to run `lean --print-prefix`")

For better UX, detect missing Lean and emit a descriptive error with installation instructions rather than a raw panic.

Consensus & Security

FFI Initialization Safety
In formal/lean-ffi/src/lib.rs:28-32, the initialization sequence is correct but lacks error checking:

fn init_lean() {
    INIT.call_once(|| unsafe {
        lean_initialize_runtime_module();
        let res = initialize_EthLambda_EthLambda(1);
        lean_ffi_dec_ref(res);
        lean_io_mark_end_initialization();
    });
}

Lean initialization can fail (e.g., memory exhaustion). While unlikely in practice, consider documenting that panics here are fatal and unrecoverable.

Consensus-Critical Equivalence
The justifiable_equiv theorem in formal/EthLambdaProofs/Justifiability/ImplEquivalence.lean:441 correctly bounds inputs to d < 2^62 to prevent overflow in 4*d+1. However, the Rust FFI wrapper at formal/lean-ffi/src/lib.rs:48-51 accepts any u64:

pub fn justifiable(delta: u64) -> bool {
    init_lean();
    unsafe { lean_justifiable(delta) != 0 }
}

While the Lean UInt64 arithmetic wraps on overflow, the theorem assumes d.toNat < 2^62. Inputs ≥ 2^62 violate the precondition. Add a debug assertion or saturating check:

debug_assert!(delta < (1 << 62), "justifiable: delta >= 2^62, theorem precondition violated");

Performance

Redundant Lake Builds
build.rs runs lake build unconditionally. While rerun-if-changed helps, Lake builds can be slow. Consider checking if the .lake/build/ir/EthLambda.c file exists and is newer than source files before invoking Lake.

FFI Call Overhead
The init_lean() check on every call (lines 49, 56) adds a Once::call_once barrier check. For hot paths, consider exposing a LeanRuntime handle that proves initialization has occurred, though for consensus checks this is likely negligible.

Code Quality

Lean 4 Idioms
In formal/EthLambda/Justifiability.lean:19-26, the isqrtLoop fuel argument is bounded at 64 iterations. This is correct (convergence within log2(excess) steps), but consider adding a static assertion or comment explaining why 64 is sufficient for all UInt64 inputs.

Rust Feature Propagation
The feature flag threading through ethlambdaethlambda-blockchainethlambda-state-transition is correct. However, ethlambda-lean-ffi is listed in workspace Cargo.toml but excluded from default builds/tests. Document this clearly in the crate-level README.

C Glue Code
formal/lean-ffi/src/lean_glue.c correctly exports lean_dec_ref as a non-inline symbol. This is necessary because lean.h declares it static inline, making it unavailable to Rust's linker. The implementation is correct.

Recommendations

  1. Fix build path: Use CARGO_WORKSPACE_DIR or env var for the formal directory path
  2. Deduplicate CI: Extract fixture generation into a reusable workflow
  3. Add overflow guard: Debug assertion for delta < 2^62 in the Rust wrapper
  4. Document precondition: Add rustdoc to justifiable() noting the 2^62 bound for equivalence with the proof

The formal verification work (classification, pronic detection, infinite liveness, density bounds) is mathematically rigorous and the FFI boundary is minimal and safe. The separation of EthLambda (no Mathlib) and EthLambdaProofs (Mathlib) is architecturally sound for controlling binary size.


Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

@github-actions
Copy link
Copy Markdown

🤖 Codex Code Review

  1. High: the lean-ffi path changes consensus behavior for large deltas because the pronic check no longer preserves Rust’s overflow semantics. The native implementation intentionally treats 4 * delta + 1 overflow as “not pronic” via checked_mul/checked_add in crates/blockchain/state_transition/src/lib.rs:531, but the Lean version uses wrapping UInt64 arithmetic in formal/EthLambda/Justifiability.lean:37. That gives false positives once delta > (u64::MAX - 1) / 4; for example, delta = 4611686018427387906 makes Lean compute (4 * delta + 1) mod 2^64 = 9, so the FFI path returns true while the Rust path returns false. Because crates/blockchain/state_transition/src/lib.rs:484 fully swaps fork-choice/finalization logic to the FFI result under the feature flag, this is a real consensus divergence. The Lean code needs an explicit no-overflow guard to match Rust, and this should get boundary tests around 2^62.

  2. Low: the new Lean CI gate only rejects sorry, so it does not actually enforce the “zero axioms” claim made by the PR. See ci.yml:156. For code that is being presented as formally verified and consensus-critical, I would also fail the job on axiom declarations at minimum; otherwise the security story is weaker than the docs/comments imply.

I did not find other obvious runtime issues in the non-FFI changes. I could not run the Lean build/tests locally because lean/lake are not installed in this environment, so the review is based on diff inspection.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@github-actions
Copy link
Copy Markdown

🤖 Claude Code Review

Now I have a comprehensive view of the PR. Let me write the review.


PR Review: feat: formalize slot_is_justifiable_after in Lean4

This PR introduces a Lean 4 formal verification package under formal/, adds a lean-ffi Rust crate wrapping the Lean C IR, and gates the verified implementation behind a lean-ffi feature flag. The architecture — splitting EthLambda (no Mathlib, goes in binary) from EthLambdaProofs (Mathlib, proofs only) — is well thought out and the binary size rationale is clearly documented.


Bugs / Correctness

Behavior divergence for delta >= 2^62 (undocumented)

formal/EthLambda/Justifiability.leanjustifiable:

let val := 4 * delta + 1
isqrt val ^ 2 == val && val % 2 == 1

When delta >= 2^62, 4 * delta + 1 wraps around in UInt64 (e.g., delta = 2^62val = 1, which IS a perfect square and odd, so the function returns true).

The native Rust implementation uses checked_mul/checked_add and returns false on overflow. justifiable_equiv correctly states its bound (h : d.toNat < 2 ^ 62), but the doc comment on the #[cfg(feature = "lean-ffi")] side in state_transition/src/lib.rs says "proven correct for all natural numbers" — which conflates the Nat-level Justifiable spec proofs (which do range over all ℕ) with the UInt64 implementation equivalence proof (which has the 2^62 precondition). The comment should say something like: "equivalence with the spec is proved for all inputs where delta < 2^62; for larger deltas, UInt64 wrapping is identical to the Rust behavior" (or note the divergence if they do differ).

This is purely theoretical at 4-second slots, but the documentation should match what is actually proved.


collect_c_files silently eats directory errors

formal/lean-ffi/build.rs:613-615:

let entries = match std::fs::read_dir(dir) {
    Ok(e) => e,
    Err(_) => return,
};

A missing or unreadable IR directory silently produces an incomplete static archive, which will fail at link time with a cryptic "undefined symbol" error rather than a clear build error. Err(e) => panic!("cannot read {}: {e}", dir.display()) is more debuggable.


Security / Supply Chain

elan-init.sh fetched without a hash pin

.github/workflows/ci.yml, both lean-build and lean-ffi-test jobs:

run: |
  curl https://raw.githubusercontent.com/leanprover/elan/master/elan-init.sh -sSf | sh -s -- ...

The script is fetched from master at runtime with no integrity check. A compromised or updated script would silently affect CI. For a blockchain project, pinning to a specific commit hash (verified against a known digest) or a tagged release is strongly preferred.

By contrast, the change from dtolnay/rust-toolchain@master to @stable in the same PR is a good improvement in the same direction.


Code Quality

Stale doc comment path in formal/lean-ffi/src/lib.rs:2

//! The Lean4 code at `formal/ffi/LeanFFI/Justifiability.lean` implements ...

The actual path is formal/EthLambda/Justifiability.lean. This was likely a leftover from an earlier directory layout.


Style linters suppressed rather than fixed (ImplEquivalence.lean:2-3)

set_option linter.style.whitespace false
set_option linter.style.show false

These are file-level suppressions that hide formatting issues. Even if the violations are minor, suppressing linters at the top of the longest proof file is a precedent that could spread. Better to fix the issues (usually trailing spaces or show tactic formatting) and remove these options.


Missing rerun-if-changed for Lake config files

formal/lean-ffi/build.rs:

println!("cargo:rerun-if-changed=../EthLambda");
println!("cargo:rerun-if-changed=src/lean_glue.c");

lakefile.toml and lake-manifest.json changes (e.g., bumping the Mathlib revision) won't trigger a Cargo rebuild. Adding:

println!("cargo:rerun-if-changed=../lakefile.toml");
println!("cargo:rerun-if-changed=../lake-manifest.json");

would close this gap.


CI fixture generation duplicated in lean-ffi-test job

The lean-ffi-test job duplicates ~40 lines of fixture generation from the test job. This works in practice (the cache populated by test is reused), but if the cache misses, both jobs independently regenerate fixtures. A reusable workflow or composite action would eliminate the duplication and make the cache dependency explicit.


Conventional [features] placement in crates/blockchain/Cargo.toml

The [features] section is placed before [package]. This is technically valid but unconventional; cargo fmt doesn't sort manifest sections, but [package] first is the community standard.


What's Good

  • The two-library split (EthLambda / EthLambdaProofs) with the binary-size motivation is well-documented and correct; 2 MB vs 10–250 MB is a real concern for a consensus node.
  • isqrt_correct covers all UInt64 inputs with no bound restriction — the division-based correction step (r+1 ≤ n/(r+1)) is a clean fix for the overflow that a naïve (r+1)*(r+1) ≤ n check would cause.
  • std::sync::Once for Lean runtime initialization is the right pattern; concurrent callers block safely until init completes.
  • lean_glue.c wrapping the static inline lean_dec_ref into a real linkable symbol is the correct approach for Rust FFI.
  • The convergence proof (natIsqrtLoop_correct) is solid: halving excess → 64 steps suffice for < 2^63 initial excess.
  • The --exclude ethlambda-lean-ffi in normal CI runs and Makefile test targets correctly avoids a hard Lean toolchain dependency for the standard build path.

Automated review by Claude (Anthropic) · sonnet · custom prompt

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 10, 2026

Greptile Summary

This PR introduces a Lean4 formal verification framework for the slot_is_justifiable_after function (3SF-mini justifiability check), consisting of two libraries — EthLambda (FFI-compiled, no Mathlib) and EthLambdaProofs (theorem proofs using Mathlib) — plus a Rust FFI bridge crate (ethlambda-lean-ffi) and CI jobs for lean-build and lean-ffi-test. The state-transition crate is feature-gated to swap the Rust implementation for the formally verified Lean one when lean-ffi is enabled.

Confidence Score: 5/5

Safe to merge; all findings are documentation and hardening suggestions with no impact on production correctness.

All findings are P2: an overstated doc-comment, a theoretical UInt64 overflow requiring delta >= 2^62 (billions of years of slots), a grep word-boundary nit, and a missing rerun-if-changed entry. The core Newton's-method correctness proof, FFI wiring, and CI structure are solid. No P0/P1 issues found.

formal/EthLambda/Justifiability.lean (overflow note on pronic check) and crates/blockchain/state_transition/src/lib.rs (doc comment accuracy)

Important Files Changed

Filename Overview
formal/EthLambda/Justifiability.lean Computable UInt64 implementation of justifiability check with FFI exports; the pronic detection via 4 * delta + 1 can overflow for delta >= 2^62, a gap not covered by the published proofs.
formal/EthLambdaProofs/Justifiability/ImplEquivalence.lean Thorough Newton's-method correctness proof bridging UInt64 isqrt to Nat.sqrt; justifiable_equiv correctly scoped to d < 2^62.
formal/lean-ffi/build.rs Builds Lean C IR and links Lean runtime; rerun-if-changed only covers EthLambda sources, so changes to lakefile.toml or lean-toolchain won't trigger a rebuild.
formal/lean-ffi/src/lib.rs Clean FFI bindings with Once-guarded runtime init; doc comment references incorrect path for the proofs directory.
crates/blockchain/state_transition/src/lib.rs Feature-gates slot_is_justifiable_after correctly; doc comment claims proven correct for all natural numbers but proof only holds for delta < 2^62, and references wrong directory path.
.github/workflows/ci.yml Adds lean-build and lean-ffi-test jobs; sorry grep check uses a plain string match that could produce false positives.
formal/EthLambdaProofs/Justifiability/Density.lean Proves O(sqrt(N)) density bound for justifiable deltas via Finset cardinality bounds — clean and well-structured.
formal/lean-ffi/src/lean_glue.c Minimal glue wrapping the static inline lean_dec_ref into a real symbol for Rust FFI — correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Rust: slot_is_justifiable_after"] --> B{lean-ffi feature?}
    B -- Yes --> C["ethlambda_lean_ffi crate"]
    B -- No --> D["Native Rust implementation"]
    C --> E["init_lean - once-guarded runtime init"]
    E --> F["FFI: lean_slot_is_justifiable_after"]
    F --> G["Lean: slotIsJustifiableAfter / justifiable delta"]
    G --> H{delta le 5?}
    H -- Yes --> OK["return true"]
    H -- No --> I{isqrt squared equals delta?}
    I -- Yes --> OK
    I -- No --> J["4*delta+1 odd perfect square? WARN: overflows for delta ge 2^62"]
    J -- Yes --> OK
    J -- No --> NOK["return false"]
    subgraph Lean Proofs
        K["isqrt_correct: UInt64 isqrt = Nat.sqrt"]
        L["justifiable_equiv: d lt 2^62"]
        M["justifiable_unbounded: liveness"]
        N["justifiable_density: O-sqrt-N"]
    end
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: crates/blockchain/state_transition/src/lib.rs
Line: 506-507

Comment:
**Inaccurate doc comment: wrong path and overstated coverage**

The comment says "proven correct for all natural numbers (see `formal/EthLambda/Justifiability/`)" — both claims are wrong. The proofs live in `formal/EthLambdaProofs/Justifiability/`, not in the implementation lib. More importantly, `justifiable_equiv` only holds for `d.toNat < 2^62`; for larger deltas the `4 * delta + 1` term overflows UInt64 and the pronic check can misfire (e.g., delta = 2^62 wraps to give `val = 1`, which passes both the square and odd checks, falsely returning pronic).

```suggestion
/// When the `lean-ffi` feature is enabled, this calls the formally verified
/// Lean4 implementation via FFI. The Lean implementation has been proven correct
/// for all inputs with delta < 2^62 (see `formal/EthLambdaProofs/Justifiability/`).
/// For deltas >= 2^62 (unreachable in practice) the UInt64 pronic check may overflow.
///
/// Without the feature, uses the native Rust implementation.
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: formal/EthLambda/Justifiability.lean
Line: 40-41

Comment:
**UInt64 overflow in pronic check for delta >= 2^62**

When `delta >= 2^62`, the expression `4 * delta + 1` overflows UInt64 (wraps mod 2^64). For example, `delta = 2^62` gives `val = 1`, which satisfies both `isqrt(1)^2 = 1` and `1 % 2 = 1`, so the pronic arm returns `true` even though 2^62 is not pronic. The theorem `justifiable_equiv` correctly scopes its guarantee to `d < 2^62`, but the exported `leanJustifiable` and `leanSlotIsJustifiableAfter` functions apply the unchecked algorithm to all UInt64 values.

In practice slot differences will never reach 2^62, and the `ImplEquivalence` comments acknowledge this bound, so this is informational — but worth a guard comment on the FFI exports clarifying the domain:

```lean
-- Note: correctness of the pronic check is proven only for delta < 2^62.
-- Slot differences in practice are bounded well below this limit.
@[export lean_justifiable]
def leanJustifiable (delta : UInt64) : UInt8 :=
  if justifiable delta then 1 else 0
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .github/workflows/ci.yml
Line: 155-160

Comment:
**`sorry` grep may produce false positives**

`grep -rn "sorry"` matches the substring anywhere — inside identifiers like `not_sorry`, in comments (`-- no sorry used here`), or in string literals. A false positive would block CI without any real sorry in the proof tree. Scoping to a word-boundary regex avoids this:

```suggestion
          if grep -rn "\bsorry\b" EthLambda/ EthLambdaProofs/ --include="*.lean"; then
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: formal/lean-ffi/build.rs
Line: 65-66

Comment:
**`rerun-if-changed` doesn't cover build-system files**

Changes to `formal/lean-toolchain` or `formal/lakefile.toml` change what the `lake build` step produces, but they don't live under `../EthLambda`, so Cargo won't re-run this build script when they're modified. Consider adding them:

```rust
    println!("cargo:rerun-if-changed=../EthLambda");
    println!("cargo:rerun-if-changed=../../formal/lakefile.toml");
    println!("cargo:rerun-if-changed=../../formal/lean-toolchain");
    println!("cargo:rerun-if-changed=src/lean_glue.c");
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "Merge branch 'main' into lean-formalizat..." | Re-trigger Greptile

Comment on lines +506 to +507
/// Lean4 implementation via FFI. The Lean implementation has been proven correct
/// for all natural numbers (see `formal/EthLambda/Justifiability/`).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Inaccurate doc comment: wrong path and overstated coverage

The comment says "proven correct for all natural numbers (see formal/EthLambda/Justifiability/)" — both claims are wrong. The proofs live in formal/EthLambdaProofs/Justifiability/, not in the implementation lib. More importantly, justifiable_equiv only holds for d.toNat < 2^62; for larger deltas the 4 * delta + 1 term overflows UInt64 and the pronic check can misfire (e.g., delta = 2^62 wraps to give val = 1, which passes both the square and odd checks, falsely returning pronic).

Suggested change
/// Lean4 implementation via FFI. The Lean implementation has been proven correct
/// for all natural numbers (see `formal/EthLambda/Justifiability/`).
/// When the `lean-ffi` feature is enabled, this calls the formally verified
/// Lean4 implementation via FFI. The Lean implementation has been proven correct
/// for all inputs with delta < 2^62 (see `formal/EthLambdaProofs/Justifiability/`).
/// For deltas >= 2^62 (unreachable in practice) the UInt64 pronic check may overflow.
///
/// Without the feature, uses the native Rust implementation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/blockchain/state_transition/src/lib.rs
Line: 506-507

Comment:
**Inaccurate doc comment: wrong path and overstated coverage**

The comment says "proven correct for all natural numbers (see `formal/EthLambda/Justifiability/`)" — both claims are wrong. The proofs live in `formal/EthLambdaProofs/Justifiability/`, not in the implementation lib. More importantly, `justifiable_equiv` only holds for `d.toNat < 2^62`; for larger deltas the `4 * delta + 1` term overflows UInt64 and the pronic check can misfire (e.g., delta = 2^62 wraps to give `val = 1`, which passes both the square and odd checks, falsely returning pronic).

```suggestion
/// When the `lean-ffi` feature is enabled, this calls the formally verified
/// Lean4 implementation via FFI. The Lean implementation has been proven correct
/// for all inputs with delta < 2^62 (see `formal/EthLambdaProofs/Justifiability/`).
/// For deltas >= 2^62 (unreachable in practice) the UInt64 pronic check may overflow.
///
/// Without the feature, uses the native Rust implementation.
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +40 to +41
|| (let val := 4 * delta + 1
isqrt val ^ 2 == val && val % 2 == 1)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 UInt64 overflow in pronic check for delta >= 2^62

When delta >= 2^62, the expression 4 * delta + 1 overflows UInt64 (wraps mod 2^64). For example, delta = 2^62 gives val = 1, which satisfies both isqrt(1)^2 = 1 and 1 % 2 = 1, so the pronic arm returns true even though 2^62 is not pronic. The theorem justifiable_equiv correctly scopes its guarantee to d < 2^62, but the exported leanJustifiable and leanSlotIsJustifiableAfter functions apply the unchecked algorithm to all UInt64 values.

In practice slot differences will never reach 2^62, and the ImplEquivalence comments acknowledge this bound, so this is informational — but worth a guard comment on the FFI exports clarifying the domain:

-- Note: correctness of the pronic check is proven only for delta < 2^62.
-- Slot differences in practice are bounded well below this limit.
@[export lean_justifiable]
def leanJustifiable (delta : UInt64) : UInt8 :=
  if justifiable delta then 1 else 0
Prompt To Fix With AI
This is a comment left during a code review.
Path: formal/EthLambda/Justifiability.lean
Line: 40-41

Comment:
**UInt64 overflow in pronic check for delta >= 2^62**

When `delta >= 2^62`, the expression `4 * delta + 1` overflows UInt64 (wraps mod 2^64). For example, `delta = 2^62` gives `val = 1`, which satisfies both `isqrt(1)^2 = 1` and `1 % 2 = 1`, so the pronic arm returns `true` even though 2^62 is not pronic. The theorem `justifiable_equiv` correctly scopes its guarantee to `d < 2^62`, but the exported `leanJustifiable` and `leanSlotIsJustifiableAfter` functions apply the unchecked algorithm to all UInt64 values.

In practice slot differences will never reach 2^62, and the `ImplEquivalence` comments acknowledge this bound, so this is informational — but worth a guard comment on the FFI exports clarifying the domain:

```lean
-- Note: correctness of the pronic check is proven only for delta < 2^62.
-- Slot differences in practice are bounded well below this limit.
@[export lean_justifiable]
def leanJustifiable (delta : UInt64) : UInt8 :=
  if justifiable delta then 1 else 0
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +155 to +160

- name: Verify zero sorries
working-directory: formal
run: |
if grep -rn "sorry" EthLambda/ EthLambdaProofs/ --include="*.lean"; then
echo "::error::Found sorry in Lean sources"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 sorry grep may produce false positives

grep -rn "sorry" matches the substring anywhere — inside identifiers like not_sorry, in comments (-- no sorry used here), or in string literals. A false positive would block CI without any real sorry in the proof tree. Scoping to a word-boundary regex avoids this:

Suggested change
- name: Verify zero sorries
working-directory: formal
run: |
if grep -rn "sorry" EthLambda/ EthLambdaProofs/ --include="*.lean"; then
echo "::error::Found sorry in Lean sources"
if grep -rn "\bsorry\b" EthLambda/ EthLambdaProofs/ --include="*.lean"; then
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/ci.yml
Line: 155-160

Comment:
**`sorry` grep may produce false positives**

`grep -rn "sorry"` matches the substring anywhere — inside identifiers like `not_sorry`, in comments (`-- no sorry used here`), or in string literals. A false positive would block CI without any real sorry in the proof tree. Scoping to a word-boundary regex avoids this:

```suggestion
          if grep -rn "\bsorry\b" EthLambda/ EthLambdaProofs/ --include="*.lean"; then
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +65 to +66
println!("cargo:rerun-if-changed=../EthLambda");
println!("cargo:rerun-if-changed=src/lean_glue.c");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 rerun-if-changed doesn't cover build-system files

Changes to formal/lean-toolchain or formal/lakefile.toml change what the lake build step produces, but they don't live under ../EthLambda, so Cargo won't re-run this build script when they're modified. Consider adding them:

    println!("cargo:rerun-if-changed=../EthLambda");
    println!("cargo:rerun-if-changed=../../formal/lakefile.toml");
    println!("cargo:rerun-if-changed=../../formal/lean-toolchain");
    println!("cargo:rerun-if-changed=src/lean_glue.c");
Prompt To Fix With AI
This is a comment left during a code review.
Path: formal/lean-ffi/build.rs
Line: 65-66

Comment:
**`rerun-if-changed` doesn't cover build-system files**

Changes to `formal/lean-toolchain` or `formal/lakefile.toml` change what the `lake build` step produces, but they don't live under `../EthLambda`, so Cargo won't re-run this build script when they're modified. Consider adding them:

```rust
    println!("cargo:rerun-if-changed=../EthLambda");
    println!("cargo:rerun-if-changed=../../formal/lakefile.toml");
    println!("cargo:rerun-if-changed=../../formal/lean-toolchain");
    println!("cargo:rerun-if-changed=src/lean_glue.c");
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +40 to +46
/// Check if `delta` is justifiable under 3SF-mini rules.
///
/// Calls the formally verified Lean4 implementation via FFI.
pub fn justifiable(delta: u64) -> bool {
init_lean();
unsafe { lean_justifiable(delta) != 0 }
}
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we need this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant