From b4ed8cf96bcba90b1d6dc2272e23d1021802d949 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Tue, 10 Dec 2024 15:59:52 +0000
Subject: [PATCH] Include the program counter in contract-reverted messages
 (#1593)

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.
---
 actors/evm/src/interpreter/instructions/control.rs   | 11 ++++++++---
 actors/evm/src/interpreter/instructions/lifecycle.rs |  3 ++-
 actors/evm/src/interpreter/instructions/mod.rs       |  2 +-
 actors/evm/src/interpreter/output.rs                 |  2 ++
 actors/evm/src/lib.rs                                |  2 +-
 5 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/actors/evm/src/interpreter/instructions/control.rs b/actors/evm/src/interpreter/instructions/control.rs
index 334d64838..246892792 100644
--- a/actors/evm/src/interpreter/instructions/control.rs
+++ b/actors/evm/src/interpreter/instructions/control.rs
@@ -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,
@@ -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,
     })
 }
 
diff --git a/actors/evm/src/interpreter/instructions/lifecycle.rs b/actors/evm/src/interpreter/instructions/lifecycle.rs
index cd2c14eae..b157ca24b 100644
--- a/actors/evm/src/interpreter/instructions/lifecycle.rs
+++ b/actors/evm/src/interpreter/instructions/lifecycle.rs
@@ -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;
@@ -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)]
diff --git a/actors/evm/src/interpreter/instructions/mod.rs b/actors/evm/src/interpreter/instructions/mod.rs
index b97b407e1..b9063200b 100644
--- a/actors/evm/src/interpreter/instructions/mod.rs
+++ b/actors/evm/src/interpreter/instructions/mod.rs
@@ -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(())
         }}
diff --git a/actors/evm/src/interpreter/output.rs b/actors/evm/src/interpreter/output.rs
index b9a1ae078..2d8220cfe 100644
--- a/actors/evm/src/interpreter/output.rs
+++ b/actors/evm/src/interpreter/output.rs
@@ -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,
 }
diff --git a/actors/evm/src/lib.rs b/actors/evm/src/lib.rs
index ad42fff70..cc87d09ef 100644
--- a/actors/evm/src/lib.rs
+++ b/actors/evm/src/lib.rs
@@ -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(),
         )),
     }