fix(l1): include blob gas in mempool balance check for EIP-4844 transactions#6509
fix(l1): include blob gas in mempool balance check for EIP-4844 transactions#6509
Conversation
…actions Transaction::cost_without_base_fee() was omitting the blob-gas component from the upfront cost it returns, so the mempool balance check at crates/blockchain/blockchain.rs:2488-2493 accepted EIP-4844 transactions whose sender could not afford the blob fees. The transactions then failed at block inclusion, wasting pool capacity and peer bandwidth. Every peer client (geth, reth, nethermind, erigon, besu) includes blob_gas * max_fee_per_blob_gas in the equivalent check; ethrex did not. Changes: - Extend cost_without_base_fee() so the EIP-4844 branch adds blob_versioned_hashes.len() * GAS_PER_BLOB * max_fee_per_blob_gas using saturating arithmetic, matching the function's existing style. - Add regression test test_cost_without_base_fee_eip4844_includes_blob_gas in crates/common/types/transaction.rs. - Document MIN_GAS_TIP in crates/common/types/constants.rs as an RPC gas-price estimator floor, not a mempool admission gate. - Document PendingTxFilter.min_tip in crates/blockchain/mempool.rs as a payload-builder query filter, not an admission gate.
ilitteri
requested review from
a team,
ManuelBilbao and
avilagaston9
as code owners
🤖 Kimi Code ReviewThis PR correctly fixes a balance-check bug for EIP-4844 blob transactions and improves architectural documentation. Specific feedback:
Test Coverage
Security/Consensus Assessment
Recommendation: Approved with minor suggestion—consider adding a Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
Greptile SummaryThis PR fixes a correctness bug where Confidence Score: 5/5Safe to merge — the fix is narrowly scoped, correct, and well-tested; only finding is a minor style note on a All remaining findings are P2 style suggestions with no impact on correctness. The core fix is arithmetically correct, uses saturating arithmetic consistently, and is validated by a regression test that would have failed against the pre-fix code. No files require special attention.
|
| Filename | Overview |
|---|---|
| crates/common/types/transaction.rs | Adds blob gas cost (GAS_PER_BLOB * blob_count * max_fee_per_blob_gas) to cost_without_base_fee() for EIP-4844 transactions; adds regression test. Core fix is correct and matches peer client behaviour. |
| crates/blockchain/mempool.rs | Documentation-only change: clarifies that PendingTxFilter.min_tip is a payload-builder query filter, not a mempool admission gate. |
| crates/common/types/constants.rs | Documentation-only change: clarifies scope of MIN_GAS_TIP as an RPC estimator floor only, not a mempool admission gate. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Mempool: validate_transaction] --> B[cost_without_base_fee]
B --> C{tx_type?}
C -->|Legacy / EIP2930| D[gas_price × gas_limit + value]
C -->|EIP1559 / EIP7702 / FeeToken| E[max_fee_per_gas × gas_limit + value]
C -->|EIP4844| F[max_fee_per_gas × gas_limit + value]
F --> G["+ GAS_PER_BLOB × blob_count × max_fee_per_blob_gas NEW"]
G --> H[total cost]
D --> H
E --> H
H --> I{sender_balance >= total cost?}
I -->|Yes| J[Admit to pool]
I -->|No| K[Reject: InsufficientFunds]
Prompt To Fix All With AI
This is a comment left during a code review.
Path: crates/common/types/transaction.rs
Line: 465
Comment:
**`usize` to `u64` cast could truncate on unusual 32-bit targets**
`tx.blob_versioned_hashes.len() as u64` is safe on all 64-bit platforms, but on 32-bit targets `usize` is already 32 bits so the cast is fine there too. However, using `as u64` is a silent truncating cast — the idiomatic, panic-free alternative is relying on the fact that `U256::from` accepts `usize` directly. Given the protocol limit is tiny (≤6 blobs) this is very low risk, but worth noting for consistency with the saturating style used throughout the function.
```suggestion
.saturating_mul(U256::from(tx.blob_versioned_hashes.len()));
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(l1): include blob gas in mempool bal..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // includes this in the balance-sufficiency check. | ||
| if let Transaction::EIP4844Transaction(tx) = self { | ||
| let blob_gas = U256::from(crate::constants::GAS_PER_BLOB) | ||
| .saturating_mul(U256::from(tx.blob_versioned_hashes.len() as u64)); |
There was a problem hiding this comment.
usize to u64 cast could truncate on unusual 32-bit targets
tx.blob_versioned_hashes.len() as u64 is safe on all 64-bit platforms, but on 32-bit targets usize is already 32 bits so the cast is fine there too. However, using as u64 is a silent truncating cast — the idiomatic, panic-free alternative is relying on the fact that U256::from accepts usize directly. Given the protocol limit is tiny (≤6 blobs) this is very low risk, but worth noting for consistency with the saturating style used throughout the function.
| .saturating_mul(U256::from(tx.blob_versioned_hashes.len() as u64)); | |
| .saturating_mul(U256::from(tx.blob_versioned_hashes.len())); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/common/types/transaction.rs
Line: 465
Comment:
**`usize` to `u64` cast could truncate on unusual 32-bit targets**
`tx.blob_versioned_hashes.len() as u64` is safe on all 64-bit platforms, but on 32-bit targets `usize` is already 32 bits so the cast is fine there too. However, using `as u64` is a silent truncating cast — the idiomatic, panic-free alternative is relying on the fact that `U256::from` accepts `usize` directly. Given the protocol limit is tiny (≤6 blobs) this is very low risk, but worth noting for consistency with the saturating style used throughout the function.
```suggestion
.saturating_mul(U256::from(tx.blob_versioned_hashes.len()));
```
How can I resolve this? If you propose a fix, please make it concise.
🤖 Codex Code Review
Aside from that, the behavioral change itself is correct: including I couldn’t run the targeted Rust tests here because Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Lines of code reportTotal lines added: Detailed view |
🤖 Claude Code ReviewNow I have all the context I need to write a thorough review. Review: fix(l1): include blob gas in mempool balance check for EIP-4844 transactionsSummaryThis is a well-targeted correctness fix. The bug is real — EIP-4844 blob transactions were admitted to the mempool without verifying the sender can afford the blob gas cost, wasting pool capacity and peer bandwidth. The diff is narrow, the arithmetic is consistent with the function's existing style, and the behavior matches all major peer clients. CorrectnessCore formula ( Using Saturating arithmetic is applied consistently throughout, matching the function's existing style. Minor Issues
U256::from(tx.blob_versioned_hashes.len() as u64)
U256::from(tx.blob_versioned_hashes.len())
// or
u64::try_from(tx.blob_versioned_hashes.len()).unwrap_or(0)The first form works if Test covers only Internal roadmap reference in a public constant's doc comment ( This is an internal tracking artefact (tier labels, internal PR names) embedded in a public-facing doc comment. It will rot and confuse external contributors. Consider replacing with a link to a GitHub issue (there's already a Positive
VerdictApprove with minor suggestions. The internal roadmap string in the Automated review by Claude (Anthropic) · sonnet · custom prompt |
There was a problem hiding this comment.
Pull request overview
Fixes mempool balance-sufficiency accounting for EIP-4844 (blob) transactions by including the blob-gas upfront cost in Transaction::cost_without_base_fee(), preventing underfunded blob txs from being admitted and later failing at inclusion.
Changes:
- Update
Transaction::cost_without_base_fee()to addblob_gas_used * max_fee_per_blob_gasfor EIP-4844 transactions (using saturating arithmetic). - Add a regression test ensuring the blob-gas component is included in the computed cost.
- Clarify documentation around
MIN_GAS_TIPandPendingTxFilter::min_tipto distinguish RPC estimation / payload-building filters from mempool admission rules.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/common/types/transaction.rs | Add blob-gas upfront cost for EIP-4844 to cost_without_base_fee() and add a regression test. |
| crates/common/types/constants.rs | Document the scope of MIN_GAS_TIP as RPC-estimator-only (not mempool admission). |
| crates/blockchain/mempool.rs | Document PendingTxFilter / min_tip as query-time payload-builder filters rather than admission gates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// A dedicated admission-time min-tip floor is tracked in the umbrella | ||
| /// `mempool-hardening-roadmap` change (Tier 1, PR T1b). |
| pub struct PendingTxFilter { | ||
| /// Minimum effective priority fee for a transaction to be surfaced to | ||
| /// the payload builder. This is a block-building filter, not an | ||
| /// admission check — see `crates/common/types/constants.rs::MIN_GAS_TIP`. |
Motivation
Transaction::cost_without_base_fee()atcrates/common/types/transaction.rs:444is consumed by the mempool balance check incrates/blockchain/blockchain.rs:2488-2493. For EIP-4844 transactions it was returningmax_fee_per_gas * gas_limit + valueand omitting the blob-gas componentblob_gas_used * max_fee_per_blob_gas. This let the mempool admit blob transactions whose sender could not actually afford them; they propagated over the network and failed at block inclusion, wasting pool capacity and peer bandwidth.Every peer Ethereum execution client (geth
tx.Cost(), rethEthPooledTransaction::cost, nethermindBalanceTooLowFilter, erigonrequiredBalance, besuupfrontCost) includes the blob-gas term in this check. ethrex did not.Description
Transaction::cost_without_base_fee()so the EIP-4844 branch addsblob_versioned_hashes.len() * GAS_PER_BLOB * max_fee_per_blob_gasusing saturating arithmetic, matching the function's existing style.test_cost_without_base_fee_eip4844_includes_blob_gasincrates/common/types/transaction.rs. The test fails against the pre-fix code with a delta of exactlyGAS_PER_BLOB * max_fee_per_blob_gas.MIN_GAS_TIPincrates/common/types/constants.rsas an RPC gas-price estimator floor only, not a mempool admission gate (it is consumed exclusively byeth_gasPrice/eth_maxPriorityFeePerGas).PendingTxFilter.min_tipincrates/blockchain/mempool.rsas a payload-builder query filter, not an admission gate.Previously-admitted EIP-4844 transactions with insufficient balance for blob gas will now be rejected at ingress. This is a correctness fix, not a policy change. A dedicated admission-time minimum-tip floor is tracked separately and is out of scope here.