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

refactor: note hashes cleanup + optimization #7132

Merged
merged 4 commits into from
Jun 20, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
messaging::process_l1_to_l2_message,
hash::{hash_args_array, ArgsHasher, compute_unencrypted_log_hash},
keys::constants::{NULLIFIER_INDEX, OUTGOING_INDEX, NUM_KEY_TYPES, sk_generators},
note::{note_interface::NoteInterface, utils::compute_note_hash_for_insertion},
note::note_interface::NoteInterface,
oracle::{
key_validation_request::get_key_validation_request, arguments, returns::pack_returns,
call_private_function::call_private_function_internal, header::get_header_at,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ mod test {
};

use crate::{
note::{note_header::NoteHeader, note_interface::NoteInterface, utils::compute_note_hash_for_consumption},
note::{note_header::NoteHeader, note_interface::NoteInterface},
event::event_interface::EventInterface, oracle::unsafe_rand::unsafe_rand,
context::PrivateContext
};
Expand Down
10 changes: 4 additions & 6 deletions noir-projects/aztec-nr/aztec/src/note/lifecycle.nr
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use dep::protocol_types::grumpkin_point::GrumpkinPoint;
use crate::context::{PrivateContext, PublicContext};
use crate::note::{
note_header::NoteHeader, note_interface::NoteInterface,
utils::{compute_note_hash_for_insertion, compute_note_hash_for_consumption},
note_emission::NoteEmission
utils::{compute_inner_note_hash, compute_note_hash_for_consumption}, note_emission::NoteEmission
};
use crate::oracle::notes::{notify_created_note, notify_nullified_note};

Expand All @@ -18,7 +17,7 @@ pub fn create_note<Note, N, M>(
let header = NoteHeader { contract_address, storage_slot, nonce: 0, note_hash_counter };
// TODO: change this to note.set_header(header) once https://github.com/noir-lang/noir/issues/4095 is fixed
Note::set_header(note, header);
let inner_note_hash = compute_note_hash_for_insertion(*note);
let inner_note_hash = compute_inner_note_hash(*note);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compute_note_hash_for_insertion was just a wrapper around compute_inner_note_hash which was just making the code messier. I am fine with having a different name than inner note hash but if we introduce it should be consistent across protocol circuits and aztec.nr. It was not the case at this point.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for changing the name later


// TODO: Strong typing required because of https://github.com/noir-lang/noir/issues/4088
let serialized_note: [Field; N] = Note::serialize_content(*note);
Expand Down Expand Up @@ -46,9 +45,8 @@ pub fn create_note_hash_from_public<Note, N, M>(
let contract_address = (*context).this_address();
// Public note hashes are transient, but have no side effect counters, so we just need note_hash_counter != 0
let header = NoteHeader { contract_address, storage_slot, nonce: 0, note_hash_counter: 1 };
// TODO: change this to note.set_header(header) once https://github.com/noir-lang/noir/issues/4095 is fixed
Note::set_header(note, header);
let inner_note_hash = compute_note_hash_for_insertion(*note);
note.set_header(header);
let inner_note_hash = compute_inner_note_hash(*note);

context.push_new_note_hash(inner_note_hash);
}
Expand Down
84 changes: 27 additions & 57 deletions noir-projects/aztec-nr/aztec/src/note/utils.nr
Original file line number Diff line number Diff line change
@@ -1,24 +1,14 @@
use crate::{context::PrivateContext, note::{note_header::NoteHeader, note_interface::NoteInterface}};

use dep::protocol_types::{
address::AztecAddress,
constants::{
GENERATOR_INDEX__OUTER_NULLIFIER, GENERATOR_INDEX__UNIQUE_NOTE_HASH,
GENERATOR_INDEX__SILOED_NOTE_HASH, GENERATOR_INDEX__INNER_NOTE_HASH
constants::GENERATOR_INDEX__INNER_NOTE_HASH,
hash::{
pedersen_hash, compute_unique_note_hash, compute_siloed_note_hash as compute_siloed_note_hash,
compute_siloed_nullifier as compute_siloed_nullifier_from_preimage
},
hash::pedersen_hash, utils::arr_copy_slice
utils::arr_copy_slice
};

fn compute_siloed_hash(contract_address: AztecAddress, unique_note_hash: Field) -> Field {
let inputs = [contract_address.to_field(), unique_note_hash];
pedersen_hash(inputs, GENERATOR_INDEX__SILOED_NOTE_HASH)
}

fn compute_unique_hash(nonce: Field, inner_note_hash: Field) -> Field {
let inputs = [nonce, inner_note_hash];
pedersen_hash(inputs, GENERATOR_INDEX__UNIQUE_NOTE_HASH)
}

fn compute_inner_note_hash<Note, N, M>(note: Note) -> Field where Note: NoteInterface<N, M> {
let header = note.get_header();
let note_hash = note.compute_note_content_hash();
Expand All @@ -29,66 +19,55 @@ fn compute_inner_note_hash<Note, N, M>(note: Note) -> Field where Note: NoteInte
)
}

fn compute_unique_note_hash<Note, N, M>(note_with_header: Note) -> Field where Note: NoteInterface<N, M> {
let header = note_with_header.get_header();

let inner_note_hash = compute_inner_note_hash(note_with_header);

compute_unique_hash(header.nonce, inner_note_hash)
}

fn compute_siloed_note_hash<Note, N, M>(note_with_header: Note) -> Field where Note: NoteInterface<N, M> {
let unique_note_hash = compute_note_hash_for_read_request(note_with_header);

let header = note_with_header.get_header();
compute_siloed_hash(header.contract_address, unique_note_hash)
}

pub fn compute_siloed_nullifier<Note, N, M>(
note_with_header: Note,
context: &mut PrivateContext
) -> Field where Note: NoteInterface<N, M> {
let header = note_with_header.get_header();
let (_, inner_nullifier) = note_with_header.compute_note_hash_and_nullifier(context);

let input = [header.contract_address.to_field(), inner_nullifier];
pedersen_hash(input, GENERATOR_INDEX__OUTER_NULLIFIER)
compute_siloed_nullifier_from_preimage(header.contract_address, inner_nullifier)
}

pub fn compute_note_hash_for_insertion<Note, N, M>(note: Note) -> Field where Note: NoteInterface<N, M> {
compute_inner_note_hash(note)
}

pub fn compute_note_hash_for_read_request<Note, N, M>(note: Note) -> Field where Note: NoteInterface<N, M> {
let header = note.get_header();

let inner_note_hash = compute_inner_note_hash(note);

fn compute_note_hash_for_read_request_from_innter_and_nonce(
inner_note_hash: Field,
nonce: Field
) -> Field {
// TODO(#1386): This if-else can be nuked once we have nonces injected from public
if (header.nonce == 0) {
if (nonce == 0) {
// If nonce is zero, that means we are reading a public note.
inner_note_hash
} else {
compute_unique_hash(header.nonce, inner_note_hash)
compute_unique_note_hash(nonce, inner_note_hash)
}
}

pub fn compute_note_hash_for_read_request<Note, N, M>(note: Note) -> Field where Note: NoteInterface<N, M> {
let inner_note_hash = compute_inner_note_hash(note);
let nonce = note.get_header().nonce;

compute_note_hash_for_read_request_from_innter_and_nonce(inner_note_hash, nonce)
}

pub fn compute_note_hash_for_consumption<Note, N, M>(note: Note) -> Field where Note: NoteInterface<N, M> {
let header = note.get_header();
// There are 3 cases for reading a note intended for consumption:
// 1. The note was inserted in this transaction, and is transient.
// 2. The note was inserted in a previous transaction, and was inserted in public
// 3. The note was inserted in a previous transaction, and was inserted in private

let inner_note_hash = compute_inner_note_hash(note);

if (header.note_hash_counter != 0) {
// If a note is transient, we just read the inner_note_hash (kernel will silo by contract address).
compute_inner_note_hash(note)
inner_note_hash
} else {
// If a note is not transient, that means we are reading a settled note (from tree) created in a
// previous TX. So we need the siloed_note_hash which has already been hashed with
// nonce and then contract address. This hash will match the existing leaf in the note hash
// tree, so the kernel can just perform a membership check directly on this hash/leaf.
compute_siloed_note_hash(note)
let unique_note_hash = compute_note_hash_for_read_request_from_innter_and_nonce(inner_note_hash, header.nonce);
compute_siloed_note_hash(header.contract_address, unique_note_hash)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks uglier but it was necessary to be able to compute the inner note hash only once.

// IMPORTANT NOTE ON REDUNDANT SILOING BY CONTRACT ADDRESS: The note hash computed above is
// "siloed" by contract address. When a note hash is computed solely for the purpose of
// nullification, it is not strictly necessary to silo the note hash before computing
Expand All @@ -101,26 +80,17 @@ pub fn compute_note_hash_for_consumption<Note, N, M>(note: Note) -> Field where
}

pub fn compute_note_hash_and_optionally_a_nullifier<T, N, M, S>(
// docs:start:compute_note_hash_and_optionally_a_nullifier_args
deserialize_content: fn([Field; N]) -> T,
note_header: NoteHeader,
compute_nullifier: bool,
serialized_note: [Field; S] // docs:end:compute_note_hash_and_optionally_a_nullifier_args
serialized_note: [Field; S]
) -> [Field; 4] where T: NoteInterface<N, M> {
let mut note = deserialize_content(arr_copy_slice(serialized_note, [0; N], 0));
note.set_header(note_header);

let inner_note_hash = compute_inner_note_hash(note);

// TODO(https://github.com/AztecProtocol/aztec-packages/issues/1386)
// Should always be calling compute_unique_hash() once notes added from public also include nonces.
let unique_note_hash = if note_header.nonce != 0 {
compute_unique_hash(note_header.nonce, inner_note_hash)
} else {
inner_note_hash
};

let siloed_note_hash = compute_siloed_hash(note_header.contract_address, unique_note_hash);
let unique_note_hash = compute_note_hash_for_read_request_from_innter_and_nonce(inner_note_hash, note_header.nonce);
let siloed_note_hash = compute_siloed_note_hash(note_header.contract_address, unique_note_hash);

let inner_nullifier = if compute_nullifier {
let (_, nullifier) = note.compute_note_hash_and_nullifier_without_context();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::hash::hash_args;

use crate::note::{
note_header::NoteHeader, note_interface::NoteInterface,
utils::{compute_note_hash_for_insertion, compute_note_hash_for_consumption}
utils::{compute_inner_note_hash, compute_note_hash_for_consumption}
};
use crate::oracle::notes::notify_created_note;

Expand Down Expand Up @@ -188,7 +188,7 @@ impl TestEnvironment {
let header = NoteHeader { contract_address, storage_slot, nonce: 0, note_hash_counter };
// TODO: change this to note.set_header(header) once https://github.com/noir-lang/noir/issues/4095 is fixed
Note::set_header(note, header);
let inner_note_hash = compute_note_hash_for_insertion(*note);
let inner_note_hash = compute_inner_note_hash(*note);

// TODO: Strong typing required because of https://github.com/noir-lang/noir/issues/4088
let serialized_note: [Field; N] = Note::serialize_content(*note);
Expand Down
21 changes: 7 additions & 14 deletions noir-projects/noir-protocol-circuits/crates/types/src/hash.nr
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,15 @@ pub fn compute_note_hash_nonce(first_nullifier: Field, note_hash_index: u32) ->
)
}

// TODO(benesjan): some of these functions seem to be duplicate to the ones in
// noir-projects/aztec-nr/aztec/src/note/utils.nr NUKE!
fn compute_unique_note_hash(nonce: Field, note_hash: Field) -> Field {
pedersen_hash(
[
nonce,
note_hash
],
GENERATOR_INDEX__UNIQUE_NOTE_HASH
)
pub fn compute_unique_note_hash(nonce: Field, inner_note_hash: Field) -> Field {
let inputs = [nonce, inner_note_hash];
pedersen_hash(inputs, GENERATOR_INDEX__UNIQUE_NOTE_HASH)
}

pub fn compute_siloed_note_hash(address: AztecAddress, unique_note_hash: Field) -> Field {
pub fn compute_siloed_note_hash(app: AztecAddress, unique_note_hash: Field) -> Field {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think app is nicer than address since it's obvious it's address from the type.

pedersen_hash(
[
address.to_field(),
app.to_field(),
unique_note_hash
],
GENERATOR_INDEX__SILOED_NOTE_HASH
Expand All @@ -77,10 +70,10 @@ pub fn silo_note_hash(note_hash: ScopedNoteHash, first_nullifier: Field, index:
}
}

pub fn compute_siloed_nullifier(address: AztecAddress, nullifier: Field) -> Field {
pub fn compute_siloed_nullifier(app: AztecAddress, nullifier: Field) -> Field {
pedersen_hash(
[
address.to_field(),
app.to_field(),
nullifier
],
GENERATOR_INDEX__OUTER_NULLIFIER
Expand Down
Loading