Skip to content

feat!: remove addNote, compute_note_hash_...#12171

Merged
nventuro merged 12 commits into
masterfrom
nv/rm-compute-note-hash
Feb 21, 2025
Merged

feat!: remove addNote, compute_note_hash_...#12171
nventuro merged 12 commits into
masterfrom
nv/rm-compute-note-hash

Conversation

@nventuro

@nventuro nventuro commented Feb 20, 2025

Copy link
Copy Markdown
Contributor

This removes the addNote function from PXE (addNullifiedNote had 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 mandatory compute_note_hash_and_optionally_a_nullifier contract 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_log can 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 calling aztec::discovery::attempt_note_nonce_discovery and then the deliver_note oracle (which is essentially what process_private_log does, sans the decoding).

The removal of compute_note_hash_and_optionally_a_nullifier freed 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_nullifier to be streamlined and converted into the new compute_note_hash_and_nullifier, which matches aztec::discovery::ComputeNoteHashAndNullifier and hence results in much easier use of the discovery functions.

Tagging @critesjosh since addNote was quite a basic and old primitive.

Closes #11638.

@nventuro nventuro requested a review from charlielye as a code owner February 20, 2025 20:48
@nventuro nventuro removed the request for review from charlielye February 20, 2025 20:48
@nventuro nventuro requested review from benesjan and iAmMichaelConnor and removed request for benesjan and iAmMichaelConnor February 20, 2025 22:05
Comment thread docs/docs/aztec/concepts/accounts/index.md
/// }
/// ```
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>;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I changed this order to one that was more natural - the old one was related to the original order of compute_note_hash....

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's "natural"?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is storage_slot before contract_address natural, ser? I do not respect this nature

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Comment thread yarn-project/end-to-end/src/e2e_offchain_note_delivery.ts
Comment thread yarn-project/simulator/src/client/private_execution.test.ts
Comment thread yarn-project/simulator/src/client/simulator.test.ts
// `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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

typo?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

which one?

@benesjan benesjan Feb 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"we call we correct unpack method from the Packable trait to obtain the underlying note type"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That seems correct?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"we call we correct" confused me to the point where I blacked out.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also got short-circuited, lol

Comment thread yarn-project/circuits.js/src/hash/hash.ts
Comment thread yarn-project/end-to-end/src/composed/e2e_persistence.test.ts
Comment thread yarn-project/end-to-end/src/composed/e2e_persistence.test.ts
Comment thread yarn-project/end-to-end/src/e2e_offchain_note_delivery.ts

@benesjan benesjan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lovely

/// }
/// ```
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>;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is storage_slot before contract_address natural, ser? I do not respect this nature

Comment thread yarn-project/circuits.js/src/hash/hash.ts
Comment thread yarn-project/end-to-end/src/composed/e2e_persistence.test.ts
Comment thread yarn-project/end-to-end/src/e2e_offchain_note_delivery.ts
@nventuro nventuro enabled auto-merge (squash) February 21, 2025 13:44
@nventuro nventuro merged commit 436def3 into master Feb 21, 2025
@nventuro nventuro deleted the nv/rm-compute-note-hash branch February 21, 2025 15:07
Mautjee added a commit to aztec-scan/chicmoz that referenced this pull request Apr 4, 2025
Mautjee added a commit to aztec-scan/chicmoz that referenced this pull request Apr 4, 2025
* Version update

* changed to stdlib and some patches

* Build errors fix

* fixed incorrect versioning tag

* Build error fix

* Contract Canon type error fix

* api update imports

* invoking node directly

* fixed it

* added comment

* new cannon senario for updating an contract

* update packages

* Api update for version bump

* types package update

* parsing fix block

* remove console log

* api runs without error and Tracking instance update event

* migrate scripts

* Update packages/backend-utils/src/parse-block.ts

Co-authored-by: Filip Harald <Filip.harald@gmail.com>

* not mendetory funcion anymore

AztecProtocol/aztec-packages#12171

* update packed byte code

* update computed address

* remove TODO

* added to do

* moved todo

---------

Co-authored-by: filip <filip.harald@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simplify variants of compute_note_hash_and_nullif

3 participants