fix: memory not added to checkpoint in unconstrained mode (#1436)

crates/core/executor/src/executor.rs

@@ -144,6 +144,10 @@ pub enum ExecutionError {
     /// The execution failed with an unimplemented feature.
     #[error("got unimplemented as opcode")]
     Unimplemented(),
+
+    /// The program ended in unconstrained mode.
+    #[error("program ended in unconstrained mode")]
+    EndInUnconstrained(),
 }
 
 macro_rules! assert_valid_memory_access {
@@ -253,7 +257,10 @@ impl<'a> Executor<'a> {
             let addr = Register::from_u32(i as u32) as u32;
             let record = self.state.memory.get(&addr);
 
-            if self.executor_mode != ExecutorMode::Simple {
+            // Only add the previous memory state to checkpoint map if we're in checkpoint mode,
+            // or if we're in unconstrained mode. In unconstrained mode, the mode is always
+            // Simple.
+            if self.executor_mode == ExecutorMode::Checkpoint || self.unconstrained {
                 match record {
                     Some(record) => {
                         self.memory_checkpoint.entry(addr).or_insert_with(|| Some(*record));
@@ -278,7 +285,7 @@ impl<'a> Executor<'a> {
         let addr = register as u32;
         let record = self.state.memory.get(&addr);
 
-        if self.executor_mode != ExecutorMode::Simple {
+        if self.executor_mode == ExecutorMode::Checkpoint || self.unconstrained {
             match record {
                 Some(record) => {
                     self.memory_checkpoint.entry(addr).or_insert_with(|| Some(*record));
@@ -301,7 +308,7 @@ impl<'a> Executor<'a> {
         #[allow(clippy::single_match_else)]
         let record = self.state.memory.get(&addr);
 
-        if self.executor_mode != ExecutorMode::Simple {
+        if self.executor_mode == ExecutorMode::Checkpoint || self.unconstrained {
             match record {
                 Some(record) => {
                     self.memory_checkpoint.entry(addr).or_insert_with(|| Some(*record));
@@ -349,7 +356,7 @@ impl<'a> Executor<'a> {
     pub fn mr(&mut self, addr: u32, shard: u32, timestamp: u32) -> MemoryReadRecord {
         // Get the memory record entry.
         let entry = self.state.memory.entry(addr);
-        if self.executor_mode != ExecutorMode::Simple {
+        if self.executor_mode == ExecutorMode::Checkpoint || self.unconstrained {
             match entry {
                 Entry::Occupied(ref entry) => {
                     let record = entry.get();
@@ -394,7 +401,7 @@ impl<'a> Executor<'a> {
     pub fn mw(&mut self, addr: u32, value: u32, shard: u32, timestamp: u32) -> MemoryWriteRecord {
         // Get the memory record entry.
         let entry = self.state.memory.entry(addr);
-        if self.executor_mode != ExecutorMode::Simple {
+        if self.executor_mode == ExecutorMode::Checkpoint || self.unconstrained {
             match entry {
                 Entry::Occupied(ref entry) => {
                     let record = entry.get();
@@ -686,7 +693,7 @@ impl<'a> Executor<'a> {
         let (addr, memory_read_value): (u32, u32);
         let mut memory_store_value: Option<u32> = None;
 
-        if self.executor_mode != ExecutorMode::Simple {
+        if self.executor_mode == ExecutorMode::Trace {
             self.memory_accesses = MemoryAccessRecord::default();
         }
         let lookup_id = if self.executor_mode == ExecutorMode::Simple {
@@ -1126,8 +1133,14 @@ impl<'a> Executor<'a> {
             }
         }
 
-        Ok(self.state.pc.wrapping_sub(self.program.pc_base)
-            >= (self.program.instructions.len() * 4) as u32)
+        let done = self.state.pc == 0
+            || self.state.pc.wrapping_sub(self.program.pc_base)
+                >= (self.program.instructions.len() * 4) as u32;
+        if done && self.unconstrained {
+            log::error!("program ended in unconstrained mode at clk {}", self.state.global_clk);
+            return Err(ExecutionError::EndInUnconstrained());
+        }
+        Ok(done)
     }
 
     /// Bump the record.
@@ -1163,7 +1176,7 @@ impl<'a> Executor<'a> {
         self.executor_mode = ExecutorMode::Checkpoint;
         self.print_report = true;
 
-        // Take memory out and then clone so that memory is not cloned.
+        // Take memory out of state before cloning it so that memory is not cloned.
         let memory = std::mem::take(&mut self.state.memory);
         let mut checkpoint = tracing::info_span!("clone").in_scope(|| self.state.clone());
         self.state.memory = memory;
@@ -1354,7 +1367,7 @@ impl<'a> Executor<'a> {
 
         self.report.touched_memory_addresses = self.state.memory.len() as u64;
         for addr in self.state.memory.keys() {
-            if addr == &0 {
+            if *addr == 0 {
                 // Handled above.
                 continue;
             }