Skip to content

refactor(l1): decouple version-specific rlpx messages#4395

Merged
fmoletta merged 21 commits into
mainfrom
decouple-versioned-messages
Sep 17, 2025
Merged

refactor(l1): decouple version-specific rlpx messages#4395
fmoletta merged 21 commits into
mainfrom
decouple-versioned-messages

Conversation

@fmoletta

@fmoletta fmoletta commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

Motivation
We already know which version of messages such as Status and Receipts we are going to send/expecting to receive, so we should handle them separately instead of having unified logic for both eth/68 & eth/68 variants. This will save us from having overly-complicated logic when decoding these messages.

Description

  • Remove Receipts enum message and instead use Receipts68 & Receipts69 directly
  • Remove Status enum message and instead use StatusMessage68 & StatusMessage69 directly. Add StatusMessage trait to allow using pre-existing status validation logic

Closes #3032

@github-actions github-actions Bot added the L1 Ethereum client label Sep 9, 2025
@github-actions

github-actions Bot commented Sep 9, 2025

Copy link
Copy Markdown

Lines of code report

Total lines added: 151
Total lines removed: 216
Total lines changed: 367

Detailed view
+---------------------------------------------------------+-------+------+
| File                                                    | Lines | Diff |
+---------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/peer_handler.rs            | 1670  | +6   |
+---------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/connection/server.rs  | 854   | +26  |
+---------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/eth/backend.rs        | 92    | -1   |
+---------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/eth/eth68/receipts.rs | 65    | +3   |
+---------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/eth/eth68/status.rs   | 107   | +48  |
+---------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/eth/eth69/receipts.rs | 43    | +3   |
+---------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/eth/eth69/status.rs   | 107   | +47  |
+---------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/eth/receipts.rs       | 81    | -118 |
+---------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/eth/status.rs         | 9     | -97  |
+---------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/message.rs            | 295   | +18  |
+---------------------------------------------------------+-------+------+

@fmoletta fmoletta force-pushed the decouple-versioned-messages branch from 227231f to 36e2211 Compare September 9, 2025 19:27
@fmoletta fmoletta marked this pull request as ready for review September 9, 2025 21:19
@fmoletta fmoletta requested a review from a team as a code owner September 9, 2025 21:19
@ethrex-project-sync ethrex-project-sync Bot moved this to In Review in ethrex_l1 Sep 9, 2025
Base automatically changed from fix-block-range-update to main September 11, 2025 13:41
StatusMessage::StatusMessage69(msg) => msg.network_id,
}
}
pub trait StatusMessage {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This trait is implemented by both the 68 and 69 version status messages, but it isn't used anywhere else, right? In that case, we could drop it.

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.

Its used so that we don't have to duplicate the logic in validate_status


impl RLPxMessage for Receipts68 {
const CODE: u8 = 0x0F;
const CODE: u8 = 0x10;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good catch!

@mpaulucci mpaulucci left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🚀

@fmoletta fmoletta added this pull request to the merge queue Sep 17, 2025
Merged via the queue into main with commit a429e2d Sep 17, 2025
41 checks passed
@fmoletta fmoletta deleted the decouple-versioned-messages branch September 17, 2025 15:04
@github-project-automation github-project-automation Bot moved this from In Review to Done in ethrex_l1 Sep 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Only use the highest shared capability

3 participants