From 02f1bb9af285a3a7da67df25af406ae6884a69ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= <nicolas.venturo@gmail.com>
Date: Wed, 6 Nov 2024 16:13:21 +0000
Subject: [PATCH] chore: add unsafe comments (#9761)

I added some small safety comments to the oracles in
https://github.com/AztecProtocol/aztec-packages/pull/9566, and moved the
tag computation over to an unconstrained helper: since the tag secret
itself is unconstrained, there is no point in constraining the
computation of the tag itself, and we might as well inject the final
value directly.
---
 aztec/src/encrypted_logs/payload.nr | 13 ++++++-------
 aztec/src/oracle/notes.nr           | 24 +++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/aztec/src/encrypted_logs/payload.nr b/aztec/src/encrypted_logs/payload.nr
index c57a0e3..92c159f 100644
--- a/aztec/src/encrypted_logs/payload.nr
+++ b/aztec/src/encrypted_logs/payload.nr
@@ -14,7 +14,7 @@ use std::{
 use crate::{
     encrypted_logs::header::EncryptedLogHeader,
     keys::point_to_symmetric_key::point_to_symmetric_key,
-    oracle::{notes::{get_app_tagging_secret, increment_app_tagging_secret}, random::random},
+    oracle::{notes::{get_app_tag_bytes, increment_app_tagging_secret}, random::random},
     utils::point::point_to_bytes,
 };
 
@@ -125,12 +125,11 @@ fn compute_encrypted_log<let P: u32, let M: u32>(
     let mut encrypted_bytes = [0; M];
     let mut offset = 0;
 
-    let tagging_secret = unsafe { get_app_tagging_secret(sender, recipient) };
-
-    unsafe { increment_app_tagging_secret(sender, recipient); };
-
-    let tag = tagging_secret.compute_tag();
-    let tag_bytes: [u8; 32] = tag.to_be_bytes();
+    // We assume that the sender wants for the recipient to find the tagged note, and therefore that they will cooperate
+    // and use the correct tag. Usage of a bad tag will result in the recipient not being able to find the note
+    // automatically.
+    let tag_bytes = unsafe { get_app_tag_bytes(sender, recipient) };
+    increment_app_tagging_secret(sender, recipient);
 
     for i in 0..32 {
         encrypted_bytes[offset + i] = tag_bytes[i];
diff --git a/aztec/src/oracle/notes.nr b/aztec/src/oracle/notes.nr
index c13d768..51ef7f3 100644
--- a/aztec/src/oracle/notes.nr
+++ b/aztec/src/oracle/notes.nr
@@ -203,6 +203,13 @@ pub unconstrained fn check_nullifier_exists(inner_nullifier: Field) -> bool {
 #[oracle(checkNullifierExists)]
 unconstrained fn check_nullifier_exists_oracle(_inner_nullifier: Field) -> Field {}
 
+/// Same as `get_app_tagging_secret`, except it returns the derived tag as an array of bytes, ready to be included in a
+/// log.
+pub unconstrained fn get_app_tag_bytes(sender: AztecAddress, recipient: AztecAddress) -> [u8; 32] {
+    let tag = get_app_tagging_secret(sender, recipient).compute_tag();
+    tag.to_be_bytes()
+}
+
 /// Returns the tagging secret for a given sender and recipient pair, siloed for the current contract address.
 /// Includes the last known index used for tagging with this secret.
 /// For this to work, PXE must know the ivpsk_m of the sender.
@@ -221,7 +228,22 @@ unconstrained fn get_app_tagging_secret_oracle(
     _recipient: AztecAddress,
 ) -> [Field; INDEXED_TAGGING_SECRET_LENGTH] {}
 
-pub unconstrained fn increment_app_tagging_secret(sender: AztecAddress, recipient: AztecAddress) {
+/// Notifies the simulator that a tag has been used in a note, and to therefore increment the associated index so that
+/// future notes get a different tag and can be discovered by the recipient.
+/// This change should only be persisted in a non-volatile database if the tagged log is found in an actual block -
+/// otherwise e.g. a reverting transaction can cause the sender to accidentally skip indices and later produce notes
+/// that are not found by the recipient.
+pub fn increment_app_tagging_secret(sender: AztecAddress, recipient: AztecAddress) {
+    // This oracle call returns nothing: we only call it for its side effects. It is therefore always safe to call.
+    unsafe {
+        increment_app_tagging_secret_wrapper(sender, recipient);
+    }
+}
+
+unconstrained fn increment_app_tagging_secret_wrapper(
+    sender: AztecAddress,
+    recipient: AztecAddress,
+) {
     increment_app_tagging_secret_oracle(sender, recipient);
 }