Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the type of the error code of u32assert2 operation from Felt to u32 #1382

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion air/src/constraints/stack/op_flags/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,7 @@ impl<E: FieldElement> OpFlags<E> {
/// Operation Flag of U32ASSERT2 operation.
#[inline(always)]
pub fn u32assert2(&self) -> E {
self.degree6_op_flags[get_op_index(Operation::U32assert2(ZERO).op_code())]
self.degree6_op_flags[get_op_index(Operation::U32assert2(0).op_code())]
}

/// Operation Flag of U32ADD3 operation.
Expand Down
12 changes: 6 additions & 6 deletions assembly/src/assembler/instruction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,17 +123,17 @@ impl Assembler {
// ----- u32 manipulation -------------------------------------------------------------
Instruction::U32Test => span_builder.push_ops([Dup0, U32split, Swap, Drop, Eqz]),
Instruction::U32TestW => u32_ops::u32testw(span_builder),
Instruction::U32Assert => span_builder.push_ops([Pad, U32assert2(ZERO), Drop]),
Instruction::U32Assert => span_builder.push_ops([Pad, U32assert2(0), Drop]),
Instruction::U32AssertWithError(err_code) => {
span_builder.push_ops([Pad, U32assert2(Felt::from(err_code.expect_value())), Drop])
span_builder.push_ops([Pad, U32assert2(err_code.expect_value()), Drop])
}
Instruction::U32Assert2 => span_builder.push_op(U32assert2(ZERO)),
Instruction::U32Assert2 => span_builder.push_op(U32assert2(0)),
Instruction::U32Assert2WithError(err_code) => {
span_builder.push_op(U32assert2(Felt::from(err_code.expect_value())))
span_builder.push_op(U32assert2(err_code.expect_value()))
}
Instruction::U32AssertW => u32_ops::u32assertw(span_builder, ZERO),
Instruction::U32AssertW => u32_ops::u32assertw(span_builder, 0),
Instruction::U32AssertWWithError(err_code) => {
u32_ops::u32assertw(span_builder, Felt::from(err_code.expect_value()))
u32_ops::u32assertw(span_builder, err_code.expect_value())
}

Instruction::U32Cast => span_builder.push_ops([U32split, Drop]),
Expand Down
5 changes: 2 additions & 3 deletions assembly/src/assembler/instruction/u32_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::{
use vm_core::{
AdviceInjector, Felt,
Operation::{self, *},
ZERO,
};

/// This enum is intended to determine the mode of operation passed to the parsing function
Expand Down Expand Up @@ -45,7 +44,7 @@ pub fn u32testw(span_builder: &mut BasicBlockBuilder) {
///
/// Implemented by executing `U32ASSERT2` on each pair of elements in the word.
/// Total of 6 VM cycles.
pub fn u32assertw(span_builder: &mut BasicBlockBuilder, err_code: Felt) {
pub fn u32assertw(span_builder: &mut BasicBlockBuilder, err_code: u32) {
#[rustfmt::skip]
let ops = [
// Test the first and the second elements
Expand Down Expand Up @@ -171,7 +170,7 @@ pub fn u32not(span_builder: &mut BasicBlockBuilder) {
let ops = [
// Perform the operation
Push(Felt::from(u32::MAX)),
U32assert2(ZERO),
U32assert2(0),
Swap,
U32sub,

Expand Down
8 changes: 4 additions & 4 deletions core/src/mast/serialization/basic_block_data_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ impl BasicBlockDataBuilder {

// For operations that have extra data, encode it in `data`.
match operation {
Operation::Assert(err_code) | Operation::MpVerify(err_code) => {
Operation::Assert(err_code)
| Operation::MpVerify(err_code)
| Operation::U32assert2(err_code) => {
self.data.extend_from_slice(&err_code.to_le_bytes())
}
Operation::U32assert2(value) | Operation::Push(value) => {
self.data.extend_from_slice(&value.as_int().to_le_bytes())
}
Operation::Push(value) => self.data.extend_from_slice(&value.as_int().to_le_bytes()),
// Note: we explicitly write out all the operations so that whenever we make a
// modification to the `Operation` enum, we get a compile error here. This
// should help us remember to properly encode/decode each operation variant.
Expand Down
5 changes: 2 additions & 3 deletions core/src/mast/serialization/basic_block_data_decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,13 @@ impl<'a> BasicBlockDataDecoder<'a> {

let operation = if op_code == Operation::Assert(0).op_code()
|| op_code == Operation::MpVerify(0).op_code()
|| op_code == Operation::U32assert2(0).op_code()
{
let value_le_bytes: [u8; 4] = self.data_reader.read_array()?;
let value = u32::from_le_bytes(value_le_bytes);

Operation::with_opcode_and_data(op_code, OperationData::U32(value))?
} else if op_code == Operation::U32assert2(ZERO).op_code()
|| op_code == Operation::Push(ZERO).op_code()
{
} else if op_code == Operation::Push(ZERO).op_code() {
// Felt operation data
let value_le_bytes: [u8; 8] = self.data_reader.read_array()?;
let value_u64 = u64::from_le_bytes(value_le_bytes);
Expand Down
7 changes: 3 additions & 4 deletions core/src/mast/serialization/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use alloc::string::ToString;
use math::FieldElement;
use miden_crypto::{hash::rpo::RpoDigest, Felt};

use super::*;
Expand All @@ -13,7 +12,7 @@ use crate::{
/// [`serialize_deserialize_all_nodes`].
#[test]
fn confirm_operation_and_decorator_structure() {
let _ = match Operation::Noop {
match Operation::Noop {
Operation::Noop => (),
Operation::Assert(_) => (),
Operation::FmpAdd => (),
Expand Down Expand Up @@ -105,7 +104,7 @@ fn confirm_operation_and_decorator_structure() {
Operation::RCombBase => (),
};

let _ = match Decorator::Event(0) {
match Decorator::Event(0) {
Decorator::Advice(advice) => match advice {
AdviceInjector::MerkleNodeMerge => (),
AdviceInjector::MerkleNodeToStack => (),
Expand Down Expand Up @@ -181,7 +180,7 @@ fn serialize_deserialize_all_nodes() {
Operation::Ext2Mul,
Operation::U32split,
Operation::U32add,
Operation::U32assert2(Felt::ONE),
Operation::U32assert2(222),
Operation::U32add3,
Operation::U32sub,
Operation::U32mul,
Expand Down
6 changes: 3 additions & 3 deletions core/src/operations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ pub enum Operation {
///
/// The internal value specifies an error code associated with the error in case when the
/// assertion fails.
U32assert2(Felt),
U32assert2(u32),

/// Pops three elements off the stack, adds them together, and splits the result into upper
/// and lower 32-bit values. Then pushes the result back onto the stack.
Expand Down Expand Up @@ -535,9 +535,9 @@ impl Operation {
0b0100_0110 => Ok(Self::U32div),
0b0100_1000 => Ok(Self::U32split),
0b0100_1010 => match data {
OperationData::Felt(value) => Ok(Self::U32assert2(value)),
OperationData::U32(value) => Ok(Self::U32assert2(value)),
_ => Err(DeserializationError::InvalidValue(
"Invalid opcode data. 'U32assert2' opcode provided, hence expected to receive Felt data.".to_string()
"Invalid opcode data. 'U32assert2' opcode provided, hence expected to receive u32 data.".to_string()
)),
},
0b0100_1100 => Ok(Self::U32add3),
Expand Down
8 changes: 4 additions & 4 deletions processor/src/operations/u32_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ where
/// Pops top two element off the stack, splits them into low and high 32-bit values, checks if
/// the high values are equal to 0; if they are, puts the original elements back onto the
/// stack; if they are not, returns an error.
pub(super) fn op_u32assert2(&mut self, err_code: Felt) -> Result<(), ExecutionError> {
pub(super) fn op_u32assert2(&mut self, err_code: u32) -> Result<(), ExecutionError> {
let a = self.stack.get(0);
let b = self.stack.get(1);

if a.as_int() >> 32 != 0 {
return Err(ExecutionError::NotU32Value(a, err_code));
return Err(ExecutionError::NotU32Value(a, Felt::from(err_code)));
}
if b.as_int() >> 32 != 0 {
return Err(ExecutionError::NotU32Value(b, err_code));
return Err(ExecutionError::NotU32Value(b, Felt::from(err_code)));
}

self.add_range_checks(Operation::U32assert2(err_code), a, b, false);
Expand Down Expand Up @@ -280,7 +280,7 @@ mod tests {
let stack = StackInputs::try_from_ints([d as u64, c as u64, b as u64, a as u64]).unwrap();
let mut process = Process::new_dummy_with_decoder_helpers(stack);

process.execute_op(Operation::U32assert2(ZERO)).unwrap();
process.execute_op(Operation::U32assert2(0)).unwrap();
let expected = build_expected(&[a, b, c, d]);
assert_eq!(expected, process.stack.trace_state());
}
Expand Down
Loading