Skip to content

Commit

Permalink
fix: make program hash computations consistent
Browse files Browse the repository at this point in the history
  • Loading branch information
bobbinth committed Jun 17, 2022
1 parent b1eeb34 commit 257a42e
Show file tree
Hide file tree
Showing 9 changed files with 105 additions and 48 deletions.
8 changes: 3 additions & 5 deletions core/src/program/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,14 @@ pub use library::Library;
#[derive(Clone, Debug)]
pub struct Script {
root: CodeBlock,
hash: Digest,
}

impl Script {
// CONSTRUCTOR
// --------------------------------------------------------------------------------------------
/// Constructs a new program from the specified code block.
pub fn new(root: CodeBlock) -> Self {
let hash = hasher::merge(&[root.hash(), Digest::default()]);
Self { root, hash }
Self { root }
}

// PUBLIC ACCESSORS
Expand All @@ -46,8 +44,8 @@ impl Script {
}

/// Returns a hash of this script.
pub fn hash(&self) -> &Digest {
&self.hash
pub fn hash(&self) -> Digest {
self.root.hash()
}
}

Expand Down
4 changes: 2 additions & 2 deletions examples/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ pub fn test_example(example: Example, fail: bool) {

if fail {
outputs[0] += 1;
assert!(miden::verify(*program.hash(), &pub_inputs, &outputs, proof).is_err())
assert!(miden::verify(program.hash(), &pub_inputs, &outputs, proof).is_err())
} else {
assert!(miden::verify(*program.hash(), &pub_inputs, &outputs, proof).is_ok());
assert!(miden::verify(program.hash(), &pub_inputs, &outputs, proof).is_ok());
}
}
2 changes: 1 addition & 1 deletion examples/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ fn main() {
// results in the expected output
let proof = StarkProof::from_bytes(&proof_bytes).unwrap();
let now = Instant::now();
match miden::verify(*program.hash(), &pub_inputs, &outputs, proof) {
match miden::verify(program.hash(), &pub_inputs, &outputs, proof) {
Ok(_) => debug!("Execution verified in {} ms", now.elapsed().as_millis()),
Err(err) => debug!("Failed to verify execution: {}", err),
}
Expand Down
4 changes: 2 additions & 2 deletions miden/tests/integration/helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ impl Test {

if test_fail {
outputs[0] += 1;
assert!(miden::verify(*script.hash(), &pub_inputs, &outputs, proof).is_err());
assert!(miden::verify(script.hash(), &pub_inputs, &outputs, proof).is_err());
} else {
assert!(miden::verify(*script.hash(), &pub_inputs, &outputs, proof).is_ok());
assert!(miden::verify(script.hash(), &pub_inputs, &outputs, proof).is_ok());
}
}

Expand Down
19 changes: 17 additions & 2 deletions processor/src/decoder/mod.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use super::{
ExecutionError, Felt, Join, Loop, OpBatch, Operation, Process, Span, Split, StarkField, Vec,
Word, MIN_TRACE_LEN, OP_BATCH_SIZE,
Word, MIN_TRACE_LEN, ONE, OP_BATCH_SIZE, ZERO,
};
use vm_core::{
decoder::{
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,
},
ONE, ZERO,
hasher::DIGEST_LEN,
};

mod trace;
Expand Down Expand Up @@ -239,6 +239,21 @@ impl Decoder {
}
}

// PUBLIC ACCESSORS
// --------------------------------------------------------------------------------------------

/// Returns execution trace length for this decoder.
pub fn trace_len(&self) -> usize {
self.trace.trace_len()
}

/// Hash of the program decoded by this decoder.
///
/// Hash of the program is taken from the last row of first 4 registers of the decoder trace.
pub fn program_hash(&self) -> [Felt; DIGEST_LEN] {
self.trace.program_hash()
}

// CONTROL BLOCKS
// --------------------------------------------------------------------------------------------

Expand Down
20 changes: 18 additions & 2 deletions processor/src/decoder/trace.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::{
Felt, Operation, Vec, Word, MIN_TRACE_LEN, NUM_HASHER_COLUMNS, NUM_OP_BATCH_FLAGS, NUM_OP_BITS,
ONE, ZERO, OP_BATCH_1_GROUPS, OP_BATCH_2_GROUPS, OP_BATCH_4_GROUPS, OP_BATCH_8_GROUPS
Felt, Operation, Vec, Word, DIGEST_LEN, MIN_TRACE_LEN, NUM_HASHER_COLUMNS, NUM_OP_BATCH_FLAGS,
NUM_OP_BITS, ONE, OP_BATCH_1_GROUPS, OP_BATCH_2_GROUPS, OP_BATCH_4_GROUPS, OP_BATCH_8_GROUPS,
ZERO,
};
use core::ops::Range;
use vm_core::{program::blocks::OP_BATCH_SIZE, utils::new_array_vec, StarkField};
Expand Down Expand Up @@ -65,6 +66,15 @@ impl DecoderTrace {
self.addr_trace.len()
}

/// Returns the contents of the first 4 registers of the hasher state at the last row.
pub fn program_hash(&self) -> [Felt; DIGEST_LEN] {
let mut result = [ZERO; DIGEST_LEN];
for (i, element) in result.iter_mut().enumerate() {
*element = self.last_hasher_value(i);
}
result
}

// TRACE MUTATORS
// --------------------------------------------------------------------------------------------

Expand Down Expand Up @@ -418,6 +428,12 @@ impl DecoderTrace {
*self.group_count_trace.last().expect("no group count")
}

/// Returns the last value in the specified hasher column.
fn last_hasher_value(&self, idx: usize) -> Felt {
debug_assert!(idx < NUM_HASHER_COLUMNS, "invalid hasher register index");
*self.hasher_trace[idx].last().expect("no last hasher value")
}

/// Returns a reference to the last value in the helper register at the specified index.
fn last_helper_mut(&mut self, idx: usize) -> &mut Felt {
debug_assert!(idx < USER_OP_HELPERS.len(), "invalid helper register index");
Expand Down
50 changes: 31 additions & 19 deletions processor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use vm_core::{
utils::collections::{BTreeMap, Vec},
AdviceInjector, DebugOptions, Felt, FieldElement, Operation, ProgramInputs, StackTopState,
StarkField, Word, AUX_TRACE_WIDTH, DECODER_TRACE_WIDTH, MIN_STACK_DEPTH, MIN_TRACE_LEN,
NUM_STACK_HELPER_COLS, RANGE_CHECK_TRACE_WIDTH, STACK_TRACE_WIDTH, SYS_TRACE_WIDTH,
NUM_STACK_HELPER_COLS, ONE, RANGE_CHECK_TRACE_WIDTH, STACK_TRACE_WIDTH, SYS_TRACE_WIDTH, ZERO,
};

mod operations;
Expand Down Expand Up @@ -78,15 +78,25 @@ pub struct RangeCheckTrace {
pub fn execute(script: &Script, inputs: &ProgramInputs) -> Result<ExecutionTrace, ExecutionError> {
let mut process = Process::new(inputs.clone());
process.execute_code_block(script.root())?;
// TODO: make sure program hash from script and trace are the same
Ok(ExecutionTrace::new(process, *script.hash()))
let trace = ExecutionTrace::new(process);
assert_eq!(
script.hash(),
trace.program_hash(),
"inconsistent program hash between "
);
Ok(trace)
}

/// Returns an iterator that allows callers to step through each execution and inspect
/// vm state information along side.
pub fn execute_iter(script: &Script, inputs: &ProgramInputs) -> VmStateIterator {
let mut process = Process::new_debug(inputs.clone());
let result = process.execute_code_block(script.root());
assert_eq!(
script.hash(),
process.decoder.program_hash().into(),
"inconsistent program hash between "
);
VmStateIterator::new(process, result)
}

Expand All @@ -105,6 +115,18 @@ pub struct Process {
}

impl Process {
// CONSTRUCTORS
// --------------------------------------------------------------------------------------------
/// Creates a new process with the provided inputs.
pub fn new(inputs: ProgramInputs) -> Self {
Self::initialize(inputs, false)
}

/// Creates a new process with provided inputs and debug options enabled.
pub fn new_debug(inputs: ProgramInputs) -> Self {
Self::initialize(inputs, true)
}

fn initialize(inputs: ProgramInputs, in_debug_mode: bool) -> Self {
Self {
system: System::new(MIN_TRACE_LEN),
Expand All @@ -118,16 +140,6 @@ impl Process {
}
}

/// Creates a new process with the provided inputs.
pub fn new(inputs: ProgramInputs) -> Self {
Self::initialize(inputs, false)
}

/// Creates a new process with provided inputs and debug options enabled.
pub fn new_debug(inputs: ProgramInputs) -> Self {
Self::initialize(inputs, true)
}

// CODE BLOCK EXECUTORS
// --------------------------------------------------------------------------------------------

Expand Down Expand Up @@ -165,9 +177,9 @@ impl Process {
let condition = self.start_split_block(block)?;

// execute either the true or the false branch of the split block based on the condition
if condition == Felt::ONE {
if condition == ONE {
self.execute_code_block(block.on_true())?;
} else if condition == Felt::ZERO {
} else if condition == ZERO {
self.execute_code_block(block.on_false())?;
} else {
return Err(ExecutionError::NotBinaryValue(condition));
Expand All @@ -183,22 +195,22 @@ impl Process {
let condition = self.start_loop_block(block)?;

// if the top of the stack is ONE, execute the loop body; otherwise skip the loop body
if condition == Felt::ONE {
if condition == ONE {
// execute the loop body at least once
self.execute_code_block(block.body())?;

// keep executing the loop body until the condition on the top of the stack is no
// longer ONE; each iteration of the loop is preceded by executing REPEAT operation
// which drops the condition from the stack
while self.stack.peek() == Felt::ONE {
while self.stack.peek() == ONE {
self.decoder.repeat();
self.execute_op(Operation::Drop)?;
self.execute_code_block(block.body())?;
}

// end the LOOP block and drop the condition from the stack
self.end_loop_block(block, true)
} else if condition == Felt::ZERO {
} else if condition == ZERO {
// end the LOOP block, but don't drop the condition from the stack because it was
// already dropped when we started the LOOP block
self.end_loop_block(block, false)
Expand Down Expand Up @@ -305,7 +317,7 @@ impl Process {
// operation groups. the groups were are processing are just NOOPs - so, the op group
// value is ZERO
if group_idx < num_batch_groups - 1 {
self.decoder.start_op_group(Felt::ZERO);
self.decoder.start_op_group(ZERO);
}
}

Expand Down
42 changes: 29 additions & 13 deletions processor/src/trace/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use super::{
StackTopState, Vec,
};
use core::slice;
use vm_core::{MIN_STACK_DEPTH, MIN_TRACE_LEN, STACK_TRACE_OFFSET, TRACE_WIDTH};
use vm_core::{MIN_STACK_DEPTH, MIN_TRACE_LEN, STACK_TRACE_OFFSET, TRACE_WIDTH, ZERO};
use winterfell::{EvaluationFrame, Matrix, Serializable, Trace, TraceLayout};

#[cfg(feature = "std")]
Expand All @@ -29,14 +29,18 @@ pub struct AuxTraceHints {
range: RangeCheckerAuxTraceHints,
}

/// TODO: for now this consists only of system register trace, stack trace, range check trace, and
/// auxiliary table trace, but will also need to include the decoder trace.
/// Execution trace which is generated when a program is executed on the VM.
///
/// The trace consists of the following components:
/// - Main traces of System, Decoder, Operand Stack, Range Checker, and Auxiliary Co-Processor
/// components.
/// - Hints used during auxiliary trace segment construction.
/// - Metadata needed by the STARK prover.
pub struct ExecutionTrace {
meta: Vec<u8>,
layout: TraceLayout,
main_trace: Matrix<Felt>,
aux_trace_hints: AuxTraceHints,
// TODO: program hash should be retrieved from decoder trace, but for now we store it explicitly
program_hash: Digest,
}

Expand All @@ -50,11 +54,12 @@ impl ExecutionTrace {
// CONSTRUCTOR
// --------------------------------------------------------------------------------------------
/// Builds an execution trace for the provided process.
pub(super) fn new(process: Process, program_hash: Digest) -> Self {
pub(super) fn new(process: Process) -> Self {
// use program hash to initialize random element generator; this generator will be used
// to inject random values at the end of the trace; using program hash here is OK because
// we are using random values only to stabilize constraint degree, and not to achieve
// we are using random values only to stabilize constraint degrees, and not to achieve
// perfect zero knowledge.
let program_hash: Digest = process.decoder.program_hash().into();
let rng = RandomCoin::new(&program_hash.to_bytes());
let (main_trace, aux_trace_hints) = finalize_trace(process, rng);

Expand All @@ -70,15 +75,14 @@ impl ExecutionTrace {
// PUBLIC ACCESSORS
// --------------------------------------------------------------------------------------------

/// TODO: add docs
/// Returns hash of the program execution of which resulted in this execution trace.
pub fn program_hash(&self) -> Digest {
// TODO: program hash should be read from the decoder trace
self.program_hash
}

/// Returns the initial state of the top 16 stack registers.
pub fn init_stack_state(&self) -> StackTopState {
let mut result = [Felt::ZERO; MIN_STACK_DEPTH];
let mut result = [ZERO; MIN_STACK_DEPTH];
for (i, result) in result.iter_mut().enumerate() {
*result = self.main_trace.get_column(i + STACK_TRACE_OFFSET)[0];
}
Expand All @@ -87,20 +91,28 @@ impl ExecutionTrace {

/// Returns the final state of the top 16 stack registers.
pub fn last_stack_state(&self) -> StackTopState {
let last_step = self.length() - NUM_RAND_ROWS - 1;
let mut result = [Felt::ZERO; MIN_STACK_DEPTH];
let last_step = self.last_step();
let mut result = [ZERO; MIN_STACK_DEPTH];
for (i, result) in result.iter_mut().enumerate() {
*result = self.main_trace.get_column(i + STACK_TRACE_OFFSET)[last_step];
}
result
}

// HELPER METHODS
// --------------------------------------------------------------------------------------------

/// Returns the index of the last row in the trace.
fn last_step(&self) -> usize {
self.length() - NUM_RAND_ROWS - 1
}

// TEST HELPERS
// --------------------------------------------------------------------------------------------
#[cfg(feature = "std")]
#[allow(dead_code)]
pub fn print(&self) {
let mut row = [Felt::ZERO; TRACE_WIDTH];
let mut row = [ZERO; TRACE_WIDTH];
for i in 0..self.length() {
self.main_trace.read_row_into(i, &mut row);
println!("{:?}", row.iter().map(|v| v.as_int()).collect::<Vec<_>>());
Expand Down Expand Up @@ -256,7 +268,11 @@ fn finalize_trace(process: Process, mut rng: RandomCoin) -> (Vec<Vec<Felt>>, Aux

// trace lengths of system and stack components must be equal to the number of executed cycles
assert_eq!(clk, system.trace_len(), "inconsistent system trace lengths");
// TODO: check decoder trace length
assert_eq!(
clk,
decoder.trace_len(),
"inconsistent decoder trace length"
);
assert_eq!(clk, stack.trace_len(), "inconsistent stack trace lengths");

// Get the trace length required to hold all execution trace steps.
Expand Down
4 changes: 2 additions & 2 deletions processor/src/trace/range/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::{
super::{Digest, ExecutionTrace, Process, NUM_RAND_ROWS},
super::{ExecutionTrace, Process, NUM_RAND_ROWS},
Felt, FieldElement, P0_COL_IDX, P1_COL_IDX,
};
use rand_utils::rand_value;
Expand Down Expand Up @@ -108,5 +108,5 @@ fn build_trace(stack: &[u64], operations: Vec<Operation>) -> ExecutionTrace {
let program = CodeBlock::new_span(operations);
process.execute_code_block(&program).unwrap();

ExecutionTrace::new(process, Digest::new([Felt::ZERO; 4]))
ExecutionTrace::new(process)
}

0 comments on commit 257a42e

Please sign in to comment.