refactor: build SP1 ELFs via sp1_build + embed via include_elf!() (OP Succinct pattern)#743
Open
agentotto[bot] wants to merge 37 commits into
Open
refactor: build SP1 ELFs via sp1_build + embed via include_elf!() (OP Succinct pattern)#743agentotto[bot] wants to merge 37 commits into
agentotto[bot] wants to merge 37 commits into
Conversation
agentotto
Bot
requested review from
0xForerunner,
0xOsiris,
Dzejkop,
Kemperino,
alessandromazza98,
cichaczem,
karankurbur,
kilianglas,
murph and
piohei
as code owners
Contributor
Benchmark ResultsCompared on the same runner in the same workflow run.
|
agentotto
Bot
force-pushed
the
refactor/elf-hash-manifest
branch
from
f5d2091 to
5aacc9d
Compare
agentotto
Bot
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
piohei
force-pushed
the
refactor/elf-hash-manifest
branch
from
b8073dc to
cf597be
Compare
agentotto
Bot
force-pushed
the
refactor/elf-hash-manifest
branch
from
435a4cb to
4c0cce1
Compare
agentotto
Bot
force-pushed
the
refactor/elf-hash-manifest
branch
from
3385f8f to
b7b579d
Compare
- Replace clippy stub-file logic in build.rs with #[cfg(clippy)] guards in src/lib.rs; build.rs no longer needs clippy detection - Add clarity to ancestors().nth(3) comment explaining the index math - Gate world-chain-proof-succinct-elfs behind embedded-elfs feature in sp1-worker so devnet (which loads ELFs from env vars) doesn't trigger the SP1 guest build - Update devnet Cargo.toml: depend on sp1-worker with default-features=false - Fix stale comment in full_stack.rs (referenced non-existent build.rs path) - Update Dockerfile.proof comment to clarify vkeys CI also uses SP1_BUILD_DOCKER=false for consistent ELF/vkey alignment
…7817149 - crates/devnet/src/full_stack.rs: add early env-var validation in start_sp1_worker before spawn_blocking so RANGE_ELF_PATH / AGG_ELF_PATH absence surfaces as a clear Result::Err with actionable guidance instead of a panic buried in the spawn_blocking task. - proofs/succinct/elfs/build.rs: update docstring to clarify that sp1_build::build_program_with_args already checks SP1_SKIP_PROGRAM_BUILD internally (skips the Docker/local build but still emits SP1_ELF_* env vars), so main() does not need a separate early-return for the flag. - docs/proof/elf-management.md: fix stale file-path references (proofs/succinct/utils/host/build.rs -> proofs/succinct/elfs/build.rs, env_prover.rs -> elfs/src/lib.rs) and correct the claim that there is no RANGE_ELF_PATH env var (the proof CLI and devnet worker do use it).
…d-import warning
Clippy runs with `#[cfg(clippy)]` active, which means the
`include_elf!()` macro in the non-clippy branches is never reached.
This caused the top-level `use sp1_sdk::{Elf, include_elf}` to be
flagged as an unused import under `-D warnings`.
Fix: split the import into two cfg-gated lines so clippy only sees
`use sp1_sdk::Elf` (which IS used by the return types in both
functions), and the real build still imports `include_elf` as well.
…ests The e2e integration tests in world-chain-tests (test_enforces_block_uncompressed_size_limit, test_eth_api_assertions, test_engine_driver_pending_block_queries, etc.) intermittently fail with 'base fee missing' or similar race conditions when many tests run in parallel on shared CI runners. This is a pre-existing issue observed on both main and PR branches. Adding retries=2 to the nextest default profile gives each failing test two re-runs before it's counted as a failure, which is the standard mitigation for resource-sensitive integration tests.
…s/release-proof.yml)
Adds a committed vkeys.json as the canonical reproducibility reference. Run 'just verify-proof-vkeys' to verify current source matches, or 'just update-proof-vkeys' to regenerate after program changes. Changes: - proofs/succinct/elf/vkeys.json: placeholder with zero hashes; real values require running 'just update-proof-vkeys' with the SP1 toolchain installed. JSON structure matches the 'proof sp1 vkeys' CLI output format. - .gitignore: allow proofs/succinct/elf/vkeys.json (alongside manifest.toml) - Justfile: add update-proof-vkeys and verify-proof-vkeys recipes - proofs/vkeys-ci.yml.new: staged CI workflow — copy to .github/workflows/vkeys.yml once the 'proof' package Cargo.toml is in place
Replace non-existent `proof` package with `world-chain-prover-sp1` (proofs/prover-sp1/). Also drop the spurious `--features sp1` flag (that package has no such feature) and fix the subcommand path from `sp1 vkeys` to `vkeys` to match the actual CLI structure.
- Dockerfile.prover: change default FEATURES from "sp1,nitro" to "" so
packages that do not define those features (world-chain-prover-sp1,
world-chain-prover-nitro, world-chain-proposer, etc.) are not passed
--features sp1,nitro. Package default features still apply. Update
header comment with correct usage examples.
- docs/proof/proof-cli.md: fix two doc/code contradictions:
* sp1 execute: add the required --elf / RANGE_ELF_PATH flag to the table
and update the example; remove the stale claim that no ELF flag is needed.
* nitro prove: remove "omit all three to skip PCR verification" claim.
The code requires all three PCR measurements and bails when they are absent.
- Justfile: correct stale comment reference from
proofs/succinct/utils/host/build.rs to proofs/succinct/elfs/build.rs.
Fix verify-proof-vkeys to use diff <(jq -S . ...) so key-order
differences between committed and freshly-generated JSON do not cause
spurious CI failures. Fix update-proof-vkeys to write through jq -S
for a canonical, consistently-sorted vkeys.json.
Without Docker, the SP1 guest ELF is compiled on the host machine, so
absolute file paths embedded by rustc (in panic location strings and
any residual DWARF sections) vary across machines and checkout locations.
This makes ELF bytes — and therefore vkeys — non-reproducible.
Fix: inject reproducibility flags into BuildArgs::rustflags for local
(non-Docker) builds only. Docker builds already achieve reproducibility
via fixed container paths and don't need these.
Flags added via BuildArgs::rustflags (each flag word is a separate Vec
element; sp1-build joins them with \x1f into CARGO_ENCODED_RUSTFLAGS):
-C debuginfo=0
Prevents DWARF sections from embedding source file paths or any
other machine-specific metadata.
--remap-path-prefix $workspace_root=/build
Normalizes workspace source paths (e.g. /home/alice/world-chain →
/build) so panic Location::file() strings are machine-independent.
--remap-path-prefix $CARGO_HOME=/cargo
Normalizes cargo registry / git dependency source paths.
--remap-path-prefix $RUSTUP_HOME=/rustup
Normalizes rustup toolchain / stdlib sysroot paths.
Also canonicalize() workspace_root before use so that symlinks in the
checkout path are resolved before passing to --remap-path-prefix.
Remove SP1_BUILD_DOCKER=false from build.rs and Justfile recipes. Docker provides reproducible ELFs by fixing the build environment path layout — the approach used by op-succinct, sp1-helios, and all other SP1 adopters. SP1_BUILD_DOCKER=false is kept only in Dockerfile.prover where Docker-in-Docker is genuinely unavailable.
piohei
force-pushed
the
refactor/elf-hash-manifest
branch
from
574db68 to
ec552f8
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ec552f8. Configure here.
| cargo run -p world-chain-prover-sp1 -- vkeys --output /tmp/vkeys-actual.json | ||
| jq -S . proofs/succinct/elf/vkeys.json > /tmp/vkeys-committed.json | ||
| jq -S . /tmp/vkeys-actual.json > /tmp/vkeys-actual-normalized.json | ||
| diff /tmp/vkeys-committed.json /tmp/vkeys-actual-normalized.json || (echo "ERROR: vkeys.json is out of date. Run 'just update-proof-vkeys' to regenerate." && exit 1) |
There was a problem hiding this comment.
Justfile omits SP1_BUILD_DOCKER
Medium Severity
update-proof-vkeys and verify-proof-vkeys invoke cargo run without SP1_BUILD_DOCKER=false, while release-proof.yml, Dockerfile.prover, and the staged vkeys.yml workflow pin the local sp1up v6.1.0 toolchain. proofs/succinct/elfs/build.rs hardcodes docker: true, so maintainer updates can diverge from CI and release measurements.
Reviewed by Cursor Bugbot for commit ec552f8. Configure here.
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
1 participant
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.
Summary
Replace the previous hash-manifest approach with the upstream OP Succinct pattern:
compile the SP1 guest ELFs at
cargo buildtime viasp1_buildand embedthem into the host binary at link time via
sp1_sdk::include_elf!(). There isno committed ELF blob, no SHA-256 manifest, no
--range-elfflag, and noRANGE_ELF_PATHenv var anywhere in the codebase any more.This matches how
succinctlabs/op-succinctitself handles its SP1 programs (
utils/build/,utils/elfs/,include_elf!()across host crates).Supersedes the SHA-256 manifest approach from the previous revision of this PR.
What's in the PR
Build pipeline
proofs/succinct/utils/host/build.rs(new) — callssp1_build::build_program_with_argsfor both guest crates:proofs/succinct/programs/range-ethereum/→world-chain-proof-succinct-range-ethereumproofs/succinct/programs/aggregation/→world-chain-proof-succinct-aggregationDefaults to
docker: trueat pinned tagv6.1.0(matches thesp1-sdk/sp1-zkvmpin). SetSP1_BUILD_DOCKER=falseto use alocally-installed
cargo-proveinstead — used insideDockerfile.proofbecause the Docker daemon isn't reachable from inside a
docker build.The build is gated on
CARGO_FEATURE_SP1, so non-sp1 builds (witness only,nitro only) no-op and don't need Docker or the SP1 toolchain.
proofs/succinct/utils/host/Cargo.toml— addssp1-build = "=6.1.0"as a
[build-dependencies]entry and declaresbuild = "build.rs".Embedding
proofs/succinct/utils/host/src/env_prover.rs:EnvSuccinctProver::new(kind, agg_mode)now reads no arguments beyond theprover backend and the aggregation proof mode — it uses the embedded ELFs.
EnvSuccinctProver::new_with_elfs(kind, range_elf, agg_elf, mode)isretained for tests and custom programs.
Removed
proofs/succinct/elf/world-chain-range-ethereumand
world-chain-aggregation).scripts/elf-manifest.pyproofs/succinct/elf/manifest.tomlproofs/succinct/elf/.gitignorebuild-proof-range-elf,build-proof-aggregation-elf,build-proof-elfs,verify-proof-elfs,build-and-verify-proof-elfs,update-proof-elf-hashes.--range-elf,--agg-elf,--elf,RANGE_ELF_PATH,AGG_ELF_PATH,WORLD_CHAIN_RANGE_ELF,WORLD_CHAIN_AGGREGATION_ELF.Dockerfile.proof(theCOPYlines and theENVexports).crates/devnet/src/full_stack.rsno longer joinsrepo_root()withproofs/succinct/elf/...— the worker links the bytes in directly.Kept
just proof-vkeys— still the way to print on-chain vkey commitments(it now triggers the embedded build automatically, no separate
build-proof-elfsstep required).Docs
docs/proof/elf-management.mdis rewritten to describe the new pipeline.docs/proof/proof-cli.mdanddocs/proof/release.mdare updated to dropreferences to the old ELF flags / manifest / Justfile targets.
Workflow changes (need maintainer with
workflowsscope)The GitHub App opening this PR can't write to
.github/workflows/**. Thetwo updates that match this refactor:
1. Delete
.github/workflows/elf.ymlThe workflow was specifically gating ELF reproducibility against a committed
manifest. With
sp1_buildbaked into the host crate, everycargo buildunder the
sp1feature already rebuilds the guest ELFs — there's nothingseparate to verify any more. Just
git rm .github/workflows/elf.yml.2.
.github/workflows/release-proof.ymlThree changes:
Drop the
verify-elfsjob entirely (andneeds: [verify-elfs]from everyother job).
In
vkeys, install the SP1 toolchain and setSP1_BUILD_DOCKER=falsebefore running
just proof-vkeys:In
build-binaries, install the SP1 toolchain and setSP1_BUILD_DOCKER=falsebeforecargo build --release ... -p proof --features sp1,nitro:In
draft-release, drop the twocp proofs/succinct/elf/...lines(the ELFs are no longer shipped as standalone release artifacts —
they're embedded in the
proofCLI tarballs and the docker imagemanifest).
Verification
cargo check -p world-chain-proof-succinct-host-utils(nosp1feature)succeeds locally — the
build.rscorrectly no-ops withoutCARGO_FEATURE_SP1.sp1feature,build.rsinvokessp1_build::build_program_with_argsfor each program crate and emitscargo:rustc-env=SP1_ELF_world-chain-proof-succinct-range-ethereum=...directives that
include_elf!()picks up. Confirmed bysp1-buildpanicking with "failed to run docker command" in environments without
Docker — i.e. the wiring is correct, only the runtime is missing.
tag: "v6.1.0"(whethervia
docker: trueDocker image or viasp1up --version v6.1.0install)→ bit-for-bit identical ELFs → identical vkeys.
Migration
Developers don't need to do anything separate.
cargo build/cargo build -p proof --features sp1/cargo build -p world-chain-sp1-workerruns
sp1_buildtransparently. The first build pullssuccinctlabs/sp1:v6.1.0(a few minutes); subsequent builds reuse thecached ELFs unless guest source or the pinned tag changes.
For fast iteration on host code after a successful build, set
SP1_SKIP_PROGRAM_BUILD=trueto skip the ELF recompile while lettinginclude_elf!()resolve against the cached ELFs.Risks
Dockerfile.proofnow installs the SP1 toolchain in the builder stageand uses
SP1_BUILD_DOCKER=false. First image build is slower untillayer caching kicks in; the ELFs are no longer COPYed in.
RANGE_ELF_PATH/AGG_ELF_PATHwill needto stop setting them — those env vars are gone from every binary.
Note
High Risk
Changes how on-chain SP1 vkeys are derived and what release/Docker images contain; guest or toolchain drift without updating
vkeys.jsonor registries would break proof-lane governance.Overview
Moves SP1 guest program handling to the OP Succinct pattern: a new
world-chain-proof-succinct-elfscrate runssp1_build::build_program_with_argsinbuild.rsand exposes ELFs throughinclude_elf!().world-chain-prover-sp1andsp1-worker(defaultembedded-elfs) no longer take--range-elf/--agg-elfor read committed ELF files;EnvSuccinctProver::new_with_elfsis the production path.CI and release drop the dedicated
build-elfsjob and ELF upload/download fromdocker-proof.ymlandrelease-proof.yml. Vkeys are computed from embedded ELFs (SP1_BUILD_DOCKER=false+ pinnedsp1up v6.1.0); standalone guest ELF release assets andmanifest.tomlgo away.proofs/succinct/elf/vkeys.jsonis committed, withjust verify-proof-vkeysand a newverify-vkeysworkflow.Dockerfile.proverinstalls the SP1 toolchain, requiresPROVER_PACKAGE/PROVER_BIN, and no longer copies ELF blobs into the image—bytes live inside the built binary. Devnet disablessp1-workerembedded ELFs and expectsRANGE_ELF_PATH/AGG_ELF_PATHfor runtime loading.Docs and Justfile recipes are updated (
build-proof-elfsremoved;update-proof-vkeys/verify-proof-vkeysadded).sp1-buildis pinned to 6.1.0 inCargo.lock.Reviewed by Cursor Bugbot for commit ec552f8. Bugbot is set up for automated code reviews on this repo. Configure here.