Skip to content

Commit

Permalink
Include the program counter in contract-reverted messages (#1593)
Browse files Browse the repository at this point in the history
This makes it possible to tell on which instruction the contract
reverted where as previously the message wasn't all that useful. The
runtime cost should be negligible.
  • Loading branch information
Stebalien authored Dec 10, 2024
1 parent 8ad9e28 commit b4ed8cf
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 6 deletions.
11 changes: 8 additions & 3 deletions actors/evm/src/interpreter/instructions/control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,33 +31,37 @@ pub fn invalid(
pub fn ret(
state: &mut ExecutionState,
_system: &System<impl Runtime>,
pc: usize,
offset: U256,
size: U256,
) -> Result<Output, ActorError> {
exit(&mut state.memory, offset, size, Outcome::Return)
exit(&mut state.memory, pc, offset, size, Outcome::Return)
}

#[inline]
pub fn revert(
state: &mut ExecutionState,
_system: &System<impl Runtime>,
pc: usize,
offset: U256,
size: U256,
) -> Result<Output, ActorError> {
exit(&mut state.memory, offset, size, Outcome::Revert)
exit(&mut state.memory, pc, offset, size, Outcome::Revert)
}

#[inline]
pub fn stop(
_state: &mut ExecutionState,
_system: &System<impl Runtime>,
pc: usize,
) -> Result<Output, ActorError> {
Ok(Output { return_data: Vec::new(), outcome: Outcome::Return })
Ok(Output { return_data: Vec::new(), outcome: Outcome::Return, pc })
}

#[inline]
fn exit(
memory: &mut Memory,
pc: usize,
offset: U256,
size: U256,
status: Outcome,
Expand All @@ -67,6 +71,7 @@ fn exit(
return_data: super::memory::get_memory_region(memory, offset, size)?
.map(|region| memory[region.offset..region.offset + region.size.get()].to_vec())
.unwrap_or_default(),
pc,
})
}

Expand Down
3 changes: 2 additions & 1 deletion actors/evm/src/interpreter/instructions/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ fn create_common(
pub fn selfdestruct(
_state: &mut ExecutionState,
system: &mut System<impl Runtime>,
pc: usize,
beneficiary: U256,
) -> Result<Output, ActorError> {
use crate::interpreter::output::Outcome;
Expand Down Expand Up @@ -185,7 +186,7 @@ pub fn selfdestruct(
//
// 1. In the constructor, this will set our code to "empty". This is correct.
// 2. Otherwise, we'll successfully return nothing to the caller.
Ok(Output { outcome: Outcome::Return, return_data: Vec::new() })
Ok(Output { outcome: Outcome::Return, return_data: Vec::new(), pc })
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion actors/evm/src/interpreter/instructions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ macro_rules! def_exit {
($op:ident ($($arg:ident),*) => $impl:path) => {
def_op!{ $op (m) => {
let &rev![$($arg),*] = m.state.stack.pop_many()?;
m.output = $impl(&mut m.state, &mut m.system, $($arg),*)?;
m.output = $impl(&mut m.state, &mut m.system, m.pc, $($arg),*)?;
m.pc = m.bytecode.len(); // stop execution
Ok(())
}}
Expand Down
2 changes: 2 additions & 0 deletions actors/evm/src/interpreter/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,6 @@ pub struct Output {
pub outcome: Outcome,
/// The return data.
pub return_data: Vec<u8>,
/// The final program counter (for debugging).
pub pc: usize,
}
2 changes: 1 addition & 1 deletion actors/evm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ where
}
Outcome::Revert => Err(ActorError::unchecked_with_data(
EVM_CONTRACT_REVERTED,
"contract reverted".to_string(),
format!("contract reverted at {0}", output.pc),
IpldBlock::serialize_cbor(&BytesSer(&output.return_data)).unwrap(),
)),
}
Expand Down

0 comments on commit b4ed8cf

Please sign in to comment.