From 5c2ceeeca4c0e0ddd48a895c3b836e210405c833 Mon Sep 17 00:00:00 2001 From: RealiCZ Date: Wed, 1 Jul 2026 10:32:29 +0800 Subject: [PATCH 1/7] fix(evm): make EnrichedMegaTx's precomputed tx_size/da_size reachable The MegaTransactionExt override on EnrichedMegaTx could never dispatch: the trait's tx_size/estimated_da_size methods carry a `where Self: Encodable2718` bound that EnrichedMegaTx never satisfied, so every caller silently recomputed from the inner transaction instead of reusing the stored fields. Add an Encodable2718 impl to satisfy the bound, and a new opt-in run_transaction_enriched/execute_mega_transaction_enriched entry point on MegaBlockExecutor that reads tx_size/da_size from the outer transaction (hitting EnrichedMegaTx's cache) instead of unwrapping to the raw inner tx. run_transaction itself is unchanged, so existing callers are unaffected. Adds a benchmark (crates/mega-evm/benches/enriched_tx.rs) and tests proving the fix changes real limit-enforcement behavior, not just a returned value. --- crates/mega-evm/Cargo.toml | 4 + crates/mega-evm/benches/enriched_tx.rs | 103 ++++++++++++++++++ crates/mega-evm/src/block/executor.rs | 49 ++++++++- crates/mega-evm/src/block/helpers.rs | 11 ++ .../tests/block_executor/block_limits.rs | 101 ++++++++++++++++- crates/mega-evm/tests/mutation/block.rs | 48 ++++++-- mutants/suppressions.toml | 24 ---- 7 files changed, 304 insertions(+), 36 deletions(-) create mode 100644 crates/mega-evm/benches/enriched_tx.rs diff --git a/crates/mega-evm/Cargo.toml b/crates/mega-evm/Cargo.toml index 3828658f..d231f737 100644 --- a/crates/mega-evm/Cargo.toml +++ b/crates/mega-evm/Cargo.toml @@ -75,6 +75,10 @@ harness = false name = "ctt" harness = false +[[bench]] +name = "enriched_tx" +harness = false + [[bench]] name = "mega_bench" harness = false diff --git a/crates/mega-evm/benches/enriched_tx.rs b/crates/mega-evm/benches/enriched_tx.rs new file mode 100644 index 00000000..ef524107 --- /dev/null +++ b/crates/mega-evm/benches/enriched_tx.rs @@ -0,0 +1,103 @@ +//! Benchmarks the cost of `MegaTransactionExt::{tx_size, estimated_da_size}` recompute vs the +//! `EnrichedMegaTx` cached fields. +//! +//! `EnrichedMegaTx` exists to precompute `tx_size`/`da_size` once (e.g. via `new_slow` from a +//! mempool-cached value) so block-execution callers can reuse them instead of recomputing on +//! every access. Three rows: +//! - `recompute_via_tx_unwrap` mirrors `MegaBlockExecutor::run_transaction`'s call pattern +//! (`tx.tx().estimated_da_size()` / `tx.tx().tx_size()`), which unwraps `EnrichedMegaTx` down +//! to the raw inner transaction and always hits the recomputing default impl. +//! - `via_trait_dispatch` mirrors `MegaBlockExecutor::run_transaction_enriched`'s call pattern +//! (`tx.estimated_da_size()` / `tx.tx_size()` on the outer wrapper), which now dispatches to +//! the stored fields. +//! - `cached_fields` reads the wrapper's precomputed fields directly, the floor `via_trait_dispatch` +//! should match. + +#![allow(missing_docs)] + +use alloy_consensus::{transaction::Recovered, Signed, TxLegacy}; +use alloy_evm::RecoveredTx; +use alloy_primitives::{address, Address, Bytes, Signature, TxKind, U256}; +use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion}; +use mega_evm::{EnrichedMegaTx, MegaTransactionExt, MegaTxEnvelope}; + +const CALLER: Address = address!("2000000000000000000000000000000000000001"); +const CONTRACT: Address = address!("3000000000000000000000000000000000000001"); + +/// Calldata sizes spanning a plain transfer up to a large multicall-style payload. +const CALLDATA_SIZES: &[usize] = &[0, 68, 180, 1000]; + +fn enriched_tx(calldata_len: usize) -> EnrichedMegaTx> { + let tx = TxLegacy { + chain_id: Some(1), + nonce: 7, + gas_price: 9, + gas_limit: 21_000, + to: TxKind::Call(CONTRACT), + value: U256::from(11), + input: Bytes::from(vec![0xabu8; calldata_len]), + }; + let envelope = MegaTxEnvelope::Legacy(Signed::new_unchecked( + tx, + Signature::test_signature(), + Default::default(), + )); + let recovered = Recovered::new_unchecked(envelope, CALLER); + EnrichedMegaTx::new_slow(recovered) +} + +/// `MegaBlockExecutor::run_transaction`'s hot-path pattern: `tx.tx().()` unwraps +/// `EnrichedMegaTx` down to the raw inner transaction, so `tx_size`/`estimated_da_size` always +/// hit the recomputing default impl even when the wrapper carries precomputed fields. +fn bench_recompute_via_tx_unwrap(c: &mut Criterion) { + let mut group = c.benchmark_group("tx_size_da_size/recompute_via_tx_unwrap"); + for &len in CALLDATA_SIZES { + let tx = enriched_tx(len); + group.bench_with_input(BenchmarkId::new("estimated_da_size", len), &tx, |b, tx| { + b.iter(|| black_box(tx.tx()).estimated_da_size()) + }); + group.bench_with_input(BenchmarkId::new("tx_size", len), &tx, |b, tx| { + b.iter(|| black_box(tx.tx()).tx_size()) + }); + } + group.finish(); +} + +/// `MegaBlockExecutor::run_transaction_enriched`'s call pattern: `tx.()` called directly +/// on the outer `EnrichedMegaTx`, dispatching to the stored fields via `MegaTransactionExt`. +fn bench_via_trait_dispatch(c: &mut Criterion) { + let mut group = c.benchmark_group("tx_size_da_size/via_trait_dispatch"); + for &len in CALLDATA_SIZES { + let tx = enriched_tx(len); + group.bench_with_input(BenchmarkId::new("estimated_da_size", len), &tx, |b, tx| { + b.iter(|| black_box(tx).estimated_da_size()) + }); + group.bench_with_input(BenchmarkId::new("tx_size", len), &tx, |b, tx| { + b.iter(|| black_box(tx).tx_size()) + }); + } + group.finish(); +} + +/// Reading `EnrichedMegaTx`'s precomputed fields directly. +fn bench_cached_fields(c: &mut Criterion) { + let mut group = c.benchmark_group("tx_size_da_size/cached_fields"); + for &len in CALLDATA_SIZES { + let tx = enriched_tx(len); + group.bench_with_input(BenchmarkId::new("estimated_da_size", len), &tx, |b, tx| { + b.iter(|| black_box(tx).da_size) + }); + group.bench_with_input(BenchmarkId::new("tx_size", len), &tx, |b, tx| { + b.iter(|| black_box(tx).tx_size) + }); + } + group.finish(); +} + +criterion_group!( + benches, + bench_recompute_via_tx_unwrap, + bench_via_trait_dispatch, + bench_cached_fields +); +criterion_main!(benches); diff --git a/crates/mega-evm/src/block/executor.rs b/crates/mega-evm/src/block/executor.rs index cbe4a22e..cdb12a9d 100644 --- a/crates/mega-evm/src/block/executor.rs +++ b/crates/mega-evm/src/block/executor.rs @@ -340,6 +340,21 @@ where self.run_transaction(tx) } + /// Alias to [`MegaBlockExecutor::run_transaction_enriched`]. + pub fn execute_mega_transaction_enriched( + &mut self, + tx: Tx, + ) -> Result, BlockExecutionError> + where + Tx: IntoTxEnv + + RecoveredTx + + MegaTransactionExt + + Encodable2718 + + Copy, + { + self.run_transaction_enriched(tx) + } + /// Execute a transaction with a commit condition function without committing the execution /// result to the block executor's inner state. /// @@ -358,9 +373,41 @@ where where Tx: IntoTxEnv + RecoveredTx + Copy, { - let is_deposit = tx.tx().ty() == DEPOSIT_TRANSACTION_TYPE; let tx_size = tx.tx().encode_2718_len() as u64; let da_size = tx.tx().estimated_da_size(); + self.run_transaction_with_sizes(tx, tx_size, da_size) + } + + /// Like [`MegaBlockExecutor::run_transaction`], but for callers whose `Tx` also implements + /// [`MegaTransactionExt`] (e.g. [`crate::EnrichedMegaTx`]): `tx_size`/`da_size` are read + /// from `Tx` itself instead of being recomputed from the raw inner transaction, so a + /// precomputed value (e.g. one already cached by a mempool) is actually reused. + pub fn run_transaction_enriched( + &mut self, + tx: Tx, + ) -> Result, BlockExecutionError> + where + Tx: IntoTxEnv + + RecoveredTx + + MegaTransactionExt + + Encodable2718 + + Copy, + { + let tx_size = tx.tx_size(); + let da_size = tx.estimated_da_size(); + self.run_transaction_with_sizes(tx, tx_size, da_size) + } + + fn run_transaction_with_sizes( + &mut self, + tx: Tx, + tx_size: u64, + da_size: u64, + ) -> Result, BlockExecutionError> + where + Tx: IntoTxEnv + RecoveredTx + Copy, + { + let is_deposit = tx.tx().ty() == DEPOSIT_TRANSACTION_TYPE; // Check transaction-level and block-level limits before transaction execution self.block_limiter.pre_execution_check( diff --git a/crates/mega-evm/src/block/helpers.rs b/crates/mega-evm/src/block/helpers.rs index 1898d993..a9c32ee5 100644 --- a/crates/mega-evm/src/block/helpers.rs +++ b/crates/mega-evm/src/block/helpers.rs @@ -2,11 +2,13 @@ use alloy_consensus::{transaction::Recovered, Transaction}; use alloy_eips::{eip2930::AccessList, eip7702::SignedAuthorization, Encodable2718, Typed2718}; use alloy_evm::{IntoTxEnv, RecoveredTx}; use alloy_primitives::{Address, Bytes, ChainId, Selector, TxHash, TxKind, B256, U256}; +use auto_impl::auto_impl; use delegate::delegate; use crate::MegaTxEnvelope; /// Helper trait that allows attaching extra information to a transaction. +#[auto_impl(&)] pub trait MegaTransactionExt { /// Get the estimated data availability size of the transaction. /// @@ -84,6 +86,15 @@ impl EnrichedMegaTx { } } +impl Encodable2718 for EnrichedMegaTx { + delegate! { + to self.inner { + fn encode_2718_len(&self) -> usize; + fn encode_2718(&self, out: &mut dyn alloy_primitives::bytes::BufMut); + } + } +} + impl MegaTransactionExt for EnrichedMegaTx { fn estimated_da_size(&self) -> u64 { self.da_size diff --git a/crates/mega-evm/tests/block_executor/block_limits.rs b/crates/mega-evm/tests/block_executor/block_limits.rs index 6dbf7f9e..fe0a742f 100644 --- a/crates/mega-evm/tests/block_executor/block_limits.rs +++ b/crates/mega-evm/tests/block_executor/block_limits.rs @@ -12,8 +12,8 @@ use alloy_op_evm::block::receipt_builder::OpAlloyReceiptBuilder; use alloy_primitives::{address, Bytes, Signature, TxKind, B256, U256}; use mega_evm::{ test_utils::{BytecodeBuilder, MemoryDatabase}, - BlockLimits, MegaBlockExecutionCtx, MegaBlockExecutor, MegaEvmFactory, MegaHardforkConfig, - MegaSpecId, MegaTxEnvelope, TestExternalEnvs, + BlockLimits, EnrichedMegaTx, MegaBlockExecutionCtx, MegaBlockExecutor, MegaEvmFactory, + MegaHardforkConfig, MegaSpecId, MegaTransactionExt, MegaTxEnvelope, TestExternalEnvs, }; use revm::{ bytecode::opcode::{ADD, DUP1, LOG0, PUSH0, SLOAD, SSTORE}, @@ -43,6 +43,22 @@ fn create_transaction( alloy_consensus::transaction::Recovered::new_unchecked(tx, CALLER) } +/// Like [`create_transaction`], but returns the bare envelope so callers can build a +/// `Recovered<&MegaTxEnvelope>` (needed for `EnrichedMegaTx: Copy`, which requires `T: Copy`). +fn create_envelope(nonce: u64, gas_limit: u64) -> MegaTxEnvelope { + let tx_legacy = TxLegacy { + chain_id: Some(8453), // Base mainnet + nonce, + gas_price: 1_000_000, + gas_limit, + to: TxKind::Call(CONTRACT), + value: U256::ZERO, + input: Bytes::new(), + }; + let signed = Signed::new_unchecked(tx_legacy, Signature::test_signature(), Default::default()); + MegaTxEnvelope::Legacy(signed) +} + /// Creates a contract that generates a log with specified data size. /// /// The contract will emit LOG0 with the specified number of zero bytes. @@ -180,6 +196,87 @@ fn test_block_custom_data_limit() { ); } +/// `MegaBlockExecutor::run_transaction_enriched` must use `EnrichedMegaTx`'s precomputed +/// `da_size` for the pre-execution DA-size limit check instead of recomputing from the raw +/// transaction. +/// +/// A freshly recomputed DA size for any legacy tx is always >= 100 bytes (the `op_alloy_flz` +/// minimum-size floor), so a `tx_da_size_limit` of 80 rejects every transaction under +/// `run_transaction` — both for a plain `Recovered<&MegaTxEnvelope>` and for an `EnrichedMegaTx` +/// wrapping it, since `run_transaction` always recomputes. A deliberately understated stored +/// `da_size` of 50 passes that same limit only via `run_transaction_enriched`, proving the +/// stored value (not a recomputation) is what actually gets checked. +#[test] +fn test_run_transaction_enriched_uses_stored_da_size_for_limit_check() { + let mut db = MemoryDatabase::default(); + db.set_account_balance(CALLER, U256::from(1_000_000_000_000_000u64)); + + let mut state = State::builder().with_database(&mut db).build(); + let external_envs = TestExternalEnvs::::new(); + let evm_factory = MegaEvmFactory::new().with_external_env_factory(external_envs); + + let mut cfg_env = revm::context::CfgEnv::default(); + cfg_env.spec = MegaSpecId::MINI_REX; + let block_env = BlockEnv { + number: U256::from(1000), + timestamp: U256::from(1_800_000_000), + gas_limit: 30_000_000, + ..Default::default() + }; + let evm_env = EvmEnv::new(cfg_env, block_env); + let evm = evm_factory.create_evm(&mut state, evm_env); + + const TX_DA_SIZE_LIMIT: u64 = 80; + const STORED_DA_SIZE: u64 = 50; + + let block_ctx = MegaBlockExecutionCtx::new( + B256::ZERO, + None, + Bytes::new(), + BlockLimits::no_limits().with_tx_da_size_limit(TX_DA_SIZE_LIMIT), + ); + + use alloy_hardforks::ForkCondition; + use mega_evm::MegaHardfork; + let chain_spec = + MegaHardforkConfig::default().with(MegaHardfork::MiniRex, ForkCondition::Timestamp(0)); + let receipt_builder = OpAlloyReceiptBuilder::default(); + let mut executor = MegaBlockExecutor::new(evm, block_ctx, chain_spec, receipt_builder); + + let envelope = create_envelope(0, 100_000); + let real_da_size = MegaTransactionExt::estimated_da_size(&envelope); + assert!( + real_da_size > TX_DA_SIZE_LIMIT, + "test setup requires the freshly recomputed da_size ({real_da_size}) to exceed the limit" + ); + const { assert!(STORED_DA_SIZE < TX_DA_SIZE_LIMIT, "stored da_size must pass the limit") }; + + let recovered = alloy_consensus::transaction::Recovered::new_unchecked(&envelope, CALLER); + + // A plain (non-enriched) transaction recomputes da_size and is rejected. + let plain_result = executor.run_transaction(&recovered); + assert!( + plain_result.is_err(), + "plain transaction should be rejected: real da_size exceeds the limit" + ); + + // `run_transaction` (unmodified path) recomputes even for an `EnrichedMegaTx`, so the + // understated stored value has no effect and the transaction is still rejected. + let enriched = EnrichedMegaTx::new(recovered, envelope.trie_hash(), STORED_DA_SIZE, 10); + let unfixed_result = executor.run_transaction(enriched); + assert!( + unfixed_result.is_err(), + "run_transaction must still recompute da_size for EnrichedMegaTx, ignoring the stored field" + ); + + // `run_transaction_enriched` reads the stored (understated) da_size, so the transaction + // that was rejected above now passes the same limit. + let outcome = executor + .run_transaction_enriched(enriched) + .expect("run_transaction_enriched must use the stored da_size, which passes the limit"); + assert_eq!(outcome.da_size, STORED_DA_SIZE, "outcome da_size must be the stored value"); +} + #[test] fn test_block_custom_kv_update_limit() { // Create database and deploy contract diff --git a/crates/mega-evm/tests/mutation/block.rs b/crates/mega-evm/tests/mutation/block.rs index 78f3e364..4a03633a 100644 --- a/crates/mega-evm/tests/mutation/block.rs +++ b/crates/mega-evm/tests/mutation/block.rs @@ -19,9 +19,9 @@ use alloy_consensus::{transaction::Recovered, Signed, TxLegacy}; use alloy_hardforks::ForkCondition; use alloy_primitives::{address, Address, Bytes, Signature, TxKind, U256}; use mega_evm::{ - constants, hardfork_schedule, mainnet_hardforks, testnet_hardforks, BlockLimits, MegaHardfork, - MegaHardforkConfig, MegaHardforks, MegaTransactionExt, MegaTxEnvelope, MAINNET_CHAIN_ID, - TESTNET_CHAIN_ID, + constants, hardfork_schedule, mainnet_hardforks, testnet_hardforks, BlockLimits, + EnrichedMegaTx, MegaHardfork, MegaHardforkConfig, MegaHardforks, MegaTransactionExt, + MegaTxEnvelope, MAINNET_CHAIN_ID, TESTNET_CHAIN_ID, }; const CALLER: Address = address!("2000000000000000000000000000000000000001"); @@ -82,12 +82,42 @@ fn test_recovered_mega_tx_envelope_ext_returns_real_hash() { assert_eq!(h, STORED_TX_HASH, "recovered tx_hash mismatch"); } -// `EnrichedMegaTx::{tx_size, estimated_da_size}` (helpers.rs:89/93) are NOT tested here: the -// stored-field overrides are unreachable — the trait method's `where Self: Encodable2718` bound is -// unsatisfiable for the wrapper (it does not implement `Encodable2718`), and `.method()` resolves -// via `Deref` to the inner tx instead, so the override never dispatches. Those mutants are recorded -// as dead/equivalent in `mutants/suppressions.toml`. (This dead override looks like a latent -// perf bug: callers recompute size/da instead of using the precomputed fields.) +/// `EnrichedMegaTx::{tx_size, estimated_da_size, tx_hash}` must dispatch to the stored fields, not +/// recompute from the inner transaction. Constructs the wrapper with stored values that +/// deliberately differ from what a fresh recomputation on the inner tx would produce, so a mutant +/// collapsing the override to a constant (or one that silently falls back to recomputing) is +/// distinguishable. +#[test] +fn test_enriched_mega_tx_ext_returns_stored_fields_not_recomputed() { + let tx = legacy_envelope(); + let recovered = Recovered::new_unchecked(tx, CALLER); + + let real_da_size = MegaTransactionExt::estimated_da_size(&recovered); + let real_tx_size = MegaTransactionExt::tx_size(&recovered); + + const STORED_DA_SIZE: u64 = 424_242; + const STORED_TX_SIZE: u64 = 131_313; + assert_ne!(STORED_DA_SIZE, real_da_size, "stored da_size must differ from a fresh recompute"); + assert_ne!(STORED_TX_SIZE, real_tx_size, "stored tx_size must differ from a fresh recompute"); + + let enriched = EnrichedMegaTx::new(recovered, STORED_TX_HASH, STORED_DA_SIZE, STORED_TX_SIZE); + + assert_eq!( + MegaTransactionExt::estimated_da_size(&enriched), + STORED_DA_SIZE, + "estimated_da_size must return the stored field, not a recomputed value" + ); + assert_eq!( + MegaTransactionExt::tx_size(&enriched), + STORED_TX_SIZE, + "tx_size must return the stored field, not a recomputed value" + ); + assert_eq!( + MegaTransactionExt::tx_hash(&enriched), + STORED_TX_HASH, + "tx_hash must return the stored field, not Default::default()" + ); +} // ============================================================================ // block/limit.rs — BlockLimits::from_hardfork_and_block_gas_limit diff --git a/mutants/suppressions.toml b/mutants/suppressions.toml index ce2feb82..8894c225 100644 --- a/mutants/suppressions.toml +++ b/mutants/suppressions.toml @@ -136,30 +136,6 @@ mutant = "replace ::external_envs justification = "EmptyExternalEnv is a unit struct; real body builds ExternalEnvs{salt_env:EmptyExternalEnv, oracle_env:EmptyExternalEnv}; from(Default::default()) resolves to Default for ExternalEnvs producing identical fields. Value-identical, no observable behavioral difference." reviewer = "improve-mutation-score (William Aaron Cheung)" -# --- EnrichedMegaTx stored-field overrides (dead: unreachable) --------------------- -# `>::{tx_size, estimated_da_size}` (helpers.rs:89/93) -# override the trait defaults to return precomputed fields, but they are unreachable: the trait -# declares these methods with `where Self: Encodable2718`, and EnrichedMegaTx does NOT implement -# Encodable2718 (only an inherent `impl EnrichedMegaTx` exists). So the bound is -# unsatisfiable for any caller, and `enriched.tx_size()` / `.estimated_da_size()` resolve via Deref -# to the inner T instead. The override bodies are never dispatched; mutating them (-> 0 / -> 1) -# changes no observable behavior. (Latent perf bug: the precomputed fields are never used.) -[[suppress]] -kind = "function" -category = "dead" -file = "crates/mega-evm/src/block/helpers.rs" -pattern = ">::tx_size" -justification = "Dead/unreachable: trait method's `where Self: Encodable2718` bound is unsatisfiable for EnrichedMegaTx (no Encodable2718 impl), and `.tx_size()` resolves via Deref to inner T. Override never dispatched; mutation changes nothing." -reviewer = "improve-mutation-score (William Aaron Cheung)" - -[[suppress]] -kind = "function" -category = "dead" -file = "crates/mega-evm/src/block/helpers.rs" -pattern = ">::estimated_da_size" -justification = "Dead/unreachable: trait method's `where Self: Encodable2718` bound is unsatisfiable for EnrichedMegaTx (no Encodable2718 impl), and `.estimated_da_size()` resolves via Deref to inner T. Override never dispatched; mutation changes nothing." -reviewer = "improve-mutation-score (William Aaron Cheung)" - # --- DeterministicHasher::write line-125 `|` combine (equivalent) ------------------ # `let combined = (value[0] as u128) | ((value[1] as u128) << 64);` where # `value: [u64; 2] = read_small(data)`. The first operand `value[0] as u128` is a u64 widened From f74e3c157017b8a9afb0164c368d4026f4a9285e Mon Sep 17 00:00:00 2001 From: RealiCZ Date: Wed, 1 Jul 2026 10:41:38 +0800 Subject: [PATCH 2/7] docs(evm): document run_transaction_with_sizes, reflow bench comment wrapping Add a doc comment explaining why run_transaction_with_sizes exists (shared body for run_transaction/run_transaction_enriched, sizes resolved by the caller). Also reflows enriched_tx.rs's module doc comment to satisfy cargo fmt's comment wrapping. --- crates/mega-evm/benches/enriched_tx.rs | 12 ++++++------ crates/mega-evm/src/block/executor.rs | 3 +++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/mega-evm/benches/enriched_tx.rs b/crates/mega-evm/benches/enriched_tx.rs index ef524107..36b60917 100644 --- a/crates/mega-evm/benches/enriched_tx.rs +++ b/crates/mega-evm/benches/enriched_tx.rs @@ -5,13 +5,13 @@ //! mempool-cached value) so block-execution callers can reuse them instead of recomputing on //! every access. Three rows: //! - `recompute_via_tx_unwrap` mirrors `MegaBlockExecutor::run_transaction`'s call pattern -//! (`tx.tx().estimated_da_size()` / `tx.tx().tx_size()`), which unwraps `EnrichedMegaTx` down -//! to the raw inner transaction and always hits the recomputing default impl. +//! (`tx.tx().estimated_da_size()` / `tx.tx().tx_size()`), which unwraps `EnrichedMegaTx` down to +//! the raw inner transaction and always hits the recomputing default impl. //! - `via_trait_dispatch` mirrors `MegaBlockExecutor::run_transaction_enriched`'s call pattern -//! (`tx.estimated_da_size()` / `tx.tx_size()` on the outer wrapper), which now dispatches to -//! the stored fields. -//! - `cached_fields` reads the wrapper's precomputed fields directly, the floor `via_trait_dispatch` -//! should match. +//! (`tx.estimated_da_size()` / `tx.tx_size()` on the outer wrapper), which now dispatches to the +//! stored fields. +//! - `cached_fields` reads the wrapper's precomputed fields directly, the floor +//! `via_trait_dispatch` should match. #![allow(missing_docs)] diff --git a/crates/mega-evm/src/block/executor.rs b/crates/mega-evm/src/block/executor.rs index cdb12a9d..acb8b7a2 100644 --- a/crates/mega-evm/src/block/executor.rs +++ b/crates/mega-evm/src/block/executor.rs @@ -398,6 +398,9 @@ where self.run_transaction_with_sizes(tx, tx_size, da_size) } + /// Shared body of [`MegaBlockExecutor::run_transaction`] and + /// [`MegaBlockExecutor::run_transaction_enriched`]: `tx_size`/`da_size` are resolved by the + /// caller (recomputed or read from a cache), this only consumes them. fn run_transaction_with_sizes( &mut self, tx: Tx, From e4a23f2de1aea1a6dbbfe1ebeefad669acfd84ff Mon Sep 17 00:00:00 2001 From: RealiCZ Date: Wed, 1 Jul 2026 10:55:57 +0800 Subject: [PATCH 3/7] fix(evm): enforce the tx_size/da_size cache contract on run_transaction_enriched run_transaction_enriched feeds Tx-reported tx_size/da_size straight into pre_execution_check's consensus-relevant limit checks with no validation. EnrichedMegaTx::new is a public, fully trusting constructor, so a stale or understated cache could let a transaction bypass tx_da_size_limit, tx_encode_size_limit, or the block cumulative-size limits. Add a debug_assert cross-check against a fresh recompute in run_transaction_enriched (compiled out in release builds, so it doesn't cost the performance this method exists to deliver), and document the caller contract on both run_transaction_enriched and EnrichedMegaTx::new. Also documents on run_transaction why the standard BlockExecutor trait path can't take the enriched shortcut. Restructures the block-limit integration test to assert the debug_assert actually fires on a deliberately mismatched cached da_size, proving the safety net works, since the assert now makes the test's original "cached value silently wins" scenario panic by design. --- crates/mega-evm/src/block/executor.rs | 38 +++++++++++++++++++ crates/mega-evm/src/block/helpers.rs | 7 ++++ .../tests/block_executor/block_limits.rs | 31 ++++++++------- 3 files changed, 63 insertions(+), 13 deletions(-) diff --git a/crates/mega-evm/src/block/executor.rs b/crates/mega-evm/src/block/executor.rs index acb8b7a2..3ed66000 100644 --- a/crates/mega-evm/src/block/executor.rs +++ b/crates/mega-evm/src/block/executor.rs @@ -341,6 +341,8 @@ where } /// Alias to [`MegaBlockExecutor::run_transaction_enriched`]. + /// + /// See that method's doc comment for the caller contract on `tx_size`/`da_size` accuracy. pub fn execute_mega_transaction_enriched( &mut self, tx: Tx, @@ -366,6 +368,14 @@ where /// /// Returns the execution outcome of the transaction. Note that the execution result is not /// committed to the block executor's inner state. + /// Recomputes `tx_size`/`da_size` from the raw inner transaction on every call. For a `Tx` + /// that carries precomputed values (e.g. [`crate::EnrichedMegaTx`]), prefer + /// [`MegaBlockExecutor::run_transaction_enriched`] to reuse them instead. + /// + /// This method exists (rather than always taking the enriched path) because it also backs + /// [`BlockExecutor::execute_transaction_with_commit_condition`], whose `tx: impl + /// ExecutableTx` parameter is constrained by `alloy_evm` and cannot be required to + /// implement [`MegaTransactionExt`]. pub fn run_transaction( &mut self, tx: Tx, @@ -382,6 +392,22 @@ where /// [`MegaTransactionExt`] (e.g. [`crate::EnrichedMegaTx`]): `tx_size`/`da_size` are read /// from `Tx` itself instead of being recomputed from the raw inner transaction, so a /// precomputed value (e.g. one already cached by a mempool) is actually reused. + /// + /// # Correctness + /// + /// `tx_size`/`da_size` feed directly into [`BlockLimiter::pre_execution_check`]'s + /// `tx_encode_size_limit`/`tx_da_size_limit`/block cumulative-size checks, with no + /// validation against the real encoded transaction. Callers MUST ensure `Tx::tx_size()`/ + /// `Tx::estimated_da_size()` accurately reflect `tx`'s actual EIP-2718 encoding — an + /// understated value lets a transaction bypass a limit it should have been rejected by. + /// This is safe for the sequencer's own block-building path (the cache is computed by the + /// same trusted process, e.g. at mempool insertion), but this method must not be fed a + /// `Tx` whose cached sizes could come from an untrusted or stale source (e.g. block + /// validation of another party's block) without first re-establishing that invariant. + /// + /// A `debug_assert` cross-checks the cached values against a fresh recompute, so a mismatch + /// is caught in tests/CI; it is compiled out in release builds, so it does not affect the + /// performance this method exists to deliver. pub fn run_transaction_enriched( &mut self, tx: Tx, @@ -395,6 +421,18 @@ where { let tx_size = tx.tx_size(); let da_size = tx.estimated_da_size(); + debug_assert_eq!( + tx_size, + tx.encode_2718_len() as u64, + "run_transaction_enriched: Tx-reported tx_size does not match a fresh recompute \ + from the encoded transaction" + ); + debug_assert_eq!( + da_size, + op_alloy_flz::tx_estimated_size_fjord_bytes(tx.encoded_2718().as_slice()), + "run_transaction_enriched: Tx-reported da_size does not match a fresh recompute \ + from the encoded transaction" + ); self.run_transaction_with_sizes(tx, tx_size, da_size) } diff --git a/crates/mega-evm/src/block/helpers.rs b/crates/mega-evm/src/block/helpers.rs index a9c32ee5..80ba5b1c 100644 --- a/crates/mega-evm/src/block/helpers.rs +++ b/crates/mega-evm/src/block/helpers.rs @@ -68,6 +68,13 @@ pub struct EnrichedMegaTx { impl EnrichedMegaTx { /// Create a new `WithDASize` wrapper with a known data availability size. + /// + /// `da_size`/`tx_size` are trusted as-is and not validated against `inner`'s actual + /// encoding. When this value reaches [`crate::MegaBlockExecutor::run_transaction_enriched`], + /// it is used directly for `tx_da_size_limit`/`tx_encode_size_limit`/block cumulative-size + /// enforcement, so an inaccurate value can let a transaction bypass a limit it should have + /// been rejected by. Only pass values computed from `inner` itself (e.g. by a trusted + /// mempool at insertion time); prefer [`EnrichedMegaTx::new_slow`] when in doubt. pub fn new(inner: T, tx_hash: TxHash, da_size: u64, tx_size: u64) -> Self { Self { inner, tx_hash, da_size, tx_size } } diff --git a/crates/mega-evm/tests/block_executor/block_limits.rs b/crates/mega-evm/tests/block_executor/block_limits.rs index fe0a742f..6221a80b 100644 --- a/crates/mega-evm/tests/block_executor/block_limits.rs +++ b/crates/mega-evm/tests/block_executor/block_limits.rs @@ -196,17 +196,21 @@ fn test_block_custom_data_limit() { ); } -/// `MegaBlockExecutor::run_transaction_enriched` must use `EnrichedMegaTx`'s precomputed -/// `da_size` for the pre-execution DA-size limit check instead of recomputing from the raw -/// transaction. +/// `MegaBlockExecutor::run_transaction_enriched` trusts `Tx`'s reported `tx_size`/`da_size` for +/// the pre-execution limit check instead of recomputing them from the raw transaction (see that +/// method's "Correctness" doc section). Its `debug_assert` cross-checks the reported values +/// against a fresh recompute, so a caller bug — or, as constructed here, a deliberately +/// understated stored `da_size` — is caught immediately in debug/test builds instead of silently +/// letting a transaction bypass a limit it should have been rejected by. /// /// A freshly recomputed DA size for any legacy tx is always >= 100 bytes (the `op_alloy_flz` /// minimum-size floor), so a `tx_da_size_limit` of 80 rejects every transaction under /// `run_transaction` — both for a plain `Recovered<&MegaTxEnvelope>` and for an `EnrichedMegaTx` -/// wrapping it, since `run_transaction` always recomputes. A deliberately understated stored -/// `da_size` of 50 passes that same limit only via `run_transaction_enriched`, proving the -/// stored value (not a recomputation) is what actually gets checked. +/// wrapping it, since `run_transaction` always recomputes. An `EnrichedMegaTx` constructed with a +/// deliberately understated stored `da_size` of 50 trips `run_transaction_enriched`'s +/// `debug_assert` rather than silently passing that same limit. #[test] +#[should_panic(expected = "does not match a fresh recompute")] fn test_run_transaction_enriched_uses_stored_da_size_for_limit_check() { let mut db = MemoryDatabase::default(); db.set_account_balance(CALLER, U256::from(1_000_000_000_000_000u64)); @@ -262,19 +266,20 @@ fn test_run_transaction_enriched_uses_stored_da_size_for_limit_check() { // `run_transaction` (unmodified path) recomputes even for an `EnrichedMegaTx`, so the // understated stored value has no effect and the transaction is still rejected. - let enriched = EnrichedMegaTx::new(recovered, envelope.trie_hash(), STORED_DA_SIZE, 10); + // `tx_size` is stored accurately here so the debug_assert below trips on `da_size` alone. + let real_tx_size = MegaTransactionExt::tx_size(&envelope); + let enriched = + EnrichedMegaTx::new(recovered, envelope.trie_hash(), STORED_DA_SIZE, real_tx_size); let unfixed_result = executor.run_transaction(enriched); assert!( unfixed_result.is_err(), "run_transaction must still recompute da_size for EnrichedMegaTx, ignoring the stored field" ); - // `run_transaction_enriched` reads the stored (understated) da_size, so the transaction - // that was rejected above now passes the same limit. - let outcome = executor - .run_transaction_enriched(enriched) - .expect("run_transaction_enriched must use the stored da_size, which passes the limit"); - assert_eq!(outcome.da_size, STORED_DA_SIZE, "outcome da_size must be the stored value"); + // `run_transaction_enriched` would use the stored (understated) da_size for the limit check — + // which would incorrectly pass a transaction the real recomputation rejects — but its + // debug_assert catches the mismatch and panics instead of silently letting it through. + let _ = executor.run_transaction_enriched(enriched); } #[test] From 67123e5e978e47bfa162686a8c414652ea0de792 Mon Sep 17 00:00:00 2001 From: RealiCZ Date: Wed, 1 Jul 2026 11:04:51 +0800 Subject: [PATCH 4/7] fix(evm): gate the debug_assert-only test to debug builds, fix broken doc link The should_panic test for run_transaction_enriched's debug_assert cache cross-check failed under `cargo test --release`: debug_assert compiles out in release builds, so the deliberately mismatched cached da_size silently succeeds instead of panicking. Gate the test (and its now debug-only-used imports/helper) behind #[cfg(debug_assertions)] and document that this is the intended profile-dependent trade-off, not an oversight. Also fixes a broken rustdoc intra-doc link on run_transaction: BlockExecutor isn't in scope in this file (only referenced via the fully qualified alloy_evm::block::BlockExecutor path), so the unqualified link never resolved. cargo doc -p mega-evm --no-deps now has no new warnings from this change. --- crates/mega-evm/src/block/executor.rs | 17 +++++++++-------- .../tests/block_executor/block_limits.rs | 16 ++++++++++++++-- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/crates/mega-evm/src/block/executor.rs b/crates/mega-evm/src/block/executor.rs index 3ed66000..bccb4b14 100644 --- a/crates/mega-evm/src/block/executor.rs +++ b/crates/mega-evm/src/block/executor.rs @@ -360,6 +360,15 @@ where /// Execute a transaction with a commit condition function without committing the execution /// result to the block executor's inner state. /// + /// Recomputes `tx_size`/`da_size` from the raw inner transaction on every call. For a `Tx` + /// that carries precomputed values (e.g. [`crate::EnrichedMegaTx`]), prefer + /// [`MegaBlockExecutor::run_transaction_enriched`] to reuse them instead. + /// + /// This method exists (rather than always taking the enriched path) because it also backs + /// [`alloy_evm::block::BlockExecutor::execute_transaction_with_commit_condition`], whose + /// `tx: impl ExecutableTx` parameter is constrained by `alloy_evm` and cannot be + /// required to implement [`MegaTransactionExt`]. + /// /// # Parameters /// /// - `tx`: The transaction to execute. @@ -368,14 +377,6 @@ where /// /// Returns the execution outcome of the transaction. Note that the execution result is not /// committed to the block executor's inner state. - /// Recomputes `tx_size`/`da_size` from the raw inner transaction on every call. For a `Tx` - /// that carries precomputed values (e.g. [`crate::EnrichedMegaTx`]), prefer - /// [`MegaBlockExecutor::run_transaction_enriched`] to reuse them instead. - /// - /// This method exists (rather than always taking the enriched path) because it also backs - /// [`BlockExecutor::execute_transaction_with_commit_condition`], whose `tx: impl - /// ExecutableTx` parameter is constrained by `alloy_evm` and cannot be required to - /// implement [`MegaTransactionExt`]. pub fn run_transaction( &mut self, tx: Tx, diff --git a/crates/mega-evm/tests/block_executor/block_limits.rs b/crates/mega-evm/tests/block_executor/block_limits.rs index 6221a80b..c0b727a7 100644 --- a/crates/mega-evm/tests/block_executor/block_limits.rs +++ b/crates/mega-evm/tests/block_executor/block_limits.rs @@ -12,9 +12,12 @@ use alloy_op_evm::block::receipt_builder::OpAlloyReceiptBuilder; use alloy_primitives::{address, Bytes, Signature, TxKind, B256, U256}; use mega_evm::{ test_utils::{BytecodeBuilder, MemoryDatabase}, - BlockLimits, EnrichedMegaTx, MegaBlockExecutionCtx, MegaBlockExecutor, MegaEvmFactory, - MegaHardforkConfig, MegaSpecId, MegaTransactionExt, MegaTxEnvelope, TestExternalEnvs, + BlockLimits, MegaBlockExecutionCtx, MegaBlockExecutor, MegaEvmFactory, MegaHardforkConfig, + MegaSpecId, MegaTxEnvelope, TestExternalEnvs, }; +// Only used by the debug_assertions-only test below (see its doc comment). +#[cfg(debug_assertions)] +use mega_evm::{EnrichedMegaTx, MegaTransactionExt}; use revm::{ bytecode::opcode::{ADD, DUP1, LOG0, PUSH0, SLOAD, SSTORE}, context::BlockEnv, @@ -45,6 +48,8 @@ fn create_transaction( /// Like [`create_transaction`], but returns the bare envelope so callers can build a /// `Recovered<&MegaTxEnvelope>` (needed for `EnrichedMegaTx: Copy`, which requires `T: Copy`). +/// Only used by the debug_assertions-only test below (see its doc comment). +#[cfg(debug_assertions)] fn create_envelope(nonce: u64, gas_limit: u64) -> MegaTxEnvelope { let tx_legacy = TxLegacy { chain_id: Some(8453), // Base mainnet @@ -209,6 +214,13 @@ fn test_block_custom_data_limit() { /// wrapping it, since `run_transaction` always recomputes. An `EnrichedMegaTx` constructed with a /// deliberately understated stored `da_size` of 50 trips `run_transaction_enriched`'s /// `debug_assert` rather than silently passing that same limit. +/// +/// `#[cfg(debug_assertions)]`: this test exercises the `debug_assert` itself, which is compiled +/// out in release builds — under `cargo test --release` the same call would not panic and would +/// instead silently succeed using the understated cached value. That is the documented, +/// deliberate trade-off on `run_transaction_enriched` (see its "Correctness" doc section): the +/// cache contract is enforced in debug/test builds only, at zero cost in release. +#[cfg(debug_assertions)] #[test] #[should_panic(expected = "does not match a fresh recompute")] fn test_run_transaction_enriched_uses_stored_da_size_for_limit_check() { From 446020e89cefa979d5c5e16557f426797aa65913 Mon Sep 17 00:00:00 2001 From: RealiCZ Date: Wed, 1 Jul 2026 11:42:19 +0800 Subject: [PATCH 5/7] fix(evm): delegate type_flag, close patch coverage gap on the enriched path Delegate Encodable2718::type_flag on EnrichedMegaTx to the inner transaction instead of relying on the trait default. Verified with a temp test that the default (computed via the existing Typed2718::ty() delegation) already returns the correct value for typed transactions -- so this isn't a behavior fix, just matching this file's own convention of exhaustively delegating rather than relying on trait defaults (all four other trait impls on EnrichedMegaTx already do this), and it's more robust against a future T that overrides type_flag non-trivially. Also closes a real patch-coverage gap: execute_mega_transaction_enriched (the alias) had no caller in any test, and run_transaction_enriched's only test deliberately panics via its debug_assert before ever reaching the success path. Add a happy-path test that constructs EnrichedMegaTx::new_slow (an accurate cache) and calls execute_mega_transaction_enriched, asserting it succeeds and the outcome carries the cached tx_size/da_size. Un-gates the create_envelope/EnrichedMegaTx/MegaTransactionExt test-only imports from #[cfg(debug_assertions)] since they're now also used unconditionally. --- crates/mega-evm/src/block/helpers.rs | 1 + .../tests/block_executor/block_limits.rs | 56 ++++++++++++++++--- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/crates/mega-evm/src/block/helpers.rs b/crates/mega-evm/src/block/helpers.rs index 80ba5b1c..1813b0d3 100644 --- a/crates/mega-evm/src/block/helpers.rs +++ b/crates/mega-evm/src/block/helpers.rs @@ -96,6 +96,7 @@ impl EnrichedMegaTx { impl Encodable2718 for EnrichedMegaTx { delegate! { to self.inner { + fn type_flag(&self) -> Option; fn encode_2718_len(&self) -> usize; fn encode_2718(&self, out: &mut dyn alloy_primitives::bytes::BufMut); } diff --git a/crates/mega-evm/tests/block_executor/block_limits.rs b/crates/mega-evm/tests/block_executor/block_limits.rs index c0b727a7..4943333f 100644 --- a/crates/mega-evm/tests/block_executor/block_limits.rs +++ b/crates/mega-evm/tests/block_executor/block_limits.rs @@ -12,12 +12,9 @@ use alloy_op_evm::block::receipt_builder::OpAlloyReceiptBuilder; use alloy_primitives::{address, Bytes, Signature, TxKind, B256, U256}; use mega_evm::{ test_utils::{BytecodeBuilder, MemoryDatabase}, - BlockLimits, MegaBlockExecutionCtx, MegaBlockExecutor, MegaEvmFactory, MegaHardforkConfig, - MegaSpecId, MegaTxEnvelope, TestExternalEnvs, + BlockLimits, EnrichedMegaTx, MegaBlockExecutionCtx, MegaBlockExecutor, MegaEvmFactory, + MegaHardforkConfig, MegaSpecId, MegaTransactionExt, MegaTxEnvelope, TestExternalEnvs, }; -// Only used by the debug_assertions-only test below (see its doc comment). -#[cfg(debug_assertions)] -use mega_evm::{EnrichedMegaTx, MegaTransactionExt}; use revm::{ bytecode::opcode::{ADD, DUP1, LOG0, PUSH0, SLOAD, SSTORE}, context::BlockEnv, @@ -48,8 +45,6 @@ fn create_transaction( /// Like [`create_transaction`], but returns the bare envelope so callers can build a /// `Recovered<&MegaTxEnvelope>` (needed for `EnrichedMegaTx: Copy`, which requires `T: Copy`). -/// Only used by the debug_assertions-only test below (see its doc comment). -#[cfg(debug_assertions)] fn create_envelope(nonce: u64, gas_limit: u64) -> MegaTxEnvelope { let tx_legacy = TxLegacy { chain_id: Some(8453), // Base mainnet @@ -294,6 +289,53 @@ fn test_run_transaction_enriched_uses_stored_da_size_for_limit_check() { let _ = executor.run_transaction_enriched(enriched); } +/// `MegaBlockExecutor::execute_mega_transaction_enriched` (the alias for +/// [`MegaBlockExecutor::run_transaction_enriched`]) must succeed and produce an outcome carrying +/// the cached fields when they accurately reflect the wrapped transaction — the intended, +/// common-case usage, as opposed to the deliberately mismatched cache exercised above. +#[test] +fn test_execute_mega_transaction_enriched_succeeds_with_accurate_cache() { + let mut db = MemoryDatabase::default(); + db.set_account_balance(CALLER, U256::from(1_000_000_000_000_000u64)); + + let mut state = State::builder().with_database(&mut db).build(); + let external_envs = TestExternalEnvs::::new(); + let evm_factory = MegaEvmFactory::new().with_external_env_factory(external_envs); + + let mut cfg_env = revm::context::CfgEnv::default(); + cfg_env.spec = MegaSpecId::MINI_REX; + let block_env = BlockEnv { + number: U256::from(1000), + timestamp: U256::from(1_800_000_000), + gas_limit: 30_000_000, + ..Default::default() + }; + let evm_env = EvmEnv::new(cfg_env, block_env); + let evm = evm_factory.create_evm(&mut state, evm_env); + + let block_ctx = + MegaBlockExecutionCtx::new(B256::ZERO, None, Bytes::new(), BlockLimits::no_limits()); + + use alloy_hardforks::ForkCondition; + use mega_evm::MegaHardfork; + let chain_spec = + MegaHardforkConfig::default().with(MegaHardfork::MiniRex, ForkCondition::Timestamp(0)); + let receipt_builder = OpAlloyReceiptBuilder::default(); + let mut executor = MegaBlockExecutor::new(evm, block_ctx, chain_spec, receipt_builder); + + let envelope = create_envelope(0, 1_000_000); + let expected_da_size = MegaTransactionExt::estimated_da_size(&envelope); + let expected_tx_size = MegaTransactionExt::tx_size(&envelope); + let recovered = alloy_consensus::transaction::Recovered::new_unchecked(&envelope, CALLER); + let enriched = EnrichedMegaTx::new_slow(recovered); + + let outcome = executor + .execute_mega_transaction_enriched(enriched) + .expect("accurate cached values must satisfy the debug_assert and succeed normally"); + assert_eq!(outcome.da_size, expected_da_size, "outcome da_size must match the accurate cache"); + assert_eq!(outcome.tx_size, expected_tx_size, "outcome tx_size must match the accurate cache"); +} + #[test] fn test_block_custom_kv_update_limit() { // Create database and deploy contract From fd4e93d31435af3c625e05aa7fc72da92940a90b Mon Sep 17 00:00:00 2001 From: William Aaron Cheung Date: Thu, 2 Jul 2026 09:18:13 +0800 Subject: [PATCH 6/7] refactor(evm): unify enriched tx execution into a single run_transaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fold the opt-in run_transaction_enriched/execute_mega_transaction_enriched entry points back into run_transaction/execute_mega_transaction. The method now bounds `MegaTransactionExt + Encodable2718` and resolves tx_size/da_size through the trait, so an EnrichedMegaTx reuses its cached fields while any other tx falls back to the recomputing default — the choice is made at compile time by trait dispatch, so callers no longer pick a variant. - The alloy_evm ExecutableTx path recomputes the sizes itself and calls the now-public run_transaction_with_sizes, since its tx cannot implement MegaTransactionExt. - Move the debug_assert size cross-check and the trust-boundary contract onto run_transaction (a no-op for bare txs, the safety net for cached overrides). - Add MegaTransactionExt for Recovered<&MegaTxEnvelope>, and Typed2718 / Encodable2718 / MegaTransactionExt for mega-evme's OverriddenTx, so replay callers satisfy the new bound. - Update the enriched tests and benchmark comments accordingly. --- bin/mega-evme/src/common/tx_override.rs | 38 +++++- crates/mega-evm/benches/enriched_tx.rs | 18 +-- crates/mega-evm/src/block/executor.rs | 111 ++++++++---------- crates/mega-evm/src/block/helpers.rs | 17 ++- .../tests/block_executor/block_limits.rs | 61 ++++------ 5 files changed, 132 insertions(+), 113 deletions(-) diff --git a/bin/mega-evme/src/common/tx_override.rs b/bin/mega-evme/src/common/tx_override.rs index 09603172..1ab6d4ad 100644 --- a/bin/mega-evme/src/common/tx_override.rs +++ b/bin/mega-evme/src/common/tx_override.rs @@ -5,11 +5,12 @@ use std::cell::RefCell; -use alloy_primitives::{Address, Bytes, U256}; +use alloy_eips::{Encodable2718, Typed2718}; +use alloy_primitives::{Address, Bytes, TxHash, U256}; use clap::Args; use mega_evm::{ alloy_evm::{IntoTxEnv, RecoveredTx}, - MegaTransaction, + MegaTransaction, MegaTransactionExt, }; use super::{load_hex, parse_ether_value, Result}; @@ -142,3 +143,36 @@ impl + Copy> RecoveredTx for OverriddenTx { self.inner.signer() } } + +// Delegate Typed2718 to inner (required as the `Encodable2718` supertrait below). +impl Typed2718 for OverriddenTx { + fn ty(&self) -> u8 { + self.inner.ty() + } +} + +// Delegate Encodable2718 to inner. Overrides only affect the `TxEnv` produced by `IntoTxEnv`, not +// the EIP-2718 encoding, so the encoded size reflects the original transaction — matching the +// `tx_size`/`da_size` the executor charged before override support existed. +impl Encodable2718 for OverriddenTx { + fn type_flag(&self) -> Option { + self.inner.type_flag() + } + + fn encode_2718_len(&self) -> usize { + self.inner.encode_2718_len() + } + + fn encode_2718(&self, out: &mut dyn alloy_primitives::bytes::BufMut) { + self.inner.encode_2718(out) + } +} + +// Delegate MegaTransactionExt to inner so `OverriddenTx` is accepted by `run_transaction`. +// `tx_size`/`estimated_da_size` fall back to the trait defaults (recomputed from the delegated +// encoding above); only `tx_hash` needs explicit forwarding. +impl MegaTransactionExt for OverriddenTx { + fn tx_hash(&self) -> TxHash { + self.inner.tx_hash() + } +} diff --git a/crates/mega-evm/benches/enriched_tx.rs b/crates/mega-evm/benches/enriched_tx.rs index 36b60917..65eb8954 100644 --- a/crates/mega-evm/benches/enriched_tx.rs +++ b/crates/mega-evm/benches/enriched_tx.rs @@ -4,12 +4,12 @@ //! `EnrichedMegaTx` exists to precompute `tx_size`/`da_size` once (e.g. via `new_slow` from a //! mempool-cached value) so block-execution callers can reuse them instead of recomputing on //! every access. Three rows: -//! - `recompute_via_tx_unwrap` mirrors `MegaBlockExecutor::run_transaction`'s call pattern +//! - `recompute_via_tx_unwrap` mirrors the `alloy_evm` block-execution path's call pattern //! (`tx.tx().estimated_da_size()` / `tx.tx().tx_size()`), which unwraps `EnrichedMegaTx` down to //! the raw inner transaction and always hits the recomputing default impl. -//! - `via_trait_dispatch` mirrors `MegaBlockExecutor::run_transaction_enriched`'s call pattern -//! (`tx.estimated_da_size()` / `tx.tx_size()` on the outer wrapper), which now dispatches to the -//! stored fields. +//! - `via_trait_dispatch` mirrors `MegaBlockExecutor::run_transaction`'s call pattern +//! (`tx.estimated_da_size()` / `tx.tx_size()` on the outer wrapper), which dispatches to the +//! stored fields for an `EnrichedMegaTx`. //! - `cached_fields` reads the wrapper's precomputed fields directly, the floor //! `via_trait_dispatch` should match. @@ -46,9 +46,9 @@ fn enriched_tx(calldata_len: usize) -> EnrichedMegaTx> EnrichedMegaTx::new_slow(recovered) } -/// `MegaBlockExecutor::run_transaction`'s hot-path pattern: `tx.tx().()` unwraps -/// `EnrichedMegaTx` down to the raw inner transaction, so `tx_size`/`estimated_da_size` always -/// hit the recomputing default impl even when the wrapper carries precomputed fields. +/// The `alloy_evm` block-execution path's pattern: `tx.tx().()` unwraps `EnrichedMegaTx` +/// down to the raw inner transaction, so `tx_size`/`estimated_da_size` always hit the recomputing +/// default impl even when the wrapper carries precomputed fields. fn bench_recompute_via_tx_unwrap(c: &mut Criterion) { let mut group = c.benchmark_group("tx_size_da_size/recompute_via_tx_unwrap"); for &len in CALLDATA_SIZES { @@ -63,8 +63,8 @@ fn bench_recompute_via_tx_unwrap(c: &mut Criterion) { group.finish(); } -/// `MegaBlockExecutor::run_transaction_enriched`'s call pattern: `tx.()` called directly -/// on the outer `EnrichedMegaTx`, dispatching to the stored fields via `MegaTransactionExt`. +/// `MegaBlockExecutor::run_transaction`'s call pattern: `tx.()` called directly on the +/// outer `EnrichedMegaTx`, dispatching to the stored fields via `MegaTransactionExt`. fn bench_via_trait_dispatch(c: &mut Criterion) { let mut group = c.benchmark_group("tx_size_da_size/via_trait_dispatch"); for &len in CALLDATA_SIZES { diff --git a/crates/mega-evm/src/block/executor.rs b/crates/mega-evm/src/block/executor.rs index bccb4b14..0901d3b9 100644 --- a/crates/mega-evm/src/block/executor.rs +++ b/crates/mega-evm/src/block/executor.rs @@ -334,19 +334,6 @@ where &mut self, tx: Tx, ) -> Result, BlockExecutionError> - where - Tx: IntoTxEnv + RecoveredTx + Copy, - { - self.run_transaction(tx) - } - - /// Alias to [`MegaBlockExecutor::run_transaction_enriched`]. - /// - /// See that method's doc comment for the caller contract on `tx_size`/`da_size` accuracy. - pub fn execute_mega_transaction_enriched( - &mut self, - tx: Tx, - ) -> Result, BlockExecutionError> where Tx: IntoTxEnv + RecoveredTx @@ -354,20 +341,42 @@ where + Encodable2718 + Copy, { - self.run_transaction_enriched(tx) + self.run_transaction(tx) } /// Execute a transaction with a commit condition function without committing the execution /// result to the block executor's inner state. /// - /// Recomputes `tx_size`/`da_size` from the raw inner transaction on every call. For a `Tx` - /// that carries precomputed values (e.g. [`crate::EnrichedMegaTx`]), prefer - /// [`MegaBlockExecutor::run_transaction_enriched`] to reuse them instead. + /// `tx_size`/`da_size` are resolved from `Tx` through [`MegaTransactionExt`]: a `Tx` that + /// carries precomputed values (e.g. [`crate::EnrichedMegaTx`]) reuses them, while any other + /// `Tx` falls back to the trait's default, which recomputes them from the EIP-2718 encoding. + /// The choice is resolved at compile time by trait dispatch, so callers do not pick a + /// variant — this is the single execution entry point regardless of whether the transaction + /// carries a size cache. /// - /// This method exists (rather than always taking the enriched path) because it also backs - /// [`alloy_evm::block::BlockExecutor::execute_transaction_with_commit_condition`], whose - /// `tx: impl ExecutableTx` parameter is constrained by `alloy_evm` and cannot be - /// required to implement [`MegaTransactionExt`]. + /// The `alloy_evm` block-execution path + /// ([`alloy_evm::block::BlockExecutor::execute_transaction_with_commit_condition`]) does not + /// route through this method: its `tx: impl ExecutableTx` parameter cannot be required + /// to implement [`MegaTransactionExt`], so it recomputes the sizes itself and calls + /// [`MegaBlockExecutor::run_transaction_with_sizes`] directly. + /// + /// # Correctness + /// + /// `tx_size`/`da_size` feed directly into [`BlockLimiter::pre_execution_check`]'s + /// `tx_encode_size_limit`/`tx_da_size_limit`/block cumulative-size checks. When `Tx` + /// overrides the defaults with cached values, those values are trusted with no validation + /// against the real encoded transaction: callers MUST ensure `Tx::tx_size()`/ + /// `Tx::estimated_da_size()` accurately reflect `tx`'s actual EIP-2718 encoding — an + /// understated value lets a transaction bypass a limit it should have been rejected by. + /// This is safe for the sequencer's own block-building path (the cache is computed by the + /// same trusted process, e.g. at mempool insertion), but a `Tx` whose cached sizes could + /// come from an untrusted or stale source (e.g. block validation of another party's block) + /// must not be fed here without first re-establishing that invariant. A `Tx` that uses the + /// recomputing default (e.g. a bare `Recovered<...>`) is unconditionally safe. + /// + /// A `debug_assert` cross-checks the resolved values against a fresh recompute, so a caller + /// bug is caught in tests/CI; it is compiled out in release builds. For the recomputing + /// default it is a no-op; for a cached override it is the real safety net. /// /// # Parameters /// @@ -381,38 +390,6 @@ where &mut self, tx: Tx, ) -> Result, BlockExecutionError> - where - Tx: IntoTxEnv + RecoveredTx + Copy, - { - let tx_size = tx.tx().encode_2718_len() as u64; - let da_size = tx.tx().estimated_da_size(); - self.run_transaction_with_sizes(tx, tx_size, da_size) - } - - /// Like [`MegaBlockExecutor::run_transaction`], but for callers whose `Tx` also implements - /// [`MegaTransactionExt`] (e.g. [`crate::EnrichedMegaTx`]): `tx_size`/`da_size` are read - /// from `Tx` itself instead of being recomputed from the raw inner transaction, so a - /// precomputed value (e.g. one already cached by a mempool) is actually reused. - /// - /// # Correctness - /// - /// `tx_size`/`da_size` feed directly into [`BlockLimiter::pre_execution_check`]'s - /// `tx_encode_size_limit`/`tx_da_size_limit`/block cumulative-size checks, with no - /// validation against the real encoded transaction. Callers MUST ensure `Tx::tx_size()`/ - /// `Tx::estimated_da_size()` accurately reflect `tx`'s actual EIP-2718 encoding — an - /// understated value lets a transaction bypass a limit it should have been rejected by. - /// This is safe for the sequencer's own block-building path (the cache is computed by the - /// same trusted process, e.g. at mempool insertion), but this method must not be fed a - /// `Tx` whose cached sizes could come from an untrusted or stale source (e.g. block - /// validation of another party's block) without first re-establishing that invariant. - /// - /// A `debug_assert` cross-checks the cached values against a fresh recompute, so a mismatch - /// is caught in tests/CI; it is compiled out in release builds, so it does not affect the - /// performance this method exists to deliver. - pub fn run_transaction_enriched( - &mut self, - tx: Tx, - ) -> Result, BlockExecutionError> where Tx: IntoTxEnv + RecoveredTx @@ -425,22 +402,27 @@ where debug_assert_eq!( tx_size, tx.encode_2718_len() as u64, - "run_transaction_enriched: Tx-reported tx_size does not match a fresh recompute \ - from the encoded transaction" + "run_transaction: Tx-reported tx_size does not match a fresh recompute from the \ + encoded transaction" ); debug_assert_eq!( da_size, op_alloy_flz::tx_estimated_size_fjord_bytes(tx.encoded_2718().as_slice()), - "run_transaction_enriched: Tx-reported da_size does not match a fresh recompute \ - from the encoded transaction" + "run_transaction: Tx-reported da_size does not match a fresh recompute from the \ + encoded transaction" ); self.run_transaction_with_sizes(tx, tx_size, da_size) } - /// Shared body of [`MegaBlockExecutor::run_transaction`] and - /// [`MegaBlockExecutor::run_transaction_enriched`]: `tx_size`/`da_size` are resolved by the - /// caller (recomputed or read from a cache), this only consumes them. - fn run_transaction_with_sizes( + /// Shared body of [`MegaBlockExecutor::run_transaction`] and the `alloy_evm` + /// block-execution path: `tx_size`/`da_size` are resolved by the caller (recomputed or read + /// from a cache), this only consumes them. + /// + /// This is the escape hatch for callers whose `Tx` cannot implement [`MegaTransactionExt`] + /// (e.g. the `alloy_evm` `ExecutableTx`-constrained path): they resolve the sizes themselves + /// and pass them in. Prefer [`MegaBlockExecutor::run_transaction`] otherwise, which resolves + /// the sizes for you and cross-checks any cached values. + pub fn run_transaction_with_sizes( &mut self, tx: Tx, tx_size: u64, @@ -661,7 +643,12 @@ where tx: impl ExecutableTx, f: impl FnOnce(&ExecutionResult<::HaltReason>) -> CommitChanges, ) -> Result, BlockExecutionError> { - let outcome = self.run_transaction(tx)?; + // `tx: impl ExecutableTx` cannot be required to implement `MegaTransactionExt`, so + // this path recomputes the sizes from the raw inner transaction and bypasses + // `run_transaction` (which reads them via the trait). See `run_transaction`'s docs. + let tx_size = tx.tx().encode_2718_len() as u64; + let da_size = tx.tx().estimated_da_size(); + let outcome = self.run_transaction_with_sizes(tx, tx_size, da_size)?; if f(&outcome.result).should_commit() { let gas_used = self.commit_execution_outcome(outcome)?; Ok(Some(gas_used)) diff --git a/crates/mega-evm/src/block/helpers.rs b/crates/mega-evm/src/block/helpers.rs index 1813b0d3..d612e351 100644 --- a/crates/mega-evm/src/block/helpers.rs +++ b/crates/mega-evm/src/block/helpers.rs @@ -39,6 +39,12 @@ impl MegaTransactionExt for Recovered { } } +impl MegaTransactionExt for Recovered<&MegaTxEnvelope> { + fn tx_hash(&self) -> TxHash { + self.inner().tx_hash() + } +} + impl MegaTransactionExt for MegaTxEnvelope { fn tx_hash(&self) -> TxHash { self.tx_hash() @@ -70,11 +76,12 @@ impl EnrichedMegaTx { /// Create a new `WithDASize` wrapper with a known data availability size. /// /// `da_size`/`tx_size` are trusted as-is and not validated against `inner`'s actual - /// encoding. When this value reaches [`crate::MegaBlockExecutor::run_transaction_enriched`], - /// it is used directly for `tx_da_size_limit`/`tx_encode_size_limit`/block cumulative-size - /// enforcement, so an inaccurate value can let a transaction bypass a limit it should have - /// been rejected by. Only pass values computed from `inner` itself (e.g. by a trusted - /// mempool at insertion time); prefer [`EnrichedMegaTx::new_slow`] when in doubt. + /// encoding. When this wrapper reaches [`crate::MegaBlockExecutor::run_transaction`], these + /// values are read via [`MegaTransactionExt`] and used directly for + /// `tx_da_size_limit`/`tx_encode_size_limit`/block cumulative-size enforcement, so an + /// inaccurate value can let a transaction bypass a limit it should have been rejected by. + /// Only pass values computed from `inner` itself (e.g. by a trusted mempool at insertion + /// time); prefer [`EnrichedMegaTx::new_slow`] when in doubt. pub fn new(inner: T, tx_hash: TxHash, da_size: u64, tx_size: u64) -> Self { Self { inner, tx_hash, da_size, tx_size } } diff --git a/crates/mega-evm/tests/block_executor/block_limits.rs b/crates/mega-evm/tests/block_executor/block_limits.rs index 4943333f..7b22aef9 100644 --- a/crates/mega-evm/tests/block_executor/block_limits.rs +++ b/crates/mega-evm/tests/block_executor/block_limits.rs @@ -196,29 +196,28 @@ fn test_block_custom_data_limit() { ); } -/// `MegaBlockExecutor::run_transaction_enriched` trusts `Tx`'s reported `tx_size`/`da_size` for -/// the pre-execution limit check instead of recomputing them from the raw transaction (see that -/// method's "Correctness" doc section). Its `debug_assert` cross-checks the reported values -/// against a fresh recompute, so a caller bug — or, as constructed here, a deliberately -/// understated stored `da_size` — is caught immediately in debug/test builds instead of silently -/// letting a transaction bypass a limit it should have been rejected by. +/// `MegaBlockExecutor::run_transaction` resolves `tx_size`/`da_size` through `MegaTransactionExt`, +/// so an `EnrichedMegaTx` feeds its cached values into the pre-execution limit check instead of a +/// fresh recompute (see the method's "Correctness" doc section). Its `debug_assert` cross-checks +/// the resolved values against a fresh recompute, so a caller bug — here a deliberately understated +/// stored `da_size` — is caught immediately in debug/test builds instead of silently letting a +/// transaction bypass a limit it should have been rejected by. /// /// A freshly recomputed DA size for any legacy tx is always >= 100 bytes (the `op_alloy_flz` -/// minimum-size floor), so a `tx_da_size_limit` of 80 rejects every transaction under -/// `run_transaction` — both for a plain `Recovered<&MegaTxEnvelope>` and for an `EnrichedMegaTx` -/// wrapping it, since `run_transaction` always recomputes. An `EnrichedMegaTx` constructed with a -/// deliberately understated stored `da_size` of 50 trips `run_transaction_enriched`'s -/// `debug_assert` rather than silently passing that same limit. +/// minimum-size floor), so a `tx_da_size_limit` of 80 rejects a bare `Recovered<&MegaTxEnvelope>` +/// (which uses the recomputing default). An `EnrichedMegaTx` carrying a deliberately understated +/// stored `da_size` of 50 would pass that same limit using the cached value, but the `debug_assert` +/// trips on the mismatch first. /// /// `#[cfg(debug_assertions)]`: this test exercises the `debug_assert` itself, which is compiled /// out in release builds — under `cargo test --release` the same call would not panic and would /// instead silently succeed using the understated cached value. That is the documented, -/// deliberate trade-off on `run_transaction_enriched` (see its "Correctness" doc section): the -/// cache contract is enforced in debug/test builds only, at zero cost in release. +/// deliberate trade-off on `run_transaction` (see its "Correctness" doc section): the cache +/// contract is enforced in debug/test builds only, at zero cost in release. #[cfg(debug_assertions)] #[test] #[should_panic(expected = "does not match a fresh recompute")] -fn test_run_transaction_enriched_uses_stored_da_size_for_limit_check() { +fn test_run_transaction_rejects_understated_cached_da_size() { let mut db = MemoryDatabase::default(); db.set_account_balance(CALLER, U256::from(1_000_000_000_000_000u64)); @@ -264,37 +263,29 @@ fn test_run_transaction_enriched_uses_stored_da_size_for_limit_check() { let recovered = alloy_consensus::transaction::Recovered::new_unchecked(&envelope, CALLER); - // A plain (non-enriched) transaction recomputes da_size and is rejected. + // A bare (non-enriched) transaction uses the recomputing default and is rejected. let plain_result = executor.run_transaction(&recovered); assert!( plain_result.is_err(), - "plain transaction should be rejected: real da_size exceeds the limit" + "bare transaction should be rejected: real da_size exceeds the limit" ); - // `run_transaction` (unmodified path) recomputes even for an `EnrichedMegaTx`, so the - // understated stored value has no effect and the transaction is still rejected. - // `tx_size` is stored accurately here so the debug_assert below trips on `da_size` alone. + // An `EnrichedMegaTx` with an understated stored `da_size` would pass the limit using the + // cached value, but `run_transaction`'s debug_assert catches the mismatch and panics instead + // of silently letting it through. `tx_size` is stored accurately here so the assert trips on + // `da_size` alone. let real_tx_size = MegaTransactionExt::tx_size(&envelope); let enriched = EnrichedMegaTx::new(recovered, envelope.trie_hash(), STORED_DA_SIZE, real_tx_size); - let unfixed_result = executor.run_transaction(enriched); - assert!( - unfixed_result.is_err(), - "run_transaction must still recompute da_size for EnrichedMegaTx, ignoring the stored field" - ); - - // `run_transaction_enriched` would use the stored (understated) da_size for the limit check — - // which would incorrectly pass a transaction the real recomputation rejects — but its - // debug_assert catches the mismatch and panics instead of silently letting it through. - let _ = executor.run_transaction_enriched(enriched); + let _ = executor.run_transaction(enriched); } -/// `MegaBlockExecutor::execute_mega_transaction_enriched` (the alias for -/// [`MegaBlockExecutor::run_transaction_enriched`]) must succeed and produce an outcome carrying -/// the cached fields when they accurately reflect the wrapped transaction — the intended, -/// common-case usage, as opposed to the deliberately mismatched cache exercised above. +/// `MegaBlockExecutor::execute_mega_transaction` (the alias for +/// [`MegaBlockExecutor::run_transaction`]) must succeed and produce an outcome carrying the cached +/// fields when they accurately reflect the wrapped transaction — the intended, common-case usage, +/// as opposed to the deliberately mismatched cache exercised above. #[test] -fn test_execute_mega_transaction_enriched_succeeds_with_accurate_cache() { +fn test_execute_mega_transaction_succeeds_with_accurate_cache() { let mut db = MemoryDatabase::default(); db.set_account_balance(CALLER, U256::from(1_000_000_000_000_000u64)); @@ -330,7 +321,7 @@ fn test_execute_mega_transaction_enriched_succeeds_with_accurate_cache() { let enriched = EnrichedMegaTx::new_slow(recovered); let outcome = executor - .execute_mega_transaction_enriched(enriched) + .execute_mega_transaction(enriched) .expect("accurate cached values must satisfy the debug_assert and succeed normally"); assert_eq!(outcome.da_size, expected_da_size, "outcome da_size must match the accurate cache"); assert_eq!(outcome.tx_size, expected_tx_size, "outcome tx_size must match the accurate cache"); From 50589a9a5c39598d7263a6cf200cce8960cef287 Mon Sep 17 00:00:00 2001 From: RealiCZ Date: Thu, 2 Jul 2026 10:32:41 +0800 Subject: [PATCH 7/7] test(evm): cover Recovered<&MegaTxEnvelope>::tx_hash William's unify-run_transaction refactor added MegaTransactionExt for Recovered<&MegaTxEnvelope> (the zero-copy shape run_transaction's new MegaTransactionExt bound needs), but no test called it. cargo-mutants diff-scoped gate reported it MISSED (tx_hash collapsing to Default::default() went uncaught) and codecov flagged the same lines as uncovered patch. Add a sibling test mirroring the existing Recovered (owned) coverage. Verified locally: mutation gate now shows 0 missed (was 1), and the previously-uncovered helpers.rs lines are covered. --- crates/mega-evm/tests/mutation/block.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/mega-evm/tests/mutation/block.rs b/crates/mega-evm/tests/mutation/block.rs index 4a03633a..cef5d674 100644 --- a/crates/mega-evm/tests/mutation/block.rs +++ b/crates/mega-evm/tests/mutation/block.rs @@ -7,8 +7,8 @@ //! //! Covered survivors: //! * `helpers.rs` — `MegaTransactionExt::{tx_size, tx_hash, estimated_da_size}` for -//! `MegaTxEnvelope`, `Recovered` and `EnrichedMegaTx` (constant-return / -//! `Default::default()` mutants). +//! `MegaTxEnvelope`, `Recovered`, `Recovered<&MegaTxEnvelope>` and +//! `EnrichedMegaTx` (constant-return / `Default::default()` mutants). //! * `limit.rs` — `BlockLimits::from_hardfork_and_block_gas_limit` per-field initializers (deleting //! a field initializer makes it fall back to `..limits`, i.e. a different value). //! * `hardfork.rs` — `MegaHardforks::is_*_active_at_timestamp` (`-> false` mutants). @@ -82,6 +82,25 @@ fn test_recovered_mega_tx_envelope_ext_returns_real_hash() { assert_eq!(h, STORED_TX_HASH, "recovered tx_hash mismatch"); } +/// `Recovered<&MegaTxEnvelope>::tx_hash` must return the inner envelope hash, not the default. +/// Same shape as the owned `Recovered` case above, but for the by-reference impl +/// `MegaBlockExecutor::run_transaction` needs now that its bound is `MegaTransactionExt` (a +/// zero-copy `Recovered<&Tx>` is the common way a caller satisfies `Copy` without cloning the +/// transaction). +#[test] +fn test_recovered_ref_mega_tx_envelope_ext_returns_real_hash() { + let tx = legacy_envelope(); + let recovered = Recovered::new_unchecked(&tx, CALLER); + + let h = MegaTransactionExt::tx_hash(&recovered); + assert_ne!( + h, + alloy_primitives::TxHash::ZERO, + "recovered-ref tx_hash returned Default::default()" + ); + assert_eq!(h, STORED_TX_HASH, "recovered-ref tx_hash mismatch"); +} + /// `EnrichedMegaTx::{tx_size, estimated_da_size, tx_hash}` must dispatch to the stored fields, not /// recompute from the inner transaction. Constructs the wrapper with stored values that /// deliberately differ from what a fresh recomputation on the inner tx would produce, so a mutant