Merge pull request #1572 from 0xPolygonMiden/plafer-1457-refactor-hos…

…t-advice-provider

Refactor `Host` and `AdviceProvider`

CHANGELOG.md

@@ -3,6 +3,7 @@
 #### Changes
 - [BREAKING] `Process` no longer takes ownership of the `Host` (#1571)
 - [BREAKING] `ProcessState` was converted from a trait to a struct (#1571)
+- [BREAKING] `Host` and `AdviceProvider` traits simplified (#1572)
 
 #### Enhancements
 - Added `miden_core::mast::MastForest::advice_map` to load it into the advice provider before the `MastForest` execution (#1574).
@@ -34,6 +35,7 @@
 - [BREAKING] `DYNCALL` operation fixed, and now expects a memory address pointing to the procedure hash (#1535).
 - Permit child `MastNodeId`s to exceed the `MastNodeId`s of their parents (#1542).
 - Don't validate export names on `Library` deserialization (#1554)
+- Compile advice injectors down to `Emit` operations (#1581)
 
 #### Fixes
 

assembly/src/assembler/basic_block_builder.rs

@@ -2,7 +2,8 @@ use alloc::{borrow::Borrow, string::ToString, vec::Vec};
 
 use vm_core::{
     mast::{DecoratorId, MastNodeId},
-    AdviceInjector, AssemblyOp, Decorator, Operation,
+    sys_events::SystemEvent,
+    AssemblyOp, Decorator, Operation,
 };
 
 use super::{mast_forest_builder::MastForestBuilder, BodyWrapper, DecoratorList, ProcedureContext};
@@ -93,6 +94,12 @@ impl BasicBlockBuilder<'_> {
         let new_len = self.ops.len() + n;
         self.ops.resize(new_len, op);
     }
+
+    /// Converts the system event into its corresponding event ID, and adds an `Emit` operation
+    /// to the list of basic block operations.
+    pub fn push_system_event(&mut self, sys_event: SystemEvent) {
+        self.push_op(Operation::Emit(sys_event.into_event_id()))
+    }
 }
 
 /// Decorators
@@ -105,11 +112,6 @@ impl BasicBlockBuilder<'_> {
         Ok(())
     }
 
-    /// Adds the specified advice injector to the list of basic block decorators.
-    pub fn push_advice_injector(&mut self, injector: AdviceInjector) -> Result<(), AssemblyError> {
-        self.push_decorator(Decorator::Advice(injector))
-    }
-
     /// Adds an AsmOp decorator to the list of basic block decorators.
     ///
     /// This indicates that the provided instruction should be tracked and the cycle count for

assembly/src/assembler/instruction/adv_ops.rs

@@ -1,7 +1,7 @@
 use vm_core::Operation;
 
 use super::{validate_param, BasicBlockBuilder};
-use crate::{ast::AdviceInjectorNode, AssemblyError, ADVICE_READ_LIMIT};
+use crate::{AssemblyError, ADVICE_READ_LIMIT};
 
 // NON-DETERMINISTIC (ADVICE) INPUTS
 // ================================================================================================
@@ -18,14 +18,3 @@ pub fn adv_push(block_builder: &mut BasicBlockBuilder, n: u8) -> Result<(), Asse
     block_builder.push_op_many(Operation::AdvPop, n as usize);
     Ok(())
 }
-
-// ADVICE INJECTORS
-// ================================================================================================
-
-/// Appends advice injector decorator to the span.
-pub fn adv_inject(
-    block_builder: &mut BasicBlockBuilder,
-    injector: &AdviceInjectorNode,
-) -> Result<(), AssemblyError> {
-    block_builder.push_advice_injector(injector.into())
-}

assembly/src/assembler/instruction/crypto_ops.rs

@@ -1,4 +1,4 @@
-use vm_core::{AdviceInjector, Felt, Operation::*};
+use vm_core::{sys_events::SystemEvent, Felt, Operation::*};
 
 use super::BasicBlockBuilder;
 use crate::AssemblyError;
@@ -113,10 +113,10 @@ pub(super) fn hmerge(block_builder: &mut BasicBlockBuilder) {
 /// - root of the tree, 4 elements.
 ///
 /// This operation takes 9 VM cycles.
-pub(super) fn mtree_get(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyError> {
+pub(super) fn mtree_get(block_builder: &mut BasicBlockBuilder) {
     // stack: [d, i, R, ...]
     // pops the value of the node we are looking for from the advice stack
-    read_mtree_node(block_builder)?;
+    read_mtree_node(block_builder);
     #[rustfmt::skip]
     let ops = [
         // verify the node V for root R with depth d and index i
@@ -128,8 +128,6 @@ pub(super) fn mtree_get(block_builder: &mut BasicBlockBuilder) -> Result<(), Ass
         MovUp4, Drop, MovUp4, Drop,
     ];
     block_builder.push_ops(ops);
-
-    Ok(())
 }
 
 /// Appends the MRUPDATE op with a parameter of "false" and stack manipulations to the span block
@@ -165,18 +163,16 @@ pub(super) fn mtree_set(block_builder: &mut BasicBlockBuilder) -> Result<(), Ass
 /// It is not checked whether the provided roots exist as Merkle trees in the advide providers.
 ///
 /// This operation takes 16 VM cycles.
-pub(super) fn mtree_merge(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyError> {
+pub(super) fn mtree_merge(block_builder: &mut BasicBlockBuilder) {
     // stack input:  [R_rhs, R_lhs, ...]
     // stack output: [R_merged, ...]
 
     // invoke the advice provider function to merge 2 Merkle trees defined by the roots on the top
     // of the operand stack
-    block_builder.push_advice_injector(AdviceInjector::MerkleNodeMerge)?;
+    block_builder.push_system_event(SystemEvent::MerkleNodeMerge);
 
     // perform the `hmerge`, updating the operand stack
-    hmerge(block_builder);
-
-    Ok(())
+    hmerge(block_builder)
 }
 
 // MERKLE TREES - HELPERS
@@ -199,20 +195,18 @@ pub(super) fn mtree_merge(block_builder: &mut BasicBlockBuilder) -> Result<(), A
 /// - new value of the node, 4 elements (only in the case of mtree_set)
 ///
 /// This operation takes 4 VM cycles.
-fn read_mtree_node(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyError> {
+fn read_mtree_node(block_builder: &mut BasicBlockBuilder) {
     // The stack should be arranged in the following way: [d, i, R, ...] so that the decorator
     // can fetch the node value from the root. In the `mtree.get` operation we have the stack in
     // the following format: [d, i, R], whereas in the case of `mtree.set` we would also have the
     // new node value post the tree root: [d, i, R, V_new]
     //
     // pops the value of the node we are looking for from the advice stack
-    block_builder.push_advice_injector(AdviceInjector::MerkleNodeToStack)?;
+    block_builder.push_system_event(SystemEvent::MerkleNodeToStack);
 
     // pops the old node value from advice the stack => MPVERIFY: [V_old, d, i, R, ...]
     // MRUPDATE: [V_old, d, i, R, V_new, ...]
     block_builder.push_op_many(AdvPop, 4);
-
-    Ok(())
 }
 
 /// Update a node in the merkle tree. This operation will always copy the tree into a new instance,
@@ -225,7 +219,7 @@ fn update_mtree(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyErr
 
     // Inject the old node value onto the stack for the call to MRUPDATE.
     // stack: [V_old, d, i, R_old, V_new, ...] (4 cycles)
-    read_mtree_node(block_builder)?;
+    read_mtree_node(block_builder);
 
     #[rustfmt::skip]
     let ops = [

assembly/src/assembler/instruction/ext2_ops.rs

@@ -1,4 +1,4 @@
-use vm_core::{AdviceInjector::Ext2Inv, Operation::*};
+use vm_core::{sys_events::SystemEvent::Ext2Inv, Operation::*};
 
 use super::BasicBlockBuilder;
 use crate::AssemblyError;
@@ -53,8 +53,8 @@ pub fn ext2_mul(block_builder: &mut BasicBlockBuilder) {
 /// operations outputs the result c = (c1, c0) where c = a * b^-1.
 ///
 /// This operation takes 11 VM cycles.
-pub fn ext2_div(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyError> {
-    block_builder.push_advice_injector(Ext2Inv)?;
+pub fn ext2_div(block_builder: &mut BasicBlockBuilder) {
+    block_builder.push_system_event(Ext2Inv);
     #[rustfmt::skip]
     let ops = [
         AdvPop,         // [b0', b1, b0, a1, a0, ...]
@@ -70,8 +70,6 @@ pub fn ext2_div(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyErr
         Drop            // [a1*b1', a0*b0'...]
     ];
     block_builder.push_ops(ops);
-
-    Ok(())
 }
 
 /// Given a stack with initial configuration given by [a1, a0, ...] where a = (a0, a1) represents
@@ -116,7 +114,7 @@ pub fn ext2_neg(block_builder: &mut BasicBlockBuilder) {
 ///
 /// This operation takes 8 VM cycles.
 pub fn ext2_inv(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyError> {
-    block_builder.push_advice_injector(Ext2Inv)?;
+    block_builder.push_system_event(Ext2Inv);
     #[rustfmt::skip]
     let ops = [
         AdvPop,         // [a0', a1, a0, ...]

assembly/src/assembler/instruction/field_ops.rs

@@ -1,4 +1,4 @@
-use vm_core::{AdviceInjector, FieldElement, Operation::*};
+use vm_core::{sys_events::SystemEvent, FieldElement, Operation::*};
 
 use super::{validate_param, BasicBlockBuilder};
 use crate::{
@@ -259,8 +259,8 @@ fn perform_exp_for_small_power(span_builder: &mut BasicBlockBuilder, pow: u64) {
 ///
 /// # Errors
 /// Returns an error if the logarithm argument (top stack element) equals ZERO.
-pub fn ilog2(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyError> {
-    block_builder.push_advice_injector(AdviceInjector::ILog2)?;
+pub fn ilog2(block_builder: &mut BasicBlockBuilder) {
+    block_builder.push_system_event(SystemEvent::ILog2);
     block_builder.push_op(AdvPop); // [ilog2, n, ...]
 
     // compute the power-of-two for the value given in the advice tape (17 cycles)
@@ -290,8 +290,6 @@ pub fn ilog2(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyError>
     ];
 
     block_builder.push_ops(ops);
-
-    Ok(())
 }
 
 // COMPARISON OPERATIONS

assembly/src/assembler/instruction/mod.rs

@@ -89,7 +89,7 @@ impl Assembler {
             Instruction::ExpBitLength(num_pow_bits) => {
                 field_ops::exp(block_builder, *num_pow_bits)?
             },
-            Instruction::ILog2 => field_ops::ilog2(block_builder)?,
+            Instruction::ILog2 => field_ops::ilog2(block_builder),
 
             Instruction::Not => block_builder.push_op(Not),
             Instruction::And => block_builder.push_op(And),
@@ -111,7 +111,7 @@ impl Assembler {
             Instruction::Ext2Add => ext2_ops::ext2_add(block_builder),
             Instruction::Ext2Sub => ext2_ops::ext2_sub(block_builder),
             Instruction::Ext2Mul => ext2_ops::ext2_mul(block_builder),
-            Instruction::Ext2Div => ext2_ops::ext2_div(block_builder)?,
+            Instruction::Ext2Div => ext2_ops::ext2_div(block_builder),
             Instruction::Ext2Neg => ext2_ops::ext2_neg(block_builder),
             Instruction::Ext2Inv => ext2_ops::ext2_inv(block_builder)?,
 
@@ -190,10 +190,10 @@ impl Assembler {
             Instruction::U32Rotr => u32_ops::u32rotr(block_builder, None)?,
             Instruction::U32RotrImm(v) => u32_ops::u32rotr(block_builder, Some(v.expect_value()))?,
             Instruction::U32Popcnt => u32_ops::u32popcnt(block_builder),
-            Instruction::U32Clz => u32_ops::u32clz(block_builder)?,
-            Instruction::U32Ctz => u32_ops::u32ctz(block_builder)?,
-            Instruction::U32Clo => u32_ops::u32clo(block_builder)?,
-            Instruction::U32Cto => u32_ops::u32cto(block_builder)?,
+            Instruction::U32Clz => u32_ops::u32clz(block_builder),
+            Instruction::U32Ctz => u32_ops::u32ctz(block_builder),
+            Instruction::U32Clo => u32_ops::u32clo(block_builder),
+            Instruction::U32Cto => u32_ops::u32cto(block_builder),
             Instruction::U32Lt => u32_ops::u32lt(block_builder),
             Instruction::U32Lte => u32_ops::u32lte(block_builder),
             Instruction::U32Gt => u32_ops::u32gt(block_builder),
@@ -361,15 +361,17 @@ impl Assembler {
                 false,
             )?,
 
-            Instruction::AdvInject(injector) => adv_ops::adv_inject(block_builder, injector)?,
+            Instruction::SysEvent(system_event) => {
+                block_builder.push_system_event(system_event.into())
+            },
 
             // ----- cryptographic instructions ---------------------------------------------------
             Instruction::Hash => crypto_ops::hash(block_builder),
             Instruction::HPerm => block_builder.push_op(HPerm),
             Instruction::HMerge => crypto_ops::hmerge(block_builder),
-            Instruction::MTreeGet => crypto_ops::mtree_get(block_builder)?,
+            Instruction::MTreeGet => crypto_ops::mtree_get(block_builder),
             Instruction::MTreeSet => crypto_ops::mtree_set(block_builder)?,
-            Instruction::MTreeMerge => crypto_ops::mtree_merge(block_builder)?,
+            Instruction::MTreeMerge => crypto_ops::mtree_merge(block_builder),
             Instruction::MTreeVerify => block_builder.push_op(MpVerify(0)),
             Instruction::MTreeVerifyWithError(err_code) => {
                 block_builder.push_op(MpVerify(err_code.expect_value()))

assembly/src/assembler/instruction/u32_ops.rs

@@ -1,5 +1,6 @@
 use vm_core::{
-    AdviceInjector, Felt,
+    sys_events::SystemEvent,
+    Felt,
     Operation::{self, *},
 };
 
@@ -298,51 +299,47 @@ pub fn u32popcnt(span_builder: &mut BasicBlockBuilder) {
 /// provider).
 ///
 /// This operation takes 42 VM cycles.
-pub fn u32clz(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyError> {
-    block_builder.push_advice_injector(AdviceInjector::U32Clz)?;
+pub fn u32clz(block_builder: &mut BasicBlockBuilder) {
+    block_builder.push_system_event(SystemEvent::U32Clz);
     block_builder.push_op(AdvPop); // [clz, n, ...]
 
     verify_clz(block_builder);
-    Ok(())
 }
 
 /// Translates `u32ctz` assembly instruction to VM operations. `u32ctz` counts the number of
 /// trailing zeros of the value using non-deterministic technique (i.e. it takes help of advice
 /// provider).
 ///
 /// This operation takes 34 VM cycles.
-pub fn u32ctz(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyError> {
-    block_builder.push_advice_injector(AdviceInjector::U32Ctz)?;
+pub fn u32ctz(block_builder: &mut BasicBlockBuilder) {
+    block_builder.push_system_event(SystemEvent::U32Ctz);
     block_builder.push_op(AdvPop); // [ctz, n, ...]
 
     verify_ctz(block_builder);
-    Ok(())
 }
 
 /// Translates `u32clo` assembly instruction to VM operations. `u32clo` counts the number of
 /// leading ones of the value using non-deterministic technique (i.e. it takes help of advice
 /// provider).
 ///
 /// This operation takes 41 VM cycles.
-pub fn u32clo(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyError> {
-    block_builder.push_advice_injector(AdviceInjector::U32Clo)?;
+pub fn u32clo(block_builder: &mut BasicBlockBuilder) {
+    block_builder.push_system_event(SystemEvent::U32Clo);
     block_builder.push_op(AdvPop); // [clo, n, ...]
 
     verify_clo(block_builder);
-    Ok(())
 }
 
 /// Translates `u32cto` assembly instruction to VM operations. `u32cto` counts the number of
 /// trailing ones of the value using non-deterministic technique (i.e. it takes help of advice
 /// provider).
 ///
 /// This operation takes 33 VM cycles.
-pub fn u32cto(block_builder: &mut BasicBlockBuilder) -> Result<(), AssemblyError> {
-    block_builder.push_advice_injector(AdviceInjector::U32Cto)?;
+pub fn u32cto(block_builder: &mut BasicBlockBuilder) {
+    block_builder.push_system_event(SystemEvent::U32Cto);
     block_builder.push_op(AdvPop); // [cto, n, ...]
 
     verify_cto(block_builder);
-    Ok(())
 }
 
 /// Specifically handles these specific inputs per the spec.
@@ -419,7 +416,7 @@ fn prepare_bitwise<const MAX_VALUE: u8>(
 }
 
 /// Appends relevant operations to the span block for the correctness check of the `U32Clz`
-/// injector.
+/// system event.
 /// The idea is to compare the actual value with a bitmask consisting of `clz` leading ones to
 /// check that every bit in `clz` leading bits is zero and `1` additional one to check that
 /// `clz + 1`'th leading bit is one:
@@ -500,7 +497,7 @@ fn verify_clz(block_builder: &mut BasicBlockBuilder) {
 }
 
 /// Appends relevant operations to the span block for the correctness check of the `U32Clo`
-/// injector.
+/// system event.
 /// The idea is to compare the actual value with a bitmask consisting of `clo` leading ones to
 /// check that every bit in `clo` leading bits is one and `1` additional one to check that
 /// `clo + 1`'th leading bit is zero:
@@ -575,7 +572,7 @@ fn verify_clo(block_builder: &mut BasicBlockBuilder) {
 }
 
 /// Appends relevant operations to the span block for the correctness check of the `U32Ctz`
-/// injector.
+/// system event.
 /// The idea is to compare the actual value with a bitmask consisting of `ctz` trailing ones to
 /// check that every bit in `ctz` trailing bits is zero and `1` additional one to check that
 /// `ctz + 1`'th trailing bit is one:
@@ -649,7 +646,7 @@ fn verify_ctz(block_builder: &mut BasicBlockBuilder) {
 }
 
 /// Appends relevant operations to the span block for the correctness check of the `U32Cto`
-/// injector.
+/// system event.
 /// The idea is to compare the actual value with a bitmask consisting of `cto` trailing ones to
 /// check that every bit in `cto` trailing bits is one and `1` additional one to check that
 /// `cto + 1`'th trailing bit is zero: