Skip to content

refactor(revm): keep fee-token logs in the journal instead of finalizing mid-transaction #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
panos-xyz wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor(revm): keep fee-token logs in the journal instead of finalizing mid-transaction #215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 10 additions & 15 deletions bin/morph-statetest/src/runner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ fn execute_case(
.with_inspector(TracerEip3155::buffered(stderr()).without_summary());
evm.enable_inspector();
let exec_result = evm.transact_commit(tx);
let receipt_logs = collect_receipt_logs(&mut evm, &exec_result);
let receipt_logs = result_logs(&exec_result);
return Ok(build_outcome(
name,
fork_name,
Expand All @@ -165,7 +165,7 @@ fn execute_case(

let mut evm = MorphEvm::new(&mut state, env);
let exec_result = evm.transact_commit(tx);
let receipt_logs = collect_receipt_logs(&mut evm, &exec_result);
let receipt_logs = result_logs(&exec_result);
Ok(build_outcome(
name,
fork_name,
Expand Down Expand Up @@ -220,20 +220,15 @@ where
}
}

fn collect_receipt_logs<DB, I, E>(
evm: &mut MorphEvm<DB, I>,
/// A token-fee transaction's deduction and refund Transfers are part of the result's logs,
/// including when the main frame reverted or halted.
fn result_logs<E>(
exec_result: &Result<ExecutionResult<morph_revm::MorphHaltReason>, E>,
) -> Vec<Log>
where
DB: alloy_evm::Database,
I: revm::Inspector<morph_revm::evm::MorphContext<DB>>,
{
let mut logs = evm.take_pre_fee_logs();
if let Ok(result) = exec_result {
logs.extend(result.logs().iter().cloned());
}
logs.extend(evm.take_post_fee_logs());
logs
) -> Vec<Log> {
exec_result
.as_ref()
.map(|result| result.logs().to_vec())
.unwrap_or_default()
}

fn validation_error<E>(
Expand Down
14 changes: 1 addition & 13 deletions crates/evm/src/block/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use alloy_evm::{
BlockExecutionError, BlockExecutionResult, BlockExecutor, ExecutableTx, GasOutput, TxResult,
},
};
use alloy_primitives::{Address, Log, U256};
use alloy_primitives::{Address, U256};
use morph_primitives::{MorphReceipt, MorphTxEnvelope};
use morph_revm::{L1_GAS_PRICE_ORACLE_ADDRESS, MorphHaltReason, TokenFeeInfo, evm::MorphContext};
use reth_primitives_traits::Recovered;
Expand All @@ -40,10 +40,6 @@ pub struct MorphTxResult {
pub recovered: Recovered<MorphTxEnvelope>,
/// L1 data fee read from the handler cache immediately after execution.
pub l1_fee: U256,
/// Token-fee deduction Transfer logs (survive main-tx revert).
pub pre_fee_logs: Vec<Log>,
/// Token-fee reimbursement Transfer logs (survive main-tx revert).
pub post_fee_logs: Vec<Log>,
}

impl TxResult for MorphTxResult {
Expand Down Expand Up @@ -242,15 +238,11 @@ where

// Read caches from the EVM immediately after execution, before the next tx resets them.
let l1_fee = self.evm.cached_l1_data_fee();
let pre_fee_logs = self.evm.take_pre_fee_logs();
let post_fee_logs = self.evm.take_post_fee_logs();

Ok(MorphTxResult {
result,
recovered: Recovered::new_unchecked(consensus_tx, signer),
l1_fee,
pre_fee_logs,
post_fee_logs,
})
}

Expand All @@ -259,8 +251,6 @@ where
result: ResultAndState { result, state },
recovered,
l1_fee,
pre_fee_logs,
post_fee_logs,
} = output;

// EIP-8037 separates regular and state gas; pre-Amsterdam morph treats
Expand Down Expand Up @@ -300,8 +290,6 @@ where
cumulative_gas_used: self.gas_used,
l1_fee,
morph_tx_fields,
pre_fee_logs,
post_fee_logs,
};
self.receipts.push(self.receipt_builder.build_receipt(ctx));

Expand Down
152 changes: 29 additions & 123 deletions crates/evm/src/block/receipt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
use alloy_consensus::Receipt;
use alloy_consensus::transaction::TxHashRef;
use alloy_evm::Evm;
use alloy_primitives::{B256, Bytes, Log, U256};
use alloy_primitives::{B256, Bytes, U256};
use morph_primitives::{MorphReceipt, MorphTransactionReceipt, MorphTxEnvelope, MorphTxType};
use revm::context::result::ExecutionResult;
use tracing::warn;
Expand All @@ -45,8 +45,6 @@ use tracing::warn;
/// - `cumulative_gas_used`: Running total of gas used in the block
/// - `l1_fee`: Pre-calculated L1 data fee for this transaction
/// - `morph_tx_fields`: MorphTx-specific fields (token fee info, version, reference, memo)
/// - `pre_fee_logs`: Transfer event logs from token fee deduction (survives tx revert)
/// - `post_fee_logs`: Transfer event logs from token fee reimbursement
#[derive(Debug)]
pub(crate) struct MorphReceiptBuilderCtx<'a, E: Evm> {
/// The executed transaction
Expand All @@ -59,11 +57,6 @@ pub(crate) struct MorphReceiptBuilderCtx<'a, E: Evm> {
pub l1_fee: U256,
/// MorphTx-specific fields (token fee info, version, reference, memo)
pub morph_tx_fields: Option<MorphReceiptTxFields>,
/// Transfer event logs from token fee deduction (before main tx execution).
/// Managed separately from the handler pipeline to survive main tx revert.
pub pre_fee_logs: Vec<Log>,
/// Transfer event logs from token fee reimbursement (after main tx execution).
pub post_fee_logs: Vec<Log>,
}

/// MorphTx (0x7F) specific fields for receipts.
Expand Down Expand Up @@ -155,25 +148,16 @@ impl MorphReceiptBuilder for DefaultMorphReceiptBuilder {
cumulative_gas_used,
l1_fee,
morph_tx_fields,
pre_fee_logs,
post_fee_logs,
} = ctx;

// Assemble logs in chronological order matching go-ethereum:
// [deduct Transfer] + [main tx logs] + [refund Transfer]
// The fee logs cannot come from `result`. The call-mode deduction runs a
// mid-transaction `finalize()` that clears the journal's logs, so the handler
// moves them out first, and it drains the refund's logs the same way. `result`
// carries only the main frame's logs, which a revert has already discarded,
// while the fee logs survive it as they do in go-ethereum, whose `StateDB.logs`
// sit outside the snapshot/revert mechanism.
// For a token-fee MorphTx `result` already holds the logs in go-ethereum's order:
// [deduct Transfer] + [main tx logs, on success] + [refund Transfer]
// The handler leaves both fee transfers' logs in the journal and the main frame
// reverts only back to its own checkpoint, so they survive a failed main frame,
// as they do in go-ethereum, whose `StateDB.logs` sit outside the snapshot/revert
// mechanism. revm returns the journal's logs for successes, reverts and halts alike.
let is_success = result.is_success();
let main_logs = result.into_logs();
let mut logs =
Vec::with_capacity(pre_fee_logs.len() + main_logs.len() + post_fee_logs.len());
logs.extend(pre_fee_logs);
logs.extend(main_logs);
logs.extend(post_fee_logs);
let logs = result.into_logs();

let inner = Receipt {
status: is_success.into(),
Expand Down Expand Up @@ -361,8 +345,6 @@ mod tests {
cumulative_gas_used: 21000,
l1_fee,
morph_tx_fields: None,
pre_fee_logs: vec![],
post_fee_logs: vec![],
};

let receipt = builder.build_receipt(ctx);
Expand All @@ -384,8 +366,6 @@ mod tests {
cumulative_gas_used: 42000,
l1_fee,
morph_tx_fields: None,
pre_fee_logs: vec![],
post_fee_logs: vec![],
};

let receipt = builder.build_receipt(ctx);
Expand All @@ -407,8 +387,6 @@ mod tests {
// L1 message gas is prepaid on L1, so no L1 fee should appear in the receipt.
l1_fee: U256::from(999_999),
morph_tx_fields: None,
pre_fee_logs: vec![],
post_fee_logs: vec![],
};

let receipt = builder.build_receipt(ctx);
Expand Down Expand Up @@ -439,8 +417,6 @@ mod tests {
cumulative_gas_used: 21000,
l1_fee,
morph_tx_fields: Some(fields),
pre_fee_logs: vec![],
post_fee_logs: vec![],
};

let receipt = builder.build_receipt(ctx);
Expand Down Expand Up @@ -475,8 +451,6 @@ mod tests {
cumulative_gas_used: 21000,
l1_fee,
morph_tx_fields: None,
pre_fee_logs: vec![],
post_fee_logs: vec![],
};

let receipt = builder.build_receipt(ctx);
Expand Down Expand Up @@ -508,8 +482,6 @@ mod tests {
cumulative_gas_used: 15000,
l1_fee: U256::from(100u64),
morph_tx_fields: None,
pre_fee_logs: vec![],
post_fee_logs: vec![],
};

let receipt = builder.build_receipt(ctx);
Expand All @@ -536,8 +508,6 @@ mod tests {
cumulative_gas_used: 21000,
l1_fee: U256::ZERO,
morph_tx_fields: None,
pre_fee_logs: vec![],
post_fee_logs: vec![],
};

let receipt = builder.build_receipt(ctx);
Expand All @@ -553,28 +523,28 @@ mod tests {
.unwrap()
}

/// Fee Transfer logs (pre/post) survive when the main transaction reverts.
///
/// go-ethereum's StateDB.logs is independent of snapshot/revert — fee logs
/// are always included. revm's ExecutionResult::Revert carries no logs field,
/// so morph-reth caches fee logs in pre_fee_logs/post_fee_logs and merges
/// them unconditionally in the receipt builder.
/// A reverted or halted `ExecutionResult` still carries the logs emitted before the
/// main frame failed, which is where the handler leaves a token-fee transaction's
/// deduction and refund Transfers (go-ethereum keeps them in `StateDB.logs`, outside
/// the snapshot/revert mechanism). The receipt must keep them, in order.
#[test]
fn test_fee_logs_survive_main_tx_revert() {
fn test_reverted_receipt_keeps_fee_logs() {
let builder = DefaultMorphReceiptBuilder;
let tx = create_legacy_tx();
let tx = create_morph_tx();

let pre_log = make_fee_log(0xAA); // fee deduction Transfer
let post_log = make_fee_log(0xBB); // fee refund Transfer
let deduct_log = make_fee_log(0xAA);
let refund_log = make_fee_log(0xBB);

let ctx = MorphReceiptBuilderCtx::<TestEvm> {
tx: &tx,
result: make_revert_result(20_000),
result: ExecutionResult::Revert {
gas: result_gas(20_000),
logs: vec![deduct_log.clone(), refund_log.clone()],
output: alloy_primitives::Bytes::new(),
},
cumulative_gas_used: 20_000,
l1_fee: U256::ZERO,
morph_tx_fields: None,
pre_fee_logs: vec![pre_log.clone()],
post_fee_logs: vec![post_log.clone()],
};

let receipt = builder.build_receipt(ctx);
Expand All @@ -583,92 +553,28 @@ mod tests {
!TxReceipt::status(&receipt),
"reverted tx must have status=false"
);

let logs = TxReceipt::logs(&receipt);
// Main tx logs are absent (revert), but fee logs must still be present.
assert_eq!(
logs.len(),
2,
"pre_fee_log + post_fee_log must appear despite revert"
);
assert_eq!(
logs[0].address, pre_log.address,
"first log must be pre_fee_log"
);
assert_eq!(
logs[1].address, post_log.address,
"second log must be post_fee_log"
);
assert_eq!(TxReceipt::logs(&receipt), &[deduct_log, refund_log]);
}

/// Log ordering on successful tx: [pre_fee_log, main_tx_log, post_fee_log].
///
/// Matches go-ethereum's receipt log ordering where fee deduction comes
/// first (before main tx), and fee refund comes last (after main tx).
/// On success the result's logs, `[deduct] + [main] + [refund]`, reach the receipt
/// unchanged.
#[test]
fn test_fee_log_ordering_on_success() {
fn test_successful_receipt_keeps_log_order() {
let builder = DefaultMorphReceiptBuilder;
let tx = create_legacy_tx();
let tx = create_morph_tx();

let pre_log = make_fee_log(0xAA);
let main_log = make_fee_log(0xCC);
let post_log = make_fee_log(0xBB);
let logs = vec![make_fee_log(0xAA), make_fee_log(0xCC), make_fee_log(0xBB)];

let ctx = MorphReceiptBuilderCtx::<TestEvm> {
tx: &tx,
result: make_success_with_logs(21_000, vec![main_log.clone()]),
result: make_success_with_logs(21_000, logs.clone()),
cumulative_gas_used: 21_000,
l1_fee: U256::ZERO,
morph_tx_fields: None,
pre_fee_logs: vec![pre_log.clone()],
post_fee_logs: vec![post_log.clone()],
};

let receipt = builder.build_receipt(ctx);
assert!(TxReceipt::status(&receipt));

let logs = TxReceipt::logs(&receipt);
assert_eq!(logs.len(), 3, "pre_fee + main + post_fee = 3 logs");
assert_eq!(
logs[0].address, pre_log.address,
"pre_fee_log must be first"
);
assert_eq!(
logs[1].address, main_log.address,
"main_tx_log must be second"
);
assert_eq!(
logs[2].address, post_log.address,
"post_fee_log must be last"
);
}

/// Fee logs without refund: only pre_fee_log when no gas is refunded.
///
/// If all gas is consumed exactly (no unused gas), the post_fee_log
/// may be empty. But the pre_fee_log must always appear.
#[test]
fn test_pre_fee_log_only_no_post_fee() {
let builder = DefaultMorphReceiptBuilder;
let tx = create_legacy_tx();

let pre_log = make_fee_log(0xAA);

let ctx = MorphReceiptBuilderCtx::<TestEvm> {
tx: &tx,
result: make_revert_result(21_000),
cumulative_gas_used: 21_000,
l1_fee: U256::ZERO,
morph_tx_fields: None,
pre_fee_logs: vec![pre_log.clone()],
post_fee_logs: vec![], // no refund
};

let receipt = builder.build_receipt(ctx);
assert!(!TxReceipt::status(&receipt));

let logs = TxReceipt::logs(&receipt);
assert_eq!(logs.len(), 1, "only pre_fee_log when there is no refund");
assert_eq!(logs[0].address, pre_log.address);
assert_eq!(TxReceipt::logs(&receipt), logs.as_slice());
}
}
12 changes: 0 additions & 12 deletions crates/evm/src/evm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,18 +113,6 @@ impl<DB: Database, I> MorphEvm<DB, I> {
pub fn cached_l1_data_fee(&self) -> alloy_primitives::U256 {
self.inner.cached_l1_data_fee()
}

/// Takes the cached pre-execution fee logs (token fee deduction Transfer events).
#[inline]
pub fn take_pre_fee_logs(&mut self) -> Vec<alloy_primitives::Log> {
self.inner.take_pre_fee_logs()
}

/// Takes the cached post-execution fee logs (token fee reimbursement Transfer events).
#[inline]
pub fn take_post_fee_logs(&mut self) -> Vec<alloy_primitives::Log> {
self.inner.take_post_fee_logs()
}
}

impl<DB: Database, I> Deref for MorphEvm<DB, I>
Expand Down
Loading
Loading