Sitelet https://github.com/ethereum-optimism/optimism/commit/f850b2caeca1f9fd56c163fe9ce78ad62373a372
Skip to content

Commit f850b2c

Browse files
geokneeclaude
andauthored
op-reth: include OP fees in txpool cost for sender affordability (#23076)
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 54e770f commit f850b2c

3 files changed

Lines changed: 123 additions & 5 deletions

File tree

‎rust/op-reth/crates/payload/src/builder/tests.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -917,6 +917,9 @@ fn miner_fee_uses_pool_wrapper_tip() {
917917
fn encoded_2718(&self) -> Cow<'_, Bytes> {
918918
OpPooledTx::encoded_2718(&self.inner)
919919
}
920+
fn set_op_fee_reservation(&mut self, fee: U256) {
921+
self.inner.set_op_fee_reservation(fee);
922+
}
920923
}
921924

922925
let signer = Address::repeat_byte(0x11);

‎rust/op-reth/crates/txpool/src/transaction.rs‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,20 @@ pub struct OpPooledTransaction<
5454

5555
/// Cached EIP-2718 encoded bytes of the transaction, lazily computed.
5656
encoded_2718: OnceLock<Bytes>,
57+
58+
/// Worst-case cost including the OP fees reserved at validation, see
59+
/// [`OpPooledTx::set_op_fee_reservation`].
60+
cost: U256,
5761
}
5862

5963
impl<Cons: SignedTransaction, Pooled> OpPooledTransaction<Cons, Pooled> {
6064
/// Create new instance of [Self].
6165
pub fn new(transaction: Recovered<Cons>, encoded_length: usize) -> Self {
66+
let inner = EthPooledTransaction::new(transaction, encoded_length);
67+
let cost = inner.cost;
6268
Self {
63-
inner: EthPooledTransaction::new(transaction, encoded_length),
69+
inner,
70+
cost,
6471
estimated_tx_compressed_size: Default::default(),
6572
conditional: None,
6673
interop: Arc::new(AtomicU64::new(NO_INTEROP_TX)),
@@ -165,7 +172,7 @@ where
165172
}
166173

167174
fn cost(&self) -> &U256 {
168-
&self.inner.cost
175+
&self.cost
169176
}
170177

171178
fn encoded_length(&self) -> usize {
@@ -299,6 +306,13 @@ pub trait OpPooledTx:
299306
{
300307
/// Returns the EIP-2718 encoded bytes of the transaction.
301308
fn encoded_2718(&self) -> Cow<'_, Bytes>;
309+
310+
/// Adds the OP fees the sender must also cover (L1 data fee and operator fee) to
311+
/// [`PoolTransaction::cost`], replacing any previous reservation.
312+
///
313+
/// The pool sums `cost()` over a sender's consecutive nonces to decide which are executable, so
314+
/// without the reservation it marks transactions pending that cannot pay their L1 data fee.
315+
fn set_op_fee_reservation(&mut self, fee: U256);
302316
}
303317

304318
impl<Cons, Pooled> OpPooledTx for OpPooledTransaction<Cons, Pooled>
@@ -310,6 +324,10 @@ where
310324
fn encoded_2718(&self) -> Cow<'_, Bytes> {
311325
Cow::Borrowed(self.encoded_2718())
312326
}
327+
328+
fn set_op_fee_reservation(&mut self, fee: U256) {
329+
self.cost = self.inner.cost.saturating_add(fee);
330+
}
313331
}
314332

315333
#[cfg(test)]

‎rust/op-reth/crates/txpool/src/validator.rs‎

Lines changed: 100 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use reth_primitives_traits::{
1414
use reth_storage_api::{AccountInfoReader, BlockReaderIdExt, StateProviderFactory};
1515
use reth_transaction_pool::{
1616
EthPoolTransaction, EthTransactionValidator, TransactionOrigin, TransactionValidationOutcome,
17-
TransactionValidator, error::InvalidPoolTransactionError,
17+
TransactionValidator, error::InvalidPoolTransactionError, validate::ValidTransaction,
1818
};
1919
use std::sync::{
2020
Arc,
@@ -262,7 +262,7 @@ where
262262
if let TransactionValidationOutcome::Valid {
263263
balance,
264264
state_nonce,
265-
transaction: valid_tx,
265+
transaction: mut valid_tx,
266266
propagate,
267267
bytecode_hash,
268268
authorities,
@@ -301,7 +301,18 @@ where
301301
valid_tx.transaction().gas_limit(),
302302
));
303303

304-
let cost = valid_tx.transaction().cost().saturating_add(cost_addition);
304+
// Fold the OP fees into `cost()` so the pool's cumulative per-sender balance check
305+
// accounts for them too. This check only sees one transaction against the on-chain
306+
// balance; a sender's later nonces must also cover the OP fees of the earlier ones, or
307+
// the pool marks them pending although execution rejects them for insufficient funds.
308+
// The fees are priced at admission.
309+
match &mut valid_tx {
310+
ValidTransaction::Valid(tx) |
311+
ValidTransaction::ValidWithSidecar { transaction: tx, .. } => {
312+
tx.set_op_fee_reservation(cost_addition)
313+
}
314+
}
315+
let cost = *valid_tx.transaction().cost();
305316

306317
// Checks for max cost
307318
if cost > balance {
@@ -508,4 +519,90 @@ mod tests {
508519
U256::ZERO
509520
);
510521
}
522+
523+
/// The pool must classify a sender's consecutive nonces against the full OP cost, not just the
524+
/// L2 `cost()`. Each tx below is affordable on its own (so both pass admission against the same
525+
/// on-chain balance), but the sender cannot pay the L1 data fee of both. Without the L1 fee in
526+
/// the pool's cumulative accounting, nonce 1 is marked pending and the payload builder retries
527+
/// it every block, failing with insufficient funds.
528+
#[tokio::test]
529+
async fn pool_parks_descendant_unaffordable_with_l1_fee() {
530+
use crate::{OpL1BlockInfo, OpPooledTransaction, OpTransactionValidator};
531+
use alloy_consensus::{SignableTransaction, TxEip1559, transaction::Recovered};
532+
use alloy_eips::eip2718::Encodable2718;
533+
use alloy_primitives::{Address, Signature, TxKind};
534+
use parking_lot::RwLock;
535+
use reth_optimism_chainspec::OP_MAINNET;
536+
use reth_optimism_evm::{OpEvmConfig, RethL1BlockInfo};
537+
use reth_optimism_primitives::{OpPrimitives, OpTransactionSigned};
538+
use reth_provider::test_utils::{ExtendedAccount, MockEthProvider};
539+
use reth_transaction_pool::{
540+
CoinbaseTipOrdering, Pool, PoolConfig, PoolTransaction, TransactionOrigin,
541+
TransactionPool, blobstore::InMemoryBlobStore,
542+
validate::EthTransactionValidatorBuilder,
543+
};
544+
use std::sync::atomic::AtomicU64;
545+
546+
let signer = Address::with_last_byte(1);
547+
let make_tx = |nonce: u64| -> OpPooledTransaction {
548+
let tx: OpTransactionSigned = TxEip1559 {
549+
chain_id: 10,
550+
nonce,
551+
gas_limit: 21_000,
552+
max_fee_per_gas: 1_000_000_000,
553+
to: TxKind::Call(Address::with_last_byte(0x42)),
554+
..Default::default()
555+
}
556+
.into_signed(Signature::test_signature())
557+
.into();
558+
let recovered = Recovered::new_unchecked(tx, signer);
559+
let len = recovered.encode_2718_len();
560+
OpPooledTransaction::new(recovered, len)
561+
};
562+
let (tx0, tx1) = (make_tx(0), make_tx(1));
563+
564+
let mut l1_block_info = L1BlockInfo {
565+
l1_base_fee: U256::from(1_000_000_000_000u64),
566+
l1_base_fee_scalar: U256::from(1_000_000),
567+
..Default::default()
568+
};
569+
let l1_fee =
570+
l1_block_info.l1_tx_data_fee(OP_MAINNET.clone(), 0, tx0.encoded_2718(), false).unwrap();
571+
let l2_cost = *tx0.cost();
572+
assert!(l1_fee > l2_cost, "test needs the L1 fee to dominate, as on ink-mainnet");
573+
574+
// Covers either tx alone (and both txs' L2 cost), but not both txs' full OP cost.
575+
let balance = l2_cost * U256::from(2) + l1_fee + l1_fee / U256::from(2);
576+
577+
let client = MockEthProvider::<OpPrimitives>::new()
578+
.with_chain_spec(OP_MAINNET.clone())
579+
.with_genesis_block();
580+
client.add_account(signer, ExtendedAccount::new(0, balance));
581+
let inner =
582+
EthTransactionValidatorBuilder::new(client, OpEvmConfig::optimism(OP_MAINNET.clone()))
583+
.build(InMemoryBlobStore::default());
584+
let validator = OpTransactionValidator::with_block_info(
585+
inner,
586+
OpL1BlockInfo {
587+
l1_block_info: RwLock::new(l1_block_info),
588+
timestamp: AtomicU64::new(0),
589+
},
590+
);
591+
let pool = Pool::new(
592+
validator,
593+
CoinbaseTipOrdering::default(),
594+
InMemoryBlobStore::default(),
595+
PoolConfig::default(),
596+
);
597+
598+
pool.add_transaction(TransactionOrigin::External, tx0).await.unwrap();
599+
pool.add_transaction(TransactionOrigin::External, tx1).await.unwrap();
600+
601+
let pending: Vec<_> =
602+
pool.get_pending_transactions_by_sender(signer).iter().map(|tx| tx.nonce()).collect();
603+
let queued: Vec<_> =
604+
pool.get_queued_transactions_by_sender(signer).iter().map(|tx| tx.nonce()).collect();
605+
assert_eq!(pending, vec![0], "only nonce 0 is executable");
606+
assert_eq!(queued, vec![1], "nonce 1 cannot cover its L1 data fee after nonce 0");
607+
}
511608
}

0 commit comments

Comments
 (0)