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

feat: some optimisations to circuits #7909

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion noir-projects/aztec-nr/aztec/src/hash.nr
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ pub fn hash_args(args: [Field]) -> Field {
index_inside_current_chunk = 0;
}
}
if index_inside_current_chunk > 0 {
if index_inside_current_chunk != 0 {
chunks_hashes[current_chunk_index] = poseidon2_hash_with_separator(current_chunk_values, GENERATOR_INDEX__FUNCTION_ARGS);
}
poseidon2_hash_with_separator(chunks_hashes, GENERATOR_INDEX__FUNCTION_ARGS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ mod tests {
builder.failed();
}

#[test]
#[test(should_fail_with="Always provide a hint, unless it's empty")]
fn propagates_unverified_requests() {
let mut builder = PrivateKernelResetInputsBuilder::new();

Expand All @@ -305,10 +305,6 @@ mod tests {

// Check that they have been propagated to the next kernel
let result = builder.execute();

assert_eq(result.validation_requests.note_hash_read_requests[0], note_hash_rr);
assert_eq(result.validation_requests.nullifier_read_requests[0], nullifier_rr);
assert_eq(result.validation_requests.scoped_key_validation_requests_and_generators[0], key_validation);
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl<NH_RR_PENDING, NH_RR_SETTLED, NLL_RR_PENDING, NLL_RR_SETTLED, KEY_VALIDATIO
for_rollup: self.validation_requests.for_rollup,
note_hash_read_requests: remaining_note_hash_read_requests.storage,
nullifier_read_requests: remaining_nullifier_read_requests.storage,
scoped_key_validation_requests_and_generators: remaining_key_validation_requests.storage,
scoped_key_validation_requests_and_generators: remaining_key_validation_requests,
split_counter: Option::some(self.validate_split_counter())
}
}
Expand Down Expand Up @@ -74,7 +74,7 @@ impl<NH_RR_PENDING, NH_RR_SETTLED, NLL_RR_PENDING, NLL_RR_SETTLED, KEY_VALIDATIO
)
}

fn validate_keys(self) -> BoundedVec<ScopedKeyValidationRequestAndGenerator, MAX_KEY_VALIDATION_REQUESTS_PER_TX> {
fn validate_keys(self) -> [ScopedKeyValidationRequestAndGenerator; MAX_KEY_VALIDATION_REQUESTS_PER_TX] {
reset_key_validation_requests(
self.validation_requests.scoped_key_validation_requests_and_generators,
self.key_validation_hints
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use dep::types::{
traits::{Empty, is_empty}, abis::{validation_requests::ScopedKeyValidationRequestAndGenerator},
constants::MAX_KEY_VALIDATION_REQUESTS_PER_TX, scalar::Scalar, hash::poseidon2_hash_with_separator,
utils::arrays::filter_array_to_bounded_vec
utils::arrays::{filter_array_to_bounded_vec, shift_array}
};
use std::embedded_curve_ops::fixed_base_scalar_mul as derive_public_key;

Expand Down Expand Up @@ -29,14 +29,15 @@ impl Eq for KeyValidationHint {
pub fn reset_key_validation_requests<N>(
key_validation_requests: [ScopedKeyValidationRequestAndGenerator; MAX_KEY_VALIDATION_REQUESTS_PER_TX],
key_validation_hints: [KeyValidationHint; N]
) -> BoundedVec<ScopedKeyValidationRequestAndGenerator, MAX_KEY_VALIDATION_REQUESTS_PER_TX> {
let mut should_propagate = key_validation_requests.map(|req| !is_empty(req));
) -> [ScopedKeyValidationRequestAndGenerator; MAX_KEY_VALIDATION_REQUESTS_PER_TX] {
for i in 0..N {
let hint = key_validation_hints[i];
// Ensure we always read the left-most elements of this array each time.
// Determining here whether a key validation request will be validated based on a hint is not a vulnerability
// because in the reset circuit we verify that there are no requests remaining to be validated. For this reason
// the circuits cannot be tricked by not providing a hint (the final check would fail).
if !is_empty(hint) {
assert_eq(hint.request_index, i);
let scoped_request = key_validation_requests[hint.request_index];
let contract_address = scoped_request.contract_address;
let request_and_generator = scoped_request.request;
Expand All @@ -59,10 +60,12 @@ pub fn reset_key_validation_requests<N>(
assert(
sk_app.eq(request.sk_app), "Failed to derive matching app secret key from the secret key."
);

should_propagate[hint.request_index] = false;
} else {
assert(is_empty(key_validation_requests[i]), "Always provide a hint, unless it's empty");
}
}

filter_array_to_bounded_vec(key_validation_requests, should_propagate)
// Assuming we always read the left-most elements of the `key_validation_requests` array each time,
// we can just shift it for 0 constraints.
shift_array::<_, _, N>(key_validation_requests)
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,50 @@ unconstrained fn filter_array_to_bounded_vec_unsafe<T, let N: u32>(
vec
}

pub fn filter_array_to_bounded_vec<T, let N: u32>(arr: [T; N], should_propagate: [bool; N]) -> BoundedVec<T, N> where T: Eq {
pub fn filter_array_to_bounded_vec<T, let N: u32>(
arr: [T; N],
should_propagate: [bool; N]
) -> BoundedVec<T, N> where T: Eq + Empty {
let vec_hint = filter_array_to_bounded_vec_unsafe(arr, should_propagate);
let mut verifying_index = 0;

for i in 0..N {
if should_propagate[i] {
assert_eq(arr[i], vec_hint.get(verifying_index));
// We can get_unchecked, because we're accessing the index incrementally,
// and after this loop we're checking we haven't gone out of bounds.
assert_eq(arr[i], vec_hint.get_unchecked(verifying_index));
verifying_index += 1;
}
}

assert_eq(verifying_index, vec_hint.len());

let empty = T::empty();
let mut end: u1 = 0;
for i in 0..N {
// Do this gymnastics with `end`, because it avoids incrementing verifying_index (which costs more gates).
// We seek to use `i` in `get_unchecked(i)` instead of `verifying_index`, to save constraints.
end = end + (i == verifying_index) as u1;
if end as bool {
assert_eq(empty, vec_hint.get_unchecked(i));
}
}

vec_hint
}

// To call this, write `shift_array::<_, _, shift>(arr)`,
// where `shift` is some value or numeric generic for `Shift`; the size of the shift.
fn shift_array<T, let N: u32, let Shift: u32>(arr: [T; N]) -> [T; N] where T: Empty {
let empty = T::empty();
let mut res: [T; N] = [empty; N];
for i in 0..N - Shift {
let j = i + Shift;
res[i] = arr[j];
}
res
}

unconstrained pub fn find_index_hint<T, let N: u32>(array: [T; N], find: T) -> u32 where T: Eq {
let mut index = 0;
for i in 0..array.len() {
Expand Down
Loading