From 9bb91d8c9bae32945f9fd7d257efba1db32ef0f0 Mon Sep 17 00:00:00 2001 From: Jan Ferdinand Sauer Date: Fri, 29 Sep 2023 18:09:49 +0200 Subject: [PATCH 1/4] add methods to determine an instruction's effect on op stack size --- triton-vm/src/instruction.rs | 179 ++++++++++++++++++++++++++++++----- triton-vm/src/program.rs | 2 +- triton-vm/src/vm.rs | 13 ++- 3 files changed, 164 insertions(+), 30 deletions(-) diff --git a/triton-vm/src/instruction.rs b/triton-vm/src/instruction.rs index 6632c0db..4f468e9f 100644 --- a/triton-vm/src/instruction.rs +++ b/triton-vm/src/instruction.rs @@ -280,6 +280,69 @@ impl AnInstruction { WriteIo => WriteIo, } } + + pub const fn grows_op_stack(&self) -> bool { + self.op_stack_size_influence() > 0 + } + + pub const fn does_not_change_op_stack_size(&self) -> bool { + self.op_stack_size_influence() == 0 + } + + pub const fn shrinks_op_stack(&self) -> bool { + self.op_stack_size_influence() < 0 + } + + pub const fn op_stack_size_influence(&self) -> i32 { + match self { + Pop => -1, + Push(_) => 1, + Divine => 1, + Dup(_) => 1, + Swap(_) => 0, + Nop => 0, + Skiz => -1, + Call(_) => 0, + Return => 0, + Recurse => 0, + Assert => -1, + Halt => 0, + ReadMem => 1, + WriteMem => -1, + Hash => 0, + DivineSibling => 0, + AssertVector => 0, + AbsorbInit => 0, + Absorb => 0, + Squeeze => 0, + Add => -1, + Mul => -1, + Invert => 0, + Eq => -1, + Split => 1, + Lt => -1, + And => -1, + Xor => -1, + Log2Floor => 0, + Pow => -1, + Div => 0, + PopCount => 0, + XxAdd => 0, + XxMul => 0, + XInvert => 0, + XbMul => -1, + ReadIo => 1, + WriteIo => -1, + } + } + + /// Indicates whether the instruction operates on base field elements that are also u32s. + pub fn is_u32_instruction(&self) -> bool { + matches!( + self, + Split | Lt | And | Xor | Log2Floor | Pow | Div | PopCount + ) + } } impl Display for AnInstruction { @@ -309,25 +372,10 @@ impl Instruction { self.arg().is_some() } - /// Indicates whether the instruction shrinks the operational stack. - pub fn shrinks_op_stack(&self) -> bool { - matches!(self, Pop | Skiz | Assert) - || matches!(self, WriteMem | WriteIo) - || matches!(self, Add | Mul | Eq | XbMul) - || matches!(self, And | Xor | Lt | Pow) - } - - /// Indicates whether the instruction operates on base field elements that are also u32s. - pub fn is_u32_instruction(&self) -> bool { - matches!( - self, - Split | Lt | And | Xor | Log2Floor | Pow | Div | PopCount - ) - } - /// Change the argument of the instruction, if it has one. /// Returns `None` if the instruction does not have an argument or /// if the argument is out of range. + #[must_use] pub fn change_arg(&self, new_arg: BFieldElement) -> Option { let maybe_ord_16 = new_arg.value().try_into(); match self { @@ -432,7 +480,7 @@ const fn all_instructions_without_args() -> [AnInstruction; Instr Push(BFIELD_ZERO), Divine, Dup(ST0), - Swap(ST0), + Swap(ST1), Nop, Skiz, Call(BFIELD_ZERO), @@ -537,6 +585,7 @@ impl TryFrom for InstructionBit { #[cfg(test)] mod instruction_tests { + use std::cmp::Ordering; use std::collections::HashMap; use itertools::Itertools; @@ -547,18 +596,16 @@ mod instruction_tests { use strum::EnumCount; use strum::IntoEnumIterator; use twenty_first::shared_math::b_field_element::BFieldElement; + use twenty_first::shared_math::b_field_element::BFIELD_ZERO; + use twenty_first::shared_math::digest::Digest; - use crate::instruction::all_instruction_names; - use crate::instruction::all_instructions_without_args; - use crate::instruction::stringify_instructions; - use crate::instruction::Instruction; - use crate::instruction::InstructionBit; - use crate::instruction::ALL_INSTRUCTIONS; - use crate::op_stack::OpStackElement::*; + use crate::instruction::*; + use crate::op_stack::NUM_OP_STACK_REGISTERS; use crate::triton_asm; use crate::triton_program; - - use super::AnInstruction::*; + use crate::vm::triton_vm_tests::test_program_for_call_recurse_return; + use crate::NonDeterminism; + use crate::Program; #[test] fn opcodes_are_unique() { @@ -742,4 +789,84 @@ mod instruction_tests { let code = stringify_instructions(&instructions); println!("{code}"); } + + #[test] + fn instructions_act_on_op_stack_as_indicated() { + for test_instruction in all_instructions_without_args() { + let (program, stack_size_before_test_instruction) = + construct_test_program_for_instruction(test_instruction); + let stack_size_after_test_instruction = terminal_op_stack_size_for_program(program); + + let stack_size_difference = + stack_size_after_test_instruction.cmp(&stack_size_before_test_instruction); + assert_op_stack_size_changed_as_instruction_indicates( + test_instruction, + stack_size_difference, + ); + } + } + + fn construct_test_program_for_instruction( + instruction: AnInstruction, + ) -> (Program, usize) { + match instruction_requires_jump_stack_setup(instruction) { + true => program_with_jump_stack_setup_for_instruction(), + false => program_without_jump_stack_setup_for_instruction(instruction), + } + } + + fn instruction_requires_jump_stack_setup(instruction: Instruction) -> bool { + matches!(instruction, Call(_) | Return | Recurse) + } + + fn program_with_jump_stack_setup_for_instruction() -> (Program, usize) { + let program = test_program_for_call_recurse_return().program; + let stack_size = NUM_OP_STACK_REGISTERS; + (program, stack_size) + } + + fn program_without_jump_stack_setup_for_instruction( + test_instruction: AnInstruction, + ) -> (Program, usize) { + let num_push_instructions = 10; + let push_instructions = triton_asm![push 1; num_push_instructions]; + let program = triton_program!({&push_instructions} {test_instruction} nop halt); + + let stack_size_when_reaching_test_instruction = + NUM_OP_STACK_REGISTERS + num_push_instructions; + (program, stack_size_when_reaching_test_instruction) + } + + fn terminal_op_stack_size_for_program(program: Program) -> usize { + let public_input = vec![BFIELD_ZERO].into(); + let mock_digests = vec![Digest::default()]; + let non_determinism: NonDeterminism<_> = vec![BFIELD_ZERO].into(); + let non_determinism = non_determinism.with_digests(mock_digests); + + let terminal_state = program + .debug_terminal_state(public_input, non_determinism, None, None) + .unwrap(); + terminal_state.op_stack.stack.len() + } + + fn assert_op_stack_size_changed_as_instruction_indicates( + test_instruction: AnInstruction, + stack_size_difference: Ordering, + ) { + assert_eq!( + stack_size_difference == Ordering::Less, + test_instruction.shrinks_op_stack(), + "{test_instruction}" + ); + assert_eq!( + stack_size_difference == Ordering::Equal, + test_instruction.does_not_change_op_stack_size(), + "{test_instruction}" + ); + assert_eq!( + stack_size_difference == Ordering::Greater, + test_instruction.grows_op_stack(), + "{test_instruction}" + ); + } } diff --git a/triton-vm/src/program.rs b/triton-vm/src/program.rs index 0d9e2d24..7239b221 100644 --- a/triton-vm/src/program.rs +++ b/triton-vm/src/program.rs @@ -386,7 +386,7 @@ impl ProfileLine { } } -#[derive(Clone, Debug, PartialEq, Eq, BFieldCodec)] +#[derive(Clone, Default, Debug, PartialEq, Eq, BFieldCodec)] pub struct PublicInput { pub individual_tokens: Vec, } diff --git a/triton-vm/src/vm.rs b/triton-vm/src/vm.rs index d932b15b..cc70e123 100644 --- a/triton-vm/src/vm.rs +++ b/triton-vm/src/vm.rs @@ -944,9 +944,16 @@ pub mod triton_vm_tests { } pub(crate) fn test_program_for_call_recurse_return() -> ProgramAndInput { - ProgramAndInput::without_input( - triton_program!(push 2 call label halt label: push -1 add dup 0 skiz recurse return), - ) + ProgramAndInput::without_input(triton_program!( + push 2 + call label + pop halt + label: + push -1 add dup 0 + skiz + recurse + return + )) } pub(crate) fn test_program_for_write_mem_read_mem() -> ProgramAndInput { From 5dc7aae9a1ad7186d65d56ae49a67133f8ef0f69 Mon Sep 17 00:00:00 2001 From: Jan Ferdinand Sauer Date: Fri, 29 Sep 2023 18:47:52 +0200 Subject: [PATCH 2/4] propagate methods about op stack size change to `LabelledInstruction` --- triton-vm/src/instruction.rs | 56 ++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/triton-vm/src/instruction.rs b/triton-vm/src/instruction.rs index 4f468e9f..873cb816 100644 --- a/triton-vm/src/instruction.rs +++ b/triton-vm/src/instruction.rs @@ -50,6 +50,27 @@ pub enum LabelledInstruction { Label(String), } +impl LabelledInstruction { + pub const fn grows_op_stack(&self) -> bool { + self.op_stack_size_influence() > 0 + } + + pub const fn does_not_change_op_stack_size(&self) -> bool { + self.op_stack_size_influence() == 0 + } + + pub const fn shrinks_op_stack(&self) -> bool { + self.op_stack_size_influence() < 0 + } + + pub const fn op_stack_size_influence(&self) -> i32 { + match self { + LabelledInstruction::Instruction(instruction) => instruction.op_stack_size_influence(), + LabelledInstruction::Label(_) => 0, + } + } +} + impl Display for LabelledInstruction { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -869,4 +890,39 @@ mod instruction_tests { "{test_instruction}" ); } + + #[test] + fn labelled_instructions_act_on_op_stack_as_indicated() { + for test_instruction in all_instructions_without_args() { + let labelled_instruction = + test_instruction.map_call_address(|_| "dummy_label".to_string()); + let labelled_instruction = LabelledInstruction::Instruction(labelled_instruction); + + assert_eq!( + test_instruction.op_stack_size_influence(), + labelled_instruction.op_stack_size_influence() + ); + assert_eq!( + test_instruction.grows_op_stack(), + labelled_instruction.grows_op_stack() + ); + assert_eq!( + test_instruction.does_not_change_op_stack_size(), + labelled_instruction.does_not_change_op_stack_size() + ); + assert_eq!( + test_instruction.shrinks_op_stack(), + labelled_instruction.shrinks_op_stack() + ); + } + } + + #[test] + fn labels_indicate_no_change_to_op_stack() { + let labelled_instruction = LabelledInstruction::Label("dummy_label".to_string()); + assert_eq!(0, labelled_instruction.op_stack_size_influence()); + assert!(!labelled_instruction.grows_op_stack()); + assert!(labelled_instruction.does_not_change_op_stack_size()); + assert!(!labelled_instruction.shrinks_op_stack()); + } } From 19ae021e3c221f9272dfd5a65e61625614c0d5a6 Mon Sep 17 00:00:00 2001 From: Jan Ferdinand Sauer Date: Fri, 29 Sep 2023 19:10:52 +0200 Subject: [PATCH 3/4] deal with illegal arguments in test instruction explicitly --- triton-vm/src/instruction.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/triton-vm/src/instruction.rs b/triton-vm/src/instruction.rs index 873cb816..929451da 100644 --- a/triton-vm/src/instruction.rs +++ b/triton-vm/src/instruction.rs @@ -501,7 +501,7 @@ const fn all_instructions_without_args() -> [AnInstruction; Instr Push(BFIELD_ZERO), Divine, Dup(ST0), - Swap(ST1), + Swap(ST0), Nop, Skiz, Call(BFIELD_ZERO), @@ -814,6 +814,7 @@ mod instruction_tests { #[test] fn instructions_act_on_op_stack_as_indicated() { for test_instruction in all_instructions_without_args() { + let test_instruction = replace_illegal_arguments_in_instruction(test_instruction); let (program, stack_size_before_test_instruction) = construct_test_program_for_instruction(test_instruction); let stack_size_after_test_instruction = terminal_op_stack_size_for_program(program); @@ -827,6 +828,15 @@ mod instruction_tests { } } + fn replace_illegal_arguments_in_instruction( + instruction: AnInstruction, + ) -> AnInstruction { + match instruction { + Swap(ST0) => Swap(ST1), + _ => instruction, + } + } + fn construct_test_program_for_instruction( instruction: AnInstruction, ) -> (Program, usize) { From 2c18cdbf1060c719468a70d7ee6bcb2877b394c2 Mon Sep 17 00:00:00 2001 From: Jan Ferdinand Sauer Date: Fri, 29 Sep 2023 19:30:12 +0200 Subject: [PATCH 4/4] increase readability: change negative method name into positive one --- triton-vm/src/instruction.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/triton-vm/src/instruction.rs b/triton-vm/src/instruction.rs index 929451da..2479c9cb 100644 --- a/triton-vm/src/instruction.rs +++ b/triton-vm/src/instruction.rs @@ -55,8 +55,8 @@ impl LabelledInstruction { self.op_stack_size_influence() > 0 } - pub const fn does_not_change_op_stack_size(&self) -> bool { - self.op_stack_size_influence() == 0 + pub const fn changes_op_stack_size(&self) -> bool { + self.op_stack_size_influence() != 0 } pub const fn shrinks_op_stack(&self) -> bool { @@ -306,8 +306,8 @@ impl AnInstruction { self.op_stack_size_influence() > 0 } - pub const fn does_not_change_op_stack_size(&self) -> bool { - self.op_stack_size_influence() == 0 + pub const fn changes_op_stack_size(&self) -> bool { + self.op_stack_size_influence() != 0 } pub const fn shrinks_op_stack(&self) -> bool { @@ -891,7 +891,7 @@ mod instruction_tests { ); assert_eq!( stack_size_difference == Ordering::Equal, - test_instruction.does_not_change_op_stack_size(), + !test_instruction.changes_op_stack_size(), "{test_instruction}" ); assert_eq!( @@ -917,8 +917,8 @@ mod instruction_tests { labelled_instruction.grows_op_stack() ); assert_eq!( - test_instruction.does_not_change_op_stack_size(), - labelled_instruction.does_not_change_op_stack_size() + test_instruction.changes_op_stack_size(), + labelled_instruction.changes_op_stack_size() ); assert_eq!( test_instruction.shrinks_op_stack(), @@ -932,7 +932,7 @@ mod instruction_tests { let labelled_instruction = LabelledInstruction::Label("dummy_label".to_string()); assert_eq!(0, labelled_instruction.op_stack_size_influence()); assert!(!labelled_instruction.grows_op_stack()); - assert!(labelled_instruction.does_not_change_op_stack_size()); + assert!(!labelled_instruction.changes_op_stack_size()); assert!(!labelled_instruction.shrinks_op_stack()); } }