feat!: remove addNote, compute_note_hash_... #12171
Conversation
nventuro
requested review from
benesjan and
iAmMichaelConnor
and removed request for
benesjan and
iAmMichaelConnor
| /// } else if note_type_id == MyOtherNoteType::get_note_type_id() { | ||
| /// ... // Similar to above but calling MyOtherNoteType::unpack_content | ||
| /// } else { | ||
| /// Option::none() // Unknown note type ID | ||
| /// }; | ||
| /// } | ||
| /// ``` | ||
| type ComputeNoteHashAndNullifier<Env> = fn[Env](/* packed_note_content */BoundedVec<Field, MAX_NOTE_PACKED_LEN>, /* contract_address */ AztecAddress, /* nonce */ Field, /* storage_slot */ Field, /* note_type_id */ Field) -> Option<NoteHashAndNullifier>; | ||
| type ComputeNoteHashAndNullifier<Env> = unconstrained fn[Env](/* packed_note_content */BoundedVec<Field, MAX_NOTE_PACKED_LEN>, /* storage_slot */ Field, /* note_type_id */ Field, /* contract_address */ AztecAddress, /* nonce */ Field) -> Option<NoteHashAndNullifier>; |
There was a problem hiding this comment.
I changed this order to one that was more natural - the old one was related to the original order of compute_note_hash....
There was a problem hiding this comment.
What's "natural"?
There was a problem hiding this comment.
Why is storage_slot before contract_address natural, ser? I do not respect this nature
There was a problem hiding this comment.
Because you need the packed content and storage slot to compute the note hash. It later gets siloed by contract address, and later by nonce. If you just compute the inner note hash you never use contract address. Storage slot and note type id are emitted in that order in the logs and passed in that order in the functions - once we extend logs to have things that are not note logs we'll likely change this (since there'll be no storage slot), but this is highly internally consistent right now.
| @@ -141,8 +138,8 @@ comptime fn generate_contract_library_method_compute_note_hash_and_optionally_a_ | |||
|
|
|||
| // Contracts that do define notes produce an if-else chain where `note_type_id` is matched against the | |||
| // `get_note_type_id()` function of each note type that we know of, in order to identify the note type. Once we | |||
| // know it we call `aztec::note::utils::compute_note_hash_and_optionally_a_nullifier` (which is the one that | |||
| // actually does the work) with the correct `unpack()` function. | |||
| // know it we call we correct `unpack` method from the `Packable` trait to obtain the underlying note type, and | |||
There was a problem hiding this comment.
"we call we correct unpack method from the Packable trait to obtain the underlying note type"
There was a problem hiding this comment.
That seems correct?
There was a problem hiding this comment.
"we call we correct" confused me to the point where I blacked out.
There was a problem hiding this comment.
Also got short-circuited, lol
yarn-project/end-to-end/src/e2e_blacklist_token_contract/blacklist_token_contract_test.ts
Show resolved
Hide resolved
yarn-project/end-to-end/src/e2e_blacklist_token_contract/minting.test.ts
Show resolved
Hide resolved
| /// } else if note_type_id == MyOtherNoteType::get_note_type_id() { | ||
| /// ... // Similar to above but calling MyOtherNoteType::unpack_content | ||
| /// } else { | ||
| /// Option::none() // Unknown note type ID | ||
| /// }; | ||
| /// } | ||
| /// ``` | ||
| type ComputeNoteHashAndNullifier<Env> = fn[Env](/* packed_note_content */BoundedVec<Field, MAX_NOTE_PACKED_LEN>, /* contract_address */ AztecAddress, /* nonce */ Field, /* storage_slot */ Field, /* note_type_id */ Field) -> Option<NoteHashAndNullifier>; | ||
| type ComputeNoteHashAndNullifier<Env> = unconstrained fn[Env](/* packed_note_content */BoundedVec<Field, MAX_NOTE_PACKED_LEN>, /* storage_slot */ Field, /* note_type_id */ Field, /* contract_address */ AztecAddress, /* nonce */ Field) -> Option<NoteHashAndNullifier>; |
There was a problem hiding this comment.
Why is storage_slot before contract_address natural, ser? I do not respect this nature
yarn-project/end-to-end/src/e2e_blacklist_token_contract/minting.test.ts
Show resolved
Hide resolved
This removes the
addNotefunction from PXE (addNullifiedNotehad been removed in #11822). With this change, PXE no longer needs to understand how to compute note hashes and perform nonce discovery, which means we can also get rid of all of that code, plus we can delete the mandatorycompute_note_hash_and_optionally_a_nullifiercontract function, plus all of the auxiliary code used to call those.Instead, contracts that wish to deliver notes to their recipients via offchain mechanisms (i.e. not the protocol logs) must create custom unconstrained functions that know how to construct said notes and add them to PXE. For cases such as
TransparentNote, where all of the note contents are public already, this is quite simple:aztec::discovery::process_private_logcan be leveraged to a great degree by mimicking the log encoding aztec-nr uses - see the TokenBlacklist and Test contracts for examples of this. More fine grained control could be achieved by callingaztec::discovery::attempt_note_nonce_discoveryand then thedeliver_noteoracle (which is essentially whatprocess_private_logdoes, sans the decoding).The removal of
compute_note_hash_and_optionally_a_nullifierfreed us from some legacy burdens in having to produce the 4 field array, dealing with optional nullifier computation, etc., which in turn allowed for the contract library method_compute_note_hash_and_optionally_a_nullifierto be streamlined and converted into the newcompute_note_hash_and_nullifier, which matchesaztec::discovery::ComputeNoteHashAndNullifierand hence results in much easier use of the discovery functions.Tagging @critesjosh since
addNotewas quite a basic and old primitive.Closes #11638.