fix: handle bad note lengths on compute_note_hash_and_nullifier

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

@@ -256,34 +256,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

@@ -50,30 +50,34 @@ pub struct NoteHashAndNullifier {
 ///
 /// 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_function]` called `_compute_note_hash_and_nullifier`, and it looks something like this:
+/// `#[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);
+///         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 = 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, owner, randomness, storage_slot,
+///                     metadata: SettledNoteMetadata::new(note_nonce).into(),
+///                 },
+///             );
 ///
-///         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 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

@@ -47,6 +47,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)
+    }
+}