Skip to content

Commit

Permalink
Merge pull request #1535 from 0xPolygonMiden/plafer-1091-dyn-op-rework
Browse files Browse the repository at this point in the history
feat: `DYN` and `DYNCALL` takes a memory address instead of digest on stack
  • Loading branch information
plafer authored Nov 1, 2024
2 parents 766292d + 7f469c4 commit 2b96b85
Show file tree
Hide file tree
Showing 38 changed files with 755 additions and 297 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
- Migrated to `miden-crypto` v0.11.0 (#1343).
- Implemented `MastForest` merging (#1534)
- Rename `EqHash` to `MastNodeFingerprint` and make it `pub` (#1539)
- [BREAKING] `DYN` operation now expects a memory address pointing to the procedure hash (#1535)
- [BREAKING] `DYNCALL` operation fixed, and now expects a memory address pointing to the procedure hash (#1535)


#### Fixes

Expand Down
9 changes: 5 additions & 4 deletions air/src/constraints/stack/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,10 @@ trait EvaluationFrameExt<E: FieldElement> {
/// Gets the current value of user op helper register located at the specified index.
fn user_op_helper(&self, index: usize) -> E;

/// Returns the value if the `h6` helper register in the decoder which is set to ONE if the
/// ending block is a `CALL` block.
fn is_call_end(&self) -> E;
/// Returns ONE if the block being `END`ed is a `CALL` or `DYNCALL`, or ZERO otherwise.
///
/// This must only be used when an `END` operation is being executed.
fn is_call_or_dyncall_end(&self) -> E;

/// Returns the value if the `h7` helper register in the decoder which is set to ONE if the
/// ending block is a `SYSCALL` block.
Expand Down Expand Up @@ -359,7 +360,7 @@ impl<E: FieldElement> EvaluationFrameExt<E> for &EvaluationFrame<E> {
}

#[inline]
fn is_call_end(&self) -> E {
fn is_call_or_dyncall_end(&self) -> E {
self.current()[DECODER_TRACE_OFFSET + IS_CALL_FLAG_COL_IDX]
}

Expand Down
27 changes: 23 additions & 4 deletions air/src/constraints/stack/op_flags/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,9 @@ impl<E: FieldElement> OpFlags<E> {
+ degree7_op_flags[47]
+ degree7_op_flags[46]
+ split_loop_flag
+ shift_left_on_end;
+ shift_left_on_end
+ degree5_op_flags[8] // DYN
+ degree5_op_flags[12]; // DYNCALL

left_shift_flags[2] = left_shift_flags[1] + left_change_1_flag;
left_shift_flags[3] =
Expand Down Expand Up @@ -398,9 +400,15 @@ impl<E: FieldElement> OpFlags<E> {
// Flag if the stack has been shifted to the right.
let right_shift = f011 + degree5_op_flags[11] + degree6_op_flags[4]; // PUSH; U32SPLIT

// Flag if the stack has been shifted to the left.
let left_shift =
f010 + add3_madd_flag + split_loop_flag + degree4_op_flags[5] + shift_left_on_end;
// Flag if the stack has been shifted to the left. Note that `DYNCALL` is not included in
// this flag even if it shifts the stack to the left. See `Opflags::left_shift()` for more
// information.
let left_shift = f010
+ add3_madd_flag
+ split_loop_flag
+ degree4_op_flags[5]
+ shift_left_on_end
+ degree5_op_flags[8]; // DYN

// Flag if the current operation being executed is a control flow operation.
// first row: SPAN, JOIN, SPLIT, LOOP
Expand Down Expand Up @@ -923,6 +931,12 @@ impl<E: FieldElement> OpFlags<E> {
self.degree4_op_flags[get_op_index(Operation::SysCall.op_code())]
}

/// Operation Flag of DYNCALL operation.
#[inline(always)]
pub fn dyncall(&self) -> E {
self.degree5_op_flags[get_op_index(Operation::Dyncall.op_code())]
}

/// Operation Flag of END operation.
#[inline(always)]
pub fn end(&self) -> E {
Expand Down Expand Up @@ -982,6 +996,11 @@ impl<E: FieldElement> OpFlags<E> {
}

/// Returns the flag when the stack operation shifts the flag to the left.
///
/// Note that although `DYNCALL` shifts the entire stack, it is not included in this flag. This
/// is because this "aggregate left shift" flag is used in constraints related to the stack
/// helper columns, and `DYNCALL` uses them unconventionally.
///
/// Degree: 5
#[inline(always)]
pub fn left_shift(&self) -> E {
Expand Down
13 changes: 8 additions & 5 deletions air/src/constraints/stack/overflow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ pub fn enforce_constraints<E: FieldElement>(
/// - If the operation is a left shift op, then, depth should be decreased by 1 provided the
/// existing depth of the stack is not 16. In the case of depth being 16, depth will not be
/// updated.
/// - If the current op being executed is `CALL` or `SYSCALL`, then the depth should be reset to 16.
/// - If the current op being executed is `CALL`, `SYSCALL` or `DYNCALL`, then the depth should be
/// reset to 16.
///
/// TODO: This skips the operation when `END` is exiting for a `CALL` or a `SYSCALL` block. It
/// should be handled later in multiset constraints.
Expand All @@ -77,13 +78,15 @@ pub fn enforce_stack_depth_constraints<E: FieldElement>(
let depth = frame.stack_depth();
let depth_next = frame.stack_depth_next();

let call_or_syscall = op_flag.call() + op_flag.syscall();
let call_or_syscall_end = op_flag.end() * (frame.is_call_end() + frame.is_syscall_end());
let call_or_dyncall_or_syscall = op_flag.call() + op_flag.dyncall() + op_flag.syscall();
let call_or_dyncall_or_syscall_end =
op_flag.end() * (frame.is_call_or_dyncall_end() + frame.is_syscall_end());

let no_shift_part = (depth_next - depth) * (E::ONE - call_or_syscall - call_or_syscall_end);
let no_shift_part = (depth_next - depth)
* (E::ONE - call_or_dyncall_or_syscall - call_or_dyncall_or_syscall_end);
let left_shift_part = op_flag.left_shift() * op_flag.overflow();
let right_shift_part = op_flag.right_shift();
let call_part = call_or_syscall * (depth_next - E::from(16u32));
let call_part = call_or_dyncall_or_syscall * (depth_next - E::from(16u32));

// Enforces constraints of the transition of depth of the stack.
result[0] = no_shift_part + left_shift_part - right_shift_part + call_part;
Expand Down
2 changes: 1 addition & 1 deletion air/src/trace/decoder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub const IS_LOOP_BODY_FLAG_COL_IDX: usize = HASHER_STATE_RANGE.start + 4;
/// Index of a flag column which indicates whether an ending block is a LOOP block.
pub const IS_LOOP_FLAG_COL_IDX: usize = HASHER_STATE_RANGE.start + 5;

/// Index of a flag column which indicates whether an ending block is a CALL block.
/// Index of a flag column which indicates whether an ending block is a CALL or DYNCALL block.
pub const IS_CALL_FLAG_COL_IDX: usize = HASHER_STATE_RANGE.start + 6;

/// Index of a flag column which indicates whether an ending block is a SYSCALL block.
Expand Down
4 changes: 3 additions & 1 deletion air/src/trace/main_trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl MainTrace {

/// Returns a specific element from the hasher state at row i.
pub fn decoder_hasher_state_element(&self, element: usize, i: RowIndex) -> Felt {
self.columns.get_column(DECODER_TRACE_OFFSET + HASHER_STATE_OFFSET + element)[i + 1]
self.columns.get_column(DECODER_TRACE_OFFSET + HASHER_STATE_OFFSET + element)[i]
}

/// Returns the current function hash (i.e., root) at row i.
Expand Down Expand Up @@ -240,6 +240,8 @@ impl MainTrace {
([e0, b3, b2, b1] == [ONE, ZERO, ONE, ZERO]) ||
// REPEAT
([b6, b5, b4, b3, b2, b1, b0] == [ONE, ONE, ONE, ZERO, ONE, ZERO, ZERO]) ||
// DYN
([b6, b5, b4, b3, b2, b1, b0] == [ONE, ZERO, ONE, ONE, ZERO, ZERO, ZERO]) ||
// END of a loop
([b6, b5, b4, b3, b2, b1, b0] == [ONE, ONE, ONE, ZERO, ZERO, ZERO, ZERO] && h5 == ONE)
}
Expand Down
7 changes: 2 additions & 5 deletions assembly/src/assembler/instruction/procedures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,12 @@ impl Assembler {
Ok(Some(dyn_node_id))
}

/// Creates a new CALL block whose target is DYN.
/// Creates a new DYNCALL block for the dynamic function call and return.
pub(super) fn dyncall(
&self,
mast_forest_builder: &mut MastForestBuilder,
) -> Result<Option<MastNodeId>, AssemblyError> {
let dyn_call_node_id = {
let dyn_node_id = mast_forest_builder.ensure_dyn()?;
mast_forest_builder.ensure_call(dyn_node_id)?
};
let dyn_call_node_id = mast_forest_builder.ensure_dyncall()?;

Ok(Some(dyn_call_node_id))
}
Expand Down
5 changes: 5 additions & 0 deletions assembly/src/assembler/mast_forest_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,11 @@ impl MastForestBuilder {
self.ensure_node(MastNode::new_dyn())
}

/// Adds a dyncall node to the forest, and returns the [`MastNodeId`] associated with it.
pub fn ensure_dyncall(&mut self) -> Result<MastNodeId, AssemblyError> {
self.ensure_node(MastNode::new_dyncall())
}

/// Adds an external node to the forest, and returns the [`MastNodeId`] associated with it.
pub fn ensure_external(&mut self, mast_root: RpoDigest) -> Result<MastNodeId, AssemblyError> {
self.ensure_node(MastNode::new_external(mast_root))
Expand Down
2 changes: 1 addition & 1 deletion assembly/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1901,7 +1901,7 @@ fn program_with_dynamic_code_execution_in_new_context() -> TestResult {
let program = context.assemble(source)?;
let expected = "\
begin
call.0xc75c340ec6a69e708457544d38783abbb604d881b7dc62d00bfc2b10f52808e6
dyncall
end";
assert_str_eq!(format!("{program}"), expected);
Ok(())
Expand Down
5 changes: 5 additions & 0 deletions core/src/mast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ impl MastForest {
self.add_node(MastNode::new_dyn())
}

/// Adds a dyncall node to the forest, and returns the [`MastNodeId`] associated with it.
pub fn add_dyncall(&mut self) -> Result<MastNodeId, MastForestError> {
self.add_node(MastNode::new_dyncall())
}

/// Adds an external node to the forest, and returns the [`MastNodeId`] associated with it.
pub fn add_external(&mut self, mast_root: RpoDigest) -> Result<MastNodeId, MastForestError> {
self.add_node(MastNode::new_external(mast_root))
Expand Down
89 changes: 75 additions & 14 deletions core/src/mast/node/dyn_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,44 +6,90 @@ use miden_formatting::prettier::{const_text, nl, Document, PrettyPrint};

use crate::{
mast::{DecoratorId, MastForest},
OPCODE_DYN,
OPCODE_DYN, OPCODE_DYNCALL,
};

// DYN NODE
// ================================================================================================

/// A Dyn node specifies that the node to be executed next is defined dynamically via the stack.
#[derive(Debug, Clone, Default, PartialEq, Eq)]
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct DynNode {
is_dyncall: bool,
before_enter: Vec<DecoratorId>,
after_exit: Vec<DecoratorId>,
}

/// Constants
impl DynNode {
/// The domain of the Dyn block (used for control block hashing).
pub const DOMAIN: Felt = Felt::new(OPCODE_DYN as u64);
pub const DYN_DOMAIN: Felt = Felt::new(OPCODE_DYN as u64);

/// The domain of the Dyncall block (used for control block hashing).
pub const DYNCALL_DOMAIN: Felt = Felt::new(OPCODE_DYNCALL as u64);
}

/// Public accessors
impl DynNode {
/// Creates a new [`DynNode`] representing a dynexec operation.
pub fn new_dyn() -> Self {
Self {
is_dyncall: false,
before_enter: Vec::new(),
after_exit: Vec::new(),
}
}

/// Creates a new [`DynNode`] representing a dyncall operation.
pub fn new_dyncall() -> Self {
Self {
is_dyncall: true,
before_enter: Vec::new(),
after_exit: Vec::new(),
}
}

/// Returns true if the [`DynNode`] represents a dyncall operation, and false for dynexec.
pub fn is_dyncall(&self) -> bool {
self.is_dyncall
}

/// Returns the domain of this dyn node.
pub fn domain(&self) -> Felt {
if self.is_dyncall() {
Self::DYNCALL_DOMAIN
} else {
Self::DYN_DOMAIN
}
}

/// Returns a commitment to a Dyn node.
///
/// The commitment is computed by hashing two empty words ([ZERO; 4]) in the domain defined
/// by [Self::DOMAIN], i.e.:
/// by [Self::DYN_DOMAIN] or [Self::DYNCALL_DOMAIN], i.e.:
///
/// ```
/// # use miden_core::mast::DynNode;
/// # use miden_crypto::{hash::rpo::{RpoDigest as Digest, Rpo256 as Hasher}};
/// Hasher::merge_in_domain(&[Digest::default(), Digest::default()], DynNode::DOMAIN);
/// Hasher::merge_in_domain(&[Digest::default(), Digest::default()], DynNode::DYN_DOMAIN);
/// Hasher::merge_in_domain(&[Digest::default(), Digest::default()], DynNode::DYNCALL_DOMAIN);
/// ```
pub fn digest(&self) -> RpoDigest {
RpoDigest::new([
Felt::new(8115106948140260551),
Felt::new(13491227816952616836),
Felt::new(15015806788322198710),
Felt::new(16575543461540527115),
])
if self.is_dyncall {
RpoDigest::new([
Felt::new(8751004906421739448),
Felt::new(13469709002495534233),
Felt::new(12584249374630430826),
Felt::new(7624899870831503004),
])
} else {
RpoDigest::new([
Felt::new(8115106948140260551),
Felt::new(13491227816952616836),
Felt::new(15015806788322198710),
Felt::new(16575543461540527115),
])
}
}

/// Returns the decorators to be executed before this node is executed.
Expand Down Expand Up @@ -132,7 +178,11 @@ impl DynNodePrettyPrint<'_> {

impl crate::prettier::PrettyPrint for DynNodePrettyPrint<'_> {
fn render(&self) -> crate::prettier::Document {
let dyn_text = const_text("dyn");
let dyn_text = if self.node.is_dyncall() {
const_text("dyncall")
} else {
const_text("dyn")
};

let single_line = self.single_line_pre_decorators()
+ dyn_text.clone()
Expand Down Expand Up @@ -164,8 +214,19 @@ mod tests {
#[test]
pub fn test_dyn_node_digest() {
assert_eq!(
DynNode::default().digest(),
Rpo256::merge_in_domain(&[RpoDigest::default(), RpoDigest::default()], DynNode::DOMAIN)
DynNode::new_dyn().digest(),
Rpo256::merge_in_domain(
&[RpoDigest::default(), RpoDigest::default()],
DynNode::DYN_DOMAIN
)
);

assert_eq!(
DynNode::new_dyncall().digest(),
Rpo256::merge_in_domain(
&[RpoDigest::default(), RpoDigest::default()],
DynNode::DYNCALL_DOMAIN
)
);
}
}
7 changes: 5 additions & 2 deletions core/src/mast/node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ impl MastNode {
}

pub fn new_dyn() -> Self {
Self::Dyn(DynNode::default())
Self::Dyn(DynNode::new_dyn())
}
pub fn new_dyncall() -> Self {
Self::Dyn(DynNode::new_dyncall())
}

pub fn new_external(mast_root: RpoDigest) -> Self {
Expand Down Expand Up @@ -173,7 +176,7 @@ impl MastNode {
MastNode::Split(_) => SplitNode::DOMAIN,
MastNode::Loop(_) => LoopNode::DOMAIN,
MastNode::Call(call_node) => call_node.domain(),
MastNode::Dyn(_) => DynNode::DOMAIN,
MastNode::Dyn(dyn_node) => dyn_node.domain(),
MastNode::External(_) => panic!("Can't fetch domain for an `External` node."),
}
}
Expand Down
Loading

0 comments on commit 2b96b85

Please sign in to comment.