Skip to content

Commit 51f07d5

Browse files
authored
fix: RootVerifierLocalProver should respect AIR heights (#1554)
- Not a soundness issue. Just an issue of `RootVerifierLocalProver` implementation in SDK. - In some edge cases, the trace heights of the root verifier circuit are less than the required heights. In this case, `RootVerifierLocalProver` should pad its traces to the required heights. - [Breaking change] Add a new field `internal_heights` into `RootVerifierProvingKey`.
1 parent 40927aa commit 51f07d5

File tree

5 files changed

+18
-17
lines changed

5 files changed

+18
-17
lines changed

crates/sdk/src/keygen/dummy.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ pub(super) fn compute_root_proof_heights(
6363
.into_iter()
6464
.map(next_power_of_two_or_zero)
6565
.collect();
66-
let mut internal_heights = res.internal_heights;
67-
internal_heights.round_to_next_power_of_two_or_zero();
68-
(air_heights, internal_heights)
66+
let mut vm_heights = res.vm_heights;
67+
vm_heights.round_to_next_power_of_two_or_zero();
68+
(air_heights, vm_heights)
6969
}
7070

7171
pub(super) fn dummy_internal_proof(
@@ -186,9 +186,9 @@ where
186186
assert_eq!(results.len(), 1, "dummy exe should have only 1 segment");
187187
let mut result = results.pop().unwrap();
188188
result.chip_complex.finalize_memory();
189-
let mut internal_heights = result.chip_complex.get_internal_trace_heights();
190-
internal_heights.round_to_next_power_of_two();
191-
internal_heights
189+
let mut vm_heights = result.chip_complex.get_internal_trace_heights();
190+
vm_heights.round_to_next_power_of_two();
191+
vm_heights
192192
};
193193
// For the dummy proof, we must override the trace heights.
194194
let app_prover =

crates/sdk/src/keygen/mod.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::sync::Arc;
33
use derivative::Derivative;
44
use dummy::{compute_root_proof_heights, dummy_internal_proof_riscv_app_vm};
55
use openvm_circuit::{
6-
arch::{VirtualMachine, VmConfig},
6+
arch::{VirtualMachine, VmComplexTraceHeights, VmConfig},
77
system::{memory::dimensions::MemoryDimensions, program::trace::VmCommittedExe},
88
};
99
use openvm_continuations::{
@@ -333,7 +333,7 @@ impl AggStarkProvingKey {
333333
let mut vm_pk = vm.keygen();
334334
assert!(vm_pk.max_constraint_degree <= config.root_fri_params.max_constraint_degree());
335335

336-
let (air_heights, _internal_heights) = compute_root_proof_heights(
336+
let (air_heights, vm_heights) = compute_root_proof_heights(
337337
root_vm_config.clone(),
338338
root_committed_exe.exe.clone(),
339339
&internal_proof,
@@ -349,6 +349,7 @@ impl AggStarkProvingKey {
349349
}),
350350
root_committed_exe,
351351
air_heights,
352+
vm_heights,
352353
}
353354
};
354355
(
@@ -392,9 +393,8 @@ pub struct RootVerifierProvingKey {
392393
pub root_committed_exe: Arc<VmCommittedExe<RootSC>>,
393394
/// The constant trace heights, ordered by AIR ID.
394395
pub air_heights: Vec<usize>,
395-
// The following is currently not used:
396-
// The constant trace heights, ordered according to an internal ordering determined by the `NativeConfig`.
397-
// pub internal_heights: VmComplexTraceHeights,
396+
/// The constant trace heights in a semantic way for VM.
397+
pub vm_heights: VmComplexTraceHeights,
398398
}
399399

400400
impl RootVerifierProvingKey {

crates/sdk/src/prover/root.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ impl RootVerifierLocalProver {
5252
impl SingleSegmentVmProver<RootSC> for RootVerifierLocalProver {
5353
fn prove(&self, input: impl Into<Streams<F>>) -> Proof<RootSC> {
5454
let input = input.into();
55-
let vm = SingleSegmentVmExecutor::new(self.vm_config().clone());
55+
let mut vm = SingleSegmentVmExecutor::new(self.vm_config().clone());
56+
vm.set_override_trace_heights(self.root_verifier_pk.vm_heights.clone());
5657
let mut proof_input = vm
5758
.execute_and_generate(self.root_verifier_pk.root_committed_exe.clone(), input)
5859
.unwrap();

crates/vm/src/arch/vm.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ pub struct SingleSegmentVmExecutionResult<F> {
376376
/// Heights of each AIR, ordered by AIR ID.
377377
pub air_heights: Vec<usize>,
378378
/// Heights of (SystemBase, Inventory), in an internal ordering.
379-
pub internal_heights: VmComplexTraceHeights,
379+
pub vm_heights: VmComplexTraceHeights,
380380
}
381381

382382
impl<F, VC> SingleSegmentVmExecutor<F, VC>
@@ -424,7 +424,7 @@ where
424424
segment
425425
};
426426
let air_heights = segment.chip_complex.current_trace_heights();
427-
let internal_heights = segment.chip_complex.get_internal_trace_heights();
427+
let vm_heights = segment.chip_complex.get_internal_trace_heights();
428428
let public_values = if let Some(pv_chip) = segment.chip_complex.public_values_chip() {
429429
pv_chip.core.get_custom_public_values()
430430
} else {
@@ -433,7 +433,7 @@ where
433433
Ok(SingleSegmentVmExecutionResult {
434434
public_values,
435435
air_heights,
436-
internal_heights,
436+
vm_heights,
437437
})
438438
}
439439

crates/vm/tests/integration_test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ fn test_vm_override_executor_height() {
132132
.unwrap();
133133
// Memory trace heights are not computed during execution.
134134
assert_eq!(
135-
res.internal_heights.system,
135+
res.vm_heights.system,
136136
SystemTraceHeights {
137137
memory: MemoryTraceHeights::Volatile(VolatileMemoryTraceHeights {
138138
boundary: 1,
@@ -141,7 +141,7 @@ fn test_vm_override_executor_height() {
141141
}
142142
);
143143
assert_eq!(
144-
res.internal_heights.inventory,
144+
res.vm_heights.inventory,
145145
VmInventoryTraceHeights {
146146
chips: vec![
147147
(ChipId::Executor(0), 0),

0 commit comments

Comments
 (0)