feat(acir_gen): RecursiveAggregation opcode and updates to black box func call generation

crates/noirc_evaluator/src/errors.rs

@@ -44,8 +44,6 @@ pub enum InternalError {
     MissingArg { name: String, arg: String, location: Option<Location> },
     #[error("ICE: {name:?} should be a constant")]
     NotAConstant { name: String, location: Option<Location> },
-    #[error("{name:?} is not implemented yet")]
-    NotImplemented { name: String, location: Option<Location> },
     #[error("ICE: Undeclared AcirVar")]
     UndeclaredAcirVar { location: Option<Location> },
     #[error("ICE: Expected {expected:?}, found {found:?}")]
@@ -61,7 +59,6 @@ impl From<RuntimeError> for FileDiagnostic {
                 | InternalError::General { location, .. }
                 | InternalError::MissingArg { location, .. }
                 | InternalError::NotAConstant { location, .. }
-                | InternalError::NotImplemented { location, .. }
                 | InternalError::UndeclaredAcirVar { location }
                 | InternalError::UnExpected { location, .. } => {
                     let file_id = location.map(|loc| loc.file).unwrap();

crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs

@@ -260,7 +260,7 @@ impl AcirContext {
         typ: AcirType,
     ) -> Result<AcirVar, RuntimeError> {
         let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)];
-        let outputs = self.black_box_function(BlackBoxFunc::XOR, inputs)?;
+        let outputs = self.black_box_function(BlackBoxFunc::XOR, inputs, 1)?;
         Ok(outputs[0])
     }
 
@@ -272,7 +272,7 @@ impl AcirContext {
         typ: AcirType,
     ) -> Result<AcirVar, RuntimeError> {
         let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)];
-        let outputs = self.black_box_function(BlackBoxFunc::AND, inputs)?;
+        let outputs = self.black_box_function(BlackBoxFunc::AND, inputs, 1)?;
         Ok(outputs[0])
     }
 
@@ -299,7 +299,7 @@ impl AcirContext {
             let a = self.sub_var(max, lhs)?;
             let b = self.sub_var(max, rhs)?;
             let inputs = vec![AcirValue::Var(a, typ.clone()), AcirValue::Var(b, typ)];
-            let outputs = self.black_box_function(BlackBoxFunc::AND, inputs)?;
+            let outputs = self.black_box_function(BlackBoxFunc::AND, inputs, 1)?;
             self.sub_var(max, outputs[0])
         }
     }
@@ -677,6 +677,7 @@ impl AcirContext {
         &mut self,
         name: BlackBoxFunc,
         mut inputs: Vec<AcirValue>,
+        output_count: usize,
     ) -> Result<Vec<AcirVar>, RuntimeError> {
         // Separate out any arguments that should be constants
         let constants = match name {
@@ -712,7 +713,7 @@ impl AcirContext {
         let inputs = self.prepare_inputs_for_black_box_func_call(inputs)?;
 
         // Call Black box with `FunctionInput`
-        let outputs = self.acir_ir.call_black_box(name, inputs, constants)?;
+        let outputs = self.acir_ir.call_black_box(name, &inputs, constants, output_count)?;
 
         // Convert `Witness` values which are now constrained to be the output of the
         // black box function call into `AcirVar`s.
@@ -728,9 +729,10 @@ impl AcirContext {
     fn prepare_inputs_for_black_box_func_call(
         &mut self,
         inputs: Vec<AcirValue>,
-    ) -> Result<Vec<FunctionInput>, RuntimeError> {
+    ) -> Result<Vec<Vec<FunctionInput>>, RuntimeError> {
         let mut witnesses = Vec::new();
         for input in inputs {
+            let mut single_val_witnesses = Vec::new();
             for (input, typ) in input.flatten() {
                 let var_data = &self.vars[&input];
 
@@ -740,8 +742,9 @@ impl AcirContext {
                 let expr = var_data.to_expression();
                 let witness = self.acir_ir.get_or_create_witness(&expr);
                 let num_bits = typ.bit_size();
-                witnesses.push(FunctionInput { witness, num_bits });
+                single_val_witnesses.push(FunctionInput { witness, num_bits });
             }
+            witnesses.push(single_val_witnesses);
         }
         Ok(witnesses)
     }

crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs

@@ -122,70 +122,75 @@ impl GeneratedAcir {
     pub(crate) fn call_black_box(
         &mut self,
         func_name: BlackBoxFunc,
-        mut inputs: Vec<FunctionInput>,
+        inputs: &[Vec<FunctionInput>],
         constants: Vec<FieldElement>,
+        output_count: usize,
     ) -> Result<Vec<Witness>, InternalError> {
-        intrinsics_check_inputs(func_name, &inputs)?;
+        let input_count = inputs.iter().fold(0usize, |sum, val| sum + val.len());
+        intrinsics_check_inputs(func_name, input_count);
+        intrinsics_check_outputs(func_name, output_count);
 
-        let output_count = black_box_expected_output_size(func_name)?;
         let outputs = vecmap(0..output_count, |_| self.next_witness_index());
 
         // clone is needed since outputs is moved when used in blackbox function.
         let outputs_clone = outputs.clone();
 
         let black_box_func_call = match func_name {
             BlackBoxFunc::AND => {
-                BlackBoxFuncCall::AND { lhs: inputs[0], rhs: inputs[1], output: outputs[0] }
+                BlackBoxFuncCall::AND { lhs: inputs[0][0], rhs: inputs[1][0], output: outputs[0] }
             }
             BlackBoxFunc::XOR => {
-                BlackBoxFuncCall::XOR { lhs: inputs[0], rhs: inputs[1], output: outputs[0] }
+                BlackBoxFuncCall::XOR { lhs: inputs[0][0], rhs: inputs[1][0], output: outputs[0] }
             }
-            BlackBoxFunc::RANGE => BlackBoxFuncCall::RANGE { input: inputs[0] },
-            BlackBoxFunc::SHA256 => BlackBoxFuncCall::SHA256 { inputs, outputs },
-            BlackBoxFunc::Blake2s => BlackBoxFuncCall::Blake2s { inputs, outputs },
-            BlackBoxFunc::HashToField128Security => {
-                BlackBoxFuncCall::HashToField128Security { inputs, output: outputs[0] }
+            BlackBoxFunc::RANGE => BlackBoxFuncCall::RANGE { input: inputs[0][0] },
+            BlackBoxFunc::SHA256 => BlackBoxFuncCall::SHA256 { inputs: inputs[0].clone(), outputs },
+            BlackBoxFunc::Blake2s => {
+                BlackBoxFuncCall::Blake2s { inputs: inputs[0].clone(), outputs }
             }
+            BlackBoxFunc::HashToField128Security => BlackBoxFuncCall::HashToField128Security {
+                inputs: inputs[0].clone(),
+                output: outputs[0],
+            },
             BlackBoxFunc::SchnorrVerify => BlackBoxFuncCall::SchnorrVerify {
-                public_key_x: inputs[0],
-                public_key_y: inputs[1],
+                public_key_x: inputs[0][0],
+                public_key_y: inputs[1][0],
                 // Schnorr signature is an r & s, 32 bytes each
-                signature: inputs[2..66].to_vec(),
-                message: inputs[66..].to_vec(),
+                signature: inputs[2].clone(),
+                message: inputs[3].clone(),
                 output: outputs[0],
             },
             BlackBoxFunc::Pedersen => BlackBoxFuncCall::Pedersen {
-                inputs,
+                inputs: inputs[0].clone(),
                 outputs: (outputs[0], outputs[1]),
                 domain_separator: constants[0].to_u128() as u32,
             },
             BlackBoxFunc::EcdsaSecp256k1 => BlackBoxFuncCall::EcdsaSecp256k1 {
                 // 32 bytes for each public key co-ordinate
-                public_key_x: inputs[0..32].to_vec(),
-                public_key_y: inputs[32..64].to_vec(),
+                public_key_x: inputs[0].clone(),
+                public_key_y: inputs[1].clone(),
                 // (r,s) are both 32 bytes each, so signature
                 // takes up 64 bytes
-                signature: inputs[64..128].to_vec(),
-                hashed_message: inputs[128..].to_vec(),
+                signature: inputs[2].clone(),
+                hashed_message: inputs[3].clone(),
                 output: outputs[0],
             },
             BlackBoxFunc::EcdsaSecp256r1 => BlackBoxFuncCall::EcdsaSecp256r1 {
                 // 32 bytes for each public key co-ordinate
-                public_key_x: inputs[0..32].to_vec(),
-                public_key_y: inputs[32..64].to_vec(),
+                public_key_x: inputs[0].clone(),
+                public_key_y: inputs[1].clone(),
                 // (r,s) are both 32 bytes each, so signature
                 // takes up 64 bytes
-                signature: inputs[64..128].to_vec(),
-                hashed_message: inputs[128..].to_vec(),
+                signature: inputs[2].clone(),
+                hashed_message: inputs[3].clone(),
                 output: outputs[0],
             },
             BlackBoxFunc::FixedBaseScalarMul => BlackBoxFuncCall::FixedBaseScalarMul {
-                input: inputs[0],
+                input: inputs[0][0],
                 outputs: (outputs[0], outputs[1]),
             },
             BlackBoxFunc::Keccak256 => {
-                let var_message_size = match inputs.pop() {
-                    Some(var_message_size) => var_message_size,
+                let var_message_size = match inputs.to_vec().pop() {
+                    Some(var_message_size) => var_message_size[0],
                     None => {
                         return Err(InternalError::MissingArg {
                             name: "".to_string(),
@@ -194,14 +199,31 @@ impl GeneratedAcir {
                         });
                     }
                 };
-                BlackBoxFuncCall::Keccak256VariableLength { inputs, var_message_size, outputs }
+                BlackBoxFuncCall::Keccak256VariableLength {
+                    inputs: inputs[0].clone(),
+                    var_message_size,
+                    outputs,
+                }
             }
-            // TODO(#1570): Generate ACIR for recursive aggregation
             BlackBoxFunc::RecursiveAggregation => {
-                return Err(InternalError::NotImplemented {
-                    name: "recursive aggregation".to_string(),
-                    location: None,
-                })
+                let has_previous_aggregation = self.opcodes.iter().any(|op| {
+                    matches!(
+                        op,
+                        AcirOpcode::BlackBoxFuncCall(BlackBoxFuncCall::RecursiveAggregation { .. })
+                    )
+                });
+
+                let input_aggregation_object =
+                    if !has_previous_aggregation { None } else { Some(inputs[4].clone()) };
+
+                BlackBoxFuncCall::RecursiveAggregation {
+                    verification_key: inputs[0].clone(),
+                    proof: inputs[1].clone(),
+                    public_inputs: inputs[2].clone(),
+                    key_hash: inputs[3][0],
+                    input_aggregation_object,
+                    output_aggregation_object: outputs,
+                }
             }
         };
 
@@ -807,68 +829,60 @@ impl GeneratedAcir {
 
 /// This function will return the number of inputs that a blackbox function
 /// expects. Returning `None` if there is no expectation.
-fn black_box_func_expected_input_size(name: BlackBoxFunc) -> Result<Option<usize>, InternalError> {
+fn black_box_func_expected_input_size(name: BlackBoxFunc) -> Option<usize> {
     match name {
         // Bitwise opcodes will take in 2 parameters
-        BlackBoxFunc::AND | BlackBoxFunc::XOR => Ok(Some(2)),
+        BlackBoxFunc::AND | BlackBoxFunc::XOR => Some(2),
         // All of the hash/cipher methods will take in a
         // variable number of inputs.
         BlackBoxFunc::Keccak256
         | BlackBoxFunc::SHA256
         | BlackBoxFunc::Blake2s
         | BlackBoxFunc::Pedersen
-        | BlackBoxFunc::HashToField128Security => Ok(None),
+        | BlackBoxFunc::HashToField128Security => None,
 
         // Can only apply a range constraint to one
         // witness at a time.
-        BlackBoxFunc::RANGE => Ok(Some(1)),
+        BlackBoxFunc::RANGE => Some(1),
 
         // Signature verification algorithms will take in a variable
         // number of inputs, since the message/hashed-message can vary in size.
         BlackBoxFunc::SchnorrVerify
         | BlackBoxFunc::EcdsaSecp256k1
-        | BlackBoxFunc::EcdsaSecp256r1 => Ok(None),
+        | BlackBoxFunc::EcdsaSecp256r1 => None,
         // Inputs for fixed based scalar multiplication
         // is just a scalar
-        BlackBoxFunc::FixedBaseScalarMul => Ok(Some(1)),
-        // TODO(#1570): Generate ACIR for recursive aggregation
-        // RecursiveAggregation has variable inputs and we could return `None` here,
-        // but as it is not fully implemented we return an ICE error for now
-        BlackBoxFunc::RecursiveAggregation => Err(InternalError::NotImplemented {
-            name: "recursive aggregation".to_string(),
-            location: None,
-        }),
+        BlackBoxFunc::FixedBaseScalarMul => Some(1),
+        // Recursive aggregation has a variable number of inputs
+        BlackBoxFunc::RecursiveAggregation => None,
     }
 }
 
 /// This function will return the number of outputs that a blackbox function
 /// expects. Returning `None` if there is no expectation.
-fn black_box_expected_output_size(name: BlackBoxFunc) -> Result<u32, InternalError> {
+fn black_box_expected_output_size(name: BlackBoxFunc) -> Option<usize> {
     match name {
         // Bitwise opcodes will return 1 parameter which is the output
         // or the operation.
-        BlackBoxFunc::AND | BlackBoxFunc::XOR => Ok(1),
+        BlackBoxFunc::AND | BlackBoxFunc::XOR => Some(1),
         // 32 byte hash algorithms
-        BlackBoxFunc::Keccak256 | BlackBoxFunc::SHA256 | BlackBoxFunc::Blake2s => Ok(32),
+        BlackBoxFunc::Keccak256 | BlackBoxFunc::SHA256 | BlackBoxFunc::Blake2s => Some(32),
         // Hash to field returns a field element
-        BlackBoxFunc::HashToField128Security => Ok(1),
+        BlackBoxFunc::HashToField128Security => Some(1),
         // Pedersen returns a point
-        BlackBoxFunc::Pedersen => Ok(2),
+        BlackBoxFunc::Pedersen => Some(2),
         // Can only apply a range constraint to one
         // witness at a time.
-        BlackBoxFunc::RANGE => Ok(0),
+        BlackBoxFunc::RANGE => Some(0),
         // Signature verification algorithms will return a boolean
         BlackBoxFunc::SchnorrVerify
         | BlackBoxFunc::EcdsaSecp256k1
-        | BlackBoxFunc::EcdsaSecp256r1 => Ok(1),
+        | BlackBoxFunc::EcdsaSecp256r1 => Some(1),
         // Output of fixed based scalar mul over the embedded curve
         // will be 2 field elements representing the point.
-        BlackBoxFunc::FixedBaseScalarMul => Ok(2),
-        // TODO(#1570): Generate ACIR for recursive aggregation
-        BlackBoxFunc::RecursiveAggregation => Err(InternalError::NotImplemented {
-            name: "recursive aggregation".to_string(),
-            location: None,
-        }),
+        BlackBoxFunc::FixedBaseScalarMul => Some(2),
+        // Recursive aggregation has a variable number of outputs
+        BlackBoxFunc::RecursiveAggregation => None,
     }
 }
 
@@ -887,16 +901,41 @@ fn black_box_expected_output_size(name: BlackBoxFunc) -> Result<u32, InternalErr
 /// #[foreign(sha256)]
 /// fn sha256<N>(_input : [u8; N]) -> [u8; 32] {}
 /// ``
-fn intrinsics_check_inputs(
-    name: BlackBoxFunc,
-    inputs: &[FunctionInput],
-) -> Result<(), InternalError> {
-    let expected_num_inputs = match black_box_func_expected_input_size(name)? {
+fn intrinsics_check_inputs(name: BlackBoxFunc, input_count: usize) {
+    let expected_num_inputs = match black_box_func_expected_input_size(name) {
+        Some(expected_num_inputs) => expected_num_inputs,
+        None => return,
+    };
+
+    assert_eq!(expected_num_inputs,input_count,"Tried to call black box function {name} with {input_count} inputs, but this function's definition requires {expected_num_inputs} inputs");
+}
+
+/// Checks that the number of outputs being used to call the blackbox function
+/// is correct according to the function definition.
+///
+/// Some functions expect a variable number of outputs and in such a case,
+/// this method will do nothing.  An example of this is recursive aggregation.
+/// In that case, this function will not check anything.
+///
+/// Since we expect black box functions to be called behind a Noir shim function,
+/// we trigger a compiler error if the inputs do not match.
+///
+/// An example of Noir shim function is the following:
+/// ``
+/// #[foreign(sha256)]
+/// fn verify_proof<N>(
+///     _verification_key : [Field],
+///     _proof : [Field],
+///     _public_inputs : [Field],
+///     _key_hash : Field,
+///     _input_aggregation_object : [Field; N]
+/// ) -> [Field; N] {}
+/// ``
+fn intrinsics_check_outputs(name: BlackBoxFunc, output_count: usize) {
+    let expected_num_outputs = match black_box_expected_output_size(name) {
         Some(expected_num_inputs) => expected_num_inputs,
-        None => return Ok(()),
+        None => return,
     };
-    let got_num_inputs = inputs.len();
 
-    assert_eq!(expected_num_inputs,inputs.len(),"Tried to call black box function {name} with {got_num_inputs} inputs, but this function's definition requires {expected_num_inputs} inputs");
-    Ok(())
+    assert_eq!(expected_num_outputs,output_count,"Tried to call black box function {name} with {output_count} inputs, but this function's definition requires {expected_num_outputs} inputs");
 }

crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs

@@ -943,7 +943,11 @@ impl Context {
             Intrinsic::BlackBox(black_box) => {
                 let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg));
 
-                let vars = self.acir_context.black_box_function(black_box, inputs)?;
+                let output_count = result_ids.iter().fold(0usize, |sum, result_id| {
+                    sum + dfg.try_get_array_length(*result_id).unwrap_or(1)
+                });
+
+                let vars = self.acir_context.black_box_function(black_box, inputs, output_count)?;
 
                 Ok(Self::convert_vars_to_values(vars, dfg, result_ids))
             }

noir_stdlib/src/lib.nr

@@ -18,11 +18,11 @@ mod compat;
 // Oracle calls are required to be wrapped in an unconstrained function
 // Thus, the only argument to the `println` oracle is expected to always be an ident 
 #[oracle(println)]
-unconstrained fn println_oracle<T, A>(_input: T) {}
+unconstrained fn println_oracle<T>(_input: T) {}
 
 unconstrained fn println<T>(input: T) {
     println_oracle(input);
 }
 
 #[foreign(recursive_aggregation)]
-fn verify_proof(_verification_key : [Field], _proof : [Field], _public_inputs : [Field], _key_hash : Field, _input_aggregation_object : [Field]) -> [Field] {}
+fn verify_proof<N>(_verification_key : [Field], _proof : [Field], _public_inputs : [Field], _key_hash : Field, _input_aggregation_object : [Field; N]) -> [Field; N] {}