Skip to content

feat: block bodies#1057

Merged
antouhou merged 94 commits into
masterfrom
feat-block-bodies
Jan 7, 2026
Merged

feat: block bodies#1057
antouhou merged 94 commits into
masterfrom
feat-block-bodies

Conversation

@antouhou

@antouhou antouhou commented Dec 10, 2025

Copy link
Copy Markdown
Contributor

Describe the changes
This PR adds block bodies to consolidate transaction processing for block validation

Related Issue(s)

Checklist

  • Tests have been added/updated for the changes.
  • Documentation has been updated for the changes (if applicable).
  • The code follows Rust's style guidelines.

Additional Context

Summary by CodeRabbit

  • New Features

    • Gossip v2: header/body model, richer BlockBody objects, block-body caching, public Gossip server/routes, and v2 broadcast for mempool/gossip.
    • Handshake-based peer negotiation replaces legacy POST /version endpoint.
    • Mempool facade: internal read guard for safe multi-query internal reads; peers now advertise protocol version.
  • Bug Fixes & Improvements

    • Stronger header/body validation, signature checks, retries, and explicit mismatch reporting.
    • Protocol-aware routing with V1↔V2 bridging and negotiated protocol versions.

✏️ Tip: You can customize this high-level summary in your review settings.

@antouhou antouhou marked this pull request as ready for review December 11, 2025 16:20
Comment thread crates/domain/src/models/peer_list.rs
Comment thread crates/types/src/version.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
crates/types/src/version.rs (2)

271-288: Doc comment still references old type name.

The doc comment on line 271 mentions "AcceptedResponse" but the struct has been renamed to HandshakeResponse. Consider updating the comment for consistency.

📝 Suggested fix
-/// Example serialized JSON AcceptedResponse:
+/// Example serialized JSON HandshakeResponse:

389-407: Consider updating variable name for consistency.

The test uses variable name version_request (line 391) which still reflects the old type name. Consider renaming to handshake_request for consistency with the type rename.

📝 Suggested fix
     #[test]
     fn should_sign_and_verify_signature() {
-        let mut version_request = HandshakeRequest::default();
+        let mut handshake_request = HandshakeRequest::default();
         let testing_config = NodeConfig::testing();
         let config = Config::new(testing_config);
         let signer = config.irys_signer();

-        signer.sign_p2p_handshake(&mut version_request).unwrap();
+        signer.sign_p2p_handshake(&mut handshake_request).unwrap();
         assert!(
-            version_request.verify_signature(),
+            handshake_request.verify_signature(),
             "Signature should be valid"
         );

-        version_request.signature = IrysSignature::default();
+        handshake_request.signature = IrysSignature::default();
         assert!(
-            !version_request.verify_signature(),
+            !handshake_request.verify_signature(),
             "Signature should be invalid after reset"
         );
     }
📜 Review details

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a09edfe and 099e2b2.

📒 Files selected for processing (1)
  • crates/types/src/version.rs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: antouhou
Repo: Irys-xyz/irys PR: 1057
File: crates/p2p/src/gossip_client.rs:143-170
Timestamp: 2026-01-05T16:21:34.820Z
Learning: In crates/p2p/src/gossip_client.rs, methods like pull_block_body_from_v1_peer intentionally convert all errors (Err(_)) to Ok(GossipResponse::Accepted(None)) because these methods must never return errors by design. This protocol will be replaced with direct socket communication later, so the current approach of treating network failures uniformly as "data not found" is acceptable.
📚 Learning: 2026-01-05T16:21:34.820Z
Learnt from: antouhou
Repo: Irys-xyz/irys PR: 1057
File: crates/p2p/src/gossip_client.rs:143-170
Timestamp: 2026-01-05T16:21:34.820Z
Learning: In crates/p2p/src/gossip_client.rs, methods like pull_block_body_from_v1_peer intentionally convert all errors (Err(_)) to Ok(GossipResponse::Accepted(None)) because these methods must never return errors by design. This protocol will be replaced with direct socket communication later, so the current approach of treating network failures uniformly as "data not found" is acceptable.

Applied to files:

  • crates/types/src/version.rs
🧬 Code graph analysis (1)
crates/types/src/version.rs (1)
crates/types/src/peer_list.rs (4)
  • default (90-106)
  • default (121-126)
  • new (29-31)
  • new (449-451)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: cargo doc
  • GitHub Check: cargo test
  • GitHub Check: cargo clippy
  • GitHub Check: cargo check
🔇 Additional comments (4)
crates/types/src/version.rs (4)

15-22: LGTM!

The rename from AcceptedResponse to HandshakeResponse in the Accepted variant is consistent with the broader handshake terminology refactor throughout this PR.


24-62: LGTM!

The ProtocolVersion expansion is well-structured:

  • Hash and Arbitrary derives enable use in hash-based collections and property testing.
  • The #[repr(u32)] ensures stable ABI for the enum discriminants.
  • current() being const fn allows compile-time evaluation.
  • supported_versions_u32() correctly uses the repr cast.

The From<u32> fallback behavior was already discussed in prior review comments and validated by the author.


149-206: LGTM!

The HandshakeRequest implementation is solid:

  • Signing includes protocol_version (line 186), binding the handshake to a specific protocol version.
  • The encode_for_signing correctly excludes the signature itself, preventing circular dependencies.
  • Timestamp truncation from u128 to u64 is safe for practical timescales.

The rename from VersionRequest aligns with the handshake terminology used throughout the PR.


297-310: LGTM!

The HandshakeResponse default implementation is consistent with HandshakeRequest, both defaulting to the current protocol version (V2).

// Read protocol_version (4 bytes) if available. If it's not available, it means that it's a
// v1 record, so we default to ProtocolVersion::V1.
let mut protocol_version = ProtocolVersion::V1;
if buf.len() >= total_consumed + 4 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have fuzz testing for this compact impl? we might want it at some point

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not right now, I don't think so

@JesseTheRobot JesseTheRobot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM o7

@antouhou antouhou merged commit 618948d into master Jan 7, 2026
18 checks passed
@antouhou antouhou deleted the feat-block-bodies branch January 7, 2026 21:32
@coderabbitai coderabbitai Bot mentioned this pull request Mar 30, 2026
3 tasks
@coderabbitai coderabbitai Bot mentioned this pull request May 27, 2026
3 tasks
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.

3 participants