From 7b21a5d1764189b738c891cf7b8400131fc5ac8b Mon Sep 17 00:00:00 2001 From: Bjerg Date: Thu, 17 Mar 2022 12:44:01 +0100 Subject: [PATCH] REVM gas fixes (#950) * feat: account for gas refunds * refactor: merge `call_raw` and committing variant * fix: actually use refund quotient * feat: strip tx gas stipend * fix: fix reported gas usage in debugger * build: use upstream revm * test: adjust `forge run` gas values in tests * chore: remove unused copy * chore: add note on push maths * feat: make stipend reduction optional * fix: remove tx stipend in `forge run` --- Cargo.lock | 4 +- cli/src/cmd/run.rs | 26 ++--- cli/tests/cmd.rs | 7 +- forge/Cargo.toml | 2 +- forge/src/executor/fuzz/mod.rs | 32 ++++-- forge/src/executor/inspector/debugger.rs | 124 +++++++++++++++-------- forge/src/executor/inspector/tracer.rs | 19 +++- forge/src/executor/inspector/utils.rs | 8 +- forge/src/executor/mod.rs | 84 ++++++++------- forge/src/runner.rs | 37 ++++--- utils/src/lib.rs | 19 ---- 11 files changed, 207 insertions(+), 155 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 187360beb682..da65e0cfba9f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3569,7 +3569,7 @@ dependencies = [ [[package]] name = "revm" version = "1.2.0" -source = "git+https://github.com/onbjerg/revm?branch=onbjerg/tracer-ends#7a243f84a4ea9f5a969e9546bbf3d6b6096c845e" +source = "git+https://github.com/bluealloy/revm#db1812f5f53b1b00c9f1bc4a0b7484dd291b4b9c" dependencies = [ "arrayref", "auto_impl", @@ -3585,7 +3585,7 @@ dependencies = [ [[package]] name = "revm_precompiles" version = "0.4.0" -source = "git+https://github.com/onbjerg/revm?branch=onbjerg/tracer-ends#7a243f84a4ea9f5a969e9546bbf3d6b6096c845e" +source = "git+https://github.com/bluealloy/revm#db1812f5f53b1b00c9f1bc4a0b7484dd291b4b9c" dependencies = [ "bytes", "k256", diff --git a/cli/src/cmd/run.rs b/cli/src/cmd/run.rs index 9167e8fabfab..d54e1b71ecf3 100644 --- a/cli/src/cmd/run.rs +++ b/cli/src/cmd/run.rs @@ -117,13 +117,7 @@ impl Cmd for RunArgs { runner.setup(&predeploy_libraries, bytecode, needs_setup)?; let RunResult { - success, - gas_used, - logs, - traces, - debug: run_debug, - labeled_addresses, - .. + success, gas, logs, traces, debug: run_debug, labeled_addresses, .. } = runner.run( address, if let Some(calldata) = self.sig.strip_prefix("0x") { @@ -135,7 +129,7 @@ impl Cmd for RunArgs { result.success &= success; - result.gas_used = gas_used; + result.gas = gas; result.logs.extend(logs); result.traces.extend(traces); result.debug = run_debug; @@ -208,7 +202,7 @@ impl Cmd for RunArgs { println!("{}", Colour::Red.paint("Script failed.")); } - println!("Gas used: {}", result.gas_used); + println!("Gas used: {}", result.gas); println!("== Logs =="); let console_logs = decode_console_logs(&result.logs); if !console_logs.is_empty() { @@ -327,7 +321,7 @@ struct RunResult { pub logs: Vec, pub traces: Vec<(TraceKind, CallTraceArena)>, pub debug: Option>, - pub gas_used: u64, + pub gas: u64, pub labeled_addresses: BTreeMap, } @@ -389,7 +383,7 @@ impl Runner { labels, logs: setup_logs, debug, - gas: gas_used, + gas, .. }) | Err(EvmError::Execution { @@ -398,7 +392,7 @@ impl Runner { labels, logs: setup_logs, debug, - gas_used, + gas, .. }) => { traces @@ -413,7 +407,7 @@ impl Runner { labeled_addresses: labels, success: !reverted, debug: vec![constructor_debug, debug].into_iter().collect(), - gas_used, + gas, }, ) } @@ -427,7 +421,7 @@ impl Runner { traces, success: true, debug: vec![constructor_debug].into_iter().collect(), - gas_used: 0, + gas: 0, labeled_addresses: Default::default(), }, ) @@ -435,11 +429,11 @@ impl Runner { } pub fn run(&mut self, address: Address, calldata: Bytes) -> eyre::Result { - let RawCallResult { reverted, gas, logs, traces, labels, debug, .. } = + let RawCallResult { reverted, gas, stipend, logs, traces, labels, debug, .. } = self.executor.call_raw(self.sender, address, calldata.0, 0.into())?; Ok(RunResult { success: !reverted, - gas_used: gas, + gas: gas - stipend, logs, traces: traces.map(|traces| vec![(TraceKind::Execution, traces)]).unwrap_or_default(), debug: vec![debug].into_iter().collect(), diff --git a/cli/tests/cmd.rs b/cli/tests/cmd.rs index 79fd011086bc..b9e2238c9de1 100644 --- a/cli/tests/cmd.rs +++ b/cli/tests/cmd.rs @@ -461,7 +461,6 @@ Compiler run successful }); // Tests that the `run` command works correctly -// TODO: The original gas usage was "1751" before the REVM port. It should be changed back forgetest!(can_execute_run_command, |prj: TestProject, mut cmd: TestCommand| { let script = prj .inner() @@ -485,7 +484,7 @@ contract Demo { assert!(output.ends_with(&format!( "Compiler run successful {} -Gas used: 22815 +Gas used: 1751 == Logs == script ran ", @@ -517,7 +516,7 @@ contract Demo { assert!(output.ends_with(&format!( "Compiler run successful {} -Gas used: 22815 +Gas used: 1751 == Logs == script ran ", @@ -552,7 +551,7 @@ contract Demo { assert!(output.ends_with(&format!( "Compiler run successful {} -Gas used: 25301 +Gas used: 3957 == Logs == script ran 1 diff --git a/forge/Cargo.toml b/forge/Cargo.toml index dd4d84ad906c..39e34f577201 100644 --- a/forge/Cargo.toml +++ b/forge/Cargo.toml @@ -25,7 +25,7 @@ rlp = "0.5.1" bytes = "1.1.0" thiserror = "1.0.29" -revm = { package = "revm", git = "https://github.com/onbjerg/revm", branch = "onbjerg/tracer-ends", default-features = false, features = ["std", "k256"] } +revm = { package = "revm", git = "https://github.com/bluealloy/revm", default-features = false, features = ["std", "k256"] } hashbrown = "0.12" once_cell = "1.9.0" parking_lot = "0.12.0" diff --git a/forge/src/executor/fuzz/mod.rs b/forge/src/executor/fuzz/mod.rs index 21056b9fcb8d..22ebed18a283 100644 --- a/forge/src/executor/fuzz/mod.rs +++ b/forge/src/executor/fuzz/mod.rs @@ -78,12 +78,16 @@ where let success = self.executor.is_success( address, call.reverted, - call.state_changeset.clone().expect("we should have a state changeset"), + call.state_changeset.clone(), should_fail, ); if success { - cases.borrow_mut().push(FuzzCase { calldata, gas: call.gas }); + cases.borrow_mut().push(FuzzCase { + calldata, + gas: call.gas, + stipend: call.stipend, + }); Ok(()) } else { Err(TestCaseError::fail( @@ -192,18 +196,26 @@ impl FuzzedCases { } /// Returns the median gas of all test cases - pub fn median_gas(&self) -> u64 { + pub fn median_gas(&self, with_stipend: bool) -> u64 { let mid = self.cases.len() / 2; - self.cases.get(mid).map(|c| c.gas).unwrap_or_default() + self.cases + .get(mid) + .map(|c| if with_stipend { c.gas } else { c.gas - c.stipend }) + .unwrap_or_default() } /// Returns the average gas use of all test cases - pub fn mean_gas(&self) -> u64 { + pub fn mean_gas(&self, with_stipend: bool) -> u64 { if self.cases.is_empty() { return 0 } - (self.cases.iter().map(|c| c.gas as u128).sum::() / self.cases.len() as u128) as u64 + (self + .cases + .iter() + .map(|c| if with_stipend { c.gas as u128 } else { (c.gas - c.stipend) as u128 }) + .sum::() / + self.cases.len() as u128) as u64 } /// Returns the case with the highest gas usage @@ -217,8 +229,10 @@ impl FuzzedCases { } /// Returns the highest amount of gas spent on a fuzz case - pub fn highest_gas(&self) -> u64 { - self.highest().map(|c| c.gas).unwrap_or_default() + pub fn highest_gas(&self, with_stipend: bool) -> u64 { + self.highest() + .map(|c| if with_stipend { c.gas } else { c.gas - c.stipend }) + .unwrap_or_default() } /// Returns the lowest amount of gas spent on a fuzz case @@ -234,6 +248,8 @@ pub struct FuzzCase { pub calldata: Bytes, /// Consumed gas pub gas: u64, + /// The initial gas stipend for the transaction + pub stipend: u64, } #[cfg(test)] diff --git a/forge/src/executor/inspector/debugger.rs b/forge/src/executor/inspector/debugger.rs index 6e56b947247c..e194c111f4d4 100644 --- a/forge/src/executor/inspector/debugger.rs +++ b/forge/src/executor/inspector/debugger.rs @@ -1,12 +1,15 @@ use crate::{ debugger::{DebugArena, DebugNode, DebugStep, Instruction}, - executor::{inspector::utils::get_create_address, CHEATCODE_ADDRESS}, + executor::{ + inspector::utils::{gas_used, get_create_address}, + CHEATCODE_ADDRESS, + }, }; use bytes::Bytes; use ethers::types::Address; use revm::{ - CallInputs, CreateInputs, Database, EVMData, Gas, Inspector, Interpreter, Memory, OpCode, - Return, + opcode, spec_opcode_gas, CallInputs, CreateInputs, Database, EVMData, Gas, Inspector, + Interpreter, Memory, Return, SpecId, }; use std::collections::BTreeMap; @@ -24,6 +27,21 @@ pub struct Debugger { /// /// The instruction counter is used in Solidity source maps. pub ic_map: BTreeMap>, + /// The amount of gas spent in the current gas block. + /// + /// REVM adds gas in blocks, so we need to keep track of this separately to get accurate gas + /// numbers on an opcode level. + /// + /// Gas blocks contain the gas costs of opcodes with a fixed cost. Dynamic costs are not + /// included in the gas block, and are instead added during execution of the contract. + pub current_gas_block: u64, + /// The amount of gas spent in the previous gas block. + /// + /// Costs for gas blocks are accounted for when *entering* the gas block, which also means that + /// every run of the interpreter will always start with a non-zero `gas.spend()`. + /// + /// For more information on gas blocks, see [current_gas_block]. + pub previous_gas_block: u64, } impl Debugger { @@ -34,7 +52,8 @@ impl Debugger { /// Builds the instruction counter map for the given bytecode. // TODO: Some of the same logic is performed in REVM, but then later discarded. We should // investigate if we can reuse it - pub fn build_ic_map(&mut self, address: &Address, code: &Bytes) { + pub fn build_ic_map(&mut self, spec: SpecId, address: &Address, code: &Bytes) { + let opcode_infos = spec_opcode_gas(spec); let mut ic_map: BTreeMap = BTreeMap::new(); let mut i = 0; @@ -42,13 +61,12 @@ impl Debugger { while i < code.len() { let op = code[i]; ic_map.insert(i, i - cumulative_push_size); - match OpCode::is_push(op) { - Some(push_size) => { - // Skip the push bytes - i += push_size as usize; - cumulative_push_size += push_size as usize; - } - None => (), + if opcode_infos[op as usize].is_push { + // Skip the push bytes. + // + // For more context on the math, see: https://github.com/bluealloy/revm/blob/007b8807b5ad7705d3cacce4d92b89d880a83301/crates/revm/src/interpreter/contract.rs#L114-L115 + i += (op - opcode::PUSH1 + 1) as usize; + cumulative_push_size += (op - opcode::PUSH1 + 1) as usize; } i += 1; } @@ -70,35 +88,6 @@ impl Debugger { self.arena.push_node(DebugNode { depth, address, creation, ..Default::default() }); } } - - /// Records a debug step in the current execution context. - // TODO: Interpreter is only taken as a mutable borrow here because `Interpreter::gas` takes - // `&mut self` by mistake. - pub fn record_debug_step(&mut self, interpreter: &mut Interpreter) { - let pc = interpreter.program_counter(); - let push_size = OpCode::is_push(interpreter.contract.code[pc]).map(|size| size as usize); - let push_bytes = push_size.as_ref().map(|push_size| { - let start = pc + 1; - let end = start + push_size; - interpreter.contract.code[start..end].to_vec() - }); - - self.arena.arena[self.head].steps.push(DebugStep { - pc, - stack: interpreter.stack().data().clone(), - memory: interpreter.memory.clone(), - instruction: Instruction::OpCode(interpreter.contract.code[pc]), - push_bytes, - ic: *self - .ic_map - .get(&interpreter.contract().address) - .expect("no instruction counter map") - .get(&pc) - .expect("unknown ic for pc"), - // TODO: The number reported here is off - total_gas_used: interpreter.gas().spend(), - }); - } } impl Inspector for Debugger @@ -128,23 +117,70 @@ where fn initialize_interp( &mut self, interp: &mut Interpreter, - _: &mut EVMData<'_, DB>, + data: &mut EVMData<'_, DB>, _: bool, ) -> Return { // TODO: This is rebuilt for all contracts every time. We should only run this if the IC // map for a given address does not exist, *but* we need to account for the fact that the // code given by the interpreter may either be the contract init code, or the runtime code. - self.build_ic_map(&interp.contract().address, &interp.contract().code); + self.build_ic_map( + data.env.cfg.spec_id, + &interp.contract().address, + &interp.contract().code, + ); + self.previous_gas_block = interp.contract.first_gas_block(); Return::Continue } fn step( &mut self, interpreter: &mut Interpreter, - _: &mut EVMData<'_, DB>, + data: &mut EVMData<'_, DB>, _is_static: bool, ) -> Return { - self.record_debug_step(interpreter); + let pc = interpreter.program_counter(); + let op = interpreter.contract.code[pc]; + + // Get opcode information + let opcode_infos = spec_opcode_gas(data.env.cfg.spec_id); + let opcode_info = &opcode_infos[op as usize]; + + // Extract the push bytes + let push_size = if opcode_info.is_push { (op - opcode::PUSH1 + 1) as usize } else { 0 }; + let push_bytes = match push_size { + 0 => None, + n => { + let start = pc + 1; + let end = start + n; + Some(interpreter.contract.code[start..end].to_vec()) + } + }; + + // Calculate the current amount of gas used + let gas = interpreter.gas(); + let total_gas_spent = gas.spend() - self.previous_gas_block + self.current_gas_block; + if opcode_info.gas_block_end { + self.previous_gas_block = interpreter.contract.gas_block(pc); + self.current_gas_block = 0; + } else { + self.current_gas_block += opcode_info.gas; + } + + self.arena.arena[self.head].steps.push(DebugStep { + pc, + stack: interpreter.stack().data().clone(), + memory: interpreter.memory.clone(), + instruction: Instruction::OpCode(op), + push_bytes, + ic: *self + .ic_map + .get(&interpreter.contract().address) + .expect("no instruction counter map") + .get(&pc) + .expect("unknown ic for pc"), + total_gas_used: gas_used(data.env.cfg.spec_id, total_gas_spent, gas.refunded() as u64), + }); + Return::Continue } diff --git a/forge/src/executor/inspector/tracer.rs b/forge/src/executor/inspector/tracer.rs index 807b480f2bcc..91eb1548a24c 100644 --- a/forge/src/executor/inspector/tracer.rs +++ b/forge/src/executor/inspector/tracer.rs @@ -1,6 +1,9 @@ use super::logs::extract_log; use crate::{ - executor::{inspector::utils::get_create_address, HARDHAT_CONSOLE_ADDRESS}, + executor::{ + inspector::utils::{gas_used, get_create_address}, + HARDHAT_CONSOLE_ADDRESS, + }, trace::{ CallTrace, CallTraceArena, LogCallOrder, RawOrDecodedCall, RawOrDecodedLog, RawOrDecodedReturnData, @@ -95,7 +98,7 @@ where fn call_end( &mut self, - _: &mut EVMData<'_, DB>, + data: &mut EVMData<'_, DB>, call: &CallInputs, gas: Gas, status: Return, @@ -103,7 +106,11 @@ where _: bool, ) -> (Return, Gas, Bytes) { if call.contract != *HARDHAT_CONSOLE_ADDRESS { - self.fill_trace(matches!(status, return_ok!()), gas.spend(), retdata.to_vec()); + self.fill_trace( + matches!(status, return_ok!()), + gas_used(data.env.cfg.spec_id, gas.spend(), gas.refunded() as u64), + retdata.to_vec(), + ); } (status, gas, retdata) @@ -147,7 +154,11 @@ where .map_or(vec![], |code| code.to_vec()), None => vec![], }; - self.fill_trace(matches!(status, return_ok!()), gas.spend(), code); + self.fill_trace( + matches!(status, return_ok!()), + gas_used(data.env.cfg.spec_id, gas.spend(), gas.refunded() as u64), + code, + ); (status, address, gas, retdata) } diff --git a/forge/src/executor/inspector/utils.rs b/forge/src/executor/inspector/utils.rs index b17b3d1c0fec..ccac89163162 100644 --- a/forge/src/executor/inspector/utils.rs +++ b/forge/src/executor/inspector/utils.rs @@ -2,7 +2,7 @@ use ethers::{ types::Address, utils::{get_contract_address, get_create2_address}, }; -use revm::{CreateInputs, CreateScheme}; +use revm::{CreateInputs, CreateScheme, SpecId}; /// Returns [Return::Continue] on an error, discarding the error. /// @@ -49,3 +49,9 @@ pub fn get_create_address(call: &CreateInputs, nonce: u64) -> Address { } } } + +/// Get the gas used, accounting for refunds +pub fn gas_used(spec: SpecId, spent: u64, refunded: u64) -> u64 { + let refund_quotient = if SpecId::enabled(spec, SpecId::LONDON) { 5 } else { 2 }; + spent - (refunded).min(spent / refund_quotient) +} diff --git a/forge/src/executor/mod.rs b/forge/src/executor/mod.rs index f1e0ca7645dd..e1cef7e6ca15 100644 --- a/forge/src/executor/mod.rs +++ b/forge/src/executor/mod.rs @@ -47,16 +47,17 @@ use std::collections::BTreeMap; #[derive(thiserror::Error, Debug)] pub enum EvmError { /// Error which occurred during execution of a transaction - #[error("Execution reverted: {reason} (gas: {gas_used})")] + #[error("Execution reverted: {reason} (gas: {gas})")] Execution { reverted: bool, reason: String, - gas_used: u64, + gas: u64, + stipend: u64, logs: Vec, traces: Option, debug: Option, labels: BTreeMap, - state_changeset: Option>, + state_changeset: HashMap, }, /// Error which occurred during ABI encoding/decoding #[error(transparent)] @@ -90,6 +91,8 @@ pub struct CallResult { pub result: D, /// The gas used for the call pub gas: u64, + /// The initial gas stipend for the transaction + pub stipend: u64, /// The logs emitted during the call pub logs: Vec, /// The labels assigned to addresses during the call @@ -102,7 +105,7 @@ pub struct CallResult { /// /// This is only present if the changed state was not committed to the database (i.e. if you /// used `call` and `call_raw` not `call_committing` or `call_raw_committing`). - pub state_changeset: Option>, + pub state_changeset: HashMap, } /// The result of a raw call. @@ -116,6 +119,8 @@ pub struct RawCallResult { pub result: Bytes, /// The gas used for the call pub gas: u64, + /// The initial gas stipend for the transaction + pub stipend: u64, /// The logs emitted during the call pub logs: Vec, /// The labels assigned to addresses during the call @@ -128,7 +133,7 @@ pub struct RawCallResult { /// /// This is only present if the changed state was not committed to the database (i.e. if you /// used `call` and `call_raw` not `call_committing` or `call_raw_committing`). - pub state_changeset: Option>, + pub state_changeset: HashMap, } impl Default for RawCallResult { @@ -138,11 +143,12 @@ impl Default for RawCallResult { reverted: false, result: Bytes::new(), gas: 0, + stipend: 0, logs: Vec::new(), labels: BTreeMap::new(), traces: None, debug: None, - state_changeset: None, + state_changeset: HashMap::new(), } } } @@ -218,8 +224,18 @@ where ) -> std::result::Result, EvmError> { let func = func.into(); let calldata = Bytes::from(encode_function_data(&func, args)?.to_vec()); - let RawCallResult { result, status, reverted, gas, logs, labels, traces, debug, .. } = - self.call_raw_committing(from, to, calldata, value)?; + let RawCallResult { + result, + status, + reverted, + gas, + stipend, + logs, + labels, + traces, + debug, + state_changeset, + } = self.call_raw_committing(from, to, calldata, value)?; match status { return_ok!() => { let result = decode_function_data(&func, result, false)?; @@ -227,11 +243,12 @@ where reverted, result, gas, + stipend, logs, labels, traces, debug, - state_changeset: None, + state_changeset, }) } _ => { @@ -240,12 +257,13 @@ where Err(EvmError::Execution { reverted, reason, - gas_used: gas, + gas, + stipend, logs, traces, debug, labels, - state_changeset: None, + state_changeset, }) } } @@ -261,30 +279,9 @@ where calldata: Bytes, value: U256, ) -> Result { - let mut evm = EVM::new(); - evm.env = self.build_env(from, TransactTo::Call(to), calldata, value); - evm.database(&mut self.db); - - // Run the call - let mut inspector = self.inspector_config.stack(); - let (status, out, gas, _) = evm.inspect_commit(&mut inspector); - let result = match out { - TransactOut::Call(data) => data, - _ => Bytes::default(), - }; - let InspectorData { logs, labels, traces, debug } = collect_inspector_states(inspector); - - Ok(RawCallResult { - status, - reverted: !matches!(status, return_ok!()), - result, - gas, - logs, - labels, - traces, - debug, - state_changeset: None, - }) + let result = self.call_raw(from, to, calldata, value)?; + self.db.commit(result.state_changeset.clone()); + Ok(result) } /// Performs a call to an account on the current state of the VM. @@ -306,6 +303,7 @@ where status, reverted, gas, + stipend, logs, labels, traces, @@ -319,6 +317,7 @@ where reverted, result, gas, + stipend, logs, labels, traces, @@ -332,7 +331,8 @@ where Err(EvmError::Execution { reverted, reason, - gas_used: gas, + gas, + stipend, logs, traces, debug, @@ -353,6 +353,9 @@ where calldata: Bytes, value: U256, ) -> Result { + let stipend = stipend(&calldata, self.env.cfg.spec_id); + + // Build VM let mut evm = EVM::new(); evm.env = self.build_env(from, TransactTo::Call(to), calldata, value); evm.database(&self.db); @@ -371,11 +374,12 @@ where reverted: !matches!(status, return_ok!()), result, gas, + stipend, logs: logs.to_vec(), labels, traces, debug, - state_changeset: Some(state_changeset), + state_changeset, }) } @@ -463,3 +467,9 @@ fn collect_inspector_states(stack: InspectorStack) -> InspectorData { debug: stack.debugger.map(|debugger| debugger.arena), } } + +/// Calculates the initial gas stipend for a transaction +fn stipend(calldata: &[u8], spec: SpecId) -> u64 { + let non_zero_data_cost = if SpecId::enabled(spec, SpecId::ISTANBUL) { 16 } else { 68 }; + calldata.iter().fold(21000, |sum, byte| sum + if *byte == 0 { 4 } else { non_zero_data_cost }) +} diff --git a/forge/src/runner.rs b/forge/src/runner.rs index 1c9469b49137..b277c3531562 100644 --- a/forge/src/runner.rs +++ b/forge/src/runner.rs @@ -103,8 +103,8 @@ impl TestKind { TestKind::Standard(gas) => TestKindGas::Standard(*gas), TestKind::Fuzz(fuzzed) => TestKindGas::Fuzz { runs: fuzzed.cases().len(), - median: fuzzed.median_gas(), - mean: fuzzed.mean_gas(), + median: fuzzed.median_gas(false), + mean: fuzzed.mean_gas(false), }, } } @@ -305,13 +305,14 @@ impl<'a, DB: DatabaseRef + Send + Sync> ContractRunner<'a, DB> { // Run unit test let start = Instant::now(); - let (reverted, reason, gas_used, execution_traces, state_changeset) = match self + let (reverted, reason, gas, stipend, execution_traces, state_changeset) = match self .executor .call::<(), _, _>(self.sender, address, func.clone(), (), 0.into(), self.errors) { Ok(CallResult { reverted, - gas: gas_used, + gas, + stipend, logs: execution_logs, traces: execution_trace, labels: new_labels, @@ -320,12 +321,13 @@ impl<'a, DB: DatabaseRef + Send + Sync> ContractRunner<'a, DB> { }) => { labeled_addresses.extend(new_labels); logs.extend(execution_logs); - (reverted, None, gas_used, execution_trace, state_changeset) + (reverted, None, gas, stipend, execution_trace, state_changeset) } Err(EvmError::Execution { reverted, reason, - gas_used, + gas, + stipend, logs: execution_logs, traces: execution_trace, labels: new_labels, @@ -334,7 +336,7 @@ impl<'a, DB: DatabaseRef + Send + Sync> ContractRunner<'a, DB> { }) => { labeled_addresses.extend(new_labels); logs.extend(execution_logs); - (reverted, Some(reason), gas_used, execution_trace, state_changeset) + (reverted, Some(reason), gas, stipend, execution_trace, state_changeset) } Err(err) => { tracing::error!(?err); @@ -343,18 +345,14 @@ impl<'a, DB: DatabaseRef + Send + Sync> ContractRunner<'a, DB> { }; traces.extend(execution_traces.map(|traces| (TraceKind::Execution, traces)).into_iter()); - let success = self.executor.is_success( - setup.address, - reverted, - state_changeset.expect("we should have a state changeset"), - should_fail, - ); + let success = + self.executor.is_success(setup.address, reverted, state_changeset, should_fail); // Record test execution time tracing::debug!( duration = ?Instant::now().duration_since(start), %success, - %gas_used + %gas ); Ok(TestResult { @@ -362,7 +360,7 @@ impl<'a, DB: DatabaseRef + Send + Sync> ContractRunner<'a, DB> { reason, counterexample: None, logs, - kind: TestKind::Standard(gas_used), + kind: TestKind::Standard(gas - stipend), traces, labeled_addresses, }) @@ -507,11 +505,12 @@ mod tests { // get the counterexample with shrinking enabled by default let counterexample = res.counterexample.unwrap(); + + // casting to u64 here is safe because the shrunk result is always gonna be small + // enough to fit in a u64, whereas as seen below, that's not possible without + // shrinking let product_with_shrinking: u64 = - // casting to u64 here is safe because the shrunk result is always gonna be small - // enough to fit in a u64, whereas as seen below, that's not possible without - // shrinking - counterexample.args.into_iter().map(|x| x.into_uint().unwrap().as_u64()).product(); + counterexample.args.into_iter().map(|x| x.into_uint().unwrap().as_u64()).product(); let mut cfg = FuzzConfig::default(); cfg.failure_persistence = None; diff --git a/utils/src/lib.rs b/utils/src/lib.rs index 9c8c84a00a2d..5e71a4829a01 100644 --- a/utils/src/lib.rs +++ b/utils/src/lib.rs @@ -149,8 +149,6 @@ pub fn recurse_link<'a>( } } -const BASE_TX_COST: u64 = 21000; - /// Helper trait for converting types to Functions. Helpful for allowing the `call` /// function on the EVM to be generic over `String`, `&str` and `Function`. pub trait IntoFunction { @@ -183,23 +181,6 @@ impl<'a> IntoFunction for &'a str { } } -/// Given a gas value and a calldata array, it subtracts the calldata cost from the -/// gas value, as well as the 21k base gas cost for all transactions. -pub fn remove_extra_costs(gas: U256, calldata: &[u8]) -> U256 { - let mut calldata_cost = 0; - for i in calldata { - if *i != 0 { - // TODO: Check if EVM pre-eip2028 and charge 64 - // GTXDATANONZERO = 16 - calldata_cost += 16 - } else { - // GTXDATAZERO = 4 - calldata_cost += 4; - } - } - gas.saturating_sub(calldata_cost.into()).saturating_sub(BASE_TX_COST.into()) -} - /// Flattens a group of contracts into maps of all events and functions pub fn flatten_known_contracts( contracts: &BTreeMap)>,