chore: use explicit Result<_, ExecutionError> and pre_compute* returns StaticProgramError (#1898)

Remove `arch::execution::Result = Result<_, ExecutionError>` because our
`use execution::*` makes it a confusing `arch::Result` typedef.

Moreover all `pre_compute` functions should return `StaticProgramError`
to distinguish from `ExecutionError` which is for runtime errors.

Author: jonathanpwang

crates/circuits/mod-builder/src/core_chip.rs

@@ -8,12 +8,7 @@ use itertools::Itertools;
 use num_bigint::BigUint;
 use num_traits::Zero;
 use openvm_circuit::{
-    arch::{
-        get_record_from_slice, AdapterAirContext, AdapterCoreLayout, AdapterCoreMetadata,
-        AdapterTraceFiller, AdapterTraceStep, CustomBorrow, DynAdapterInterface, DynArray,
-        InstructionExecutor, MinimalInstruction, RecordArena, Result, SizedRecord, TraceFiller,
-        VmAdapterInterface, VmCoreAir, VmStateMut,
-    },
+    arch::*,
     system::memory::{online::TracingMemory, MemoryAuxColsFactory},
 };
 use openvm_circuit_primitives::{
@@ -395,7 +390,7 @@ where
         &mut self,
         state: VmStateMut<F, TracingMemory, RA>,
         instruction: &Instruction<F>,
-    ) -> Result<()> {
+    ) -> Result<(), ExecutionError> {
         let (mut adapter_record, mut core_record) = state.ctx.alloc(self.get_record_layout());
 
         A::start(*state.pc, state.memory, &mut adapter_record);

crates/vm/derive/src/lib.rs

@@ -57,7 +57,7 @@ pub fn instruction_executor_derive(input: TokenStream) -> TokenStream {
                         &mut self,
                         state: ::openvm_circuit::arch::VmStateMut<#field_ty_generic, ::openvm_circuit::system::memory::online::TracingMemory, RA>,
                         instruction: &::openvm_circuit::arch::instructions::instruction::Instruction<#field_ty_generic>,
-                    ) -> ::openvm_circuit::arch::Result<()> {
+                    ) -> Result<(), ::openvm_circuit::arch::ExecutionError> {
                         self.0.execute(state, instruction)
                     }
 
@@ -109,7 +109,7 @@ pub fn instruction_executor_derive(input: TokenStream) -> TokenStream {
                         &mut self,
                         state: ::openvm_circuit::arch::VmStateMut<#field_ty_generic, ::openvm_circuit::system::memory::online::TracingMemory, RA>,
                         instruction: &::openvm_circuit::arch::instructions::instruction::Instruction<#field_ty_generic>,
-                    ) -> ::openvm_circuit::arch::Result<()> {
+                    ) -> Result<(), ::openvm_circuit::arch::ExecutionError> {
                         match self {
                             #(#execute_arms,)*
                         }
@@ -167,7 +167,7 @@ pub fn ins_executor_e1_derive(input: TokenStream) -> TokenStream {
                         pc: u32,
                         inst: &::openvm_circuit::arch::instructions::instruction::Instruction<F>,
                         data: &mut [u8],
-                    ) -> ::openvm_circuit::arch::execution::Result<::openvm_circuit::arch::ExecuteFunc<F, Ctx>>
+                    ) -> Result<::openvm_circuit::arch::ExecuteFunc<F, Ctx>, ::openvm_circuit::arch::StaticProgramError>
                     where
                         Ctx: ::openvm_circuit::arch::execution_mode::E1ExecutionCtx, {
                         self.0.pre_compute_e1(pc, inst, data)
@@ -240,7 +240,7 @@ pub fn ins_executor_e1_derive(input: TokenStream) -> TokenStream {
                         pc: u32,
                         instruction: &::openvm_circuit::arch::instructions::instruction::Instruction<F>,
                         data: &mut [u8],
-                    ) -> ::openvm_circuit::arch::execution::Result<::openvm_circuit::arch::ExecuteFunc<F, Ctx>>
+                    ) -> Result<::openvm_circuit::arch::ExecuteFunc<F, Ctx>, ::openvm_circuit::arch::StaticProgramError>
                     where
                         Ctx: ::openvm_circuit::arch::execution_mode::E1ExecutionCtx, {
                         match self {
@@ -295,7 +295,7 @@ pub fn ins_executor_e2_derive(input: TokenStream) -> TokenStream {
                         pc: u32,
                         inst: &::openvm_circuit::arch::instructions::instruction::Instruction<F>,
                         data: &mut [u8],
-                    ) -> ::openvm_circuit::arch::execution::Result<::openvm_circuit::arch::ExecuteFunc<F, Ctx>>
+                    ) -> Result<::openvm_circuit::arch::ExecuteFunc<F, Ctx>, ::openvm_circuit::arch::StaticProgramError>
                     where
                         Ctx: ::openvm_circuit::arch::execution_mode::E2ExecutionCtx, {
                         self.0.pre_compute_e2(chip_idx, pc, inst, data)
@@ -369,7 +369,7 @@ pub fn ins_executor_e2_derive(input: TokenStream) -> TokenStream {
                         pc: u32,
                         instruction: &::openvm_circuit::arch::instructions::instruction::Instruction<F>,
                         data: &mut [u8],
-                    ) -> ::openvm_circuit::arch::execution::Result<::openvm_circuit::arch::ExecuteFunc<F, Ctx>>
+                    ) -> Result<::openvm_circuit::arch::ExecuteFunc<F, Ctx>, ::openvm_circuit::arch::StaticProgramError>
                     where
                         Ctx: ::openvm_circuit::arch::execution_mode::E2ExecutionCtx, {
                         match self {

crates/vm/src/arch/execution.rs

@@ -18,12 +18,10 @@ use crate::{
     arch::{execution_mode::E2ExecutionCtx, ExecutorInventoryError, MatrixRecordArena},
     system::{
         memory::online::{GuestMemory, TracingMemory},
-        program::{ProgramBus, StaticProgramError},
+        program::ProgramBus,
     },
 };
 
-pub type Result<T> = std::result::Result<T, ExecutionError>;
-
 #[derive(Error, Debug)]
 pub enum ExecutionError {
     #[error("execution failed at pc {pc}")]
@@ -72,9 +70,17 @@ pub enum ExecutionError {
     Inventory(#[from] ExecutorInventoryError),
     #[error("static program error: {0}")]
     Static(#[from] StaticProgramError),
-    // TODO[jpw]: this should be in StaticProgramError
+}
+
+/// Errors in the program that can be statically analyzed before runtime.
+#[derive(Error, Debug)]
+pub enum StaticProgramError {
     #[error("invalid instruction at pc {0}")]
     InvalidInstruction(u32),
+    #[error("Too many executors")]
+    TooManyExecutors,
+    #[error("Executor not found for opcode {opcode}")]
+    ExecutorNotFound { opcode: VmOpcode },
 }
 
 /// Global VM state accessible during instruction execution.
@@ -99,7 +105,7 @@ pub trait InstructionExecutor<F, RA = MatrixRecordArena<F>>: Clone {
         &mut self,
         state: VmStateMut<F, TracingMemory, RA>,
         instruction: &Instruction<F>,
-    ) -> Result<()>;
+    ) -> Result<(), ExecutionError>;
 
     /// For display purposes. From absolute opcode as `usize`, return the string name of the opcode
     /// if it is a supported opcode by the present executor.
@@ -129,7 +135,7 @@ pub trait InsExecutorE1<F> {
         pc: u32,
         inst: &Instruction<F>,
         data: &mut [u8],
-    ) -> Result<ExecuteFunc<F, Ctx>>
+    ) -> Result<ExecuteFunc<F, Ctx>, StaticProgramError>
     where
         Ctx: E1ExecutionCtx;
 }
@@ -143,7 +149,7 @@ pub trait InsExecutorE2<F> {
         pc: u32,
         inst: &Instruction<F>,
         data: &mut [u8],
-    ) -> Result<ExecuteFunc<F, Ctx>>
+    ) -> Result<ExecuteFunc<F, Ctx>, StaticProgramError>
     where
         Ctx: E2ExecutionCtx;
 }

crates/vm/src/arch/interpreter.rs

@@ -21,8 +21,8 @@ use crate::{
         create_memory_image,
         execution_mode::{E1ExecutionCtx, E2ExecutionCtx},
         ExecuteFunc, ExecutionError, ExecutorInventory, ExecutorInventoryError, ExitCode,
-        InsExecutorE1, InsExecutorE2, PreComputeInstruction, Streams, SystemConfig,
-        VmExecutionConfig, VmSegmentState,
+        InsExecutorE1, InsExecutorE2, PreComputeInstruction, StaticProgramError, Streams,
+        SystemConfig, VmExecutionConfig, VmSegmentState,
     },
     system::memory::online::GuestMemory,
 };
@@ -386,7 +386,8 @@ fn get_pre_compute_instructions<'a, F: PrimeField32, E: InsExecutorE1<F>, Ctx: E
             } else {
                 PreComputeInstruction {
                     handler: |_, vm_state| {
-                        vm_state.exit_code = Err(ExecutionError::InvalidInstruction(vm_state.pc));
+                        vm_state.exit_code =
+                            Err(StaticProgramError::InvalidInstruction(vm_state.pc).into());
                     },
                     pre_compute: buf,
                 }
@@ -442,7 +443,8 @@ fn get_e2_pre_compute_instructions<
             } else {
                 PreComputeInstruction {
                     handler: |_, vm_state| {
-                        vm_state.exit_code = Err(ExecutionError::InvalidInstruction(vm_state.pc));
+                        vm_state.exit_code =
+                            Err(StaticProgramError::InvalidInstruction(vm_state.pc).into());
                     },
                     pre_compute: buf,
                 }

crates/vm/src/arch/mod.rs

@@ -2,6 +2,7 @@ mod config;
 /// Instruction execution traits and types.
 /// Execution bus and interface.
 pub mod execution;
+/// Execution context types for different execution modes.
 pub mod execution_mode;
 /// Traits and builders to compose collections of chips into a virtual machine.
 mod extensions;
@@ -27,6 +28,7 @@ pub mod testing;
 
 pub use config::*;
 pub use execution::*;
+pub use execution_mode::{E1ExecutionCtx, E2ExecutionCtx};
 pub use extensions::*;
 pub use integration_api::*;
 pub use openvm_instructions as instructions;

crates/vm/src/system/phantom/execution.rs

@@ -11,7 +11,7 @@ use crate::{
     arch::{
         execution_mode::{E1ExecutionCtx, E2ExecutionCtx},
         E2PreCompute, ExecuteFunc, ExecutionError, InsExecutorE1, InsExecutorE2,
-        PhantomSubExecutor, Streams, VmSegmentState,
+        PhantomSubExecutor, StaticProgramError, Streams, VmSegmentState,
     },
     system::{memory::online::GuestMemory, phantom::PhantomExecutor},
 };
@@ -45,7 +45,7 @@ where
         _pc: u32,
         inst: &Instruction<F>,
         data: &mut [u8],
-    ) -> Result<ExecuteFunc<F, Ctx>, ExecutionError>
+    ) -> Result<ExecuteFunc<F, Ctx>, StaticProgramError>
     where
         Ctx: E1ExecutionCtx,
     {
@@ -177,7 +177,7 @@ where
         _pc: u32,
         inst: &Instruction<F>,
         data: &mut [u8],
-    ) -> Result<ExecuteFunc<F, Ctx>, ExecutionError>
+    ) -> Result<ExecuteFunc<F, Ctx>, StaticProgramError>
     where
         Ctx: E2ExecutionCtx,
     {

crates/vm/src/system/program/mod.rs

@@ -1,17 +1,16 @@
 use openvm_instructions::{
     instruction::Instruction,
     program::{Program, DEFAULT_PC_STEP},
-    LocalOpcode, SystemOpcode, VmOpcode,
+    LocalOpcode, SystemOpcode,
 };
 use openvm_stark_backend::{
     config::StarkGenericConfig,
     p3_field::Field,
     p3_maybe_rayon::prelude::*,
     prover::{cpu::CpuBackend, types::CommittedTraceData},
 };
-use thiserror::Error;
 
-use crate::arch::{ExecutionError, ExecutorId, ExecutorInventory};
+use crate::arch::{ExecutionError, ExecutorId, ExecutorInventory, StaticProgramError};
 
 #[cfg(test)]
 pub mod tests;
@@ -159,15 +158,6 @@ impl<F: Field, E> ProgramHandler<F, E> {
     }
 }
 
-/// Errors in the program that can be statically analyzed before runtime.
-#[derive(Error, Debug)]
-pub enum StaticProgramError {
-    #[error("Too many executors")]
-    TooManyExecutors,
-    #[error("Executor not found for opcode {opcode}")]
-    ExecutorNotFound { opcode: VmOpcode },
-}
-
 // For CPU backend only
 pub struct ProgramChip<SC: StarkGenericConfig> {
     /// `i` -> frequency of instruction in `i`th row of trace matrix. This requires filtering

crates/vm/src/system/public_values/core.rs

@@ -25,7 +25,7 @@ use crate::{
         get_record_from_slice, AdapterAirContext, AdapterTraceFiller, AdapterTraceStep,
         BasicAdapterInterface, E2PreCompute, EmptyAdapterCoreLayout, ExecuteFunc, ExecutionError,
         InsExecutorE1, InsExecutorE2, InstructionExecutor, MinimalInstruction, RecordArena,
-        TraceFiller, VmCoreAir, VmSegmentState, VmStateMut,
+        StaticProgramError, TraceFiller, VmCoreAir, VmSegmentState, VmStateMut,
     },
     system::{
         memory::{
@@ -267,7 +267,7 @@ where
         _pc: u32,
         inst: &Instruction<F>,
         data: &mut [u8],
-    ) -> Result<ExecuteFunc<F, Ctx>, ExecutionError>
+    ) -> Result<ExecuteFunc<F, Ctx>, StaticProgramError>
     where
         Ctx: E1ExecutionCtx,
     {
@@ -298,7 +298,7 @@ where
         _pc: u32,
         inst: &Instruction<F>,
         data: &mut [u8],
-    ) -> Result<ExecuteFunc<F, Ctx>, ExecutionError>
+    ) -> Result<ExecuteFunc<F, Ctx>, StaticProgramError>
     where
         Ctx: E2ExecutionCtx,
     {

extensions/algebra/circuit/src/lib.rs

@@ -7,18 +7,16 @@ use derive_more::derive::{Deref, DerefMut};
 use num_bigint::BigUint;
 use openvm_algebra_transpiler::{Fp2Opcode, Rv32ModularArithmeticOpcode};
 use openvm_circuit::{
-    arch::{
-        execution::ExecuteFunc,
-        execution_mode::{E1ExecutionCtx, E2ExecutionCtx},
-        instructions::riscv::{RV32_MEMORY_AS, RV32_REGISTER_AS},
-        DynArray, E2PreCompute, ExecutionError, InsExecutorE1, InsExecutorE2, Result,
-        VmSegmentState,
-    },
+    arch::*,
     system::memory::{online::GuestMemory, POINTER_MAX_BITS},
 };
 use openvm_circuit_derive::InstructionExecutor;
 use openvm_circuit_primitives::AlignedBytesBorrow;
-use openvm_instructions::{instruction::Instruction, program::DEFAULT_PC_STEP};
+use openvm_instructions::{
+    instruction::Instruction,
+    program::DEFAULT_PC_STEP,
+    riscv::{RV32_MEMORY_AS, RV32_REGISTER_AS},
+};
 use openvm_mod_circuit_builder::{
     run_field_expression_precomputed, FieldExpr, FieldExpressionStep,
 };
@@ -117,7 +115,7 @@ impl<'a, const BLOCKS: usize, const BLOCK_SIZE: usize, const IS_FP2: bool>
         pc: u32,
         inst: &Instruction<F>,
         data: &mut FieldExpressionPreCompute<'a>,
-    ) -> Result<Option<Operation>> {
+    ) -> Result<Option<Operation>, StaticProgramError> {
         let Instruction {
             opcode,
             a,
@@ -134,7 +132,7 @@ impl<'a, const BLOCKS: usize, const BLOCK_SIZE: usize, const IS_FP2: bool>
         let d = d.as_canonical_u32();
         let e = e.as_canonical_u32();
         if d != RV32_REGISTER_AS || e != RV32_MEMORY_AS {
-            return Err(ExecutionError::InvalidInstruction(pc));
+            return Err(StaticProgramError::InvalidInstruction(pc));
         }
 
         let local_opcode = opcode.local_opcode_idx(self.0.offset);
@@ -213,7 +211,7 @@ impl<F: PrimeField32, const BLOCKS: usize, const BLOCK_SIZE: usize, const IS_FP2
         pc: u32,
         inst: &Instruction<F>,
         data: &mut [u8],
-    ) -> Result<ExecuteFunc<F, Ctx>>
+    ) -> Result<ExecuteFunc<F, Ctx>, StaticProgramError>
     where
         Ctx: E1ExecutionCtx,
     {
@@ -310,7 +308,7 @@ impl<F: PrimeField32, const BLOCKS: usize, const BLOCK_SIZE: usize, const IS_FP2
         pc: u32,
         inst: &Instruction<F>,
         data: &mut [u8],
-    ) -> Result<ExecuteFunc<F, Ctx>>
+    ) -> Result<ExecuteFunc<F, Ctx>, StaticProgramError>
     where
         Ctx: E2ExecutionCtx,
     {