feat: add devnet4 support

[MegaRedHand]

This PR adds devnet 4 support to ethlambda, removing devnet 3 support.

[github-actions]

🤖 Kimi Code Review

Review Summary

This PR implements devnet4 compatibility for ethlambda, introducing a dual XMSS key system (separate attestation and proposal keys per validator) and simplifying the block structure by removing the proposer attestation wrapper. The changes are extensive but well-structured.

---

Critical Consensus-Layer Changes

1. Dual Key Architecture (Major)

Files: crates/common/types/src/state.rs, crates/blockchain/src/key_manager.rs, bin/ethlambda/src/main.rs

• Change: Each validator now has independent attestation_pubkey and proposal_pubkey (52 bytes each).
• Security: Correctly isolates attestation and proposal signing domains, preventing OTS (one-time signature) exhaustion attacks where a validator might accidentally reuse a key.
• Implementation: ValidatorKeyPair struct properly manages both keys with independent XMSS window advancement (sign_with_attestation_key vs sign_with_proposal_key).

Line-specific notes:

• crates/blockchain/src/key_manager.rs:23-30 - Good documentation explaining why dual keys are needed.
• crates/blockchain/src/key_manager.rs:83-103 and crates/blockchain/src/key_manager.rs:120-140 - Both keys correctly advance their preparation windows independently. This is critical for validators to sign both attestations and blocks in the same slot.

2. Block Structure Simplification

Files: crates/common/types/src/block.rs, crates/blockchain/src/lib.rs, crates/blockchain/src/store.rs

• Change: Removed BlockWithAttestation wrapper and proposer_attestation field. Proposer now signs block_root directly with proposal key.
• Consensus Impact:
  • Breaking change in SSZ schema (SignedBlock replaces SignedBlockWithAttestation).
  • Proposer attestations are now treated as regular gossip attestations, eliminating circular weight advantage in fork choice (proposer no longer gets "free" attestation weight in their own block).
• Verification: verify_signatures in crates/blockchain/src/store.rs:1176-1198 correctly uses:
  • get_attestation_pubkey() for attestation verification
  • get_proposal_pubkey() for block proposer signature verification

3. Attestation Compaction & Limits

Files: crates/blockchain/src/store.rs

• New: MAX_ATTESTATIONS_DATA = 16 limit per block (leanSpec #536 compliance).
• New: compact_attestations function merges duplicate AttestationData entries by unioning aggregation bits.
• Validation: on_block_core checks for duplicates using HashSet (lines 569-580).

Security consideration: The duplicate check prevents validators from including the same attestation data multiple times to artificially inflate block weight or exploit fork choice edge cases.

---

Cryptographic Updates

XMSS Scheme Upgrade

Files: crates/common/types/src/signature.rs, crates/common/crypto/src/lib.rs

• Change: Updated from lifetime_2_to_the_32::hashing_optimized::SIGTopLevelTargetSumLifetime32Dim64Base8 to instantiations_aborting::lifetime_2_to_the_32::SchemeAbortingTargetSumLifetime32Dim46Base8.
• Signature size: Reduced from 3112 to 2536 bytes (correct for new scheme).
• Aggregation: Updated to use xmss_aggregate with log_inv_rate=2 (cross-client compatibility with zeam, ream, lantern).

Verification: The new scheme uses aborting hypercube message hash which is safer against certain forgery attacks.

---

Network & Storage

P2P Protocol Updates

Files: crates/net/p2p/src/gossipsub/handler.rs, crates/net/p2p/src/req_resp/

• Change: All references updated from SignedBlockWithAttestation to SignedBlock.
• Encoding: SSZ encoding remains compatible (same field order), but wire format changes due to removed wrapper.

Storage Schema

Files: crates/storage/src/store.rs

• Change: BlockSignaturesWithAttestation removed; now stores BlockSignatures directly.
• Migration: Existing databases will need regeneration (breaking change acceptable for devnet).

---

Testing & Spec Compliance

Test Fixture Updates

Files: crates/blockchain/tests/, crates/common/test-fixtures/src/lib.rs

• Good: Test validators now include both attestationPubkey and proposalPubkey in YAML fixtures.
• Good: on_gossip_attestation_without_verification added for fork choice spec tests that don't include signature data.

Fork Choice Test Runner

Files: crates/blockchain/tests/forkchoice_spectests.rs

• Note: Test runner now resolves headRootLabel to headRoot for checks (lines 196-210), handling the fixture format where labels reference previously seen blocks.

---

Minor Issues & Suggestions

1. Error Message Clarity

File: crates/blockchain/src/store.rs:578-580

return Err(StoreError::DuplicateAttestationData {
    count: attestations.len(),
    unique: seen.len(),
});

The error counts total attestations vs unique, but the check happens after seeing a duplicate. Consider reporting which specific AttestationData was duplicated for easier debugging.

2. Key Loading Validation

File: bin/ethlambda/src/main.rs:271-286
The classify_role function correctly parses filenames for "attester" and "proposer" substrings. This matches zeam and lantern conventions, but consider making this more robust with explicit metadata in the YAML rather than filename parsing.

3. Comment Accuracy

File: crates/blockchain/src/store.rs:1179-1183
The comment mentions "bounds check in a single pass" but the code uses .map() followed by .collect::<Result<_, _>>(). This is fine but the comment is slightly misleading—it's not a single pass in the traditional sense, though it is iterator-based.

4. SSZ Test Data

File: crates/net/p2p/test_data/signed_block.ssz
The test file is renamed but the test in encoding.rs is ignored with comment "needs devnet4 block". Ensure test data is regenerated before merging.

---

Security Assessment

Component	Assessment	Notes
Key Separation	✅ Secure	Proper isolation prevents cross-protocol attacks
XMSS Window	✅ Correct	Both keys advance independently, preventing exhaustion
Signature Verification	✅ Correct	Uses correct pubkey type per signature purpose
Attestation Limits	✅ Compliant	MAX_ATTESTATIONS_DATA prevents DoS
Duplicate Detection	✅ Secure	Prevents attestation stuffing attacks

---

Conclusion

This PR is ready for merge pending resolution of minor test data regeneration. The dual-key architecture is correctly implemented, the block structure simplification removes technical debt, and all consensus-critical paths (fork choice, justification, finalization) are properly updated.

Recommended follow-ups:

1. Regenerate signed_block.ssz test fixture.
2. Update documentation (CLAUDE.md changes look good, but verify all diagrams).
3. Run full sync test against devnet4 before tagging release.

---

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

[github-actions]

🤖 Codex Code Review

1. store.rs and store.rs: compact_attestations is cryptographically invalid. When multiple proofs for the same AttestationData are selected, it unions the participant bits but discards the real proof bytes and replaces them with AggregatedSignatureProof::empty(...). The resulting SignedBlock cannot pass verify_aggregated_signature for the merged participant set, so any block that actually hits this path will be produced invalid and rejected by other nodes. This needs either “at most one proof per AttestationData” or real re-aggregation of proofs, not bitset union plus an empty proof.

2. store.rs, handler.rs, and codec.rs: the PR changes the SSZ shape from SignedBlockWithAttestation / BlockSignaturesWithAttestation to SignedBlock / BlockSignatures, but there is no migration or protocol/version gate. Existing on-disk blocks will now panic on expect("valid signatures") when read back, and mixed-version peers will silently drop each other’s blocks at decode time. If this rollout assumes a fresh DB and homogeneous network, that needs to be enforced or documented explicitly.

I did not run tests successfully because cargo/rustup is blocked in this sandbox by a read-only ~/.rustup temp path. Beyond the two points above, the split attestation/proposal key usage in the verification paths looks consistent.

---

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

[github-actions]

🤖 Claude Code Review

Here is the code review for PR #283:

---

PR Overview

This PR migrates ethlambda to the devnet4 protocol by:

• Introducing dual XMSS keys per validator (attestation_key + proposal_key) to prevent OTS reuse when a proposer signs both an attestation and a block in the same slot
• Removing the BlockWithAttestation / proposer_attestation wrapper — the proposer now signs the block root directly with the proposal key
• Allowing all validators (including proposer) to attest at interval 1
• Switching to a new XMSS scheme (dim 46, 2536-byte sigs) and updating lean-multisig API
• Fixing log_inv_rate from 1 → 2 for cross-client aggregation compatibility
• Adding a MAX_ATTESTATIONS_DATA = 16 per-block cap with deduplication enforcement

The dual-key design and the block-root signing change are architecturally sound.

---

Critical Issues

aggregate_signatures swallows all error information (crates/common/crypto/src/lib.rs)

// New
let (_sorted_pubkeys, aggregate) = xmss_aggregate(&[], raw_xmss, &message.0, slot, 2);
let serialized = aggregate.serialize();

xmss_aggregate now returns a bare tuple, not a Result. If the function panics internally the node crashes without a recoverable error path. The leftover AggregationError::AggregationPanicked variant is defined but never constructed — it is dead code. If the upstream API is truly infallible, remove the variant and add a comment. If it can panic, wrap the call in std::panic::catch_unwind and return AggregationPanicked.

---

compact_attestations silently discards real cryptographic proof data (crates/blockchain/src/store.rs)

// When merging duplicates:
compacted_proofs.push(AggregatedSignatureProof::empty(merged_bits.clone()));

Any non-empty proof_data in the merged entries is thrown away. In the current block-building path this cannot trigger (build_block already deduplicates via processed_data_roots), making compact_attestations a dead-code safety belt in practice. However, if it ever fires on real proofs the block will fail signature verification on the receiver side. At minimum add a debug_assert!(items[idx].take()... .proof_data.is_empty()) guard to surface this if it occurs, or document explicitly that the function is only called on empty proofs.

---

Notable Concerns

leansig pinned to a branch, not a commit hash (Cargo.toml)

leansig = { git = "https://github.com/leanEthereum/leanSig", branch = "devnet4" }

Any push to devnet4 will silently change the dependency on cargo update. For a consensus client, reproducible builds are critical. Pin to a specific rev once the branch stabilises.

---

_pubkey_hex is parsed but never verified against the loaded secret key (bin/ethlambda/src/main.rs:252)

The comment acknowledges this: "leansig doesn't expose any pk getters." A validator misconfiguring attester and proposer key files swapped would sign with the wrong key and produce invalid blocks, with no detection at startup. This is a known limitation, but it should be tracked as a follow-up issue since it's a silent misconfiguration surface.

---

Cross-client XMSS compatibility tests removed without replacement (crates/common/crypto/src/lib.rs)

The ream cross-client test vectors (4 tests) were removed. This is expected because the signature scheme changed, but there are now zero cross-client signature compatibility tests. New test vectors from ream / zeam / grandine against the devnet4 scheme should be added before this PR merges.

---

Minor Issues

sign_with_proposal_key has no metrics (crates/blockchain/src/key_manager.rs:100-128)

sign_with_attestation_key records timing and increments inc_pq_sig_attestation_signatures. sign_with_proposal_key has neither. Add equivalent counters so proposal signing is observable.

---

stf_spectests.rs justified slots comparison iterates fixture length, not state length

// Before: iterates post_state.justified_slots.len()
// After:
let post_slots: Vec<bool> = (0..justified_slots.data.len())
    .map(|i| post_state.justified_slots.get(i) == Some(true))
    .collect();

If the state's bitfield is longer than the fixture (post_state.justified_slots.len() > justified_slots.data.len()), extra bits set in the state are not checked. The direction should be the union, or the assertion should verify both lengths match before comparing.

---

on_block_rejects_duplicate_attestation_data test uses 0 validators (crates/blockchain/src/store.rs:1676)

The test creates State::from_genesis(1000, vec![]) and submits a block with proposer_index: 0. The proposer-validity check runs before the duplicate-attestation check. Calling is_proposer(0, 1, 0) with num_validators = 0 is undefined (modulo by zero). The test should either use a state with at least one validator, or place the duplicate-attestation check before the proposer check.

---

deser_pubkey_hex error message loses field context (crates/common/types/src/genesis.rs:32)

.map_err(|_| D::Error::custom(format!("pubkey is not valid hex: {s}")))?;

The old per-array-index message ("GENESIS_VALIDATORS[{idx}] is not valid hex") is gone. Serde adds struct field context at the outer level, so this is likely acceptable. Minor.

---

ValidatorKeyRole doesn't implement Display, used with {role:?} in error string (bin/ethlambda/src/main.rs:351)

format!("validator {}: duplicate {role:?} entry", entry.index) uses Debug formatting. The output (Attestation / Proposal) is readable, but implementing Display would produce cleaner error messages for operators.

---

Good Changes

• read_validator_keys now returns Result<_, String> and propagates errors cleanly instead of panicking at multiple sites. The classify_role helper is clear and matches zeam/lantern semantics.
• The compact_attestations function is well-tested (no-duplicates, merge, order-preservation cases) and the on_block_rejects_duplicate_attestation_data integration test covers the rejection path.
• log_inv_rate = 2 fix is well-documented with a cross-client rationale comment. This is a real correctness fix.
• Removing BlockWithAttestation and the proposer-attestation wrapper simplifies the type hierarchy significantly. The diff is mechanical and consistent across all layers.
• Checkpoint sync verification correctly extended to check both attestation_pubkey and proposal_pubkey (bin/ethlambda/src/checkpoint_sync.rs:148).

---

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