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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions crates/context/interface/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,15 @@ pub trait Host {
})
})
}

/// Records a gas refund for the current transaction.
///
/// This is called by SSTORE instruction to record gas refunds at the journal level,
/// enabling proper reverting in case of revert/oog/stackoverflow.
fn record_refund(&mut self, refund: i64);

/// Returns the current accumulated gas refund for the transaction.
fn refund(&self) -> i64;
}

/// Dummy host that implements [`Host`] trait and returns all default values.
Expand Down Expand Up @@ -304,4 +313,10 @@ impl Host for DummyHost {
) -> Result<StateLoad<StorageValue>, LoadError> {
Err(LoadError::DBError)
}

fn record_refund(&mut self, _refund: i64) {}

fn refund(&self) -> i64 {
0
}
}
13 changes: 13 additions & 0 deletions crates/context/interface/src/journaled_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,19 @@ pub trait JournalTr {
_load_code: bool,
_skip_cold_load: bool,
) -> Result<AccountInfoLoad<'_>, JournalLoadError<<Self::Database as Database>::Error>>;

/// Records a gas refund for the current transaction.
///
/// This accumulates refunds at the global journal level, enabling proper
/// reverting in case of revert/oog/stackoverflow. A journal entry is created
/// to track the change.
///
/// The `refund` value can be negative (e.g., for SSTORE that increases storage)
/// but the total accumulated refund should be non-negative at transaction end.
fn record_refund(&mut self, refund: i64);

/// Returns the current accumulated gas refund for the transaction.
fn refund(&self) -> i64;
}

/// Error that can happen when loading account info.
Expand Down
34 changes: 34 additions & 0 deletions crates/context/interface/src/journaled_state/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ pub trait JournalEntryTr {
/// Creates a journal entry for when an account's code is modified
fn code_changed(address: Address) -> Self;

/// Creates a journal entry for when the gas refund changes.
/// Records the previous refund value for reverting.
fn refund_changed(old_refund: i64) -> Self;

/// Returns the old refund value if this is a RefundChange entry.
/// Used during checkpoint revert to restore the previous refund value.
fn get_refund_revert(&self) -> Option<i64>;

/// Reverts the state change recorded by this journal entry
///
/// More information on what is reverted can be found in [`JournalEntry`] enum.
Expand Down Expand Up @@ -226,6 +234,16 @@ pub enum JournalEntry {
/// Address of account that had its code changed.
address: Address,
},
/// Gas refund changed
/// Action: Gas refund value changed
/// Revert: Revert to previous refund value
///
/// This tracks gas refund changes at the journal level to enable proper
/// reverting in case of revert/oog/stackoverflow. See issue #2145.
RefundChange {
/// Previous refund value before the change.
old_refund: i64,
},
}
impl JournalEntryTr for JournalEntry {
fn account_warmed(address: Address) -> Self {
Expand Down Expand Up @@ -307,6 +325,17 @@ impl JournalEntryTr for JournalEntry {
JournalEntry::CodeChange { address }
}

fn refund_changed(old_refund: i64) -> Self {
JournalEntry::RefundChange { old_refund }
}

fn get_refund_revert(&self) -> Option<i64> {
match self {
JournalEntry::RefundChange { old_refund } => Some(*old_refund),
_ => None,
}
}

fn revert(
self,
state: &mut EvmState,
Expand Down Expand Up @@ -432,6 +461,11 @@ impl JournalEntryTr for JournalEntry {
acc.info.code_hash = KECCAK_EMPTY;
acc.info.code = None;
}
JournalEntry::RefundChange { .. } => {
// Refund revert is handled separately in checkpoint_revert
// because it needs access to the refund field in JournalInner,
// which is not part of EvmState.
}
}
}
}
10 changes: 10 additions & 0 deletions crates/context/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,4 +606,14 @@ impl<
}
}
}

#[inline]
fn record_refund(&mut self, refund: i64) {
self.journal_mut().record_refund(refund);
}

#[inline]
fn refund(&self) -> i64 {
self.journal().refund()
}
}
10 changes: 10 additions & 0 deletions crates/context/src/journal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,4 +377,14 @@ impl<DB: Database, ENTRY: JournalEntryTr> JournalTr for Journal<DB, ENTRY> {
AccountInfoLoad::new(&a.data.info, a.is_cold, a.state_clear_aware_is_empty(spec))
})
}

#[inline]
fn record_refund(&mut self, refund: i64) {
self.inner.record_refund(refund)
}

#[inline]
fn refund(&self) -> i64 {
self.inner.refund()
}
}
40 changes: 40 additions & 0 deletions crates/context/src/journal/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ pub struct JournalInner<ENTRY> {
pub spec: SpecId,
/// Warm addresses containing both coinbase and current precompiles.
pub warm_addresses: WarmAddresses,
/// Accumulated gas refund for the current transaction.
///
/// This is tracked at the journal level to enable proper reverting in case of
/// revert/oog/stackoverflow. The refund value can be negative during execution
/// but should be non-negative at the end of a successful transaction.
///
/// See [EIP-3529](https://eips.ethereum.org/EIPS/eip-3529) for refund limits.
pub refund: i64,
}

impl<ENTRY: JournalEntryTr> Default for JournalInner<ENTRY> {
Expand All @@ -81,6 +89,7 @@ impl<ENTRY: JournalEntryTr> JournalInner<ENTRY> {
depth: 0,
spec: SpecId::default(),
warm_addresses: WarmAddresses::new(),
refund: 0,
}
}

Expand Down Expand Up @@ -109,6 +118,7 @@ impl<ENTRY: JournalEntryTr> JournalInner<ENTRY> {
transaction_id,
spec,
warm_addresses,
refund,
} = self;
// Spec precompiles and state are not changed. It is always set again execution.
let _ = spec;
Expand All @@ -124,6 +134,8 @@ impl<ENTRY: JournalEntryTr> JournalInner<ENTRY> {
// increment transaction id.
*transaction_id += 1;
logs.clear();
// Reset refund for next transaction
*refund = 0;
}

/// Discard the current transaction, by reverting the journal entries and incrementing the transaction id.
Expand All @@ -138,6 +150,7 @@ impl<ENTRY: JournalEntryTr> JournalInner<ENTRY> {
transaction_id,
spec,
warm_addresses,
refund,
} = self;
let is_spurious_dragon_enabled = spec.is_enabled_in(SPURIOUS_DRAGON);
// iterate over all journals entries and revert our global state
Expand All @@ -148,6 +161,8 @@ impl<ENTRY: JournalEntryTr> JournalInner<ENTRY> {
*depth = 0;
logs.clear();
*transaction_id += 1;
// Reset refund for next transaction
*refund = 0;

// Clear coinbase address warming for next tx
warm_addresses.clear_coinbase_and_access_list();
Expand All @@ -170,6 +185,7 @@ impl<ENTRY: JournalEntryTr> JournalInner<ENTRY> {
transaction_id,
spec,
warm_addresses,
refund,
} = self;
// Spec is not changed. And it is always set again in execution.
let _ = spec;
Expand All @@ -185,6 +201,8 @@ impl<ENTRY: JournalEntryTr> JournalInner<ENTRY> {
*depth = 0;
// reset transaction id.
*transaction_id = 0;
// reset refund
*refund = 0;

state
}
Expand Down Expand Up @@ -476,6 +494,10 @@ impl<ENTRY: JournalEntryTr> JournalInner<ENTRY> {
.drain(checkpoint.journal_i..)
.rev()
.for_each(|entry| {
// Handle RefundChange specially - restore the old refund value
if let Some(old_refund) = entry.get_refund_revert() {
self.refund = old_refund;
}
entry.revert(state, Some(transient_storage), is_spurious_dragon_enabled);
});
}
Expand Down Expand Up @@ -918,6 +940,24 @@ impl<ENTRY: JournalEntryTr> JournalInner<ENTRY> {
pub fn log(&mut self, log: Log) {
self.logs.push(log);
}

/// Records a gas refund for the current transaction.
///
/// This accumulates refunds at the global journal level, avoiding negative
/// refund values being visible during individual call frame execution.
/// A journal entry is created to enable reverting in case of revert/oog/stackoverflow.
#[inline]
pub fn record_refund(&mut self, refund: i64) {
// Create journal entry with the old refund value before changing it
self.journal.push(ENTRY::refund_changed(self.refund));
self.refund += refund;
}

/// Returns the current accumulated gas refund for the transaction.
#[inline]
pub fn refund(&self) -> i64 {
self.refund
}
}

#[cfg(test)]
Expand Down
10 changes: 6 additions & 4 deletions crates/handler/src/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,9 +499,9 @@ impl EthFrame<EthInterpreter> {
.set(mem_start, &interpreter.return_data.buffer()[..target_len]);
}

if ins_result.is_ok() {
interpreter.gas.record_refund(out_gas.refunded());
}
// Note: refund is now tracked at the journal level, so we don't need to
// propagate it from child to parent frame here. The journal handles
// refund accumulation and revert automatically.
}
FrameResult::Create(outcome) => {
let instruction_result = *outcome.instruction_result();
Expand All @@ -528,8 +528,10 @@ impl EthFrame<EthInterpreter> {
this_gas.erase_cost(outcome.gas().remaining());
}

// Note: refund is now tracked at the journal level, so we don't need to
// propagate it from child to parent frame here. The journal handles
// refund accumulation and revert automatically.
let stack_item = if instruction_result.is_ok() {
this_gas.record_refund(outcome.gas().refunded());
outcome.address.unwrap_or_default().into_word().into()
} else {
U256::ZERO
Expand Down
12 changes: 6 additions & 6 deletions crates/handler/src/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,6 @@ pub trait Handler {
let instruction_result = frame_result.interpreter_result().result;
let gas = frame_result.gas_mut();
let remaining = gas.remaining();
let refunded = gas.refunded();

// Spend the gas limit. Gas is reimbursed when the tx returns successfully.
*gas = Gas::new_spent(evm.ctx().tx().gas_limit());
Expand All @@ -363,9 +362,8 @@ pub trait Handler {
gas.erase_cost(remaining);
}

if instruction_result.is_ok() {
gas.record_refund(refunded);
}
// Note: refund is now tracked at the journal level and will be applied
// in the refund() handler function. No need to propagate frame refund here.
Ok(())
}

Expand Down Expand Up @@ -428,15 +426,17 @@ pub trait Handler {
}

/// Calculates the final gas refund amount, including any EIP-7702 refunds.
///
/// This function retrieves the accumulated refund from the journal (SSTORE/SELFDESTRUCT)
/// and combines it with the EIP-7702 refund to calculate the final refund amount.
#[inline]
fn refund(
&self,
evm: &mut Self::Evm,
exec_result: &mut <<Self::Evm as EvmTr>::Frame as FrameTr>::FrameResult,
eip7702_refund: i64,
) {
let spec = evm.ctx().cfg().spec().into();
post_execution::refund(spec, exec_result.gas_mut(), eip7702_refund)
post_execution::refund(evm.ctx(), exec_result.gas_mut(), eip7702_refund)
}

/// Returns unused gas costs to the transaction sender's account.
Expand Down
15 changes: 13 additions & 2 deletions crates/handler/src/post_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,19 @@ pub fn eip7623_check_gas_floor(gas: &mut Gas, init_and_floor_gas: InitialAndFloo
}

/// Calculates and applies gas refunds based on the specification.
pub fn refund(spec: SpecId, gas: &mut Gas, eip7702_refund: i64) {
gas.record_refund(eip7702_refund);
///
/// This function takes the accumulated refund from the journal and applies it to the Gas struct.
/// The journal refund includes SSTORE and SELFDESTRUCT refunds that were recorded during execution.
/// The eip7702_refund is added on top of the journal refund.
pub fn refund<CTX: ContextTr>(context: &mut CTX, gas: &mut Gas, eip7702_refund: i64) {
let spec: SpecId = context.cfg().spec().into();

// Get the accumulated refund from the journal (SSTORE/SELFDESTRUCT refunds)
let journal_refund = context.journal().refund();

// Record total refund (journal + eip7702) to the Gas struct
gas.record_refund(journal_refund + eip7702_refund);

// Calculate gas refund for transaction.
// If spec is set to london, it will decrease the maximum refund amount to 5th part of
// gas spend. (Before london it was 2th part of gas spend)
Expand Down
3 changes: 2 additions & 1 deletion crates/handler/src/precompile_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ impl<CTX: ContextTr> PrecompileProvider<CTX> for EthPrecompiles {

match exec_result {
Ok(output) => {
result.gas.record_refund(output.gas_refunded);
// Record refund at journal level for proper revert support
context.journal_mut().record_refund(output.gas_refunded);
let underflow = result.gas.record_cost(output.gas_used);
assert!(underflow, "Gas underflow is not possible");
result.result = if output.reverted {
Expand Down
8 changes: 5 additions & 3 deletions crates/inspector/src/eip3155.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ where
self.gas_inspector.initialize_interp(&interp.gas);
}

fn step(&mut self, interp: &mut Interpreter<INTR>, _: &mut CTX) {
fn step(&mut self, interp: &mut Interpreter<INTR>, context: &mut CTX) {
self.gas_inspector.step(&interp.gas);
self.stack.clear();
interp.stack.clone_into(&mut self.stack);
Expand All @@ -239,7 +239,9 @@ where
self.opcode = interp.bytecode.opcode();
self.mem_size = interp.memory.size();
self.gas = interp.gas.remaining();
self.refunded = interp.gas.refunded();
// Use journal-level refund which properly tracks global refund state
// and avoids negative values during individual call execution.
self.refunded = context.journal().refund();
}

fn step_end(&mut self, interp: &mut Interpreter<INTR>, context: &mut CTX) {
Expand All @@ -252,7 +254,7 @@ where
stack: &self.stack,
depth: context.journal_mut().depth() as u64,
return_data: "0x",
refund: self.refunded as u64,
refund: self.refunded.max(0) as u64,
mem_size: self.mem_size as u64,

op_name: OpCode::new(self.opcode).map(|i| i.as_str()),
Expand Down
Loading
Loading