test(proof): combined n-version groundwork for zeronet pin - #4459
test(proof): combined n-version groundwork for zeronet pin#44590xth4nh wants to merge 14 commits into
Conversation
Adds the read-only half of N-version proof routing: the commitments that determine which prover artifacts can satisfy a given dispute game, and a way to read them off a historical game proxy. `ProofProtocolDescriptor` reads CONFIG_HASH, TEE_IMAGE_HASH, ZK_RANGE_HASH, ZK_AGGREGATE_HASH and the journal schedule era, and hashes them under a domain tag into a canonical fingerprint. Both schedule-aware eras expose scheduleId, so the era is classified on which getters exist rather than on the value returned — zero is valid for both. All six getters issue in one RPC round. `basectl proofs protocol` prints those descriptors and groups games by fingerprint, so `--proof-protocol-version <fingerprint>=<version>` mappings can be written from observed chain state instead of guessed. Nothing consumes the fingerprint yet; routing lands on top of this. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Codex <codex-noreply@coinbase.com>
Co-authored-by: Codex <codex-noreply@coinbase.com>
Co-authored-by: Codex <codex-noreply@coinbase.com>
Schedule and CONFIG_HASH are not routing keys: the guest derives ScheduleID from the CL oracle, and one prover-service per network already isolates chains. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Core of N-version routing. Every piece is inert until a second version is configured: requests default to version 0, workers announce [0], and every fingerprint maps to one version, so behaviour is unchanged on deploy. - `protocol_version` on the request, persisted as `request_protocol_version` (migration 018). Both claim queries match `= ANY($versions)` inside the existing atomic claim, so a worker cannot take a job it did not announce. - Workers announce a version list through shared job discovery; absent or empty means [0], so a pre-versioning binary keeps working. - The challenger maps each game's capability fingerprint to a version and fails closed on an unmapped fingerprint or a full-schedule-era game, each behind its own counter. - Migration 018 builds its replacement indexes before dropping the originals, so claim queries never run uncovered. - Gauges for pending jobs and open games by version. Nitro image selection is deliberately absent: `ProofRequest.image_hash` does not exist on main, and restoring it belongs with the change that reads it. `CandidateGame.tee_image_hash` already carries the value for that PR to wire. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Codex <codex-noreply@coinbase.com>
Fingerprint no longer differs by schedule kind, so Activated-era tests claim version 0. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
zk-host takes `PROVER_PROTOCOL_VERSION` (default 0) and announces exactly that version through job discovery, so a fleet claims only jobs whose games its embedded range and aggregation ELFs can satisfy. One version per deployment, unlike Nitro's list. A ZK fleet is a thin claimer in front of a shared SP1 cluster, so serving another version means another Rollout rather than another artifact in the same process. The chart renders one per entry in `workers[]`. Also resyncs the SP1 guest lockfile with the root workspace. It resolved alloy-rpc-types-engine 2.0.5 while root resolved 2.2.0, so the guest failed to build against `consensus/derive`'s `target_gas_limit` (E0560). Pre-existing on main and invisible to CI, which compiles against stub ELFs; only the zk-host image build touches the real ones. Pinned each shared crate to the version root already resolves rather than running a blanket update, which overshot to alloy 2.4.0 and diverged the other way. NOTE: the lockfile change alters the ELF bytes, and therefore ZK_RANGE_HASH and ZK_AGGREGATE_HASH. The deployed AggregateVerifier must commit to the new keys, and those hashes feed the capability fingerprint this stack routes on. No previously-buildable ELF is replaced: the guest workspace does not compile at this commit without the fix. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Restores `ProofRequest.image_hash` and makes the Nitro pool honour it, so a host holding enclaves of more than one image proves each job on the enclave that game can accept. #4321 removed the field as having "no production reader". This is that reader: the challenger sets it from the disputed game's own `TEE_IMAGE_HASH`, and `select_enclaves_for_image` picks a registered enclave whose on-chain `signerImageHash` matches, failing the job when none does. That mirrors `TEEVerifier.verify`, which compares the same pair — without it a host spends a full proof run before the mismatch returns as an on-chain revert. Zero means "no expectation" and matches any registered enclave. Proposals take that path: they create games at the current image, so they pin nothing. Only the challenger, disputing games created under possibly-retired images, names one. Also: - the claim gate moves from `is_valid_signer` to `is_registered_signer`. The former compares against the factory's *current* image, so it would reject a still-registered old-image signer — precisely the one an old game needs. - nitro-host takes a comma-separated `--protocol-version` list, because one enclave image can satisfy several versions: the fingerprint mixes TEE and ZK commitments, so a ZK-only rotation mints a version for an unchanged image. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Codex <codex-noreply@coinbase.com>
Co-authored-by: Codex <codex-noreply@coinbase.com>
Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
🟡 Heimdall Review Status
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| r#" | ||
| SELECT request_protocol_version, | ||
| COUNT(*) FILTER (WHERE job_status = 'PENDING') AS pending_count | ||
| FROM proof_requests | ||
| GROUP BY request_protocol_version | ||
| "#, |
There was a problem hiding this comment.
Concern (performance): This query scans the entire proof_requests table (no WHERE clause) on every status-poller tick just to produce a few gauge values. As the table grows with historical completed/failed rows, this will become increasingly expensive.
Consider adding a WHERE job_status = 'PENDING' filter so the query only scans pending rows, which should be a small fraction of the table. The GROUP BY would still work correctly, and the existing idx_proof_requests_job_claim_by_version index (which has job_status as the leading column) could help.
SELECT request_protocol_version,
COUNT(*) AS pending_count
FROM proof_requests
WHERE job_status = 'PENDING'
GROUP BY request_protocol_versionThis also avoids the somewhat odd current behavior of returning rows for completed versions with pending_count = 0, which the gauge publishing code then sets to zero — the same result as not emitting the gauge at all.
| async { | ||
| contract_call!( | ||
| contract.ZK_AGGREGATE_HASH().call(), | ||
| "ZK_AGGREGATE_HASH failed" | ||
| ) | ||
| }, | ||
| ) | ||
| }, | ||
| async { contract.scheduleId().call().await }, | ||
| async { contract.L2_BLOCK_TIME().call().await }, | ||
| ); | ||
| let (config_hash, tee_image_hash, zk_range_hash, zk_aggregate_hash) = hashes?; | ||
|
|
||
| let (schedule_kind, schedule_id) = match schedule_id { | ||
| // Both schedule-aware eras expose `scheduleId`; only the activated-prefix era also | ||
| // exposes the L2 timestamp anchors it needs. Classify on which getters exist, never | ||
| // on the returned value: zero is valid for both. | ||
| Ok(schedule_id) => match l2_block_time { | ||
| Ok(_) => (ProofScheduleKind::Activated, B256::ZERO), | ||
| Err(error) if Self::is_missing_getter(&error) => { | ||
| (ProofScheduleKind::Full, schedule_id) | ||
| } | ||
| Err(error) => return Err(ContractError::call("L2_BLOCK_TIME failed", error)), | ||
| }, | ||
| Err(error) if Self::is_missing_getter(&error) => (ProofScheduleKind::None, B256::ZERO), | ||
| Err(error) => return Err(ContractError::call("scheduleId failed", error)), | ||
| }; | ||
|
|
||
| Ok(ProofProtocolDescriptor { | ||
| schedule_kind, | ||
| schedule_id, | ||
| config_hash, | ||
| tee_image_hash, | ||
| zk_range_hash, | ||
| zk_aggregate_hash, | ||
| }) | ||
| } | ||
|
|
||
| async fn zk_prover(&self, game_address: Address) -> Result<Address, ContractError> { | ||
| let contract = | ||
| IAggregateVerifier::IAggregateVerifierInstance::new(game_address, &self.provider); |
There was a problem hiding this comment.
Concern (correctness): The era classification logic relies on string-matching RPC error messages to distinguish "this getter doesn't exist on the contract" from other errors. The is_missing_getter check below does to_ascii_lowercase().contains("execution reverted"), which works for Geth and Reth but may not match error messages from other EVM clients (Erigon, Besu, Nethermind) — a limitation already acknowledged elsewhere in the codebase (see crates/utilities/tx-manager/src/error.rs).
The defensive fallback (treating unrecognized errors as real failures and retrying on the next scan) is the right behavior and is well-documented. Just calling this out as worth tracking: if the L1 node is ever switched to a non-Geth-derived client, this detection could silently classify all games as erroring instead of properly detecting their era, causing the scanner to skip every game on every tick until someone notices the unmapped_fingerprint_games_total counter.
| let mut open_games: HashMap<u32, usize> = | ||
| self.protocol_versions.values().map(|&version| (version, 0)).collect(); | ||
| for candidate in &candidates { | ||
| *open_games.entry(candidate.protocol_version).or_default() += 1; | ||
| } | ||
| for (protocol_version, count) in open_games { | ||
| ChallengerMetrics::open_games(protocol_version.to_string()).set(count as f64); | ||
| } | ||
|
|
||
| ChallengerMetrics::games_scanned_total().increment(games_to_scan); | ||
| ChallengerMetrics::scan_head().set(end as f64); | ||
|
|
There was a problem hiding this comment.
Concern (correctness): When proof_protocol_descriptor fails with a transient RPC error, evaluate_game returns Err, which causes evaluated_every_index to be set to false — but if the same game also failed to have its fingerprint mapped (because it was never evaluated), it won't be counted in unmapped_fingerprint_games_total. That's correct behavior.
However, there's a subtle issue: if proof_protocol_descriptor succeeds but returns a fingerprint not in self.protocol_versions, the game is skipped via Err and evaluated_every_index becomes false (preventing cache eviction). This is probably overly conservative — the unmapped fingerprint is a permanent condition for that game (not transient), so it shouldn't prevent cache eviction. Consider tracking whether the error was transient vs. permanent to allow eviction when only permanent failures are present.
Review SummaryThis PR adds protocol-version routing throughout the proof pipeline: from on-chain game fingerprinting in the challenger scanner, through the prover-service DB and claim queries, to worker job discovery. It also adds TEE image-hash pinning so the challenger can dispatch proofs to the correct enclave image for historical games. Architecture & DesignThe overall design is well thought out. The fingerprint-based routing (hashing TEE image + ZK range + ZK aggregate verification keys into a canonical fingerprint, mapped to an opaque protocol version) is a clean abstraction. The separation of concerns is good:
The backward compatibility story is solid: Specific FindingsThree inline comments posted:
Not Block-Production-SensitiveThis PR modifies the challenger (dispute game scanning/proving) and proposer (proof request dispatch) paths, not the builder/execution/payload assembly paths. The proposer changes only add a |
2 participants
Summary
base-proofscan pin oneBASE_COMMITand redeploy zeronet.image_hash) onto currentmain.810bcbf372d850798d689593a64a94b46d638082.Rebased through the hinted-registrar cutover and the new
basectl proofs games/propose/submitcommands.basectl proofs protocolis preserved for the live fingerprint inventory.Test plan
base-proofsDockerfiles / CodeflowBASE_COMMITpoint at810bcbf372d850798d689593a64a94b46d638082<live_fingerprint>=0)unmapped_fingerprint_games_totalstays 0basectl proofs protocolagainst the zeronet factory prints the live fingerprint(s)Made with Cursor