Skip to content

Commit

Permalink
refactor: split AdviceInjector::HdWordToMap into `HdwordToMap{WithD…
Browse files Browse the repository at this point in the history
…omain}`
  • Loading branch information
plafer committed Nov 21, 2024
1 parent bc225e4 commit 17b5dff
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 95 deletions.
13 changes: 4 additions & 9 deletions assembly/src/ast/instruction/advice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ use core::fmt;

use vm_core::AdviceInjector;

use crate::{ast::ImmU8, Felt, ZERO};

// ADVICE INJECTOR NODE
// ================================================================================================

Expand All @@ -22,7 +20,7 @@ pub enum AdviceInjectorNode {
PushMtNode,
InsertMem,
InsertHdword,
InsertHdwordImm { domain: ImmU8 },
InsertHdwordWithDomain,
InsertHperm,
PushSignature { kind: SignatureKind },
}
Expand All @@ -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,
Expand All @@ -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}"),
}
Expand Down
38 changes: 6 additions & 32 deletions assembly/src/ast/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,25 +347,13 @@ where
}

pub fn visit_advice_injector<V, T>(
visitor: &mut V,
node: Span<&AdviceInjectorNode>,
_visitor: &mut V,
_node: Span<&AdviceInjectorNode>,
) -> ControlFlow<T>
where
V: ?Sized + Visit<T>,
{
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<V, T>(visitor: &mut V, options: Span<&DebugOptions>) -> ControlFlow<T>
Expand Down Expand Up @@ -793,27 +781,13 @@ where
}

pub fn visit_mut_advice_injector<V, T>(
visitor: &mut V,
node: Span<&mut AdviceInjectorNode>,
_visitor: &mut V,
_node: Span<&mut AdviceInjectorNode>,
) -> ControlFlow<T>
where
V: ?Sized + VisitMut<T>,
{
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<V, T>(
Expand Down
7 changes: 3 additions & 4 deletions assembly/src/parser/grammar.lalrpop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -665,10 +666,8 @@ Inst: Instruction = {

#[inline]
AdviceInjector: Instruction = {
"adv" "." "insert_hdword" <domain:MaybeImm<RawU8>> => {
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),
Expand Down
4 changes: 4 additions & 0 deletions assembly/src/parser/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ pub enum Token<'input> {
Add,
Adv,
InsertHdword,
InsertHdwordWithDomain,
InsertHperm,
InsertMem,
AdvLoadw,
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -514,6 +516,7 @@ impl<'input> Token<'input> {
Token::Add
| Token::Adv
| Token::InsertHdword
| Token::InsertHdwordWithDomain
| Token::InsertHperm
| Token::InsertMem
| Token::AdvLoadw
Expand Down Expand Up @@ -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),
Expand Down
42 changes: 8 additions & 34 deletions core/src/mast/serialization/decorator.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -223,6 +218,7 @@ pub enum EncodedDecoratorVariant {
AdviceInjectorILog2,
AdviceInjectorMemToMap,
AdviceInjectorHdwordToMap,
AdviceInjectorHdwordToMapWithDomain,
AdviceInjectorHpermToMap,
AdviceInjectorFalconSigToStack,
AssemblyOp,
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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());
Expand Down
6 changes: 4 additions & 2 deletions core/src/mast/serialization/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => (),
},
Expand Down Expand Up @@ -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)),
(
Expand Down
23 changes: 18 additions & 5 deletions core/src/operations/decorators/advice.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use core::fmt;

use super::SignatureKind;
use crate::Felt;

// ADVICE INJECTORS
// ================================================================================================
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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),
}
Expand Down
5 changes: 3 additions & 2 deletions docs/src/user_docs/assembly/io_operations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <br> 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

Expand Down
8 changes: 4 additions & 4 deletions miden/tests/integration/operations/decorators/advice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]);
}
Expand Down
12 changes: 9 additions & 3 deletions processor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down Expand Up @@ -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)?;
Expand Down

0 comments on commit 17b5dff

Please sign in to comment.