feat: note hash read requests fixes and refactoring (AztecProtocol#6125)

Validating note hash read requests in the private tail circuits.
Also ensure that a read happens before the note hash is nullified.

l1-contracts/src/core/libraries/ConstantsGen.sol

@@ -132,7 +132,7 @@ library Constants {
   uint256 internal constant HEADER_LENGTH = APPEND_ONLY_TREE_SNAPSHOT_LENGTH
     + CONTENT_COMMITMENT_LENGTH + STATE_REFERENCE_LENGTH + GLOBAL_VARIABLES_LENGTH;
   uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = CALL_CONTEXT_LENGTH + 3
-    + MAX_BLOCK_NUMBER_LENGTH + (SIDE_EFFECT_LENGTH * MAX_NOTE_HASH_READ_REQUESTS_PER_CALL)
+    + MAX_BLOCK_NUMBER_LENGTH + (READ_REQUEST_LENGTH * MAX_NOTE_HASH_READ_REQUESTS_PER_CALL)
     + (READ_REQUEST_LENGTH * MAX_NULLIFIER_READ_REQUESTS_PER_CALL)
     + (NULLIFIER_KEY_VALIDATION_REQUEST_LENGTH * MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_CALL)
     + (NOTE_HASH_LENGTH * MAX_NEW_NOTE_HASHES_PER_CALL)

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

@@ -48,7 +48,7 @@ struct PrivateContext {
 
     max_block_number: MaxBlockNumber,
 
-    note_hash_read_requests: BoundedVec<SideEffect, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL>,
+    note_hash_read_requests: BoundedVec<ReadRequest, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL>,
     nullifier_read_requests: BoundedVec<ReadRequest, MAX_NULLIFIER_READ_REQUESTS_PER_CALL>,
     nullifier_key_validation_requests: BoundedVec<NullifierKeyValidationRequest, MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_CALL>,
 
@@ -192,7 +192,7 @@ impl PrivateContext {
     }
 
     pub fn push_note_hash_read_request(&mut self, note_hash: Field) {
-        let side_effect = SideEffect { value: note_hash, counter: self.side_effect_counter };
+        let side_effect = ReadRequest { value: note_hash, counter: self.side_effect_counter };
         self.note_hash_read_requests.push(side_effect);
         self.side_effect_counter = self.side_effect_counter + 1;
     }

noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr

@@ -1,4 +1,4 @@
-use dep::protocol_types::{constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, abis::side_effect::SideEffect};
+use dep::protocol_types::{constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, abis::read_request::ReadRequest};
 use crate::context::{PrivateContext, PublicContext, Context};
 use crate::note::{
     constants::MAX_NOTES_PER_PAGE, lifecycle::{create_note, create_note_hash_from_public, destroy_note},
@@ -59,7 +59,7 @@ impl<Note> PrivateSet<Note> {
     pub fn remove<N>(self, note: Note) where Note: NoteInterface<N> {
         let context = self.context.private.unwrap();
         let note_hash = compute_note_hash_for_consumption(note);
-        let has_been_read = context.note_hash_read_requests.any(|r: SideEffect| r.value == note_hash);
+        let has_been_read = context.note_hash_read_requests.any(|r: ReadRequest| r.value == note_hash);
         assert(has_been_read, "Can only remove a note that has been read from the set.");
 
         destroy_note(context, note);

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

@@ -2,15 +2,12 @@ use dep::std;
 use dep::types::{
     abis::{
     call_request::CallRequest, accumulated_data::PrivateAccumulatedData,
-    membership_witness::NoteHashReadRequestMembershipWitness,
     private_circuit_public_inputs::PrivateCircuitPublicInputs,
-    private_kernel::private_call_data::PrivateCallData, side_effect::SideEffect
+    private_kernel::private_call_data::PrivateCallData
 },
     address::{AztecAddress, PartialAddress}, contract_class_id::ContractClassId,
-    constants::MAX_NOTE_HASH_READ_REQUESTS_PER_CALL,
     hash::{private_functions_root_from_siblings, stdlib_recursion_verification_key_compress_native_vk},
-    merkle_tree::check_membership, utils::{arrays::{array_length, validate_array}},
-    traits::{is_empty, is_empty_array}
+    utils::{arrays::{array_length, validate_array}}, traits::{is_empty, is_empty_array}
 };
 
 fn validate_arrays(app_public_inputs: PrivateCircuitPublicInputs) {
@@ -30,48 +27,6 @@ fn validate_arrays(app_public_inputs: PrivateCircuitPublicInputs) {
     validate_array(app_public_inputs.unencrypted_logs_hashes);
 }
 
-// Validate all read requests against the historical note hash tree root.
-// Use their membership witnesses to do so. If the historical root is not yet
-// initialized, initialize it using the first read request here (if present).
-//
-// More info here:
-// - https://discourse.aztec.network/t/to-read-or-not-to-read/178
-// - https://discourse.aztec.network/t/spending-notes-which-havent-yet-been-inserted/180
-pub fn validate_note_hash_read_requests(
-    historical_note_hash_tree_root: Field,
-    read_requests: [SideEffect; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL],
-    read_request_membership_witnesses: [NoteHashReadRequestMembershipWitness; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL]
-) {
-    // membership witnesses must resolve to the same note hash tree root
-    // for every request in all kernel iterations
-    for rr_idx in 0..MAX_NOTE_HASH_READ_REQUESTS_PER_CALL {
-        let read_request = read_requests[rr_idx].value;
-        let witness = read_request_membership_witnesses[rr_idx];
-
-        // A pending note hash is the one that is not yet added to note hash tree
-        // A "transient read" is when we try to "read" a pending note hash within a transaction
-        // between function calls, as opposed to reading the outputs of a previous transaction
-        // which is a "pending read".
-        // A transient read is when we try to "read" a pending note hash
-        // We determine if it is a transient read depending on the leaf index from the membership witness
-        // Note that the Merkle membership proof would be null and void in case of an transient read
-        // but we use the leaf index as a placeholder to detect a 'pending note read'.
-
-        if (read_request != 0) & (witness.is_transient == false) {
-            assert(
-                check_membership(
-                    read_request,
-                    witness.leaf_index,
-                    witness.sibling_path,
-                    historical_note_hash_tree_root
-                ), "note hash tree root mismatch"
-            );
-            // TODO(https://github.com/AztecProtocol/aztec-packages/issues/1354): do we need to enforce
-            // that a non-transient read_request was derived from the proper/current contract address?
-        }
-    }
-}
-
 fn perform_static_call_checks(private_call: PrivateCallData) {
     let public_inputs = private_call.call_stack_item.public_inputs;
     let is_static_call = public_inputs.call_context.is_static_call;

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

@@ -1,25 +1,16 @@
-use crate::common;
-use dep::std::{cmp::Eq, option::Option};
-use dep::reset_kernel_lib::{
-    NullifierReadRequestHints, PrivateValidationRequestProcessor,
-    verify_squashed_transient_note_hashes_and_nullifiers
-};
+use dep::reset_kernel_lib::verify_squashed_transient_note_hashes_and_nullifiers;
 use dep::types::{
     abis::{
     kernel_data::PrivateKernelData,
     kernel_circuit_public_inputs::{KernelCircuitPublicInputs, PrivateKernelCircuitPublicInputsBuilder, PublicKernelCircuitPublicInputs},
     note_hash::NoteHashContext, nullifier::Nullifier, side_effect::{SideEffect, Ordered}, gas::Gas
 },
     constants::{
-    MAX_NEW_NOTE_HASHES_PER_TX, MAX_NEW_NULLIFIERS_PER_TX, MAX_NOTE_HASH_READ_REQUESTS_PER_TX,
-    MAX_NULLIFIER_READ_REQUESTS_PER_TX, MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_TX,
-    MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX, MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX,
-    MAX_ENCRYPTED_LOGS_PER_TX, MAX_UNENCRYPTED_LOGS_PER_TX, DA_BYTES_PER_FIELD, FIXED_DA_GAS,
-    DA_GAS_PER_BYTE
+    MAX_NEW_NOTE_HASHES_PER_TX, MAX_NEW_NULLIFIERS_PER_TX, MAX_ENCRYPTED_LOGS_PER_TX,
+    MAX_UNENCRYPTED_LOGS_PER_TX
 },
-    grumpkin_private_key::GrumpkinPrivateKey,
     hash::{compute_note_hash_nonce, compute_unique_siloed_note_hash},
-    utils::arrays::{array_length, array_to_bounded_vec, assert_sorted_array}, traits::{Empty, is_empty}
+    utils::arrays::{array_length, array_to_bounded_vec, assert_sorted_array}
 };
 
 fn asc_sort_by_counters<T>(a: T, b: T) -> bool where T: Ordered {

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

@@ -2,9 +2,8 @@ use dep::types::{
     abis::{
     call_request::CallRequest, combined_constant_data::CombinedConstantData,
     kernel_circuit_public_inputs::{PrivateKernelCircuitPublicInputs, PrivateKernelCircuitPublicInputsBuilder},
-    max_block_number::MaxBlockNumber, membership_witness::NoteHashReadRequestMembershipWitness,
-    nullifier::Nullifier, private_circuit_public_inputs::PrivateCircuitPublicInputs,
-    side_effect::SideEffect
+    max_block_number::MaxBlockNumber, nullifier::Nullifier,
+    private_circuit_public_inputs::PrivateCircuitPublicInputs
 },
     address::AztecAddress,
     constants::{
@@ -19,7 +18,6 @@ struct DataSource {
     private_call_public_inputs: PrivateCircuitPublicInputs,
     storage_contract_address: AztecAddress,
     note_hash_nullifier_counters: [u32; MAX_NEW_NOTE_HASHES_PER_CALL],
-    note_hash_read_request_membership_witnesses: [NoteHashReadRequestMembershipWitness; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL],
     private_call_requests: [CallRequest; MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL],
     public_call_requests: [CallRequest; MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL],
 }
@@ -80,7 +78,6 @@ impl PrivateKernelCircuitPublicInputsComposer {
         &mut self,
         private_call_public_inputs: PrivateCircuitPublicInputs,
         note_hash_nullifier_counters: [u32; MAX_NEW_NOTE_HASHES_PER_CALL],
-        note_hash_read_request_membership_witnesses: [NoteHashReadRequestMembershipWitness; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL],
         private_call_requests: [CallRequest; MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL],
         public_call_requests: [CallRequest; MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL]
     ) -> Self {
@@ -89,7 +86,6 @@ impl PrivateKernelCircuitPublicInputsComposer {
             storage_contract_address,
             private_call_public_inputs,
             note_hash_nullifier_counters,
-            note_hash_read_request_membership_witnesses,
             private_call_requests,
             public_call_requests
         };
@@ -118,19 +114,11 @@ impl PrivateKernelCircuitPublicInputsComposer {
     }
 
     fn propagate_note_hash_read_requests(&mut self, source: DataSource) {
-        // TODO: Propagate all requests to verify in a reset circuit.
-        // Transient read requests and witnesses are accumulated in public_inputs.end
-        // We silo the read requests (domain separation per contract address)
         let read_requests = source.private_call_public_inputs.note_hash_read_requests;
         for i in 0..read_requests.len() {
-            let read_request = read_requests[i];
-            let witness = source.note_hash_read_request_membership_witnesses[i];
-            if witness.is_transient & (read_request.value != 0) { // only forward transient to public inputs
-                let siloed_read_request = SideEffect {
-                    counter: read_request.counter,
-                    value: silo_note_hash(source.storage_contract_address, read_request.value)
-                };
-                self.public_inputs.validation_requests.note_hash_read_requests.push(siloed_read_request);
+            let request = read_requests[i];
+            if !is_empty(request) {
+                self.public_inputs.validation_requests.note_hash_read_requests.push(request.to_context(source.storage_contract_address));
             }
         }
     }