Skip to content

Commit

Permalink
Merge branch 'master' into feat/4824-multiple-acir-calls
Browse files Browse the repository at this point in the history
  • Loading branch information
anaPerezGhiglia committed Jun 21, 2024
2 parents daacdac + 49e1b0c commit e0e529b
Show file tree
Hide file tree
Showing 56 changed files with 825 additions and 863 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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 @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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",
}
}

Expand All @@ -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),
Expand All @@ -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,
}
}
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
38 changes: 36 additions & 2 deletions acvm-repo/acvm/src/compiler/transformers/csat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ fn fits_in_one_identity<F: AcirField>(expr: &Expression<F>, width: usize) -> boo
return true;
}

// We now know that we have a single mul term. We also know that the mul term must match up with two other terms
// We now know that we have a single mul term. We also know that the mul term must match up with at least one of the other terms
// A polynomial whose mul terms are non zero which do not match up with two terms in the fan-in cannot fit into one opcode
// An example of this is: Axy + Bx + Cy + ...
// Notice how the bivariate monomial xy has two univariate monomials with their respective coefficients
Expand Down Expand Up @@ -461,7 +461,25 @@ fn fits_in_one_identity<F: AcirField>(expr: &Expression<F>, width: usize) -> boo
}
}

found_x & found_y
// If the multiplication is a squaring then we must assign the two witnesses to separate wires and so we
// can never get a zero contribution to the width.
let multiplication_is_squaring = mul_term.1 == mul_term.2;

let mul_term_width_contribution = if !multiplication_is_squaring && (found_x & found_y) {
// Both witnesses involved in the multiplication exist elsewhere in the expression.
// They both do not contribute to the width of the expression as this would be double-counting
// due to their appearance in the linear terms.
0
} else if found_x || found_y {
// One of the witnesses involved in the multiplication exists elsewhere in the expression.
// The multiplication then only contributes 1 new witness to the width.
1
} else {
// Worst case scenario, the multiplication is using completely unique witnesses so has a contribution of 2.
2
};

mul_term_width_contribution + expr.linear_combinations.len() <= width
}

#[cfg(test)]
Expand Down Expand Up @@ -573,4 +591,20 @@ mod tests {
let contains_b = got_optimized_opcode_a.linear_combinations.iter().any(|(_, w)| *w == b);
assert!(contains_b);
}

#[test]
fn recognize_expr_with_single_shared_witness_which_fits_in_single_identity() {
// Regression test for an expression which Zac found which should have been preserved but
// was being split into two expressions.
let expr = Expression {
mul_terms: vec![(-FieldElement::from(555u128), Witness(8), Witness(10))],
linear_combinations: vec![
(FieldElement::one(), Witness(10)),
(FieldElement::one(), Witness(11)),
(-FieldElement::one(), Witness(13)),
],
q_c: FieldElement::zero(),
};
assert!(fits_in_one_identity(&expr, 4));
}
}
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 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};
Expand All @@ -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;
Expand All @@ -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},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"),
}
}
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
Loading

0 comments on commit e0e529b

Please sign in to comment.