chore: remove from_ssz_bytes_compat for devnet-3 serialization compatibility

[pablodeymo]

Change BlocksByRootRequest::from_ssz_bytes_compat to use SszDecode::from_ssz_bytes with the payload.

We don't need to maintain compatibility with devnet-2 format, so we remove the previous serialization.

[github-actions]

🤖 Kimi Code Review

Review Summary

The PR removes backward compatibility logic for BlocksByRootRequest SSZ decoding. This is a breaking change that will cause network partition between old and new nodes.

Critical Issues

1. Network Compatibility Break (codec.rs:45)

   • Removing from_ssz_bytes_compat means nodes running this code cannot decode requests from older nodes using the direct list format
   • This will cause immediate network partition during rollout

2. Silent Protocol Break (messages.rs:142-144)

   • The BlocksByRootRequest struct is now strictly SSZ-encoded as a container with a roots field
   • No migration strategy or version negotiation mechanism provided

Security & Consensus Impact

• Fork Risk: Network split between old/new clients could lead to different views of the chain
• Attestation Loss: Validators on partitioned network may miss attestations
• No Grace Period: Immediate incompatibility without deprecation warning

Recommendations

1. Keep compatibility logic until a network-wide upgrade is coordinated
2. Add protocol versioning to negotiate supported formats
3. Implement feature flags for gradual rollout
4. Document breaking changes in release notes with migration timeline

The code itself is clean and follows Rust best practices, but the breaking change is too severe for consensus-critical software without proper migration strategy.

---

Automated review by Kimi (Moonshot AI) · custom prompt

[github-actions]

🤖 Codex Code Review

Findings

1. Potential network incompatibility/regression: BlocksByRootRequest::from_ssz_bytes_compat was removed and SszDecode::from_ssz_bytes is now used directly. This drops backward compatibility with the old “transparent list” encoding. If any peers still send the old format, requests will now fail with InvalidData, which can reduce sync robustness or cause request retries. Consider keeping a compatibility fallback or gating it behind a feature/negotiated protocol version.

   • crates/net/p2p/src/req_resp/codec.rs:42-46
   • crates/net/p2p/src/req_resp/messages.rs:139-192 (removal)

2. Test coverage regression: The compatibility test was removed. If the intent is to enforce the new format only, add a new test that asserts old-format decoding fails (so behavior is explicit). If compatibility is still desired, restore the test or add one for both.

   • crates/net/p2p/src/req_resp/messages.rs:155-192 (removal)

Consensus-layer considerations
No consensus logic touched in this PR (fork choice, attestations, justification/finalization, state transition, XMSS, SSZ of consensus objects).

Summary
The only functional change is stricter SSZ decoding for BlocksByRootRequest. It is correct if the network protocol guarantees the new container format, but it may break interoperability with older peers. If that’s intentional, document it and add an explicit failing test.

Residual risk
Interoperability with peers still sending the old encoding.

---

Automated review by OpenAI Codex · custom prompt

[greptile-apps]

Greptile Overview

Greptile Summary

This PR removes backward compatibility code for BlocksByRootRequest SSZ deserialization. The changes:

• Replaced BlocksByRootRequest::from_ssz_bytes_compat() with standard SszDecode::from_ssz_bytes() in codec.rs:44-47
• Removed the entire from_ssz_bytes_compat() method implementation from messages.rs:142-155 (including fallback logic for old format)
• Removed the test validating backward compatibility (test_blocks_by_root_backward_compatibility in messages.rs:158-195)
• Cleaned up unused imports (BlocksByRootRequest from codec.rs, SszDecode alias from messages.rs)

The backward compatibility was originally added in #63 (commit c4ecabb, Jan 30 2026) to support both the new container format (used by Zeam) and old transparent format (used by other clients). This PR assumes all clients have migrated to the new format.

Confidence Score: 4/5

• Safe to merge assuming all network peers have upgraded to the new BlocksByRoot format
• The code changes are clean and technically correct - removing backward compatibility code and using standard SSZ deserialization. However, this is a breaking change for network compatibility. If any peers still use the old format, they will fail to communicate. Score is 4 (not 5) because the PR description doesn't explicitly confirm all clients have migrated, which was the condition mentioned in the original backward compatibility commit.
• No files require special attention - changes are straightforward

Important Files Changed

Filename	Overview
crates/net/p2p/src/req_resp/codec.rs	Replaced from_ssz_bytes_compat with standard from_ssz_bytes, removed backward compatibility import, cleaned up unused import
crates/net/p2p/src/req_resp/messages.rs	Removed backward compatibility method from_ssz_bytes_compat and its tests, cleaned up unused import

_{Last reviewed commit: 468ee98}

[github-actions]

🤖 Claude Code Review

Let me review based on what I can see from the diff and the code. I'll compile my findings now.

Based on my review of PR #127, here are my findings:

---

PR #127 Review: Remove backward compatibility from BlocksByRootRequest

Summary

This PR removes the from_ssz_bytes_compat method and its backward compatibility logic from BlocksByRootRequest, replacing it with a direct call to the standard SSZ decoder. The change deletes 59 lines (including tests) and adds only 7 lines.

Findings

1. Breaking Protocol Change (Critical)

The removed from_ssz_bytes_compat method had fallback logic to support both:

• New format: Container with roots field
• Old format: Transparent/direct list of roots

Impact: This PR will break compatibility with peers running older versions of ethlambda that send BlocksByRoot requests in the old format. When an old-format request arrives, SSZ decoding will fail and return an io::Error with InvalidData kind (codec.rs:46).

Questions to address:

• Is there a coordinated network upgrade that makes all nodes incompatible with the old format?
• Have all other lean consensus clients (zeam, ream, qlean, grandine, gean, Lantern) migrated to the new format?
• What happens to peers who fail to decode requests - are they disconnected or just logged?

2. Test Coverage Removed

The PR removes test_blocks_by_root_backward_compatibility() which verified both encoding formats work. There's no replacement test coverage for the standard format.

Recommendation: Add a basic test to verify BlocksByRootRequest can be encoded and decoded correctly:

#[test]
fn test_blocks_by_root_request_encode_decode() {
    let root1 = H256::from_slice(&[1u8; 32]);
    let root2 = H256::from_slice(&[2u8; 32]);
    let roots = RequestedBlockRoots::new(vec![root1, root2]).unwrap();
    let request = BlocksByRootRequest { roots };
    
    let encoded = request.as_ssz_bytes();
    let decoded = BlocksByRootRequest::from_ssz_bytes(&encoded).unwrap();
    
    assert_eq!(decoded.roots.len(), 2);
    assert_eq!(decoded.roots[0], root1);
    assert_eq!(decoded.roots[1], root2);
}

3. Code Quality (Good)

The simplification is excellent from a maintainability perspective:

• Removes complexity and technical debt
• Uses idiomatic Rust patterns (the standard SszDecode::from_ssz_bytes)
• Consistent with Status message handling (codec.rs:39)
• Properly removes unused import of BlocksByRootRequest from codec.rs

4. Unused Import (Minor Issue)

In codec.rs:16, there's now a redundant import alias:

use ethlambda_types::primitives::ssz::Decode as SszDecode;

This is imported solely for the BlocksByRootRequest decoding at line 45. Since Decode is already imported at line 3, you could simplify:

Current (line 45):

let request = SszDecode::from_ssz_bytes(&payload).map_err(|err| {

Could be:

let request = BlocksByRootRequest::from_ssz_bytes(&payload).map_err(|err| {

Then remove the SszDecode import at line 16. This is purely stylistic - both approaches work.

5. Error Handling (Good)

Error handling remains consistent - SSZ decode errors are properly converted to io::Error with InvalidData kind and include the debug representation of the error.

Verdict

Conditionally approve pending clarification on:

1. Protocol coordination: Confirm this is part of a coordinated network upgrade and all clients have migrated to the container format
2. Test coverage: Add basic encode/decode test for the current format

If this is a breaking protocol change without coordination, it should either:

• Be delayed until all clients upgrade
• Keep the compatibility layer with a deprecation timeline
• Include a protocol version bump in the stream protocol string (e.g., /leanconsensus/req/blocks_by_root/2/ssz_snappy)

The code quality of the change itself is good, but the protocol implications need careful consideration for a consensus client.

---

Automated review by Claude (Anthropic) · custom prompt

[MegaRedHand]

🔥