Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Pedersen commitment in Noir #5221

Merged
merged 34 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
669f317
pedersen hash in Noir
guipublic Jun 10, 2024
e7b5583
remove blackbox perdersen commitment and hash
guipublic Jun 11, 2024
adabad0
update cargo.lock
guipublic Jun 11, 2024
79e971b
Merge branch 'master' into gd/issue_4931_1
guipublic Jun 11, 2024
72bf2d0
clippy
guipublic Jun 11, 2024
4e8c863
allow commitment to zero
guipublic Jun 11, 2024
af056a2
Merge branch 'gd/issue_4931_1' into gd/issue_4931_2
guipublic Jun 11, 2024
f73b1de
remove pedersen serialization test
guipublic Jun 11, 2024
f52624a
Merge branch 'master' into gd/issue_4931_1
guipublic Jun 11, 2024
0a43c8c
Merge branch 'master' into gd/issue_4931_1
guipublic Jun 11, 2024
a6cf3b3
remove benchs for pedersen blackboxes
guipublic Jun 11, 2024
948b6ae
Merge branch 'gd/issue_4931_1' into gd/issue_4931_2
guipublic Jun 11, 2024
351d732
fix test case
guipublic Jun 11, 2024
ef85aef
fix unit tests
guipublic Jun 11, 2024
df29b2c
Handle separator for Pedersen
guipublic Jun 12, 2024
0360de2
code review
guipublic Jun 12, 2024
051706f
Code review
guipublic Jun 12, 2024
e9c3628
Merge branch 'gd/issue_4931_1' into gd/issue_4931_2
guipublic Jun 12, 2024
caefdec
code review
guipublic Jun 12, 2024
68c9828
Merge branch 'gd/issue_4931_1' into gd/issue_4931_2
guipublic Jun 12, 2024
86582b0
fix test cases
guipublic Jun 12, 2024
b429ea3
Add back pedersen blackboxes for CI
guipublic Jun 12, 2024
268676b
fix for unit tests
guipublic Jun 12, 2024
9da503e
Merge branch 'master' into gd/issue_4931_1
guipublic Jun 12, 2024
cd46e1a
Merge branch 'gd/issue_4931_1' into gd/issue_4931_2
guipublic Jun 12, 2024
2f53502
Remove pedersen tests using pedersen opcode
guipublic Jun 12, 2024
c8debe4
code review
guipublic Jun 12, 2024
6682f32
Merge branch 'master' into gd/issue_4931_1
guipublic Jun 12, 2024
a852350
Get the generator length from the result array
guipublic Jun 12, 2024
04526d0
Merge branch 'gd/issue_4931_1' into gd/issue_4931_2
guipublic Jun 12, 2024
95bcc4c
fix merge issue
guipublic Jun 12, 2024
2ae23f4
Merge branch 'master' into gd/issue_4931_2
TomAFrench Jun 19, 2024
4cdfd40
chore: avoid test program being immediately optimized away
TomAFrench Jun 19, 2024
aea8c65
chore: fmt
TomAFrench Jun 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 6 additions & 39 deletions acvm-repo/acir/src/circuit/black_box_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#[cfg_attr(test, derive(EnumIter))]
pub enum BlackBoxFunc {
/// Ciphers (encrypts) the provided plaintext using AES128 in CBC mode,
/// padding the input using PKCS#7.

Check warning on line 15 in acvm-repo/acir/src/circuit/black_box_functions.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (PKCS)
/// - inputs: byte array `[u8; N]`
/// - iv: initialization vector `[u8; 16]`
/// - key: user key `[u8; 16]`
Expand Down Expand Up @@ -82,43 +82,10 @@
///
/// [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
Expand Down Expand Up @@ -242,8 +209,6 @@
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",
Expand All @@ -262,6 +227,8 @@
BlackBoxFunc::BigIntToLeBytes => "bigint_to_le_bytes",
BlackBoxFunc::Poseidon2Permutation => "poseidon2_permutation",
BlackBoxFunc::Sha256Compression => "sha256_compression",
BlackBoxFunc::PedersenCommitment => "deprecated pedersen commitment",
BlackBoxFunc::PedersenHash => "deprecated pedersen hash",
}
}

Expand All @@ -272,8 +239,6 @@
"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),
Expand All @@ -292,6 +257,8 @@
"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,
}
}
Expand Down
30 changes: 17 additions & 13 deletions acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
}
}

Expand All @@ -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(),

Expand Down Expand Up @@ -318,6 +318,8 @@ impl BlackBoxFuncCall {
inputs.push(*key_hash);
inputs
}
BlackBoxFuncCall::PedersenCommitment { .. } => todo!(),
BlackBoxFuncCall::PedersenHash { .. } => todo!(),
}
}

Expand All @@ -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]
Expand All @@ -356,6 +356,8 @@ impl BlackBoxFuncCall {
vec![]
}
BlackBoxFuncCall::BigIntToLeBytes { outputs, .. } => outputs.to_vec(),
BlackBoxFuncCall::PedersenCommitment { .. } => todo!(),
BlackBoxFuncCall::PedersenHash { .. } => todo!(),
}
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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, "")
}
}

Expand Down
27 changes: 0 additions & 27 deletions acvm-repo/acir/tests/test_program_serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
12 changes: 3 additions & 9 deletions acvm-repo/acvm/src/pwg/blackbox/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

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};
Expand All @@ -18,7 +18,6 @@
mod embedded_curve_ops;
mod hash;
mod logic;
mod pedersen;
mod range;
mod signature;
pub(crate) mod utils;
Expand All @@ -27,10 +26,9 @@
// 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},

Check warning on line 31 in acvm-repo/acvm/src/pwg/blackbox/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (prehashed)

Check warning on line 31 in acvm-repo/acvm/src/pwg/blackbox/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (prehashed)
schnorr::schnorr_verify,
};

Expand Down Expand Up @@ -127,19 +125,13 @@
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,
signature,
hashed_message: message,
output,
} => secp256k1_prehashed(

Check warning on line 134 in acvm-repo/acvm/src/pwg/blackbox/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (prehashed)
initial_witness,
public_key_x,
public_key_y,
Expand All @@ -153,7 +145,7 @@
signature,
hashed_message: message,
output,
} => secp256r1_prehashed(

Check warning on line 148 in acvm-repo/acvm/src/pwg/blackbox/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (prehashed)
initial_witness,
public_key_x,
public_key_y,
Expand Down Expand Up @@ -187,5 +179,7 @@
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"),
}
}
47 changes: 0 additions & 47 deletions acvm-repo/acvm/src/pwg/blackbox/pedersen.rs

This file was deleted.

10 changes: 0 additions & 10 deletions acvm-repo/acvm_js/test/browser/execute_circuit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
30 changes: 0 additions & 30 deletions acvm-repo/acvm_js/test/node/execute_circuit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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
Expand Down
13 changes: 0 additions & 13 deletions acvm-repo/acvm_js/test/shared/pedersen.ts

This file was deleted.

Loading
Loading