diff --git a/substrate/frame/revive/fixtures/contracts/Memory.sol b/substrate/frame/revive/fixtures/contracts/Memory.sol index dec39b12a1ce8..5e3b80cecb71b 100644 --- a/substrate/frame/revive/fixtures/contracts/Memory.sol +++ b/substrate/frame/revive/fixtures/contracts/Memory.sol @@ -2,6 +2,20 @@ pragma solidity ^0.8.24; contract Memory { + /// @notice Expands memory to the specified size by writing a byte at that offset + /// @param memorySize The memory size in bytes to expand to + function expandMemory(uint256 memorySize) public pure returns (bool success) { + // Allocate memory by accessing a byte at the specified offset + // This will trigger memory expansion up to at least memorySize + 1 + assembly { + // Store a single byte (0xFF) at the memory offset + // This forces the EVM to expand memory to accommodate this write + mstore8(memorySize, 0xFF) + } + + return false; + } + function testMemory() public { uint256 value = 0xfe; assembly { diff --git a/substrate/frame/revive/src/tests/sol/memory.rs b/substrate/frame/revive/src/tests/sol/memory.rs index 2103c5159f9f5..bb6255864c6a0 100644 --- a/substrate/frame/revive/src/tests/sol/memory.rs +++ b/substrate/frame/revive/src/tests/sol/memory.rs @@ -19,15 +19,51 @@ use crate::{ evm::decode_revert_reason, test_utils::{builder::Contract, ALICE}, tests::{builder, ExtBuilder, Test}, - Code, Config, + Code, Config, Error, ExecReturnValue, LOG_TARGET, }; -use alloy_core::{primitives::U256, sol_types::SolInterface}; +use alloy_core::{ + primitives::U256, + sol_types::{SolCall, SolInterface}, +}; use frame_support::traits::fungible::Mutate; use pallet_revive_fixtures::{compile_module_with_type, FixtureType, Memory}; use pallet_revive_uapi::ReturnFlags; use pretty_assertions::assert_eq; +#[test] +fn memory_limit_works() { + let (code, _) = compile_module_with_type("Memory", FixtureType::Solc).unwrap(); + + ExtBuilder::default().build().execute_with(|| { + ::Currency::set_balance(&ALICE, 100_000_000_000); + let Contract { addr, .. } = + builder::bare_instantiate(Code::Upload(code)).build_and_unwrap_contract(); + + let test_cases = [ + ( + // Writing 1 byte from 0 to the limit - 1 should work. + Memory::expandMemoryCall { + memorySize: U256::from(crate::limits::code::BASELINE_MEMORY_LIMIT - 1), + }, + Ok(ExecReturnValue { data: vec![0u8; 32], flags: ReturnFlags::empty() }), + ), + ( + // Writing 1 byte from the limit should revert. + Memory::expandMemoryCall { + memorySize: U256::from(crate::limits::code::BASELINE_MEMORY_LIMIT), + }, + Err(>::OutOfGas.into()), + ), + ]; + + for (data, expected_result) in test_cases { + let result = builder::bare_call(addr).data(data.abi_encode()).build().result; + assert_eq!(result, expected_result); + } + }); +} + #[test] fn memory_works() { for fixture_type in [FixtureType::Solc, FixtureType::Resolc] { @@ -43,9 +79,9 @@ fn memory_works() { .build_and_unwrap_result(); if result.flags == ReturnFlags::REVERT { if let Some(revert_msg) = decode_revert_reason(&result.data) { - log::error!("Revert message: {}", revert_msg); + log::error!(target: LOG_TARGET, "Revert message: {}", revert_msg); } else { - log::error!("Revert without message, raw data: {:?}", result.data); + log::error!(target: LOG_TARGET, "Revert without message, raw data: {:?}", result.data); } } assert!(!result.did_revert(), "test reverted"); diff --git a/substrate/frame/revive/src/vm/evm.rs b/substrate/frame/revive/src/vm/evm.rs index 652d65785a244..8efa4f70cd03c 100644 --- a/substrate/frame/revive/src/vm/evm.rs +++ b/substrate/frame/revive/src/vm/evm.rs @@ -158,10 +158,8 @@ fn run<'a, E: Ext>( InterpreterAction::Return(result) => { log::trace!(target: LOG_TARGET, "Evm return {:?}", result); debug_assert!( - result.gas.limit() == 0 && - result.gas.remaining() == 0 && - result.gas.refunded() == 0, - "Interpreter gas state should remain unchanged; found: {:?}", + result.gas == Default::default(), + "Interpreter gas state is unused; found: {:?}", result.gas, ); return result; diff --git a/substrate/frame/revive/src/vm/evm/instructions/macros.rs b/substrate/frame/revive/src/vm/evm/instructions/macros.rs index 89513ba1646be..c3d201f8be93d 100644 --- a/substrate/frame/revive/src/vm/evm/instructions/macros.rs +++ b/substrate/frame/revive/src/vm/evm/instructions/macros.rs @@ -122,28 +122,6 @@ macro_rules! gas_or_fail_legacy { }; } -use crate::vm::Ext; -use revm::interpreter::gas::{MemoryExtensionResult, MemoryGas}; - -/// Adapted from -/// https://docs.rs/revm/latest/revm/interpreter/struct.Gas.html#method.record_memory_expansion -pub fn record_memory_expansion( - memory: &mut MemoryGas, - _ext: &mut E, - new_len: usize, -) -> MemoryExtensionResult { - let Some(_additional_cost) = memory.record_new_len(new_len) else { - return MemoryExtensionResult::Same; - }; - - // TODO: Commented this for now so that extending memory will not cost any gas. - // if ext.gas_meter_mut().charge_evm_gas(additional_cost).is_err() { - // return MemoryExtensionResult::OutOfGas; - // } - - MemoryExtensionResult::Extended -} - /// Resizes the interpreterreter memory if necessary. Fails the instruction if the memory or gas /// limit is exceeded. #[macro_export] @@ -152,20 +130,16 @@ macro_rules! resize_memory { resize_memory!($interpreter, $offset, $len, ()) }; ($interpreter:expr, $offset:expr, $len:expr, $ret:expr) => { - let words_num = revm::interpreter::num_words($offset.saturating_add($len)); - match crate::vm::evm::instructions::macros::record_memory_expansion( - $interpreter.gas.memory_mut(), - $interpreter.extend, - words_num, - ) { - revm::interpreter::gas::MemoryExtensionResult::Extended => { - $interpreter.memory.resize(words_num * 32); - }, - revm::interpreter::gas::MemoryExtensionResult::OutOfGas => { - $interpreter.halt(revm::interpreter::InstructionResult::MemoryOOG); - return $ret; - }, - revm::interpreter::gas::MemoryExtensionResult::Same => (), // no action + let current_len = $interpreter.memory.len(); + let target_len = revm::interpreter::num_words($offset.saturating_add($len)) * 32; + if target_len as u32 > $crate::limits::code::BASELINE_MEMORY_LIMIT { + log::debug!(target: $crate::LOG_TARGET, "check memory bounds failed: offset={} target_len={target_len} current_len={current_len}", $offset); + $interpreter.halt(revm::interpreter::InstructionResult::MemoryOOG); + return $ret; + } + + if target_len > current_len { + $interpreter.memory.resize(target_len); }; }; }