Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions crates/blockchain/mempool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,8 +498,15 @@ impl Mempool {
}
}

/// Filter applied by the payload builder when querying pending transactions
/// from the pool. NOT a mempool admission gate — all fields here are
/// query-time filters used to pick block-includable transactions. Admission
/// rules are enforced in `Blockchain::validate_transaction`.
#[derive(Debug, Default)]
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`.
pub min_tip: Option<u64>,
pub base_fee: Option<u64>,
pub blob_fee: Option<u64>,
Expand Down
5 changes: 5 additions & 0 deletions crates/common/types/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ pub const MIN_BASE_FEE_PER_BLOB_GAS: u64 = 1; // Defined in [EIP-4844](https://e
pub const BLOB_BASE_FEE_UPDATE_FRACTION: u64 = 3338477; // Defined in [EIP-4844](https://eips.ethereum.org/EIPS/eip-4844)
pub const VERSIONED_HASH_VERSION_KZG: u8 = 0x01; // Defined in [EIP-4844](https://eips.ethereum.org/EIPS/eip-4844)
/// Minimum tip, obtained from geth's default miner config (https://github.com/ethereum/go-ethereum/blob/f750117ad19d623622cc4a46ea361a716ba7407e/miner/miner.go#L56)
///
/// Scope: this constant is consumed only by the RPC gas-price estimators
/// (`eth_gasPrice`, `eth_maxPriorityFeePerGas`). It is NOT a mempool
/// admission gate — zero-tip transactions are currently admitted.
///
/// TODO: This should be configurable along with the tip filter on https://github.com/lambdaclass/ethrex/issues/680
pub const MIN_GAS_TIP: u64 = 1000000;

Expand Down
51 changes: 49 additions & 2 deletions crates/common/types/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,10 +452,22 @@ impl Transaction {
TxType::Privileged => self.gas_price(),
};

Some(U256::saturating_add(
let base = U256::saturating_add(
U256::saturating_mul(price, self.gas_limit().into()),
self.value(),
))
);

// EIP-4844 blob txs pay an additional `blob_gas * max_fee_per_blob_gas`
// upfront. Every peer client (geth, reth, nethermind, erigon, besu)
// 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));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 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.

Suggested change
.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.

let blob_cost = blob_gas.saturating_mul(tx.max_fee_per_blob_gas);
return Some(base.saturating_add(blob_cost));
}

Some(base)
}

pub fn fee_token(&self) -> Option<Address> {
Expand Down Expand Up @@ -3717,4 +3729,39 @@ mod tests {
let tx = Transaction::EIP1559Transaction(EIP1559Transaction::default());
assert_eq!(tx.encode_to_vec().len(), EIP1559_DEFAULT_SERIALIZED_LENGTH);
}

#[test]
fn test_cost_without_base_fee_eip4844_includes_blob_gas() {
// Regression test for mempool balance check: for EIP-4844 txs,
// cost_without_base_fee() MUST include blob_gas_used * max_fee_per_blob_gas.
// Every peer client (geth, reth, nethermind, erigon, besu) does this.
use crate::constants::GAS_PER_BLOB;

let max_fee_per_gas: u64 = 100;
let gas: u64 = 21_000;
let value = U256::from(7u64);
let max_fee_per_blob_gas = U256::from(50u64);
let blob_count: usize = 1;

let tx = Transaction::EIP4844Transaction(EIP4844Transaction {
max_fee_per_gas,
gas,
value,
max_fee_per_blob_gas,
blob_versioned_hashes: vec![H256::zero(); blob_count],
..Default::default()
});

let got = tx.cost_without_base_fee().expect("cost is computable");

let gas_cost = U256::from(max_fee_per_gas) * U256::from(gas);
let blob_gas = U256::from(GAS_PER_BLOB) * U256::from(blob_count as u64);
let blob_cost = blob_gas * max_fee_per_blob_gas;
let expected = gas_cost + blob_cost + value;

assert_eq!(
got, expected,
"blob-gas term missing from cost_without_base_fee() for EIP-4844"
);
}
}
Loading