Skip to content

Conversation

@tomip01
Copy link
Contributor

@tomip01 tomip01 commented Oct 28, 2025

Motivation

We don't want to broadcast Privileged transactions between peers as every lead sequencer can get them through the OnChainProposer contract and look for the logs.

Description

The P2P messages that broadcast transactions are:

  • Transactions
    • Added a new matches!(..) to deny them
  • NewPooledTransactionHashes
    • Add a new check for sending tx hashes to not be a privileged one
  • GetPooledTransactions
    • Are currently rejected inside the function get_p2p_transaction_by_hash
  • PooledTransactions: the response form the GetPooledTransactions
    • Added a new matches!(..) inside the handle method to reject adding them to the mempool

@tomip01 tomip01 requested a review from a team as a code owner October 28, 2025 20:54
Copilot AI review requested due to automatic review settings October 28, 2025 20:54
@github-actions github-actions bot added the L2 Rollup client label Oct 28, 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 adds rejection logic for privileged L2 transactions when operating in L2 mode, preventing these transactions from being broadcasted through the peer-to-peer network.

  • Added validation to reject privileged L2 transactions in L2 mode alongside existing blob transaction rejections
  • Updated debug messages to provide clearer information about rejected transaction types

Reviewed Changes

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

File Description
crates/networking/p2p/rlpx/eth/transactions.rs Added check to reject privileged L2 transactions in pooled transactions handler
crates/networking/p2p/rlpx/connection/server.rs Expanded transaction rejection logic to include privileged L2 transactions and improved debug message formatting

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

Comment on lines 299 to 301
debug!(
peer=%node,
"Rejecting privileged L2 transaction in L2 mode - privileged L2 transactions are not broadcasted",
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

The error message is inconsistent with the one in server.rs (line 941). Consider using a similar pattern by extracting the transaction type to make messages consistent: let tx_type = tx.tx_type(); debug!(peer=%node, \"Rejecting transaction in L2 mode - {tx_type} transactions are not broadcasted in L2\");

Suggested change
debug!(
peer=%node,
"Rejecting privileged L2 transaction in L2 mode - privileged L2 transactions are not broadcasted",
let tx_type = tx.tx_type();
debug!(
peer=%node,
"Rejecting transaction in L2 mode - {tx_type} transactions are not broadcasted in L2",

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

github-actions bot commented Oct 28, 2025

Lines of code report

Total lines added: 7
Total lines removed: 11
Total lines changed: 18

Detailed view
+--------------------------------------------------------+-------+------+
| File                                                   | Lines | Diff |
+--------------------------------------------------------+-------+------+
| ethrex/crates/common/types/transaction.rs              | 2837  | -11  |
+--------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/connection/server.rs | 1022  | +4   |
+--------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/tx_broadcaster.rs         | 309   | +3   |
+--------------------------------------------------------+-------+------+

@ilitteri ilitteri requested a review from MegaRedHand October 28, 2025 21:09
@github-project-automation github-project-automation bot moved this to Requires Changes in ethrex_l2 Oct 28, 2025
EIP1559Transaction(EIP1559Transaction),
EIP4844TransactionWithBlobs(WrappedEIP4844Transaction),
EIP7702Transaction(EIP7702Transaction),
PrivilegedL2Transaction(PrivilegedL2Transaction),
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably document this change somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 63675e2

@tomip01 tomip01 added this pull request to the merge queue Oct 29, 2025
Merged via the queue into main with commit c04173a Oct 29, 2025
29 checks passed
@tomip01 tomip01 deleted the reject_priv_tx_p2p_l2 branch October 29, 2025 16:15
@github-project-automation github-project-automation bot moved this from Requires Changes to Done in ethrex_l2 Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L2 Rollup client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants