Skip to content

docs: add stateless-validator review checks to REVIEW.md#157

Open
vincent-k2026 wants to merge 1 commit into
mainfrom
krabat/docs/review-guidelines
Open

docs: add stateless-validator review checks to REVIEW.md#157
vincent-k2026 wants to merge 1 commit into
mainfrom
krabat/docs/review-guidelines

Conversation

@vincent-k2026

Copy link
Copy Markdown
Collaborator

Supplements the baseline rubric with stateless-validator-specific checks it doesn't cover:

  • Fail closed on the terminal publish — a permanently failed final step surfaces a non-zero exit / Err, not silent success.
  • Validate config at the boundary — reject/clamp a 0 timeout and unbounded alloc hints before they reach the retry loop.
  • Classify remote errors precisely — match known error codes (not body.contains), with an inter-cycle retry throttle.

Additive only — no existing rules changed.

@vincent-k2026 vincent-k2026 added the documentation Improvements or additions to documentation label Jul 22, 2026
Comment thread REVIEW.md

- Changes to consensus-critical code — EVM execution, witness verification, state root computation, withdrawals — require extra scrutiny.
- Watch for read-check-write without atomicity (lock, CAS, or database constraint), CPU-intensive work that should use `spawn_blocking` inside async contexts, and deadlocks from nested lock acquisition.
- **Fail closed on the terminal publish.** The final step (e.g. `mega_setValidatedBlocks`) must not look like success when it permanently failed — surface a non-zero exit / `Err` so the orchestrator detects the miss and retries, instead of advancing while upstream never saw the tip.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Tighten "permanently failed" in rule 1 (REVIEW.md:20). The codebase deliberately tolerates a failed final report on a chain-following run — the next start re-reports anchor→tip and the miss heals (workers.rs:170-178). Only the slice run fails closed. As written, the rule relies entirely on the word "permanently" to encode that distinction, and the automated reviewer reads this line without the workers.rs context — it could flag the deliberately-tolerant chain-following path as a violation. Suggest: "…must not look like success when the run has no later opportunity to retry (e.g. an --end-block slice run)".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants