feat(l1): cap 7702-delegated EOAs at 1 in-flight mempool tx#6630
feat(l1): cap 7702-delegated EOAs at 1 in-flight mempool tx#6630ilitteri wants to merge 8 commits into
Conversation
Shared predicate + constants (EIP7702_DELEGATION_PREFIX = 0xef0100, EIP7702_DELEGATION_CODE_LEN = 23) for detecting EIP-7702 delegation designations from sender bytecode. Used by both the EIP-3607 admission check (PR #6600) and the new delegated-sender cap.
Delegated EOAs (those whose code is the EIP-7702 designation `0xef0100 || address`) can be used by their delegate to amplify spam, so peer execution clients (geth, reth) tighten the per-sender cap to 1 for these accounts. Match that. `Blockchain::validate_transaction` reads the sender's code metadata (length-pre-check via `Store::get_code_metadata`, fetching full bytecode only when length == 23) and applies `options.delegated_sender_cap` (default 1) instead of `max_pending_txs_per_account` when the sender is delegated. RBF replacements at the same (sender, nonce) still bypass the cap. `Mempool::pending_tx_count_for_sender` is the cap-side accessor. New error `MempoolError::MaxDelegatedPendingTxsExceeded(u64)`. A TODO marks the future move into `Mempool::add_transaction` once the atomic per-sender cap check from PR #6603 lands on main.
Operator knob (env ETHREX_MEMPOOL_DELEGATED_SENDER_CAP, default 1) for the new EIP-7702 delegated-sender cap. Plumbed through both L1 and L2 initializers and documented in docs/CLI.md.
Four integration tests on Blockchain::validate_transaction: - delegated_sender_second_tx_rejected_by_default_cap - delegated_sender_replacement_bypasses_cap - non_delegated_sender_second_tx_admitted - delegated_sender_cap_override_admits_more_txs Uses a genesis allocation that pre-installs `0xef0100 || address` bytecode on the sender so admission can exercise the delegation branch without simulated state.
ilitteri
requested review from
a team,
ManuelBilbao and
avilagaston9
as code owners
🤖 Kimi Code ReviewThis PR correctly implements EIP-7702 delegated sender caps in the mempool. The logic is sound and the implementation is well-tested. Code Correctness & Security
Performance
Issues & Suggestions
Nits
The implementation is ready for merge pending resolution of the TOCTOU item (tracked in the TODO). Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
Lines of code reportTotal lines added: Detailed view |
🤖 Claude Code ReviewNow I have enough context for the review. PR #6630 —
|
🤖 Codex Code Review
Aside from those two points, the patch is straightforward and the code-length precheck for delegation detection is a sensible optimization. Assumption: the intent is a hard admission cap for Prague+ delegated EOAs, not a best-effort heuristic. I couldn’t run the targeted Rust tests here because Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
There was a problem hiding this comment.
Pull request overview
Adds a stricter per-sender pending-transaction cap for EIP-7702 delegated EOAs to reduce delegated-account spam potential, wiring the new limit through blockchain options, CLI configuration, and test coverage.
Changes:
- Introduces an EIP-7702 delegation predicate/constants in
ethrex-commonand uses code-length prechecks to avoid unnecessary bytecode reads. - Enforces a dedicated
delegated_sender_cap(default1) during mempool admission, while preserving RBF replacement behavior. - Plumbs a new CLI flag/env var through L1/L2 initializers and documents it; adds tests for delegated vs non-delegated behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/tests/blockchain/mempool_tests.rs | Adds integration-style tests validating delegated-sender cap behavior and RBF bypass. |
| docs/CLI.md | Documents new --mempool.delegated-sender-cap option in CLI help output. |
| crates/common/types/account.rs | Adds EIP-7702 delegation constants + predicate and unit tests. |
| crates/blockchain/mempool.rs | Adds pending_tx_count_for_sender helper and tests for counting behavior. |
| crates/blockchain/error.rs | Adds MempoolError::MaxDelegatedPendingTxsExceeded. |
| crates/blockchain/constants.rs | Adds DEFAULT_DELEGATED_SENDER_CAP default value. |
| crates/blockchain/blockchain.rs | Adds BlockchainOptions::delegated_sender_cap and delegated-sender admission check using code metadata. |
| cmd/ethrex/l2/initializers.rs | Wires delegated cap option into L2 blockchain initialization. |
| cmd/ethrex/initializers.rs | Wires delegated cap option into L1 blockchain initialization. |
| cmd/ethrex/cli.rs | Adds CLI flag/env var/defaults for delegated sender cap and passes into BlockchainOptions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // TODO: once an atomic version of the per-sender pending-tx cap lands | ||
| // (mirroring the count + insert pair under the mempool write lock), | ||
| // move the effective-cap computation to be passed into | ||
| // `Mempool::add_transaction` and enforce the check there to close the | ||
| // TOCTOU window between this read and insertion. | ||
| if tx_to_replace_hash.is_none() && self.is_sender_delegated(sender_code_hash)? { | ||
| let cap = self.options.delegated_sender_cap; | ||
| let pending = self.mempool.pending_tx_count_for_sender(sender)?; | ||
| if pending >= cap { | ||
| return Err(MempoolError::MaxDelegatedPendingTxsExceeded(cap)); | ||
| } | ||
| } |
|
|
||
| /// Returns the number of pending transactions in the pool for `sender`. | ||
| /// | ||
| /// Used by per-sender admission caps (see [`Blockchain::validate_transaction`]). |
Greptile SummaryThis PR introduces a tighter mempool admission cap for EIP-7702 delegated EOAs — accounts whose bytecode is the 23-byte designation
Confidence Score: 4/5The change is well-scoped and safe to merge; the only non-obvious behaviour is that setting the cap to 0 permanently blocks all delegated senders rather than acting as an "unlimited" sentinel. The core admission check, the metadata length pre-filter, the RBF bypass, and the CLI wiring are all correct. The acknowledged TOCTOU window between the count read and the actual mempool insert is a known limitation with a clear follow-up plan, not a latent correctness issue in typical usage. The only concern worth noting is that crates/blockchain/blockchain.rs — specifically the cap guard at line 2541, where the
|
| Filename | Overview |
|---|---|
| crates/blockchain/blockchain.rs | Core logic: extracts sender_code_hash, then after find_tx_to_replace calls is_sender_delegated and enforces the cap; TOCTOU between count read and add_transaction is acknowledged via TODO. |
| crates/blockchain/mempool.rs | Adds pending_tx_count_for_sender via a BTreeMap range over txs_by_sender_nonce; new unit tests for isolation and decrement behavior are correct. |
| crates/common/types/account.rs | Adds EIP7702_DELEGATION_PREFIX, EIP7702_DELEGATION_CODE_LEN constants and is_eip7702_delegation predicate; well-tested and correctly scoped. |
| crates/blockchain/constants.rs | Adds DEFAULT_DELEGATED_SENDER_CAP = 1 with clear documentation; straightforward. |
| crates/blockchain/error.rs | New MaxDelegatedPendingTxsExceeded(u64) variant with descriptive error message; straightforward addition. |
| cmd/ethrex/cli.rs | Adds mempool_delegated_sender_cap CLI flag with env var, default, and help text; correctly initialized in all three option preset functions. |
| cmd/ethrex/initializers.rs | Plumbs delegated_sender_cap into BlockchainOptions for L1; correct and minimal. |
| cmd/ethrex/l2/initializers.rs | Plumbs delegated_sender_cap into BlockchainOptions for L2; correct and minimal. |
| test/tests/blockchain/mempool_tests.rs | Four integration tests cover: cap rejection, RBF bypass, non-delegated sender exemption, and cap override; good coverage of the expected behaviors. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[validate_transaction called] --> B[get_account_info - nonce / balance checks]
B --> C[extract sender_code_hash]
C --> D[find_tx_to_replace - nonce conflict / RBF check]
D --> E{tx_to_replace_hash is None?}
E -- No / RBF replacement --> F[Skip delegation cap check]
E -- Yes / new slot --> G[is_sender_delegated?]
G --> H[get_code_metadata - length pre-check]
H -- length != 23 --> I[Not delegated - skip cap]
H -- length == 23 --> J[get_account_code - fetch bytecode]
J --> K{starts with 0xef0100?}
K -- No --> I
K -- Yes --> L[pending_tx_count_for_sender]
L --> M{count >= delegated_sender_cap?}
M -- Yes --> N[MaxDelegatedPendingTxsExceeded]
M -- No --> O[Ok - proceed to add_transaction]
F --> O
I --> O
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
crates/blockchain/blockchain.rs:2538-2543
**`delegated_sender_cap = 0` silently blocks every delegated-sender tx**
When `delegated_sender_cap` is set to `0`, the condition `pending >= cap` evaluates to `0 >= 0 = true` for the very first transaction from a delegated EOA — so *no* delegated sender can ever enter the mempool. This is probably unintentional for operators who set the value thinking "0 means unlimited" (a common convention), but the code treats it as "ban all delegated senders." There is no guard, clamp, or documentation in the CLI help or doc-comment that explains this edge case. If the intent is that `0` means "no delegated senders allowed," that should be explicitly called out in the `--mempool.delegated-sender-cap` help text.
Reviews (1): Last reviewed commit: "test(l1): cover EIP-7702 delegated-sende..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if tx_to_replace_hash.is_none() && self.is_sender_delegated(sender_code_hash)? { | ||
| let cap = self.options.delegated_sender_cap; | ||
| let pending = self.mempool.pending_tx_count_for_sender(sender)?; | ||
| if pending >= cap { | ||
| return Err(MempoolError::MaxDelegatedPendingTxsExceeded(cap)); | ||
| } |
There was a problem hiding this comment.
delegated_sender_cap = 0 silently blocks every delegated-sender tx
When delegated_sender_cap is set to 0, the condition pending >= cap evaluates to 0 >= 0 = true for the very first transaction from a delegated EOA — so no delegated sender can ever enter the mempool. This is probably unintentional for operators who set the value thinking "0 means unlimited" (a common convention), but the code treats it as "ban all delegated senders." There is no guard, clamp, or documentation in the CLI help or doc-comment that explains this edge case. If the intent is that 0 means "no delegated senders allowed," that should be explicitly called out in the --mempool.delegated-sender-cap help text.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/blockchain/blockchain.rs
Line: 2538-2543
Comment:
**`delegated_sender_cap = 0` silently blocks every delegated-sender tx**
When `delegated_sender_cap` is set to `0`, the condition `pending >= cap` evaluates to `0 >= 0 = true` for the very first transaction from a delegated EOA — so *no* delegated sender can ever enter the mempool. This is probably unintentional for operators who set the value thinking "0 means unlimited" (a common convention), but the code treats it as "ban all delegated senders." There is no guard, clamp, or documentation in the CLI help or doc-comment that explains this edge case. If the intent is that `0` means "no delegated senders allowed," that should be explicitly called out in the `--mempool.delegated-sender-cap` help text.
How can I resolve this? If you propose a fix, please make it concise.…prune count) Phase 2 cross-check produced three confirmed refinements: 1. **Skip prune when count <= 1**. With cap=1 the previous code wiped the sender's only pending tx on every cap-breach attempt — especially harmful under PR #6630's delegated cap=1 where every collision would leave the sender with nothing. Now the prune is gated on `count > 1`. 2. **Single-pass range scan**. The three independent `txs_by_sender_nonce.range(...)` traversals (count + victims + new_top_nonce) collapse into one `.range().rev().collect()`. The victims are taken from the front of the reversed Vec, new_top_nonce from `entries.get(dropped_count)`. 3. **Post-prune count in error**. `MaxPendingTxsPerAccountExceeded.count` now reports the sender's count after the prune (was: pre-prune), matching the actual post-state visible to RPC clients. Two new tests: punish_spammer_skips_prune_when_count_is_one, punish_spammer_reports_post_prune_count.
Phase 2 review (Copilot): the delegated-sender cap check ran in `validate_transaction` under a mempool read lock, but the actual insertion happens later in `Mempool::add_transaction` under a separate write lock. Concurrent admissions from the same delegated EOA could both pass the count check and exceed `delegated_sender_cap`. Move the authoritative check inside `Mempool::add_transaction`: - `add_transaction` takes `max_pending_txs_per_account: u64` and re-counts under the write lock; `u64::MAX` disables the cap. - New `Blockchain::effective_per_sender_cap(sender)` async helper computes the cap from the sender's code hash (returns `delegated_sender_cap` for delegated EOAs, `u64::MAX` otherwise). - `Blockchain::add_transaction_to_pool` / `add_blob_transaction_to_pool` compute the effective cap after `validate_transaction` and pass it to `add_transaction`. The read-lock check in `validate_transaction` is kept as defense-in-depth and so the existing integration tests (`delegated_sender_second_tx_rejected_by_default_cap` and friends) continue to exercise the rejection path. Existing `mempool.add_transaction(...)` test callers pass `u64::MAX` to disable the cap in unit-test scenarios that don't care about it.
Phase 2 review (Copilot + greptile): - The intra-doc link `[Blockchain::validate_transaction]` on `Mempool::pending_tx_count_for_sender` (and a similar one on `add_transaction`'s docstring, already updated) wouldn't resolve from inside the `mempool` module because `Blockchain` isn't in scope. Use the fully-qualified `crate::Blockchain::validate_transaction`. - The `--mempool.delegated-sender-cap` help text didn't mention what `0` means. Some operators assume "0 means unlimited" by convention; ethrex's check (`pending >= cap`) treats 0 as "ban all delegated senders". Spell that out explicitly so the surprise is documented. Also regenerated `docs/CLI.md` with the new help text.
ElFantasma
left a comment
ElFantasma
left a comment
There was a problem hiding this comment.
This is the cleanest of the recent mempool-hardening stack — the atomic count-and-insert inside Mempool::add_transaction under the write lock is exactly the right shape for a per-sender cap, and it closes the TOCTOU window I flagged on #6606 and #6609. The shared is_eip7702_delegation predicate in ethrex-common (coordinating with #6600) is the right place for a definition this load-bearing.
One inline note — a stale TODO that contradicts what the PR actually shipped.
| // is `Some`) bypass the cap: they swap a slot rather than consuming a | ||
| // new one. | ||
| // | ||
| // TODO: once an atomic version of the per-sender pending-tx cap lands |
There was a problem hiding this comment.
This TODO describes a future state — "once an atomic version of the per-sender pending-tx cap lands… move the effective-cap computation to be passed into Mempool::add_transaction and enforce the check there" — but the atomic version IS landing in this PR (mempool.rs:154-172 does exactly that: takes max_pending_txs_per_account: u64, checks count under the write lock, refuses if at cap).
So the TODO is stale on arrival. Per the PR description, this read-lock check at :2548-2554 is deliberately kept as defense-in-depth (and to keep the existing integration tests exercising the rejection path), which is a different story than the TODO tells.
Swap the TODO for a note explaining the actual rationale, e.g.:
// Defense-in-depth: the authoritative cap is enforced atomically inside
// `Mempool::add_transaction` under the write lock. This read-lock check
// surfaces the rejection earlier (before the storage write paths in
// `add_transaction_to_pool`) and keeps the existing integration tests
// asserting via `validate_transaction`.Non-blocking, but future readers will trip on the current TODO.
…d-sender-cap # Conflicts: # crates/blockchain/blockchain.rs # crates/blockchain/constants.rs
|
| EIP | Bucket | Count |
|---|---|---|
| EIP-7702 | set_code_txs |
24 |
| EIP-7702 | set_code_txs_2 |
15 |
| EIP-7702 | gas |
1 |
| EIP-8037 | state_gas_set_code |
17 |
| EIP-8037 | state_gas_pricing |
1 |
| EIP-8037 | state_gas_sstore |
1 |
| EIP-7928 | block_access_lists_eip7702 |
8 |
| EIP-7928 | block_access_lists |
1 |
| EIP-7778 | gas_accounting |
3 |
| EIP-7708 | transfer_logs |
1 |
| EIP-7976 | refunds |
1 |
| EIP-1344 | chainid (Amsterdam fork-transition fixture) |
1 |
| Total | 74 |
Re-enable once we either:
- (a) bump fixtures to a snobal-devnet-7 release that locks in the new
accounting; or - (b) revert the bal-devnet-7-prep subtraction for bal-devnet-6
compatibility.
Full test list (74)
EIP-7702 — for_amsterdam/prague/eip7702_set_code_tx/set_code_txs/
delegation_clearingdelegation_clearing_and_setdelegation_clearing_failing_txdelegation_clearing_tx_toeoa_tx_after_set_codeext_code_on_chain_delegating_set_codeext_code_on_self_delegating_set_codeext_code_on_self_set_codeext_code_on_set_codemany_delegationsnonce_overflow_after_first_authorizationnonce_validityreset_codeself_code_on_set_codeself_sponsored_set_codeset_code_multiple_valid_authorization_tuples_same_signer_increasing_nonceset_code_multiple_valid_authorization_tuples_same_signer_increasing_nonce_self_sponsoredset_code_to_logset_code_to_non_empty_storage_non_zero_nonceset_code_to_self_destructset_code_to_self_destructing_account_deployed_in_same_txset_code_to_sstoreset_code_to_sstore_then_sloadset_code_to_system_contract
EIP-7702 — for_amsterdam/prague/eip7702_set_code_tx/set_code_txs_2/
call_pointer_to_created_from_create_after_oog_call_againcall_to_precompile_in_pointer_contextcontract_storage_to_pointer_with_storagedelegation_replacement_call_previous_contractdouble_authpointer_measurementspointer_normalpointer_reentrypointer_resets_an_empty_code_account_with_storagepointer_revertspointer_to_pointerpointer_to_precompilepointer_to_staticpointer_to_static_reentrystatic_to_pointer
EIP-7702 — for_amsterdam/prague/eip7702_set_code_tx/gas/
account_warming
EIP-8037 — for_amsterdam/amsterdam/eip8037_state_creation_gas_cost_increase/state_gas_set_code/
auth_refund_block_gas_accountingauth_refund_bypasses_one_fifth_capauth_with_calldata_and_access_listauth_with_multiple_sstoresauthorization_exact_state_gas_boundaryauthorization_to_precompile_addressauthorization_with_sstoreduplicate_signer_authorizationsexisting_account_auth_header_gas_used_uses_worst_caseexisting_account_refundexisting_account_refund_enables_sstoreexisting_auth_with_reverted_execution_preserves_intrinsicmany_authorizations_state_gasmixed_auths_header_gas_used_uses_worst_casemixed_new_and_existing_authsmixed_valid_and_invalid_authsmulti_tx_block_auth_refund_and_sstore
EIP-8037 — state_gas_pricing/
auth_state_gas_scales_with_cpsb
EIP-8037 — state_gas_sstore/
sstore_state_gas_all_tx_types
EIP-7928 — for_amsterdam/amsterdam/eip7928_block_level_access_lists/block_access_lists_eip7702/
bal_7702_delegation_clearbal_7702_delegation_createbal_7702_delegation_updatebal_7702_double_auth_resetbal_7702_double_auth_swapbal_7702_null_address_delegation_no_code_changebal_selfdestruct_to_7702_delegationbal_withdrawal_to_7702_delegation
EIP-7928 — block_access_lists/
bal_all_transaction_types
EIP-7778 — for_amsterdam/amsterdam/eip7778_block_gas_accounting_without_refunds/gas_accounting/
multiple_refund_types_in_one_txsimple_gas_accountingvarying_calldata_costs
EIP-7708 — for_amsterdam/amsterdam/eip7708_eth_transfer_logs/transfer_logs/
transfer_with_all_tx_types
EIP-7976 — for_amsterdam/amsterdam/eip7976_increase_calldata_floor_cost/refunds/
gas_refunds_from_data_floor
EIP-1344 — for_amsterdam/istanbul/eip1344_chainid/chainid/
chainid(Amsterdam fork-transition fixture)
Motivation
EIP-7702 delegated EOAs are accounts whose code is the designation
0xef0100 || address. A delegate contract can act on behalf of multiple identities, amplifying spam potential. Peer execution clients (geth, reth) hold delegated EOAs to a tighter per-sender pending-tx cap than regular accounts.Description
is_eip7702_delegationand constants inethrex-commonso both this PR and feat(l1): reject contract senders at mempool admission (EIP-3607) #6600 (EIP-3607 contract-sender rejection) work against the same definition.Blockchain::effective_per_sender_cap(sender)async helper returnsoptions.delegated_sender_capfor delegated EOAs,u64::MAXotherwise. Uses a length pre-check viaStore::get_code_metadataso the full bytecode is only fetched when the length matches the 23-byte designation.Mempool::add_transactionunder the same write lock as the insertion (no TOCTOU window).Mempool::add_transactionnow takesmax_pending_txs_per_account: u64— passu64::MAXto disable.Blockchain::validate_transactionkeeps a read-lock check as defense-in-depth (and so the existing integration tests continue to exercise the rejection path). RBF replacements at the same(sender, nonce)bypass the cap regardless.--mempool.delegated-sender-cap(envETHREX_MEMPOOL_DELEGATED_SENDER_CAP, default 1, typeu64). Setting this to 0 admits zero pending transactions from delegated senders (i.e. blocks them entirely, NOT "unlimited"); use a large value such asu64::MAXfor environments that want no cap.Behavioral change
Delegated EOAs are limited to
delegated_sender_cappending txs by default. Non-delegated senders are unaffected.Notes
The constants in
ethrex-common(EIP7702_DELEGATION_PREFIX,EIP7702_DELEGATION_CODE_LEN) will need to align with the same names used in #6600 at merge time; whichever PR lands second should rename to match. Currently this PR usesEIP7702_DELEGATION_CODE_LEN.