Skip to content

fix(l1): use eth capability version when decoding messages + fix BlockRangeUpdate handling#4360

Merged
fmoletta merged 17 commits into
mainfrom
fix-block-range-update
Sep 11, 2025
Merged

fix(l1): use eth capability version when decoding messages + fix BlockRangeUpdate handling#4360
fmoletta merged 17 commits into
mainfrom
fix-block-range-update

Conversation

@fmoletta

@fmoletta fmoletta commented Sep 8, 2025

Copy link
Copy Markdown
Contributor

Motivation
Being able to support eth/69 messages

Description

  • Add missing validation to BlockRangeUpdate (latest_hash != zero)
  • Disconnect from peers if BlockRangeUpdate is invalid
  • Enable eth/69 as supported capability
  • Use negotiated eth capability version when decoding incoming messages
  • Enable hive tests for BlockRangeUpdate

Closes #2785

@github-actions github-actions Bot added the L1 Ethereum client label Sep 8, 2025
@fmoletta fmoletta force-pushed the fix-block-range-update branch from 6d9ea4f to 66f4cdc Compare September 8, 2025 19:02
@github-actions

github-actions Bot commented Sep 8, 2025

Copy link
Copy Markdown

Lines of code report

Total lines added: 112
Total lines removed: 0
Total lines changed: 112

Detailed view
+-----------------------------------------------------------+-------+------+
| File                                                      | Lines | Diff |
+-----------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/metrics.rs                   | 481   | +6   |
+-----------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/connection/codec.rs     | 231   | +22  |
+-----------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/connection/handshake.rs | 501   | +6   |
+-----------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/connection/server.rs    | 802   | +23  |
+-----------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/error.rs                | 97    | +2   |
+-----------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/eth/update.rs           | 65    | +6   |
+-----------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/message.rs              | 277   | +47  |
+-----------------------------------------------------------+-------+------+

@fmoletta fmoletta changed the title fix(l1): validate incoming BlockRangeUpdate message fix(l1): use eth capability version when decoding messages + fix BlockRangeUpdate handling Sep 9, 2025
@fmoletta fmoletta marked this pull request as ready for review September 9, 2025 17:30
@fmoletta fmoletta requested a review from a team as a code owner September 9, 2025 17:30
@ethrex-project-sync ethrex-project-sync Bot moved this to In Review in ethrex_l1 Sep 9, 2025
- uses: actions/checkout@v4
- name: Setup Hive
# TODO: Change repo back to https://github.com/ethereum/hive when
# https://github.com/ethereum/hive/pull/1325 is merged

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.

can you remove this comment?

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.

The whole comment or just the part mentioning the PR? As we are still using our forked hive

*self
.eth_version
.read()
.map_err(|err| RLPxError::InternalError(err.to_string()))?,

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.

if we fail to decode a message, it's not necesarily an internal error, right? It could be that they sent us random bytes?

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.

The mapping there is for when we fail to read from the RwLock that holds the eth_version, so it is indeed an internal error

@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 11, 2025
@fmoletta fmoletta removed this pull request from the merge queue due to a manual request Sep 11, 2025
@fmoletta fmoletta added this pull request to the merge queue Sep 11, 2025
Merged via the queue into main with commit 8d31871 Sep 11, 2025
38 of 39 checks passed
@fmoletta fmoletta deleted the fix-block-range-update branch September 11, 2025 13:41
@github-project-automation github-project-automation Bot moved this from In Review to Done in ethrex_l1 Sep 11, 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.

[EIP-7642] Implement eth/69

3 participants