feat: add support for devnet 3

[MegaRedHand]

Closes #73

Summary

Upgrades ethlambda from pq-devnet-2 to pq-devnet-3, implementing the new 5-interval timing model, committee aggregation pipeline, and networking topology.

Timing model (5 intervals × 800ms = 4s slots)

The slot is now divided into 5 intervals of 800ms each (previously 4 × 1s):

Interval 0: Accept attestations (if proposal exists) + block proposal
Interval 1: Vote propagation (no-op)
Interval 2: Committee signature aggregation (aggregator nodes only)
Interval 3: Safe target update (2/3 threshold fork choice)
Interval 4: Accept accumulated attestations into fork choice

Aggregated attestation pipeline

Replaced the per-validator attestation maps (LatestNewAttestations / LatestKnownAttestations) with an aggregated payload pipeline:

Gossip attestation → AttestationDataByRoot + GossipSignatures
         ↓ (interval 2, aggregator only)
    aggregate_committee_signatures() → LatestNewAggregatedPayloads
         ↓ (intervals 0/4)
    promote → LatestKnownAggregatedPayloads → fork choice

For skip-signature-verification mode (tests), individual attestations bypass aggregation and insert directly into LatestNewAggregatedPayloads with dummy proofs.

Networking

• Per-committee aggregation subnets: /leanconsensus/devnet3/attestation_{subnet_id}/ssz_snappy
• New aggregation gossip topic: /leanconsensus/devnet3/aggregation/ssz_snappy
• Network name: devnet0 → devnet3
• SignedAggregatedAttestation type for aggregation gossip
• --is-aggregator CLI flag

Block building

• select_aggregated_proofs() (renamed from compute_aggregated_signatures) only selects existing proofs from the AggregatedPayloads table — no more inline gossip signature collection
• build_block() sources attestations from LatestKnownAggregatedPayloads
• produce_block_with_signatures() uses the same aggregated payload source

Fork choice

• update_head() and update_safe_target() reconstruct per-validator votes from aggregated payloads via extract_attestations_from_aggregated_payloads()
• on_gossip_aggregated_attestation() handles aggregated attestations from the network, expanding per-participant into payload entries

Test fixtures

Updated LEAN_SPEC_COMMIT_HASH to b39472e (leanSpec devnet-3 with INTERVALS_PER_SLOT=5). Regenerated all fixtures.

[github-actions]

🤖 Kimi Code Review

Security & Correctness Issues

1. Critical: Slot/Interval Calculation Bug (blockchain/src/store.rs:253)

let current_slot = store.time() / INTERVALS_PER_SLOT;

This is wrong - store.time() is in 800ms intervals, but INTERVALS_PER_SLOT is 5. This makes current_slot 5x larger than actual slots. This affects:

• Attestation validation (lines 253, 267, 298)
• Block production timing
• All fork choice calculations

Fix: Use proper slot calculation:

let current_slot = store.time() / INTERVALS_PER_SLOT;

2. Race Condition in Aggregation (blockchain/src/store.rs:aggregate_committee_signatures)

The aggregation function deletes gossip signatures after processing, but there's no atomicity guarantee. If a validator sends a new attestation between aggregation and deletion, the signature could be lost.

Fix: Use a transaction or batch operation for the entire aggregation process.

3. Memory Leak in Aggregated Payloads (storage/src/store.rs:insert_*_aggregated_payload)

The functions append to vectors without deduplication. Over time, this could lead to unbounded growth of duplicate payloads for the same (validator_id, data_root) key.

Fix: Check for existing payloads before insertion or implement periodic cleanup.

4. Incorrect Network Name (net/p2p/src/lib.rs:158)

Changed from "devnet0" to "devnet3" without corresponding spec update. This could cause network partition.

Fix: Ensure this matches the expected network configuration.

Performance Issues

1. Inefficient Aggregation (blockchain/src/store.rs:aggregate_committee_signatures)

The function iterates all gossip signatures for every slot, even when most are for different slots. Consider indexing by slot.

2. Redundant Storage (storage/src/store.rs)

Multiple copies of the same attestation data are stored (by root, in payloads, etc.). This could be optimized with reference counting.

Code Quality Issues

1. Magic Numbers (blockchain/src/lib.rs)

Constants like INTERVALS_PER_SLOT = 5 and MILLISECONDS_PER_INTERVAL = 800 should be configurable or come from the chain spec.

2. Complexity in select_aggregated_proofs

The function has nested loops and complex logic that's hard to verify. Consider breaking into smaller functions.

3. Inconsistent Error Handling

Some functions use inspect_err for logging while others use expect. Standardize error handling patterns.

Minor Issues

1. Typo in comment (blockchain/src/store.rs:290): "Vote propagation" should probably be "Vote preparation"
2. Dead code: post_state variable in build_block is assigned but never used (line 915)
3. Missing validation: No check that SignedAggregatedAttestation.proof actually covers the validators it claims to

Recommendations

1. Add comprehensive tests for the new aggregation logic, especially edge cases
2. Implement metrics for aggregation efficiency (signatures aggregated vs. total)
3. Add configuration for committee counts and subnet topics
4. Consider using a proper BLS library for signature aggregation instead of the current implementation

---

Automated review by Kimi (Moonshot AI) · custom prompt

[github-actions]

🤖 Claude Code Review

Based on my comprehensive review of PR #107, here's my code review:

---

PR #107 Review: Devnet 3 Support

Overview

This PR implements significant protocol changes for devnet 3, including:

• 5-interval slots (800ms intervals instead of 1s)
• Committee-based attestation subnets
• Aggregated attestation gossip
• Refactored attestation storage using aggregated payloads

Critical Issues

1. Time Conversion Bug (HIGH SEVERITY)

Location: crates/blockchain/src/lib.rs:134

let interval = (time_since_genesis % SECONDS_PER_SLOT) * 1000 / MILLISECONDS_PER_INTERVAL;

Problem: This calculation computes the interval from the timestamp's seconds component, but on_tick is called at 800ms boundaries. This creates a mismatch:

• At timestamp 4.8s: (4 % 4) * 1000 / 800 = 0 (should be interval 1)
• The calculation assumes tick happens exactly at second boundaries

Impact: Interval calculation will be incorrect for all non-second-aligned ticks, causing proposers/attesters to act at wrong intervals.

Recommendation: Remove this calculation entirely. The store.time() already tracks intervals correctly (set by on_tick). Use:

let slot = store.time() / INTERVALS_PER_SLOT;
let interval = store.time() % INTERVALS_PER_SLOT;

2. Non-Aggregator Nodes Miss Attestations (HIGH SEVERITY)

Location: crates/blockchain/src/store.rs:371-377

} else if is_aggregator {
    let signature = ValidatorSignature::from_bytes(&signed_attestation.signature)
        .map_err(|_| StoreError::SignatureDecodingFailed)?;
    store.insert_gossip_signature(&attestation.data, validator_id, signature);
}

Problem: Non-aggregator nodes only store attestation data by root but never insert into new_aggregated_payloads. Fork choice will not see these attestations.

Impact: Non-aggregator validator nodes will make incorrect fork choice decisions due to missing attestation data.

Recommendation: Always store in new_aggregated_payloads with an appropriate proof (either fresh aggregation or a single-validator proof).

3. Unused Variable (MEDIUM)

Location: crates/blockchain/src/store.rs:915

let _post_state = loop {

Problem: Variable prefixed with _ but the value is used at line 845 (Ok((final_block, post_state, aggregated_signatures))), causing a compilation error since post_state is not in scope.

Impact: Code won't compile.

4. Validator Index Out of Bounds Risk (MEDIUM SEVERITY)

Location: crates/blockchain/src/store.rs:164

let Ok(pubkey) = validators[*vid as usize].get_pubkey() else {
    continue;
};

Problem: No bounds check before indexing. If a malicious peer sends an attestation with an invalid validator ID and it gets stored in gossip_signatures, this will panic.

Impact: Potential panic causing node crash.

Recommendation: Add bounds check:

if *vid as usize >= validators.len() {
    continue;
}

5. Signature Parsing Duplication (LOW SEVERITY)

Location: crates/blockchain/src/store.rs:374-376

The signature is already parsed and verified at lines 351-355. Parsing it again at 374 is redundant.

Recommendation: Store the already-parsed signature instead of re-parsing.

Design Concerns

6. Aggregation Architecture

The two-table approach (LatestNewAggregatedPayloads and LatestKnownAggregatedPayloads) stores one entry per validator per attestation data root. With subnets, this could lead to storage amplification.

Observation: This is acceptable for devnet 3 with ATTESTATION_COMMITTEE_COUNT=1, but may need revisiting for production with many committees.

7. Gossip Signature Deletion Timing

Location: crates/blockchain/src/store.rs:196

Gossip signatures are deleted immediately after aggregation at interval 2. If aggregation fails partially (e.g., due to pubkey errors), those signatures are lost and won't be retried.

Recommendation: Only delete signatures that were successfully aggregated, not all attempted ones.

8. Pruning Efficiency

Location: crates/storage/src/store.rs:533-567

Pruning functions iterate over entire tables, decode every entry, check slots, then batch delete. For large devnets, this could be expensive.

Consideration: Not a bug, but monitor performance as network grows.

Minor Issues

9. Comment Accuracy

Location: crates/blockchain/src/lib.rs:164

// Update safe target slot metric (updated by store.on_tick at interval 3)

Should be "interval 3" (correct in code, line 298-300).

10. Network Name Hardcoded

Location: crates/net/p2p/src/lib.rs:157

let network = "devnet3";

Should be configurable or match genesis config network field.

11. Metrics Variable Assignment

Location: crates/net/p2p/src/metrics.rs:20

The refactored code is cleaner but the variable name name_registry was removed. Not an issue, just a style change.

Security Analysis

✅ Signature verification: Properly maintained in gossip path
✅ Bounds checking: Good validator index validation in on_gossip_attestation
⚠️ Array indexing: Missing bounds check in aggregate_committee_signatures (issue #4)
✅ State transition: Properly called with updated intervals
✅ Fork choice: Architecture correctly separates new/known attestations

Testing Recommendations

1. Time interval edge cases: Test ticks at non-second boundaries (e.g., 0.8s, 1.6s)
2. Non-aggregator fork choice: Verify non-aggregator nodes correctly participate in consensus
3. Invalid validator IDs: Fuzz test with out-of-bounds validator indices
4. Partial aggregation failures: Test behavior when some validators have invalid pubkeys

Positive Aspects

✅ Comprehensive refactoring to support committee subnets
✅ Clean separation of aggregation concerns
✅ Good use of content-addressed storage for attestation data
✅ Proper RAII patterns maintained
✅ Logging patterns follow project conventions
✅ Error handling with inspect_err is idiomatic

Summary

Blockers (must fix before merge):

• Issue docs: add initial roadmap #1: Time interval calculation bug
• Issue chore: initial repo setup #2: Non-aggregator attestation handling
• Issue docs: improve readme #3: Unused variable compilation error

Recommended fixes:

• Issue docs: add philosophy and design principles #4: Add bounds checking in aggregation
• Issue feat: load genesis network configuration and state #5: Remove duplicate signature parsing
• Issue feat: initial libp2p integration with req-resp status decoding #7: Improve gossip signature deletion logic

The architectural approach is sound, but the critical bugs need resolution before this can safely deploy to devnet 3.

---

Automated review by Claude (Anthropic) · custom prompt

[github-actions]

🤖 Codex Code Review

Findings (ordered by severity)

• Critical: interval tracking is inconsistent and likely skips interval 4 entirely. BlockChainServer::on_tick computes intervals from second-granularity timestamps and passes timestamp.as_secs() into store::on_tick, which converts seconds into 800ms intervals. This causes interval counts to advance in 1s jumps and skip the 4th interval (e.g., 3s→interval 3, 4s→interval 5), so accept_new_attestations at interval 4 never runs and the “never skips intervals” assumption is violated. This can break fork choice weight updates, aggregation cadence, and safe-target timing. Fix by feeding ms-precision timestamps into store and computing interval from milliseconds consistently. Files: crates/blockchain/src/lib.rs:128-151, crates/blockchain/src/store.rs:258-307.
• Critical: aggregated attestations from gossip are not verified. on_gossip_aggregated_attestation stores payloads and therefore influences fork choice without checking the aggregated signature/proof or participant indices. A malicious peer can inject weight and bias LMD GHOST. You should verify the aggregated proof against participant pubkeys (and ensure all participant indices are in range) before inserting payloads. Files: crates/blockchain/src/store.rs:397-429, and fork-choice usage via extract_attestations_from_aggregated_payloads in crates/blockchain/src/store.rs:93-121.
• High: store::on_tick subtracts timestamp - genesis_time without saturation. If local time is before genesis, this underflows and yields a huge time_delta_ms, forcing a large fast-forward and corrupting state timing. Use saturating_sub or guard with a check. File: crates/blockchain/src/store.rs:258-262.
• Medium: time validation is inconsistent. validate_attestation_data allows slot <= current_slot + 1 for clock skew, but on_gossip_attestation immediately rejects any slot > current_slot, eliminating that skew tolerance and contradicting the comment. Align these policies to avoid dropping valid attestations when local time lags slightly. Files: crates/blockchain/src/store.rs:240-335.
• Medium: aggregate_committee_signatures indexes validators[*vid as usize] without bounds checks. If gossip-signature entries are corrupted or from a previous buggy version, this can panic and DoS the node. Add bounds checks and drop invalid entries (and consider pruning them). File: crates/blockchain/src/store.rs:163-169.

If you want, I can sanity-check the expected 800ms schedule and propose a concrete patch for the tick/time plumbing and aggregated attestation verification.

---

Automated review by OpenAI Codex · custom prompt

[greptile-apps]

Greptile Summary

This PR implements a major protocol upgrade from pq-devnet-2 to pq-devnet-3, introducing a new 5-interval timing model (5 × 800ms = 4s slots) and replacing the per-validator attestation architecture with an aggregated payload pipeline.

Key architectural changes:

• Timing model: Migrated from 4 × 1s intervals to 5 × 800ms intervals with reorganized slot phases (interval 0: attestation acceptance, interval 1: vote propagation, interval 2: aggregation, interval 3: safe target update, interval 4: final acceptance)
• Attestation aggregation: Replaced LatestNewAttestations/LatestKnownAttestations tables with LatestNewAggregatedPayloads/LatestKnownAggregatedPayloads, centralizing signature aggregation at aggregator nodes at interval 2
• Network topology: Added per-committee attestation subnets (/leanconsensus/devnet3/attestation_{subnet_id}/ssz_snappy) and a new aggregation gossip topic for distributing aggregated attestations
• Fork choice: Updated to reconstruct per-validator attestations from aggregated payloads via content-addressed attestation data storage (AttestationDataByRoot)

Implementation quality:

• The timing calculations correctly handle millisecond-to-interval conversions
• Storage layer properly separates "new" (pending) and "known" (active) aggregated payloads
• Validation logic strengthened with additional head/target/source consistency checks
• Fork choice tree visualization added for debugging

Issues found:

• Network name remains "devnet0" in code despite PR description stating it should be "devnet3" - needs clarification
• Inconsistent error handling in topic subscriptions (mix of .unwrap() and ?)
• Removed .validate_messages() from gossipsub config, which may reduce security posture

The changes are comprehensive and architecturally sound for a devnet upgrade, with proper separation of concerns between aggregators and non-aggregators.

Confidence Score: 4/5

• This PR is safe to merge with minor configuration verification needed
• The implementation is architecturally sound with proper timing calculations, storage layer refactoring, and network topology changes. However, there are minor inconsistencies (network name, error handling, gossipsub validation removal) that should be verified before merge. The changes are well-tested with updated fixtures from the leanSpec devnet-3 branch.
• Pay attention to crates/net/p2p/src/lib.rs for network name and error handling consistency verification

Important Files Changed

Filename	Overview
crates/blockchain/src/lib.rs	Updated timing model to 5 intervals of 800ms, added aggregated attestation handling, and refactored interval-based tick scheduling
crates/blockchain/src/store.rs	Major refactor replacing per-validator attestation maps with aggregated payload pipeline, added committee signature aggregation at interval 2
crates/storage/src/store.rs	Replaced attestation storage tables with aggregated payload tables, added attestation-data-by-root content-addressed storage
crates/storage/src/api/tables.rs	Updated table definitions removing old attestation tables, adding new aggregated payload tables and attestation data by root
crates/net/p2p/src/lib.rs	Added per-committee attestation subnets, aggregation topic, and conditional subscription logic for aggregators vs non-aggregators, but has inconsistent error handling
crates/net/p2p/src/gossipsub/handler.rs	Added aggregation topic handler and updated attestation handling to support subnet-based routing

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Gossip: Individual Attestation] -->|Interval 1| B{Is Aggregator?}
    B -->|Yes| C[Store in GossipSignatures]
    B -->|No| D[Discard - Wait for Aggregated]
    
    C -->|Interval 2| E[aggregate_committee_signatures]
    E --> F[Create SignedAggregatedAttestation]
    F --> G[Publish to Aggregation Topic]
    
    G --> H[All Nodes Receive]
    H --> I[Store in LatestNewAggregatedPayloads]
    
    I -->|Interval 0 with proposal| J[promote_new_aggregated_payloads]
    I -->|Interval 4| J
    
    J --> K[LatestKnownAggregatedPayloads]
    K --> L[extract_latest_known_attestations]
    L --> M[Fork Choice: compute_lmd_ghost_head]
    
    N[Block Building] --> O[select_aggregated_proofs]
    K --> O
    O --> P[Block with Aggregated Attestations]

Loading

_{Last reviewed commit: 5918e8b}

[greptile-apps]

_{29 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}