feat: add note hash and nullifier helper functions with domain separation

docs/docs-developers/docs/resources/migration_notes.md

@@ -9,6 +9,16 @@ Aztec is in active development. Each version may introduce breaking changes that
 
 ## TBD
 
+### [Aztec.nr] Made `compute_note_hash_for_nullification` unconstrained
+
+This function shouldn't have been constrained in the first place, as constrained computation of `HintedNote` nullifiers is dangerous (constrained computation of nullifiers can be performed only on the `ConfirmedNote` type). If you were calling this from a constrained function, consider using `compute_confirmed_note_hash_for_nullification` instead. Unconstrained usage is safe.
+
+### [Aztec.nr] Changes to standard note hash computation
+
+Note hashes used to be computed with the storage slot being the last value of the preimage, it is now the first. This is to make it easier to ensure all note hashes have proper domain separation.
+
+This change requires no input from your side unless you were testing or relying on hardcoded note hashes.
+
 ### [Aztec.js] `getPublicEvents` now returns an object instead of an array
 
 `getPublicEvents` now returns a `GetPublicEventsResult<T>` object with `events` and `maxLogsHit` fields instead of a plain array. This enables pagination through large result sets using the new `afterLog` filter option.

noir-projects/aztec-nr/aztec/src/macros/notes.nr

@@ -78,8 +78,8 @@ comptime fn generate_note_hash_trait_impl(s: TypeDefinition) -> Quoted {
     quote {
         impl aztec::note::note_interface::NoteHash for $name {
             fn compute_note_hash(self, owner: aztec::protocol::address::AztecAddress, storage_slot: Field, randomness: Field) -> Field {
-                let inputs = aztec::protocol::traits::Packable::pack(self).concat( [aztec::protocol::traits::ToField::to_field(owner), storage_slot, randomness]);
-                aztec::protocol::hash::poseidon2_hash_with_separator(inputs, aztec::protocol::constants::DOM_SEP__NOTE_HASH)
+                let data = aztec::protocol::traits::Packable::pack(self).concat([aztec::protocol::traits::ToField::to_field(owner), randomness]);
+                aztec::note::utils::compute_note_hash(storage_slot, data)
             }
 
             fn compute_nullifier(
@@ -93,10 +93,7 @@ comptime fn generate_note_hash_trait_impl(s: TypeDefinition) -> Quoted {
                 // in the quote to avoid "trait not in scope" compiler warnings.
                 let owner_npk_m_hash = aztec::protocol::traits::Hash::hash(owner_npk_m);
                 let secret = context.request_nhk_app(owner_npk_m_hash);
-                aztec::protocol::hash::poseidon2_hash_with_separator(
-                    [note_hash_for_nullification, secret],
-                    aztec::protocol::constants::DOM_SEP__NOTE_NULLIFIER as Field,
-                )
+                aztec::note::utils::compute_note_nullifier(note_hash_for_nullification, [secret])
             }
 
             unconstrained fn compute_nullifier_unconstrained(
@@ -113,10 +110,7 @@ comptime fn generate_note_hash_trait_impl(s: TypeDefinition) -> Quoted {
                     // in the quote to avoid "trait not in scope" compiler warnings.
                     let owner_npk_m_hash = aztec::protocol::traits::Hash::hash(owner_npk_m);
                     let secret = aztec::keys::getters::get_nhk_app(owner_npk_m_hash);
-                    aztec::protocol::hash::poseidon2_hash_with_separator(
-                        [note_hash_for_nullification, secret],
-                        aztec::protocol::constants::DOM_SEP__NOTE_NULLIFIER as Field,
-                    )
+                    aztec::note::utils::compute_note_nullifier(note_hash_for_nullification, [secret])
                 })
             }
         }

noir-projects/aztec-nr/aztec/src/note/utils.nr

@@ -1,6 +1,40 @@
 use crate::{context::NoteExistenceRequest, note::{ConfirmedNote, HintedNote, note_interface::NoteHash}};
 
-use crate::protocol::hash::{compute_siloed_note_hash, compute_unique_note_hash};
+use crate::protocol::{
+    constants::{DOM_SEP__NOTE_HASH, DOM_SEP__NOTE_NULLIFIER},
+    hash::{compute_siloed_note_hash, compute_unique_note_hash, poseidon2_hash_with_separator},
+};
+
+/// Computes a domain-separated note hash.
+///
+/// Receives the `storage_slot` of the [`crate::state_vars::StateVariable`] that holds the note, plus any arbitrary
+/// note `data`. This typically includes randomness, owner, and domain specific values (e.g. numeric amount, address,
+/// id, etc.).
+///
+/// Usage of this function guarantees that different state variables will never produce colliding note hashes, even if
+/// their underlying notes have different implementations.
+pub fn compute_note_hash<let N: u32>(storage_slot: Field, data: [Field; N]) -> Field {
+    // All state variables have different storage slots, so by placing this at a fixed first position in the preimage
+    // we prevent collisions.
+    poseidon2_hash_with_separator([storage_slot].concat(data), DOM_SEP__NOTE_HASH)
+}
+
+/// Computes a domain-separated note nullifier.
+///
+/// Receives the `note_hash_for_nullification` of the note (usually returned by
+/// [`compute_confirmed_note_hash_for_nullification`]), plus any arbitrary note `data`. This typically includes
+/// secrets, such as the app-siloed nullifier hiding key of the note's owner.
+///
+/// Usage of this function guarantees that different state variables will never produce colliding note nullifiers, even
+/// if their underlying notes have different implementations.
+pub fn compute_note_nullifier<let N: u32>(note_hash_for_nullification: Field, data: [Field; N]) -> Field {
+    // All notes have different note hashes for nullification (i.e. transient or settled), so by placing this at a
+    // fixed first position in the preimage we prevent collisions.
+    poseidon2_hash_with_separator(
+        [note_hash_for_nullification].concat(data),
+        DOM_SEP__NOTE_NULLIFIER,
+    )
+}
 
 /// Returns the [`NoteExistenceRequest`] used to prove a note exists.
 pub fn compute_note_existence_request<Note>(hinted_note: HintedNote<Note>) -> NoteExistenceRequest
@@ -26,21 +60,27 @@ where
     }
 }
 
-/// Returns the note hash that must be used to compute a note's nullifier when calling `NoteHash::compute_nullifier` or
-/// `NoteHash::compute_nullifier_unconstrained`.
-pub fn compute_note_hash_for_nullification<Note>(hinted_note: HintedNote<Note>) -> Field
+/// Unconstrained variant of [`compute_confirmed_note_hash_for_nullification`].
+pub unconstrained fn compute_note_hash_for_nullification<Note>(hinted_note: HintedNote<Note>) -> Field
 where
     Note: NoteHash,
 {
+    // Creating a ConfirmedNote like we do here is typically unsafe, as we've not confirmed existence. We can do it
+    // here because this is an unconstrained function, so the returned value should not make its way to a constrained
+    // function. This lets us reuse the `compute_confirmed_note_hash_for_nullification` implementation.
     compute_confirmed_note_hash_for_nullification(ConfirmedNote::new(
         hinted_note,
         compute_note_existence_request(hinted_note).note_hash(),
     ))
 }
 
-/// Same as `compute_note_hash_for_nullification`, except it takes the note hash used in a read request (i.e. what
-/// `compute_note_existence_request` would return). This is useful in scenarios where that hash has already been
-/// computed to reduce constraints by reusing this value.
+/// Returns the note hash to use when computing its nullifier.
+///
+/// The `note_hash_for_nullification` parameter [`NoteHash::compute_nullifier`] takes depends on the note's stage, e.g.
+/// settled notes use the unique note hash, but pending notes cannot as they have no nonce. This function returns the
+/// correct note hash to use.
+///
+/// Use [`compute_note_hash_for_nullification`] when computing this value in unconstrained functions.
 pub fn compute_confirmed_note_hash_for_nullification<Note>(confirmed_note: ConfirmedNote<Note>) -> Field {
     // There is just one instance in which the note hash for nullification does not match the note hash used for a read
     // request, which is when dealing with pending previous phase notes. These had their existence proven using their

noir-projects/aztec-nr/aztec/src/state_vars/single_use_claim.nr

@@ -1,5 +1,6 @@
 use crate::protocol::{
-    address::AztecAddress, constants::DOM_SEP__NOTE_HASH, hash::poseidon2_hash_with_separator, traits::Hash,
+    address::AztecAddress, constants::DOM_SEP__SINGLE_USE_CLAIM_NULLIFIER, hash::poseidon2_hash_with_separator,
+    traits::Hash,
 };
 
 use crate::{
@@ -62,7 +63,6 @@ mod test;
 ///
 /// Public effects you emit alongside a claim (e.g. a  public function call to update a tally) may still let observers
 /// infer who likely exercised the claim, so consider that when designing flows.
-/// ```
 pub struct SingleUseClaim<Context> {
     context: Context,
     storage_slot: Field,
@@ -82,8 +82,10 @@ impl<Context> SingleUseClaim<Context> {
     /// This function is primarily used internally by functions [`SingleUseClaim::claim`],
     /// [`SingleUseClaim::assert_claimed`] and [`SingleUseClaim::has_claimed`] to coherently write and read state.
     fn compute_nullifier(self, owner_nhk_app: Field) -> Field {
-        // TODO(F-180): make sure we follow the nullifier convention
-        poseidon2_hash_with_separator([owner_nhk_app, self.storage_slot], DOM_SEP__NOTE_HASH)
+        poseidon2_hash_with_separator(
+            [owner_nhk_app, self.storage_slot],
+            DOM_SEP__SINGLE_USE_CLAIM_NULLIFIER,
+        )
     }
 }
 

noir-projects/aztec-nr/aztec/src/test/mocks/mock_note.nr

@@ -5,11 +5,9 @@ use crate::{
     note::{HintedNote, note_interface::{NoteHash, NoteType}, note_metadata::NoteMetadata},
 };
 
-use crate::protocol::{
-    address::AztecAddress,
-    constants::{DOM_SEP__NOTE_HASH, DOM_SEP__NOTE_NULLIFIER},
-    hash::poseidon2_hash_with_separator,
-    traits::{Packable, ToField},
+use crate::{
+    note::utils::{compute_note_hash, compute_note_nullifier},
+    protocol::{address::AztecAddress, traits::{Packable, ToField}},
 };
 
 #[derive(Eq, Packable)]
@@ -26,8 +24,8 @@ impl NoteType for MockNote {
 
 impl NoteHash for MockNote {
     fn compute_note_hash(self, owner: AztecAddress, storage_slot: Field, randomness: Field) -> Field {
-        let input = self.pack().concat([owner.to_field(), storage_slot, randomness]);
-        poseidon2_hash_with_separator(input, DOM_SEP__NOTE_HASH)
+        let data = self.pack().concat([owner.to_field(), randomness]);
+        compute_note_hash(storage_slot, data)
     }
 
     fn compute_nullifier(
@@ -38,10 +36,7 @@ impl NoteHash for MockNote {
     ) -> Field {
         // We don't use any kind of secret here since this is only a mock note and having it here would make tests more
         // cumbersome
-        poseidon2_hash_with_separator(
-            [note_hash_for_nullification],
-            DOM_SEP__NOTE_NULLIFIER as Field,
-        )
+        compute_note_nullifier(note_hash_for_nullification, [])
     }
 
     unconstrained fn compute_nullifier_unconstrained(
@@ -51,12 +46,7 @@ impl NoteHash for MockNote {
     ) -> Option<Field> {
         // We don't use any kind of secret here since this is only a mock note and having it here would make tests more
         // cumbersome
-        Option::some(
-            poseidon2_hash_with_separator(
-                [note_hash_for_nullification],
-                DOM_SEP__NOTE_NULLIFIER as Field,
-            ),
-        )
+        Option::some(compute_note_nullifier(note_hash_for_nullification, []))
     }
 }
 

noir-projects/aztec-nr/uint-note/src/uint_note.nr

@@ -4,14 +4,11 @@ use aztec::{
     keys::getters::{get_nhk_app, get_public_keys, try_get_public_keys},
     macros::notes::custom_note,
     messages::logs::partial_note::compute_partial_note_private_content_log,
-    note::note_interface::{NoteHash, NoteType},
+    note::{note_interface::{NoteHash, NoteType}, utils::compute_note_nullifier},
     oracle::random::random,
     protocol::{
         address::AztecAddress,
-        constants::{
-            DOM_SEP__NOTE_HASH, DOM_SEP__NOTE_NULLIFIER, DOM_SEP__PARTIAL_NOTE_VALIDITY_COMMITMENT,
-            PRIVATE_LOG_SIZE_IN_FIELDS,
-        },
+        constants::{DOM_SEP__NOTE_HASH, DOM_SEP__PARTIAL_NOTE_VALIDITY_COMMITMENT, PRIVATE_LOG_SIZE_IN_FIELDS},
         hash::{compute_siloed_nullifier, poseidon2_hash_with_separator},
         traits::{Deserialize, FromField, Hash, Packable, Serialize, ToField},
     },
@@ -65,10 +62,7 @@ impl NoteHash for UintNote {
         let owner_npk_m = get_public_keys(owner).npk_m;
         let owner_npk_m_hash = owner_npk_m.hash();
         let secret = context.request_nhk_app(owner_npk_m_hash);
-        poseidon2_hash_with_separator(
-            [note_hash_for_nullification, secret],
-            DOM_SEP__NOTE_NULLIFIER,
-        )
+        compute_note_nullifier(note_hash_for_nullification, [secret])
     }
 
     unconstrained fn compute_nullifier_unconstrained(
@@ -80,10 +74,7 @@ impl NoteHash for UintNote {
             let owner_npk_m = public_keys.npk_m;
             let owner_npk_m_hash = owner_npk_m.hash();
             let secret = get_nhk_app(owner_npk_m_hash);
-            poseidon2_hash_with_separator(
-                [note_hash_for_nullification, secret],
-                DOM_SEP__NOTE_NULLIFIER,
-            )
+            compute_note_nullifier(note_hash_for_nullification, [secret])
         })
     }
 }

noir-projects/noir-contracts/contracts/app/claim_contract/src/main.nr

@@ -7,7 +7,10 @@ pub contract Claim {
     // docs:end:history_import
     use aztec::{
         macros::{functions::{external, initializer}, storage::storage},
-        note::{HintedNote, note_interface::NoteHash, utils::compute_note_hash_for_nullification},
+        note::{
+            HintedNote, note_interface::NoteHash,
+            utils::compute_confirmed_note_hash_for_nullification,
+        },
         protocol::address::AztecAddress,
         state_vars::PublicImmutable,
     };
@@ -48,7 +51,7 @@ pub contract Claim {
         // 3) Prove that the note hash exists in the note hash tree
         // docs:start:prove_note_inclusion
         let header = self.context.get_anchor_block_header();
-        let _ = assert_note_existed_by(header, hinted_note);
+        let confirmed_note = assert_note_existed_by(header, hinted_note);
         // docs:end:prove_note_inclusion
 
         // 4) Compute and emit a nullifier which is unique to the note and this contract to ensure the reward can be
@@ -58,7 +61,8 @@ pub contract Claim {
         // the address of a contract it was emitted from.
         // TODO(#7775): manually computing the hash and passing it to compute_nullifier func is not great as note could
         // handle it on its own or we could make assert_note_existed_by return note_hash_for_nullification.
-        let note_hash_for_nullification = compute_note_hash_for_nullification(hinted_note);
+        let note_hash_for_nullification =
+            compute_confirmed_note_hash_for_nullification(confirmed_note);
         let nullifier = hinted_note.note.compute_nullifier(
             self.context,
             hinted_note.owner,

noir-projects/noir-contracts/contracts/app/nft_contract/src/types/nft_note.nr

@@ -3,13 +3,11 @@ use aztec::{
     keys::getters::{get_nhk_app, get_public_keys, try_get_public_keys},
     macros::notes::custom_note,
     messages::logs::partial_note::compute_partial_note_private_content_log,
-    note::note_interface::{NoteHash, NoteType},
+    note::{note_interface::{NoteHash, NoteType}, utils::compute_note_nullifier},
     oracle::random::random,
     protocol::{
         address::AztecAddress,
-        constants::{
-            DOM_SEP__NOTE_HASH, DOM_SEP__NOTE_NULLIFIER, DOM_SEP__PARTIAL_NOTE_VALIDITY_COMMITMENT,
-        },
+        constants::{DOM_SEP__NOTE_HASH, DOM_SEP__PARTIAL_NOTE_VALIDITY_COMMITMENT},
         hash::poseidon2_hash_with_separator,
         traits::{Deserialize, Hash, Packable, Serialize, ToField},
     },
@@ -66,10 +64,7 @@ impl NoteHash for NFTNote {
         let owner_npk_m = get_public_keys(owner).npk_m;
         let owner_npk_m_hash = owner_npk_m.hash();
         let secret = context.request_nhk_app(owner_npk_m_hash);
-        poseidon2_hash_with_separator(
-            [note_hash_for_nullification, secret],
-            DOM_SEP__NOTE_NULLIFIER,
-        )
+        compute_note_nullifier(note_hash_for_nullification, [secret])
     }
 
     unconstrained fn compute_nullifier_unconstrained(
@@ -81,10 +76,7 @@ impl NoteHash for NFTNote {
             let owner_npk_m = public_keys.npk_m;
             let owner_npk_m_hash = owner_npk_m.hash();
             let secret = get_nhk_app(owner_npk_m_hash);
-            poseidon2_hash_with_separator(
-                [note_hash_for_nullification, secret],
-                DOM_SEP__NOTE_NULLIFIER,
-            )
+            compute_note_nullifier(note_hash_for_nullification, [secret])
         })
     }
 }

noir-projects/noir-contracts/contracts/app/orderbook_contract/src/main.nr

@@ -145,9 +145,9 @@ pub contract Orderbook {
 
         // Nullify the order such that it cannot be fulfilled again. We emit a nullifier instead of deleting the order
         // from public storage because we get no refund for resetting public storage to zero and just emitting
-        // a nullifier is cheaper (1 Field in DA instead of multiple Fields for the order). We use the `order_id`
-        // itself as the nullifier because this contract does not work with notes and hence there is no risk of
-        // colliding with a real note nullifier.
+        // a nullifier is cheaper (1 Field in DA instead of multiple Fields for the order).
+        // TODO(F-399): pushing a raw nullifier with no domain separator like we do here is unsafe: we should instead
+        // use something like a singleton `SingleUseClaim`.
         self.context.push_nullifier(order_id);
 
         // Enqueue the fulfillment to finalize both partial notes

noir-projects/noir-contracts/contracts/app/token_blacklist_contract/src/types/transparent_note.nr

@@ -1,13 +1,8 @@
 use aztec::{
     context::PrivateContext,
     macros::notes::custom_note,
-    note::note_interface::NoteHash,
-    protocol::{
-        address::AztecAddress,
-        constants::{DOM_SEP__NOTE_HASH, DOM_SEP__NOTE_NULLIFIER},
-        hash::poseidon2_hash_with_separator,
-        traits::Packable,
-    },
+    note::{note_interface::NoteHash, utils::{compute_note_hash, compute_note_nullifier}},
+    protocol::{address::AztecAddress, traits::Packable},
 };
 use std::mem::zeroed;
 
@@ -29,8 +24,8 @@ impl NoteHash for TransparentNote {
         storage_slot: Field,
         randomness: Field,
     ) -> Field {
-        let inputs = self.pack().concat([storage_slot, randomness]);
-        poseidon2_hash_with_separator(inputs, DOM_SEP__NOTE_HASH)
+        let data = self.pack().concat([randomness]);
+        compute_note_hash(storage_slot, data)
     }
 
     // Computing a nullifier in a transparent note is not guarded by making secret a part of the nullifier preimage (as
@@ -47,10 +42,7 @@ impl NoteHash for TransparentNote {
         _owner: AztecAddress,
         note_hash_for_nullification: Field,
     ) -> Field {
-        poseidon2_hash_with_separator(
-            [note_hash_for_nullification],
-            DOM_SEP__NOTE_NULLIFIER as Field,
-        )
+        compute_note_nullifier(note_hash_for_nullification, [])
     }
 
     unconstrained fn compute_nullifier_unconstrained(

noir-projects/noir-contracts/contracts/protocol/contract_class_registry_contract/src/main.nr

@@ -93,6 +93,7 @@ pub contract ContractClassRegistry {
         // Emit the contract class id as a nullifier:
         // - to demonstrate that this contract class hasn't been published before
         // - to enable apps to read that this contract class has been published.
+        // We use no domain separators because these are the only nullifiers this contract uses.
         context.push_nullifier(contract_class_id.to_field());
 
         // Broadcast class info including public bytecode

noir-projects/noir-contracts/contracts/protocol/contract_instance_registry_contract/src/main.nr

@@ -139,6 +139,7 @@ pub contract ContractInstanceRegistry {
         let address = AztecAddress::compute(public_keys, partial_address);
 
         // Emit address as nullifier: prevents duplicate deployment and proves publication.
+        // We use no domain separators because these are the only nullifiers this contract uses.
         context.push_nullifier(address.to_field());
 
         // Broadcast deployment event. Uses non-standard serialization (2 fields per point,