From 28a2d8ce199e92929d7d8820e677ffc66219160c Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 20 Nov 2024 12:37:11 -0500 Subject: [PATCH] refactor: split `AdviceInjector::HdWordToMap` into `HdwordToMap{WithDomain}` --- assembly/src/ast/instruction/advice.rs | 13 ++---- assembly/src/ast/visit.rs | 38 +++-------------- assembly/src/parser/grammar.lalrpop | 7 ++-- assembly/src/parser/token.rs | 4 ++ core/src/mast/serialization/decorator.rs | 42 ++++--------------- core/src/mast/serialization/tests.rs | 6 ++- core/src/operations/decorators/advice.rs | 23 +++++++--- docs/src/user_docs/assembly/io_operations.md | 5 ++- .../operations/decorators/advice.rs | 8 ++-- processor/src/lib.rs | 12 ++++-- 10 files changed, 63 insertions(+), 95 deletions(-) diff --git a/assembly/src/ast/instruction/advice.rs b/assembly/src/ast/instruction/advice.rs index 472d70e50..b8b7b0d4b 100644 --- a/assembly/src/ast/instruction/advice.rs +++ b/assembly/src/ast/instruction/advice.rs @@ -2,8 +2,6 @@ use core::fmt; use vm_core::AdviceInjector; -use crate::{ast::ImmU8, Felt, ZERO}; - // ADVICE INJECTOR NODE // ================================================================================================ @@ -22,7 +20,7 @@ pub enum AdviceInjectorNode { PushMtNode, InsertMem, InsertHdword, - InsertHdwordImm { domain: ImmU8 }, + InsertHdwordWithDomain, InsertHperm, PushSignature { kind: SignatureKind }, } @@ -38,11 +36,8 @@ impl From<&AdviceInjectorNode> for AdviceInjector { PushMapValN => Self::MapValueToStackN, PushMtNode => Self::MerkleNodeToStack, InsertMem => Self::MemToMap, - InsertHdword => Self::HdwordToMap { domain: ZERO }, - InsertHdwordImm { domain: ImmU8::Value(domain) } => { - Self::HdwordToMap { domain: Felt::from(domain.into_inner()) } - }, - InsertHdwordImm { domain } => panic!("unresolved constant '{domain}'"), + InsertHdword => Self::HdwordToMap, + InsertHdwordWithDomain => Self::HdwordToMapWithDomain, InsertHperm => Self::HpermToMap, PushSignature { kind } => match kind { SignatureKind::RpoFalcon512 => Self::FalconSigToStack, @@ -68,7 +63,7 @@ impl fmt::Display for AdviceInjectorNode { Self::PushMtNode => write!(f, "push_mtnode"), Self::InsertMem => write!(f, "insert_mem"), Self::InsertHdword => write!(f, "insert_hdword"), - Self::InsertHdwordImm { domain } => write!(f, "insert_hdword.{domain}"), + Self::InsertHdwordWithDomain => write!(f, "insert_hdword_d"), Self::InsertHperm => writeln!(f, "insert_hperm"), Self::PushSignature { kind } => write!(f, "push_sig.{kind}"), } diff --git a/assembly/src/ast/visit.rs b/assembly/src/ast/visit.rs index c16388526..9a9afd8d2 100644 --- a/assembly/src/ast/visit.rs +++ b/assembly/src/ast/visit.rs @@ -347,25 +347,13 @@ where } pub fn visit_advice_injector( - visitor: &mut V, - node: Span<&AdviceInjectorNode>, + _visitor: &mut V, + _node: Span<&AdviceInjectorNode>, ) -> ControlFlow where V: ?Sized + Visit, { - match node.into_inner() { - AdviceInjectorNode::InsertHdwordImm { domain: ref imm } => visitor.visit_immediate_u8(imm), - AdviceInjectorNode::PushU64Div - | AdviceInjectorNode::PushExt2intt - | AdviceInjectorNode::PushSmtPeek - | AdviceInjectorNode::PushMapVal - | AdviceInjectorNode::PushMapValN - | AdviceInjectorNode::PushMtNode - | AdviceInjectorNode::InsertMem - | AdviceInjectorNode::InsertHdword - | AdviceInjectorNode::InsertHperm - | AdviceInjectorNode::PushSignature { .. } => ControlFlow::Continue(()), - } + ControlFlow::Continue(()) } pub fn visit_debug_options(visitor: &mut V, options: Span<&DebugOptions>) -> ControlFlow @@ -793,27 +781,13 @@ where } pub fn visit_mut_advice_injector( - visitor: &mut V, - node: Span<&mut AdviceInjectorNode>, + _visitor: &mut V, + _node: Span<&mut AdviceInjectorNode>, ) -> ControlFlow where V: ?Sized + VisitMut, { - match node.into_inner() { - AdviceInjectorNode::InsertHdwordImm { domain: ref mut imm } => { - visitor.visit_mut_immediate_u8(imm) - }, - AdviceInjectorNode::PushU64Div - | AdviceInjectorNode::PushExt2intt - | AdviceInjectorNode::PushSmtPeek - | AdviceInjectorNode::PushMapVal - | AdviceInjectorNode::PushMapValN - | AdviceInjectorNode::PushMtNode - | AdviceInjectorNode::InsertMem - | AdviceInjectorNode::InsertHdword - | AdviceInjectorNode::InsertHperm - | AdviceInjectorNode::PushSignature { .. } => ControlFlow::Continue(()), - } + ControlFlow::Continue(()) } pub fn visit_mut_debug_options( diff --git a/assembly/src/parser/grammar.lalrpop b/assembly/src/parser/grammar.lalrpop index 3717d822b..f0b6d882f 100644 --- a/assembly/src/parser/grammar.lalrpop +++ b/assembly/src/parser/grammar.lalrpop @@ -43,6 +43,7 @@ extern { "add" => Token::Add, "adv" => Token::Adv, "insert_hdword" => Token::InsertHdword, + "insert_hdword_d" => Token::InsertHdwordWithDomain, "insert_hperm" => Token::InsertHperm, "insert_mem" => Token::InsertMem, "adv_loadw" => Token::AdvLoadw, @@ -665,10 +666,8 @@ Inst: Instruction = { #[inline] AdviceInjector: Instruction = { - "adv" "." "insert_hdword" > => { - domain.map(|domain| Instruction::AdvInject(AdviceInjectorNode::InsertHdwordImm { domain })) - .unwrap_or(Instruction::AdvInject(AdviceInjectorNode::InsertHdword)) - }, + "adv" "." "insert_hdword" => Instruction::AdvInject(AdviceInjectorNode::InsertHdword), + "adv" "." "insert_hdword_d" => Instruction::AdvInject(AdviceInjectorNode::InsertHdwordWithDomain), "adv" "." "insert_hperm" => Instruction::AdvInject(AdviceInjectorNode::InsertHperm), "adv" "." "insert_mem" => Instruction::AdvInject(AdviceInjectorNode::InsertMem), "adv" "." "push_ext2intt" => Instruction::AdvInject(AdviceInjectorNode::PushExt2intt), diff --git a/assembly/src/parser/token.rs b/assembly/src/parser/token.rs index b55201f1a..94c4f73fd 100644 --- a/assembly/src/parser/token.rs +++ b/assembly/src/parser/token.rs @@ -138,6 +138,7 @@ pub enum Token<'input> { Add, Adv, InsertHdword, + InsertHdwordWithDomain, InsertHperm, InsertMem, AdvLoadw, @@ -322,6 +323,7 @@ impl fmt::Display for Token<'_> { Token::Add => write!(f, "add"), Token::Adv => write!(f, "adv"), Token::InsertHdword => write!(f, "insert_hdword"), + Token::InsertHdwordWithDomain => write!(f, "insert_hdword_d"), Token::InsertHperm => write!(f, "insert_hperm"), Token::InsertMem => write!(f, "insert_mem"), Token::AdvLoadw => write!(f, "adv_loadw"), @@ -514,6 +516,7 @@ impl<'input> Token<'input> { Token::Add | Token::Adv | Token::InsertHdword + | Token::InsertHdwordWithDomain | Token::InsertHperm | Token::InsertMem | Token::AdvLoadw @@ -658,6 +661,7 @@ impl<'input> Token<'input> { ("add", Token::Add), ("adv", Token::Adv), ("insert_hdword", Token::InsertHdword), + ("insert_hdword_d", Token::InsertHdwordWithDomain), ("insert_hperm", Token::InsertHperm), ("insert_mem", Token::InsertMem), ("adv_loadw", Token::AdvLoadw), diff --git a/core/src/mast/serialization/decorator.rs b/core/src/mast/serialization/decorator.rs index 0004e9080..5815b988c 100644 --- a/core/src/mast/serialization/decorator.rs +++ b/core/src/mast/serialization/decorator.rs @@ -1,6 +1,5 @@ use alloc::vec::Vec; -use miden_crypto::Felt; use num_derive::{FromPrimitive, ToPrimitive}; use num_traits::{FromPrimitive, ToPrimitive}; use winter_utils::{ @@ -94,14 +93,10 @@ impl DecoratorInfo { Ok(Decorator::Advice(AdviceInjector::MemToMap)) }, EncodedDecoratorVariant::AdviceInjectorHdwordToMap => { - let domain = data_reader.read_u64()?; - let domain = Felt::try_from(domain).map_err(|err| { - DeserializationError::InvalidValue(format!( - "Error when deserializing HdwordToMap decorator domain: {err}" - )) - })?; - - Ok(Decorator::Advice(AdviceInjector::HdwordToMap { domain })) + Ok(Decorator::Advice(AdviceInjector::HdwordToMap)) + }, + EncodedDecoratorVariant::AdviceInjectorHdwordToMapWithDomain => { + Ok(Decorator::Advice(AdviceInjector::HdwordToMapWithDomain)) }, EncodedDecoratorVariant::AdviceInjectorHpermToMap => { Ok(Decorator::Advice(AdviceInjector::HpermToMap)) @@ -223,6 +218,7 @@ pub enum EncodedDecoratorVariant { AdviceInjectorILog2, AdviceInjectorMemToMap, AdviceInjectorHdwordToMap, + AdviceInjectorHdwordToMapWithDomain, AdviceInjectorHpermToMap, AdviceInjectorFalconSigToStack, AssemblyOp, @@ -268,7 +264,8 @@ impl From<&Decorator> for EncodedDecoratorVariant { AdviceInjector::U32Cto => Self::AdviceInjectorU32Cto, AdviceInjector::ILog2 => Self::AdviceInjectorILog2, AdviceInjector::MemToMap => Self::AdviceInjectorMemToMap, - AdviceInjector::HdwordToMap { domain: _ } => Self::AdviceInjectorHdwordToMap, + AdviceInjector::HdwordToMap => Self::AdviceInjectorHdwordToMap, + AdviceInjector::HdwordToMapWithDomain => Self::AdviceInjectorHdwordToMapWithDomain, AdviceInjector::HpermToMap => Self::AdviceInjectorHpermToMap, AdviceInjector::FalconSigToStack => Self::AdviceInjectorFalconSigToStack, }, @@ -331,30 +328,7 @@ impl DecoratorDataBuilder { let data_offset = self.decorator_data.len() as DecoratorDataOffset; match decorator { - Decorator::Advice(advice_injector) => match advice_injector { - AdviceInjector::HdwordToMap { domain } => { - self.decorator_data.extend(domain.as_int().to_le_bytes()); - - Some(data_offset) - }, - AdviceInjector::MapValueToStack - | AdviceInjector::MapValueToStackN - | AdviceInjector::MerkleNodeMerge - | AdviceInjector::MerkleNodeToStack - | AdviceInjector::UpdateMerkleNode - | AdviceInjector::U64Div - | AdviceInjector::Ext2Inv - | AdviceInjector::Ext2Intt - | AdviceInjector::SmtPeek - | AdviceInjector::U32Clz - | AdviceInjector::U32Ctz - | AdviceInjector::U32Clo - | AdviceInjector::U32Cto - | AdviceInjector::ILog2 - | AdviceInjector::MemToMap - | AdviceInjector::HpermToMap - | AdviceInjector::FalconSigToStack => None, - }, + Decorator::Advice(_) => None, Decorator::AsmOp(assembly_op) => { self.decorator_data.push(assembly_op.num_cycles()); self.decorator_data.write_bool(assembly_op.should_break()); diff --git a/core/src/mast/serialization/tests.rs b/core/src/mast/serialization/tests.rs index 0682cb28f..0fb2c2036 100644 --- a/core/src/mast/serialization/tests.rs +++ b/core/src/mast/serialization/tests.rs @@ -125,7 +125,8 @@ fn confirm_operation_and_decorator_structure() { AdviceInjector::U32Cto => (), AdviceInjector::ILog2 => (), AdviceInjector::MemToMap => (), - AdviceInjector::HdwordToMap { domain: _ } => (), + AdviceInjector::HdwordToMap => (), + AdviceInjector::HdwordToMapWithDomain => (), AdviceInjector::HpermToMap => (), AdviceInjector::FalconSigToStack => (), }, @@ -257,7 +258,8 @@ fn serialize_deserialize_all_nodes() { (10, Decorator::Advice(AdviceInjector::U32Cto)), (10, Decorator::Advice(AdviceInjector::ILog2)), (10, Decorator::Advice(AdviceInjector::MemToMap)), - (10, Decorator::Advice(AdviceInjector::HdwordToMap { domain: Felt::new(423) })), + (10, Decorator::Advice(AdviceInjector::HdwordToMap)), + (10, Decorator::Advice(AdviceInjector::HdwordToMapWithDomain)), (15, Decorator::Advice(AdviceInjector::HpermToMap)), (15, Decorator::Advice(AdviceInjector::FalconSigToStack)), ( diff --git a/core/src/operations/decorators/advice.rs b/core/src/operations/decorators/advice.rs index 66210952b..4cc60ec4b 100644 --- a/core/src/operations/decorators/advice.rs +++ b/core/src/operations/decorators/advice.rs @@ -1,7 +1,6 @@ use core::fmt; use super::SignatureKind; -use crate::Felt; // ADVICE INJECTORS // ================================================================================================ @@ -245,9 +244,22 @@ pub enum AdviceInjector { /// Operand stack: [B, A, ...] /// Advice map: {KEY: [a0, a1, a2, a3, b0, b1, b2, b3]} /// - /// Where KEY is computed as hash(A || B, domain), where domain is provided via the immediate - /// value. - HdwordToMap { domain: Felt }, + /// Where KEY is computed as hash(A || B, domain=0) + HdwordToMap, + + /// Reads two word from the operand stack and inserts them into the advice map under the key + /// defined by the hash of these words (using `d` as the domain). + /// + /// Inputs: + /// Operand stack: [B, A, d, ...] + /// Advice map: {...} + /// + /// Outputs: + /// Operand stack: [B, A, d, ...] + /// Advice map: {KEY: [a0, a1, a2, a3, b0, b1, b2, b3]} + /// + /// Where KEY is computed as hash(A || B, d). + HdwordToMapWithDomain, /// Reads three words from the operand stack and inserts the top two words into the advice map /// under the key defined by applying an RPO permutation to all three words. @@ -306,7 +318,8 @@ impl fmt::Display for AdviceInjector { Self::U32Cto => write!(f, "u32cto"), Self::ILog2 => write!(f, "ilog2"), Self::MemToMap => write!(f, "mem_to_map"), - Self::HdwordToMap { domain } => write!(f, "hdword_to_map.{domain}"), + Self::HdwordToMap => write!(f, "hdword_to_map"), + Self::HdwordToMapWithDomain => write!(f, "hdword_to_map_with_domain"), Self::HpermToMap => write!(f, "hperm_to_map"), Self::FalconSigToStack => write!(f, "sig_to_stack.{}", SignatureKind::RpoFalcon512), } diff --git a/docs/src/user_docs/assembly/io_operations.md b/docs/src/user_docs/assembly/io_operations.md index ca180adc6..23e31bd18 100644 --- a/docs/src/user_docs/assembly/io_operations.md +++ b/docs/src/user_docs/assembly/io_operations.md @@ -57,8 +57,9 @@ Advice injectors fall into two categories: (1) injectors which push new data ont | adv.push_sig.*kind* | [K, M, ...] | [K, M, ...] | Pushes values onto the advice stack which are required for verification of a DSA with scheme specified by *kind* against the public key commitment $K$ and message $M$. | | adv.push_smtpeek | [K, R, ... ] | [K, R, ... ] | Pushes value onto the advice stack which is associated with key $K$ in a Sparse Merkle Tree with root $R$. | | adv.insert_mem | [K, a, b, ... ] | [K, a, b, ... ] | Reads words $data \leftarrow mem[a] .. mem[b]$ from memory, and save the data into $advice\_map[K] \leftarrow data$. | -| adv.insert_hdword
adv.insert_hdword.*d* | [B, A, ... ] | [B, A, ... ] | Reads top two words from the stack, computes a key as $K \leftarrow hash(A || b, d)$, and saves the data into $advice\_map[K] \leftarrow [A, B]$. $d$ is an optional domain value which can be between $0$ and $255$, default value $0$. | -| adv.insert_hperm | [B, A, C, ...] | [B, A, C, ...] | Reads top three words from the stack, computes a key as $K \leftarrow permute(C, A, B).digest$, and saves data into $advice\_mpa[K] \leftarrow [A, B]$. | +| adv.insert_hdword | [B, A, ... ] | [B, A, ... ] | Reads top two words from the stack, computes a key as $K \leftarrow hash(A || b, domain=0)$, and saves the data into $advice\_map[K] \leftarrow [A, B]$. | +| adv.insert_hdword_d | [B, A, d, ... ] | [B, A, d, ... ] | Reads top two words from the stack, computes a key as $K \leftarrow hash(A || b, domain=d)$, and saves the data into $advice\_map[K] \leftarrow [A, B]$. $d$ is the domain value, where changing the domain changes the resulting hash given the same `A` and `B`. | +| adv.insert_hperm | [B, A, C, ...] | [B, A, C, ...] | Reads top three words from the stack, computes a key as $K \leftarrow permute(C, A, B).digest$, and saves data into $advice\_map[K] \leftarrow [A, B]$. | ### Random access memory diff --git a/miden/tests/integration/operations/decorators/advice.rs b/miden/tests/integration/operations/decorators/advice.rs index 868fb338a..3e3c6a6bd 100644 --- a/miden/tests/integration/operations/decorators/advice.rs +++ b/miden/tests/integration/operations/decorators/advice.rs @@ -305,13 +305,13 @@ fn advice_insert_hdword() { // --- test hashing with domain ------------------------------------------- let source: &str = " begin - # stack: [1, 2, 3, 4, 5, 6, 7, 8, ...] + # stack: [1, 2, 3, 4, 5, 6, 7, 8, 9, ...] # hash and insert top two words into the advice map - adv.insert_hdword.3 + adv.insert_hdword_d # manually compute the hash of the two words - push.0.3.0.0 + push.0.9.0.0 swapw.2 swapw hperm dropw swapw dropw @@ -325,7 +325,7 @@ fn advice_insert_hdword() { adv_push.8 swapdw dropw dropw end"; - let stack_inputs = [8, 7, 6, 5, 4, 3, 2, 1]; + let stack_inputs = [9, 8, 7, 6, 5, 4, 3, 2, 1]; let test = build_test!(source, &stack_inputs); test.expect_stack(&[1, 2, 3, 4, 5, 6, 7, 8]); } diff --git a/processor/src/lib.rs b/processor/src/lib.rs index c67a3d2ec..6f0a7f88a 100644 --- a/processor/src/lib.rs +++ b/processor/src/lib.rs @@ -586,7 +586,7 @@ impl Process { match decorator { Decorator::Advice(injector) => { let advice_provider = host.advice_provider_mut(); - let process_state: ProcessState = self.into(); + let process_state: ProcessState = (&*self).into(); match injector { AdviceInjector::MerkleNodeMerge => { advice_provider.merge_merkle_nodes(process_state)?; @@ -622,8 +622,14 @@ impl Process { AdviceInjector::MemToMap => { advice_provider.insert_mem_values_into_adv_map(process_state)?; }, - AdviceInjector::HdwordToMap { domain } => { - advice_provider.insert_hdword_into_adv_map(process_state, *domain)?; + AdviceInjector::HdwordToMap => { + advice_provider.insert_hdword_into_adv_map(process_state, ZERO)?; + }, + AdviceInjector::HdwordToMapWithDomain => { + // TODO(plafer): get domain from stack + let domain = self.stack.get(8); + advice_provider.insert_hdword_into_adv_map(process_state, domain)?; + todo!() }, AdviceInjector::HpermToMap => { advice_provider.insert_hperm_into_adv_map(process_state)?;