chore: fix issue #6929 (off-by-one error in `UltraCircuitBuilder::cre…

…ate_range_constraint`) (AztecProtocol/aztec-packages#6931)

Fix small bug that creates innaccurate range constraints for small
scalar values in UltraCircuitBuilder

.aztec-sync-commit

@@ -1 +1 @@
-1d785fd1087d7387fc29213ca3be50b2fc9c4725
+16deef6a83a9fe41e1f865e79e17c2f671604bb0

.github/workflows/test-rust-workspace-msrv.yml

@@ -111,7 +111,7 @@ jobs:
           fi
         env:
           # We treat any cancelled, skipped or failing jobs as a failure for the workflow as a whole.
-          FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }}
+          FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'skipped') }}
 
       - name: Checkout
         if: ${{ failure() }}

acvm-repo/acir_field/Cargo.toml

@@ -16,7 +16,6 @@ repository.workspace = true
 hex.workspace = true
 num-bigint.workspace = true
 serde.workspace = true
-num-traits.workspace = true
 
 ark-bn254 = { version = "^0.4.0", default-features = false, features = ["curve"] }
 ark-bls12-381 = { version = "^0.4.0", optional = true, default-features = false, features = ["curve"] }

acvm-repo/acir_field/src/lib.rs

@@ -2,9 +2,6 @@
 #![warn(unreachable_pub)]
 #![warn(clippy::semicolon_if_nothing_returned)]
 #![cfg_attr(not(test), warn(unused_crate_dependencies, unused_extern_crates))]
-
-use num_bigint::BigUint;
-use num_traits::Num;
 mod generic_ark;
 
 pub use generic_ark::AcirField;
@@ -15,40 +12,11 @@ pub use generic_ark::FieldElement as GenericFieldElement;
 cfg_if::cfg_if! {
     if #[cfg(feature = "bls12_381")] {
         pub type FieldElement = generic_ark::FieldElement<ark_bls12_381::Fr>;
-        pub const CHOSEN_FIELD : FieldOptions = FieldOptions::BLS12_381;
     } else {
         pub type FieldElement = generic_ark::FieldElement<ark_bn254::Fr>;
-        pub const CHOSEN_FIELD : FieldOptions = FieldOptions::BN254;
     }
 }
 
-#[derive(Debug, PartialEq, Eq)]
-pub enum FieldOptions {
-    BN254,
-    BLS12_381,
-}
-
-impl FieldOptions {
-    pub fn to_string(&self) -> &str {
-        match self {
-            FieldOptions::BN254 => "bn254",
-            FieldOptions::BLS12_381 => "bls12_381",
-        }
-    }
-
-    pub fn is_native_field(str: &str) -> bool {
-        let big_num = if let Some(hex) = str.strip_prefix("0x") {
-            BigUint::from_str_radix(hex, 16)
-        } else {
-            BigUint::from_str_radix(str, 10)
-        };
-        if let Ok(big_num) = big_num {
-            big_num == FieldElement::modulus()
-        } else {
-            CHOSEN_FIELD.to_string() == str
-        }
-    }
-}
 // This is needed because features are additive through the dependency graph; if a dependency turns on the bn254, then it
 // will be turned on in all crates that depend on it
 #[macro_export]

acvm-repo/acvm_js/build.sh

@@ -25,7 +25,7 @@ function run_if_available {
 require_command jq
 require_command cargo
 require_command wasm-bindgen
-# require_command wasm-opt
+#require_command wasm-opt
 
 self_path=$(dirname "$(readlink -f "$0")")
 pname=$(cargo read-manifest | jq -r '.name')

acvm-repo/brillig_vm/src/memory.rs

@@ -294,6 +294,13 @@ impl<F: AcirField> Memory<F> {
     }
 
     pub fn read_slice(&self, addr: MemoryAddress, len: usize) -> &[MemoryValue<F>] {
+        // Allows to read a slice of uninitialized memory if the length is zero.
+        // Ideally we'd be able to read uninitialized memory in general (as read does)
+        // but that's not possible if we want to return a slice instead of owned data.
+        if len == 0 {
+            return &[];
+        }
+
         &self.inner[addr.to_usize()..(addr.to_usize() + len)]
     }
 

aztec_macros/src/transforms/functions.rs

@@ -216,12 +216,30 @@ pub fn export_fn_abi(
 ///
 /// Inserts the following code at the beginning of an unconstrained function
 /// ```noir
-/// let storage = Storage::init(Context::none());
+/// let context = UnconstrainedContext::new();
+/// let storage = Storage::init(context);
 /// ```
 ///
 /// This will allow developers to access their contract' storage struct in unconstrained functions
 pub fn transform_unconstrained(func: &mut NoirFunction, storage_struct_name: String) {
+    // let context = UnconstrainedContext::new();
+    let let_context = assignment(
+        "context", // Assigned to
+        call(
+            variable_path(chained_dep!(
+                "aztec",
+                "context",
+                "unconstrained_context",
+                "UnconstrainedContext",
+                "new"
+            )),
+            vec![],
+        ),
+    );
+
+    // We inject the statements at the beginning, in reverse order.
     func.def.body.statements.insert(0, abstract_storage(storage_struct_name, true));
+    func.def.body.statements.insert(0, let_context);
 }
 
 /// Helper function that returns what the private context would look like in the ast
@@ -597,30 +615,26 @@ fn abstract_return_values(func: &NoirFunction) -> Result<Option<Vec<Statement>>,
 /// ```noir
 /// #[aztec(private)]
 /// fn lol() {
-///     let storage = Storage::init(context);
+///     let storage = Storage::init(&mut context);
 /// }
 /// ```
 ///
 /// For public functions:
 /// ```noir
 /// #[aztec(public)]
 /// fn lol() {
-///    let storage = Storage::init(context);
+///    let storage = Storage::init(&mut context);
 /// }
 /// ```
 ///
 /// For unconstrained functions:
 /// ```noir
 /// unconstrained fn lol() {
-///   let storage = Storage::init(());
+///   let storage = Storage::init(context);
 /// }
 fn abstract_storage(storage_struct_name: String, unconstrained: bool) -> Statement {
-    let context_expr = if unconstrained {
-        // Note that the literal unit type (i.e. '()') is not the same as a tuple with zero elements
-        expression(ExpressionKind::Literal(Literal::Unit))
-    } else {
-        mutable_reference("context")
-    };
+    let context_expr =
+        if unconstrained { variable("context") } else { mutable_reference("context") };
 
     assignment(
         "storage", // Assigned to

compiler/noirc_driver/src/lib.rs

@@ -103,6 +103,10 @@ pub struct CompileOptions {
     /// Enable the experimental elaborator pass
     #[arg(long, hide = true)]
     pub use_elaborator: bool,
+
+    /// Outputs the paths to any modified artifacts
+    #[arg(long, hide = true)]
+    pub show_artifact_paths: bool,
 }
 
 fn parse_expression_width(input: &str) -> Result<ExpressionWidth, std::io::Error> {

compiler/noirc_evaluator/src/ssa.rs

@@ -58,8 +58,9 @@ pub(crate) fn optimize_into_acir(
     let ssa = SsaBuilder::new(program, print_passes, force_brillig_output, print_timings)?
         .run_pass(Ssa::defunctionalize, "After Defunctionalization:")
         .run_pass(Ssa::remove_paired_rc, "After Removing Paired rc_inc & rc_decs:")
-        .run_pass(Ssa::inline_functions, "After Inlining:")
+        .run_pass(Ssa::separate_runtime, "After Runtime Separation:")
         .run_pass(Ssa::resolve_is_unconstrained, "After Resolving IsUnconstrained:")
+        .run_pass(Ssa::inline_functions, "After Inlining:")
         // Run mem2reg with the CFG separated into blocks
         .run_pass(Ssa::mem2reg, "After Mem2Reg:")
         .run_pass(Ssa::as_slice_optimization, "After `as_slice` optimization")

compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs

@@ -255,7 +255,7 @@ impl AcirContext {
     }
 
     /// Converts an [`AcirVar`] to a [`Witness`]
-    fn var_to_witness(&mut self, var: AcirVar) -> Result<Witness, InternalError> {
+    pub(crate) fn var_to_witness(&mut self, var: AcirVar) -> Result<Witness, InternalError> {
         let expression = self.var_to_expression(var)?;
         let witness = if let Some(constant) = expression.to_const() {
             // Check if a witness has been assigned this value already, if so reuse it.
@@ -979,6 +979,7 @@ impl AcirContext {
         let max_power_of_two = self.add_constant(
             FieldElement::from(2_i128).pow(&FieldElement::from(bit_size as i128 - 1)),
         );
+        let zero = self.add_constant(FieldElement::zero());
         let one = self.add_constant(FieldElement::one());
 
         // Get the sign bit of rhs by computing rhs / max_power_of_two
@@ -998,10 +999,19 @@ impl AcirContext {
         // Unsigned to signed: derive q and r from q1,r1 and the signs of lhs and rhs
         // Quotient sign is lhs sign * rhs sign, whose resulting sign bit is the XOR of the sign bits
         let q_sign = self.xor_var(lhs_leading, rhs_leading, AcirType::unsigned(1))?;
-
         let quotient = self.two_complement(q1, q_sign, bit_size)?;
         let remainder = self.two_complement(r1, lhs_leading, bit_size)?;
 
+        // Issue #5129 - When q1 is zero and quotient sign is -1, we compute -0=2^{bit_size},
+        // which is not valid because we do not wrap integer operations
+        // Similar case can happen with the remainder.
+        let q_is_0 = self.eq_var(q1, zero)?;
+        let q_is_not_0 = self.not_var(q_is_0, AcirType::unsigned(1))?;
+        let quotient = self.mul_var(quotient, q_is_not_0)?;
+        let r_is_0 = self.eq_var(r1, zero)?;
+        let r_is_not_0 = self.not_var(r_is_0, AcirType::unsigned(1))?;
+        let remainder = self.mul_var(remainder, r_is_not_0)?;
+
         Ok((quotient, remainder))
     }
 
@@ -1017,15 +1027,6 @@ impl AcirContext {
         Ok(remainder)
     }
 
-    /// Converts the `AcirVar` to a `Witness` if it hasn't been already, and appends it to the
-    /// `GeneratedAcir`'s return witnesses.
-    pub(crate) fn return_var(&mut self, acir_var: AcirVar) -> Result<(), InternalError> {
-        let return_var = self.get_or_create_witness_var(acir_var)?;
-        let witness = self.var_to_witness(return_var)?;
-        self.acir_ir.push_return_witness(witness);
-        Ok(())
-    }
-
     /// Constrains the `AcirVar` variable to be of type `NumericType`.
     pub(crate) fn range_constrain_var(
         &mut self,
@@ -1528,9 +1529,11 @@ impl AcirContext {
     pub(crate) fn finish(
         mut self,
         inputs: Vec<Witness>,
+        return_values: Vec<Witness>,
         warnings: Vec<SsaReport>,
     ) -> GeneratedAcir {
         self.acir_ir.input_witnesses = inputs;
+        self.acir_ir.return_witnesses = return_values;
         self.acir_ir.warnings = warnings;
         self.acir_ir
     }

compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs

@@ -45,9 +45,6 @@ pub(crate) struct GeneratedAcir {
     opcodes: Vec<AcirOpcode<FieldElement>>,
 
     /// All witness indices that comprise the final return value of the program
-    ///
-    /// Note: This may contain repeated indices, which is necessary for later mapping into the
-    /// abi's return type.
     pub(crate) return_witnesses: Vec<Witness>,
 
     /// All witness indices which are inputs to the main function
@@ -164,11 +161,6 @@ impl GeneratedAcir {
 
         fresh_witness
     }
-
-    /// Adds a witness index to the program's return witnesses.
-    pub(crate) fn push_return_witness(&mut self, witness: Witness) {
-        self.return_witnesses.push(witness);
-    }
 }
 
 impl GeneratedAcir {