diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f74d7c517..66f2006f3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Added `procedure_names` to `DebugInfo` for storing procedure name mappings by MAST root digest, enabling debuggers to resolve human-readable procedure names during execution (#[2474](https://github.com/0xMiden/miden-vm/pull/2474)). +#### Fixes + +- `FastProcessor` now correctly returns an error if the maximum number of cycles was exceeded during execution ([#2537](https://github.com/0xMiden/miden-vm/pull/2537)) + #### Changes - Added `--kernel` flag to CLI commands (`run`, `prove`, `verify`, `debug`) to allow loading custom kernels from `.masm` or `.masp` files ([#2363](https://github.com/0xMiden/miden-vm/pull/2363)). diff --git a/air/src/errors.rs b/air/src/errors.rs index 35ee149711..e75e525a38 100644 --- a/air/src/errors.rs +++ b/air/src/errors.rs @@ -19,6 +19,4 @@ pub enum ExecutionOptionsError { InvalidHashFunction { hash_function: String }, #[error("core trace fragment size must be greater than 0")] CoreTraceFragmentSizeTooSmall, - #[error("core trace fragment size {0} must be a power of two")] - CoreTraceFragmentSizeNotPowerOfTwo(usize), } diff --git a/air/src/options.rs b/air/src/options.rs index 68554d46bd..666932a6f6 100644 --- a/air/src/options.rs +++ b/air/src/options.rs @@ -115,7 +115,7 @@ impl ExecutionOptions { /// Returns an error if: /// - `max_cycles` is outside the valid range /// - `expected_cycles` exceeds `max_cycles` - /// - `core_trace_fragment_size` is zero or not a power of two + /// - `core_trace_fragment_size` is zero pub fn new( max_cycles: Option, expected_cycles: u32, @@ -156,11 +156,6 @@ impl ExecutionOptions { if core_trace_fragment_size == 0 { return Err(ExecutionOptionsError::CoreTraceFragmentSizeTooSmall); } - if !core_trace_fragment_size.is_power_of_two() { - return Err(ExecutionOptionsError::CoreTraceFragmentSizeNotPowerOfTwo( - core_trace_fragment_size, - )); - } Ok(ExecutionOptions { max_cycles, @@ -173,7 +168,7 @@ impl ExecutionOptions { /// Sets the fragment size for core trace generation. /// - /// Returns an error if the size is zero or not a power of two. + /// Returns an error if the size is zero. pub fn with_core_trace_fragment_size( mut self, size: usize, @@ -181,9 +176,6 @@ impl ExecutionOptions { if size == 0 { return Err(ExecutionOptionsError::CoreTraceFragmentSizeTooSmall); } - if !size.is_power_of_two() { - return Err(ExecutionOptionsError::CoreTraceFragmentSizeNotPowerOfTwo(size)); - } self.core_trace_fragment_size = size; Ok(self) } @@ -266,21 +258,6 @@ mod tests { assert!(matches!(opts, Err(ExecutionOptionsError::CoreTraceFragmentSizeTooSmall))); } - #[test] - fn non_power_of_two_fragment_size_fails() { - let opts = ExecutionOptions::new(None, 64, 1000, false, false); - assert!(matches!( - opts, - Err(ExecutionOptionsError::CoreTraceFragmentSizeNotPowerOfTwo(1000)) - )); - - let opts = ExecutionOptions::new(None, 64, 3, false, false); - assert!(matches!( - opts, - Err(ExecutionOptionsError::CoreTraceFragmentSizeNotPowerOfTwo(3)) - )); - } - #[test] fn with_core_trace_fragment_size_validates() { // Valid size should succeed @@ -291,12 +268,5 @@ mod tests { // Zero should fail let result = ExecutionOptions::default().with_core_trace_fragment_size(0); assert!(matches!(result, Err(ExecutionOptionsError::CoreTraceFragmentSizeTooSmall))); - - // Non-power-of-two should fail - let result = ExecutionOptions::default().with_core_trace_fragment_size(100); - assert!(matches!( - result, - Err(ExecutionOptionsError::CoreTraceFragmentSizeNotPowerOfTwo(100)) - )); } } diff --git a/crates/test-utils/src/lib.rs b/crates/test-utils/src/lib.rs index d38eee944b..b18f964563 100644 --- a/crates/test-utils/src/lib.rs +++ b/crates/test-utils/src/lib.rs @@ -387,12 +387,15 @@ impl Test { let fast_stack_result = { let stack_inputs: Vec = self.stack_inputs.clone().into_iter().rev().collect(); let advice_inputs: AdviceInputs = self.advice_inputs.clone(); - let fast_processor = if self.in_debug_mode { - FastProcessor::new_debug(&stack_inputs, advice_inputs) - } else { - FastProcessor::new_with_advice_inputs(&stack_inputs, advice_inputs) - }; - fast_processor.execute_for_trace_sync(&program, &mut host, FRAGMENT_SIZE) + let fast_processor = FastProcessor::new_with_options( + &stack_inputs, + advice_inputs, + miden_air::ExecutionOptions::default() + .with_debugging(self.in_debug_mode) + .with_core_trace_fragment_size(FRAGMENT_SIZE) + .unwrap(), + ); + fast_processor.execute_for_trace_sync(&program, &mut host) }; // compare fast and slow processors' stack outputs diff --git a/miden-vm/benches/build_trace.rs b/miden-vm/benches/build_trace.rs index c5840f931c..61e2cf2f27 100644 --- a/miden-vm/benches/build_trace.rs +++ b/miden-vm/benches/build_trace.rs @@ -65,18 +65,19 @@ fn build_trace(c: &mut Criterion) { .with_library(&CoreLibrary::default()) .unwrap(); - let processor = FastProcessor::new_with_advice_inputs( + let processor = FastProcessor::new_with_options( &stack_inputs, advice_inputs.clone(), + ExecutionOptions::default() + .with_core_trace_fragment_size(TRACE_FRAGMENT_SIZE) + .unwrap(), ); (host, program.clone(), processor) }, |(mut host, program, processor)| async move { - let (execution_output, trace_generation_context) = processor - .execute_for_trace(&program, &mut host, TRACE_FRAGMENT_SIZE) - .await - .unwrap(); + let (execution_output, trace_generation_context) = + processor.execute_for_trace(&program, &mut host).await.unwrap(); let trace = parallel::build_trace( execution_output, diff --git a/miden-vm/benches/program_execution_for_trace.rs b/miden-vm/benches/program_execution_for_trace.rs index f0cf123f2f..540dae2524 100644 --- a/miden-vm/benches/program_execution_for_trace.rs +++ b/miden-vm/benches/program_execution_for_trace.rs @@ -2,7 +2,7 @@ use std::hint::black_box; use criterion::{BatchSize, Criterion, criterion_group, criterion_main}; use miden_core_lib::CoreLibrary; -use miden_processor::{AdviceInputs, fast::FastProcessor}; +use miden_processor::{AdviceInputs, ExecutionOptions, fast::FastProcessor}; use miden_vm::{Assembler, DefaultHost, StackInputs, internal::InputFile}; use tokio::runtime::Runtime; use walkdir::WalkDir; @@ -61,18 +61,19 @@ fn program_execution_for_trace(c: &mut Criterion) { .with_library(&CoreLibrary::default()) .unwrap(); - let processor = FastProcessor::new_with_advice_inputs( + let processor = FastProcessor::new_with_options( &stack_inputs, advice_inputs.clone(), + ExecutionOptions::default() + .with_core_trace_fragment_size(TRACE_FRAGMENT_SIZE) + .unwrap(), ); (host, program.clone(), processor) }, |(mut host, program, processor)| async move { - let out = processor - .execute_for_trace(&program, &mut host, TRACE_FRAGMENT_SIZE) - .await - .unwrap(); + let out = + processor.execute_for_trace(&program, &mut host).await.unwrap(); black_box(out); }, BatchSize::SmallInput, diff --git a/miden-vm/src/cli/run.rs b/miden-vm/src/cli/run.rs index 9a47314167..b3ccb353fe 100644 --- a/miden-vm/src/cli/run.rs +++ b/miden-vm/src/cli/run.rs @@ -6,7 +6,7 @@ use miden_core_lib::CoreLibrary; use miden_processor::{ DefaultHost, ExecutionOptions, ExecutionTrace, fast::FastProcessor, parallel::build_trace, }; -use miden_vm::{DEFAULT_CORE_TRACE_FRAGMENT_SIZE, internal::InputFile}; +use miden_vm::internal::InputFile; use tracing::instrument; use super::{ @@ -154,7 +154,7 @@ fn run_masp_program(params: &RunCmd) -> Result<(ExecutionTrace, [u8; 32]), Repor }; let (execution_output, trace_generation_context) = processor - .execute_for_trace_sync(&program, &mut host, DEFAULT_CORE_TRACE_FRAGMENT_SIZE) + .execute_for_trace_sync(&program, &mut host) .wrap_err("Failed to execute program")?; let trace = build_trace( @@ -225,7 +225,7 @@ fn run_masm_program(params: &RunCmd) -> Result<(ExecutionTrace, [u8; 32]), Repor }; let (execution_output, trace_generation_context) = processor - .execute_for_trace_sync(&program, &mut host, DEFAULT_CORE_TRACE_FRAGMENT_SIZE) + .execute_for_trace_sync(&program, &mut host) .wrap_err("Failed to execute program")?; let trace = build_trace( diff --git a/miden-vm/tests/integration/prove_verify.rs b/miden-vm/tests/integration/prove_verify.rs index bca9730791..924392306f 100644 --- a/miden-vm/tests/integration/prove_verify.rs +++ b/miden-vm/tests/integration/prove_verify.rs @@ -201,7 +201,9 @@ mod fast_parallel { use miden_assembly::{Assembler, DefaultSourceManager}; use miden_core::Felt; - use miden_processor::{AdviceInputs, StackInputs, fast::FastProcessor, parallel::build_trace}; + use miden_processor::{ + AdviceInputs, ExecutionOptions, StackInputs, fast::FastProcessor, parallel::build_trace, + }; use miden_prover::{ ExecutionProof, HashFunction, ProcessorAir, config, execution_trace_to_row_major, stark, }; @@ -237,10 +239,15 @@ mod fast_parallel { // Convert stack inputs for fast processor (reversed order) let stack_inputs_vec: Vec = stack_inputs.clone().into_iter().rev().collect(); - let fast_processor = - FastProcessor::new_with_advice_inputs(&stack_inputs_vec, advice_inputs.clone()); + let fast_processor = FastProcessor::new_with_options( + &stack_inputs_vec, + advice_inputs.clone(), + ExecutionOptions::default() + .with_core_trace_fragment_size(FRAGMENT_SIZE) + .unwrap(), + ); let (execution_output, trace_context) = fast_processor - .execute_for_trace_sync(&program, &mut host, FRAGMENT_SIZE) + .execute_for_trace_sync(&program, &mut host) .expect("Fast processor execution failed"); let fast_stack_outputs = execution_output.stack.clone(); diff --git a/processor/src/chiplets/tests.rs b/processor/src/chiplets/tests.rs index 9c3fce4300..707fdb778b 100644 --- a/processor/src/chiplets/tests.rs +++ b/processor/src/chiplets/tests.rs @@ -1,13 +1,16 @@ use alloc::vec::Vec; -use miden_air::trace::{ - CHIPLETS_RANGE, CHIPLETS_WIDTH, - chiplets::{ - NUM_BITWISE_SELECTORS, NUM_KERNEL_ROM_SELECTORS, NUM_MEMORY_SELECTORS, - bitwise::{BITWISE_XOR, OP_CYCLE_LEN, TRACE_WIDTH as BITWISE_TRACE_WIDTH}, - hasher::{HASH_CYCLE_LEN, LINEAR_HASH, RETURN_STATE}, - kernel_rom::TRACE_WIDTH as KERNEL_ROM_TRACE_WIDTH, - memory::TRACE_WIDTH as MEMORY_TRACE_WIDTH, +use miden_air::{ + ExecutionOptions, + trace::{ + CHIPLETS_RANGE, CHIPLETS_WIDTH, + chiplets::{ + NUM_BITWISE_SELECTORS, NUM_KERNEL_ROM_SELECTORS, NUM_MEMORY_SELECTORS, + bitwise::{BITWISE_XOR, OP_CYCLE_LEN, TRACE_WIDTH as BITWISE_TRACE_WIDTH}, + hasher::{HASH_CYCLE_LEN, LINEAR_HASH, RETURN_STATE}, + kernel_rom::TRACE_WIDTH as KERNEL_ROM_TRACE_WIDTH, + memory::TRACE_WIDTH as MEMORY_TRACE_WIDTH, + }, }, }; use miden_core::{ @@ -15,7 +18,7 @@ use miden_core::{ mast::{BasicBlockNodeBuilder, MastForest, MastForestContributor}, }; -use crate::{DefaultHost, Kernel, Operation, fast::FastProcessor}; +use crate::{AdviceInputs, DefaultHost, Kernel, Operation, fast::FastProcessor}; type ChipletsTrace = [Vec; CHIPLETS_WIDTH]; @@ -117,7 +120,11 @@ fn build_trace( kernel: Kernel, ) -> (ChipletsTrace, usize) { let stack_inputs: Vec = stack_inputs.iter().map(|v| Felt::new(*v)).collect(); - let processor = FastProcessor::new(&stack_inputs); + let processor = FastProcessor::new_with_options( + &stack_inputs, + AdviceInputs::default(), + ExecutionOptions::default().with_core_trace_fragment_size(1 << 10).unwrap(), + ); let mut host = DefaultHost::default(); let program = { @@ -132,7 +139,7 @@ fn build_trace( }; let (execution_output, trace_generation_context) = - processor.execute_for_trace_sync(&program, &mut host, 1 << 10).unwrap(); + processor.execute_for_trace_sync(&program, &mut host).unwrap(); let trace = crate::parallel::build_trace( execution_output, trace_generation_context, diff --git a/processor/src/fast/basic_block.rs b/processor/src/fast/basic_block.rs index 360eb4d5e7..5d7dc09e7a 100644 --- a/processor/src/fast/basic_block.rs +++ b/processor/src/fast/basic_block.rs @@ -44,12 +44,12 @@ impl FastProcessor { self.execute_before_enter_decorators(node_id, current_forest, host)?; // Corresponds to the row inserted for the BASIC BLOCK operation added to the trace. - self.increment_clk(tracer, stopper).map_break(|_| { - BreakReason::Stopped(Some(Continuation::ResumeBasicBlock { + self.increment_clk_with_continuation(tracer, stopper, || { + Some(Continuation::ResumeBasicBlock { node_id, batch_index: 0, op_idx_in_batch: 0, - })) + }) })?; // execute first batch @@ -117,17 +117,17 @@ impl FastProcessor { // Corresponds to the RESPAN operation added to the trace. // - // Note: in `map_break()`, the continuation encodes resuming from the start of the - // batch *after* the RESPAN operation. This is because the continuation encodes what - // happens *after* the clock is incremented. In other words, if we were to put a - // `Continuation::Respan` here instead, the next call to `FastProcessor::step()` - // would re-execute the RESPAN (over, and over). - self.increment_clk(tracer, stopper).map_break(|_| { - BreakReason::Stopped(Some(Continuation::ResumeBasicBlock { + // Note: in the continuation closure, the continuation encodes resuming from the + // start of the batch *after* the RESPAN operation. This is because the continuation + // encodes what happens *after* the clock is incremented. In other words, if we were + // to put a `Continuation::Respan` here instead, the next call to + // `FastProcessor::step()` would re-execute the RESPAN (over, and over). + self.increment_clk_with_continuation(tracer, stopper, || { + Some(Continuation::ResumeBasicBlock { node_id, batch_index, op_idx_in_batch: 0, - })) + }) })?; } @@ -232,7 +232,7 @@ impl FastProcessor { for (op_idx_in_batch, op) in batch.ops().iter().enumerate().skip(start_op_idx) { let op_idx_in_block = batch_offset_in_block + op_idx_in_batch; - if self.in_debug_mode { + if self.should_execute_decorators() { #[cfg(test)] self.record_decorator_retrieval(); @@ -257,7 +257,7 @@ impl FastProcessor { // performance improvement). { let err_ctx = - err_ctx!(current_forest, node_id, host, self.in_debug_mode, op_idx_in_block); + err_ctx!(current_forest, node_id, host, self.in_debug_mode(), op_idx_in_block); match op { Operation::Emit => self.op_emit(host, &err_ctx).await?, _ => { @@ -271,15 +271,13 @@ impl FastProcessor { } } - self.increment_clk(tracer, stopper).map_break(|_| { - let continuation = get_continuation_after_executing_operation( + self.increment_clk_with_continuation(tracer, stopper, || { + Some(get_continuation_after_executing_operation( basic_block, node_id, batch_index, op_idx_in_batch, - ); - - BreakReason::Stopped(Some(continuation)) + )) })?; } @@ -306,8 +304,8 @@ impl FastProcessor { ); // Corresponds to the row inserted for the END operation added to the trace. - self.increment_clk(tracer, stopper).map_break(|_| { - BreakReason::Stopped(Some(Continuation::AfterExitDecoratorsBasicBlock(node_id))) + self.increment_clk_with_continuation(tracer, stopper, || { + Some(Continuation::AfterExitDecoratorsBasicBlock(node_id)) })?; self.execute_end_of_block_decorators(basic_block_node, node_id, current_forest, host)?; @@ -327,7 +325,7 @@ impl FastProcessor { current_forest: &Arc, host: &mut impl AsyncHost, ) -> ControlFlow { - if self.in_debug_mode { + if self.should_execute_decorators() { #[cfg(test)] self.record_decorator_retrieval(); diff --git a/processor/src/fast/call_and_dyn.rs b/processor/src/fast/call_and_dyn.rs index 49c3465555..3c2445cebe 100644 --- a/processor/src/fast/call_and_dyn.rs +++ b/processor/src/fast/call_and_dyn.rs @@ -43,7 +43,7 @@ impl FastProcessor { // Execute decorators that should be executed before entering the node self.execute_before_enter_decorators(current_node_id, current_forest, host)?; - let err_ctx = err_ctx!(current_forest, current_node_id, host, self.in_debug_mode); + let err_ctx = err_ctx!(current_forest, current_node_id, host, self.in_debug_mode()); let callee_hash = current_forest[call_node.callee()].digest(); @@ -81,7 +81,7 @@ impl FastProcessor { continuation_stack.push_start_node(call_node.callee()); // Corresponds to the CALL or SYSCALL operation added to the trace. - self.increment_clk(tracer, stopper).map_break(BreakReason::stopped) + self.increment_clk(tracer, stopper) } /// Executes the finish phase of a Call node. @@ -107,7 +107,7 @@ impl FastProcessor { // so we prefix it with underscore to indicate this intentional unused state // and suppress warnings in feature combinations that include `no_err_ctx`. let _call_node = current_forest[node_id].unwrap_call(); - let err_ctx = err_ctx!(current_forest, node_id, host, self.in_debug_mode); + let err_ctx = err_ctx!(current_forest, node_id, host, self.in_debug_mode()); // when returning from a function call or a syscall, restore the // context of the // system registers and the operand stack to what it was prior @@ -115,8 +115,8 @@ impl FastProcessor { self.restore_context(tracer, &err_ctx)?; // Corresponds to the row inserted for the END operation added to the trace. - self.increment_clk(tracer, stopper).map_break(|_| { - BreakReason::Stopped(Some(Continuation::AfterExitDecorators(node_id))) + self.increment_clk_with_continuation(tracer, stopper, || { + Some(Continuation::AfterExitDecorators(node_id)) })?; self.execute_after_exit_decorators(node_id, current_forest, host) @@ -147,7 +147,7 @@ impl FastProcessor { // added to the trace. let dyn_node = current_forest[current_node_id].unwrap_dyn(); - let err_ctx = err_ctx!(¤t_forest, current_node_id, host, self.in_debug_mode); + let err_ctx = err_ctx!(¤t_forest, current_node_id, host, self.in_debug_mode()); // Retrieve callee hash from memory, using stack top as the memory // address. @@ -226,7 +226,7 @@ impl FastProcessor { // Increment the clock, corresponding to the row inserted for the DYN or DYNCALL operation // added to the trace. - self.increment_clk(tracer, stopper).map_break(BreakReason::stopped) + self.increment_clk(tracer, stopper) } /// Executes the finish phase of a Dyn node. @@ -248,7 +248,7 @@ impl FastProcessor { ); let dyn_node = current_forest[node_id].unwrap_dyn(); - let err_ctx = err_ctx!(current_forest, node_id, host, self.in_debug_mode); + let err_ctx = err_ctx!(current_forest, node_id, host, self.in_debug_mode()); // For dyncall, restore the context. if dyn_node.is_dyncall() { self.restore_context(tracer, &err_ctx)?; @@ -256,8 +256,8 @@ impl FastProcessor { // Corresponds to the row inserted for the END operation added to // the trace. - self.increment_clk(tracer, stopper).map_break(|_| { - BreakReason::Stopped(Some(Continuation::AfterExitDecorators(node_id))) + self.increment_clk_with_continuation(tracer, stopper, || { + Some(Continuation::AfterExitDecorators(node_id)) })?; self.execute_after_exit_decorators(node_id, current_forest, host) diff --git a/processor/src/fast/join.rs b/processor/src/fast/join.rs index 42cd7281d5..1b4487b588 100644 --- a/processor/src/fast/join.rs +++ b/processor/src/fast/join.rs @@ -38,7 +38,7 @@ impl FastProcessor { // Corresponds to the row inserted for the JOIN operation added // to the trace. - self.increment_clk(tracer, stopper).map_break(BreakReason::stopped) + self.increment_clk(tracer, stopper) } /// Executes the finish phase of a Join node. @@ -61,8 +61,8 @@ impl FastProcessor { // Corresponds to the row inserted for the END operation added // to the trace. - self.increment_clk(tracer, stopper).map_break(|_| { - BreakReason::Stopped(Some(Continuation::AfterExitDecorators(node_id))) + self.increment_clk_with_continuation(tracer, stopper, || { + Some(Continuation::AfterExitDecorators(node_id)) })?; self.execute_after_exit_decorators(node_id, current_forest, host) diff --git a/processor/src/fast/loop.rs b/processor/src/fast/loop.rs index b202933caf..6898151988 100644 --- a/processor/src/fast/loop.rs +++ b/processor/src/fast/loop.rs @@ -50,14 +50,14 @@ impl FastProcessor { // Corresponds to the row inserted for the LOOP operation added // to the trace. - self.increment_clk(tracer, stopper).map_break(BreakReason::stopped) + self.increment_clk(tracer, stopper) } else if condition == ZERO { // Start and exit the loop immediately - corresponding to adding a LOOP and END row // immediately since there is no body to execute. // Increment the clock, corresponding to the LOOP operation - self.increment_clk(tracer, stopper).map_break(|_| { - BreakReason::Stopped(Some(Continuation::FinishLoopUnentered(current_node_id))) + self.increment_clk_with_continuation(tracer, stopper, || { + Some(Continuation::FinishLoopUnentered(current_node_id)) })?; self.finish_loop_node_unentered( @@ -69,7 +69,7 @@ impl FastProcessor { stopper, ) } else { - let err_ctx = err_ctx!(current_forest, current_node_id, host, self.in_debug_mode); + let err_ctx = err_ctx!(current_forest, current_node_id, host, self.in_debug_mode()); ControlFlow::Break(BreakReason::Err(ExecutionError::not_binary_value_loop( condition, &err_ctx, ))) @@ -108,7 +108,7 @@ impl FastProcessor { continuation_stack.push_start_node(loop_node.body()); // Corresponds to the REPEAT operation added to the trace. - self.increment_clk(tracer, stopper).map_break(BreakReason::stopped) + self.increment_clk(tracer, stopper) } else if condition == ZERO { // Exit the loop - add END row tracer.start_clock_cycle( @@ -120,13 +120,13 @@ impl FastProcessor { self.decrement_stack_size(tracer); // Corresponds to the END operation added to the trace. - self.increment_clk(tracer, stopper).map_break(|_| { - BreakReason::Stopped(Some(Continuation::AfterExitDecorators(current_node_id))) + self.increment_clk_with_continuation(tracer, stopper, || { + Some(Continuation::AfterExitDecorators(current_node_id)) })?; self.execute_after_exit_decorators(current_node_id, current_forest, host) } else { - let err_ctx = err_ctx!(current_forest, current_node_id, host, self.in_debug_mode); + let err_ctx = err_ctx!(current_forest, current_node_id, host, self.in_debug_mode()); ControlFlow::Break(BreakReason::Err(ExecutionError::not_binary_value_loop( condition, &err_ctx, ))) @@ -155,8 +155,8 @@ impl FastProcessor { ); // Increment the clock, corresponding to the END operation added to the trace. - self.increment_clk(tracer, stopper).map_break(|_| { - BreakReason::Stopped(Some(Continuation::AfterExitDecorators(current_node_id))) + self.increment_clk_with_continuation(tracer, stopper, || { + Some(Continuation::AfterExitDecorators(current_node_id)) })?; // Execute decorators that should be executed after exiting the node diff --git a/processor/src/fast/mod.rs b/processor/src/fast/mod.rs index 4d08ad3435..030f631787 100644 --- a/processor/src/fast/mod.rs +++ b/processor/src/fast/mod.rs @@ -5,7 +5,7 @@ use alloc::{boxed::Box, sync::Arc, vec::Vec}; use core::cell::Cell; use core::{cmp::min, ops::ControlFlow}; -use miden_air::{Felt, trace::RowIndex}; +use miden_air::{ExecutionOptions, Felt, trace::RowIndex}; use miden_core::{ Decorator, EMPTY_WORD, Kernel, Program, StackOutputs, WORD_SIZE, Word, ZERO, mast::{MastForest, MastNode, MastNodeExt, MastNodeId}, @@ -131,8 +131,9 @@ pub struct FastProcessor { /// return. It is a stack since calls can be nested. call_stack: Vec, - /// Whether to enable debug statements and tracing. - in_debug_mode: bool, + /// Options for execution, including but not limited to whether debug or tracing is enabled, + /// the size of core trace fragments during execution, etc. + options: ExecutionOptions, /// Transcript used to record commitments via `log_precompile` instruction (implemented via RPO /// sponge). @@ -147,37 +148,47 @@ impl FastProcessor { // CONSTRUCTORS // ---------------------------------------------------------------------------------------------- - /// Creates a new `FastProcessor` instance with the given stack inputs. + /// Creates a new `FastProcessor` instance with the given stack inputs, where debug and tracing + /// are disabled. /// /// # Panics /// - Panics if the length of `stack_inputs` is greater than `MIN_STACK_DEPTH`. pub fn new(stack_inputs: &[Felt]) -> Self { - Self::initialize(stack_inputs, AdviceInputs::default(), false) + Self::new_with_options(stack_inputs, AdviceInputs::default(), ExecutionOptions::default()) } - /// Creates a new `FastProcessor` instance with the given stack and advice inputs. + /// Creates a new `FastProcessor` instance with the given stack and advice inputs, where debug + /// and tracing are disabled. /// /// # Panics /// - Panics if the length of `stack_inputs` is greater than `MIN_STACK_DEPTH`. pub fn new_with_advice_inputs(stack_inputs: &[Felt], advice_inputs: AdviceInputs) -> Self { - Self::initialize(stack_inputs, advice_inputs, false) + Self::new_with_options(stack_inputs, advice_inputs, ExecutionOptions::default()) } - /// Creates a new `FastProcessor` instance, set to debug mode, with the given stack - /// and advice inputs. + /// Creates a new `FastProcessor` instance with the given stack and advice inputs, where + /// debugging and tracing are enabled. /// /// # Panics /// - Panics if the length of `stack_inputs` is greater than `MIN_STACK_DEPTH`. pub fn new_debug(stack_inputs: &[Felt], advice_inputs: AdviceInputs) -> Self { - Self::initialize(stack_inputs, advice_inputs, true) + Self::new_with_options( + stack_inputs, + advice_inputs, + ExecutionOptions::default().with_debugging(true).with_tracing(), + ) } - /// Generic constructor unifying the above public ones. + /// Most general constructor unifying all the other ones. /// /// The stack inputs are expected to be stored in reverse order. For example, if `stack_inputs = /// [1,2,3]`, then the stack will be initialized as `[3,2,1,0,0,...]`, with `3` being on /// top. - fn initialize(stack_inputs: &[Felt], advice_inputs: AdviceInputs, in_debug_mode: bool) -> Self { + pub fn new_with_options( + stack_inputs: &[Felt], + advice_inputs: AdviceInputs, + options: ExecutionOptions, + ) -> Self { assert!(stack_inputs.len() <= MIN_STACK_DEPTH); let stack_top_idx = INITIAL_STACK_TOP_IDX; @@ -204,7 +215,7 @@ impl FastProcessor { memory: Memory::new(), call_stack: Vec::new(), ace: Ace::default(), - in_debug_mode, + options, pc_transcript: PrecompileTranscript::new(), #[cfg(test)] decorator_retrieval_count: Rc::new(Cell::new(0)), @@ -230,6 +241,21 @@ impl FastProcessor { // ACCESSORS // ------------------------------------------------------------------------------------------- + /// Returns whether the processor is executing in debug mode. + #[inline(always)] + pub fn in_debug_mode(&self) -> bool { + self.options.enable_debugging() + } + + /// Returns true if decorators should be executed. + /// + /// This corresponds to either being in debug mode (for debug decorators) or having tracing + /// enabled (for trace decorators). + #[inline(always)] + fn should_execute_decorators(&self) -> bool { + self.in_debug_mode() || self.options.enable_tracing() + } + #[cfg(test)] #[inline(always)] fn record_decorator_retrieval(&self) { @@ -360,9 +386,8 @@ impl FastProcessor { self, program: &Program, host: &mut impl AsyncHost, - fragment_size: usize, ) -> Result<(ExecutionOutput, TraceGenerationContext), ExecutionError> { - let mut tracer = ExecutionTracer::new(fragment_size); + let mut tracer = ExecutionTracer::new(self.options.core_trace_fragment_size()); let execution_output = self.execute_with_tracer(program, host, &mut tracer).await?; // Pass the final precompile transcript from execution output to the trace generation @@ -692,7 +717,7 @@ impl FastProcessor { current_forest: &MastForest, host: &mut impl AsyncHost, ) -> ControlFlow { - if !self.in_debug_mode { + if !self.should_execute_decorators() { return ControlFlow::Continue(()); } @@ -717,7 +742,7 @@ impl FastProcessor { current_forest: &MastForest, host: &mut impl AsyncHost, ) -> ControlFlow { - if !self.in_debug_mode { + if !self.in_debug_mode() { return ControlFlow::Continue(()); } @@ -743,7 +768,7 @@ impl FastProcessor { ) -> ControlFlow { match decorator { Decorator::Debug(options) => { - if self.in_debug_mode { + if self.in_debug_mode() { let clk = self.clk; let process = &mut self.state(); if let Err(err) = host.on_debug(process, options) { @@ -773,18 +798,36 @@ impl FastProcessor { // ---------------------------------------------------------------------------------------------- /// Increments the clock by 1. + /// + /// To provide a continuation in case the execution is stopped, use + /// [Self::increment_clk_with_continuation()]. #[inline(always)] pub(crate) fn increment_clk( &mut self, tracer: &mut impl Tracer, stopper: &impl Stopper, - ) -> ControlFlow<()> { + ) -> ControlFlow { + self.increment_clk_with_continuation(tracer, stopper, || None) + } + + /// Increments the clock by 1, and provides a closure to compute a continuation in case the + /// execution is stopped. + pub(crate) fn increment_clk_with_continuation( + &mut self, + tracer: &mut impl Tracer, + stopper: &impl Stopper, + continuation: impl FnOnce() -> Option, + ) -> ControlFlow { self.clk += 1_u32; tracer.increment_clk(); - if stopper.should_stop(self) { - ControlFlow::Break(()) + if self.clk >= self.options.max_cycles() as usize { + ControlFlow::Break(BreakReason::Err(ExecutionError::CycleLimitExceeded( + self.options.max_cycles(), + ))) + } else if stopper.should_stop(self) { + ControlFlow::Break(BreakReason::Stopped(continuation())) } else { ControlFlow::Continue(()) } @@ -978,7 +1021,6 @@ impl FastProcessor { self, program: &Program, host: &mut impl AsyncHost, - fragment_size: usize, ) -> Result<(ExecutionOutput, TraceGenerationContext), ExecutionError> { match tokio::runtime::Handle::try_current() { Ok(_handle) => { @@ -993,7 +1035,7 @@ impl FastProcessor { Err(_) => { // No runtime exists - create one and use it let rt = tokio::runtime::Builder::new_current_thread().build().unwrap(); - rt.block_on(self.execute_for_trace(program, host, fragment_size)) + rt.block_on(self.execute_for_trace(program, host)) }, } } diff --git a/processor/src/fast/split.rs b/processor/src/fast/split.rs index ebe70e0c5f..e9e62b5514 100644 --- a/processor/src/fast/split.rs +++ b/processor/src/fast/split.rs @@ -48,7 +48,7 @@ impl FastProcessor { } else if condition == ZERO { continuation_stack.push_start_node(split_node.on_false()); } else { - let err_ctx = err_ctx!(current_forest, node_id, host, self.in_debug_mode); + let err_ctx = err_ctx!(current_forest, node_id, host, self.in_debug_mode()); return ControlFlow::Break(BreakReason::Err(ExecutionError::not_binary_value_if( condition, &err_ctx, ))); @@ -56,7 +56,7 @@ impl FastProcessor { // Corresponds to the row inserted for the SPLIT operation added // to the trace. - self.increment_clk(tracer, stopper).map_break(BreakReason::stopped) + self.increment_clk(tracer, stopper) } /// Executes the finish phase of a Split node. @@ -79,8 +79,8 @@ impl FastProcessor { // Corresponds to the row inserted for the END operation added // to the trace. - self.increment_clk(tracer, stopper).map_break(|_| { - BreakReason::Stopped(Some(Continuation::AfterExitDecorators(node_id))) + self.increment_clk_with_continuation(tracer, stopper, || { + Some(Continuation::AfterExitDecorators(node_id)) })?; self.execute_after_exit_decorators(node_id, current_forest, host)?; diff --git a/processor/src/fast/step.rs b/processor/src/fast/step.rs index 8402da372c..7edbf00366 100644 --- a/processor/src/fast/step.rs +++ b/processor/src/fast/step.rs @@ -85,10 +85,3 @@ pub enum BreakReason { /// step in `FastProcessor::execute_impl()` should be returned. Stopped(Option), } - -impl BreakReason { - #[inline(always)] - pub fn stopped(_: ()) -> Self { - Self::Stopped(None) - } -} diff --git a/processor/src/fast/tests/mod.rs b/processor/src/fast/tests/mod.rs index b960b01a5c..652b94a8ba 100644 --- a/processor/src/fast/tests/mod.rs +++ b/processor/src/fast/tests/mod.rs @@ -1,5 +1,6 @@ -use alloc::{string::ToString, sync::Arc}; +use alloc::{string::ToString, sync::Arc, vec}; +use miden_air::trace::MIN_TRACE_LEN; use miden_assembly::{Assembler, DefaultSourceManager}; use miden_core::{ ONE, Operation, assert_matches, @@ -127,6 +128,33 @@ fn test_syscall_fail() { ); } +/// Tests that `ExecutionError::CycleLimitExceeded` is correctly emitted when a program exceeds the +/// number of allowed cycles. +#[test] +fn test_cycle_limit_exceeded() { + use miden_air::{DEFAULT_CORE_TRACE_FRAGMENT_SIZE, ExecutionOptions}; + + let mut host = DefaultHost::default(); + + let options = ExecutionOptions::new( + Some(MIN_TRACE_LEN as u32), + MIN_TRACE_LEN as u32, + DEFAULT_CORE_TRACE_FRAGMENT_SIZE, + false, + false, + ) + .unwrap(); + + // Note: when executing, the processor executes `SPAN`, `END` and `HALT` operations, and hence + // the total number of operations is certain to be greater than `MIN_TRACE_LEN`. + let program = simple_program_with_ops(vec![Operation::Swap; MIN_TRACE_LEN]); + + let processor = FastProcessor::new_with_options(&[], AdviceInputs::default(), options); + let err = processor.execute_sync(&program, &mut host).unwrap_err(); + + assert_matches!(err, ExecutionError::CycleLimitExceeded(max_cycles) if max_cycles == MIN_TRACE_LEN as u32); +} + #[test] fn test_assert() { let mut host = DefaultHost::default(); diff --git a/processor/src/lib.rs b/processor/src/lib.rs index 8580982c91..921088f92c 100644 --- a/processor/src/lib.rs +++ b/processor/src/lib.rs @@ -176,10 +176,9 @@ pub async fn execute( options: ExecutionOptions, ) -> Result { let stack_inputs: Vec = stack_inputs.into_iter().rev().collect(); - let processor = FastProcessor::new_with_advice_inputs(&stack_inputs, advice_inputs); - let (execution_output, trace_generation_context) = processor - .execute_for_trace(program, host, options.core_trace_fragment_size()) - .await?; + let processor = FastProcessor::new_with_options(&stack_inputs, advice_inputs, options); + let (execution_output, trace_generation_context) = + processor.execute_for_trace(program, host).await?; let trace = build_trace( execution_output, diff --git a/processor/src/parallel/core_trace_fragment/tests.rs b/processor/src/parallel/core_trace_fragment/tests.rs index 232dff09a3..fe08689fb1 100644 --- a/processor/src/parallel/core_trace_fragment/tests.rs +++ b/processor/src/parallel/core_trace_fragment/tests.rs @@ -1,12 +1,15 @@ use alloc::{sync::Arc, vec::Vec}; -use miden_air::trace::{ - CTX_COL_IDX, - decoder::{ - ADDR_COL_IDX, GROUP_COUNT_COL_IDX, HASHER_STATE_RANGE, IN_SPAN_COL_IDX, NUM_HASHER_COLUMNS, - NUM_OP_BATCH_FLAGS, NUM_OP_BITS, OP_BATCH_1_GROUPS, OP_BATCH_2_GROUPS, OP_BATCH_4_GROUPS, - OP_BATCH_8_GROUPS, OP_BATCH_FLAGS_RANGE, OP_BITS_EXTRA_COLS_RANGE, OP_BITS_OFFSET, - OP_INDEX_COL_IDX, +use miden_air::{ + ExecutionOptions, + trace::{ + CTX_COL_IDX, + decoder::{ + ADDR_COL_IDX, GROUP_COUNT_COL_IDX, HASHER_STATE_RANGE, IN_SPAN_COL_IDX, + NUM_HASHER_COLUMNS, NUM_OP_BATCH_FLAGS, NUM_OP_BITS, OP_BATCH_1_GROUPS, + OP_BATCH_2_GROUPS, OP_BATCH_4_GROUPS, OP_BATCH_8_GROUPS, OP_BATCH_FLAGS_RANGE, + OP_BITS_EXTRA_COLS_RANGE, OP_BITS_OFFSET, OP_INDEX_COL_IDX, + }, }, }; use miden_core::{ @@ -19,7 +22,7 @@ use miden_core::{ use miden_utils_testing::rand::rand_value; use crate::{ - DefaultHost, ExecutionTrace, NoopEventHandler, PrimeField64, fast::FastProcessor, + AdviceInputs, DefaultHost, ExecutionTrace, NoopEventHandler, PrimeField64, fast::FastProcessor, parallel::build_trace, }; @@ -1596,12 +1599,18 @@ const MAX_FRAGMENT_SIZE: usize = 1 << 20; /// portion. fn build_trace_helper(stack_inputs: &[u64], program: &Program) -> (DecoderTrace, usize) { let stack_inputs: Vec = stack_inputs.iter().map(|&v| Felt::new(v)).collect(); - let processor = FastProcessor::new(&stack_inputs); + let processor = FastProcessor::new_with_options( + &stack_inputs, + AdviceInputs::default(), + ExecutionOptions::default() + .with_core_trace_fragment_size(MAX_FRAGMENT_SIZE) + .unwrap(), + ); let mut host = DefaultHost::default(); host.register_handler(EMIT_EVENT, Arc::new(NoopEventHandler)).unwrap(); let (execution_output, trace_generation_context) = - processor.execute_for_trace_sync(program, &mut host, MAX_FRAGMENT_SIZE).unwrap(); + processor.execute_for_trace_sync(program, &mut host).unwrap(); let trace = build_trace( execution_output, @@ -1630,11 +1639,17 @@ fn build_call_trace_helper( program: &Program, kernel: Kernel, ) -> (SystemTrace, DecoderTrace, usize) { - let processor = FastProcessor::new(&[]); + let processor = FastProcessor::new_with_options( + &[], + AdviceInputs::default(), + ExecutionOptions::default() + .with_core_trace_fragment_size(MAX_FRAGMENT_SIZE) + .unwrap(), + ); let mut host = DefaultHost::default(); let (execution_output, trace_generation_context) = - processor.execute_for_trace_sync(program, &mut host, MAX_FRAGMENT_SIZE).unwrap(); + processor.execute_for_trace_sync(program, &mut host).unwrap(); let trace = build_trace(execution_output, trace_generation_context, program.hash(), kernel); diff --git a/processor/src/parallel/tests.rs b/processor/src/parallel/tests.rs index 989406dc48..91484a8529 100644 --- a/processor/src/parallel/tests.rs +++ b/processor/src/parallel/tests.rs @@ -14,7 +14,7 @@ use pretty_assertions::assert_eq; use rstest::{fixture, rstest}; use super::*; -use crate::{DefaultHost, HostLibrary, fast::FastProcessor}; +use crate::{AdviceInputs, DefaultHost, ExecutionOptions, HostLibrary, fast::FastProcessor}; const DEFAULT_STACK: &[Felt] = &[Felt::new(1), Felt::new(2), Felt::new(3)]; @@ -285,11 +285,17 @@ fn test_trace_generation_at_fragment_boundaries( const MAX_FRAGMENT_SIZE: usize = 1 << 20; let trace_from_fragments = { - let processor = FastProcessor::new(stack_inputs); + let processor = FastProcessor::new_with_options( + stack_inputs, + AdviceInputs::default(), + ExecutionOptions::default() + .with_core_trace_fragment_size(fragment_size) + .unwrap(), + ); let mut host = DefaultHost::default(); host.load_library(create_simple_library()).unwrap(); let (execution_output, trace_fragment_contexts) = - processor.execute_for_trace_sync(&program, &mut host, fragment_size).unwrap(); + processor.execute_for_trace_sync(&program, &mut host).unwrap(); build_trace( execution_output, @@ -300,12 +306,17 @@ fn test_trace_generation_at_fragment_boundaries( }; let trace_from_single_fragment = { - let processor = FastProcessor::new(stack_inputs); + let processor = FastProcessor::new_with_options( + stack_inputs, + AdviceInputs::default(), + ExecutionOptions::default() + .with_core_trace_fragment_size(MAX_FRAGMENT_SIZE) + .unwrap(), + ); let mut host = DefaultHost::default(); host.load_library(create_simple_library()).unwrap(); - let (execution_output, trace_fragment_contexts) = processor - .execute_for_trace_sync(&program, &mut host, MAX_FRAGMENT_SIZE) - .unwrap(); + let (execution_output, trace_fragment_contexts) = + processor.execute_for_trace_sync(&program, &mut host).unwrap(); assert!(trace_fragment_contexts.core_trace_contexts.len() == 1); build_trace( diff --git a/processor/src/trace/tests/mod.rs b/processor/src/trace/tests/mod.rs index 7d4b8825db..4cfdf38a16 100644 --- a/processor/src/trace/tests/mod.rs +++ b/processor/src/trace/tests/mod.rs @@ -7,7 +7,10 @@ use miden_core::{ use miden_utils_testing::rand::rand_array; use super::{super::chiplets::init_state_from_words, ExecutionTrace, Felt}; -use crate::{AdviceInputs, DefaultHost, StackInputs, fast::FastProcessor, parallel::build_trace}; +use crate::{ + AdviceInputs, DefaultHost, ExecutionOptions, StackInputs, fast::FastProcessor, + parallel::build_trace, +}; mod chiplets; mod decoder; @@ -27,10 +30,15 @@ const TEST_TRACE_FRAGMENT_SIZE: usize = 1 << 10; pub fn build_trace_from_program(program: &Program, stack_inputs: &[u64]) -> ExecutionTrace { let stack_inputs = stack_inputs.iter().map(|&v| Felt::new(v)).collect::>(); let mut host = DefaultHost::default(); - let processor = FastProcessor::new(&stack_inputs); - let (execution_output, trace_generation_context) = processor - .execute_for_trace_sync(program, &mut host, TEST_TRACE_FRAGMENT_SIZE) - .unwrap(); + let processor = FastProcessor::new_with_options( + &stack_inputs, + AdviceInputs::default(), + ExecutionOptions::default() + .with_core_trace_fragment_size(TEST_TRACE_FRAGMENT_SIZE) + .unwrap(), + ); + let (execution_output, trace_generation_context) = + processor.execute_for_trace_sync(program, &mut host).unwrap(); build_trace(execution_output, trace_generation_context, program.hash(), Kernel::default()) } @@ -72,10 +80,15 @@ pub fn build_trace_from_ops_with_inputs( let stack_values: Vec = stack_inputs.iter().rev().copied().collect(); let mut host = DefaultHost::default(); - let processor = FastProcessor::new_with_advice_inputs(&stack_values, advice_inputs); - let (execution_output, trace_generation_context) = processor - .execute_for_trace_sync(&program, &mut host, TEST_TRACE_FRAGMENT_SIZE) - .unwrap(); + let processor = FastProcessor::new_with_options( + &stack_values, + advice_inputs, + ExecutionOptions::default() + .with_core_trace_fragment_size(TEST_TRACE_FRAGMENT_SIZE) + .unwrap(), + ); + let (execution_output, trace_generation_context) = + processor.execute_for_trace_sync(&program, &mut host).unwrap(); build_trace(execution_output, trace_generation_context, program.hash(), Kernel::default()) } diff --git a/prover/src/lib.rs b/prover/src/lib.rs index 8b81ed77fe..12cd594711 100644 --- a/prover/src/lib.rs +++ b/prover/src/lib.rs @@ -59,15 +59,14 @@ pub async fn prove( // (first element = bottom of stack, last element = top) let stack_inputs_reversed: alloc::vec::Vec = stack_inputs.iter().copied().rev().collect(); - let processor = if options.execution_options().enable_debugging() { - FastProcessor::new_debug(&stack_inputs_reversed, advice_inputs) - } else { - FastProcessor::new_with_advice_inputs(&stack_inputs_reversed, advice_inputs) - }; + let processor = FastProcessor::new_with_options( + &stack_inputs_reversed, + advice_inputs, + *options.execution_options(), + ); - let (execution_output, trace_generation_context) = processor - .execute_for_trace(program, host, options.execution_options().core_trace_fragment_size()) - .await?; + let (execution_output, trace_generation_context) = + processor.execute_for_trace(program, host).await?; let trace = build_trace( execution_output,