feat: Always increment timestamp (#667)

openvm-org · Oct 24, 2024 · 836217b · 836217b
1 parent 07e3122
commit 836217b
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 45 deletions.
diff --git a/vm/src/kernels/core/bridge.rs b/vm/src/kernels/core/bridge.rs
@@ -1,7 +1,7 @@
 use afs_stark_backend::interaction::InteractionBuilder;
 use p3_field::AbstractField;
 
-use super::{columns::CoreIoCols, timestamp_delta, CoreAir};
+use super::{columns::CoreIoCols, CoreAir};
 use crate::{arch::ExecutionState, kernels::core::columns::CoreAuxCols};
 
 impl CoreAir {
@@ -11,29 +11,16 @@ impl CoreAir {
         io: &CoreIoCols<AB::Var>,
         aux: &CoreAuxCols<AB::Var>,
     ) {
-        let &CoreAuxCols {
-            is_valid,
-            ref operation_flags,
-        } = aux;
-
-        let next_pc = io.pc + AB::Expr::from_canonical_usize(1);
-
         self.execution_bridge
             .execute(
                 io.opcode + AB::Expr::from_canonical_usize(self.offset),
                 [io.a, io.b, io.c, io.d, io.e],
                 ExecutionState::new(io.pc, io.timestamp),
                 ExecutionState::<AB::Expr>::new(
-                    next_pc,
-                    io.timestamp
-                        + operation_flags
-                            .iter()
-                            .map(|(op, flag)| {
-                                AB::Expr::from_canonical_u32(timestamp_delta(*op)) * (*flag).into()
-                            })
-                            .fold(AB::Expr::zero(), |x, y| x + y),
+                    io.pc + AB::Expr::one(),
+                    io.timestamp + AB::Expr::one(),
                 ),
             )
-            .eval(builder, is_valid);
+            .eval(builder, aux.is_valid);
     }
 }
diff --git a/vm/src/kernels/core/execute.rs b/vm/src/kernels/core/execute.rs
@@ -3,7 +3,7 @@ use std::collections::BTreeMap;
 use p3_field::PrimeField32;
 use strum::IntoEnumIterator;
 
-use super::{timestamp_delta, CoreChip};
+use super::CoreChip;
 use crate::{
     arch::{
         instructions::{
@@ -22,7 +22,7 @@ impl<F: PrimeField32> InstructionExecutor<F> for CoreChip<F> {
         instruction: Instruction<F>,
         from_state: ExecutionState<u32>,
     ) -> Result<ExecutionState<u32>, ExecutionError> {
-        let ExecutionState { pc, mut timestamp } = from_state;
+        let ExecutionState { pc, timestamp } = from_state;
 
         let local_opcode_index = instruction.opcode - self.offset;
         let Instruction { a, b, c, d, e, .. } = instruction;
@@ -38,21 +38,12 @@ impl<F: PrimeField32> InstructionExecutor<F> for CoreChip<F> {
             e,
         };
 
-        macro_rules! read {
-            ($addr_space: expr, $pointer: expr) => {{
-                self.memory_controller
-                    .borrow_mut()
-                    .read_cell($addr_space, $pointer)
-                    .data[0]
-            }};
-        }
-
         let mut streams = self.streams.lock();
 
         let local_opcode_index = CoreOpcode::from_usize(local_opcode_index);
         match local_opcode_index {
             PRINTF => {
-                let value = read!(d, a);
+                let value = self.memory_controller.borrow().unsafe_read_cell(d, a);
                 println!("{}", value);
             }
             HINT_INPUT => {
@@ -100,7 +91,6 @@ impl<F: PrimeField32> InstructionExecutor<F> for CoreChip<F> {
             }
             _ => unreachable!(),
         };
-        timestamp += timestamp_delta(local_opcode_index);
 
         // TODO[zach]: Only collect a record of { from_state, instruction, read_records, write_records }
         // and move this logic into generate_trace().
@@ -122,7 +112,11 @@ impl<F: PrimeField32> InstructionExecutor<F> for CoreChip<F> {
             self.rows.push(cols.flatten());
         }
 
-        Ok(ExecutionState::new(pc + 1, timestamp))
+        self.memory_controller.borrow_mut().increment_timestamp();
+        Ok(ExecutionState::new(
+            pc + 1,
+            self.memory_controller.borrow().timestamp(),
+        ))
     }
 
     fn get_opcode_name(&self, opcode: usize) -> String {

diff --git a/vm/src/kernels/core/mod.rs b/vm/src/kernels/core/mod.rs
@@ -6,10 +6,7 @@ use p3_field::PrimeField32;
 use parking_lot::Mutex;
 
 use crate::{
-    arch::{
-        instructions::CoreOpcode::{self, *},
-        ExecutionBridge, ExecutionBus,
-    },
+    arch::{ExecutionBridge, ExecutionBus},
     system::{memory::MemoryControllerRef, program::ProgramBus, vm::Streams},
 };
 // TODO[zach]: Restore tests once we have control flow chip.
@@ -30,16 +27,6 @@ pub const POSEIDON2_DIRECT_BUS: usize = 6;
 pub const BYTE_XOR_BUS: XorBus = XorBus(8);
 pub const RANGE_TUPLE_CHECKER_BUS: usize = 11;
 
-fn timestamp_delta(opcode: CoreOpcode) -> u32 {
-    match opcode {
-        FAIL => 0,
-        PRINTF => 1,
-        HINT_INPUT | HINT_BITS | HINT_BYTES => 0,
-        CT_START | CT_END => 0,
-    }
-}
-
-/// Chip for the Core. Carries all state and owns execution.
 #[derive(Debug)]
 pub struct CoreChip<F: PrimeField32> {
     pub air: CoreAir,

diff --git a/vm/src/system/vm/segment.rs b/vm/src/system/vm/segment.rs
@@ -166,6 +166,7 @@ impl<F: PrimeField32> ExecutionSegment<F> {
                     instruction,
                     ExecutionState::new(pc, timestamp),
                 )?;
+                assert!(next_state.timestamp > timestamp);
                 if collect_metrics {
                     opcode_name = Some(executor.get_opcode_name(opcode));
                 }