feat(l1): reject contract senders at mempool admission (EIP-3607)#6600
feat(l1): reject contract senders at mempool admission (EIP-3607)#6600ilitteri wants to merge 7 commits into
Conversation
Adds the EIP-3607 sender check that every major Ethereum execution client (geth, reth, nethermind, erigon, besu) enforces. Transactions whose recovered sender has deployed code are rejected, except where the code is a valid EIP-7702 delegation designation (the account is still an EOA in spirit, just pointing at delegate code). - New helper `is_eip7702_delegation(code: &[u8]) -> bool` plus `EIP7702_DELEGATION_PREFIX = 0xef0100` and `EIP7702_DELEGATED_CODE_LEN = 23` constants in `crates/common/types/account.rs`. - `Blockchain::validate_transaction` now inspects the sender's `code_hash`; if non-empty, it fetches the code (single extra store hit only when the sender actually has code) and routes through `is_eip7702_delegation`. Non-delegation contract senders are rejected with the new `MempoolError::SenderIsContract`. - Six unit tests for the delegation predicate covering: empty, valid 23-byte designation, too-short, too-long, wrong-prefix, and arbitrary contract code.
🤖 Kimi Code ReviewOverall Assessment: The PR correctly implements EIP-3607 (rejecting transactions from accounts with code) with the proper EIP-7702 delegation exception. The logic is sound and well-tested, but there's a typo in the imported constant name. Issues Found1. Typo in constant name (Critical)File: // Current (incorrect):
use ethrex_common::constants::{EMPTY_KECCACK_HASH, ...};
// Should be:
use ethrex_common::constants::{EMPTY_KECCAK_HASH, ...};2. Unnecessary database read for common case (Minor/Performance)File: Suggestion: The check is unavoidable for correctness, but ensure the storage layer efficiently handles cache misses for the empty code case. 3. Error propagation clarityFile: Positive Feedback
RecommendationFix the Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
Lines of code reportTotal lines added: Detailed view |
Greptile SummaryThis PR implements EIP-3607 mempool admission enforcement: transactions from senders with deployed code are rejected unless that code is an EIP-7702 delegation designation (
Confidence Score: 5/5Safe to merge — the change is self-contained, correctly implements the spec, and the fast path for normal EOAs is untouched. The predicate logic matches the EIP-7702 wire format exactly, the unit tests cover all edge cases (empty, valid, too-short, too-long, wrong prefix, arbitrary bytecode), the storage call is only made when necessary, and the conservative Ok(None) handling is appropriate for mempool admission. No correctness issues found. No files require special attention.
|
| Filename | Overview |
|---|---|
| crates/common/types/account.rs | Adds is_eip7702_delegation predicate, EIP7702_DELEGATION_PREFIX, and EIP7702_DELEGATED_CODE_LEN constants; six unit tests cover all boundary cases (empty, valid, short, long, wrong prefix, arbitrary code). Logic and constants match the EIP-7702 spec exactly. |
| crates/blockchain/blockchain.rs | Adds EIP-3607 check inside validate_transaction: inspects sender's code_hash, fetches bytecode only when non-empty, and routes through is_eip7702_delegation; non-delegation contract senders are rejected. Storage call is correctly synchronous and the check is correctly placed within the existing sender_acc_info guard. |
| crates/blockchain/error.rs | Adds SenderIsContract variant to MempoolError with a descriptive error message referencing EIP-3607. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[validate_transaction] --> B[Get sender account info]
B --> C{Account exists?}
C -- No --> D[Return NotEnoughBalance]
C -- Yes --> E{code_hash == EMPTY_KECCACK_HASH?}
E -- Yes: EOA, no code --> F[Skip EIP-3607 check]
E -- No: account has code --> G[get_account_code from storage]
G --> H{Code found?}
H -- Ok None: DB inconsistency --> I[is_delegation = false]
H -- Err --> J[Return StoreError]
H -- Ok Some code --> K[is_eip7702_delegation check]
K --> L{Starts with 0xef0100 AND len == 23?}
L -- Yes --> M[is_delegation = true]
L -- No --> I
M --> N[Allow: EIP-7702 carve-out]
I --> O[Return SenderIsContract]
F --> P[Continue balance & nonce checks]
N --> P
Reviews (1): Last reviewed commit: "feat(l1): reject contract senders at mem..." | Re-trigger Greptile
🤖 Claude Code ReviewNow I have enough context. Here's the review: PR Review: feat(l1): reject contract senders at mempool admission (EIP-3607)SummaryThe implementation is correct and well-structured. The EIP-3607 check is placed at the right point in Issues1. Constant/predicate duplication — the new common definition is not yet canonical This PR adds the third independent definition of the same magic bytes and length:
Placing the canonical definition in // levm/src/utils.rs
pub fn code_has_delegation(code: &Bytes) -> Result<bool, VMError> {
Ok(is_eip7702_delegation(code.as_ref()))
}The 2.
Ok(None) => false,If 3. Missing integration-level test for the rejection path The six unit tests verify Minor observations
VerdictCorrect behavior, good test coverage of the predicate, and appropriate placement in the common crate. The main ask before merge is addressing the duplication (Item 1) — either update the levm/L2 callers to use the new common function, or open a tracking issue for the follow-up and note it in the PR description. Item 2 (the Automated review by Claude (Anthropic) · sonnet · custom prompt |
There was a problem hiding this comment.
Pull request overview
This PR adds EIP-3607 enforcement at mempool admission by rejecting transactions whose recovered sender has deployed code, while allowing the EIP-7702 “delegation designation” bytecode pattern as an exception.
Changes:
- Introduces an
is_eip7702_delegation(code: &[u8]) -> boolhelper plus constants for the EIP-7702 delegation prefix/length. - Adds
MempoolError::SenderIsContractand wires sender-code checks intoBlockchain::validate_transaction. - Adds unit tests validating the delegation predicate against several edge cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/common/types/account.rs | Adds EIP-7702 delegation predicate/constants and unit tests for delegation bytecode detection. |
| crates/blockchain/error.rs | Adds a dedicated mempool error variant for EIP-3607 contract-sender rejection. |
| crates/blockchain/blockchain.rs | Enforces EIP-3607 at mempool validation time, with an EIP-7702 delegation exception by inspecting sender code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let is_delegation = match self.storage.get_account_code(sender_acc_info.code_hash) { | ||
| Ok(Some(code)) => is_eip7702_delegation(code.bytecode.as_ref()), | ||
| Ok(None) => false, |
| let is_delegation = match self.storage.get_account_code(sender_acc_info.code_hash) { | ||
| Ok(Some(code)) => is_eip7702_delegation(code.bytecode.as_ref()), | ||
| Ok(None) => false, | ||
| Err(e) => return Err(e.into()), | ||
| }; | ||
| if !is_delegation { |
| // EIP-3607: reject txs from senders with deployed code, unless the | ||
| // code is an EIP-7702 delegation designation (the account is still | ||
| // an EOA in spirit, just pointing at delegate code). | ||
| if sender_acc_info.code_hash != *EMPTY_KECCACK_HASH { | ||
| let is_delegation = match self.storage.get_account_code(sender_acc_info.code_hash) { | ||
| Ok(Some(code)) => is_eip7702_delegation(code.bytecode.as_ref()), | ||
| Ok(None) => false, | ||
| Err(e) => return Err(e.into()), | ||
| }; | ||
| if !is_delegation { | ||
| return Err(MempoolError::SenderIsContract); | ||
| } | ||
| } |
🤖 Codex Code Review
No other obvious correctness, gas-accounting, or EVM-consensus issues stood out in the diff. I couldn’t run Rust tests here because Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Replaces the `match get_account_code` block with the idiomatic `?`-chain + `is_some_and`. Same semantics, no behavior change: error propagation via `?`, `Ok(None)` (code-hash set but code body missing — state inconsistency) remains a "not a delegation" result and falls through to the `SenderIsContract` rejection, which is the conservative choice peer clients also make. Doc-comment updated to document this intent explicitly so a future "fix" doesn't accidentally admit inconsistent storage. Follow-up not in this PR: a full integration test of `validate_transaction → SenderIsContract` requires seeding sender state (AccountInfo + code) via `apply_account_updates_batch`, which the test scaffolding in `test/tests/blockchain/mempool_tests.rs` doesn't currently do. The six unit tests of `is_eip7702_delegation` already cover the byte-level predicate; the six-line wire-up here is visually verifiable.
Copilot + @MegaRedHand flagged that `validate_transaction` decodes the sender's full bytecode via `get_account_code` for any non-empty `code_hash` just to conclude "not a delegation". For large contracts this is a wasted DB hit on every admission. Use `Store::get_code_metadata` (which the storage layer already exposes for length lookups) to fast-reject any code whose length is not exactly `EIP7702_DELEGATED_CODE_LEN` (23 bytes) — those cannot encode the `0xef0100 || address` delegation designation. Only when the metadata length matches do we fetch + verify the prefix.
…607 path Copilot + @codex review: when `code_hash` is non-empty but the bytecode is missing from the store, the previous code silently treated the account as "not a delegation" and rejected the tx with `SenderIsContract`. That masks an inconsistent DB state (code hash present but code missing) and — more importantly — wrongly rejects a valid EIP-7702-delegated EOA whenever the local copy of the delegation bytecode happens to be absent (snapsync recovery, pruning window, etc.). The VM path treats this same condition as a DB error. Now: when the code metadata claims a 23-byte payload, missing bytecode returns `StoreError::Custom("code missing for hash …")` so callers see the real cause rather than a spurious EIP-3607 rejection.
…2_delegation @MegaRedHand: "There are already many implementations of this." The new `is_eip7702_delegation` predicate in `ethrex-common` duplicates the existing levm `code_has_delegation` (and the predicate in `utils.rs:181`). levm's `code_has_delegation` now delegates to the common helper so both crates exercise the same byte-level check. Signature (`Result<bool, VMError>`) is preserved because several callers use `?`; the result is always `Ok(_)` now since the common helper is infallible. The `SET_CODE_DELEGATION_BYTES` / `EIP7702_DELEGATED_CODE_LEN` consts in levm/constants.rs are retained for the in-place slicing in `get_authorized_address_from_code` and adjacent helpers; cross-crate constant unification can be a follow-up.
|
c1ed9c0 — addresses the @codex finding (#6600 (comment)). Missing code is now surfaced as StoreError instead of being silently treated as non-delegation. |
|
To use Codex here, create an environment for this repo. |
Benchmark Results ComparisonNo significant difference was registered for any benchmark run. Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
| // we reject without loading the bytecode. Only when the metadata | ||
| // length matches do we fetch + verify the prefix. This avoids | ||
| // pulling potentially large contract bytecode on every contract | ||
| // sender that hits admission (Copilot / @MegaRedHand review). |
There was a problem hiding this comment.
| // sender that hits admission (Copilot / @MegaRedHand review). | |
| // sender that hits admission. |
| // wrongly reject a valid 7702-delegated EOA per | ||
| // Copilot/@codex review). |
There was a problem hiding this comment.
| // wrongly reject a valid 7702-delegated EOA per | |
| // Copilot/@codex review). | |
| // wrongly reject a valid 7702-delegated EOA). |
Comments describe the behavior; reviewer attribution belongs in the PR body and the commit message, not in `// ...`. Two references in validate_transaction (one to the length pre-check rationale, one to the missing-bytecode StoreError fallback) named the reviewers who raised them — stripped without changing the explanations themselves.
Motivation
Every major Ethereum execution client enforces EIP-3607: transactions whose recovered sender has deployed code are rejected, except where the code is an EIP-7702 delegation designation (the account is still an EOA in spirit, just pointing at delegate code). Without this check, contract accounts can submit invalid transactions that waste pool space and gossip bandwidth.
Description
is_eip7702_delegation(code: &[u8]) -> boolplusEIP7702_DELEGATION_PREFIX = 0xef0100andEIP7702_DELEGATED_CODE_LEN = 23constants incrates/common/types/account.rs.Blockchain::validate_transactionnow inspects the sender'scode_hash; if non-empty, it fetches the code (a single extra store hit only when the sender actually has code) and routes throughis_eip7702_delegation. Non-delegation contract senders are rejected with the newMempoolError::SenderIsContract.