From 40418f54c95e99bc74339727f8aefd42947ecc95 Mon Sep 17 00:00:00 2001 From: Ayush Shukla Date: Sun, 7 May 2023 12:56:49 +0200 Subject: [PATCH 1/2] fix: to-bits and to-radix for > 128 bits --- crates/noirc_driver/src/contract.rs | 2 +- .../src/ssa/acir_gen/constraints.rs | 7 +++--- .../noirc_evaluator/src/ssa/optimizations.rs | 22 ++++++++++--------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/crates/noirc_driver/src/contract.rs b/crates/noirc_driver/src/contract.rs index 3f69a06d1e..c0a5453494 100644 --- a/crates/noirc_driver/src/contract.rs +++ b/crates/noirc_driver/src/contract.rs @@ -1,7 +1,7 @@ +use crate::program::{deserialize_circuit, serialize_circuit}; use acvm::acir::circuit::Circuit; use noirc_abi::Abi; use serde::{Deserialize, Serialize}; -use crate::program::{serialize_circuit, deserialize_circuit}; /// Describes the types of smart contract functions that are allowed. /// Unlike the similar enum in noirc_frontend, 'open' and 'unconstrained' diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/constraints.rs b/crates/noirc_evaluator/src/ssa/acir_gen/constraints.rs index 11371dc54a..8b61652e23 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/constraints.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/constraints.rs @@ -405,9 +405,10 @@ pub(crate) fn to_radix_base( evaluator: &mut Evaluator, ) -> Vec { // ensure there is no overflow - let mut max = BigUint::from(radix); - max = max.pow(limb_size) - BigUint::one(); - assert!(max < FieldElement::modulus()); + let mut min = BigUint::from(radix); + min = min.pow(limb_size - 1); + // minimum value that can be represented with limb_size limbs should be less than the modulus + assert!(min < FieldElement::modulus()); let (mut result, bytes) = to_radix_little(radix, limb_size, evaluator); diff --git a/crates/noirc_evaluator/src/ssa/optimizations.rs b/crates/noirc_evaluator/src/ssa/optimizations.rs index 8dcc7cfecf..f1cfca9c24 100644 --- a/crates/noirc_evaluator/src/ssa/optimizations.rs +++ b/crates/noirc_evaluator/src/ssa/optimizations.rs @@ -9,7 +9,7 @@ use crate::ssa::{ }, }; use acvm::FieldElement; -use num_bigint::ToBigUint; +use num_bigint::BigUint; pub(super) fn simplify_id(ctx: &mut SsaContext, ins_id: NodeId) -> Result<(), RuntimeError> { let mut ins = ctx.instruction(ins_id).clone(); @@ -74,14 +74,16 @@ pub(super) fn simplify(ctx: &mut SsaContext, ins: &mut Instruction) -> Result<() fn evaluate_intrinsic( ctx: &mut SsaContext, op: builtin::Opcode, - args: Vec, + args: Vec, res_type: &ObjectType, block_id: BlockId, ) -> Result, RuntimeErrorKind> { match op { builtin::Opcode::ToBits(_) => { - let bit_count = args[1] as u32; + let bit_count = args[1].to_u128() as u32; let mut result = Vec::new(); + let mut bits = args[0].bits(); + bits.reverse(); if let ObjectType::ArrayPointer(a) = res_type { for i in 0..bit_count { @@ -89,7 +91,7 @@ fn evaluate_intrinsic( FieldElement::from(i as i128), ObjectType::native_field(), ); - let op = if args[0] & (1 << i) != 0 { + let op = if i < bits.len() as u32 && bits[i as usize] { Operation::Store { array_id: *a, index, @@ -116,9 +118,10 @@ fn evaluate_intrinsic( ); } builtin::Opcode::ToRadix(endian) => { - let mut element = args[0].to_biguint().unwrap().to_radix_le(args[1] as u32); - let byte_count = args[2] as u32; - let diff = if byte_count > element.len() as u32 { + let mut element = BigUint::from_bytes_be(&args[0].to_be_bytes()) + .to_radix_le(args[1].to_u128() as u32); + let byte_count = args[2].to_u128() as u32; + let diff = if byte_count >= element.len() as u32 { byte_count - element.len() as u32 } else { return Err(RuntimeErrorKind::ArrayOutOfBounds { @@ -532,9 +535,8 @@ fn cse_block_with_anchor( // We do not simplify print statements builtin::Opcode::Println(_) => (), _ => { - let args = args.iter().map(|arg| { - NodeEval::from_id(ctx, *arg).into_const_value().map(|f| f.to_u128()) - }); + let args = + args.iter().map(|arg| NodeEval::from_id(ctx, *arg).into_const_value()); if let Some(args) = args.collect() { update2.mark = Mark::Deleted; From 49fc4c6928a7a78fee2ef00b3e894f774e4f4091 Mon Sep 17 00:00:00 2001 From: Ayush Shukla Date: Thu, 11 May 2023 12:04:25 +0200 Subject: [PATCH 2/2] revert: use earlier overflow check that ensures unicity --- crates/noirc_evaluator/src/ssa/acir_gen/constraints.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/constraints.rs b/crates/noirc_evaluator/src/ssa/acir_gen/constraints.rs index 8b61652e23..11371dc54a 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/constraints.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/constraints.rs @@ -405,10 +405,9 @@ pub(crate) fn to_radix_base( evaluator: &mut Evaluator, ) -> Vec { // ensure there is no overflow - let mut min = BigUint::from(radix); - min = min.pow(limb_size - 1); - // minimum value that can be represented with limb_size limbs should be less than the modulus - assert!(min < FieldElement::modulus()); + let mut max = BigUint::from(radix); + max = max.pow(limb_size) - BigUint::one(); + assert!(max < FieldElement::modulus()); let (mut result, bytes) = to_radix_little(radix, limb_size, evaluator);