chore: fixing @safety warnings (#11094)

Fixes #11087

## Note for reviewer
I originally addressed a bunch of other stuff in this PR but as
@nventuro pointed out it became too messy so I separated those changes
into a PR up the stack.

~**Merging currently blocked by** [this nargo fmt
bug](noir-lang/noir#7045

---------

Co-authored-by: Santiago Palladino <[email protected]>

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

@@ -267,6 +267,9 @@ impl PrivateContext {
             // about secrets from other contracts. We therefore silo secret keys, and rely on the private kernel to
             // validate that we siloed secret key corresponds to correct siloing of the master secret key that hashes
             // to `pk_m_hash`.
+
+            /// Safety: Kernels verify that the key validation request is valid and below we verify that a request
+            /// for the correct public key has been received.
             let request = unsafe { get_key_validation_request(pk_m_hash, key_index) };
             assert(request.pk_m.hash() == pk_m_hash);
 
@@ -388,11 +391,11 @@ impl PrivateContext {
         let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call;
         let start_side_effect_counter = self.side_effect_counter;
 
-        // The oracle simulates the private call and returns the value of the side effects counter after execution of
-        // the call (which means that end_side_effect_counter - start_side_effect_counter is the number of side effects
-        // that took place), along with the hash of the return values. We validate these by requesting a private kernel
-        // iteration in which the return values are constrained to hash to `returns_hash` and the side effects counter
-        // to increment from start to end.
+        /// Safety: The oracle simulates the private call and returns the value of the side effects counter after
+        /// execution of the call (which means that end_side_effect_counter - start_side_effect_counter is
+        /// the number of side effects that took place), along with the hash of the return values. We validate these
+        /// by requesting a private kernel iteration in which the return values are constrained to hash
+        /// to `returns_hash` and the side effects counter to increment from start to end.
         let (end_side_effect_counter, returns_hash) = unsafe {
             call_private_function_internal(
                 contract_address,
@@ -491,12 +494,12 @@ impl PrivateContext {
         let counter = self.next_counter();
 
         let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call;
-        // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Fix this.
-        // WARNING: This is insecure and should be temporary!
-        // The oracle hashes the arguments and returns a new args_hash.
-        // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function.
-        // We don't validate or compute it in the circuit because a) it's harder to do with slices, and
-        // b) this is only temporary.
+        /// Safety: TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Fix this.
+        /// WARNING: This is insecure and should be temporary!
+        /// The oracle repacks the arguments and returns a new args_hash.
+        /// new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function.
+        /// We don't validate or compute it in the circuit because a) it's harder to do with slices, and
+        /// b) this is only temporary.
         let args_hash = unsafe {
             enqueue_public_function_call_internal(
                 contract_address,
@@ -547,12 +550,12 @@ impl PrivateContext {
         let counter = self.next_counter();
 
         let mut is_static_call = is_static_call | self.inputs.call_context.is_static_call;
-        // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Fix this.
-        // WARNING: This is insecure and should be temporary!
-        // The oracle hashes the arguments and returns a new args_hash.
-        // new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function.
-        // We don't validate or compute it in the circuit because a) it's harder to do with slices, and
-        // b) this is only temporary.
+        /// Safety: TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Fix this.
+        /// WARNING: This is insecure and should be temporary!
+        /// The oracle repacks the arguments and returns a new args_hash.
+        /// new_args = [selector, ...old_args], so as to make it suitable to call the public dispatch function.
+        /// We don't validate or compute it in the circuit because a) it's harder to do with slices, and
+        /// b) this is only temporary.
         let args_hash = unsafe {
             set_public_teardown_function_call_internal(
                 contract_address,

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

@@ -21,22 +21,22 @@ impl PublicContext {
     where
         T: Serialize<N>,
     {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe { emit_unencrypted_log(Serialize::serialize(log).as_slice()) };
     }
 
     pub fn note_hash_exists(_self: Self, note_hash: Field, leaf_index: Field) -> bool {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe { note_hash_exists(note_hash, leaf_index) } == 1
     }
 
     pub fn l1_to_l2_msg_exists(_self: Self, msg_hash: Field, msg_leaf_index: Field) -> bool {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe { l1_to_l2_msg_exists(msg_hash, msg_leaf_index) } == 1
     }
 
     pub fn nullifier_exists(_self: Self, unsiloed_nullifier: Field, address: AztecAddress) -> bool {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe { nullifier_exists(unsiloed_nullifier, address.to_field()) } == 1
     }
 
@@ -73,7 +73,7 @@ impl PublicContext {
     }
 
     pub fn message_portal(_self: &mut Self, recipient: EthAddress, content: Field) {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe { send_l2_to_l1_msg(recipient, content) };
     }
 
@@ -114,29 +114,29 @@ impl PublicContext {
     }
 
     pub fn push_note_hash(_self: &mut Self, note_hash: Field) {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe { emit_note_hash(note_hash) };
     }
     pub fn push_nullifier(_self: &mut Self, nullifier: Field) {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe { emit_nullifier(nullifier) };
     }
 
     pub fn this_address(_self: Self) -> AztecAddress {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe {
             address()
         }
     }
     pub fn msg_sender(_self: Self) -> AztecAddress {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe {
             sender()
         }
     }
     pub fn selector(_self: Self) -> FunctionSelector {
         // The selector is the first element of the calldata when calling a public function through dispatch.
-        // AVM opcodes are constrained by the AVM itself.
+        /// Safety: AVM opcodes are constrained by the AVM itself
         let raw_selector: [Field; 1] = unsafe { calldata_copy(0, 1) };
         FunctionSelector::from_field(raw_selector[0])
     }
@@ -148,70 +148,70 @@ impl PublicContext {
         self.args_hash.unwrap_unchecked()
     }
     pub fn transaction_fee(_self: Self) -> Field {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe {
             transaction_fee()
         }
     }
 
     pub fn chain_id(_self: Self) -> Field {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe {
             chain_id()
         }
     }
     pub fn version(_self: Self) -> Field {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe {
             version()
         }
     }
     pub fn block_number(_self: Self) -> Field {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe {
             block_number()
         }
     }
     pub fn timestamp(_self: Self) -> u64 {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe {
             timestamp()
         }
     }
     pub fn fee_per_l2_gas(_self: Self) -> Field {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe {
             fee_per_l2_gas()
         }
     }
     pub fn fee_per_da_gas(_self: Self) -> Field {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe {
             fee_per_da_gas()
         }
     }
 
     pub fn l2_gas_left(_self: Self) -> Field {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe {
             l2_gas_left()
         }
     }
     pub fn da_gas_left(_self: Self) -> Field {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe {
             da_gas_left()
         }
     }
     pub fn is_static_call(_self: Self) -> bool {
-        // AVM opcodes are constrained by the AVM itself
+        /// Safety: AVM opcodes are constrained by the AVM itself
         unsafe { is_static_call() } == 1
     }
 
     pub fn raw_storage_read<let N: u32>(_self: Self, storage_slot: Field) -> [Field; N] {
         let mut out = [0; N];
         for i in 0..N {
-            // AVM opcodes are constrained by the AVM itself
+            /// Safety: AVM opcodes are constrained by the AVM itself
             out[i] = unsafe { storage_read(storage_slot + i as Field) };
         }
         out
@@ -226,7 +226,7 @@ impl PublicContext {
 
     pub fn raw_storage_write<let N: u32>(_self: Self, storage_slot: Field, values: [Field; N]) {
         for i in 0..N {
-            // AVM opcodes are constrained by the AVM itself
+            /// Safety: AVM opcodes are constrained by the AVM itself
             unsafe { storage_write(storage_slot + i as Field, values[i]) };
         }
     }

noir-projects/aztec-nr/aztec/src/encrypted_logs/encrypted_event_emission.nr

@@ -55,8 +55,8 @@ where
     Event: EventInterface<N>,
 {
     |e: Event| {
-        // Unconstrained logs have both their content and encryption unconstrained - it could occur that the
-        // recipient is unable to decrypt the payload.
+        /// Safety: Unconstrained logs have both their content and encryption unconstrained - it could occur that the
+        /// recipient is unable to decrypt the payload.
         let encrypted_log =
             unsafe { compute_payload_unconstrained(*context, e, recipient, sender) };
         context.emit_private_log(encrypted_log);

noir-projects/aztec-nr/aztec/src/encrypted_logs/encrypted_note_emission.nr

@@ -73,16 +73,15 @@ where
     Note: NoteInterface<N>,
 {
     |e: NoteEmission<Note>| {
-        //   Unconstrained logs have both their content and encryption unconstrained - it could occur that the
-        // recipient is unable to decrypt the payload.
-        //   Regarding the note hash counter, this is used for squashing. The kernel assumes that a given note can have
-        // more than one log and removes all of the matching ones, so all a malicious sender could do is either: cause
-        // for the log to be deleted when it shouldn't have (which is fine - they can already make the content be
-        // whatever), or cause for the log to not be deleted when it should have (which is also fine - it'll be a log
-        // for a note that doesn't exist).
-        //   It's important here that we do not
-        // return the log from this function to the app, otherwise it could try to do stuff with it and then that might
-        // be wrong.
+        /// Safety:   Unconstrained logs have both their content and encryption unconstrained - it could occur that
+        /// the recipient is unable to decrypt the payload.
+        ///   Regarding the note hash counter, this is used for squashing. The kernel assumes that a given note can
+        /// have more than one log and removes all of the matching ones, so all a malicious sender could do is
+        /// either: cause for the log to be deleted when it shouldn't have (which is fine - they can already make
+        /// the content be whatever), or cause for the log to not be deleted when it should have (which is also fine
+        /// - it'll be a log for a note that doesn't exist).
+        ///   It's important here that we do not return the log from this function to the app, otherwise it could
+        /// try to do stuff with it and then that might be wrong.
         let (encrypted_log, note_hash_counter) =
             unsafe { compute_payload_unconstrained(*context, e.note, recipient, sender) };
         context.emit_raw_note_log(encrypted_log, note_hash_counter);

noir-projects/aztec-nr/aztec/src/encrypted_logs/payload.nr

@@ -62,9 +62,9 @@ pub fn compute_private_log_payload<let P: u32>(
     let encrypted: [u8; ENCRYPTED_PAYLOAD_SIZE_IN_BYTES] =
         compute_encrypted_log(contract_address, recipient, extended_plaintext);
 
-    // 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.
+    /// Safety: 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 = unsafe { get_app_tag_as_sender(sender, recipient) };
     increment_app_tagging_secret_index_as_sender(sender, recipient);
 
@@ -82,9 +82,9 @@ pub fn compute_partial_public_log_payload<let P: u32, let M: u32>(
     let encrypted: [u8; M - 32] =
         compute_encrypted_log(contract_address, recipient, extended_plaintext);
 
-    // 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.
+    /// Safety: 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 = unsafe { get_app_tag_as_sender(sender, recipient) };
     increment_app_tagging_secret_index_as_sender(sender, recipient);
     // Silo the tag with contract address.
@@ -158,6 +158,8 @@ fn compute_encrypted_log<let P: u32, let M: u32>(
 // Prepend the plaintext length as the first byte, then copy the plaintext itself starting from the second byte.
 // Fill the remaining bytes with random values to reach a fixed length of N.
 fn extend_private_log_plaintext<let P: u32, let N: u32>(plaintext: [u8; P]) -> [u8; N] {
+    /// Safety: A malicious sender could reveal the whole contents of the encrypted log so trusting it to set
+    /// a random padding in plaintext is fine.
     let mut padded = unsafe { get_random_bytes() };
     padded[0] = (P >> 8) as u8;
     padded[1] = P as u8;
@@ -192,10 +194,11 @@ fn fr_to_fq(r: Field) -> Scalar {
 
 fn generate_ephemeral_key_pair() -> (Scalar, Point) {
     // @todo Need to draw randomness from the full domain of Fq not only Fr
-    // We use the randomness to preserve the privacy of both the sender and recipient via encryption, so a malicious
-    // sender could use non-random values to reveal the plaintext. But they already know it themselves anyway, and so
-    // the recipient already trusts them to not disclose this information. We can therefore assume that the sender will
-    // cooperate in the random value generation.
+
+    /// Safety: We use the randomness to preserve the privacy of both the sender and recipient via encryption, so
+    /// a malicious sender could use non-random values to reveal the plaintext. But they already know it themselves
+    /// anyway, and so the recipient already trusts them to not disclose this information. We can therefore assume
+    /// that the sender will cooperate in the random value generation.
     let randomness = unsafe { random() };
 
     // We use the unsafe version of `fr_to_fq` because multi_scalar_mul (called by derive_public_key) will constrain

noir-projects/aztec-nr/aztec/src/history/note_inclusion.nr

@@ -19,6 +19,7 @@ impl ProveNoteInclusion for BlockHeader {
     {
         let note_hash = compute_note_hash_for_nullify(note);
 
+        /// Safety: The witness is only used as a "magical value" that makes the merkle proof below pass. Hence it's safe.
         let witness = unsafe {
             get_note_hash_membership_witness(self.global_variables.block_number as u32, note_hash)
         };

noir-projects/aztec-nr/aztec/src/history/nullifier_inclusion.nr

@@ -14,6 +14,7 @@ trait ProveNullifierInclusion {
 impl ProveNullifierInclusion for BlockHeader {
     fn prove_nullifier_inclusion(self, nullifier: Field) {
         // 1) Get the membership witness of the nullifier
+        /// Safety: The witness is only used as a "magical value" that makes the proof below pass. Hence it's safe.
         let witness = unsafe {
             get_nullifier_membership_witness(self.global_variables.block_number as u32, nullifier)
         };

noir-projects/aztec-nr/aztec/src/history/nullifier_non_inclusion.nr

@@ -16,6 +16,7 @@ trait ProveNullifierNonInclusion {
 impl ProveNullifierNonInclusion for BlockHeader {
     fn prove_nullifier_non_inclusion(self, nullifier: Field) {
         // 1) Get the membership witness of a low nullifier of the nullifier
+        /// Safety: The witness is only used as a "magical value" that makes the proof below pass. Hence it's safe.
         let witness = unsafe {
             get_low_nullifier_membership_witness(
                 self.global_variables.block_number as u32,

noir-projects/aztec-nr/aztec/src/history/public_storage.nr

@@ -27,6 +27,7 @@ impl PublicStorageHistoricalRead for BlockHeader {
         );
 
         // 2) Get the membership witness for the tree index.
+        /// Safety: The witness is only used as a "magical value" that makes the proof below pass. Hence it's safe.
         let witness = unsafe {
             get_public_data_witness(
                 self.global_variables.block_number as u32,

noir-projects/aztec-nr/aztec/src/keys/getters/mod.nr

@@ -24,7 +24,7 @@ pub unconstrained fn get_ovsk_app(ovpk_m_hash: Field) -> Field {
 // keys at once since the constraints for reading them all are actually fewer than if we read them one at a time - any
 // read keys that are not required by the caller can simply be discarded.
 pub fn get_public_keys(account: AztecAddress) -> PublicKeys {
-    // Public keys are constrained by showing their inclusion in the address's preimage.
+    /// Safety: Public keys are constrained by showing their inclusion in the address's preimage.
     let (public_keys, partial_address) = unsafe { get_public_keys_and_partial_address(account) };
     assert_eq(
         account,

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

@@ -31,6 +31,7 @@ pub fn process_l1_to_l2_message(
 
     // We prove that `message_hash` is in the tree by showing the derivation of the tree root, using a merkle path we
     // get from an oracle.
+    /// Safety: The witness is only used as a "magical value" that makes the merkle proof below pass. Hence it's safe.
     let (_leaf_index, sibling_path) =
         unsafe { get_l1_to_l2_membership_witness(contract_address, message_hash, secret) };