From 7da9d0c0769581901d157479c1e4c371fa5a9ac1 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Wed, 22 Jan 2025 08:47:23 +0000 Subject: [PATCH] 11295: Revert noir repo changes related to toRadix error handling --- .../acir/src/circuit/black_box_functions.rs | 38 ----- .../acvm-repo/brillig/src/black_box.rs | 146 ------------------ noir/noir-repo/acvm-repo/brillig/src/lib.rs | 2 +- .../acvm-repo/brillig_vm/src/black_box.rs | 108 +++++-------- 4 files changed, 42 insertions(+), 252 deletions(-) diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/black_box_functions.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/black_box_functions.rs index d792c9b8820..d0ec7d02201 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/black_box_functions.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/black_box_functions.rs @@ -3,7 +3,6 @@ //! This makes certain zk-snark unfriendly computations cheaper than if they were //! implemented in more basic constraints. -use crate::brillig::BrilligBlackBoxFunc; use serde::{Deserialize, Serialize}; use strum_macros::EnumIter; @@ -222,43 +221,6 @@ impl BlackBoxFunc { } } -impl From for BrilligBlackBoxFunc { - fn from(func: BlackBoxFunc) -> Self { - match func { - BlackBoxFunc::AES128Encrypt => BrilligBlackBoxFunc::AES128Encrypt, - BlackBoxFunc::AND => { - unreachable!("BlackBoxFunc::AND does not have a Brillig counterpart") - } - BlackBoxFunc::XOR => { - unreachable!("BlackBoxFunc::XOR does not have a Brillig counterpart") - } - BlackBoxFunc::RANGE => { - unreachable!("BlackBoxFunc::RANGE does not have a Brillig counterpart") - } - BlackBoxFunc::Blake2s => BrilligBlackBoxFunc::Blake2s, - BlackBoxFunc::Blake3 => BrilligBlackBoxFunc::Blake3, - BlackBoxFunc::EcdsaSecp256k1 => BrilligBlackBoxFunc::EcdsaSecp256k1, - BlackBoxFunc::EcdsaSecp256r1 => BrilligBlackBoxFunc::EcdsaSecp256r1, - BlackBoxFunc::MultiScalarMul => BrilligBlackBoxFunc::MultiScalarMul, - BlackBoxFunc::Keccakf1600 => BrilligBlackBoxFunc::Keccakf1600, - BlackBoxFunc::RecursiveAggregation => { - unreachable!( - "BlackBoxFunc::RecursiveAggregation does not have a Brillig counterpart" - ) - } - BlackBoxFunc::EmbeddedCurveAdd => BrilligBlackBoxFunc::EmbeddedCurveAdd, - BlackBoxFunc::BigIntAdd => BrilligBlackBoxFunc::BigIntAdd, - BlackBoxFunc::BigIntSub => BrilligBlackBoxFunc::BigIntSub, - BlackBoxFunc::BigIntMul => BrilligBlackBoxFunc::BigIntMul, - BlackBoxFunc::BigIntDiv => BrilligBlackBoxFunc::BigIntDiv, - BlackBoxFunc::BigIntFromLeBytes => BrilligBlackBoxFunc::BigIntFromLeBytes, - BlackBoxFunc::BigIntToLeBytes => BrilligBlackBoxFunc::BigIntToLeBytes, - BlackBoxFunc::Poseidon2Permutation => BrilligBlackBoxFunc::Poseidon2Permutation, - BlackBoxFunc::Sha256Compression => BrilligBlackBoxFunc::Sha256Compression, - } - } -} - #[cfg(test)] mod tests { use strum::IntoEnumIterator; diff --git a/noir/noir-repo/acvm-repo/brillig/src/black_box.rs b/noir/noir-repo/acvm-repo/brillig/src/black_box.rs index 7c636f57aea..be9ba20ed49 100644 --- a/noir/noir-repo/acvm-repo/brillig/src/black_box.rs +++ b/noir/noir-repo/acvm-repo/brillig/src/black_box.rs @@ -107,149 +107,3 @@ pub enum BlackBoxOp { output_bits: MemoryAddress, }, } - -/// Enum listing Brillig black box functions. There is one-to-one correspondence with the previous enum BlackBoxOp. -#[allow(clippy::upper_case_acronyms)] -#[derive(Clone, Debug, Hash, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub enum BrilligBlackBoxFunc { - /// Ciphers (encrypts) the provided plaintext using AES128 in CBC mode, - /// padding the input using PKCS#7. - /// - inputs: HeapVector of `u8` - /// - iv: initialization vector of type HeapArray `[u8; 16]` - /// - key: HeapArray `[u8; 16]` - /// - outputs: HeapVector of `u8` of length `inputs.len() + (16 - inputs.len() % 16)` - AES128Encrypt, - - /// Computes the Blake2s hash of the message, as specified in - /// https://tools.ietf.org/html/rfc7693 - /// - message: HeapVector of `u8` - /// - output: HeapArray of `u8` of length 32 - Blake2s, - - /// Computes the Blake3 hash of the message. - /// - message: HeapVector of `u8` - /// - output: HeapArray of `u8` of length 32 - Blake3, - - /// Keccak Permutation function of width 1600 - /// - input: HeapArray of `u8` representing 25 64-bit Keccak lanes that represent a keccak sponge of 1600 bits - /// - output: HeapArray of `u8` containing the result of a keccak f1600 permutation on the input state. Also an array of 25 Keccak lanes. - Keccakf1600, - - /// Verifies an ECDSA signature over the secp256k1 curve. - /// - inputs: - /// - x coordinate of public key as 32 bytes (HeapArray of `u8`) - /// - y coordinate of public key as 32 bytes (HeapArray of `u8`) - /// - the signature, as a 64 bytes array (HeapArray of `u8`) - /// - the hash of the message, as a vector of bytes (HeapVector of `u8`) - /// - result: 0 for failure and 1 for success - EcdsaSecp256k1, - - /// Verifies an ECDSA signature over the secp256r1 curve. - /// - /// Same as EcdsaSecp256k1, but done over another curve. - EcdsaSecp256r1, - - /// Multiple scalar multiplication (MSM) with a variable base/input point - /// (P) of the embedded (Grumpkin) curve. An MSM multiplies the points and scalars and - /// sums the results. - /// - input: - /// points: HeapVector of x and y coordinates (and infinity point flag) of the input - /// points `[x1, y1, x2, y2,...]`. - /// scalars: HeapVector of low and high limbs of input - /// scalars `[s1_low, s1_high, s2_low, s2_high, ...]`. (witness, N) - /// - outputs: HeapArray containing a tuple of `x` and `y` coordinates - /// (and infinity point flag) of output. - /// - /// Because the Grumpkin scalar field is bigger than the ACIR field, we - /// provide 2 ACIR fields representing the low and high parts of the Grumpkin - /// scalar $a$: `a=low+high*2^{128}`, with `low, high < 2^{128}` - MultiScalarMul, - - /// Addition over the embedded (Grumpkin) curve. - EmbeddedCurveAdd, - - /// BigInt addition - BigIntAdd, - - /// BigInt subtraction - BigIntSub, - - /// BigInt multiplication - BigIntMul, - - /// BigInt division - BigIntDiv, - - /// BigInt from le bytes - BigIntFromLeBytes, - - /// BigInt to le bytes - BigIntToLeBytes, - - /// Permutation function of Poseidon2 - Poseidon2Permutation, - - /// SHA256 compression function - /// - input: HeapArray of `u32` of length 16 - /// - state (hash_values): HeapArray of `u32` of length 8 - /// - output: HeapArray of `u32` of length 8 - Sha256Compression, - - /// Big-endian radix decomposition - ToRadix, -} - -impl std::fmt::Display for BrilligBlackBoxFunc { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.name()) - } -} - -impl From for BrilligBlackBoxFunc { - fn from(op: BlackBoxOp) -> Self { - match op { - BlackBoxOp::AES128Encrypt { .. } => BrilligBlackBoxFunc::AES128Encrypt, - BlackBoxOp::Blake2s { .. } => BrilligBlackBoxFunc::Blake2s, - BlackBoxOp::Blake3 { .. } => BrilligBlackBoxFunc::Blake3, - BlackBoxOp::Keccakf1600 { .. } => BrilligBlackBoxFunc::Keccakf1600, - BlackBoxOp::EcdsaSecp256k1 { .. } => BrilligBlackBoxFunc::EcdsaSecp256k1, - BlackBoxOp::EcdsaSecp256r1 { .. } => BrilligBlackBoxFunc::EcdsaSecp256r1, - BlackBoxOp::MultiScalarMul { .. } => BrilligBlackBoxFunc::MultiScalarMul, - BlackBoxOp::EmbeddedCurveAdd { .. } => BrilligBlackBoxFunc::EmbeddedCurveAdd, - BlackBoxOp::BigIntAdd { .. } => BrilligBlackBoxFunc::BigIntAdd, - BlackBoxOp::BigIntSub { .. } => BrilligBlackBoxFunc::BigIntSub, - BlackBoxOp::BigIntMul { .. } => BrilligBlackBoxFunc::BigIntMul, - BlackBoxOp::BigIntDiv { .. } => BrilligBlackBoxFunc::BigIntDiv, - BlackBoxOp::BigIntFromLeBytes { .. } => BrilligBlackBoxFunc::BigIntFromLeBytes, - BlackBoxOp::BigIntToLeBytes { .. } => BrilligBlackBoxFunc::BigIntToLeBytes, - BlackBoxOp::Poseidon2Permutation { .. } => BrilligBlackBoxFunc::Poseidon2Permutation, - BlackBoxOp::Sha256Compression { .. } => BrilligBlackBoxFunc::Sha256Compression, - BlackBoxOp::ToRadix { .. } => BrilligBlackBoxFunc::ToRadix, - } - } -} - -impl BrilligBlackBoxFunc { - pub fn name(&self) -> &'static str { - match self { - BrilligBlackBoxFunc::AES128Encrypt => "aes128_encrypt", - BrilligBlackBoxFunc::Blake2s => "blake2s", - BrilligBlackBoxFunc::Blake3 => "blake3", - BrilligBlackBoxFunc::EcdsaSecp256k1 => "ecdsa_secp256k1", - BrilligBlackBoxFunc::MultiScalarMul => "multi_scalar_mul", - BrilligBlackBoxFunc::EmbeddedCurveAdd => "embedded_curve_add", - BrilligBlackBoxFunc::Keccakf1600 => "keccakf1600", - BrilligBlackBoxFunc::EcdsaSecp256r1 => "ecdsa_secp256r1", - BrilligBlackBoxFunc::BigIntAdd => "bigint_add", - BrilligBlackBoxFunc::BigIntSub => "bigint_sub", - BrilligBlackBoxFunc::BigIntMul => "bigint_mul", - BrilligBlackBoxFunc::BigIntDiv => "bigint_div", - BrilligBlackBoxFunc::BigIntFromLeBytes => "bigint_from_le_bytes", - BrilligBlackBoxFunc::BigIntToLeBytes => "bigint_to_le_bytes", - BrilligBlackBoxFunc::Poseidon2Permutation => "poseidon2_permutation", - BrilligBlackBoxFunc::Sha256Compression => "sha256_compression", - BrilligBlackBoxFunc::ToRadix => "to_radix", - } - } -} diff --git a/noir/noir-repo/acvm-repo/brillig/src/lib.rs b/noir/noir-repo/acvm-repo/brillig/src/lib.rs index 9636f6c3960..cf31ff79996 100644 --- a/noir/noir-repo/acvm-repo/brillig/src/lib.rs +++ b/noir/noir-repo/acvm-repo/brillig/src/lib.rs @@ -13,7 +13,7 @@ mod black_box; mod foreign_call; mod opcodes; -pub use black_box::{BlackBoxOp, BrilligBlackBoxFunc}; +pub use black_box::BlackBoxOp; pub use foreign_call::{ForeignCallParam, ForeignCallResult}; pub use opcodes::{ BinaryFieldOp, BinaryIntOp, HeapArray, HeapValueType, HeapVector, MemoryAddress, ValueOrArray, diff --git a/noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs b/noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs index 30edf60d268..9ebbbd3f087 100644 --- a/noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs +++ b/noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs @@ -1,32 +1,15 @@ -use acir::brillig::{BlackBoxOp, BrilligBlackBoxFunc, HeapArray, HeapVector, IntegerBitSize}; +use acir::brillig::{BlackBoxOp, HeapArray, HeapVector, IntegerBitSize}; use acir::{AcirField, BlackBoxFunc}; use acvm_blackbox_solver::{ aes128_encrypt, blake2s, blake3, ecdsa_secp256k1_verify, ecdsa_secp256r1_verify, keccakf1600, sha256_compression, BigIntSolverWithId, BlackBoxFunctionSolver, BlackBoxResolutionError, }; use num_bigint::BigUint; -use num_traits::{ToPrimitive, Zero}; -use thiserror::Error; +use num_traits::Zero; use crate::memory::MemoryValue; use crate::Memory; -#[derive(Clone, PartialEq, Eq, Debug, Error)] -pub(crate) enum BrilligBlackBoxResolutionError { - #[error("failed to solve brillig blackbox function: {0}, reason: {1}")] - Failed(BrilligBlackBoxFunc, String), -} - -impl From for BrilligBlackBoxResolutionError { - fn from(err: BlackBoxResolutionError) -> Self { - match err { - BlackBoxResolutionError::Failed(func, string) => { - BrilligBlackBoxResolutionError::Failed(func.into(), string) - } - } - } -} - fn read_heap_vector<'a, F: AcirField>( memory: &'a Memory, vector: &HeapVector, @@ -62,23 +45,19 @@ pub(crate) fn evaluate_black_box solver: &Solver, memory: &mut Memory, bigint_solver: &mut BrilligBigIntSolver, -) -> Result<(), BrilligBlackBoxResolutionError> { +) -> Result<(), BlackBoxResolutionError> { match op { BlackBoxOp::AES128Encrypt { inputs, iv, key, outputs } => { + let bb_func = black_box_function_from_op(op); + let inputs = to_u8_vec(read_heap_vector(memory, inputs)); let iv: [u8; 16] = to_u8_vec(read_heap_array(memory, iv)).try_into().map_err(|_| { - BrilligBlackBoxResolutionError::Failed( - (*op).into(), - "Invalid iv length".to_string(), - ) + BlackBoxResolutionError::Failed(bb_func, "Invalid iv length".to_string()) })?; let key: [u8; 16] = to_u8_vec(read_heap_array(memory, key)).try_into().map_err(|_| { - BrilligBlackBoxResolutionError::Failed( - (*op).into(), - "Invalid key length".to_string(), - ) + BlackBoxResolutionError::Failed(bb_func, "Invalid key length".to_string()) })?; let ciphertext = aes128_encrypt(&inputs, iv, key)?; @@ -126,26 +105,25 @@ pub(crate) fn evaluate_black_box signature, result: result_address, } => { + let bb_func = black_box_function_from_op(op); + let public_key_x: [u8; 32] = to_u8_vec(read_heap_array(memory, public_key_x)).try_into().map_err(|_| { - BrilligBlackBoxResolutionError::Failed( - (*op).into(), + BlackBoxResolutionError::Failed( + bb_func, "Invalid public key x length".to_string(), ) })?; let public_key_y: [u8; 32] = to_u8_vec(read_heap_array(memory, public_key_y)).try_into().map_err(|_| { - BrilligBlackBoxResolutionError::Failed( - (*op).into(), + BlackBoxResolutionError::Failed( + bb_func, "Invalid public key y length".to_string(), ) })?; let signature: [u8; 64] = to_u8_vec(read_heap_array(memory, signature)).try_into().map_err(|_| { - BrilligBlackBoxResolutionError::Failed( - (*op).into(), - "Invalid signature length".to_string(), - ) + BlackBoxResolutionError::Failed(bb_func, "Invalid signature length".to_string()) })?; let hashed_msg = to_u8_vec(read_heap_vector(memory, hashed_msg)); @@ -306,8 +284,8 @@ pub(crate) fn evaluate_black_box let mut message = [0; 16]; let inputs = read_heap_array(memory, input); if inputs.len() != 16 { - return Err(BrilligBlackBoxResolutionError::Failed( - BrilligBlackBoxFunc::Sha256Compression, + return Err(BlackBoxResolutionError::Failed( + BlackBoxFunc::Sha256Compression, format!("Expected 16 inputs but encountered {}", &inputs.len()), )); } @@ -317,8 +295,8 @@ pub(crate) fn evaluate_black_box let mut state = [0; 8]; let values = read_heap_array(memory, hash_values); if values.len() != 8 { - return Err(BrilligBlackBoxResolutionError::Failed( - BrilligBlackBoxFunc::Sha256Compression, + return Err(BlackBoxResolutionError::Failed( + BlackBoxFunc::Sha256Compression, format!("Expected 8 values but encountered {}", &values.len()), )); } @@ -338,12 +316,7 @@ pub(crate) fn evaluate_black_box .read(*radix) .expect_integer_with_bit_size(IntegerBitSize::U32) .expect("ToRadix opcode's radix bit size does not match expected bit size 32"); - let num_limbs = memory - .read(*num_limbs) - .expect_integer_with_bit_size(IntegerBitSize::U32) - .expect("ToRadix opcode's number of limbs does not match expected bit size 32") - .to_usize() - .unwrap(); // Will not panic as 32 bits must fit into usize type. + let num_limbs = memory.read(*num_limbs).to_usize(); let output_bits = !memory .read(*output_bits) .expect_integer_with_bit_size(IntegerBitSize::U1) @@ -353,27 +326,6 @@ pub(crate) fn evaluate_black_box let mut input = BigUint::from_bytes_be(&input.to_be_bytes()); let radix = BigUint::from_bytes_be(&radix.to_be_bytes()); - if radix < BigUint::from(2u32) || radix > BigUint::from(256u32) { - return Err(BrilligBlackBoxResolutionError::Failed( - BrilligBlackBoxFunc::ToRadix, - format!("Radix out of the valid range [2,256]. Value: {}", radix), - )); - } - - if num_limbs < 1 && input != BigUint::from(0u32) { - return Err(BrilligBlackBoxResolutionError::Failed( - BrilligBlackBoxFunc::ToRadix, - format!("Input value {} is not zero but number of limbs is zero.", input), - )); - } - - if output_bits && radix != BigUint::from(2u32) { - return Err(BrilligBlackBoxResolutionError::Failed( - BrilligBlackBoxFunc::ToRadix, - format!("Radix {} is not equal to 2 and bit mode is activated.", radix), - )); - } - let mut limbs: Vec> = vec![MemoryValue::default(); num_limbs]; for i in (0..num_limbs).rev() { @@ -396,3 +348,25 @@ pub(crate) fn evaluate_black_box } } } + +fn black_box_function_from_op(op: &BlackBoxOp) -> BlackBoxFunc { + match op { + BlackBoxOp::AES128Encrypt { .. } => BlackBoxFunc::AES128Encrypt, + BlackBoxOp::Blake2s { .. } => BlackBoxFunc::Blake2s, + BlackBoxOp::Blake3 { .. } => BlackBoxFunc::Blake3, + BlackBoxOp::Keccakf1600 { .. } => BlackBoxFunc::Keccakf1600, + BlackBoxOp::EcdsaSecp256k1 { .. } => BlackBoxFunc::EcdsaSecp256k1, + BlackBoxOp::EcdsaSecp256r1 { .. } => BlackBoxFunc::EcdsaSecp256r1, + BlackBoxOp::MultiScalarMul { .. } => BlackBoxFunc::MultiScalarMul, + BlackBoxOp::EmbeddedCurveAdd { .. } => BlackBoxFunc::EmbeddedCurveAdd, + BlackBoxOp::BigIntAdd { .. } => BlackBoxFunc::BigIntAdd, + BlackBoxOp::BigIntSub { .. } => BlackBoxFunc::BigIntSub, + BlackBoxOp::BigIntMul { .. } => BlackBoxFunc::BigIntMul, + BlackBoxOp::BigIntDiv { .. } => BlackBoxFunc::BigIntDiv, + BlackBoxOp::BigIntFromLeBytes { .. } => BlackBoxFunc::BigIntFromLeBytes, + BlackBoxOp::BigIntToLeBytes { .. } => BlackBoxFunc::BigIntToLeBytes, + BlackBoxOp::Poseidon2Permutation { .. } => BlackBoxFunc::Poseidon2Permutation, + BlackBoxOp::Sha256Compression { .. } => BlackBoxFunc::Sha256Compression, + BlackBoxOp::ToRadix { .. } => unreachable!("ToRadix is not an ACIR BlackBoxFunc"), + } +}