fix(acir_gen): Pass accurate contents to slice inputs for bb func cal…

…ls (#2435)
noir-lang · Aug 25, 2023 · 054642b · 054642b
1 parent 1435a86
commit 054642b
Show file tree

Hide file tree

Showing 7 changed files with 241 additions and 72 deletions.
diff --git a/crates/nargo_cli/tests/execution_success/brillig_schnorr/Prover.toml b/crates/nargo_cli/tests/execution_success/brillig_schnorr/Prover.toml
@@ -1,4 +1,5 @@
 message = [0,1,2,3,4,5,6,7,8,9]
+message_field = "0x010203040506070809"
 pub_key_x = "0x17cbd3ed3151ccfd170efe1d54280a6a4822640bf5c369908ad74ea21518a9c5"
 pub_key_y = "0x0e0456e3795c1a31f20035b741cd6158929eeccd320d299cfcac962865a6bc74"
 signature = [

diff --git a/crates/nargo_cli/tests/execution_success/brillig_schnorr/src/main.nr b/crates/nargo_cli/tests/execution_success/brillig_schnorr/src/main.nr
@@ -2,9 +2,20 @@ use dep::std;
 
 // Note: If main has any unsized types, then the verifier will never be able
 // to figure out the circuit instance
-unconstrained fn main(message: [u8; 10], pub_key_x: Field, pub_key_y: Field, signature: [u8; 64]) {
+unconstrained fn main(message: [u8; 10], message_field: Field, pub_key_x: Field, pub_key_y: Field, signature: [u8; 64]) {
+    // Regression for issue #2421
+    // We want to make sure that we can accurately verify a signature whose message is a slice vs. an array
+    let message_field_bytes = message_field.to_be_bytes(10);
+    for i in 0..10 {
+        assert(message[i] == message_field_bytes[i]);
+    }
     // Is there ever a situation where someone would want 
     // to ensure that a signature was invalid?
+    // Check that passing a slice as the message is valid
+    let valid_signature = std::schnorr::verify_signature(pub_key_x,pub_key_y,signature, message_field_bytes);
+    assert(valid_signature);
+
+    // Check that passing an array as the message is valid
     let valid_signature = std::schnorr::verify_signature(pub_key_x,pub_key_y,signature, message);
     assert(valid_signature);
 }
diff --git a/crates/nargo_cli/tests/execution_success/schnorr/Prover.toml b/crates/nargo_cli/tests/execution_success/schnorr/Prover.toml
@@ -1,4 +1,5 @@
 message = [0,1,2,3,4,5,6,7,8,9]
+message_field = "0x010203040506070809"
 pub_key_x = "0x17cbd3ed3151ccfd170efe1d54280a6a4822640bf5c369908ad74ea21518a9c5"
 pub_key_y = "0x0e0456e3795c1a31f20035b741cd6158929eeccd320d299cfcac962865a6bc74"
 signature = [

diff --git a/crates/nargo_cli/tests/execution_success/schnorr/src/main.nr b/crates/nargo_cli/tests/execution_success/schnorr/src/main.nr
@@ -2,9 +2,20 @@ use dep::std;
 
 // Note: If main has any unsized types, then the verifier will never be able
 // to figure out the circuit instance
-fn main(message: [u8; 10], pub_key_x: Field, pub_key_y: Field, signature: [u8; 64]) {
+fn main(message: [u8; 10], message_field: Field, pub_key_x: Field, pub_key_y: Field, signature: [u8; 64]) {
+    // Regression for issue #2421
+    // We want to make sure that we can accurately verify a signature whose message is a slice vs. an array
+    let message_field_bytes = message_field.to_be_bytes(10);
+    for i in 0..10 {
+        assert(message[i] == message_field_bytes[i]);
+    }
     // Is there ever a situation where someone would want 
     // to ensure that a signature was invalid?
+    // Check that passing a slice as the message is valid
+    let valid_signature = std::schnorr::verify_signature(pub_key_x,pub_key_y,signature, message_field_bytes);
+    assert(valid_signature);
+
+    // Check that passing an array as the message is valid
     let valid_signature = std::schnorr::verify_signature(pub_key_x,pub_key_y,signature, message);
     assert(valid_signature);
 }
diff --git a/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs b/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs
@@ -16,12 +16,24 @@ pub(crate) fn convert_black_box_call(
 ) {
     match bb_func {
         BlackBoxFunc::SHA256 => {
-            if let (
-                [RegisterOrMemory::HeapArray(message_array)],
-                [RegisterOrMemory::HeapArray(result_array)],
-            ) = (function_arguments, function_results)
+            if let ([..], [RegisterOrMemory::HeapArray(result_array)]) =
+                (function_arguments, function_results)
             {
-                let message_vector = brillig_context.array_to_vector(message_array);
+                // Slices are represented as a tuple of (length, slice contents).
+                // We must check the number of inputs to differentiate between arrays and slices
+                // and make sure that we pass the correct inputs to the function call.
+                let message = if function_arguments.len() > 1 {
+                    &function_arguments[1]
+                } else {
+                    &function_arguments[0]
+                };
+                let message_vector = match message {
+                    RegisterOrMemory::HeapArray(message_array) => {
+                        brillig_context.array_to_vector(message_array)
+                    }
+                    RegisterOrMemory::HeapVector(message_vector) => *message_vector,
+                    _ => unreachable!("ICE: SHA256 expects the message to be an array or a vector"),
+                };
                 brillig_context.black_box_op_instruction(BlackBoxOp::Sha256 {
                     message: message_vector,
                     output: *result_array,
@@ -31,12 +43,26 @@ pub(crate) fn convert_black_box_call(
             }
         }
         BlackBoxFunc::Blake2s => {
-            if let (
-                [RegisterOrMemory::HeapArray(message_array)],
-                [RegisterOrMemory::HeapArray(result_array)],
-            ) = (function_arguments, function_results)
+            if let ([..], [RegisterOrMemory::HeapArray(result_array)]) =
+                (function_arguments, function_results)
             {
-                let message_vector = brillig_context.array_to_vector(message_array);
+                // Slices are represented as a tuple of (length, slice contents).
+                // We must check the number of inputs to differentiate between arrays and slices
+                // and make sure that we pass the correct inputs to the function call.
+                let message = if function_arguments.len() > 1 {
+                    &function_arguments[1]
+                } else {
+                    &function_arguments[0]
+                };
+                let message_vector = match message {
+                    RegisterOrMemory::HeapArray(message_array) => {
+                        brillig_context.array_to_vector(message_array)
+                    }
+                    RegisterOrMemory::HeapVector(message_vector) => *message_vector,
+                    _ => {
+                        unreachable!("ICE: Blake2s expects the message to be an array or a vector")
+                    }
+                };
                 brillig_context.black_box_op_instruction(BlackBoxOp::Blake2s {
                     message: message_vector,
                     output: *result_array,
@@ -47,12 +73,27 @@ pub(crate) fn convert_black_box_call(
         }
         BlackBoxFunc::Keccak256 => {
             if let (
-                [RegisterOrMemory::HeapArray(message_array), RegisterOrMemory::RegisterIndex(array_size)],
+                [.., RegisterOrMemory::RegisterIndex(array_size)],
                 [RegisterOrMemory::HeapArray(result_array)],
             ) = (function_arguments, function_results)
             {
-                let message_vector =
-                    HeapVector { size: *array_size, pointer: message_array.pointer };
+                // Slices are represented as a tuple of (length, slice contents).
+                // We must check the number of inputs to differentiate between arrays and slices
+                // and make sure that we pass the correct inputs to the function call.
+                let message = if function_arguments.len() > 2 {
+                    &function_arguments[1]
+                } else {
+                    &function_arguments[0]
+                };
+                let message_vector = match message {
+                    RegisterOrMemory::HeapArray(message_array) => {
+                        HeapVector { size: *array_size, pointer: message_array.pointer }
+                    }
+                    RegisterOrMemory::HeapVector(message_vector) => *message_vector,
+                    _ => unreachable!(
+                        "ICE: Keccak256 expects the message to be an array or a vector"
+                    ),
+                };
                 brillig_context.black_box_op_instruction(BlackBoxOp::Keccak256 {
                     message: message_vector,
                     output: *result_array,
@@ -62,12 +103,26 @@ pub(crate) fn convert_black_box_call(
             }
         }
         BlackBoxFunc::HashToField128Security => {
-            if let (
-                [RegisterOrMemory::HeapArray(message_array)],
-                [RegisterOrMemory::RegisterIndex(result_register)],
-            ) = (function_arguments, function_results)
+            if let ([..], [RegisterOrMemory::RegisterIndex(result_register)]) =
+                (function_arguments, function_results)
             {
-                let message_vector = brillig_context.array_to_vector(message_array);
+                // Slices are represented as a tuple of (length, slice contents).
+                // We must check the number of inputs to differentiate between arrays and slices
+                // and make sure that we pass the correct inputs to the function call.
+                let message = if function_arguments.len() > 1 {
+                    &function_arguments[1]
+                } else {
+                    &function_arguments[0]
+                };
+                let message_vector = match message {
+                    RegisterOrMemory::HeapArray(message_array) => {
+                        brillig_context.array_to_vector(message_array)
+                    }
+                    RegisterOrMemory::HeapVector(message_vector) => {
+                        *message_vector
+                    }
+                    _ => unreachable!("ICE: HashToField128Security expects the message to be an array or a vector"),
+                };
                 brillig_context.black_box_op_instruction(BlackBoxOp::HashToField128Security {
                     message: message_vector,
                     output: *result_register,
@@ -78,11 +133,27 @@ pub(crate) fn convert_black_box_call(
         }
         BlackBoxFunc::EcdsaSecp256k1 => {
             if let (
-                [RegisterOrMemory::HeapArray(public_key_x), RegisterOrMemory::HeapArray(public_key_y), RegisterOrMemory::HeapArray(signature), RegisterOrMemory::HeapArray(message_hash)],
+                [RegisterOrMemory::HeapArray(public_key_x), RegisterOrMemory::HeapArray(public_key_y), RegisterOrMemory::HeapArray(signature), ..],
                 [RegisterOrMemory::RegisterIndex(result_register)],
             ) = (function_arguments, function_results)
             {
-                let message_hash_vector = brillig_context.array_to_vector(message_hash);
+                // Slices are represented as a tuple of (length, slice contents).
+                // We must check the number of inputs to differentiate between arrays and slices
+                // and make sure that we pass the correct inputs to the function call.
+                let message = if function_arguments.len() > 4 {
+                    &function_arguments[4]
+                } else {
+                    &function_arguments[3]
+                };
+                let message_hash_vector = match message {
+                    RegisterOrMemory::HeapArray(message_hash) => {
+                        brillig_context.array_to_vector(message_hash)
+                    }
+                    RegisterOrMemory::HeapVector(message_hash_vector) => *message_hash_vector,
+                    _ => unreachable!(
+                        "ICE: EcdsaSecp256k1 expects the message to be an array or a vector"
+                    ),
+                };
                 brillig_context.black_box_op_instruction(BlackBoxOp::EcdsaSecp256k1 {
                     hashed_msg: message_hash_vector,
                     public_key_x: *public_key_x,
@@ -98,11 +169,27 @@ pub(crate) fn convert_black_box_call(
         }
         BlackBoxFunc::Pedersen => {
             if let (
-                [RegisterOrMemory::HeapArray(message_array), RegisterOrMemory::RegisterIndex(domain_separator)],
+                [.., RegisterOrMemory::RegisterIndex(domain_separator)],
                 [RegisterOrMemory::HeapArray(result_array)],
             ) = (function_arguments, function_results)
             {
-                let message_vector = brillig_context.array_to_vector(message_array);
+                // Slices are represented as a tuple of (length, slice contents).
+                // We must check the number of inputs to differentiate between arrays and slices
+                // and make sure that we pass the correct inputs to the function call.
+                let message = if function_arguments.len() > 2 {
+                    &function_arguments[1]
+                } else {
+                    &function_arguments[0]
+                };
+                let message_vector = match message {
+                    RegisterOrMemory::HeapArray(message_array) => {
+                        brillig_context.array_to_vector(message_array)
+                    }
+                    RegisterOrMemory::HeapVector(message_vector) => *message_vector,
+                    _ => {
+                        unreachable!("ICE: Pedersen expects the message to be an array or a vector")
+                    }
+                };
                 brillig_context.black_box_op_instruction(BlackBoxOp::Pedersen {
                     inputs: message_vector,
                     domain_separator: *domain_separator,
@@ -114,11 +201,27 @@ pub(crate) fn convert_black_box_call(
         }
         BlackBoxFunc::SchnorrVerify => {
             if let (
-                [RegisterOrMemory::RegisterIndex(public_key_x), RegisterOrMemory::RegisterIndex(public_key_y), RegisterOrMemory::HeapArray(signature), RegisterOrMemory::HeapArray(message_hash)],
+                [RegisterOrMemory::RegisterIndex(public_key_x), RegisterOrMemory::RegisterIndex(public_key_y), RegisterOrMemory::HeapArray(signature), ..],
                 [RegisterOrMemory::RegisterIndex(result_register)],
             ) = (function_arguments, function_results)
             {
-                let message_hash = brillig_context.array_to_vector(message_hash);
+                // Slices are represented as a tuple of (length, slice contents).
+                // We must check the number of inputs to differentiate between arrays and slices
+                // and make sure that we pass the correct inputs to the function call.
+                let message = if function_arguments.len() > 4 {
+                    &function_arguments[4]
+                } else {
+                    &function_arguments[3]
+                };
+                let message_hash = match message {
+                    RegisterOrMemory::HeapArray(message_hash) => {
+                        brillig_context.array_to_vector(message_hash)
+                    }
+                    RegisterOrMemory::HeapVector(message_hash) => *message_hash,
+                    _ => unreachable!(
+                        "ICE: Schnorr verify expects the message to be an array or a vector"
+                    ),
+                };
                 let signature = brillig_context.array_to_vector(signature);
                 brillig_context.black_box_op_instruction(BlackBoxOp::SchnorrVerify {
                     public_key_x: *public_key_x,