docs: add spec deviations page

[MegaRedHand]

Summary

Adds docs/spec_deviations.md documenting where ethlambda intentionally diverges from the leanSpec reference, mainly for performance:

• Asynchronous signature aggregation with early stop — aggregation runs off the actor loop on a spawn_blocking worker with a 750ms deadline, vs leanSpec's inline synchronous aggregate().
• Attestation scoring on block building — attestations are tier-scored (Finalize > Justify > Build) and selected against a projected post-state when packing a block, vs leanSpec's greedy slot-ordered inclusion.

Also registers the page in the mdBook SUMMARY.md under a new Development section so it actually renders in the book.

Source references were verified against current code (line numbers, and the tier-dependent attestation tie-break per leanSpec #1149).

Split out from #498, which now carries only the image-tag wording changes.

[github-actions]

🤖 Claude Code Review

Review: PR #499 — docs: add spec deviations page

This is a documentation-only change (new docs/spec_deviations.md + a SUMMARY.md entry). No source code is touched, so the usual correctness/security/consensus concerns don't apply — I instead verified the doc's factual claims against the current codebase.

Verification performed:

Claim in doc	Verified against
run_aggregation_worker, aggregation.rs:504	Matches exactly
AGGREGATION_DEADLINE, aggregation.rs:33 = 750ms	Matches
cancel.is_cancelled() check, aggregation.rs:519	Matches
MILLISECONDS_PER_INTERVAL = 800ms, lib.rs:47	Matches
select_attestations, block_builder.rs:170	Matches
enum Tier, block_builder.rs:473 (Finalize > Justify > Build)	Matches
EntryScore::ordering_key, block_builder.rs:517 (tier-dependent tiebreak)	Matches
MAX_ATTESTATIONS_DATA = 8, common/types/src/block.rs:109	Matches

All line numbers and code references are accurate as of the current main. The SUMMARY.md change follows the existing mdBook section pattern (# Consensus, # Operations, now # Development) and will render correctly.

Assessment: No issues found. The content is accurate, well-sourced, and appropriately scoped (split cleanly from PR #498 per the description). Good practice documenting why each deviation exists rather than just what differs from leanSpec — useful for onboarding and future spec-conformance audits.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt

[github-actions]

🤖 Codex Code Review

1. docs/spec_deviations.md overstates the aggregation equivalence. The text says “same XMSS proofs are produced,” but the implementation cancels the worker at +750 ms and stops starting new jobs once cancel.is_cancelled() is set, so a timed-out session can publish only a subset of the proofs the synchronous leanSpec path would have completed. See crates/blockchain/src/lib.rs and crates/blockchain/src/aggregation.rs. I’d reword this to say the proof logic is the same per completed job, but ethlambda may intentionally emit partial results under deadline pressure.

2. docs/spec_deviations.md embeds exact source line numbers in rendered documentation (aggregation.rs:504, lib.rs:47, block_builder.rs:517, etc.). Those will drift on routine refactors and leave the doc stale even when the behavior is unchanged. For maintainability, prefer stable symbol/file references or repo permalinks instead of raw line numbers in prose.

Aside from those doc issues, I did not find code correctness, security, or performance regressions in this PR because it only changes documentation. I did not run tests.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[github-actions]

🤖 Kimi Code Review

Review of PR #499: Spec Deviations Documentation

Overall, this is valuable documentation for consensus auditability. However, several clarifications are needed regarding determinism, safety bounds, and documentation maintenance.

Critical Concerns

1. Wall-clock dependence in consensus-critical code (aggregation.rs)
The 750ms early-stop mechanism introduces hardware/execution-speed dependence. Different validators with different CPU loads may produce different partial aggregates at the deadline.

• Line 12-14: Clarify whether this non-determinism is acceptable because:
  • The block is still valid with any subset of valid aggregates, OR
  • There is a deterministic minimum set that must complete before cancellation
• Risk: If two validators produce conflicting blocks due to timing jitter, fork choice must handle this. Document that partial aggregation does not affect slashability or consensus safety.

2. "Same XMSS proofs are produced" contradiction

• Line 16: Claims "same XMSS proofs are produced" but then describes "partial-result bound". These are contradictory.
• Fix: Change to "same validation rules apply; deviation is in the subset of signatures aggregated, not the aggregation algorithm itself."

3. Determinism of attestation scoring (block_builder.rs)

• Line 24: The scoring uses data_root as final tiebreak. Ensure the doc notes that:
  • The ordering must be strictly deterministic across all architectures (no floating-point in score calculation, no HashMap iteration order)
  • EntryScore::ordering_key (line 517) must use canonical SSZ roots, not runtime pointers or non-deterministic hashes

Security & Performance

4. Timing attack surface

• Line 13: The 50ms safety margin (800ms - 750ms) assumes specific network topology. Consider documenting:
  • The assumption of sub-50ms gossip propagation time
  • Whether this is configurable for different network conditions (testnets vs mainnet)
  • Consequences if the deadline hits during the 50ms window (missed slot?)

5. Cancellation safety

• Line 14: "in-flight jobs finish" suggests cooperative cancellation. Verify and document:
  • Whether XMSS aggregation is atomic per group (no partial state corruption if cancelled)
  • Whether cancel.is_cancelled() (line 519) is checked between groups or also during long XMSS operations
  • Memory safety: Ensure the spawned blocking task doesn't hold references to actor state that could be invalidated by the deadline timer

Documentation Quality

6. Fragile line number references
Line numbers will drift immediately upon code changes. Suggest:

• Use module paths + function names as primary references: crates/blockchain/src/aggregation.rs::run_aggregation_worker
• Add GitHub permanent links with commit hash for precise line references: https://github.com/lambdaclass/ethlambda/blob/<commit>/crates/...

7. Missing equivalence proof for attestation scoring

• Line 27: "both produce a valid block" is asserted but not proven. While true, consider noting:
  • Whether ethlambda's scoring could produce a different canonical chain than leanSpec under extreme network latency
  • Whether the tier prioritization (Finalize > Justify > Build) could delay finalization in edge cases compared to slot-order greedy inclusion

Minor Issues

8. Inconsistent formatting

• Line 1: "ethlambda" vs repository name "ethlambda" (consistency check)
• Line 11: tokio::task::spawn_blocking code formatting is good, but consider linking to Tokio docs for cancellation behavior

9. XMSS context
Since XMSS is non-standard (Ethereum typically uses BLS), consider adding a footnote explaining that XMSS signature aggregation is itself a spec deviation from standard Ethereum consensus, or link to where that's documented.

Summary

The deviations described appear sound from a consensus perspective (partial aggregation produces valid states; scoring preserves validity), but the documentation should explicitly address determinism and timing assumptions. Request changes for Items 1, 2, and 6 before merge.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

[greptile-apps]

Greptile Summary

This PR adds a docs/spec_deviations.md page cataloguing where ethlambda intentionally diverges from the leanSpec reference, and registers it in the mdBook SUMMARY.md under a new Development section. A minor path correction to CLAUDE.md is also included.

• Async aggregation with early stop — documents that aggregation runs off-actor via spawn_blocking, scoped to current-slot raw gossip only, with a 750 ms deadline, vs leanSpec's inline synchronous aggregate().
• Attestation scoring on block building — documents that select_attestations tier-ranks candidates (Finalize > Justify > Build) against a projected post-state before inclusion, vs leanSpec's greedy slot-ordered pass.

Confidence Score: 5/5

Documentation-only change; no production code is modified.

All three files are documentation or a developer guide. The new spec_deviations page is well-structured, references concrete code locations (functions, constants, file paths), and correctly distinguishes ethlambda behaviour from leanSpec. The SUMMARY.md and CLAUDE.md edits are minimal and correct.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
docs/spec_deviations.md	New documentation page describing two intentional spec deviations; content is detailed and accurate to the referenced code locations.
docs/SUMMARY.md	Adds a new Development section and links the new spec_deviations.md page; mdBook structure is correct.
CLAUDE.md	Minor path correction for leanSpec reference directory, adding fork-logic sub-path detail.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Aggregation["Async Aggregation (ethlambda)"]
        A[Interval 2 tick] --> B[snapshot_current_slot_aggregation_inputs\ncurrent-slot raw gossip only]
        B --> C[spawn_blocking worker]
        C --> D{cancel.is_cancelled?}
        D -- No --> E[Process group\nXMSS aggregation]
        E --> F[Emit AggregateProduced]
        F --> D
        D -- Yes --> G[Drop remaining jobs]
        H[send_after 750ms\nAGGREGATION_DEADLINE] --> I[Cancel token]
        I --> G
    end

    subgraph LeanSpec_Agg["Aggregation (leanSpec)"]
        J[tick_interval interval 2] --> K[aggregate\nsynchronous, no budget, no cancellation]
    end

    subgraph BlockBuild["Attestation Scoring on Block Build (ethlambda)"]
        L[select_attestations] --> M{Tier}
        M -- Finalize / Justify --> N[Sort: target slot, attest slot, new-voter count\ndata_root tiebreak]
        M -- Build --> O[Sort: new-voter count, target slot, attest slot\ndata_root tiebreak]
        N --> P[Pick best vs projected post-state]
        O --> P
        P --> Q[Repeat up to MAX_ATTESTATIONS_DATA = 8]
        Q --> R[compact_attestations\nmerge proofs per AttestationData]
    end

    subgraph LeanSpec_Block["Block Build (leanSpec)"]
        S[build_block] --> T[Sort by target.slot oldest-first\nGreedy inclusion up to 8]
    end

Loading

_{Reviews (2): Last reviewed commit: "Merge branch 'main' into docs/spec-devia..." | Re-trigger Greptile}

[github-actions]

🤖 Kimi Code Review

This PR adds documentation describing ethlambda's deviations from the leanSpec reference implementation. While documentation-only, the described behaviors have consensus and security implications that warrant review.

Security & Consensus Considerations

Asynchronous Aggregation with Early Stop (spec_deviations.md, lines 10-14)

The described 750ms deadline with 50ms buffer for propagation is aggressive. Consider:

• Network jitter or GC pauses could consistently push nodes over the deadline, causing systematic signature loss
• Different nodes may cancel at different times, leading to divergent views of available aggregates before block building
• Suggestion: Document the recovery mechanism—are raw signatures retained for the next slot's aggregation if cancelled, or permanently dropped? This affects liveness if the network is slow.

Determinism of Partial Results (spec_deviations.md, line 14)

The text states "any such subset still yields a valid block." While true, clarify whether this affects fork choice weight calculations. If Node A aggregates 100 signatures and Node B aggregates 60 due to timing, their fork choice views diverge until the block is published. Ensure this doesn't violate the "safety never depends on timing" property.

Documentation Accuracy

File Path Verification

• crates/blockchain/src/aggregation.rs – Verify this path exists; the codebase may use crates/consensus/ or similar structure
• MAX_ATTESTATIONS_DATA = 8 – Confirm this constant is defined in crates/common/types/src/block.rs rather than a config or constants module

Missing Critical Details
The aggregation section should document:

1. Thread-safety of the snapshot operation (snapshot_current_slot_aggregation_inputs)—is it lock-free or under a mutex?
2. XMSS context initialization cost—if spawn_blocking creates new threads per aggregation, this could cause latency spikes. Is a thread pool used?
3. Cancellation boundary—does cancel.is_cancelled() check happen between groups or between individual XMSS operations? (The latter could leave signatures in an invalid intermediate state)

Code Quality (Described Behavior)

Attestation Ordering Tiebreakers (spec_deviations.md, lines 18-20)

The tiered scoring system (Finalize > Justify > Build) with data_root as final tiebreaker is well-designed for determinism. However:

• Ensure EntryScore::ordering_key uses consistent byte representation (big-endian) across architectures for the data_root hash comparison
• Clarify whether "new-voter count" is computed against the current state or the projected post-state mentioned in the text (affects correctness of the 2/3 threshold check)

MAX_ATTESTATIONS_DATA Limit
The documentation notes the limit is 8 for both implementations. Verify this matches the actual code constant—if the spec changes this value, the documentation will silently diverge.

Suggested Additions

Add a "Risk Assessment" subsection to each deviation:

### Risk Assessment
- **Liveness:** Low—cancelled signatures are retained for next slot (if true)
- **Consensus:** None—block building is local state
- **Worst Case:** Block contains fewer attestations, reducing rewards but not safety

Nitpicks

• Line 12: "The worker streams each finished group back as an AggregateProduced message" — specify if this uses tokio::sync::mpsc or actor mailboxes
• Line 20: "compaction step (compact_attestations)" — clarify if this is XMSS signature aggregation or just deduplication of AttestationData

Overall: The documentation is clear and technically sound. Address the signature persistence question and thread pool usage to ensure operators understand the liveness implications of the 750ms deadline.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

[github-actions]

🤖 Codex Code Review

1. docs/spec_deviations.md is materially inaccurate about the aggregation input path. It refers to snapshot_current_slot_aggregation_inputs, which does not exist, and says interval-2 aggregation only uses current-slot raw gossip while skipping proof reuse and stale-slot groups. The implementation actually calls snapshot_aggregation_inputs from crates/blockchain/src/lib.rs, and that snapshot includes both gossip groups and payload-only groups via store.new_payload_keys() in crates/blockchain/src/aggregation.rs. The scoring prefers current-slot work, but it does not hard-filter stale groups out. This should be corrected before merging, otherwise future work on attestation aggregation will be guided by the wrong behavior.

2. docs/spec_deviations.md says the session is cancelled after 750ms, leaving ~50ms for publish/propagation. The code does not match that: AGGREGATION_DEADLINE is 800ms in crates/blockchain/src/aggregation.rs, and MILLISECONDS_PER_INTERVAL is also 800ms in crates/blockchain/src/lib.rs. Also, early sessions publish alignment is enforced by publish_at inside the worker, not by reserving 50ms at the end of the interval, as shown in crates/blockchain/src/aggregation.rs. The doc should either describe the actual 800ms behavior or explain the intended reserve if the code is meant to change later.

No runtime code is changed in this PR, so I don’t see new fork-choice, STF, XMSS, SSZ, or memory-safety risk introduced by the patch itself. The main issue is that the new documentation currently misdescribes consensus-adjacent behavior in ways that could mislead later implementers.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt