refactor: kernel circuit refactor (#20147)

- Change from `counter >= split_counter` to `counter > split_counter`
- Use existing utils to get squash flags to build kept arrays
- Remove if-else from general hash utils: having it in the general utils
seems like a footgun. Moving the if-else to the parents that know if an
item should be hashed or not.
- Remove duplicate util `poseidon2_cheaper_variable_hash`: it's exactly
the same as `poseidon2_hash_subarray` now that poseidon2 does not add an
extra field for variable length input.

Author: LeilaWang

noir-projects/aztec-nr/aztec/src/messages/processing/log_retrieval_request.nr

@@ -2,7 +2,7 @@ use crate::protocol::{address::AztecAddress, traits::Serialize};
 
 /// A request for the `bulk_retrieve_logs` oracle to fetch either:
 /// - a public log emitted by `contract_address` with `unsiloed_tag`
-/// - a private log with tag equal to `silo_private_log(unsiloed_tag, contract_address)`.
+/// - a private log with tag equal to `compute_siloed_private_log_first_field(contract_address, unsiloed_tag)`.
 #[derive(Serialize)]
 pub(crate) struct LogRetrievalRequest {
     pub contract_address: AztecAddress,

noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/accumulated_data/assert_sorted_padded_transformed_array/check_padded_items.nr

@@ -19,15 +19,18 @@ use types::traits::Empty;
 ///
 /// Note: Technically, values outside this range could be non-empty, since they are ignored. However, enforcing emptiness
 /// helps prevent unexpected output if the array is misconfigured.
+///
+/// Returns the number of padded items.
 pub unconstrained fn check_padded_items<T, let N: u32>(
     padded_items: [T; N],
     num_original_items: u32,
     capped_size: u32,
-)
+) -> u32
 where
     T: Empty,
 {
     let mut seen_empty = false;
+    let mut num_padded_items = 0;
     for i in 0..N {
         let item_is_empty = padded_items[i].is_empty();
         if i < num_original_items | i >= capped_size {
@@ -41,8 +44,11 @@ where
                 "non-empty padded items should be consecutive within the range [num_original_items, capped_size)",
             );
             seen_empty |= item_is_empty;
+            num_padded_items += 1 * !item_is_empty as u32;
         }
     }
+
+    num_padded_items
 }
 
 mod tests {
@@ -63,35 +69,36 @@ mod tests {
             }
         }
 
-        pub fn execute(self) {
+        pub fn execute(self) -> u32 {
             // Safety: `check_padded_items` is supposed to be called only in unconstrained functions.
             unsafe {
-                check_padded_items(self.padded_items, self.num_original_items, self.capped_size);
+                check_padded_items(self.padded_items, self.num_original_items, self.capped_size)
             }
         }
     }
 
     #[test]
     fn full_padded_items() {
         let builder = TestBuilder::new();
-        builder.execute();
+        assert_eq(builder.execute(), 3);
     }
 
     #[test]
     fn partially_full_padded_items() {
         let mut builder = TestBuilder::new();
+        let expected_num_padded_items = 3;
 
         // One empty padded item.
         builder.capped_size = 6;
-        builder.execute();
+        assert_eq(builder.execute(), expected_num_padded_items);
 
         // Two empty padded items.
         builder.capped_size = 7;
-        builder.execute();
+        assert_eq(builder.execute(), expected_num_padded_items);
 
         // Three empty padded items.
         builder.capped_size = 8;
-        builder.execute();
+        assert_eq(builder.execute(), expected_num_padded_items);
     }
 
     #[test]
@@ -100,7 +107,7 @@ mod tests {
 
         builder.padded_items = [0, 0, 0, 0, 0, 0, 0, 0];
 
-        builder.execute();
+        assert_eq(builder.execute(), 0);
     }
 
     #[test(should_fail_with = "padded items should be empty outside of the range [num_original_items, capped_size)")]
@@ -109,7 +116,7 @@ mod tests {
 
         builder.padded_items[0] = 99;
 
-        builder.execute();
+        let _ = builder.execute();
     }
 
     #[test(should_fail_with = "padded items should be empty outside of the range [num_original_items, capped_size)")]
@@ -118,7 +125,7 @@ mod tests {
 
         builder.capped_size = 4;
 
-        builder.execute();
+        let _ = builder.execute();
     }
 
     #[test(should_fail_with = "padded items should be empty outside of the range [num_original_items, capped_size)")]
@@ -127,7 +134,7 @@ mod tests {
 
         builder.padded_items[6] = 99;
 
-        builder.execute();
+        let _ = builder.execute();
     }
 
     #[test(should_fail_with = "non-empty padded items should be consecutive within the range [num_original_items, capped_size)")]
@@ -136,7 +143,7 @@ mod tests {
 
         builder.padded_items[2] = 0;
 
-        builder.execute();
+        let _ = builder.execute();
     }
 
     #[test(should_fail_with = "non-empty padded items should be consecutive within the range [num_original_items, capped_size)")]
@@ -145,6 +152,6 @@ mod tests {
 
         builder.padded_items[3] = 0;
 
-        builder.execute();
+        let _ = builder.execute();
     }
 }

noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/private_kernel_circuit_output_composer.nr

@@ -32,7 +32,7 @@ use types::{
         validation_requests::PrivateValidationRequests,
     },
     address::AztecAddress,
-    hash::{create_protocol_nullifier, silo_nullifier},
+    hash::{compute_siloed_nullifier, create_protocol_nullifier},
     side_effect::Ordered,
     traits::Empty,
     utils::arrays::ClaimedLengthArray,
@@ -78,7 +78,10 @@ impl PrivateKernelCircuitOutputComposer {
         // If `first_nullifier_hint` equals the protocol nullifier, it is inserted at index 0. Otherwise, at least one
         // private call must emit a nullifier, and the hint should match the first nullifier emitted.
         let scoped_protocol_nullifier = create_protocol_nullifier(inputs.tx_request);
-        let protocol_nullifier = silo_nullifier(scoped_protocol_nullifier);
+        let protocol_nullifier = compute_siloed_nullifier(
+            scoped_protocol_nullifier.contract_address,
+            scoped_protocol_nullifier.innermost().value,
+        );
         let should_inject_protocol_nullifier = inputs.first_nullifier_hint == protocol_nullifier;
         if should_inject_protocol_nullifier {
             // Safety: we should ordinarily use `push`, so that the length correctly updates, but we know this is the very

noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/components/reset_output_composer.nr

@@ -23,11 +23,69 @@ use types::{
         MAX_NULLIFIER_READ_REQUESTS_PER_TX, MAX_NULLIFIERS_PER_TX, MAX_PRIVATE_LOGS_PER_TX,
         MAX_U32_VALUE, SIDE_EFFECT_MASKING_ADDRESS,
     },
-    hash::{compute_unique_siloed_note_hash, silo_note_hash, silo_nullifier, silo_private_log},
+    hash::{
+        compute_note_nonce_and_unique_note_hash, compute_siloed_note_hash, compute_siloed_nullifier,
+        compute_siloed_private_log,
+    },
     side_effect::{Counted, Scoped},
     utils::arrays::ClaimedLengthArray,
 };
 
+pub fn silo_note_hash(note_hash: Scoped<Counted<NoteHash>>) -> Scoped<Counted<NoteHash>> {
+    let mut siloed = note_hash;
+    siloed.inner.inner =
+        compute_siloed_note_hash(note_hash.contract_address, note_hash.innermost());
+
+    // We set the contract address to zero to indicate that the note hash has been siloed.
+    // The reset output validator will check the contract address of the first note hash to ensure that siloing has not
+    // been performed in a previous reset.
+    siloed.contract_address = AztecAddress::zero();
+
+    siloed
+}
+
+pub fn uniquify_siloed_note_hash(
+    siloed_note_hash: Scoped<Counted<NoteHash>>,
+    first_nullifier: Field,
+    index_in_tx: u32,
+) -> Scoped<Counted<NoteHash>> {
+    let mut uniquified = siloed_note_hash;
+    uniquified.inner.inner = compute_note_nonce_and_unique_note_hash(
+        siloed_note_hash.inner.inner,
+        first_nullifier,
+        index_in_tx,
+    );
+    uniquified
+}
+
+pub fn silo_nullifier(nullifier: Scoped<Counted<Nullifier>>) -> Scoped<Counted<Nullifier>> {
+    let mut siloed = nullifier;
+    siloed.inner.inner.value =
+        compute_siloed_nullifier(nullifier.contract_address, nullifier.innermost().value);
+
+    // We set the contract address to zero to indicate that the nullifier has been siloed.
+    // The reset output validator will check the contract address of the first nullifier to ensure that siloing has not
+    // been performed in a previous reset.
+    siloed.contract_address = AztecAddress::zero();
+
+    siloed
+}
+
+pub fn silo_private_log(
+    private_log: Scoped<Counted<PrivateLogData>>,
+) -> Scoped<Counted<PrivateLogData>> {
+    let mut siloed = private_log;
+    siloed.inner.inner.log =
+        compute_siloed_private_log(private_log.contract_address, private_log.innermost().log);
+
+    // We set the contract address to zero to indicate that the private log has been siloed.
+    // The reset output validator will check the contract address of the first private log to ensure that siloing has not
+    // been performed in a previous reset.
+    siloed.contract_address = AztecAddress::zero();
+
+    siloed
+}
+
 pub struct ResetOutputComposer {
     pub previous_kernel: PrivateKernelCircuitPublicInputs,
     pub padded_side_effects: PaddedSideEffects,
@@ -142,19 +200,18 @@ impl ResetOutputComposer {
         // Calling this in the unconstrained context is not a problem. Since it doesn't matter if there are non-empty
         // items outside of the range [num_kept_note_hashes, NoteHashSiloingAmount). The values will simply be ignored.
         // Having the check here is useful to catch any bugs where the padded values are not configured correctly by mistake.
-        check_padded_items(
+        let num_padded_items = check_padded_items(
             padded_note_hashes,
             num_kept_note_hashes,
             NoteHashSiloingAmount,
         );
 
         let mut padded_unique_siloed_sorted_kept_note_hashes = sorted_kept_note_hashes;
 
-        for i in 0..sorted_kept_note_hashes.array.len() {
+        for i in 0..num_kept_note_hashes + num_padded_items {
             // Push padding:
-            if (i >= num_kept_note_hashes)
+            if i >= num_kept_note_hashes {
                 // We only apply padding if the executor of this circuit provides padding as input:
-                & (padded_note_hashes[i] != 0) {
                 // We 'push' so that the length increments.
                 // Note: beyond `NoteHashSiloingAmount`, these so-called "padded" note_hashes
                 // will be empty, because we just called `check_padded_items`.
@@ -176,7 +233,7 @@ impl ResetOutputComposer {
             let siloed_note_hash = silo_note_hash(note_hash);
 
             // Unique:
-            let unique_siloed_note_hash = compute_unique_siloed_note_hash(
+            let unique_siloed_note_hash = uniquify_siloed_note_hash(
                 siloed_note_hash,
                 self.previous_kernel.claimed_first_nullifier,
                 i,
@@ -224,7 +281,7 @@ impl ResetOutputComposer {
             // This means the "revertible from private-land" note_hashes cannot be made unique in
             // private-land, since the uniqueness can only be derived in public-land, because the
             // revertible notes' indices (within the note_hashes of the tx) will only be known in
-            // public-land. See `compute_unique_siloed_note_hash` for more explanation on how the
+            // public-land. See `compute_note_nonce_and_unique_note_hash` for more explanation on how the
             // uniqueness (aka note_nonce) is derived.
             //
             // In summary:
@@ -239,18 +296,11 @@ impl ResetOutputComposer {
                 self.previous_kernel.claimed_revertible_counter,
             );
 
-            padded_unique_siloed_sorted_kept_note_hashes.array[i].inner.inner = if is_non_revertible {
+            padded_unique_siloed_sorted_kept_note_hashes.array[i] = if is_non_revertible {
                 unique_siloed_note_hash
             } else {
                 siloed_note_hash
             };
-
-            // Why is it doing this? Is the contract_address being overloaded to mean something else?
-            // Oh, is it being used to say "This has already been siloed"?
-            // TODO: Why not use an extra bool to convey that?
-            // A contract address will be assigned to the padding within constrained-land (see validate_sorted_siloed_unique_padded_note_hashes).
-            padded_unique_siloed_sorted_kept_note_hashes.array[i].contract_address =
-                AztecAddress::zero();
         }
         padded_unique_siloed_sorted_kept_note_hashes
     }
@@ -270,19 +320,18 @@ impl ResetOutputComposer {
         // Calling this in the unconstrained context is not a problem. Since it doesn't matter if there are non-empty
         // items outside of the range [num_nullifiers, NullifierSiloingAmount). The values will simply be ignored.
         // Having the check here is useful to catch any bugs where the padded values are not configured correctly by mistake.
-        check_padded_items(
+        let num_padded_items = check_padded_items(
             padded_nullifiers,
             num_kept_nullifiers,
             NullifierSiloingAmount,
         );
 
         let mut padded_siloed_sorted_kept_nullifiers = sorted_kept_nullifiers;
 
-        for i in 0..sorted_kept_nullifiers.array.len() {
+        for i in 0..num_kept_nullifiers + num_padded_items {
             // Push padding:
-            if (i >= num_kept_nullifiers)
+            if i >= num_kept_nullifiers {
                 // we only apply padding if the executor of this circuit provides padding as input:
-                & (padded_nullifiers[i] != 0) {
                 padded_siloed_sorted_kept_nullifiers.push(Nullifier {
                     value: padded_nullifiers[i],
                     note_hash: 0,
@@ -295,11 +344,8 @@ impl ResetOutputComposer {
                     .scope(SIDE_EFFECT_MASKING_ADDRESS));
             }
 
-            // Silo:
-            padded_siloed_sorted_kept_nullifiers.array[i].inner.inner.value =
-                silo_nullifier(padded_siloed_sorted_kept_nullifiers.array[i]);
-            // A contract address will be assigned to the padding within constrained-land (see validate_sorted_siloed_padded_nullifiers).
-            padded_siloed_sorted_kept_nullifiers.array[i].contract_address = AztecAddress::zero();
+            let nullifier = padded_siloed_sorted_kept_nullifiers.array[i];
+            padded_siloed_sorted_kept_nullifiers.array[i] = silo_nullifier(nullifier);
         }
 
         padded_siloed_sorted_kept_nullifiers
@@ -320,7 +366,7 @@ impl ResetOutputComposer {
         // Calling this in the unconstrained context is not a problem. Since it doesn't matter if there are non-empty
         // items outside of the range [num_private_logs, PrivateLogSiloingAmount). The values will simply be ignored.
         // Having the check here is useful to catch any bugs where the padded values are not configured correctly by mistake.
-        check_padded_items(
+        let num_padded_items = check_padded_items(
             padded_private_logs,
             num_kept_private_logs,
             PrivateLogSiloingAmount,
@@ -329,11 +375,10 @@ impl ResetOutputComposer {
         // The ordering of adjectives in this name is intentional (back to front).
         let mut padded_siloed_sorted_kept_private_logs = sorted_kept_private_logs;
 
-        for i in 0..sorted_kept_private_logs.array.len() {
+        for i in 0..num_kept_private_logs + num_padded_items {
             // Pad:
-            if (i >= num_kept_private_logs)
+            if i >= num_kept_private_logs {
                 // we only apply padding if the executor of this circuit provides padding as input:
-                & (padded_private_logs[i].length != 0) {
                 padded_siloed_sorted_kept_private_logs.push(PrivateLogData {
                     log: padded_private_logs[i],
                     note_hash_counter: 0, // Recall: this is only used to hint which note (if any) this log corresponds to.
@@ -346,11 +391,8 @@ impl ResetOutputComposer {
                     .scope(SIDE_EFFECT_MASKING_ADDRESS));
             }
 
-            // Silo:
-            padded_siloed_sorted_kept_private_logs.array[i].inner.inner.log =
-                silo_private_log(padded_siloed_sorted_kept_private_logs.array[i]);
-            // A contract address will be assigned to the padding within constrained-land (see validate_sorted_siloed_padded_private_logs).
-            padded_siloed_sorted_kept_private_logs.array[i].contract_address = AztecAddress::zero();
+            let private_log = padded_siloed_sorted_kept_private_logs.array[i];
+            padded_siloed_sorted_kept_private_logs.array[i] = silo_private_log(private_log);
         }
 
         padded_siloed_sorted_kept_private_logs