Skip to content

Commit

Permalink
feat(test): Fuzz poseidon hases against an external library (noir-lan…
Browse files Browse the repository at this point in the history
…g/noir#6273)

feat: remove byte decomposition in `compute_decomposition` (noir-lang/noir#6159)
fix: address inactive public key check in `verify_signature_noir` (noir-lang/noir#6270)
feat(test): Fuzz test poseidon2 hash equivalence (noir-lang/noir#6265)
fix!: Integer division is not the inverse of integer multiplication (noir-lang/noir#6243)
feat(perf): Flamegraphs for test program execution benchmarks (noir-lang/noir#6253)
fix: visibility for impl methods (noir-lang/noir#6261)
feat: Add `checked_transmute` (noir-lang/noir#6262)
feat!: kind size checks (noir-lang/noir#6137)
feat: don't crash LSP when there are errors resolving the workspace (noir-lang/noir#6257)
fix: don't warn on unuse global if it has an abi annotation (noir-lang/noir#6258)
fix: don't warn on unused struct that has an abi annotation (noir-lang/noir#6254)
feat: don't suggest private struct fields in LSP (noir-lang/noir#6256)
feat: visibility for struct fields (noir-lang/noir#6221)
fix: handle dfg databus in SSA normalization (noir-lang/noir#6249)
fix: homogeneous input points for EC ADD (noir-lang/noir#6241)
chore: add regression test for #5756 (noir-lang/noir#5770)
feat: allow `unconstrained` after visibility (noir-lang/noir#6246)
feat: optimize `Quoted::as_expr` by parsing just once (noir-lang/noir#6237)
fix(frontend): Do not warn when a nested struct is provided as input to main (noir-lang/noir#6239)
fix!: Change tag attributes to require a ' prefix (noir-lang/noir#6235)
feat: recover from '=' instead of ':' in struct constructor/pattern (noir-lang/noir#6236)
  • Loading branch information
AztecBot committed Oct 10, 2024
2 parents caab224 + 0abe72d commit 3a60573
Show file tree
Hide file tree
Showing 16 changed files with 353 additions and 71 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
70cbeb4322a0b11c1c167ab27bf0408d04fe7b7d
8d8ea8963c5e4e23bd387aa729e09d3a9553a698
28 changes: 20 additions & 8 deletions noir/noir-repo/Cargo.lock

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

14 changes: 12 additions & 2 deletions noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/poseidon2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,13 +544,23 @@ impl<'a> Poseidon2<'a> {
}

/// Performs a poseidon hash with a sponge construction equivalent to the one in poseidon2.nr
pub fn poseidon_hash(inputs: &[FieldElement]) -> Result<FieldElement, BlackBoxResolutionError> {
///
/// The `is_variable_length` parameter is there to so we can produce an equivalent hash with
/// the Barretenberg implementation which distinguishes between variable and fixed length inputs.
/// Set it to true if the input length matches the static size expected by the Noir function.
pub fn poseidon_hash(
inputs: &[FieldElement],
is_variable_length: bool,
) -> Result<FieldElement, BlackBoxResolutionError> {
let two_pow_64 = 18446744073709551616_u128.into();
let iv = FieldElement::from(inputs.len()) * two_pow_64;
let mut sponge = Poseidon2Sponge::new(iv, 3);
for input in inputs.iter() {
sponge.absorb(*input)?;
}
if is_variable_length {
sponge.absorb(FieldElement::from(1u32))?;
}
sponge.squeeze()
}

Expand Down Expand Up @@ -640,7 +650,7 @@ mod test {
FieldElement::from(3u128),
FieldElement::from(4u128),
];
let result = super::poseidon_hash(&fields).expect("should hash successfully");
let result = super::poseidon_hash(&fields, false).expect("should hash successfully");
assert_eq!(
result,
field_from_hex("130bf204a32cac1f0ace56c78b731aa3809f06df2731ebcf6b3464a15788b1b9"),
Expand Down
3 changes: 0 additions & 3 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -849,9 +849,6 @@ impl<'context> Elaborator<'context> {
for (_, typ) in struct_type.borrow().get_fields(generics) {
self.mark_type_as_used(&typ);
}
for (_, typ) in struct_type.borrow().get_fields(generics) {
self.mark_parameter_type_as_used(&typ);
}
}
Type::Alias(alias_type, generics) => {
self.mark_type_as_used(&alias_type.borrow().get_type(generics));
Expand Down
13 changes: 12 additions & 1 deletion noir/noir-repo/compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1761,7 +1761,7 @@ impl Type {
Err(UnificationError)
}
} else if let InfixExpr(lhs, op, rhs) = other {
if let Some(inverse) = op.inverse() {
if let Some(inverse) = op.approx_inverse() {
// Handle cases like `4 = a + b` by trying to solve to `a = 4 - b`
let new_type = InfixExpr(
Box::new(Constant(*value, kind.clone())),
Expand Down Expand Up @@ -2569,6 +2569,17 @@ impl BinaryTypeOperator {

/// Return the operator that will "undo" this operation if applied to the rhs
fn inverse(self) -> Option<BinaryTypeOperator> {
match self {
BinaryTypeOperator::Addition => Some(BinaryTypeOperator::Subtraction),
BinaryTypeOperator::Subtraction => Some(BinaryTypeOperator::Addition),
BinaryTypeOperator::Multiplication => None,
BinaryTypeOperator::Division => None,
BinaryTypeOperator::Modulo => None,
}
}

/// Return the operator that will "undo" this operation if applied to the rhs
fn approx_inverse(self) -> Option<BinaryTypeOperator> {
match self {
BinaryTypeOperator::Addition => Some(BinaryTypeOperator::Subtraction),
BinaryTypeOperator::Subtraction => Some(BinaryTypeOperator::Addition),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl Type {
/// Precondition: `lhs & rhs are in canonical form`
///
/// - Simplifies `(N +/- M) -/+ M` to `N`
/// - Simplifies `(N * M) ÷/* M` to `N`
/// - Simplifies `(N * M) ÷ M` to `N`
fn try_simplify_non_constants_in_lhs(
lhs: &Type,
op: BinaryTypeOperator,
Expand All @@ -132,7 +132,10 @@ impl Type {

// Note that this is exact, syntactic equality, not unification.
// `rhs` is expected to already be in canonical form.
if l_op.inverse() != Some(op) || l_rhs.canonicalize() != *rhs {
if l_op.approx_inverse() != Some(op)
|| l_op == BinaryTypeOperator::Division
|| l_rhs.canonicalize() != *rhs
{
return None;
}

Expand Down Expand Up @@ -199,7 +202,8 @@ impl Type {
/// Precondition: `lhs & rhs are in canonical form`
///
/// - Simplifies `(N +/- C1) +/- C2` to `N +/- (C1 +/- C2)` if C1 and C2 are constants.
/// - Simplifies `(N */÷ C1) */÷ C2` to `N */÷ (C1 */÷ C2)` if C1 and C2 are constants.
/// - Simplifies `(N * C1) ÷ C2` to `N * (C1 ÷ C2)` if C1 and C2 are constants which divide
/// without a remainder.
fn try_simplify_partial_constants(
lhs: &Type,
mut op: BinaryTypeOperator,
Expand All @@ -218,12 +222,8 @@ impl Type {
let constant = Type::Constant(result, lhs.infix_kind(rhs));
Some(Type::InfixExpr(l_type, l_op, Box::new(constant)))
}
(Multiplication | Division, Multiplication | Division) => {
// If l_op is a division we want to inverse the rhs operator.
if l_op == Division {
op = op.inverse()?;
}

(Multiplication, Division) => {
// We need to ensure the result divides evenly to preserve integer division semantics
let divides_evenly = !lhs.infix_kind(rhs).is_type_level_field_element()
&& l_const.to_i128().checked_rem(r_const.to_i128()) == Some(0);

Expand All @@ -248,7 +248,7 @@ impl Type {
bindings: &mut TypeBindings,
) -> Result<(), UnificationError> {
if let Type::InfixExpr(lhs_a, op_a, rhs_a) = self {
if let Some(inverse) = op_a.inverse() {
if let Some(inverse) = op_a.approx_inverse() {
let kind = lhs_a.infix_kind(rhs_a);
if let Some(rhs_a_value) = rhs_a.evaluate_to_field_element(&kind) {
let rhs_a = Box::new(Type::Constant(rhs_a_value, kind));
Expand All @@ -264,7 +264,7 @@ impl Type {
}

if let Type::InfixExpr(lhs_b, op_b, rhs_b) = other {
if let Some(inverse) = op_b.inverse() {
if let Some(inverse) = op_b.approx_inverse() {
let kind = lhs_b.infix_kind(rhs_b);
if let Some(rhs_b_value) = rhs_b.evaluate_to_field_element(&kind) {
let rhs_b = Box::new(Type::Constant(rhs_b_value, kind));
Expand Down
31 changes: 31 additions & 0 deletions noir/noir-repo/compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3357,3 +3357,34 @@ fn error_if_attribute_not_in_scope() {
CompilationError::ResolverError(ResolverError::AttributeFunctionNotInScope { .. })
));
}

#[test]
fn arithmetic_generics_rounding_pass() {
let src = r#"
fn main() {
// 3/2*2 = 2
round::<3, 2>([1, 2]);
}
fn round<let N: u32, let M: u32>(_x: [Field; N / M * M]) {}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 0);
}

#[test]
fn arithmetic_generics_rounding_fail() {
let src = r#"
fn main() {
// Do not simplify N/M*M to just N
// This should be 3/2*2 = 2, not 3
round::<3, 2>([1, 2, 3]);
}
fn round<let N: u32, let M: u32>(_x: [Field; N / M * M]) {}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);
}
9 changes: 3 additions & 6 deletions noir/noir-repo/deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ yanked = "warn"

ignore = [
"RUSTSEC-2020-0168", # mach unmaintained
"RUSTSEC-2020-0016" # net2 unmaintained
"RUSTSEC-2020-0016", # net2 unmaintained
]

# This section is considered when running `cargo deny check bans`.
Expand Down Expand Up @@ -58,7 +58,7 @@ allow = [
# bitmaps 2.1.0, im 15.1.0
"MPL-2.0",
# Boost Software License
"BSL-1.0"
"BSL-1.0",
]

# Allow 1 or more licenses on a per-crate basis, so that particular licenses
Expand Down Expand Up @@ -101,7 +101,4 @@ unknown-git = "deny"
#
# crates.io rejects git dependencies so anything depending on these is unpublishable and you'll ruin my day
# when I find out.
allow-git = [
"https://github.com/jfecher/chumsky",
"https://github.com/noir-lang/clap-markdown",
]
allow-git = ["https://github.com/noir-lang/clap-markdown"]
22 changes: 10 additions & 12 deletions noir/noir-repo/noir_stdlib/src/field/bn254.nr
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,18 @@ global PLO: Field = 53438638232309528389504892708671455233;
global PHI: Field = 64323764613183177041862057485226039389;

pub(crate) global TWO_POW_128: Field = 0x100000000000000000000000000000000;
global TWO_POW_64: Field = 0x10000000000000000;

// Decomposes a single field into two 16 byte fields.
fn compute_decomposition(x: Field) -> (Field, Field) {
let x_bytes: [u8; 32] = x.to_le_bytes();

let mut low: Field = 0;
let mut high: Field = 0;

let mut offset = 1;
for i in 0..16 {
low += (x_bytes[i] as Field) * offset;
high += (x_bytes[i + 16] as Field) * offset;
offset *= 256;
}
fn compute_decomposition(mut x: Field) -> (Field, Field) {
// Here's we're taking advantage of truncating 64 bit limbs from the input field
// and then subtracting them from the input such the field division is equivalent to integer division.
let low_lower_64 = (x as u64) as Field;
x = (x - low_lower_64) / TWO_POW_64;
let low_upper_64 = (x as u64) as Field;

let high = (x - low_upper_64) / TWO_POW_64;
let low = low_upper_64 * TWO_POW_64 + low_lower_64;

(low, high)
}
Expand Down
2 changes: 1 addition & 1 deletion noir/noir-repo/noir_stdlib/src/schnorr.nr
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub fn verify_signature_noir<let N: u32>(
if ((sig_s.lo != 0) | (sig_s.hi != 0)) & ((sig_e.lo != 0) | (sig_e.hi != 0)) {
let (r_is_infinite, result) = calculate_signature_challenge(public_key, sig_s, sig_e, message);

is_ok = !r_is_infinite;
is_ok &= !r_is_infinite;
for i in 0..32 {
is_ok &= result[i] == signature[32 + i];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,12 @@ fn test_constant_folding<let N: u32>() {

// N * C1 / C2 = N * (C1 / C2)
let _: W<N * 10 / 2> = W::<N * 5> {};

// This case is invalid due to integer division
// If N does not divide evenly with 10 then we cannot simplify it.
// e.g. 15 / 10 * 2 = 2 versus 15 / 5 = 3
//
// N / C1 * C2 = N / (C1 / C2)
let _: W<N / 10 * 2> = W::<N / 5> {};
// let _: W<N / 10 * 2> = W::<N / 5> {};
}

fn test_non_constant_folding<let N: u32, let M: u32>() {
Expand All @@ -131,7 +134,9 @@ fn test_non_constant_folding<let N: u32, let M: u32>() {

// N * M / M = N
let _: W<N * M / M> = W::<N> {};

// This case is not true due to integer division rounding!
// Consider 5 / 2 * 2 which should equal 4, not 5
//
// N / M * M = N
let _: W<N / M * M> = W::<N> {};
// let _: W<N / M * M> = W::<N> {};
}
3 changes: 3 additions & 0 deletions noir/noir-repo/tooling/nargo_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ tracing-appender = "0.2.3"
tokio-util = { version = "0.7.8", features = ["compat"] }

[dev-dependencies]
ark-bn254.workspace = true
tempfile.workspace = true
dirs.workspace = true
assert_cmd = "2.0.8"
Expand All @@ -85,6 +86,8 @@ sha2.workspace = true
sha3.workspace = true
iai = "0.1.1"
test-binary = "3.0.2"
light-poseidon = "0.2.0"



[[bench]]
Expand Down
13 changes: 13 additions & 0 deletions noir/noir-repo/tooling/nargo_cli/benches/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Benchmarks

To generate flamegraphs for the execution of a specific program, execute the following commands:

```shell
./scripts/benchmark_start.sh
cargo bench -p nargo_cli --bench criterion <test-program-name> -- --profile-time=30
./scripts/benchmark_stop.sh
```

Afterwards the flamegraph is available at `target/criterion/<test-program-name>_execute/profile/flamegraph.svg`

Alternatively, omit `<test-program-name>` to run profiling on all test programs defined in [utils.rs](./utils.rs).
Loading

0 comments on commit 3a60573

Please sign in to comment.