docs: refresh CLAUDE.md, devnet skills, and Table docs to match code#532
Conversation
🤖 Kimi Code ReviewThis PR consists entirely of documentation updates (markdown files and Rust doc comments) with no executable code changes. The updates are accurate and important for operator correctness. Critical Documentation Fix
The correction to the supermajority calculation is critical and fixes a dangerous documentation error:
The formula Storage Schema Documentation
The updated comments accurately describe the composite key
Architectural Accuracy
Minor Suggestion
The log dump loop uses a hardcoded node list: for node in zeam_0 ream_0 qlean_0 ethlambda_0; doConsider making this configurable or documenting that this list must match the actual devnet topology if nodes are added/removed. SummaryAll documentation changes accurately reflect the current implementation. The supermajority fix prevents operational misconfiguration. No security issues or bugs introduced. Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Claude Code ReviewAll claims verified against source. This is a clean, well-fact-checked documentation-only PR. Review: PR 532 — docs: refresh CLAUDE.md, devnet skills, and Table docs to match codeThis PR is comments/docs-only (
Minor nit
/// every `SNAPSHOT_ANCHOR_INTERVAL` slots. Never pruned. Non-anchor states live in `StateDiffs` and
Purely cosmetic (rustfmt doesn't rewrap doc comments), but worth a manual wrap for consistency with the surrounding doc block. SummaryNo correctness, security, or behavioral concerns — this is a pure documentation/comment accuracy pass, and every changed factual claim I checked against the code was accurate. Good, careful work; the only note is the one cosmetic line-wrap nit above. Automated review by Claude (Anthropic) · sonnet · custom prompt |
Greptile SummaryThis PR refreshes development and storage documentation to match the current code. The main changes are:
Confidence Score: 4/5The changed devnet workflow invokes a missing script, while two comments misstate runtime retention and retry behavior.
.claude/skills/test-pr-devnet/SKILL.md, CLAUDE.md, and crates/storage/src/api/tables.rs
|
| Filename | Overview |
|---|---|
| .claude/skills/devnet-log-review/SKILL.md | Updates finalization thresholds and the pre-built block proposal flow. |
| .claude/skills/test-pr-devnet/SKILL.md | Corrects links and log collection guidance, but references a missing local analyzer. |
| CLAUDE.md | Refreshes repository guidance, with an incorrect maximum block-fetch backoff. |
| crates/storage/src/api/tables.rs | Clarifies persistent table behavior, but describes the signature pruning boundary too broadly. |
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
.claude/skills/test-pr-devnet/SKILL.md:147
**Local Analyzer Script Is Missing**
The new detailed-analysis step invokes `.claude/skills/devnet-log-review/scripts/analyze-logs.sh`, but that script is not present in the repository. Following this workflow collects the container logs and then fails with `No such file or directory` instead of analyzing them.
### Issue 2 of 3
CLAUDE.md:282-284
**Backoff Endpoint Is Unreachable**
The handler stops when `pending.attempts >= MAX_FETCH_RETRIES`, before scheduling the next delay. With attempts starting at 1, it schedules nine retries from 5 ms through 1280 ms; the documented 2560 ms delay is never reached, so developers calculating fetch timing from this section get the wrong retry window.
```suggestion
### Retry Strategy on Block Requests
- Exponential backoff: doubling from `INITIAL_BACKOFF_MS` (5ms → 1280ms)
- Max `MAX_FETCH_RETRIES` (10) total attempts: the initial request plus 9 retries, with random peer selection on retry
```
### Issue 3 of 3
crates/storage/src/api/tables.rs:11-14
**Pruning Boundary Is Misstated**
Signatures are not pruned merely because they are below the finalized slot. The implementation first computes `tip_slot - SIGNATURE_PRUNING_RANGE` and only removes entries below that cutoff when the cutoff is finalized, so this comment can make maintainers expect recently finalized signatures to be absent when they are deliberately retained.
```suggestion
/// Keyed by slot || root so pruning can scan in slot order and stop early.
/// Non-genesis blocks have an entry until they fall below the retention cutoff
/// (`tip_slot - SIGNATURE_PRUNING_RANGE`) and that cutoff is finalized. Headers
/// and bodies are kept forever.
```
Reviews (1): Last reviewed commit: "docs: refresh CLAUDE.md, devnet skills, ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| docker logs "$node" > "${node}.log" 2>&1 | ||
| done | ||
| .claude/skills/devnet-log-review/scripts/analyze-logs.sh | ||
| ``` |
There was a problem hiding this comment.
The new detailed-analysis step invokes .claude/skills/devnet-log-review/scripts/analyze-logs.sh, but that script is not present in the repository. Following this workflow collects the container logs and then fails with No such file or directory instead of analyzing them.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .claude/skills/test-pr-devnet/SKILL.md
Line: 147
Comment:
**Local Analyzer Script Is Missing**
The new detailed-analysis step invokes `.claude/skills/devnet-log-review/scripts/analyze-logs.sh`, but that script is not present in the repository. Following this workflow collects the container logs and then fails with `No such file or directory` instead of analyzing them.
How can I resolve this? If you propose a fix, please make it concise.| ### Retry Strategy on Block Requests | ||
| - Exponential backoff: 10ms, 40ms, 160ms, 640ms, 2560ms | ||
| - Max 5 attempts, random peer selection on retry | ||
| - Exponential backoff: doubling from `INITIAL_BACKOFF_MS` (5ms → 2560ms) | ||
| - Max `MAX_FETCH_RETRIES` (10) attempts, random peer selection on retry |
There was a problem hiding this comment.
The handler stops when pending.attempts >= MAX_FETCH_RETRIES, before scheduling the next delay. With attempts starting at 1, it schedules nine retries from 5 ms through 1280 ms; the documented 2560 ms delay is never reached, so developers calculating fetch timing from this section get the wrong retry window.
| ### Retry Strategy on Block Requests | |
| - Exponential backoff: 10ms, 40ms, 160ms, 640ms, 2560ms | |
| - Max 5 attempts, random peer selection on retry | |
| - Exponential backoff: doubling from `INITIAL_BACKOFF_MS` (5ms → 2560ms) | |
| - Max `MAX_FETCH_RETRIES` (10) attempts, random peer selection on retry | |
| ### Retry Strategy on Block Requests | |
| - Exponential backoff: doubling from `INITIAL_BACKOFF_MS` (5ms → 1280ms) | |
| - Max `MAX_FETCH_RETRIES` (10) total attempts: the initial request plus 9 retries, with random peer selection on retry |
Prompt To Fix With AI
This is a comment left during a code review.
Path: CLAUDE.md
Line: 282-284
Comment:
**Backoff Endpoint Is Unreachable**
The handler stops when `pending.attempts >= MAX_FETCH_RETRIES`, before scheduling the next delay. With attempts starting at 1, it schedules nine retries from 5 ms through 1280 ms; the documented 2560 ms delay is never reached, so developers calculating fetch timing from this section get the wrong retry window.
```suggestion
### Retry Strategy on Block Requests
- Exponential backoff: doubling from `INITIAL_BACKOFF_MS` (5ms → 1280ms)
- Max `MAX_FETCH_RETRIES` (10) total attempts: the initial request plus 9 retries, with random peer selection on retry
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| /// Keyed by slot || root so pruning can scan in slot order and stop early. | ||
| /// Non-genesis blocks have an entry until finalized: signatures below the | ||
| /// finalized boundary are pruned (`prune_old_block_signatures`), while | ||
| /// headers and bodies are kept forever. |
There was a problem hiding this comment.
Signatures are not pruned merely because they are below the finalized slot. The implementation first computes tip_slot - SIGNATURE_PRUNING_RANGE and only removes entries below that cutoff when the cutoff is finalized, so this comment can make maintainers expect recently finalized signatures to be absent when they are deliberately retained.
| /// Keyed by slot || root so pruning can scan in slot order and stop early. | |
| /// Non-genesis blocks have an entry until finalized: signatures below the | |
| /// finalized boundary are pruned (`prune_old_block_signatures`), while | |
| /// headers and bodies are kept forever. | |
| /// Keyed by slot || root so pruning can scan in slot order and stop early. | |
| /// Non-genesis blocks have an entry until they fall below the retention cutoff | |
| /// (`tip_slot - SIGNATURE_PRUNING_RANGE`) and that cutoff is finalized. Headers | |
| /// and bodies are kept forever. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/storage/src/api/tables.rs
Line: 11-14
Comment:
**Pruning Boundary Is Misstated**
Signatures are not pruned merely because they are below the finalized slot. The implementation first computes `tip_slot - SIGNATURE_PRUNING_RANGE` and only removes entries below that cutoff when the cutoff is finalized, so this comment can make maintainers expect recently finalized signatures to be absent when they are deliberately retained.
```suggestion
/// Keyed by slot || root so pruning can scan in slot order and stop early.
/// Non-genesis blocks have an entry until they fall below the retention cutoff
/// (`tip_slot - SIGNATURE_PRUNING_RANGE`) and that cutoff is finalized. Headers
/// and bodies are kept forever.
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
🤖 Codex Code Review
Aside from those documentation mismatches, I did not find code correctness, security, or consensus-logic issues in this PR; the diff is comment/docs-only. Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
The docs drifted from the code as PRs landed: - CLAUDE.md: 12 workspace crates (net/api and common/test-fixtures were missing from the tree), Store buffer names (new_payloads/known_payloads), BlocksByRange in req/resp, actual fetch backoff (5ms doubling, MAX_FETCH_RETRIES attempts), XMSS signature size (SIGNATURE_SIZE), libssz/libssz-merkle dependency renames, hash_tree_root(), the stf spec-test location, and a docs/ index in Resources - devnet-log-review skill: the justification threshold is 3*votes >= 2*N, i.e. ceil(2N/3) — the worked examples said 5-of-6 and 7-of-9 where the code needs 4 and 6; block proposal flow updated for the pre-build path (#445/#474) - test-pr-devnet skill: point log analysis at this repo's own devnet-log-review skill and fix a broken relative link to CLAUDE.md - tables.rs: BlockSignatures is keyed (slot || root) for slot-ordered pruning and entries are pruned below the finalized boundary; States anchors reference SNAPSHOT_ANCHOR_INTERVAL by name
9f67af5 to
32138e7
Compare
2 participants
Why
CLAUDE.md and the devnet skills had drifted from the code as PRs landed. Every claim below was verified against the current source before changing.
What
CLAUDE.md
net/apiandcommon/test-fixtureswere missing from the tree; added the newerblockchain/srcmodules (block_builder.rs,aggregation.rs,reaggregate.rs,sync_status.rs)new_payloads/known_payloads(promotion at intervals 0/4), notnew_attestations/known_attestationsBlocksByRangeINITIAL_BACKOFF_MS(5ms → 2560ms),MAX_FETCH_RETRIES(10) attempts — was "10/40/160/640/2560, max 5"SIGNATURE_SIZE= 2536 bytes, not 3112ethereum_ssz/tree_hash→libssz(+derive/types)/libssz-merkle;tree_hash_root()→hash_tree_root()stf_spectests.rslives incrates/blockchain/state_transition/tests/, notcrates/blockchain/tests/store.rs:368line reference with the function name (on_gossip_attestation)docs/index line to Resourcesdevnet-log-review skill
3 * votes >= 2 * N(=ceil(2N/3)), so 6 validators need 4 votes and 9 need 6 — the skill said 5 and 7 (">2/3")Finished building blockmarker (feat(blockchain): log when block building finishes #474)test-pr-devnet skill
devnet-log-reviewskill (was pointing into lean-quickstart) and dumps container logs firsttables.rs
BlockSignaturesdoc saidH256 -> BlockSignaturesand "all other blocks must have an entry"; the key is(slot || root)(slot-ordered pruning) and entries are pruned below the finalized boundaryStatesdoc now referencesSNAPSHOT_ANCHOR_INTERVALby name instead of hardcoding 1024Docs/comments only — no behavior change.