From 5bda91859071119d7884795bb9eed22cb9987f81 Mon Sep 17 00:00:00 2001 From: Robin Salen Date: Tue, 30 Apr 2024 16:04:56 -0400 Subject: [PATCH 1/6] Remove redundant simulation of last segment --- evm_arithmetization/src/prover.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/evm_arithmetization/src/prover.rs b/evm_arithmetization/src/prover.rs index 0b8f1ddce..b5b777c0f 100644 --- a/evm_arithmetization/src/prover.rs +++ b/evm_arithmetization/src/prover.rs @@ -601,6 +601,8 @@ pub fn generate_all_data_segments( /// A utility module designed to test witness generation externally. pub mod testing { + use log::info; + use super::*; use crate::{ cpu::kernel::interpreter::Interpreter, @@ -662,9 +664,15 @@ pub mod testing { F: Field, { let mut index = 0; - while generate_segment::(max_cpu_len_log, index, &inputs)?.is_some() { + while let Some((_, final_registers, _, _)) = + generate_segment::(max_cpu_len_log, index, &inputs)? + { + if final_registers.program_counter == KERNEL.global_labels["halt"] { + return Ok(()); + } index += 1; } + Ok(()) } } From 313fdac649cc6f63718f1f131e57982586e44bbe Mon Sep 17 00:00:00 2001 From: Robin Salen Date: Tue, 30 Apr 2024 16:31:04 -0400 Subject: [PATCH 2/6] Do some refactoring --- .../src/cpu/kernel/interpreter.rs | 91 +----------- evm_arithmetization/src/prover.rs | 133 ++++++++++-------- 2 files changed, 77 insertions(+), 147 deletions(-) diff --git a/evm_arithmetization/src/cpu/kernel/interpreter.rs b/evm_arithmetization/src/cpu/kernel/interpreter.rs index 514f1a392..886a0281b 100644 --- a/evm_arithmetization/src/cpu/kernel/interpreter.rs +++ b/evm_arithmetization/src/cpu/kernel/interpreter.rs @@ -122,91 +122,15 @@ pub(crate) struct ExtraSegmentData { pub(crate) jumpdest_table: Option>>, } -/// Given a segment `index`, returns the initial and final registers, as well -/// as the initial memory of segment `index`, and returns `None` if the -/// execution stops at segment `index`. These can then be passed to the -/// prover for initialization. -pub(crate) fn generate_segment( - max_cpu_len_log: usize, - index: usize, - inputs: &GenerationInputs, -) -> anyhow::Result< - Option<( - RegistersState, - RegistersState, - MemoryState, - ExtraSegmentData, - )>, -> { - let init_label = KERNEL.global_labels["init"]; - let initial_registers = RegistersState::new(); - let mut interpreter = Interpreter::::new_with_generation_inputs( - init_label, - vec![], - inputs, - Some(max_cpu_len_log), - ); - - let (mut registers_before, mut registers_after, mut before_mem_values, mut after_mem_values) = ( - initial_registers, - initial_registers, - MemoryState::default(), - MemoryState::default(), - ); - - let mut extra_data = ExtraSegmentData::default(); - - for i in 0..=index { - if i == index { - extra_data = ExtraSegmentData { - trimmed_inputs: interpreter.generation_state.inputs.clone(), - bignum_modmul_result_limbs: interpreter - .generation_state - .bignum_modmul_result_limbs - .clone(), - rlp_prover_inputs: interpreter.generation_state.rlp_prover_inputs.clone(), - withdrawal_prover_inputs: interpreter - .generation_state - .withdrawal_prover_inputs - .clone(), - trie_root_ptrs: interpreter.generation_state.trie_root_ptrs.clone(), - jumpdest_table: interpreter.generation_state.jumpdest_table.clone(), - }; - } - - // Write initial registers. - if registers_after.program_counter == KERNEL.global_labels["halt"] { - return Ok(None); - } - - (registers_before, before_mem_values) = (registers_after, after_mem_values); - interpreter.generation_state.registers = registers_before; - interpreter.generation_state.registers.program_counter = init_label; - interpreter.generation_state.registers.is_kernel = true; - interpreter.clock = 0; - - let (updated_registers_after, opt_after_mem_values) = - set_registers_and_run(registers_after, &mut interpreter)?; - - registers_after = updated_registers_after; - after_mem_values = opt_after_mem_values.expect( - "We are in the interpreter: the run should return a memory - state", - ); - } - - Ok(Some(( - registers_before, - registers_after, - before_mem_values, - extra_data, - ))) -} - pub(crate) fn set_registers_and_run( registers: RegistersState, interpreter: &mut Interpreter, ) -> anyhow::Result<(RegistersState, Option)> { + interpreter.generation_state.registers = registers; + interpreter.generation_state.registers.program_counter = KERNEL.global_labels["init"]; + interpreter.generation_state.registers.is_kernel = true; + interpreter.clock = 0; + // Write initial registers. [ registers.program_counter.into(), @@ -225,10 +149,7 @@ pub(crate) fn set_registers_and_run( (Segment::RegistersStates.unscale()).into(), i.into(), ) - .expect( - "All input values are known to be valid for -MemoryAddress", - ), + .expect("All input values are known to be valid for MemoryAddress"), *reg_content, ); interpreter.generation_state.memory.set(addr, val); diff --git a/evm_arithmetization/src/prover.rs b/evm_arithmetization/src/prover.rs index b5b777c0f..c07508ea6 100644 --- a/evm_arithmetization/src/prover.rs +++ b/evm_arithmetization/src/prover.rs @@ -24,9 +24,7 @@ use starky::stark::Stark; use crate::all_stark::{AllStark, Table, NUM_TABLES}; use crate::cpu::kernel::aggregator::KERNEL; -use crate::cpu::kernel::interpreter::{ - generate_segment, set_registers_and_run, ExtraSegmentData, Interpreter, -}; +use crate::cpu::kernel::interpreter::{set_registers_and_run, ExtraSegmentData, Interpreter}; use crate::generation::state::GenerationState; use crate::generation::{generate_traces, GenerationInputs}; use crate::get_challenges::observe_public_values; @@ -482,6 +480,40 @@ pub fn check_abort_signal(abort_signal: Option>) -> Result<()> { Ok(()) } +/// Builds a new `GenerationSegmentData`. +/// This new segment's `is_dummy` field must be updated manually +/// in case it corresponds to a dummy segment. +fn build_segment_data( + segment_index: usize, + registers_before: Option, + registers_after: Option, + memory: Option, + interpreter: &Interpreter, +) -> GenerationSegmentData { + GenerationSegmentData { + is_dummy: false, + segment_index, + registers_before: registers_before.unwrap_or(RegistersState::new()), + registers_after: registers_after.unwrap_or(RegistersState::new()), + memory: memory.unwrap_or_default(), + max_cpu_len_log: interpreter.get_max_cpu_len_log(), + extra_data: ExtraSegmentData { + trimmed_inputs: interpreter.generation_state.inputs.clone(), + bignum_modmul_result_limbs: interpreter + .generation_state + .bignum_modmul_result_limbs + .clone(), + rlp_prover_inputs: interpreter.generation_state.rlp_prover_inputs.clone(), + withdrawal_prover_inputs: interpreter + .generation_state + .withdrawal_prover_inputs + .clone(), + trie_root_ptrs: interpreter.generation_state.trie_root_ptrs.clone(), + jumpdest_table: interpreter.generation_state.jumpdest_table.clone(), + }, + } +} + /// Returns a vector containing the data required to generate all the segments /// of a transaction. pub fn generate_all_data_segments( @@ -499,35 +531,9 @@ pub fn generate_all_data_segments( let mut segment_index = 0; - let mut segment_data = GenerationSegmentData { - is_dummy: false, - segment_index, - registers_before: RegistersState::new(), - registers_after: RegistersState::new(), - memory: MemoryState::default(), - max_cpu_len_log, - extra_data: ExtraSegmentData { - trimmed_inputs: interpreter.generation_state.inputs.clone(), - bignum_modmul_result_limbs: interpreter - .generation_state - .bignum_modmul_result_limbs - .clone(), - rlp_prover_inputs: interpreter.generation_state.rlp_prover_inputs.clone(), - withdrawal_prover_inputs: interpreter - .generation_state - .withdrawal_prover_inputs - .clone(), - trie_root_ptrs: interpreter.generation_state.trie_root_ptrs.clone(), - jumpdest_table: interpreter.generation_state.jumpdest_table.clone(), - }, - }; + let mut segment_data = build_segment_data(segment_index, None, None, None, &interpreter); while segment_data.registers_after.program_counter != KERNEL.global_labels["halt"] { - interpreter.generation_state.registers = segment_data.registers_after; - interpreter.generation_state.registers.program_counter = KERNEL.global_labels["init"]; - interpreter.generation_state.registers.is_kernel = true; - interpreter.clock = 0; - let (updated_registers, mem_after) = set_registers_and_run(segment_data.registers_after, &mut interpreter)?; @@ -537,30 +543,13 @@ pub fn generate_all_data_segments( segment_index += 1; - segment_data = GenerationSegmentData { - is_dummy: false, + segment_data = build_segment_data( segment_index, - registers_before: updated_registers, - // `registers_after` will be set correctly at the next iteration.` - registers_after: updated_registers, - max_cpu_len_log, - memory: mem_after - .expect("The interpreter was running, so it should have returned a MemoryState"), - extra_data: ExtraSegmentData { - trimmed_inputs: interpreter.generation_state.inputs.clone(), - bignum_modmul_result_limbs: interpreter - .generation_state - .bignum_modmul_result_limbs - .clone(), - rlp_prover_inputs: interpreter.generation_state.rlp_prover_inputs.clone(), - withdrawal_prover_inputs: interpreter - .generation_state - .withdrawal_prover_inputs - .clone(), - trie_root_ptrs: interpreter.generation_state.trie_root_ptrs.clone(), - jumpdest_table: interpreter.generation_state.jumpdest_table.clone(), - }, - }; + Some(updated_registers), + Some(updated_registers), + mem_after, + &interpreter, + ); } // We need at least two segments to prove a segment aggregation. @@ -661,16 +650,36 @@ pub mod testing { max_cpu_len_log: usize, ) -> anyhow::Result<()> where - F: Field, + F: RichField, { - let mut index = 0; - while let Some((_, final_registers, _, _)) = - generate_segment::(max_cpu_len_log, index, &inputs)? - { - if final_registers.program_counter == KERNEL.global_labels["halt"] { - return Ok(()); - } - index += 1; + let max_cpu_len_log = Some(max_cpu_len_log); + let mut interpreter = Interpreter::::new_with_generation_inputs( + KERNEL.global_labels["init"], + vec![], + &inputs, + max_cpu_len_log, + ); + + let mut segment_index = 0; + + let mut segment_data = build_segment_data(segment_index, None, None, None, &interpreter); + + while segment_data.registers_after.program_counter != KERNEL.global_labels["halt"] { + let (updated_registers, mem_after) = + set_registers_and_run(segment_data.registers_after, &mut interpreter)?; + + // Set `registers_after` correctly and push the data. + segment_data.registers_after = updated_registers; + + segment_index += 1; + + segment_data = build_segment_data( + segment_index, + Some(updated_registers), + Some(updated_registers), + mem_after, + &interpreter, + ); } Ok(()) From 5f22d52aace76f28474787b6a1380572d1413d1f Mon Sep 17 00:00:00 2001 From: Robin Salen Date: Tue, 30 Apr 2024 16:34:23 -0400 Subject: [PATCH 3/6] Cleanup --- evm_arithmetization/src/prover.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/evm_arithmetization/src/prover.rs b/evm_arithmetization/src/prover.rs index c07508ea6..c734542cf 100644 --- a/evm_arithmetization/src/prover.rs +++ b/evm_arithmetization/src/prover.rs @@ -537,8 +537,6 @@ pub fn generate_all_data_segments( let (updated_registers, mem_after) = set_registers_and_run(segment_data.registers_after, &mut interpreter)?; - // Set `registers_after` correctly and push the data. - segment_data.registers_after = updated_registers; all_seg_data.push(segment_data); segment_index += 1; @@ -665,14 +663,11 @@ pub mod testing { let mut segment_data = build_segment_data(segment_index, None, None, None, &interpreter); while segment_data.registers_after.program_counter != KERNEL.global_labels["halt"] { + segment_index += 1; + let (updated_registers, mem_after) = set_registers_and_run(segment_data.registers_after, &mut interpreter)?; - // Set `registers_after` correctly and push the data. - segment_data.registers_after = updated_registers; - - segment_index += 1; - segment_data = build_segment_data( segment_index, Some(updated_registers), From 2495a772c29efaa2dd42e7e470e42ce46cd8df2c Mon Sep 17 00:00:00 2001 From: Robin Salen Date: Tue, 30 Apr 2024 17:54:06 -0400 Subject: [PATCH 4/6] Fix --- evm_arithmetization/src/prover.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/evm_arithmetization/src/prover.rs b/evm_arithmetization/src/prover.rs index c734542cf..f014fd900 100644 --- a/evm_arithmetization/src/prover.rs +++ b/evm_arithmetization/src/prover.rs @@ -535,8 +535,10 @@ pub fn generate_all_data_segments( while segment_data.registers_after.program_counter != KERNEL.global_labels["halt"] { let (updated_registers, mem_after) = - set_registers_and_run(segment_data.registers_after, &mut interpreter)?; + set_registers_and_run(segment_data.registers_before, &mut interpreter)?; + // Set `registers_after` correctly and push the data. + segment_data.registers_after = updated_registers; all_seg_data.push(segment_data); segment_index += 1; @@ -666,7 +668,10 @@ pub mod testing { segment_index += 1; let (updated_registers, mem_after) = - set_registers_and_run(segment_data.registers_after, &mut interpreter)?; + set_registers_and_run(segment_data.registers_before, &mut interpreter)?; + + // Set `registers_after`. + segment_data.registers_after = updated_registers; segment_data = build_segment_data( segment_index, From 3d190ba31d96599effab62dfa3ee9e91626bf766 Mon Sep 17 00:00:00 2001 From: Robin Salen Date: Tue, 30 Apr 2024 17:57:04 -0400 Subject: [PATCH 5/6] Remove needless import --- evm_arithmetization/src/prover.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/evm_arithmetization/src/prover.rs b/evm_arithmetization/src/prover.rs index f014fd900..8b2af5c69 100644 --- a/evm_arithmetization/src/prover.rs +++ b/evm_arithmetization/src/prover.rs @@ -590,8 +590,6 @@ pub fn generate_all_data_segments( /// A utility module designed to test witness generation externally. pub mod testing { - use log::info; - use super::*; use crate::{ cpu::kernel::interpreter::Interpreter, From 65b20139f0f12dca3490303802e29f44eaa932fb Mon Sep 17 00:00:00 2001 From: Robin Salen Date: Tue, 30 Apr 2024 18:03:44 -0400 Subject: [PATCH 6/6] Silence clippy --- evm_arithmetization/src/prover.rs | 1 + evm_arithmetization/src/witness/state.rs | 8 +------- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/evm_arithmetization/src/prover.rs b/evm_arithmetization/src/prover.rs index 8b2af5c69..9eb2c3370 100644 --- a/evm_arithmetization/src/prover.rs +++ b/evm_arithmetization/src/prover.rs @@ -483,6 +483,7 @@ pub fn check_abort_signal(abort_signal: Option>) -> Result<()> { /// Builds a new `GenerationSegmentData`. /// This new segment's `is_dummy` field must be updated manually /// in case it corresponds to a dummy segment. +#[allow(clippy::unwrap_or_default)] fn build_segment_data( segment_index: usize, registers_before: Option, diff --git a/evm_arithmetization/src/witness/state.rs b/evm_arithmetization/src/witness/state.rs index 7553d47a1..76bc27c5a 100644 --- a/evm_arithmetization/src/witness/state.rs +++ b/evm_arithmetization/src/witness/state.rs @@ -36,13 +36,7 @@ impl RegistersState { pub(crate) fn new() -> Self { Self { program_counter: KERNEL.global_labels["main"], - is_kernel: true, - stack_len: 0, - stack_top: U256::zero(), - is_stack_top_read: false, - check_overflow: false, - context: 0, - gas_used: 0, + ..Self::default() } } }