test: e2e sp1 proof defense - #734
Conversation
Benchmark ResultsCompared on the same runner in the same workflow run.
|
c17eee3 to
ec848f5
Compare
* chore: allow custom image tags (#739) * fix: optimize witness collection
…in/world-chain into osiris/e2e-defender-test
alessandromazza98
left a comment
alessandromazza98
left a comment
There was a problem hiding this comment.
Generally the e2e tests look good to me, I left some comments. I think you can split this PR into more than once, at least one where you add the sp1 verifier contract first.
| } | ||
| } | ||
|
|
||
| function hasThreshold(uint8 bitmap) internal pure returns (bool) { |
There was a problem hiding this comment.
you can remove the PROOF_THRESHOLD const then (line 5)
| /// @notice Verifies an SP1 Groth16 aggregation proof and binds its committed | ||
| /// outputs to the challenged game's proposal. Implements the validity-proof lane | ||
| /// of [`IWorldChainProofVerifier`]. | ||
| contract SP1ValidityVerifier is IWorldChainProofVerifier { |
There was a problem hiding this comment.
I'd create a PR that only includes this Sp1ValidityVerifier contract first because this contract is critical for the proof-system. Then create the e2e tests PR on top
| /// worker exists to service the `Nitro` backend queue. | ||
| pub(crate) const DEFENDED_LANE_COUNT: usize = 1; | ||
|
|
||
| /// The proof lanes the defender drives, paired with the prover-service | ||
| /// backend that generates each proof. | ||
| pub(crate) const DEFENDED_LANES: [(ProofLane, ProofBackend); DEFENDED_LANE_COUNT] = [ | ||
| (ProofLane::ValidityProof, ProofBackend::Sp1), | ||
| (ProofLane::TeeAttestation, ProofBackend::Nitro), | ||
| ]; | ||
| pub(crate) const DEFENDED_LANES: [(ProofLane, ProofBackend); DEFENDED_LANE_COUNT] = | ||
| [(ProofLane::ValidityProof, ProofBackend::Sp1)]; |
There was a problem hiding this comment.
can you add a TODO in both const so that we remember to change them once nitro worker exists ?
|
|
||
| ## 8. Full end-to-end sequence | ||
|
|
||
| ```mermaid |
There was a problem hiding this comment.
I see an error here:
Parse error on line 39:
...n → defense complete
-----------------------^
Expecting '()', 'SOLID_OPEN_ARROW', 'DOTTED_OPEN_ARROW', 'SOLID_ARROW', 'SOLID_ARROW_TOP', 'SOLID_ARROW_BOTTOM', 'STICK_ARROW_TOP', 'STICK_ARROW_BOTTOM', 'SOLID_ARROW_TOP_DOTTED', 'SOLID_ARROW_BOTTOM_DOTTED', 'STICK_ARROW_TOP_DOTTED', 'STICK_ARROW_BOTTOM_DOTTED', 'SOLID_ARROW_TOP_REVERSE', 'SOLID_ARROW_BOTTOM_REVERSE', 'STICK_ARROW_TOP_REVERSE', 'STICK_ARROW_BOTTOM_REVERSE', 'SOLID_ARROW_TOP_REVERSE_DOTTED', 'SOLID_ARROW_BOTTOM_REVERSE_DOTTED', 'STICK_ARROW_TOP_REVERSE_DOTTED', 'STICK_ARROW_BOTTOM_REVERSE_DOTTED', 'BIDIRECTIONAL_SOLID_ARROW', 'DOTTED_ARROW', 'BIDIRECTIONAL_DOTTED_ARROW', 'SOLID_CROSS', 'DOTTED_CROSS', 'SOLID_POINT', 'DOTTED_POINT', got 'NEWLINE'
| @@ -0,0 +1,252 @@ | |||
| //! End-to-end test: the defender finalizes a challenged-but-valid game with a real SP1 | |||
| //! Groth16 validity proof. | |||
There was a problem hiding this comment.
further, what do you think of putting e2e tests for the proof-system inside the proofs folder such as:
/proofs
|
|-- /e2e
This way we have both e2e and integration tests for proof-system inside the proofs folder. See #744 for integration tests that I'm adding
|
for tracking, I created the following issues on linear: |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
❌ 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 1aef7fa. Configure here.
| 2026-06-15T18:03:53.222368Z WARN op_batcher: msg="publishSignal channel is full, skipping signal" process=op-batcher | ||
| 2026-06-15T18:03:54.218118Z WARN op_batcher: msg="publishSignal channel is full, skipping signal" process=op-batcher | ||
| 2026-06-15T18:03:55.228298Z WARN op_batcher: msg="publishSignal channel is full, skipping signal" process=op-batcher | ||
| 2026-06-15T18:03:56.218412Z WARN op_batcher: msg="publishSignal channel is full, skipping signal" process=op-batcher |
There was a problem hiding this comment.
Debug panic log committed
Low Severity
A full panic.log with local stack traces and test run output is added at the repository root. It looks like a developer debug artifact from a failed defender e2e run, not production or test source code.
Reviewed by Cursor Bugbot for commit 1aef7fa. Configure here.
| "proofs/proposer", | ||
| "proofs/challenger", | ||
| "proofs/prover-service", | ||
| "proofs/defender", |
There was a problem hiding this comment.
Duplicate workspace member entry
Low Severity
proofs/defender appears twice in the workspace members list. The duplicate entry is redundant and can confuse workspace maintenance or tooling that expects unique paths.
Reviewed by Cursor Bugbot for commit 1aef7fa. Configure here.
|
|
||
| function hasThreshold(uint8 bitmap) internal pure returns (bool) { | ||
| return proofCount(bitmap) >= PROOF_THRESHOLD; | ||
| function hasThreshold(uint8 bitmap, uint8 threshold) internal pure returns (bool) { |
There was a problem hiding this comment.
Unused proof threshold constant
Low Severity
WorldChainProofLib.PROOF_THRESHOLD is no longer used after hasThreshold takes a threshold argument and games use proofThreshold immutables. The constant is dead code left behind by this refactor.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1aef7fa. Configure here.
0xForerunner
left a comment
0xForerunner
left a comment
There was a problem hiding this comment.
We should have some solidity experts take a look at this before merging as well.
There was a problem hiding this comment.
This file is mistakenly committed.
| p.rollupConfigHash = vm.envBytes32("ROLLUP_CONFIG_HASH"); | ||
| p.blockInterval = vm.envOr("PROOF_SYSTEM_BLOCK_INTERVAL", uint256(10)); | ||
| p.intermediateBlockInterval = vm.envOr("PROOF_SYSTEM_INTERMEDIATE_BLOCK_INTERVAL", uint256(5)); | ||
| p.proofThreshold = uint8(vm.envOr("PROOF_SYSTEM_THRESHOLD", uint256(1))); |
There was a problem hiding this comment.
We should keep this default at 2.
There was a problem hiding this comment.
We just only have 1 available prover atm, but yes it will be
|
I'd still suggest to split this PR into at least 2: #734 (comment) And to eventually change folder structure as: #734 (comment) |
3 participants
Note
High Risk
Touches dispute finalization contracts, L1 devnet topology, and real SP1 proving; misconfiguration can break proof deploy or leave challenged games stuck. The committed
panic.loglooks like debug output and should probably be dropped before merge.Overview
Adds an ignored, opt-in e2e test (
defender_finalizes_challenged_game_with_sp1_proof) that runs the HA devnet withDEVNET_SP1_WORKER_PROVER, stakes a griefer to challenge a valid game, and waits for the in-process defender + SP1 worker to post a real Groth16 proof and finalize withproofThreshold = 1. Ajust defender-e2erecipe andDEFENDER_LOG/PROVING_LOGcargo env presets support running it.The full-stack proof devnet switches L1 from anvil to reth + Lighthouse so kona witness collection gets
debug_*L1 data and a real beacon API; op-node/op-challenger/SP1 worker wiring uses separate L1 EL and beacon URLs, L2--rpc.eth-proof-window, delayed proof-system deploy (after L2 is healthy), standalone SP1 Groth16 verifier deploy, vkey computation, and richer L1 genesis funding. Proving ranges shrink to 1-block intervals when SP1 is enabled.On-chain: new
SP1ValidityVerifier, factory/gameproofThreshold(replacing a fixed lib constant), deploy script env for real SP1; sp1-contracts submodule. Defender submits lane calldata as ABI(bytes, bytes)for SP1/Nitro and documents the flow; defended lanes are SP1-only for this devnet path.Also adds
world-chain-defenderto the workspace members list and host-utils deps (kona-providers-alloy, L1 config path) for witness building.Reviewed by Cursor Bugbot for commit 1aef7fa. Configure here.