Skip to content

Commit

Permalink
fix: trace decoding off-by-one (#6277)
Browse files Browse the repository at this point in the history
  • Loading branch information
DaniPopes authored Nov 10, 2023
1 parent 4c11a23 commit 90d4dce
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 43 deletions.
26 changes: 22 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,28 @@ jobs:
with:
target: ${{ matrix.target }}
- uses: taiki-e/install-action@nextest
# - uses: taiki-e/setup-cross-toolchain-action@v1
# with:
# target: ${{ matrix.target }}

# External tests dependencies
- name: Setup Python
if: contains(matrix.name, 'external')
uses: actions/setup-python@v4
with:
python-version: 3.11
- name: Setup Node.js
if: contains(matrix.name, 'external')
uses: actions/setup-node@v4
with:
node-version: 20
- name: Install pnpm
if: contains(matrix.name, 'external')
uses: pnpm/action-setup@v2
with:
version: latest
run_install: false
- name: Install Vyper
if: contains(matrix.name, 'external')
run: pip install vyper

- name: Forge RPC cache
uses: actions/cache@v3
with:
Expand All @@ -61,7 +80,6 @@ jobs:
key: rpc-cache-${{ hashFiles('crates/forge/tests/rpc-cache-keyfile') }}
- uses: Swatinem/rust-cache@v2
with:
# key: ${{ matrix.target }}
cache-on-failure: true
- name: Setup Git config
run: |
Expand Down
5 changes: 4 additions & 1 deletion crates/anvil/tests/it/otterscan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,12 @@ async fn can_call_ots_has_code() {
.await
.unwrap());

client.send_transaction(deploy_tx, None).await.unwrap();
let pending = client.send_transaction(deploy_tx, None).await.unwrap();
let receipt = pending.await.unwrap().unwrap();

let num = client.get_block_number().await.unwrap();
assert_eq!(num, receipt.block_number.unwrap());

// code is detected after deploying
assert!(api.ots_has_code(pending_contract_address, BlockNumber::Number(num)).await.unwrap());

Expand Down
2 changes: 1 addition & 1 deletion crates/evm/traces/src/decoder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ impl CallTraceDecoder {
let &[t0, ..] = raw_log.topics() else { return };

let mut events = Vec::new();
let events = match self.events.get(&(t0, raw_log.topics().len())) {
let events = match self.events.get(&(t0, raw_log.topics().len() - 1)) {
Some(es) => es,
None => {
if let Some(identifier) = &self.signature_identifier {
Expand Down
3 changes: 1 addition & 2 deletions crates/evm/traces/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,7 @@ impl fmt::Display for CallTraceArena {
let node = &arena.arena[idx];

// Display trace header
f.write_str(left)?;
node.trace.fmt(f)?;
writeln!(f, "{left}{}", node.trace)?;

// Display logs and subcalls
let left_prefix = format!("{child}{BRANCH}");
Expand Down
1 change: 1 addition & 0 deletions crates/forge/tests/cli/ext_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ forgetest_external!(
forgetest_external!(stringutils, "Arachnid/solidity-stringutils");
forgetest_external!(lootloose, "gakonst/lootloose");
forgetest_external!(lil_web3, "m1guelpf/lil-web3");
forgetest_external!(snekmate, "pcaversaccio/snekmate");

// Forking tests

Expand Down
25 changes: 6 additions & 19 deletions crates/test-utils/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,31 +147,18 @@ macro_rules! forgetest_external {
prj.wipe();

// Clone the external repository
let git_clone =
$crate::util::clone_remote(&format!("https://github.com/{}", $repo), prj.root())
.expect("Could not clone repository. Is git installed?");
assert!(
git_clone.status.success(),
"could not clone repository:\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&git_clone.stdout),
String::from_utf8_lossy(&git_clone.stderr)
);
$crate::util::clone_remote(concat!("https://github.com/", $repo), prj.root().to_str().unwrap());

// We just run make install, but we do not care if it worked or not,
// since some repositories do not have that target
let make_install = Command::new("make")
.arg("install")
.current_dir(prj.root())
.stdout(Stdio::null())
.stderr(Stdio::null())
.status();
// Run common installation commands
$crate::util::run_install_commands(prj.root());

// Run the tests
cmd.arg("test").args($forge_opts).args([
"--optimize",
"--optimizer-runs",
"20000",
"--optimizer-runs=20000",
"--fuzz-runs=256",
"--ffi",
"-vvvvv",
]);
cmd.set_env("FOUNDRY_FUZZ_RUNS", "1");

Expand Down
53 changes: 38 additions & 15 deletions crates/test-utils/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,21 +76,44 @@ pub fn initialize(target: impl AsRef<Path>) {
}
}

/// Clones a remote repository into the specified directory.
pub fn clone_remote(
repo_url: &str,
target_dir: impl AsRef<Path>,
) -> std::io::Result<process::Output> {
Command::new("git")
.args([
"clone",
"--depth",
"1",
"--recursive",
repo_url,
target_dir.as_ref().to_str().expect("Target path for git clone does not exist"),
])
.output()
/// Clones a remote repository into the specified directory. Panics if the command fails.
pub fn clone_remote(repo_url: &str, target_dir: &str) {
let mut cmd = Command::new("git");
let status = cmd
.args(["clone", "--depth=1", "--recursive", "--shallow-submodules", repo_url, target_dir])
.status()
.unwrap();
if !status.success() {
panic!("{cmd:?}");
}
eprintln!();
}

/// Runs common installation commands, such as `make` and `npm`. Continues if any command fails.
pub fn run_install_commands(root: &Path) {
let root_files =
std::fs::read_dir(root).unwrap().flatten().map(|x| x.path()).collect::<Vec<_>>();
let contains = |path: &str| root_files.iter().any(|p| p.to_str().unwrap().contains(path));
let run = |args: &[&str]| {
let mut cmd = Command::new(args[0]);
cmd.args(&args[1..]).current_dir(root).stdout(Stdio::null()).stderr(Stdio::null());
let st = cmd.status();
eprintln!("\n\n{cmd:?} -> {st:?}");
};
let maybe_run = |path: &str, args: &[&str]| {
let c = contains(path);
if c {
run(args);
}
c
};

maybe_run("Makefile", &["make", "install"]);
let pnpm = maybe_run("pnpm-lock.yaml", &["pnpm", "install", "--prefer-offline"]);
let yarn = maybe_run("yarn.lock", &["yarn", "install", "--prefer-offline"]);
if !pnpm && !yarn && contains("package.json") {
run(&["npm", "install"]);
}
}

/// Setup an empty test project and return a command pointing to the forge
Expand Down
4 changes: 3 additions & 1 deletion testdata/cheats/UnixTime.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import "./Vm.sol";

contract UnixTimeTest is DSTest {
Vm constant vm = Vm(HEVM_ADDRESS);
uint256 errMargin = 100; // allow errors of up to errMargin milliseconds

// This is really wide because CI sucks.
uint256 constant errMargin = 300;

function testUnixTimeAgainstDate() public {
string[] memory inputs = new string[](2);
Expand Down

0 comments on commit 90d4dce

Please sign in to comment.