destroy_note cleanup

noir-projects/aztec-nr/aztec/src/context/private_context.nr

@@ -139,6 +139,8 @@ impl PrivateContext {
         self.new_note_hashes.push(NoteHash { value: note_hash, counter: self.next_counter() });
     }
 
+    // Note: This function is called with non-zero note hash only in 1 of 25 cases in aztec-packages repo - consider
+    // creating a separate function with 1 arg for the zero note hash case.
     fn push_new_nullifier(&mut self, nullifier: Field, nullified_note_hash: Field) {
         self.new_nullifiers.push(Nullifier { value: nullifier, note_hash: nullified_note_hash, counter: self.next_counter() });
     }

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

@@ -57,27 +57,24 @@ pub fn destroy_note<Note, N, M>(
     context: &mut PrivateContext,
     note: Note
 ) where Note: NoteInterface<N, M> {
-    let mut consumed_note_hash: Field = 0;
-    // TODO(benesjan): try just directly setting the value to consumed_note_hash and not doing the if-else down
-    // let [note_hash, nullifier] = note.compute_note_hash_and_nullifier(context);
     let result = note.compute_note_hash_and_nullifier(context);
-    let note_hash = result[0];
     let nullifier = result[1];
 
-    // We also need the note hash corresponding to the "nullifier"
-    let header = note.get_header();
-
-    // A non-zero note hash counter implies that we're nullifying a transient note (i.e. one that has not yet been
-    // persisted in the trees and is instead in the pending new note hashes array). In such a case we compute its hash 
-    // to inform the kernel which note we're nullifyng so that it can find it and squash both the note and the 
-    // nullifier. This value is unused when nullifying non-transient notes - in that case the kernel simply persists
-    // the nullifier in the tree.
-    if (header.note_hash_counter != 0) {
-        consumed_note_hash = note_hash;
-    }
+    let note_hash_counter = note.get_header().note_hash_counter;
+    let note_hash = if (note_hash_counter == 0) {
+        // Counter is zero, so we're nullifying a non-transient note and we don't populate the note_hash with real
+        // value (if we did so the `notifyNullifiedNote` oracle would throw).
+        0
+    } else {
+        // A non-zero note hash counter implies that we're nullifying a transient note (i.e. one that has not yet been
+        // persisted in the trees and is instead in the pending new note hashes array). In such a case we populate its
+        // hash with real value to inform the kernel which note we're nullifyng so that it can find it and squash both
+        // the note and the nullifier.
+        result[0]
+    };
 
     let nullifier_counter = context.side_effect_counter;
-    assert(notify_nullified_note(nullifier, consumed_note_hash, nullifier_counter) == 0);
+    assert(notify_nullified_note(nullifier, note_hash, nullifier_counter) == 0);
 
-    context.push_new_nullifier(nullifier, consumed_note_hash)
+    context.push_new_nullifier(nullifier, note_hash)
 }