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

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

@@ -9,6 +9,10 @@ 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. If you were calling this from a constrained function, consider using `compute_confirmed_note_hash_for_nullification` instead. Unconstrained usage is safe.
+
 ### [Aztec.nr] Removed `get_random_bytes`
 
 The `get_random_bytes` unconstrained function has been removed from `aztec::utils::random`. If you were using it, you can replace it with direct calls to the `random` oracle from `aztec::oracle::random` and convert to bytes yourself.

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,