fix(validator): resync report range when upstream last_validated is behind

[mega-putin]

Generated by engineer-agent — review carefully before merging.

Summary

Keep the validation reporter alive on rejected mega_setValidatedBlocks responses, log validation gaps, and retry from upstream last_validated to the local tip when that block is still in the local canonical chain. Added tests for successful resync, hash mismatch handling, and greppable gap logging.

[mega-maxwell]

Claude review status

Living comment — rewritten in place. The review workflow keeps this single comment up to date instead of posting a new one each round, so it always describes the latest reviewed commit and the earlier text is intentionally gone. No reply is needed here; reply to a finding in its own review thread, and answer an open question in a reply on this PR. The next review round reconciles your answer.

✅ Review clean

Last reviewed: head 845a79ed · updated 2026-10-10T07:36:16+00:00

New this round: 0 finding(s), 0 question(s) · Resolved this round: 0 · Open questions: 0

[mega-putin]

No actionable review feedback is available yet; the only PR comment says the review is still in progress. No code changes or commit were made.

[mega-putin]

Added the bug label to PR #225. No code changes or commit were needed.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d32b5cb819

ℹ️ 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".

[chatgpt-codex-connector]

Resync from a range the validator can actually retain

When the upstream pointer is below anchor, this is the only path that attempts recovery, but a normally produced ValidatorDB cannot retain that pointer: initialization creates the chain at the anchor, pipeline advancement is strictly forward, and retention only removes older rows. Consequently get_block_hash(upstream_number) will always return None; the passing test manufactures a non-contiguous row (#15 after anchor #70). Deployed validators therefore still cannot heal this report gap, and fixed-range runs will fail after their retries. Base recoverability on an actually retained range or obtain and verify the needed predecessor hash.

AGENTS.md reference: AGENTS.md:L71-L72

Useful? React with 👍 / 👎.

[mega-putin]

Addressed the review feedback and committed:

497b14c address review feedback: verify gap predecessor header

Changes:

• If upstream last_validated is below the local anchor and no local canonical row is retained, the reporter now fetches and verifies the canonical header at that height before retrying [upstream last_validated, local tip].
• Tests no longer manufacture a non-contiguous retained row; they exercise the verified-header recovery path.

Validation run:

• cargo check
• cargo build
• cargo test -p stateless-validator
• cargo fmt --all --check
• cargo sort --check --workspace --grouped --order package,workspace,lints,profile,bin,benches,dependencies,dev-dependencies,features
• cargo clippy --workspace --all-targets --all-features

No push performed.

[mega-putin]

🔧 Pushed CI fixes.

Agent log

Verified job-level checks for `497b14c`: all code checks passed (`lint`, `no-std`, `test`, CodeQL, coverage); the only failed check was `pr-review` with `MODEL_ACTION_FAILED` in `review_retry`, an automated review infrastructure failure.

I attempted to rerun the failed workflow job, but GitHub returned `Resource not accessible by integration`. Since there was no code failure to fix, I created an empty commit to retrigger CI/review on the next external push:

`34dcf0e fix CI: retrigger pr review`

No push performed.

[vincent-k2026]

Reviewed against HEAD (34dcf0eb).

Theme: keeping the reporter alive is a real fix. The resync path, though, changes what this validator claims to have validated, and that needs an explicit decision rather than coming in as a side effect of a bug fix.

Good: on main, a validation gap makes report_range_once return Err, which ends the validation_reporter task. The pipeline keeps validating, but nothing reports upstream and nothing surfaces it. Logging and looping instead (runner.rs:218-222) is the right call.

Blocking (needs a decision)

1. The resync reports [upstream_last_validated, tip], which covers heights this validator never executed.

   • The anchor comes from the operator's --start-block and is trusted. This node only re-executes (anchor, tip].
   • When the receiver is behind the anchor, the retry claims (upstream, anchor] as well (runner.rs:110-130, retry_covering_report).
   • On main this case was deliberately fatal for --end-block slices. The removed comment called a detected gap deterministic.
   • The header check added in 497b14c only proves the receiver's pointer is canonical, which mega-reth already checks on first_block against its durable chain before accepting. It says nothing about whether the heights in between were validated.
   • Concretely: a receiver with no validated pointer, or whose pointer is no longer canonical, reports (0, genesis) on rejection (mega-reth witness.rs:560-575). This path then fetches header 0, it matches, and the validator pushes [0, tip].
   • Note on Codex's P1: it pointed out the local-DB branch is unreachable in production. The 497b14c response made the branch reachable through the header lookup, rather than revisiting whether the range should be reported at all.

   I'm aware mega-reth's FullNode publisher already does this on purpose. The set_validated_blocks docs say it re-executes (anchor, tip] but publishes [genesis, tip], and that coverage inside the range is "the pusher's own claim and is trusted as-is". If the team wants the stateless validator to make the same claim, that's a legitimate choice. But it should be made explicitly, by an owner, and documented here. In that case the header lookup is redundant: report [genesis, tip] the same way FullNode does.

   If not, keep the gap non-recoverable. Leave the reporter alive and alarming loudly (error log plus a metric, with backoff), and keep failing --end-block slices. Never push a range that starts below the anchor.

Non-blocking

2. An unresolvable gap repeats every second.
   • The reporter interval is 1s, and last_reported is not updated on rejection.
   • On a hash mismatch, every round costs one mega_setValidatedBlocks call, one eth_getHeaderByNumber call (5s deadline) and three warn/error lines.
   • Fix: back off, and log once per distinct (upstream_number, upstream_hash) pointer.
3. report_range_once can no longer return Err, but its signature still says it can.
   • The Result<bool> is now misleading.
   • Dead code: the Err arms in validation_reporter and in the final-flush loop, plus the doc comment about non-retryable errors.
   • Fix: return bool, and drop those arms.
4. The log-capture test does not discriminate.
   • install_log_capture installs a global subscriber with one shared buffer, so tests running in parallel write into it concurrently.
   • Three other log lines on this path also contain "validation gap", so rejected_gap_logs_validation_gap still passes after deleting the line it is meant to pin.
   • Fix: scope the capture with tracing::subscriber::with_default and assert the exact message. Adding tracing-subscriber as a dev-dependency just for this probably isn't worth it either way.
5. Missing case: the (0, genesis) response from item 1. Whichever way item 1 is decided, add a test for it.
6. Nits.
   • BlockHash::from(x.0) round-trips are no-ops, since BlockHash is B256.
   • The PR body does not say which incident or log line motivated the change.
   • The branch is based on 727a34b and has not run CI on top of #226/#227/#230.

Happy to be wrong if there is an operational reason to treat pre-anchor heights as validated that I'm missing.

[mega-putin]

Thanks — agreed. I removed the header-lookup recovery path and kept pre-anchor gaps non-recoverable for the standalone validator. It now only retries a covering report when the upstream pointer is actually retained in the local canonical-chain window; otherwise it logs the validation gap, keeps the reporter alive, and fixed-range final flushes fail after their bounded retries rather than claiming pre-anchor heights.

I also simplified report_range_once to return bool and added tests for the (0, genesis)/missing-local-pointer case, reporter liveness, and the exact uncovered-gap log.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-10T07:33:58.156112Z	845a79e	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[mega-putin]

No actionable review feedback is available yet; the latest Codex and Claude comments only show reviews still in progress for 845a79e. No code changes or commit were made.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.7%. Comparing base (727a34b) to head (845a79e).
⚠️ Report is 3 commits behind head on main.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mega-putin]

Codecov reports full coverage for modified coverable lines and no requested code changes. The “3 commits behind head on main” note is informational, so no changes or commit were made.