chore: Pedersen commitment in Noir (#5221)

# Description ## Problem\* Related to #4931 ## Summary\* Implements Pedersen commitment in Noir and remove Pedersen Commitment, Pedersen Hash blackboxes. ## Additional Context The PR is in draft until #5217 is merged. ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Tom French <[email protected]> Co-authored-by: TomAFrench <[email protected]>
noir-lang · Jun 19, 2024 · 1a794e3 · 1a794e3
1 parent d2ea8a9
commit 1a794e3
Show file tree

Hide file tree

Showing 24 changed files with 104 additions and 469 deletions.
diff --git a/acvm-repo/acir/src/circuit/black_box_functions.rs b/acvm-repo/acir/src/circuit/black_box_functions.rs
@@ -82,43 +82,10 @@ pub enum BlackBoxFunc {
     ///
     /// [grumpkin]: https://hackmd.io/@aztec-network/ByzgNxBfd#2-Grumpkin---A-curve-on-top-of-BN-254-for-SNARK-efficient-group-operations
     SchnorrVerify,
-
-    /// Calculates a Pedersen commitment to the inputs.
-    ///
-    /// Computes a Pedersen commitment of the inputs using generators of the
-    /// embedded curve
-    /// - input: vector of (witness, 254)
-    /// - output: 2 witnesses representing the x,y coordinates of the resulting
-    ///   Grumpkin point
-    /// - domain separator: a constant public value (a field element) that you
-    ///   can use so that the commitment also depends on the domain separator.
-    ///   Noir uses 0 as domain separator.
-    ///
-    /// The backend should handle proper conversion between the inputs being ACIR
-    /// field elements and the scalar field of the embedded curve. In the case of
-    /// Aztec's Barretenberg, the latter is bigger than the ACIR field so it is
-    /// straightforward. The Pedersen generators are managed by the proving
-    /// system.
-    ///
-    /// The commitment is expected to be additively homomorphic
+    /// Deprecated. To be removed with a sync from aztec-packages
     PedersenCommitment,
-
-    /// Calculates a Pedersen hash to the inputs.
-    ///
-    /// Computes a Pedersen hash of the inputs and their number, using
-    /// generators of the embedded curve
-    /// - input: vector of (witness, 254)
-    /// - output: the x-coordinate of the pedersen commitment of the
-    ///   'prepended input' (see below)
-    /// - domain separator: a constant public value (a field element) that you
-    ///   can use so that the hash also depends on the domain separator. Noir
-    ///   uses 0 as domain separator.
-    ///
-    /// In Barretenberg, PedersenHash is doing the same as PedersenCommitment,
-    /// except that it prepends the inputs with their length. This is expected
-    /// to not be additively homomorphic.
+    /// Deprecated. To be removed with a sync from aztec-packages
     PedersenHash,
-
     /// Verifies a ECDSA signature over the secp256k1 curve.
     /// - inputs:
     ///     - x coordinate of public key as 32 bytes
@@ -242,8 +209,6 @@ impl BlackBoxFunc {
             BlackBoxFunc::SchnorrVerify => "schnorr_verify",
             BlackBoxFunc::Blake2s => "blake2s",
             BlackBoxFunc::Blake3 => "blake3",
-            BlackBoxFunc::PedersenCommitment => "pedersen_commitment",
-            BlackBoxFunc::PedersenHash => "pedersen_hash",
             BlackBoxFunc::EcdsaSecp256k1 => "ecdsa_secp256k1",
             BlackBoxFunc::MultiScalarMul => "multi_scalar_mul",
             BlackBoxFunc::EmbeddedCurveAdd => "embedded_curve_add",
@@ -262,6 +227,8 @@ impl BlackBoxFunc {
             BlackBoxFunc::BigIntToLeBytes => "bigint_to_le_bytes",
             BlackBoxFunc::Poseidon2Permutation => "poseidon2_permutation",
             BlackBoxFunc::Sha256Compression => "sha256_compression",
+            BlackBoxFunc::PedersenCommitment => "deprecated pedersen commitment",
+            BlackBoxFunc::PedersenHash => "deprecated pedersen hash",
         }
     }
 
@@ -272,8 +239,6 @@ impl BlackBoxFunc {
             "schnorr_verify" => Some(BlackBoxFunc::SchnorrVerify),
             "blake2s" => Some(BlackBoxFunc::Blake2s),
             "blake3" => Some(BlackBoxFunc::Blake3),
-            "pedersen_commitment" => Some(BlackBoxFunc::PedersenCommitment),
-            "pedersen_hash" => Some(BlackBoxFunc::PedersenHash),
             "ecdsa_secp256k1" => Some(BlackBoxFunc::EcdsaSecp256k1),
             "ecdsa_secp256r1" => Some(BlackBoxFunc::EcdsaSecp256r1),
             "multi_scalar_mul" => Some(BlackBoxFunc::MultiScalarMul),
@@ -292,6 +257,8 @@ impl BlackBoxFunc {
             "bigint_to_le_bytes" => Some(BlackBoxFunc::BigIntToLeBytes),
             "poseidon2_permutation" => Some(BlackBoxFunc::Poseidon2Permutation),
             "sha256_compression" => Some(BlackBoxFunc::Sha256Compression),
+            "deprecated pedersen commitment" => Some(BlackBoxFunc::PedersenCommitment),
+            "deprecated pedersen hash" => Some(BlackBoxFunc::PedersenHash),
             _ => None,
         }
     }

diff --git a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs
@@ -54,11 +54,13 @@ pub enum BlackBoxFuncCall {
         message: Vec<FunctionInput>,
         output: Witness,
     },
+    /// Deprecated. To be removed with a sync from aztec-packages
     PedersenCommitment {
         inputs: Vec<FunctionInput>,
         domain_separator: u32,
         outputs: (Witness, Witness),
     },
+    /// Deprecated. To be removed with a sync from aztec-packages
     PedersenHash {
         inputs: Vec<FunctionInput>,
         domain_separator: u32,
@@ -189,8 +191,6 @@ impl BlackBoxFuncCall {
             BlackBoxFuncCall::Blake2s { .. } => BlackBoxFunc::Blake2s,
             BlackBoxFuncCall::Blake3 { .. } => BlackBoxFunc::Blake3,
             BlackBoxFuncCall::SchnorrVerify { .. } => BlackBoxFunc::SchnorrVerify,
-            BlackBoxFuncCall::PedersenCommitment { .. } => BlackBoxFunc::PedersenCommitment,
-            BlackBoxFuncCall::PedersenHash { .. } => BlackBoxFunc::PedersenHash,
             BlackBoxFuncCall::EcdsaSecp256k1 { .. } => BlackBoxFunc::EcdsaSecp256k1,
             BlackBoxFuncCall::EcdsaSecp256r1 { .. } => BlackBoxFunc::EcdsaSecp256r1,
             BlackBoxFuncCall::MultiScalarMul { .. } => BlackBoxFunc::MultiScalarMul,
@@ -206,6 +206,8 @@ impl BlackBoxFuncCall {
             BlackBoxFuncCall::BigIntToLeBytes { .. } => BlackBoxFunc::BigIntToLeBytes,
             BlackBoxFuncCall::Poseidon2Permutation { .. } => BlackBoxFunc::Poseidon2Permutation,
             BlackBoxFuncCall::Sha256Compression { .. } => BlackBoxFunc::Sha256Compression,
+            BlackBoxFuncCall::PedersenCommitment { .. } => BlackBoxFunc::PedersenCommitment,
+            BlackBoxFuncCall::PedersenHash { .. } => BlackBoxFunc::PedersenHash,
         }
     }
 
@@ -219,8 +221,6 @@ impl BlackBoxFuncCall {
             | BlackBoxFuncCall::SHA256 { inputs, .. }
             | BlackBoxFuncCall::Blake2s { inputs, .. }
             | BlackBoxFuncCall::Blake3 { inputs, .. }
-            | BlackBoxFuncCall::PedersenCommitment { inputs, .. }
-            | BlackBoxFuncCall::PedersenHash { inputs, .. }
             | BlackBoxFuncCall::BigIntFromLeBytes { inputs, .. }
             | BlackBoxFuncCall::Poseidon2Permutation { inputs, .. } => inputs.to_vec(),
 
@@ -318,6 +318,8 @@ impl BlackBoxFuncCall {
                 inputs.push(*key_hash);
                 inputs
             }
+            BlackBoxFuncCall::PedersenCommitment { .. } => todo!(),
+            BlackBoxFuncCall::PedersenHash { .. } => todo!(),
         }
     }
 
@@ -339,9 +341,7 @@ impl BlackBoxFuncCall {
             | BlackBoxFuncCall::XOR { output, .. }
             | BlackBoxFuncCall::SchnorrVerify { output, .. }
             | BlackBoxFuncCall::EcdsaSecp256k1 { output, .. }
-            | BlackBoxFuncCall::PedersenHash { output, .. }
             | BlackBoxFuncCall::EcdsaSecp256r1 { output, .. } => vec![*output],
-            BlackBoxFuncCall::PedersenCommitment { outputs, .. } => vec![outputs.0, outputs.1],
             BlackBoxFuncCall::MultiScalarMul { outputs, .. }
             | BlackBoxFuncCall::EmbeddedCurveAdd { outputs, .. } => {
                 vec![outputs.0, outputs.1, outputs.2]
@@ -356,6 +356,8 @@ impl BlackBoxFuncCall {
                 vec![]
             }
             BlackBoxFuncCall::BigIntToLeBytes { outputs, .. } => outputs.to_vec(),
+            BlackBoxFuncCall::PedersenCommitment { .. } => todo!(),
+            BlackBoxFuncCall::PedersenHash { .. } => todo!(),
         }
     }
 }
@@ -421,6 +423,14 @@ fn get_outputs_string(outputs: &[Witness]) -> String {
 
 impl std::fmt::Display for BlackBoxFuncCall {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            BlackBoxFuncCall::PedersenCommitment { .. } => {
+                return write!(f, "BLACKBOX::Deprecated")
+            }
+            BlackBoxFuncCall::PedersenHash { .. } => return write!(f, "BLACKBOX::Deprecated"),
+            _ => (),
+        }
+
         let uppercase_name = self.name().to_uppercase();
         write!(f, "BLACKBOX::{uppercase_name} ")?;
         // INPUTS
@@ -440,13 +450,7 @@ impl std::fmt::Display for BlackBoxFuncCall {
 
         write!(f, "]")?;
 
-        // SPECIFIC PARAMETERS
-        match self {
-            BlackBoxFuncCall::PedersenCommitment { domain_separator, .. } => {
-                write!(f, " domain_separator: {domain_separator}")
-            }
-            _ => write!(f, ""),
-        }
+        write!(f, "")
     }
 }
 

diff --git a/acvm-repo/acir/tests/test_program_serialization.rs b/acvm-repo/acir/tests/test_program_serialization.rs
@@ -100,33 +100,6 @@ fn multi_scalar_mul_circuit() {
     assert_eq!(bytes, expected_serialization)
 }
 
-#[test]
-fn pedersen_circuit() {
-    let pedersen = Opcode::BlackBoxFuncCall(BlackBoxFuncCall::PedersenCommitment {
-        inputs: vec![FunctionInput { witness: Witness(1), num_bits: FieldElement::max_num_bits() }],
-        outputs: (Witness(2), Witness(3)),
-        domain_separator: 0,
-    });
-
-    let circuit: Circuit<FieldElement> = Circuit {
-        current_witness_index: 4,
-        opcodes: vec![pedersen],
-        private_parameters: BTreeSet::from([Witness(1)]),
-        return_values: PublicInputs(BTreeSet::from_iter(vec![Witness(2), Witness(3)])),
-        ..Circuit::default()
-    };
-    let program = Program { functions: vec![circuit], unconstrained_functions: vec![] };
-
-    let bytes = Program::serialize_program(&program);
-
-    let expected_serialization: Vec<u8> = vec![
-        31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 74, 73, 10, 0, 0, 4, 180, 29, 252, 255, 193, 66, 40,
-        76, 77, 179, 34, 20, 36, 136, 237, 83, 245, 101, 107, 79, 65, 94, 253, 214, 217, 255, 239,
-        192, 1, 43, 124, 181, 238, 113, 0, 0, 0,
-    ];
-    assert_eq!(bytes, expected_serialization)
-}
-
 #[test]
 fn schnorr_verify_circuit() {
     let public_key_x =

diff --git a/acvm-repo/acvm/src/pwg/blackbox/mod.rs b/acvm-repo/acvm/src/pwg/blackbox/mod.rs
@@ -7,7 +7,7 @@ use acvm_blackbox_solver::{blake2s, blake3, keccak256, keccakf1600, sha256};
 
 use self::{
     aes128::solve_aes128_encryption_opcode, bigint::AcvmBigIntSolver,
-    hash::solve_poseidon2_permutation_opcode, pedersen::pedersen_hash,
+    hash::solve_poseidon2_permutation_opcode,
 };
 
 use super::{insert_value, OpcodeNotSolvable, OpcodeResolutionError};
@@ -18,7 +18,6 @@ pub(crate) mod bigint;
 mod embedded_curve_ops;
 mod hash;
 mod logic;
-mod pedersen;
 mod range;
 mod signature;
 pub(crate) mod utils;
@@ -27,7 +26,6 @@ use embedded_curve_ops::{embedded_curve_add, multi_scalar_mul};
 // Hash functions should eventually be exposed for external consumers.
 use hash::{solve_generic_256_hash_opcode, solve_sha_256_permutation_opcode};
 use logic::{and, xor};
-use pedersen::pedersen;
 pub(crate) use range::solve_range_opcode;
 use signature::{
     ecdsa::{secp256k1_prehashed, secp256r1_prehashed},
@@ -127,12 +125,6 @@ pub(crate) fn solve<F: AcirField>(
             message,
             *output,
         ),
-        BlackBoxFuncCall::PedersenCommitment { inputs, domain_separator, outputs } => {
-            pedersen(backend, initial_witness, inputs, *domain_separator, *outputs)
-        }
-        BlackBoxFuncCall::PedersenHash { inputs, domain_separator, output } => {
-            pedersen_hash(backend, initial_witness, inputs, *domain_separator, *output)
-        }
         BlackBoxFuncCall::EcdsaSecp256k1 {
             public_key_x,
             public_key_y,
@@ -187,5 +179,7 @@ pub(crate) fn solve<F: AcirField>(
         BlackBoxFuncCall::Poseidon2Permutation { inputs, outputs, len } => {
             solve_poseidon2_permutation_opcode(backend, initial_witness, inputs, outputs, *len)
         }
+        BlackBoxFuncCall::PedersenCommitment { .. } => todo!("Deprecated BlackBox"),
+        BlackBoxFuncCall::PedersenHash { .. } => todo!("Deprecated BlackBox"),
     }
 }
diff --git a/acvm-repo/acvm/src/pwg/blackbox/pedersen.rs b/acvm-repo/acvm/src/pwg/blackbox/pedersen.rs
diff --git a/acvm-repo/acvm_js/test/browser/execute_circuit.test.ts b/acvm-repo/acvm_js/test/browser/execute_circuit.test.ts
@@ -75,16 +75,6 @@ it('successfully processes complex brillig foreign call opcodes', async () => {
   expect(solved_witness).to.be.deep.eq(expectedWitnessMap);
 });
 
-it('successfully executes a Pedersen opcode', async function () {
-  const { bytecode, initialWitnessMap, expectedWitnessMap } = await import('../shared/pedersen');
-
-  const solvedWitness: WitnessMap = await executeCircuit(bytecode, initialWitnessMap, () => {
-    throw Error('unexpected oracle');
-  });
-
-  expect(solvedWitness).to.be.deep.eq(expectedWitnessMap);
-});
-
 it('successfully executes a MultiScalarMul opcode', async () => {
   const { bytecode, initialWitnessMap, expectedWitnessMap } = await import('../shared/multi_scalar_mul');
 

diff --git a/acvm-repo/acvm_js/test/node/execute_circuit.test.ts b/acvm-repo/acvm_js/test/node/execute_circuit.test.ts
@@ -76,17 +76,6 @@ it('successfully processes complex brillig foreign call opcodes', async () => {
   expect(solved_witness).to.be.deep.eq(expectedWitnessMap);
 });
 
-it('successfully executes a Pedersen opcode', async function () {
-  this.timeout(10000);
-  const { bytecode, initialWitnessMap, expectedWitnessMap } = await import('../shared/pedersen');
-
-  const solvedWitness: WitnessMap = await executeCircuit(bytecode, initialWitnessMap, () => {
-    throw Error('unexpected oracle');
-  });
-
-  expect(solvedWitness).to.be.deep.eq(expectedWitnessMap);
-});
-
 it('successfully executes a MultiScalarMul opcode', async () => {
   const { bytecode, initialWitnessMap, expectedWitnessMap } = await import('../shared/multi_scalar_mul');
 
@@ -117,25 +106,6 @@ it('successfully executes a MemoryOp opcode', async () => {
   expect(solvedWitness).to.be.deep.eq(expectedWitnessMap);
 });
 
-it('successfully executes 500 pedersen circuits', async function () {
-  this.timeout(100000);
-
-  // Pedersen opcodes used to have a large upfront cost due to generator calculation
-  // so we'd need to pass around the blackbox solver in JS to avoid redoing this work.
-  //
-  // This test now shows that we don't need to do this anymore without a performance regression.
-
-  const { bytecode, initialWitnessMap, expectedWitnessMap } = await import('../shared/pedersen');
-
-  for (let i = 0; i < 500; i++) {
-    const solvedWitness = await executeCircuit(bytecode, initialWitnessMap, () => {
-      throw Error('unexpected oracle');
-    });
-
-    expect(solvedWitness).to.be.deep.eq(expectedWitnessMap);
-  }
-});
-
 /**
  * Below are all the same tests as above but using `executeProgram`
  * TODO: also add a couple tests for executing multiple circuits

diff --git a/acvm-repo/acvm_js/test/shared/pedersen.ts b/acvm-repo/acvm_js/test/shared/pedersen.ts