test: Emulated overflow checks

noir/.gitrepo

@@ -5,8 +5,8 @@
 ;
 [subrepo]
 	remote = https://github.com/noir-lang/noir
-	branch = master
-	commit = e80c5f73a4cdcba3f5cf44576c605ba1e611a2ab
-	parent = 8307dadd853d5091841e169c841ab6b09c223efb
+	branch = arv/emulate_checked_arith_brillig
+	commit = d77d1e7cc51b5ab535f5e7d0200e44187e88bf02
+	parent = efd70f4b8bb284815c5345bd16d79018ed2dd812
 	method = merge
 	cmdver = 0.4.6

noir/aztec_macros/src/lib.rs

@@ -63,12 +63,12 @@ impl MacroProcessor for AztecMacro {
 }
 
 const FUNCTION_TREE_HEIGHT: u32 = 5;
-const MAX_CONTRACT_PRIVATE_FUNCTIONS: usize = 2_usize.pow(FUNCTION_TREE_HEIGHT);
+const MAX_CONTRACT_FUNCTIONS: usize = 2_usize.pow(FUNCTION_TREE_HEIGHT);
 
 #[derive(Debug, Clone)]
 pub enum AztecMacroError {
     AztecDepNotFound,
-    ContractHasTooManyPrivateFunctions { span: Span },
+    ContractHasTooManyFunctions { span: Span },
     ContractConstructorMissing { span: Span },
     UnsupportedFunctionArgumentType { span: Span, typ: UnresolvedTypeData },
     UnsupportedStorageType { span: Option<Span>, typ: UnresolvedTypeData },
@@ -84,8 +84,8 @@ impl From<AztecMacroError> for MacroError {
                 secondary_message: None,
                 span: None,
             },
-            AztecMacroError::ContractHasTooManyPrivateFunctions { span } => MacroError {
-                primary_message: format!("Contract can only have a maximum of {} private functions", MAX_CONTRACT_PRIVATE_FUNCTIONS),
+            AztecMacroError::ContractHasTooManyFunctions { span } => MacroError {
+                primary_message: format!("Contract can only have a maximum of {} functions", MAX_CONTRACT_FUNCTIONS),
                 secondary_message: None,
                 span: Some(span),
             },
@@ -456,22 +456,10 @@ fn transform_module(
     if has_transformed_module {
         // We only want to run these checks if the macro processor has found the module to be an Aztec contract.
 
-        let private_functions_count = module
-            .functions
-            .iter()
-            .filter(|func| {
-                func.def
-                    .attributes
-                    .secondary
-                    .iter()
-                    .any(|attr| is_custom_attribute(attr, "aztec(private)"))
-            })
-            .count();
-
-        if private_functions_count > MAX_CONTRACT_PRIVATE_FUNCTIONS {
+        if module.functions.len() > MAX_CONTRACT_FUNCTIONS {
             let crate_graph = &context.crate_graph[crate_id];
             return Err((
-                AztecMacroError::ContractHasTooManyPrivateFunctions { span: Span::default() },
+                AztecMacroError::ContractHasTooManyFunctions { span: Span::default() },
                 crate_graph.root_file_id,
             ));
         }

noir/bootstrap.sh

@@ -19,4 +19,4 @@ fi
 [ -n "${USE_CACHE:-}" ] && ./bootstrap_cache.sh && exit
 
 ./scripts/bootstrap_native.sh
-./scripts/bootstrap_packages.sh
+./scripts/bootstrap_packages.sh

noir/bootstrap_cache.sh

@@ -1,6 +1,8 @@
 #!/usr/bin/env bash
 set -eu
 
+[ -z "${NO_CACHE:-}" ] && type docker &> /dev/null && [ -f ~/.aws/credentials ] || exit 1
+
 cd "$(dirname "$0")"
 source ../build-system/scripts/setup_env '' '' mainframe_$USER > /dev/null
 

noir/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -1,9 +1,7 @@
 use crate::brillig::brillig_ir::brillig_variable::{
     type_to_heap_value_type, BrilligArray, BrilligVariable, BrilligVector, SingleAddrVariable,
 };
-use crate::brillig::brillig_ir::{
-    BrilligBinaryOp, BrilligContext, BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE,
-};
+use crate::brillig::brillig_ir::{BrilligBinaryOp, BrilligContext};
 use crate::ssa::ir::dfg::CallStack;
 use crate::ssa::ir::instruction::ConstrainError;
 use crate::ssa::ir::{
@@ -626,37 +624,40 @@ impl<'block> BrilligBlock<'block> {
             }
             Instruction::RangeCheck { value, max_bit_size, assert_message } => {
                 let value = self.convert_ssa_single_addr_value(*value, dfg);
-                // Cast original value to field
-                let left = SingleAddrVariable {
-                    address: self.brillig_context.allocate_register(),
-                    bit_size: FieldElement::max_num_bits(),
-                };
-                self.convert_cast(left, value);
+                // SSA generates redundant range checks. A range check with a max bit size >= value.bit_size will always pass.
+                if value.bit_size > *max_bit_size {
+                    // Cast original value to field
+                    let left = SingleAddrVariable {
+                        address: self.brillig_context.allocate_register(),
+                        bit_size: FieldElement::max_num_bits(),
+                    };
+                    self.convert_cast(left, value);
 
-                // Create a field constant with the max
-                let max = BigUint::from(2_u128).pow(*max_bit_size) - BigUint::from(1_u128);
-                let right = self.brillig_context.make_constant(
-                    FieldElement::from_be_bytes_reduce(&max.to_bytes_be()).into(),
-                    FieldElement::max_num_bits(),
-                );
+                    // Create a field constant with the max
+                    let max = BigUint::from(2_u128).pow(*max_bit_size) - BigUint::from(1_u128);
+                    let right = self.brillig_context.make_constant(
+                        FieldElement::from_be_bytes_reduce(&max.to_bytes_be()).into(),
+                        FieldElement::max_num_bits(),
+                    );
 
-                // Check if lte max
-                let brillig_binary_op = BrilligBinaryOp::Integer {
-                    op: BinaryIntOp::LessThanEquals,
-                    bit_size: FieldElement::max_num_bits(),
-                };
-                let condition = self.brillig_context.allocate_register();
-                self.brillig_context.binary_instruction(
-                    left.address,
-                    right,
-                    condition,
-                    brillig_binary_op,
-                );
+                    // Check if lte max
+                    let brillig_binary_op = BrilligBinaryOp::Integer {
+                        op: BinaryIntOp::LessThanEquals,
+                        bit_size: FieldElement::max_num_bits(),
+                    };
+                    let condition = self.brillig_context.allocate_register();
+                    self.brillig_context.binary_instruction(
+                        left.address,
+                        right,
+                        condition,
+                        brillig_binary_op,
+                    );
 
-                self.brillig_context.constrain_instruction(condition, assert_message.clone());
-                self.brillig_context.deallocate_register(condition);
-                self.brillig_context.deallocate_register(left.address);
-                self.brillig_context.deallocate_register(right);
+                    self.brillig_context.constrain_instruction(condition, assert_message.clone());
+                    self.brillig_context.deallocate_register(condition);
+                    self.brillig_context.deallocate_register(left.address);
+                    self.brillig_context.deallocate_register(right);
+                }
             }
             Instruction::IncrementRc { value } => {
                 let rc_register = match self.convert_ssa_value(*value, dfg) {
@@ -1197,15 +1198,116 @@ impl<'block> BrilligBlock<'block> {
         let left = self.convert_ssa_single_addr_value(binary.lhs, dfg);
         let right = self.convert_ssa_single_addr_value(binary.rhs, dfg);
 
-        let brillig_binary_op =
+        let (brillig_binary_op, is_signed) =
             convert_ssa_binary_op_to_brillig_binary_op(binary.operator, &binary_type);
 
+        self.add_underflow_check(brillig_binary_op, left, right, is_signed);
         self.brillig_context.binary_instruction(
             left.address,
             right.address,
             result_variable.address,
             brillig_binary_op,
         );
+
+        self.add_overflow_check(brillig_binary_op, left, right, result_variable, is_signed);
+    }
+
+    fn add_underflow_check(
+        &mut self,
+        binary_operation: BrilligBinaryOp,
+        left: SingleAddrVariable,
+        right: SingleAddrVariable,
+        is_signed: bool,
+    ) {
+        let (op, bit_size) = if let BrilligBinaryOp::Integer { op, bit_size } = binary_operation {
+            (op, bit_size)
+        } else {
+            return;
+        };
+
+        if let (BinaryIntOp::Sub, false) = (op, is_signed) {
+            let condition = self.brillig_context.allocate_register();
+            // Check that rhs <= lhs
+            self.brillig_context.binary_instruction(
+                right.address,
+                left.address,
+                condition,
+                BrilligBinaryOp::Integer { op: BinaryIntOp::LessThanEquals, bit_size },
+            );
+            self.brillig_context
+                .constrain_instruction(condition, Some("Underflow sub check failed".to_string()));
+            self.brillig_context.deallocate_register(condition);
+        }
+    }
+
+    fn add_overflow_check(
+        &mut self,
+        binary_operation: BrilligBinaryOp,
+        left: SingleAddrVariable,
+        right: SingleAddrVariable,
+        result: SingleAddrVariable,
+        is_signed: bool,
+    ) {
+        let (op, bit_size) = if let BrilligBinaryOp::Integer { op, bit_size } = binary_operation {
+            (op, bit_size)
+        } else {
+            return;
+        };
+
+        match (op, is_signed) {
+            (BinaryIntOp::Add, false) => {
+                let condition = self.brillig_context.allocate_register();
+                // Check that lhs <= result
+                self.brillig_context.binary_instruction(
+                    left.address,
+                    result.address,
+                    condition,
+                    BrilligBinaryOp::Integer { op: BinaryIntOp::LessThanEquals, bit_size },
+                );
+                self.brillig_context.constrain_instruction(
+                    condition,
+                    Some("Overflow add check failed".to_string()),
+                );
+                self.brillig_context.deallocate_register(condition);
+            }
+            (BinaryIntOp::Mul, false) => {
+                // Multiplication overflow is only possible for bit sizes > 1
+                if bit_size > 1 {
+                    let is_right_zero = self.brillig_context.allocate_register();
+                    let zero = self.brillig_context.make_constant(0_usize.into(), bit_size);
+                    self.brillig_context.binary_instruction(
+                        zero,
+                        right.address,
+                        is_right_zero,
+                        BrilligBinaryOp::Integer { op: BinaryIntOp::Equals, bit_size },
+                    );
+                    self.brillig_context.if_not_instruction(is_right_zero, |ctx| {
+                        let condition = ctx.allocate_register();
+                        // Check that result / rhs == lhs
+                        ctx.binary_instruction(
+                            result.address,
+                            right.address,
+                            condition,
+                            BrilligBinaryOp::Integer { op: BinaryIntOp::UnsignedDiv, bit_size },
+                        );
+                        ctx.binary_instruction(
+                            condition,
+                            left.address,
+                            condition,
+                            BrilligBinaryOp::Integer { op: BinaryIntOp::Equals, bit_size },
+                        );
+                        ctx.constrain_instruction(
+                            condition,
+                            Some("Overflow mul check failed".to_string()),
+                        );
+                        ctx.deallocate_register(condition);
+                    });
+                    self.brillig_context.deallocate_register(is_right_zero);
+                    self.brillig_context.deallocate_register(zero);
+                }
+            }
+            _ => {}
+        }
     }
 
     /// Converts an SSA `ValueId` into a `RegisterOrMemory`. Initializes if necessary.
@@ -1403,8 +1505,6 @@ impl<'block> BrilligBlock<'block> {
 }
 
 /// Returns the type of the operation considering the types of the operands
-/// TODO: SSA issues binary operations between fields and integers.
-/// This probably should be explicitly casted in SSA to avoid having to coerce at this level.
 pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type) -> Type {
     match (lhs_type, rhs_type) {
         (_, Type::Function) | (Type::Function, _) => {
@@ -1419,10 +1519,6 @@ pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type) -> Type
         (_, Type::Slice(..)) | (Type::Slice(..), _) => {
             unreachable!("Arrays are invalid in binary operations")
         }
-        // If either side is a Field constant then, we coerce into the type
-        // of the other operand
-        (Type::Numeric(NumericType::NativeField), typ)
-        | (typ, Type::Numeric(NumericType::NativeField)) => typ.clone(),
         // If both sides are numeric type, then we expect their types to be
         // the same.
         (Type::Numeric(lhs_type), Type::Numeric(rhs_type)) => {
@@ -1441,7 +1537,7 @@ pub(crate) fn type_of_binary_operation(lhs_type: &Type, rhs_type: &Type) -> Type
 pub(crate) fn convert_ssa_binary_op_to_brillig_binary_op(
     ssa_op: BinaryOp,
     typ: &Type,
-) -> BrilligBinaryOp {
+) -> (BrilligBinaryOp, bool) {
     // First get the bit size and whether its a signed integer, if it is a numeric type
     // if it is not,then we return None, indicating that
     // it is a Field.
@@ -1461,10 +1557,6 @@ pub(crate) fn convert_ssa_binary_op_to_brillig_binary_op(
             BinaryOp::Mul => BrilligBinaryOp::Field { op: BinaryFieldOp::Mul },
             BinaryOp::Div => BrilligBinaryOp::Field { op: BinaryFieldOp::Div },
             BinaryOp::Eq => BrilligBinaryOp::Field { op: BinaryFieldOp::Equals },
-            BinaryOp::Lt => BrilligBinaryOp::Integer {
-                op: BinaryIntOp::LessThan,
-                bit_size: BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE,
-            },
             _ => unreachable!(
                 "Field type cannot be used with {op}. This should have been caught by the frontend"
             ),
@@ -1500,7 +1592,9 @@ pub(crate) fn convert_ssa_binary_op_to_brillig_binary_op(
 
     // If bit size is available then it is a binary integer operation
     match bit_size_signedness {
-        Some((bit_size, is_signed)) => binary_op_to_int_op(ssa_op, *bit_size, is_signed),
-        None => binary_op_to_field_op(ssa_op),
+        Some((bit_size, is_signed)) => {
+            (binary_op_to_int_op(ssa_op, *bit_size, is_signed), is_signed)
+        }
+        None => (binary_op_to_field_op(ssa_op), false),
     }
 }

noir/compiler/noirc_evaluator/src/brillig/brillig_ir.rs

@@ -29,17 +29,6 @@ use acvm::{
 use debug_show::DebugShow;
 use num_bigint::BigUint;
 
-/// Integer arithmetic in Brillig is limited to 127 bit
-/// integers.
-///
-/// We could lift this in the future and have Brillig
-/// do big integer arithmetic when it exceeds the field size
-/// or we could have users re-implement big integer arithmetic
-/// in Brillig.
-/// Since constrained functions do not have this property, it
-/// would mean that unconstrained functions will differ from
-/// constrained functions in terms of syntax compatibility.
-pub(crate) const BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE: u32 = 127;
 /// The Brillig VM does not apply a limit to the memory address space,
 /// As a convention, we take use 64 bits. This means that we assume that
 /// memory has 2^64 memory slots.
@@ -356,6 +345,21 @@ impl BrilligContext {
         self.enter_section(end_section);
     }
 
+    /// This instruction issues a branch that jumps over the code generated by the given function if the condition is truthy
+    pub(crate) fn if_not_instruction(
+        &mut self,
+        condition: MemoryAddress,
+        mut f: impl FnMut(&mut BrilligContext),
+    ) {
+        let (end_section, end_label) = self.reserve_next_section_label();
+
+        self.jump_if_instruction(condition, end_label.clone());
+
+        f(self);
+
+        self.enter_section(end_section);
+    }
+
     /// Adds a label to the next opcode
     pub(crate) fn enter_context<T: ToString>(&mut self, label: T) {
         self.debug_show.enter_context(label.to_string());
@@ -1082,7 +1086,7 @@ impl BrilligContext {
 }
 
 /// Type to encapsulate the binary operation types in Brillig
-#[derive(Clone)]
+#[derive(Clone, Copy)]
 pub(crate) enum BrilligBinaryOp {
     Field { op: BinaryFieldOp },
     Integer { op: BinaryIntOp, bit_size: u32 },

noir/noir_stdlib/src/collections/map.nr

@@ -400,7 +400,7 @@ impl<K, V, N, B, H> HashMap<K, V, N, B> {
     // n * MAX_LOAD_FACTOR_DEN0MINATOR >= m * MAX_LOAD_FACTOR_NUMERATOR
     fn assert_load_factor(self) {
         let lhs = self._len * MAX_LOAD_FACTOR_DEN0MINATOR;
-        let rhs = self._table.len() * MAX_LOAD_FACTOR_NUMERATOR;
+        let rhs = self._table.len() as u64 * MAX_LOAD_FACTOR_NUMERATOR;
         let exceeded = lhs >= rhs;
         assert(!exceeded, "Load factor is exceeded, consider increasing the capacity.");
     }

noir/noir_stdlib/src/sha256.nr

@@ -57,7 +57,7 @@ pub fn digest<N>(msg: [u8; N]) -> [u8; 32] {
             msg_block[i as Field] = 0;
             i = i + 1;
         } else if i < 64 {
-            let mut len = 8 * msg.len();
+            let mut len = 8 * msg.len() as u64;
             for j in 0..8 {
                 msg_block[63 - j] = len as u8;
                 len >>= 8;