fix: handle bad note lengths on compute_note_hash_and_nullifier (backport #21271)

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

@@ -183,34 +183,50 @@ comptime fn generate_contract_library_method_compute_note_hash_and_nullifier() -
                         // unpack function on it.
                         let expected_len = <$typ as $crate::protocol::traits::Packable>::N;
                         let actual_len = packed_note.len();
-                        assert(
-                            actual_len == expected_len,
-                            f"Expected packed note of length {expected_len} but got {actual_len} for note type id {note_type_id}"
-                        );
-
-                        let note = $unpack(aztec::utils::array::subarray(packed_note.storage(), 0));
-
-                        let note_hash = $compute_note_hash(note, owner, storage_slot, randomness);
-
-                        // The message discovery process finds settled notes, that is, notes that were created in prior transactions and are therefore already part of the note hash tree. We therefore compute the nullification note hash by treating the note as a settled note with the provided note nonce.
-                        let note_hash_for_nullification = aztec::note::utils::compute_note_hash_for_nullification(
-                            aztec::note::HintedNote{
+                        if actual_len != expected_len {
+                            aztec::protocol::logging::warn_log_format(
+                                "[aztec-nr] Packed note length mismatch for note type id {2}: expected {0} fields, got {1}. Skipping note.",
+                                [expected_len as Field, actual_len as Field, note_type_id],
+                            );
+                            Option::none()
+                        } else {
+                            let note = $unpack(aztec::utils::array::subarray(packed_note.storage(), 0));
+
+                            let note_hash = $compute_note_hash(note, owner, storage_slot, randomness);
+
+                            // The message discovery process finds settled notes, that is, notes that were created in
+                            // prior transactions and are therefore already part of the note hash tree. We therefore
+                            // compute the nullification note hash by treating the note as a settled note with the
+                            // provided note nonce.
+                            let note_hash_for_nullification =
+                                aztec::note::utils::compute_note_hash_for_nullification(
+                                    aztec::note::HintedNote {
+                                        note,
+                                        contract_address,
+                                        owner,
+                                        randomness,
+                                        storage_slot,
+                                        metadata:
+                                            aztec::note::note_metadata::SettledNoteMetadata::new(
+                                                note_nonce,
+                                            )
+                                            .into(),
+                                    },
+                                );
+
+                            let inner_nullifier = $compute_nullifier_unconstrained(
                                 note,
-                                contract_address,
                                 owner,
-                                randomness,
-                                storage_slot,
-                                metadata: aztec::note::note_metadata::SettledNoteMetadata::new(note_nonce).into()
-                            }
-                        );
-
-                        let inner_nullifier = $compute_nullifier_unconstrained(note, owner, note_hash_for_nullification);
-
-                        Option::some(
-                            aztec::messages::discovery::NoteHashAndNullifier {
-                                note_hash, inner_nullifier
-                            }
-                        )
+                                note_hash_for_nullification,
+                            );
+
+                            Option::some(
+                                aztec::messages::discovery::NoteHashAndNullifier {
+                                    note_hash,
+                                    inner_nullifier,
+                                },
+                            )
+                        }
                     }
                 },
             );

noir-projects/aztec-nr/aztec/src/messages/discovery/mod.nr

@@ -25,36 +25,48 @@ pub struct NoteHashAndNullifier {
     pub inner_nullifier: Option<Field>,
 }
 
-/// A function which takes a note's packed content, address of the emitting contract, note nonce, storage slot and note
-/// type ID and attempts to compute its note hash (not hashed by note nonce nor siloed by address) and inner nullifier
-/// (not siloed by address).
+/// This function takes a note's packed content, storage slot, note type ID, address of the emitting contract,
+/// randomness and note nonce, and attempts to compute its inner note hash (not siloed by address nor uniqued by nonce)
+/// and inner nullifier (not siloed by address).
 ///
-/// This function must be user-provided as its implementation requires knowledge of how note type IDs are allocated in
-/// a contract. The `#[aztec]` macro automatically creates such a contract library method called
-/// `_compute_note_hash_and_nullifier`, which looks something like this:
+/// ## Transient Notes
 ///
-/// ```
+/// This function is meant to always be used on **settled** notes, i.e. those that have been inserted into the trees
+/// and for which the nonce is known. It is never invoked in the context of a transient note, as those are not involved
+/// in message processing.
+///
+/// ## Automatic Implementation
+///
+/// The [`[#aztec]`](crate::macros::aztec::aztec) macro automatically creates a correct implementation of this function
+/// for each contract by inspecting all note types in use and the storage layout. This injected function is a
+/// `#[contract_library_method]` called `_compute_note_hash_and_nullifier`, and it looks something like this:
+///
+/// ```noir
 /// |packed_note, owner, storage_slot, note_type_id, contract_address, randomness, note_nonce| {
 ///     if note_type_id == MyNoteType::get_id() {
-///         assert(packed_note.len() == MY_NOTE_TYPE_SERIALIZATION_LENGTH);
-///
-///         let note = MyNoteType::unpack(aztec::utils::array::subarray(packed_note.storage(), 0));
+///         if packed_note.len() != MY_NOTE_TYPE_SERIALIZATION_LENGTH {
+///             Option::none()
+///         } else {
+///             let note = MyNoteType::unpack(aztec::utils::array::subarray(packed_note.storage(), 0));
 ///
-///         let note_hash = note.compute_note_hash(owner, storage_slot, randomness);
-///         let note_hash_for_nullification = aztec::note::utils::compute_note_hash_for_nullification(
-///             HintedNote{ note, contract_address, metadata: SettledNoteMetadata::new(note_nonce).into() },
-///             storage_slot
-///         );
+///             let note_hash = note.compute_note_hash(owner, storage_slot, randomness);
+///             let note_hash_for_nullification = aztec::note::utils::compute_note_hash_for_nullification(
+///                 HintedNote {
+///                     note, contract_address, owner, randomness, storage_slot,
+///                     metadata: SettledNoteMetadata::new(note_nonce).into(),
+///                 },
+///             );
 ///
-///         let inner_nullifier = note.compute_nullifier_unconstrained(owner, note_hash_for_nullification);
+///             let inner_nullifier = note.compute_nullifier_unconstrained(owner, note_hash_for_nullification);
 ///
-///         Option::some(
-///             aztec::messages::discovery::NoteHashAndNullifier {
-///                 note_hash, inner_nullifier
-///             }
-///         )
+///             Option::some(
+///                 aztec::messages::discovery::NoteHashAndNullifier {
+///                     note_hash, inner_nullifier
+///                 }
+///             )
+///         }
 ///     } else if note_type_id == MyOtherNoteType::get_id() {
-///           ... // Similar to above but calling MyOtherNoteType::unpack_content
+///           ... // Similar to above but calling MyOtherNoteType::unpack
 ///     } else {
 ///         Option::none() // Unknown note type ID
 ///     };

noir-projects/noir-contracts/Nargo.toml

@@ -48,6 +48,7 @@ members = [
     "contracts/test/import_test_contract",
     "contracts/test/invalid_account_contract",
     "contracts/test/no_constructor_contract",
+    "contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract",
     "contracts/test/note_getter_contract",
     "contracts/test/offchain_effect_contract",
     "contracts/test/only_self_contract",

noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/Nargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "note_hash_and_nullifier_contract"
+authors = [""]
+compiler_version = ">=0.25.0"
+type = "contract"
+
+[dependencies]
+aztec = { path = "../../../../../aztec-nr/aztec" }

noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/main.nr

@@ -0,0 +1,37 @@
+pub mod test_note;
+mod test;
+
+use aztec::macros::aztec;
+
+/// A minimal contract used to test the macro-generated `_compute_note_hash_and_nullifier` function.
+#[aztec]
+pub contract NoteHashAndNullifier {
+    use aztec::{
+        messages::{
+            discovery::NoteHashAndNullifier as NoteHashAndNullifierResult,
+            logs::note::MAX_NOTE_PACKED_LEN,
+        },
+        protocol::address::AztecAddress,
+    };
+
+    #[contract_library_method]
+    pub unconstrained fn test_compute_note_hash_and_nullifier(
+        packed_note: BoundedVec<Field, MAX_NOTE_PACKED_LEN>,
+        owner: AztecAddress,
+        storage_slot: Field,
+        note_type_id: Field,
+        contract_address: AztecAddress,
+        randomness: Field,
+        note_nonce: Field,
+    ) -> Option<NoteHashAndNullifierResult> {
+        _compute_note_hash_and_nullifier(
+            packed_note,
+            owner,
+            storage_slot,
+            note_type_id,
+            contract_address,
+            randomness,
+            note_nonce,
+        )
+    }
+}

noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/test.nr

@@ -0,0 +1,65 @@
+use crate::{NoteHashAndNullifier, test_note::{TEST_NOTE_NULLIFIER, TestNote}};
+use aztec::note::note_interface::{NoteHash, NoteType};
+use aztec::protocol::address::AztecAddress;
+
+#[test]
+unconstrained fn returns_none_for_bad_note_length() {
+    // TestNote has Packable N=1, but we provide 2 fields
+    let packed_note = BoundedVec::from_array([42, 99]);
+
+    let result = NoteHashAndNullifier::test_compute_note_hash_and_nullifier(
+        packed_note,
+        AztecAddress::zero(),
+        0,
+        TestNote::get_id(),
+        AztecAddress::zero(),
+        0,
+        0,
+    );
+
+    assert(result.is_none());
+}
+
+#[test]
+unconstrained fn returns_correct_note_hash_and_nullifier() {
+    // TestNote has Packable N=1
+    let packed_note = BoundedVec::from_array([42]);
+
+    let owner = AztecAddress::zero();
+    let storage_slot = 0;
+    let randomness = 0;
+
+    let result = NoteHashAndNullifier::test_compute_note_hash_and_nullifier(
+        packed_note,
+        owner,
+        storage_slot,
+        TestNote::get_id(),
+        AztecAddress::zero(),
+        randomness,
+        1,
+    );
+
+    let note_hash_and_nullifier = result.unwrap();
+    let note = TestNote { value: 42 };
+    let expected_note_hash = note.compute_note_hash(owner, storage_slot, randomness);
+    assert_eq(note_hash_and_nullifier.note_hash, expected_note_hash);
+
+    assert_eq(note_hash_and_nullifier.inner_nullifier.unwrap(), TEST_NOTE_NULLIFIER);
+}
+
+#[test]
+unconstrained fn returns_none_for_empty_packed_note() {
+    let packed_note = BoundedVec::new();
+
+    let result = NoteHashAndNullifier::test_compute_note_hash_and_nullifier(
+        packed_note,
+        AztecAddress::zero(),
+        0,
+        TestNote::get_id(),
+        AztecAddress::zero(),
+        0,
+        0,
+    );
+
+    assert(result.is_none());
+}

noir-projects/noir-contracts/contracts/test/note_hash_and_nullifier/note_hash_and_nullifier_contract/src/test_note.nr

@@ -0,0 +1,48 @@
+use aztec::{
+    context::PrivateContext,
+    macros::notes::custom_note,
+    note::note_interface::NoteHash,
+    protocol::{
+        address::AztecAddress, constants::DOM_SEP__NOTE_HASH, hash::poseidon2_hash_with_separator,
+        traits::Packable,
+    },
+};
+
+#[derive(Eq, Packable)]
+#[custom_note]
+pub struct TestNote {
+    pub value: Field,
+}
+
+pub global TEST_NOTE_NULLIFIER: Field = 2;
+
+impl NoteHash for TestNote {
+    fn compute_note_hash(
+        self,
+        _owner: AztecAddress,
+        storage_slot: Field,
+        randomness: Field,
+    ) -> Field {
+        let inputs = self.pack().concat([storage_slot, randomness]);
+        poseidon2_hash_with_separator(inputs, DOM_SEP__NOTE_HASH)
+    }
+
+    fn compute_nullifier(
+        _self: Self,
+        _context: &mut PrivateContext,
+        _owner: AztecAddress,
+        _note_hash_for_nullification: Field,
+    ) -> Field {
+        // Not used in any meaningful way
+        0
+    }
+
+    unconstrained fn compute_nullifier_unconstrained(
+        _self: Self,
+        _owner: AztecAddress,
+        _note_hash_for_nullification: Field,
+    ) -> Option<Field> {
+        // Returns a hardcoded value so we can verify that `_compute_note_hash_and_nullifier` propagates it correctly.
+        Option::some(TEST_NOTE_NULLIFIER)
+    }
+}