Skip to content

Conversation

@MegaRedHand
Copy link
Collaborator

Motivation

We weren't checking the transactions and withdrawals match with the roots in the header during snap sync body downloads. This was due to one of the body download methods being without validation, which leads to confusion.

Description

This PR changes the method that's used during snap-sync to request_and_validate_block_bodies. It also removes the old request_block_bodies, renaming request_and_validate_block_bodies.

Copilot AI review requested due to automatic review settings November 27, 2025 20:37
@MegaRedHand MegaRedHand requested a review from a team as a code owner November 27, 2025 20:37
@github-actions github-actions bot added the L1 Ethereum client label Nov 27, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a security vulnerability where block body validation was skipped during snap sync. The fix consolidates two similar methods into one that always validates block bodies against their headers by checking that transactions and withdrawals match the roots in the header.

  • Renamed request_and_validate_block_bodies to request_block_bodies and removed the old non-validating version
  • Updated store_block_bodies to accept block headers instead of block hashes to enable validation
  • Modified snap sync code path to pass the pivot block header for validation

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
crates/networking/p2p/peer_handler.rs Removed the old non-validating request_block_bodies method and renamed request_and_validate_block_bodies to request_block_bodies to ensure all block body requests are validated
crates/networking/p2p/sync.rs Updated store_block_bodies to accept block headers instead of hashes, enabling validation; updated the snap sync code path to pass the pivot header for validation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@github-actions
Copy link

Lines of code report

Total lines added: 8
Total lines removed: 11
Total lines changed: 19

Detailed view
+----------------------------------------------+-------+------+
| File                                         | Lines | Diff |
+----------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/peer_handler.rs | 1745  | -11  |
+----------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/sync.rs         | 1380  | +8   |
+----------------------------------------------+-------+------+

@github-project-automation github-project-automation bot moved this to In Review in ethrex_l1 Nov 28, 2025
@MegaRedHand MegaRedHand added this pull request to the merge queue Nov 28, 2025
Merged via the queue into main with commit c00b76c Nov 28, 2025
44 checks passed
@MegaRedHand MegaRedHand deleted the fix-validate-pivot-block-body branch November 28, 2025 16:02
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Nov 28, 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.

4 participants