feat(stateless-core): validate_block_deriving_updates — changeset-deriving validation entry point#155
Open
flyq wants to merge 31 commits into
Open
feat(stateless-core): validate_block_deriving_updates — changeset-deriving validation entry point#155flyq wants to merge 31 commits into
flyq wants to merge 31 commits into
Conversation
Port the R2 witness primitives (keys / sigv4 / endpoint / client) from mega-reth c290c3e into a dedicated leaf crate, replacing the git-rev dependency; mega-reth will delete its copy and consume this one via git tag, making the cross-repo dependency one-way. - add keys::block_object_key as the single primary-key template (object_keys delegates, the validator's R2 source calls it directly) - add golden wire-vector tests: a real migrated mainnet object key and a full SigV4 header set computed with an independent implementation - unify the workspace on reqwest 0.12 (0.13 was only used by debug-trace-server dev-deps); stateless-r2 keeps an explicit 0.12 pin since &reqwest::Client is part of its public API - r2_witness: carry Bytes end-to-end (no multi-MB copy per block), dedicated DecodePanicked variant, module-level retry consts, drop the unused deadline parameter and the per-fetch debug header log - docs: neutral witness-source wording; R2 is a first-class source Co-Authored-By: Claude Fable 5 <[email protected]>
- Record R2 witness fetch metrics: fetch+decode duration histogram, GET retry counter, surfaced-error counter labelled by kind, and the same witness-size breakdown the RPC path reports (pre-registered at startup like the RPC method counters). - Add an end-to-end happy-path test: a fixture witness encoded with encode_witness_payload served over the mock R2 server must decode back to the original tuple in exactly one GET (mock now serves byte bodies). - Warn at startup when --witness-endpoint is set but ignored because --witness-source r2 is active. - Fix rustdoc intra-doc links introduced by this branch. Co-Authored-By: Claude Fable 5 <[email protected]>
…fore exit Addresses the two open Codex review findings on this PR: - R2WitnessClient now enforces --witness-max-concurrent-requests with its own semaphore (permit scoped per GET attempt, like the RPC path's), so the documented knob for bounding witness I/O works in R2 mode instead of being silently ignored. Queue wait on the cap is excluded from the fetch-duration histogram so it cannot masquerade as R2 latency. Concurrency-probe test asserts the in-flight high-water mark. - run_with_signals sends one final validation report after the pipeline (and any signal drain) has fully stopped. The periodic reporter is cancelled the instant the pipeline completes, so an --end-block slice run could previously finish without ever reporting its tail upstream (no later restart re-reports it). The reporting round is extracted into report_range_once, shared by the tick loop and the final flush. New end-to-end regression test drives run_with_signals to a sync target and asserts the mock upstream saw the final tip; both new tests were mutation-tested against the reverted fixes. Co-Authored-By: Claude Fable 5 <[email protected]>
The final shutdown report inherited the periodic reporter's best-effort semantics (log + skip), but it has no next tick behind it: a transient endpoint blip at exactly shutdown would permanently lose an --end-block slice run's tail report while the process exits 0. report_range_once now returns whether the round settled, and the final flush retries up to 3 attempts (1s apart) before giving up with an ERROR log; a detected validation gap stays non-retried. Regression test fails the first report call via the mock and asserts the retry lands the tail (mutation-tested against FINAL_REPORT_ATTEMPTS = 1). Also fixes both pre-existing rustdoc warnings: re-export ValidationTask (it is ValidatorFetcher's public Output type) and backtick the <metrics-port> placeholder that rustdoc read as an unclosed HTML tag. Co-Authored-By: Claude Fable 5 <[email protected]>
Comment-only pass over PR #152: compress multi-sentence narratives to the minimal statement of the constraint, keep every load-bearing "why" (throttle rationale, redirect policy, wire-format contracts, golden-vector docs), and give each rationale a single home instead of restating it at every call site. No code changes. Co-Authored-By: Claude Fable 5 <[email protected]>
Address review feedback: R2WitnessClient::new now takes the same BackoffPolicy that app.rs builds from --rpc-initial-backoff-ms / --rpc-max-backoff-ms instead of hard-coded INITIAL_BACKOFF/MAX_BACKOFF constants, so R2 retry pacing is field-tunable. The retry loop gains up to 50% jitter (mirroring round_robin_with_backoff) so parallel validators slicing a range don't retry in lockstep through a shared R2 brownout, and the cfg!(test) constant shrinking is replaced by tests passing a millisecond-scale policy directly. Co-Authored-By: Claude Fable 5 <[email protected]>
…validation Adds a validate_block variant that returns the replay-derived salt::StateUpdates instead of computing the post state root, for embedders that already hold a hash-verified per-block changeset to compare against (mega-reth full nodes). Also adds optional IPA-verify skipping and pre-state-root anchoring against the parent header (activating the dormant PreStateRootMismatch), and extracts the withdrawal-storage / state-update-derivation stages shared with validate_block. validate_block itself is unchanged. Co-Authored-By: Claude Fable 5 <[email protected]>
…erver onto it Extracted from the coverage-replayer branch (PR #153) so it can merge independently, on top of PR #152. - stateless-core: LightWitnessFromSalt decodes kvs+levels directly from full SaltWitness bytes via a layout-mirroring deserializer — proof material is structurally consumed as raw bytes, no curve point is ever constructed (~1.4ms vs ~110ms wall on a 6.3MiB mainnet witness). collect_code_hashes helper; metadata-to-None corruption is now an error instead of a panic. - stateless-common: get_witness_light / get_witness_light_with_deadline on RpcClient, decode_witness_payload_light / decode_witness_response_light, WitnessSizeBreakdown::new_light (documented lower bound). - debug-trace-server: both RPC witness paths (data_provider fetch, chain-sync prefetch) use the light decode; the server never verifies proofs, so the full decode's EC work bought nothing. salt dep dropped. Co-Authored-By: Claude Fable 5 <[email protected]>
…erf docs Review follow-ups on this PR: - estimate_witness_size removed: its only caller was the data-provider path this PR migrates to WitnessSizeBreakdown::new_light; no other in-workspace or mega-reth usage exists. - Doc comments no longer pre-reference the coverage-replayer (it lands with PR #153); consumers are named generically. - Perf numbers in docs recalibrated to a reproducible measurement on the committed 6.3MiB mainnet payload (14-core M4 Pro, median of 20): full decode ~112ms wall / ~1.4 core.s CPU, light ~3.7ms single-threaded (bincode stage); payload-level incl. zstd: 124.8ms vs 6.0ms. Co-Authored-By: Claude Fable 5 <[email protected]>
- RpcClient: the primary-failover round_robin_with_backoff invocation (6 constant args) lives once in a private witness_round_robin<T>; the fetch_witness_raw / fetch_witness_light thin wrappers are gone (callers pass the decoder + trace message directly). - witness_encoding: response framing (version prefix + base64) and payload decode (zstd + bincode-legacy) are each fixed in one place (decode_response_with / decode_payload_as); the four public decoders are now one-line delegations. - WitnessSizeBreakdown: new / new_light share from_counts; the light path passes 0 commitments, so the size constants and the MPT byte-sum live once. - fetch_witness_with: the trace message is emitted as the log message again instead of a field named trace_msg. No behavior change; 4 review agents (reuse / simplification / efficiency / altitude) found nothing else actionable. Co-Authored-By: Claude Fable 5 <[email protected]>
Machine-specific measurements go stale silently in code; the PR description is their canonical home. Comments keep the qualitative reasoning (per-commitment EC work dominates; light decode skips it) and light_witness.rs points at PR #154 for the numbers. Co-Authored-By: Claude Fable 5 <[email protected]>
Provenance belongs to git blame / GitHub, not comments; the qualitative constraint stands on its own. Co-Authored-By: Claude Fable 5 <[email protected]>
…' into liquan/feat/validate-block-updates
…tion over the light decode Adds a validate_block_updates variant that consumes a zero-validation LightWitness (PR #154's light decode), so proof-skipping embedders never construct a curve point anywhere on the decode + replay path: the env oracle comes from from_light_witness, replay runs over WitnessDatabase<LightWitnessExecutor>, and derive_state_updates is now generic over the StateReader store. No IPA verification or pre-state-root anchor is possible in this form (the light decode discards all proof material); the caller's binding is the returned updates against a header-committed changeset. Locked by a fixture test asserting the light path derives byte-identical StateUpdates to the full-witness path and still reproduces the header state root. Co-Authored-By: Claude Fable 5 <[email protected]>
no_std builds have no monotonic clock, so every ValidationStats timing reads 0.0
("not measured") — the witness_verification_time > 0.0 expectation in
validate_block_updates_mainnet_fixtures only holds with std enabled. This was the
single failure behind both the no-std CI job and the coverage job (whose script
runs the suite with --no-default-features).
Co-Authored-By: Claude Fable 5 <[email protected]>
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…erify_witness knob The full-node light validation mode is being removed on security-assumption grounds: without IPA verification and without a pre-state anchor, the witness is bound to the chain only through replay-changeset equality against the header-committed SaltDeltas, and that trust model is not worth carrying as a production switch. validate_block_updates now always verifies the witness IPA proof; ValidationOptions keeps only the expected_pre_state_root anchor. The light witness decode itself (#154) stays — it still serves debug-trace-server. Co-Authored-By: Claude Fable 5 <[email protected]>
flyq
marked this pull request as ready for review
July 20, 2026 09:40
flyq
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b229531fb8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// When set, require the witness's own state root to equal this value (the parent block's | ||
| /// post state root) before any other work, failing with | ||
| /// [`ValidationError::PreStateRootMismatch`] otherwise. | ||
| pub expected_pre_state_root: Option<B256>, |
There was a problem hiding this comment.
In embedded use outside the validator pipeline, setting expected_pre_state_root anchors only the SALT witness; there is no analogous check that mpt_witness.storage_root equals the canonical parent withdrawals root. verify_replay_outputs eventually calls MptWitness::verify, which proves a transition from the witness's own pre-root to the header's withdrawals_root, so a stale or forked withdrawals witness can still pass as long as that self-contained transition reaches the header root. Please add an expected_pre_withdrawals_root option/check, or otherwise expose/check the MPT pre-root before accepting success from this new entry point.
Useful? React with 👍 / 👎.
- Extract the shared witness-verify + replay prelude of validate_block and validate_block_updates into verify_and_replay (per-stage timing, same spans). - Add ValidationOptions::with_expected_pre_state_root so out-of-crate callers can build the #[non_exhaustive] options in one expression. - Lock the unconditional IPA proof check with a tampered-witness test through both entry points. - Document that the returned updates cannot cover newly deployed bytecode. - Un-shadow the anchored counter/options binding in the anchor test. Co-Authored-By: Claude Fable 5 <[email protected]>
…updates - Fold the four cfg(std) timing sandwiches into one timed() helper; the entry points now destructure a ValidationStats pre-filled by verify_and_replay and only set salt_update_time (stats layout spelled once). validate_block's salt_update_time now excludes the microsecond withdrawal-storage extraction, matching its documented meaning exactly. - tampered_witness re-encodes through SaltValue::new/key/value instead of hand-rolling the wire layout offsets. - Test runners run_updates/run_block take the witness as a parameter, so the tamper test stops inlining full entry-point invocations and validate_block call sites share the same helper. - assert_updates_reproduce_state_root feeds StateRoot the borrowed SaltWitness directly, dropping a multi-MB clone + Witness rebuild per fixture block. - The anchor test asserts the witness-root/parent-root invariant cheaply across all paired fixtures and exercises the accept/reject gate once, instead of re-running 18 full validations the mainnet sweep already covers (plus 18 discarded witness clones on the reject leg). Co-Authored-By: Claude Fable 5 <[email protected]>
…tate pre-root Review follow-up on the pair-anchor gap: ValidationOptions gains expected_pre_withdrawals_root, checked against the MPT witness's own storage_root before any proof or replay work and failing with the previously dormant PreWithdrawalsRootMismatch. Together with expected_pre_state_root this anchors the same (state root, withdrawals root) pair the standalone pipeline's continuity check enforces between consecutive blocks. Note this is more than defense-in-depth for the pre-root binding: MptWitness::verify only proves the witness against its own claimed storage_root and binds the post root to the header, so a fabricated pre-state confined to slots the block rewrites converges to the correct post root and passes verify — the anchor is the sole check that catches it. Tests cover both anchors' accept/reject paths plus an ordering proof (tampered witness + bogus anchor still fails with the anchor error, so the anchors run before verification). Co-Authored-By: Claude Fable 5 <[email protected]>
…cleanups - Replace the two independent anchor options with a single ValidationOptions::parent_anchor pair (new ParentAnchor type, built via with_parent_anchor(state_root, withdrawals_root)): every real caller takes both roots from the same parent header or block meta, and the split API made it silently easy to anchor only half — the exact gap a review pass already had to catch once on this branch. Gate semantics and both error variants are unchanged; ParentAnchor is re-exported from lib.rs. - timed() is now fallible, dropping the intermediate un-?-ed bindings and closure return-type annotations at all four stage call sites. - Tests: a module-level LazyLock CHAIN_SPEC replaces seven per-test genesis re-parses and the chain_spec parameter on the runners; the mainnet updates sweep anchors to the parent whenever it is in the fixture set (the embedder's real call shape, covering the anchored accept path for free); the two anchor tests merge into one paired test whose reject legs use a tampered witness, folding the anchor-before-verification ordering proof into the same runs and dropping the now-redundant standalone accept legs. - Docs: the anchor semantics now live once on the parent_anchor field; the entry-point bullet and inline comment point at it instead of restating it. Co-Authored-By: Claude Fable 5 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 614704f429
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…test builds Under no_std the crate aliases `std` to `alloc` (lib.rs), so the test module's `use std::sync::LazyLock` resolved into alloc and broke `cargo test --no-default-features --lib` — the CI no-std job, and the coverage job which compiles tests the same way. The memoizing static was not worth a real-std rebinding trick: the 1.8 KB genesis parse costs ~29µs per call, noise next to the witness work every runner call already performs. Replace it with a plain `fn chain_spec()` — the caller-side pattern stateless-test-utils prescribes — and document the `alloc as std` alias at its cause site in lib.rs, including how `cfg(test)` code names real-std-only items when it must. Co-Authored-By: Claude Fable 5 <[email protected]>
Troublor
requested changes
Jul 21, 2026
Troublor
left a comment
Troublor
left a comment
Collaborator
There was a problem hiding this comment.
Solid PR overall: the stage extraction is pure code motion (validate_block behavior and error precedence unchanged), the ParentAnchor design is sound, and the test suite is strong — the trie-based equivalence cross-check in particular locks the two entry points together.
Requesting changes on one point: the name of the main new export (see inline comment). Since it's the PR's public API surface, it's worth settling before merge.
One more note: the PR description still describes ValidationOptions { expected_pre_state_root }, but the code now anchors a (state_root, withdrawals_root) pair via parent_anchor. Since the description becomes the squash commit message, please update it before merging.
…e unanchored explicit
- Rename validate_block_deriving_updates: the updates are the function's
output, not the object of validation ("validate_block_updates" read as
validating the updates); the new name matches the internal
derive_state_updates stage and keeps the validate_block_ prefix that
signals unconditional proof verification.
- Drop ValidationOptions' Default. Options are now built with
ValidationOptions::anchored(state_root, withdrawals_root) — the standard
form — or the explicit ValidationOptions::unanchored(): the silent
default skipped the only binding between the witnesses and the canonical
parent, so opting out of the anchor must be spelled out at the call site.
Co-Authored-By: Claude Fable 5 <[email protected]>
flyq
changed the title
Move init_test_logging out of the shared run_end_block_slice helper into end_block_run_reports_final_tip, so only it and integration_test emit logs. The scripted-failure twins ran the same pipeline with deliberately failing report calls, printing ERROR/WARN lines that read like real faults in a passing suite. Co-Authored-By: Claude Fable 5 <[email protected]>
Troublor
approved these changes
Jul 22, 2026
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
2 participants
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
Adds
validate_block_deriving_updates, avalidate_blockvariant that returns the replay-derivedsalt::StateUpdatesinstead of computing the post state root. It targets embedders that already hold an independently verified per-block changeset for the same block — a MegaETH full node, whose state sync hash-verifies the sequencer'sSaltDeltasagainst the signed header (salt_deltas_hash) and recomputes/checks the post state root at commit time. Comparing the replay-derived net update map against that stored changeset replaces the validator-side SALT trie/commitment recompute (~0.65s/block measured), which duplicates work state sync has already done.Design
validate_block_deriving_updates(chain_spec, block, salt_witness, mpt_witness, contracts, options, writer)→(StateUpdates, ValidationStats). Both witness proofs are always verified (the SALT witness's IPA proof before replay, the MPT witness's Merkle proof on the replay outputs); no post state root is computed — the caller owns the changeset comparison.ValidationOptions(#[non_exhaustive], deliberately noDefault) is built viaValidationOptions::anchored(state_root, withdrawals_root)— the standard form, binding both witnesses' pre-roots to the parent's post-root pair before any other work (PreStateRootMismatch/PreWithdrawalsRootMismatchon divergence) — or the explicitValidationOptions::unanchored()opt-out for callers that genuinely lack a parent header. TheParentAnchorpair mirrors the(state root, withdrawals root)pair the standalone pipeline's continuity check enforces and is atomic by construction: real callers take both roots from the same parent header and never hold one without the other. The anchor is the only check binding the MPT witness's pre-state to the chain —MptWitness::verifyproves the witness against its own claimed root and binds only the post root to the header.validate_block, before the state-update derivation.verify_and_replay,withdrawal_storage,derive_state_updates,verify_replay_outputs,timed) are extracted and reused byvalidate_block, whose behavior is unchanged (crates/stateless-core/src/executor.rs).ValidationStats::salt_update_timeholds only the state-update derivation time (no trie math happens here) — documented on the field.Why strict changeset equality is sound
salt::StateUpdatesis a canonical net{key ↦ (old, new)}map uniquely determined by the (pre, post) state endpoints — order-independent, no-op-free. The embedder's stored row is RLP-hash-committed in the signed header and its application to the pre-state is root-verified by state sync at commit; the replay derivation here produces the net map of the same transition, so field-for-field equality on puts/deletes/updates is exactly the ingest-time-enforced equation (codesis not derivable from state updates and stays covered by the ingest-time header-hash and bytecode checks).Review changes
validate_block_updates(review: the updates are the function's output, not the object of validation; the new name matches the internalderive_state_updatesstage and keeps thevalidate_block_prefix signalling unconditional proof verification).ParentAnchorpair (the withdrawals pre-root was previously unbound) → silentDefaultremoved in favor of explicitanchored/unanchoredconstructors, so skipping the parent binding must be spelled out at the call site.History note
Earlier revisions of this branch also introduced
validate_block_updates_light(changeset validation over the zero-validation light witness, no IPA and no anchor) plus averify_witnessopt-out. Both were removed in b229531 on security-assumption grounds — proof verification is now unconditional. The light witness decode itself (#154, merged) is untouched and still serves debug-trace-server.Testing
cargo nextest run --workspace— 264/264 pass; the CI no-std job commandcargo test -p stateless-core --no-default-features --lib— 54/54 locally. Key tests: the mainnet fixture sweep runs anchored to each parent (the embedder's real call shape) and cross-checks that the returned updates reproduce the header state root through the SALT trie update, locking equivalence withvalidate_block; the paired-anchor test asserts the witness-root/parent-root invariant across all paired fixtures and rejects each mismatched half with exact error fields, using a tampered witness to prove the anchor runs before proof verification;tampered_witness_fails_proof_verificationlocks the unconditional IPA check through both entry points.🤖 Generated with Claude Code