Skip to content

Commit

Permalink
fix: return all traces, turn on necessary tracer config
Browse files Browse the repository at this point in the history
There was OOM concern but using the get-by-index style, despite improved, does not solve the root cause.
The main issue is that the tracer config did not turn off after the stop recording cheatcode being called.
It seems too much burden for the tracer to record the returned traces inside forge tests as the tests will
also pass around the debug traces, causing memory boost.

This commit also only turns on necessary tracer config instead of using all().
  • Loading branch information
boolafish committed Sep 20, 2024
1 parent 8ea53b1 commit 141aea7
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 90 deletions.
60 changes: 20 additions & 40 deletions crates/cheatcodes/assets/cheatcodes.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 2 additions & 7 deletions crates/cheatcodes/spec/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,14 +312,9 @@ interface Vm {
#[cheatcode(group = Evm, safety = Safe)]
function startDebugTraceRecording() external;

/// Stop debug trace recording and returns the total size of the recorded debug trace.
/// Stop debug trace recording and returns the recorded debug trace.
#[cheatcode(group = Evm, safety = Safe)]
function stopDebugTraceRecording() external returns (uint256 size);


/// Get the recorded debug trace by step index.
#[cheatcode(group = Evm, safety = Safe)]
function getDebugTraceByIndex(uint256 index) external returns (DebugStep memory step);
function stopAndReturnDebugTraceRecording() external returns (DebugStep[] memory step);


// -------- Record Storage --------
Expand Down
35 changes: 10 additions & 25 deletions crates/cheatcodes/src/evm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ use crate::{
};
use alloy_consensus::TxEnvelope;
use alloy_genesis::{Genesis, GenesisAccount};
use alloy_primitives::{Address, Bytes, Uint, B256, U256};
use alloy_primitives::{Address, Bytes, B256, U256};
use alloy_rlp::Decodable;
use alloy_sol_types::SolValue;
use foundry_common::fs::{read_json_file, write_json_file};
use foundry_evm_core::{
backend::{DatabaseExt, RevertSnapshotAction},
constants::{CALLER, CHEATCODE_ADDRESS, HARDHAT_CONSOLE_ADDRESS, TEST_CONTRACT_ADDRESS},
};
use foundry_evm_traces::TracingInspectorConfig;
use foundry_evm_traces::StackSnapshotType;
use rand::Rng;
use revm::{
primitives::{Account, Bytecode, SpecId, KECCAK_EMPTY},
Expand Down Expand Up @@ -646,7 +646,12 @@ impl Cheatcode for startDebugTraceRecordingCall {
};

// turn on tracer configuration for recording
tracer.update_config(|_config| TracingInspectorConfig::all());
tracer.update_config(|config| {
config
.set_steps(true)
.set_memory_snapshots(true)
.set_stack_snapshots(StackSnapshotType::Full)
});

// track where the recording starts
if let Some(last_node) = tracer.traces().nodes().last() {
Expand All @@ -658,7 +663,7 @@ impl Cheatcode for startDebugTraceRecordingCall {
}
}

impl Cheatcode for stopDebugTraceRecordingCall {
impl Cheatcode for stopAndReturnDebugTraceRecordingCall {
fn apply_full<DB: DatabaseExt, E: CheatcodesExecutor>(
&self,
ccx: &mut CheatsCtxt<DB>,
Expand All @@ -683,31 +688,11 @@ impl Cheatcode for stopDebugTraceRecordingCall {
// store the recorded debug steps
let debug_steps: Vec<DebugStep> =
steps.iter().map(|&step| convert_call_trace_to_debug_step(step)).collect();
ccx.state.recorded_debug_steps = Some(debug_steps);

// Clean up the recording info
ccx.state.record_debug_steps_info = None;

// return the length of the debug steps
let length = ccx.state.recorded_debug_steps.as_ref().map_or(0, |v| v.len());
let length_uint = Uint::<256, 4>::from(length);
Ok(length_uint.abi_encode())
}
}

impl Cheatcode for getDebugTraceByIndexCall {
fn apply(&self, state: &mut Cheatcodes) -> Result {
let Self { index } = self;
let idx: usize =
index.try_into().map_err(|_| format!("index [{index}] cannot convert to usize"))?;

if let Some(debug_steps) = state.recorded_debug_steps.as_ref() {
if let Some(debug_step) = debug_steps.get(idx) {
return Ok(debug_step.abi_encode());
}
}

Err(Error::from("debug trace access failed"))
Ok(debug_steps.abi_encode())
}
}

Expand Down
6 changes: 0 additions & 6 deletions crates/cheatcodes/src/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,11 +394,6 @@ pub struct Cheatcodes {
/// merged into the previous vector.
pub recorded_account_diffs_stack: Option<Vec<Vec<AccountAccess>>>,

/// Recorded debug trace/steps during the call.
/// This field stores a debug trace that were recorded during the call.
/// It is empty if nothing were recorded.
pub recorded_debug_steps: Option<Vec<crate::Vm::DebugStep>>,

/// The information of the debug step recording.
pub record_debug_steps_info: Option<RecordDebugStepInfo>,

Expand Down Expand Up @@ -490,7 +485,6 @@ impl Cheatcodes {
accesses: Default::default(),
recorded_account_diffs_stack: Default::default(),
recorded_logs: Default::default(),
recorded_debug_steps: Default::default(),
record_debug_steps_info: Default::default(),
mocked_calls: Default::default(),
mocked_functions: Default::default(),
Expand Down
3 changes: 1 addition & 2 deletions testdata/cheats/Vm.sol

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 10 additions & 10 deletions testdata/default/cheats/RecordDebugTrace.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,13 @@ contract RecordDebugTraceTest is DSTest {
uint256 val = testContract.storeAndLoadValueFromMemory();
assertTrue(val == testContract.expectedValueInMemory());

uint256 stepsLen = cheats.stopDebugTraceRecording();
Vm.DebugStep[] memory steps = cheats.stopAndReturnDebugTraceRecording();

bool mstoreCalled = false;
bool mloadCalled = false;

for (uint256 i = 0; i < stepsLen; i++) {
Vm.DebugStep memory step = cheats.getDebugTraceByIndex(i);
for (uint256 i = 0; i < steps.length; i++) {
Vm.DebugStep memory step = steps[i];
if (
step.opcode == 0x52 /*MSTORE*/ && step.stack[0] == testContract.memPtr() // MSTORE offset
&& step.stack[1] == testContract.expectedValueInMemory() // MSTORE val
Expand Down Expand Up @@ -118,12 +118,12 @@ contract RecordDebugTraceTest is DSTest {

first.callSecondLayer();

uint256 stepsLen = cheats.stopDebugTraceRecording();
Vm.DebugStep[] memory steps = cheats.stopAndReturnDebugTraceRecording();

bool goToDepthTwo = false;
bool goToDepthThree = false;
for (uint256 i = 0; i < stepsLen; i++) {
Vm.DebugStep memory step = cheats.getDebugTraceByIndex(i);
for (uint256 i = 0; i < steps.length; i++) {
Vm.DebugStep memory step = steps[i];

if (step.depth == 2) {
assertTrue(step.contractAddr == address(first), "must be first layer on depth 2");
Expand All @@ -149,17 +149,17 @@ contract RecordDebugTraceTest is DSTest {

testContract.triggerOOG();

uint256 stepsLen = cheats.stopDebugTraceRecording();
Vm.DebugStep[] memory steps = cheats.stopAndReturnDebugTraceRecording();

bool isOOG = false;
for (uint256 i = 0; i < stepsLen; i++) {
Vm.DebugStep memory step = cheats.getDebugTraceByIndex(i);
for (uint256 i = 0; i < steps.length; i++) {
Vm.DebugStep memory step = steps[i];

// https://github.com/bluealloy/revm/blob/5a47ae0d2bb0909cc70d1b8ae2b6fc721ab1ca7d/crates/interpreter/src/instruction_result.rs#L23
if (step.isOutOfGas) {
isOOG = true;
}
}
assertTrue(isOOG, "should have OOG instruction result");
assertTrue(isOOG, "should OOG");
}
}

0 comments on commit 141aea7

Please sign in to comment.