Skip to content

Commit

Permalink
fix(forge): Access list not resetting after setUp (foundry-rs#951)
Browse files Browse the repository at this point in the history
* fix access list not resetting after setUp

* comment fix
  • Loading branch information
brockelmore authored Mar 16, 2022
1 parent b501848 commit d66f9d5
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 4 deletions.
17 changes: 15 additions & 2 deletions forge/src/multi_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,8 @@ mod tests {
let mut runner = runner();
let results = runner.test(&Filter::matches_all(), None).unwrap();

// 9 contracts being built
assert_eq!(results.keys().len(), 9);
// 10 contracts being built
assert_eq!(results.keys().len(), 10);
for (key, contract_tests) in results {
// for a bad setup, we dont want a successful test
if key == "SetupTest.json:SetupTest" {
Expand Down Expand Up @@ -353,6 +353,19 @@ mod tests {
);
}

#[test]
fn test_access_list_reset() {
let mut runner = runner();
let results = runner.test(&Filter::matches_all(), None).unwrap();
let results = &results["WhichTest.json:WhichTest"];
assert!(results["testA()"].success);
assert_eq!(results["testA()"].gas_used, 65577);
assert!(results["testB()"].success);
assert_eq!(results["testB()"].gas_used, 68263);
assert!(results["testC()"].success);
assert_eq!(results["testC()"].gas_used, 63611);
}

#[test]
fn test_sputnik_multi_runner() {
test_multi_runner();
Expand Down
16 changes: 14 additions & 2 deletions forge/src/runner.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
use crate::TestFilter;
use evm_adapters::{
evm_opts::EvmOpts,
sputnik::{helpers::TestSputnikVM, Executor, PRECOMPILES_MAP},
sputnik::{helpers::TestSputnikVM, Executor, SputnikExecutor, PRECOMPILES_MAP},
};
use rayon::iter::ParallelIterator;
use sputnik::{backend::Backend, Config};
use sputnik::{
backend::Backend,
executor::stack::{StackState, StackSubstateMetadata},
Config,
};

use ethers::{
abi::{Abi, Event, Function, Token},
Expand Down Expand Up @@ -345,6 +349,14 @@ impl<'a, B: Backend + Clone + Send + Sync> ContractRunner<'a, B> {
}
};
logs.extend_from_slice(&setup_logs);
// this is dumb, but resetting metadata is the only way to reset access list, which
// affects setup -> call gas costs because setUp can make addresses and slots warm
//
// long live REVM. This is totally unnecessary once we switch.
let state = evm.executor.state_mut();
let metadata = StackSubstateMetadata::new(self.evm_opts.env.gas_limit, self.evm_cfg);
let meta = state.metadata_mut();
*meta = metadata;
}

let (status, reason, gas_used, logs) = match evm.call::<(), _, _>(
Expand Down
82 changes: 82 additions & 0 deletions forge/testdata/WarmColdTest.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// SPDX-License-Identifier: MIT
pragma solidity =0.8.7;

contract DsTestMini {
bool public failed;

function fail() private {
failed = true;
}

function assertEq(uint a, uint b) internal {
if (a != b) {
fail();
}
}
}

contract Which {
uint256[] public s_playerFunds;
uint256 public s_totalFunds;

constructor() {
s_playerFunds = [
1,
15,
22,
199,
234,
5,
234,
5,
2345,
234,
555,
23423,
55
];
}

function a() public {
uint256 totalFunds;
for (uint256 i = 0; i < s_playerFunds.length; i++) {
totalFunds = totalFunds + s_playerFunds[i];
}
s_totalFunds = totalFunds;
}

function b() external {
for (uint256 i = 0; i < s_playerFunds.length; i++) {
s_totalFunds += s_playerFunds[i];
}
}

function c() public {
uint256 totalFunds;
uint256[] memory playerFunds = s_playerFunds;
for (uint256 i = 0; i < playerFunds.length; i++) {
totalFunds = totalFunds + playerFunds[i];
}
s_totalFunds = totalFunds;
}
}

contract WhichTest is DsTestMini {
Which internal which;

function setUp() public {
which = new Which();
}

function testA() public {
which.a();
}

function testB() public {
which.b();
}

function testC() public {
which.c();
}
}

0 comments on commit d66f9d5

Please sign in to comment.