Skip to content

celo-reth: Delete CeloConsensusTx newtype if op-reth upstreams pool-layer miner-fee accounting #152

@karlb

Description

@karlb

We added a lot of code in 8c1a706, just to avoid requiring changes to upstream. In the long run, it might be better to have a small change in upstream to remove the added code. LLM summary follows.

Background

Commit 8c1a706 (feat(celo-reth): add CeloConsensusTx newtype with cached native fees) introduced a ~600-line newtype wrapper around CeloTxEnvelope whose sole purpose is to carry ephemeral, native-equivalent fee values for CIP-64 transactions across the pool→consensus boundary. The wrapper is invisible to encoding, hashing, equality, serde, and every other codepath — it exists exclusively so that <CeloConsensusTx as Transaction>::effective_tip_per_gas can return a correct native-denominated tip when op-reth's payload builder calls it on the consensus tx.

The root cause is in op-reth's OpPayloadBuilderCtx::execute_best_transactions:

while let Some(tx) = best_txs.next(()) {
    let interop = tx.interop_deadline();
    let tx_da_size = tx.estimated_da_size();
    let tx = tx.into_consensus();   // ← pool-layer view discarded herelet miner_fee = tx.effective_tip_per_gas(base_fee).expect(...);
}

For CIP-64, CeloTxEnvelope::max_fee_per_gas is denominated in an ERC20 fee currency, not native wei, so the default effective_tip_per_gas(base_fee_in_wei) returns None and panics. CeloPoolTx already has the native-equivalent tip cached from pool validation, but into_consensus() throws that information away before miner_fee is computed.

The upstream fix

There's a trivial one-commit change to op-reth that makes the newtype unnecessary: celo-org/optimism@ebdd8bc5f — move the effective_tip_per_gas call to before into_consensus():

while let Some(tx) = best_txs.next(()) {
    let interop = tx.interop_deadline();
    let tx_da_size = tx.estimated_da_size();
    let miner_fee = tx
        .effective_tip_per_gas(base_fee)
        .expect("fee is always valid; enforced by the pool validator");
    let tx = tx.into_consensus();
    …
    info.total_fees += U256::from(miner_fee) * U256::from(gas_used);
}

Why this is a legitimate upstream pitch

  • Semantically identical for all current users. For stock OpPooledTransaction, the pool tx's fee methods just delegate to the inner consensus tx, so effective_tip_per_gas produces the same value whether called before or after into_consensus().
  • More informative layering. The pool tx is a strict superset of the consensus tx: it can carry fee-abstraction context (exchange-rate conversions, cached native equivalents) that doesn't exist on-chain. Computing miner_fee from the pool-layer view lets OP-derived chains with richer pool tx types plug in correct fee accounting without hooks in the builder.
  • Keeps the .expect(). No fault-tolerance degradation. The invariant just shifts from "execution succeeded" to "pool validator enforced max_fee_per_gas >= base_fee" — which is strictly when the guarantee is actually established.
  • Tiny diff. ~10 lines in one function in reth-optimism-payload-builder, with no type-system or API-surface changes.

What lands in celo-kona if upstreamed

If this one-line relocation gets merged into ethereum-optimism/optimism and makes it into a kona-client tag, the entire crates/celo-reth/src/signed_tx.rs module (~600 lines) can be deleted, along with:

  • The CeloConsensusTx::with_native_fees plumbing in CeloPoolTx::into_consensus / clone_into_consensus (crates/celo-reth/src/pool.rs)
  • The CeloTransactionSigned = CeloConsensusTx alias — it can go back to = CeloTxEnvelope (crates/celo-reth/src/primitives.rs)
  • CeloConsensusTx::new(...) wrapping in rpc.rs, test_utils.rs, node.rs::rpc_to_primitive_block

The CeloPoolTx::Transaction impl already overrides max_fee_per_gas / max_priority_fee_per_gas to return native-equivalent values (set during pool validation), so the default effective_tip_per_gas on CeloPoolTx will produce the correct native tip out of the box — no celo-side code change needed beyond deleting the newtype.

Action items

  • Open a PR against ethereum-optimism/optimism moving the effective_tip_per_gas call to before into_consensus() in OpPayloadBuilderCtx::execute_best_transactions (pitch as: "compute miner fee from the pool-layer view so fee-abstraction chains can plug in correct tip accounting").
  • If merged and tagged, bump celo-kona's ethereum-optimism/optimism dep to the new tag.
  • Delete crates/celo-reth/src/signed_tx.rs, revert the CeloTransactionSigned alias to CeloTxEnvelope, and strip the now-unnecessary wrapping at construction sites (pool, rpc, test_utils, node).

References

  • Introduced by: 8c1a706 feat(celo-reth): add CeloConsensusTx newtype with cached native fees
  • Proposed op-reth fix: celo-org/optimism@ebdd8bc5f
  • Upstream target file: rust/op-reth/crates/payload/src/builder.rs, OpPayloadBuilderCtx::execute_best_transactions

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions