fix(l1): align mempool intrinsic-gas and EIP-7702 auth-list validation with execution#6892
fix(l1): align mempool intrinsic-gas and EIP-7702 auth-list validation with execution#6892ElFantasma wants to merge 4 commits into
Conversation
🤖 Kimi Code ReviewOverall Assessment: This PR correctly fixes a critical mempool-EVM inconsistency (pool pollution bug) by delegating intrinsic gas calculations to LEVM and adds proper EIP-7702 empty authorization list validation. The approach is sound and the implementation is idiomatic. Specific Issues: 1. Incorrect EIP reference in comment
2. Test expectation change verification
3. Fork gating for EIP-7702 empty auth list
Positive Aspects:
Nit:
Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
Lines of code reportTotal lines added: Detailed view |
🤖 Claude Code ReviewNow I have all the context I need. Here's the full review: PR Review:
|
🤖 Codex Code ReviewFindings
I could not run the targeted Rust tests in this environment because Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Greptile SummaryThis PR closes two EF-reported mempool/LEVM divergence gaps on Prague: the per-EIP-7702 authorization-tuple intrinsic cost (
Confidence Score: 4/5Safe to merge. The change is a targeted fix to two well-scoped mempool admission gaps; LEVM itself is untouched and serves as the authoritative gate at inclusion time, so no consensus risk is introduced. The refactoring is well-motivated and the new implementation delegates directly to the VM helpers it was previously duplicating, which is strictly better. The regression tests cover all three newly-enforced invariants. The only item worth a second look is the ordering of the empty-auth-list guard relative to the intrinsic-gas check: for the degenerate case of a type-4 tx with both an empty auth list and a gas_limit below 21 000, the returned error misidentifies the root cause — harmless in practice since the tx is rejected either way, but a diagnostics inconsistency. crates/blockchain/blockchain.rs — review the placement of the EmptyAuthorizationList guard relative to the intrinsic-gas check.
|
| Filename | Overview |
|---|---|
| crates/blockchain/blockchain.rs | Adds empty-auth-list rejection for EIP-7702 type-4 txs; the guard is placed after the intrinsic-gas check, which can surface a misleading error for the extreme edge case of gas_limit < 21000. |
| crates/blockchain/mempool.rs | transaction_intrinsic_gas now delegates unconditionally to VM helpers (intrinsic_gas_dimensions + intrinsic_gas_floor), eliminating the drift-prone hand-rolled computation; EIP-7623 floor correctly gated at Fork::Prague. |
| crates/blockchain/error.rs | Adds MempoolError::EmptyAuthorizationList variant with a clear error message. |
| test/tests/blockchain/mempool_tests.rs | Adds regression tests for EIP-7623 floor, EIP-7702 auth-list cost, and empty-auth rejection; fixes pre-existing wrong gas constants (TX_DATA_NON_ZERO_GAS → TX_DATA_NON_ZERO_GAS_EIP2028) in the two Shanghai create tests; removes the unreachable pre-Istanbul calldata test. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[validate_transaction] --> B{tx.gas_limit < intrinsic_gas?}
B -->|yes| C[Err: TxIntrinsicGasCostAboveLimitError]
B -->|no| D{EIP-7702 tx with\nempty auth_list?}
D -->|yes| E[Err: EmptyAuthorizationList]
D -->|no| F[continue validation...]
subgraph transaction_intrinsic_gas
G[intrinsic_gas_dimensions tx, fork, gas_limit]
G --> H["(regular_gas, state_gas)"]
H --> I[intrinsic = regular + state]
I --> J{fork >= Prague?}
J -->|yes| K[floor = intrinsic_gas_floor tx, fork]
J -->|no| L[floor = 0]
K --> M[return max intrinsic, floor]
L --> M
end
B -. calls .-> G
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[validate_transaction] --> B{tx.gas_limit < intrinsic_gas?}
B -->|yes| C[Err: TxIntrinsicGasCostAboveLimitError]
B -->|no| D{EIP-7702 tx with\nempty auth_list?}
D -->|yes| E[Err: EmptyAuthorizationList]
D -->|no| F[continue validation...]
subgraph transaction_intrinsic_gas
G[intrinsic_gas_dimensions tx, fork, gas_limit]
G --> H["(regular_gas, state_gas)"]
H --> I[intrinsic = regular + state]
I --> J{fork >= Prague?}
J -->|yes| K[floor = intrinsic_gas_floor tx, fork]
J -->|no| L[floor = 0]
K --> M[return max intrinsic, floor]
L --> M
end
B -. calls .-> G
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:2911-2918
**Empty-auth check ordering — wrong error for gas_limit < 21000**
The `EmptyAuthorizationList` guard runs after the intrinsic-gas check. For an EIP-7702 transaction that has both an empty `authorization_list` and `gas_limit < TX_GAS_COST` (21 000), the mempool returns `TxIntrinsicGasCostAboveLimitError` rather than `EmptyAuthorizationList`. The primary invalidity — the empty auth list — is structurally obvious; the intrinsic-gas error is a secondary consequence. Consider moving this check to just before (or just after the tx-type check earlier in the function) so the returned error accurately diagnoses the root cause. The transaction is rejected either way, so this only affects callers that inspect the error variant for diagnostics.
Reviews (1): Last reviewed commit: "fix(l1): align mempool intrinsic-gas and..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // EIP-7702: an empty `authorization_list` makes a type-4 tx invalid | ||
| // (LEVM rejects it in `validate_type_4_tx`). Reject at admission so it | ||
| // never enters the pool. | ||
| if let Transaction::EIP7702Transaction(eip7702) = tx | ||
| && eip7702.authorization_list.is_empty() | ||
| { | ||
| return Err(MempoolError::EmptyAuthorizationList); | ||
| } |
There was a problem hiding this comment.
The EmptyAuthorizationList guard runs after the intrinsic-gas check. For an EIP-7702 transaction that has both an empty authorization_list and gas_limit < TX_GAS_COST (21 000), the mempool returns TxIntrinsicGasCostAboveLimitError rather than EmptyAuthorizationList. The primary invalidity — the empty auth list — is structurally obvious; the intrinsic-gas error is a secondary consequence. Consider moving this check to just before (or just after the tx-type check earlier in the function) so the returned error accurately diagnoses the root cause. The transaction is rejected either way, so this only affects callers that inspect the error variant for diagnostics.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/blockchain/blockchain.rs
Line: 2911-2918
Comment:
**Empty-auth check ordering — wrong error for gas_limit < 21000**
The `EmptyAuthorizationList` guard runs after the intrinsic-gas check. For an EIP-7702 transaction that has both an empty `authorization_list` and `gas_limit < TX_GAS_COST` (21 000), the mempool returns `TxIntrinsicGasCostAboveLimitError` rather than `EmptyAuthorizationList`. The primary invalidity — the empty auth list — is structurally obvious; the intrinsic-gas error is a secondary consequence. Consider moving this check to just before (or just after the tx-type check earlier in the function) so the returned error accurately diagnoses the root cause. The transaction is rejected either way, so this only affects callers that inspect the error variant for diagnostics.
How can I resolve this? If you propose a fix, please make it concise.
|
Thanks for the reviews. Follow-up commits AddressedEmpty-auth check ordering (Claude, Greptile P2) — moved the EIP-7702 type-4 structural validation ahead of the intrinsic-gas check, so an empty-auth tx now reports Pre-Prague type-4 gate (Codex #2, Claude #3, Kimi #3) — real gap, good catch. ethrex supports pre-Prague forks (Paris/Shanghai/Cancun), and the mempool was admitting type-4 txs that LEVM rejects at inclusion with Lossy intrinsic-gas error mapping (Claude #5) — the helper failures now map to a new Cosmetics — False positivePre-Istanbul calldata pricing "regression" (Codex #1, Claude #1, Kimi #2) — not a regression. Not changed
Genesis-setup verbosity in the async test (Claude) — left as-is; it follows the existing pattern in All mempool tests green (32), clippy + fmt clean. |
2 participants
Motivation
On Prague (the active mainnet fork) the mempool's intrinsic-gas check did not match what LEVM enforces at execution, so transactions the VM will reject at inclusion were admitted to the pool — wasting payload-builder cycles and crowding out valid txs. Two gaps (EF findings
eip7702-mempool-intrinsic-gas-gap,mempool-calldata-floor-gas-gap):PER_EMPTY_ACCOUNT_COST) was omitted;Additionally, an EIP-7702 transaction with an empty
authorization_list(invalid per the EIP, rejected by LEVM invalidate_type_4_tx) was admitted.None of these are consensus issues — LEVM independently rejects all three at inclusion — but they let invalid txs pollute the pool, diverging from geth/reth which reject them at admission.
Description
transaction_intrinsic_gasnow delegates to the VM's own helpers (intrinsic_gas_dimensions+intrinsic_gas_floor) for every fork instead of a parallel hand-rolled computation. This picks up the EIP-7702 auth-list cost and the EIP-7623 calldata floor automatically, and removes a drift-prone duplicate implementation. The floor is gated tofork >= Fork::Prague, matching the VM's default-hook gating.validate_transactionrejects type-4 txs with an emptyauthorization_listvia the newMempoolError::EmptyAuthorizationList.get_fork()floors at Paris, so EIP-2028 is always in effect, making it a duplicate of the post-Istanbul test); the two Shanghai create tests were corrected to EIP-2028 pricing.Resolves #6889
Checklist
cargo fmt+cargo clippyclean on the touched cratesStorechanges (noSTORE_SCHEMA_VERSIONbump needed)