Skip to content

docs: document --is-aggregator flag requirement for finalization#132

Merged
pablodeymo merged 1 commit intodevnet-3from
docs/aggregator-flag-gotcha
Feb 13, 2026
Merged

docs: document --is-aggregator flag requirement for finalization#132
pablodeymo merged 1 commit intodevnet-3from
docs/aggregator-flag-gotcha

Conversation

@pablodeymo
Copy link
Collaborator

Motivation

During devnet-3 debugging we found that the network was producing blocks but never finalizing (justified_slot=0, finalized_slot=0 after 450+ slots). The root cause: no node was started with --is-aggregator, so attestation signatures were silently dropped after verification (store.rs:368), resulting in all blocks having attestation_count=0.

Description

  • CLAUDE.md: Add "Aggregator Flag Required for Finalization" to the Common Gotchas section, documenting the full attestation pipeline, root cause, and symptom
  • README.md: Add a note in the devnet section warning that at least one node must use --is-aggregator when running manually

How to Test

  • Read the docs changes for accuracy
  • Verify by running a devnet without --is-aggregator (blocks produce, no finalization) vs. with it on at least one node (finalization advances)

Without this flag, attestations pass signature verification but the
signature is never stored for aggregation (store.rs:368), so blocks
are always built with attestation_count=0 and the network never
finalizes. At least one node must run as aggregator.
@github-actions
Copy link

🤖 Kimi Code Review

Review Summary

The changes are documentation-only and correctly identify a critical operational issue with attestation aggregation. The documentation additions accurately describe the aggregator flag requirement and its impact on finalization.

Key observations:

  1. Correctness: The documentation accurately reflects the attestation pipeline flow and the aggregator flag's role in the signature storage logic.

  2. Critical operational issue: The fact that blocks can be produced indefinitely without finalization when no aggregator is present is a significant UX trap. Consider adding a runtime warning or log message when:

    • No aggregator is detected in the network for N slots
    • Attestations are being received but attestation_count=0 in blocks
  3. Minor improvement suggestion: In CLAUDE.md, line 312 could be more precise: "At least one node must be started with --is-aggregator" → "At least one online node" since offline aggregators don't help.

  4. Consider adding: A metrics counter or log line that tracks when attestations are received but not aggregated due to missing aggregator flag, which would help operators diagnose this issue.

The documentation changes are accurate and necessary. No code changes required.


Automated review by Kimi (Moonshot AI) · custom prompt

@github-actions
Copy link

🤖 Codex Code Review

Summary
Only documentation changes in CLAUDE.md and README.md. No code changes to review for correctness, security, performance, or consensus logic.

Findings
None.

Notes

  • The added operational guidance about --is-aggregator is clear and actionable. See CLAUDE.md in the new “Aggregator Flag Required for Finalization” section and README.md near the devnet instructions.

If you intended to include code changes for review, please share the updated diff.


Automated review by OpenAI Codex · custom prompt

@MegaRedHand MegaRedHand changed the title Document --is-aggregator flag requirement for finalization docs: document --is-aggregator flag requirement for finalization Feb 13, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 13, 2026

Greptile Overview

Greptile Summary

Documents critical --is-aggregator flag requirement discovered during devnet-3 debugging. Without this flag on at least one node, attestation signatures are silently dropped after verification, preventing finalization despite healthy block production.

  • CLAUDE.md: Added comprehensive section under Common Gotchas explaining the attestation pipeline, root cause (signatures not stored at store.rs:368), and symptom (justified_slot=0, finalized_slot=0 indefinitely)
  • README.md: Added warning in devnet section for users running nodes manually outside make run-devnet

The documentation accurately describes the technical behavior verified in the codebase and provides clear guidance to prevent this production issue.

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • Documentation-only changes that accurately reflect the codebase behavior. Technical details verified against source code (store.rs:368 conditional logic, interval 2 aggregation). Clear, actionable guidance prevents a critical production issue discovered during devnet-3 debugging.
  • No files require special attention

Important Files Changed

Filename Overview
CLAUDE.md Added accurate documentation about --is-aggregator flag requirement under Common Gotchas section
README.md Added warning about --is-aggregator flag requirement when running nodes manually outside make run-devnet

Last reviewed commit: 5f09082

@pablodeymo pablodeymo merged commit 1702d6a into devnet-3 Feb 13, 2026
6 of 7 checks passed
@github-actions
Copy link

🤖 Claude Code Review

Now let me generate the review:


PR Review: Document --is-aggregator flag requirement for finalization

Summary

This is a documentation-only PR that adds critical operational guidance discovered during devnet-3 debugging. The changes document a subtle but essential requirement: at least one node must run with --is-aggregator for the network to finalize blocks.

Overall Assessment: ✅ LGTM with minor observations

The documentation is accurate, well-placed, and addresses a real operational footgun. The changes follow existing documentation patterns and will prevent future debugging sessions.


Detailed Review

CLAUDE.md Changes (Lines 311-316)

Strengths:

  1. Accurate technical description: The attestation pipeline explanation matches the implementation:

    • Line 368 in store.rs confirms signatures are only stored when is_aggregator=true
    • Line 300-302 confirms aggregation happens at interval 2
    • Lines 367/292/310 confirm the promotion logic (new → known attestations)
  2. Clear symptom description: "justified_slot=0 and finalized_slot=0 indefinitely" precisely captures the observable failure mode

  3. Well-positioned: Placed in "Common Gotchas" section where operators will look during troubleshooting

Minor Observation (not blocking):

The existing "Tick-Based Validator Duties" section in CLAUDE.md (lines ~60-66) states "4 intervals per slot" but the code clearly shows 5 intervals (0-4):

  • Interval 0: Accept attestations (if proposal)
  • Interval 1: Vote propagation (no-op)
  • Interval 2: Aggregation (only if is_aggregator) ← Critical for this PR's context
  • Interval 3: Update safe target
  • Interval 4: Accept accumulated attestations

This pre-existing inaccuracy doesn't affect this PR's correctness, but the new documentation correctly states "aggregate at interval 2" which aligns with the implementation. Consider fixing the interval count in a follow-up.

Security/Correctness: ✅ No issues — documentation accurately reflects code behavior


README.md Changes (Lines 34)

Strengths:

  1. User-facing warning: Well-positioned immediately after the make run-devnet command
  2. Scope clarification: Correctly distinguishes between automated devnet (works OOTB) and manual node operation
  3. Consequence explanation: Clear causal link (no aggregator → blocks but no finalization)

Minor Wording Suggestion:

The note says "When running nodes manually (outside make run-devnet)..." but doesn't explicitly confirm that make run-devnet does set the aggregator flag correctly. Consider adding half a sentence for completeness:

> **Important:** The `make run-devnet` command handles this automatically. When running nodes manually, at least one node must be started with `--is-aggregator` for attestations to be aggregated and included in blocks. Without this flag, the network will produce blocks but never finalize.

However, this is optional — the current wording is already clear enough for practitioners.


Verification Checklist

  • ✅ Code reference store.rs:368 is accurate (verified line 368: } else if is_aggregator {)
  • ✅ Attestation pipeline description matches implementation
  • ✅ Interval 2 aggregation timing is correct (lines 298-302 in store.rs)
  • ✅ Symptom description matches expected behavior (no aggregation → attestation_count=0 → no finalization)
  • skip-signature-verification bypass explanation is accurate (lines 360-367 in store.rs)
  • ✅ No code changes, only documentation — zero regression risk

Recommendation

Approve and merge. This PR solves a real operational pain point with clear, accurate documentation. The changes follow project conventions and are well-positioned for discoverability.


Automated review by Claude (Anthropic) · custom prompt

@pablodeymo pablodeymo deleted the docs/aggregator-flag-gotcha branch February 13, 2026 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants