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`
foundry-rs · Mar 17, 2022 · 7b21a5d · 7b21a5d
1 parent 28deb75
commit 7b21a5d
Show file tree

Hide file tree

Showing 11 changed files with 207 additions and 155 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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<RawLog>,
     pub traces: Vec<(TraceKind, CallTraceArena)>,
     pub debug: Option<Vec<DebugArena>>,
-    pub gas_used: u64,
+    pub gas: u64,
     pub labeled_addresses: BTreeMap<Address, String>,
 }
 
@@ -389,7 +383,7 @@ impl<DB: DatabaseRef> Runner<DB> {
                     labels,
                     logs: setup_logs,
                     debug,
-                    gas: gas_used,
+                    gas,
                     ..
                 }) |
                 Err(EvmError::Execution {
@@ -398,7 +392,7 @@ impl<DB: DatabaseRef> Runner<DB> {
                     labels,
                     logs: setup_logs,
                     debug,
-                    gas_used,
+                    gas,
                     ..
                 }) => {
                     traces
@@ -413,7 +407,7 @@ impl<DB: DatabaseRef> Runner<DB> {
                             labeled_addresses: labels,
                             success: !reverted,
                             debug: vec![constructor_debug, debug].into_iter().collect(),
-                            gas_used,
+                            gas,
                         },
                     )
                 }
@@ -427,19 +421,19 @@ impl<DB: DatabaseRef> Runner<DB> {
                     traces,
                     success: true,
                     debug: vec![constructor_debug].into_iter().collect(),
-                    gas_used: 0,
+                    gas: 0,
                     labeled_addresses: Default::default(),
                 },
             )
         })
     }
 
     pub fn run(&mut self, address: Address, calldata: Bytes) -> eyre::Result<RunResult> {
-        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
@@ -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
@@ -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
@@ -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::<u128>() / 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::<u128>() /
+            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)]