Skip to content

Commit

Permalink
Fix(tracing): Do not panic on empty call stack (#817)
Browse files Browse the repository at this point in the history
## Description

Fixes a bug where standalone engine can crash on tracing transactions
with too large contract deployment.

## Testing

New regression test.
  • Loading branch information
birchmd authored Aug 10, 2023
1 parent ce14890 commit f57df5c
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 6 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/lints.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ jobs:
git checkout -f $(git -c user.name=x -c user.email=x@x commit-tree $(git hash-object -t tree /dev/null) < /dev/null) || :
- name: Clone the repository
uses: actions/checkout@v3
- name: Update udeps
run: cargo install --force cargo-udeps
- name: Run udeps
run: cargo make udeps
contracts:
Expand Down
100 changes: 96 additions & 4 deletions engine-standalone-tracing/src/types/call_tracer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,37 @@ pub struct CallFrame {
#[derive(Debug, Default, Clone, PartialEq, Eq)]
pub struct CallTracer {
pub call_stack: Vec<CallFrame>,
pub top_level_transact: Option<CallFrame>,
}

impl CallTracer {
fn end(&mut self, output: Vec<u8>, error: Option<&evm::ExitReason>) {
if self.call_stack.is_empty() {
debug_assert!(
error.is_some(),
"Empty stack can only occur if there is an early error"
);
debug_assert!(
self.top_level_transact.is_some(),
"Top level transact events always come before any exit event"
);
let frame = self.top_level_transact.take().unwrap_or_else(|| CallFrame {
call_type: CallType::Call,
from: Address::default(),
to: None,
value: U256::zero(),
gas: 0,
gas_used: 0,
input: Vec::new(),
output: Vec::new(),
error: Some("Tracing bug: Exit before Enter".into()),
calls: Vec::new(),
});
self.call_stack.push(frame);
}

// unwrap is safe because we push a new frame if the
// stack was empty at the start of this method.
let frame = self.call_stack.first_mut().unwrap();
match error {
None => {
Expand Down Expand Up @@ -161,6 +188,7 @@ impl evm_runtime::tracing::EventListener for CallTracer {
}

impl evm::tracing::EventListener for CallTracer {
#[allow(clippy::too_many_lines)]
fn event(&mut self, event: evm::tracing::Event) {
match event {
evm::tracing::Event::Call {
Expand Down Expand Up @@ -255,11 +283,75 @@ impl evm::tracing::EventListener for CallTracer {
self.exit(return_value.to_vec(), error);
}

evm::tracing::Event::TransactCall {
caller,
address,
value,
data,
gas_limit,
} => {
let frame = CallFrame {
call_type: CallType::Call,
from: Address::new(caller),
to: Some(Address::new(address)),
value,
gas: gas_limit,
gas_used: 0,
input: data.to_vec(),
output: Vec::new(),
error: None,
calls: Vec::new(),
};
self.top_level_transact = Some(frame);
}

evm::tracing::Event::TransactCreate {
caller,
value,
init_code,
gas_limit,
address,
} => {
let frame = CallFrame {
call_type: CallType::Create,
from: Address::new(caller),
to: Some(Address::new(address)),
value,
gas: gas_limit,
gas_used: 0,
input: init_code.to_vec(),
output: Vec::new(),
error: None,
calls: Vec::new(),
};
self.top_level_transact = Some(frame);
}

evm::tracing::Event::TransactCreate2 {
caller,
value,
init_code,
gas_limit,
address,
..
} => {
let frame = CallFrame {
call_type: CallType::Create2,
from: Address::new(caller),
to: Some(Address::new(address)),
value,
gas: gas_limit,
gas_used: 0,
input: init_code.to_vec(),
output: Vec::new(),
error: None,
calls: Vec::new(),
};
self.top_level_transact = Some(frame);
}

// not useful
evm::tracing::Event::PrecompileSubcall { .. }
| evm::tracing::Event::TransactCall { .. }
| evm::tracing::Event::TransactCreate { .. }
| evm::tracing::Event::TransactCreate2 { .. } => (),
evm::tracing::Event::PrecompileSubcall { .. } => (),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions engine-tests/src/tests/res/contract_data_too_large.hex

Large diffs are not rendered by default.

37 changes: 37 additions & 0 deletions engine-tests/src/tests/standalone/call_tracer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,43 @@ fn test_trace_contract_with_precompile_sub_call() {
runner.close();
}

#[test]
fn test_contract_create_too_large() {
let mut runner = standalone::StandaloneRunner::default();
let signer = Signer::random();

runner.init_evm();

let tx_data = {
let tx_data_hex =
std::fs::read_to_string("src/tests/res/contract_data_too_large.hex").unwrap();
hex::decode(
tx_data_hex
.strip_prefix("0x")
.unwrap_or(&tx_data_hex)
.trim(),
)
.unwrap()
};
let tx = aurora_engine_transactions::legacy::TransactionLegacy {
nonce: U256::zero(),
gas_price: U256::zero(),
gas_limit: u64::MAX.into(),
to: None,
value: Wei::zero(),
data: tx_data,
};

let mut listener = CallTracer::default();
let standalone_result = sputnik::traced_call(&mut listener, || {
runner.submit_transaction(&signer.secret_key, tx)
});
assert!(
standalone_result.is_err(),
"Expected contract too large error"
);
}

#[allow(clippy::too_many_lines)]
#[test]
fn test_trace_precompiles_with_subcalls() {
Expand Down

0 comments on commit f57df5c

Please sign in to comment.