Custom implementation of NoteInterface::get_note_type_id needs to return the same id as generated by macros #12012

Activity

1. benesjan

   added

   team-fairiesNico's teamNico's team

   C-aztec.nrComponent: Aztec smart contract frameworkComponent: Aztec smart contract framework

   T-bugType: Bug. Something is broken.Type: Bug. Something is broken.

   on Feb 14, 2025
2. benesjan commented on Feb 19, 2025

   

   benesjan

   on Feb 19, 2025

   Last edited by benesjan

   ContributorAuthor

   More actions

   I was thinking more about this and I would say that most developers will not care about implementing the compute_note_hash function nor the nullifier one. They will be implementing their own notes only because they need a data container, not to fiddle with hash functions.

   So I would say that by default we should not bother the dev with implementing any of these. Currently all the notes in aztec-packages use the same compute_nullifier function implementation --> it's copied around literally 10 times.

   So I would say by default all the notes should just look like this:

   #[note]
   pub struct ValueNote {
       value: Field,
       owner: AztecAddress,
   }
   

   But we still want the flexibility and since we cannot partially implement a trait using macros we need to split the interface into 2.

   Currently this is already the case and we have:

   pub trait NoteInterface {
       fn get_note_type_id() -> Field;
       fn compute_note_hash(...) -> Field;
   }
   pub trait NullifiableNote {
       fn compute_nullifier(...) -> Field;
       unconstrained fn compute_nullifier_without_context(...) -> Field;
   }
   

   Instead I propose having the following 2:

   pub trait NoteInterface {
       fn get_note_type_id() -> Field;
   }
   pub trait NoteHashing {
       fn compute_note_hash(...) -> Field;
       fn compute_nullifier(...) -> Field;
       unconstrained fn compute_nullifier_without_context(...) -> Field;
   }
   

   I checked the current trait bounds in our codebase and I realized that this would actually improve them because we are either:

   1. Computing note hashes and nullifiers,
   2. emitting note logs,
   3. doing both.

   For the first case we only need the hash functions but we don't need the note type id. This would result in the trait bounds getting simplified!:

   

   For the second case we don't need the hashing function but we need the note type id (it gets stored in logs) and we need to pack the note so the trait bounds would be unchanged and they would stay NOTE: NoteInterface + Packable<N>.

   For the 3rd case we need all of these and trait bounds would also be unchanged.

   How to update the macros

   I propose having #[note] macro that would auto-implement both NoteInterface and NoteHashing. Then I would have #[partial_note] which would also implement both but with the partial notes functionality. And then I would have #[custom_note] and #[custom_partial_note] and both of these would not implement the NoteHashing interface and the only difference between the 2 would be that #[custom_partial_note] would implement the setup and finalization payloads and #[custom_note] would not.

   Update: Realized that #[custom_partial_note] macro does not make sense as the setup and finalization payloads of partial notes are tightly coupled with the implementation of NoteHashing interface. Hence auto-generating them does not work when NoteHashing is not auto-generatted.

   Summary

   1. This would simplify the code and make ~97% of the smart contract devs not waste time with boilerplate nullifier functions,
   2. ~2.9% of devs will most likely just want to play with nullifier schemes (as Mike's PLUME scheme suggests) and for this ~2.9% the DEVEX will become slightly worse by having to manually implement the compute_note_hash function,
   3. the ~0.1% of devs which will want to implement their own compute_note_hash function will still be able to do so,
   4. this tackles this issue as now we will be able to force the implementation of get_note_type_id function by macros in all the cases while retaining the flexibility for the ~0.1% of devs which want to implement their own custom compute_note_hash functions.
3. benesjan

   closed this as completedin #12106on Feb 26, 2025