From f6f8005ef3a9ccc8d937d8ad4ce59e1669ec7906 Mon Sep 17 00:00:00 2001 From: Nicolas Chamo Date: Tue, 10 Mar 2026 15:05:46 -0300 Subject: [PATCH 1/6] fix: handle missing nullifiers in nonce discovery by skipping notes instead of panicking --- .../src/messages/discovery/nonce_discovery.nr | 132 +++++++++++++----- 1 file changed, 98 insertions(+), 34 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr index 1cfe86133bf6..70c6ba2407f2 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr @@ -1,6 +1,6 @@ use crate::messages::{discovery::ComputeNoteHashAndNullifier, logs::note::MAX_NOTE_PACKED_LEN}; -use crate::logging::aztecnr_debug_log_format; +use crate::logging::{aztecnr_debug_log_format, aztecnr_warn_log_format}; use crate::protocol::{ address::AztecAddress, constants::MAX_NOTE_HASHES_PER_TX, @@ -49,10 +49,11 @@ pub(crate) unconstrained fn attempt_note_nonce_discovery( // note hashes array. We therefore know for each note in every transaction what its nonce is. let candidate_nonce = compute_note_hash_nonce(first_nullifier_in_tx, i); - // Given note nonce, note content and metadata, we can compute the note hash and silo it to check if it matches - // the note hash at the array index we're currently processing. TODO(#11157): handle failed - // note_hash_and_nullifier computation - let hashes = compute_note_hash_and_nullifier( + // Given note nonce, note content and metadata, we can compute the note hash and silo it to check if it + // matches the note hash at the array index we're currently processing. This computation may fail, in which + // case + // we skip the candidate. + let maybe_hashes = compute_note_hash_and_nullifier( packed_note, owner, storage_slot, @@ -60,34 +61,46 @@ pub(crate) unconstrained fn attempt_note_nonce_discovery( contract_address, randomness, candidate_nonce, - ) - .expect(f"Failed to compute a note hash for note type {note_type_id}"); - - let siloed_note_hash = compute_siloed_note_hash(contract_address, hashes.note_hash); - let unique_note_hash = compute_unique_note_hash(candidate_nonce, siloed_note_hash); - - if unique_note_hash == expected_unique_note_hash { - // Note that while we did check that the note hash is the preimage of the expected unique note hash, we - // perform no validations on the nullifier - we fundamentally cannot, since only the application knows how - // to compute nullifiers. We simply trust it to have provided the correct one: if it hasn't, then PXE may - // fail to realize that a given note has been nullified already, and calls to the application could result - // in invalid transactions (with duplicate nullifiers). This is not a concern because an application - // already has more direct means of making a call to it fail the transaction. - discovered_notes.push( - DiscoveredNoteInfo { - note_nonce: candidate_nonce, - note_hash: hashes.note_hash, - // TODO: The None case will be handled in a followup PR. - // https://linear.app/aztec-labs/issue/F-265/store-external-notes - inner_nullifier: hashes.inner_nullifier.expect( - f"Failed to compute nullifier for note type {note_type_id}", - ), - }, - ); + ); - // We don't exit the loop - it is possible (though rare) for the exact same note content to be present - // multiple times in the same transaction with different nonces. This typically doesn't happen due to notes - // containing random values in order to hide their contents. + if maybe_hashes.is_some() { + let hashes = maybe_hashes.unwrap(); + + let siloed_note_hash = compute_siloed_note_hash(contract_address, hashes.note_hash); + let unique_note_hash = compute_unique_note_hash(candidate_nonce, siloed_note_hash); + + if unique_note_hash == expected_unique_note_hash { + // Note that while we did check that the note hash is the preimage of the expected unique note + // hash, we perform no validations on the nullifier - we fundamentally cannot, since only the + // application knows how to compute nullifiers. We simply trust it to have provided the correct + // one: if it hasn't, then PXE may fail to realize that a given note has been nullified already, + // and calls to the application could result in invalid transactions (with duplicate nullifiers). + // This is not a concern because an application already has more direct means of making a call to + // it fail the transaction. + // TODO(F-265): consider the case for external notes even when nullifier is not computable + if hashes.inner_nullifier.is_some() { + discovered_notes.push( + DiscoveredNoteInfo { + note_nonce: candidate_nonce, + note_hash: hashes.note_hash, + inner_nullifier: hashes.inner_nullifier.unwrap(), + }, + ); + } else { + // The nullifier could not be computed (e.g. the owner's nullifier secret key is not + // available). We skip this note because without a nullifier we cannot track whether it + // has been spent. + aztecnr_warn_log_format!( + "Skipping note discovery: nullifier could not be computed for note type {0} on contract {1}", + )( + [note_type_id, contract_address.to_field()], + ); + } + + // We don't exit the loop - it is possible (though rare) for the exact same note content to be + // present multiple times in the same transaction with different nonces. This typically doesn't + // happen due to notes containing random values in order to hide their contents. + } } }); @@ -152,6 +165,30 @@ mod test { } } + // Like compute_note_hash_and_nullifier but returns None for the inner nullifier. + unconstrained fn compute_note_hash_only( + packed_note: BoundedVec, + owner: AztecAddress, + storage_slot: Field, + note_type_id: Field, + contract_address: AztecAddress, + randomness: Field, + note_nonce: Field, + ) -> Option { + compute_note_hash_and_nullifier( + packed_note, + owner, + storage_slot, + note_type_id, + contract_address, + randomness, + note_nonce, + ) + .map(|hashes: NoteHashAndNullifier| { + NoteHashAndNullifier { note_hash: hashes.note_hash, inner_nullifier: Option::none() } + }) + } + global VALUE: Field = 7; global FIRST_NULLIFIER_IN_TX: Field = 47; global CONTRACT_ADDRESS: AztecAddress = AztecAddress::from_field(13); @@ -179,8 +216,8 @@ mod test { assert_eq(discovered_notes.len(), 0); } - #[test(should_fail_with = "Failed to compute a note hash")] - unconstrained fn failed_hash_computation() { + #[test] + unconstrained fn unknown_note_type_is_skipped() { let unique_note_hashes_in_tx = BoundedVec::from_array([random()]); let packed_note = BoundedVec::new(); let note_type_id = 0; // This note type id is unknown to compute_note_hash_and_nullifier @@ -262,6 +299,33 @@ mod test { assert_eq(discovered_note.inner_nullifier, note_and_data.inner_nullifier); } + #[test] + unconstrained fn note_without_nullifier_is_skipped() { + let note_index_in_tx = 2; + let note_and_data = construct_note(VALUE, note_index_in_tx); + + let mut unique_note_hashes_in_tx = BoundedVec::from_array([ + random(), random(), random(), random(), random(), random(), random(), + ]); + unique_note_hashes_in_tx.set(note_index_in_tx, note_and_data.unique_note_hash); + + // Simulates a note without a nullifier: the note hash is computed correctly but the nullifier is None. + let discovered_notes = attempt_note_nonce_discovery( + unique_note_hashes_in_tx, + FIRST_NULLIFIER_IN_TX, + compute_note_hash_only, + CONTRACT_ADDRESS, + OWNER, + STORAGE_SLOT, + RANDOMNESS, + MockNote::get_id(), + BoundedVec::from_array(note_and_data.note.pack()), + ); + + // The note hash matches but the nullifier couldn't be computed, so the note should be skipped. + assert_eq(discovered_notes.len(), 0); + } + #[test] unconstrained fn multiple_notes_same_preimage() { let first_note_index_in_tx = 3; From 669fd2b2f792ea2d4d17dab8676b8ffb00408c9b Mon Sep 17 00:00:00 2001 From: Nicolas Chamo Date: Tue, 10 Mar 2026 15:12:45 -0300 Subject: [PATCH 2/6] chore(aztec-nr): reflow comment to avoid nargo fmt line break --- .../aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr index 70c6ba2407f2..df75014c2926 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr @@ -51,8 +51,7 @@ pub(crate) unconstrained fn attempt_note_nonce_discovery( // Given note nonce, note content and metadata, we can compute the note hash and silo it to check if it // matches the note hash at the array index we're currently processing. This computation may fail, in which - // case - // we skip the candidate. + // case we skip the candidate. let maybe_hashes = compute_note_hash_and_nullifier( packed_note, owner, From 0365281e682ffaf12882dcba142db84942e1e43d Mon Sep 17 00:00:00 2001 From: Nicolas Chamo Date: Thu, 12 Mar 2026 14:59:56 +0000 Subject: [PATCH 3/6] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jan Beneš --- .../aztec/src/messages/discovery/nonce_discovery.nr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr index df75014c2926..021655479946 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr @@ -216,7 +216,7 @@ mod test { } #[test] - unconstrained fn unknown_note_type_is_skipped() { + unconstrained fn unknown_note_type_is_discarded() { let unique_note_hashes_in_tx = BoundedVec::from_array([random()]); let packed_note = BoundedVec::new(); let note_type_id = 0; // This note type id is unknown to compute_note_hash_and_nullifier @@ -299,7 +299,7 @@ mod test { } #[test] - unconstrained fn note_without_nullifier_is_skipped() { + unconstrained fn note_without_nullifier_is_discarded() { let note_index_in_tx = 2; let note_and_data = construct_note(VALUE, note_index_in_tx); @@ -321,7 +321,7 @@ mod test { BoundedVec::from_array(note_and_data.note.pack()), ); - // The note hash matches but the nullifier couldn't be computed, so the note should be skipped. + // The note hash matches but the nullifier couldn't be computed, so the note should be discarded. assert_eq(discovered_notes.len(), 0); } From 4a640f2b3c880f24fa439d0855b91a0c6b8eafd4 Mon Sep 17 00:00:00 2001 From: Nicolas Chamo Date: Thu, 12 Mar 2026 12:15:18 -0300 Subject: [PATCH 4/6] chore(aztec-nr): include nonce in nullifier-skip warning log --- .../aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr index 021655479946..8c0d8297b1e4 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr @@ -90,9 +90,9 @@ pub(crate) unconstrained fn attempt_note_nonce_discovery( // available). We skip this note because without a nullifier we cannot track whether it // has been spent. aztecnr_warn_log_format!( - "Skipping note discovery: nullifier could not be computed for note type {0} on contract {1}", + "Skipping note with nonce {0}: nullifier not computable for note type {1} on contract {2}", )( - [note_type_id, contract_address.to_field()], + [candidate_nonce, note_type_id, contract_address.to_field()], ); } From 0f40a4e016ea9f7c2d857da9447afcc6ebc36b45 Mon Sep 17 00:00:00 2001 From: Nicolas Chamo Date: Fri, 13 Mar 2026 11:10:36 -0300 Subject: [PATCH 5/6] fix(aztec-nr): check nullifier availability only after confirming note hash match --- .../src/messages/discovery/nonce_discovery.nr | 76 +++++++++---------- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr index 202aa8b1b7d6..fde8269ca293 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr @@ -35,6 +35,8 @@ pub(crate) unconstrained fn attempt_note_nonce_discovery( packed_note: BoundedVec, ) -> BoundedVec { let discovered_notes = &mut BoundedVec::new(); + let mut had_hash_failure = false; + let mut had_nullifier_failure = false; aztecnr_debug_log_format!( "Attempting nonce discovery on {0} potential notes on contract {1} for storage slot {2}", @@ -51,7 +53,8 @@ pub(crate) unconstrained fn attempt_note_nonce_discovery( let nonce_for_i = compute_note_hash_nonce(first_nullifier_in_tx, i); // Given note nonce, note content and metadata, we can compute the note hash and silo it to check if - // the resulting unique note matches any in the transaction. This computation may fail, in which case we + // the resulting unique note matches any in the transaction. The contract may not know how to handle this data + // (e.g. unrecognized note_type_id, incorrect packed_note length), in which case it returns None and we // skip the candidate. let maybe_hashes = compute_note_hash_and_nullifier( packed_note, @@ -63,13 +66,13 @@ pub(crate) unconstrained fn attempt_note_nonce_discovery( nonce_for_i, ); - if maybe_hashes.is_some() { + if maybe_hashes.is_none() { + had_hash_failure = true; + } else { let hashes = maybe_hashes.unwrap(); - let siloed_note_hash_for_i = - compute_siloed_note_hash(contract_address, hashes.note_hash); - let unique_note_hash_for_i = - compute_unique_note_hash(nonce_for_i, siloed_note_hash_for_i); + let siloed_note_hash_for_i = compute_siloed_note_hash(contract_address, hashes.note_hash); + let unique_note_hash_for_i = compute_unique_note_hash(nonce_for_i, siloed_note_hash_for_i); let matching_notes = bvec_filter( unique_note_hashes_in_tx, @@ -92,7 +95,6 @@ pub(crate) unconstrained fn attempt_note_nonce_discovery( // to the application could result in invalid transactions (with duplicate nullifiers). This is // not a concern because an application already has more direct means of making a call to it // fail the transaction. - // TODO(F-265): consider the case for external notes even when nullifier is not computable if hashes.inner_nullifier.is_some() { discovered_notes.push( DiscoveredNoteInfo { @@ -101,27 +103,34 @@ pub(crate) unconstrained fn attempt_note_nonce_discovery( inner_nullifier: hashes.inner_nullifier.unwrap(), }, ); + + // We don't exit the loop - it is possible (though rare) for the exact same note content to + // be present multiple times in the same transaction with different nonces. This typically + // doesn't happen due to notes containing random values in order to hide their contents. } else { - // The nullifier could not be computed (e.g. the owner's nullifier secret key is not - // available). We skip this note because without a nullifier we cannot track whether it - // has been spent. - aztecnr_warn_log_format!( - "Skipping note with nonce {0}: nullifier not computable for note type {1} on contract {2}", - )( - [nonce_for_i, note_type_id, contract_address.to_field()], - ); + // TODO(F-265): consider the case for external notes even when nullifier is not computable + had_nullifier_failure = true; } - - // We don't exit the loop - it is possible (though rare) for the exact same note content to be - // present multiple times in the same transaction with different nonces. This typically doesn't - // happen due to notes containing random values in order to hide their contents. } } } - aztecnr_debug_log_format!("Found valid nonces for a total of {0} notes")( - [discovered_notes.len() as Field], - ); + if had_hash_failure { + aztecnr_warn_log_format!( + "Failed to compute note hash during discovery for note type {0} on contract {1}", + )( + [note_type_id, contract_address.to_field()], + ); + } + if had_nullifier_failure { + aztecnr_warn_log_format!( + "Nullifier not computable for note type {0} on contract {1}, discarding note", + )( + [note_type_id, contract_address.to_field()], + ); + } + + aztecnr_debug_log_format!("Found valid nonces for a total of {0} notes")([discovered_notes.len() as Field]); *discovered_notes } @@ -190,8 +199,7 @@ mod test { }, ); - 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(NoteHashAndNullifier { note_hash, inner_nullifier }) } else { @@ -219,10 +227,7 @@ mod test { note_nonce, ) .map(|hashes: NoteHashAndNullifier| { - NoteHashAndNullifier { - note_hash: hashes.note_hash, - inner_nullifier: Option::none(), - } + NoteHashAndNullifier { note_hash: hashes.note_hash, inner_nullifier: Option::none() } }) } @@ -300,10 +305,7 @@ mod test { compute_siloed_note_hash(CONTRACT_ADDRESS, note_hash), ); let inner_nullifier = note - .compute_nullifier_unconstrained( - OWNER, - compute_note_hash_for_nullification(hinted_note), - ) + .compute_nullifier_unconstrained(OWNER, compute_note_hash_for_nullification(hinted_note)) .expect(f"Could not compute nullifier for note owned by {OWNER}"); NoteAndData { note, note_nonce, note_hash, unique_note_hash, inner_nullifier } @@ -382,14 +384,8 @@ mod test { let mut unique_note_hashes_in_tx = BoundedVec::from_array([ random(), random(), random(), random(), random(), random(), random(), ]); - unique_note_hashes_in_tx.set( - first_note_index_in_tx, - first_note_and_data.unique_note_hash, - ); - unique_note_hashes_in_tx.set( - second_note_index_in_tx, - second_note_and_data.unique_note_hash, - ); + unique_note_hashes_in_tx.set(first_note_index_in_tx, first_note_and_data.unique_note_hash); + unique_note_hashes_in_tx.set(second_note_index_in_tx, second_note_and_data.unique_note_hash); let discovered_notes = attempt_note_nonce_discovery( unique_note_hashes_in_tx, From 9503d5a876eae11205eca7178e66ba24e66b670c Mon Sep 17 00:00:00 2001 From: Nicolas Chamo Date: Fri, 13 Mar 2026 11:21:04 -0300 Subject: [PATCH 6/6] chore(aztec-nr): clean up nonce discovery warning messages --- .../src/messages/discovery/nonce_discovery.nr | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr index fde8269ca293..42cc62251a31 100644 --- a/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr +++ b/noir-projects/aztec-nr/aztec/src/messages/discovery/nonce_discovery.nr @@ -115,19 +115,21 @@ pub(crate) unconstrained fn attempt_note_nonce_discovery( } } - if had_hash_failure { - aztecnr_warn_log_format!( - "Failed to compute note hash during discovery for note type {0} on contract {1}", - )( - [note_type_id, contract_address.to_field()], - ); - } - if had_nullifier_failure { - aztecnr_warn_log_format!( - "Nullifier not computable for note type {0} on contract {1}, discarding note", - )( - [note_type_id, contract_address.to_field()], - ); + if discovered_notes.len() == 0 { + if had_hash_failure { + aztecnr_warn_log_format!( + "Could not compute note hash for note type {0} on contract {1}, discarding note", + )( + [note_type_id, contract_address.to_field()], + ); + } + if had_nullifier_failure { + aztecnr_warn_log_format!( + "Could not compute nullifier for note type {0} on contract {1}, discarding note", + )( + [note_type_id, contract_address.to_field()], + ); + } } aztecnr_debug_log_format!("Found valid nonces for a total of {0} notes")([discovered_notes.len() as Field]);