feat: Nullifier non membership (#5152)

Allow public functions to push non-existent read requests for
nullifiers. The kernel will check that the values being read are not in
the tree and not in the pending set.
Note that the read requests are only propagated to non-revertible data.
We will move all types of requests to `validation_requests` in function
circuit's public inputs. And they should always be verified whether the
tx reverts or not.

l1-contracts/slither_output.md

@@ -321,15 +321,15 @@ src/core/messagebridge/Inbox.sol#L148-L153
 Impact: Informational
 Confidence: Medium
  - [ ] ID-35
-Variable [Constants.LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L129) is too similar to [Constants.NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L122)
+Variable [Constants.LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L131) is too similar to [Constants.NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L124)
 
-src/core/libraries/ConstantsGen.sol#L129
+src/core/libraries/ConstantsGen.sol#L131
 
 
  - [ ] ID-36
-Variable [Constants.L1_TO_L2_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L109) is too similar to [Constants.L2_TO_L1_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L110)
+Variable [Constants.L1_TO_L2_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L111) is too similar to [Constants.L2_TO_L1_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L112)
 
-src/core/libraries/ConstantsGen.sol#L109
+src/core/libraries/ConstantsGen.sol#L111
 
 
  - [ ] ID-37

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

@@ -25,6 +25,7 @@ library Constants {
   uint256 internal constant MAX_PUBLIC_DATA_READS_PER_CALL = 16;
   uint256 internal constant MAX_NOTE_HASH_READ_REQUESTS_PER_CALL = 32;
   uint256 internal constant MAX_NULLIFIER_READ_REQUESTS_PER_CALL = 2;
+  uint256 internal constant MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL = 2;
   uint256 internal constant MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_CALL = 1;
   uint256 internal constant MAX_NEW_NOTE_HASHES_PER_TX = 64;
   uint256 internal constant MAX_NON_REVERTIBLE_NOTE_HASHES_PER_TX = 8;
@@ -45,6 +46,7 @@ library Constants {
   uint256 internal constant MAX_NEW_L2_TO_L1_MSGS_PER_TX = 2;
   uint256 internal constant MAX_NOTE_HASH_READ_REQUESTS_PER_TX = 128;
   uint256 internal constant MAX_NULLIFIER_READ_REQUESTS_PER_TX = 8;
+  uint256 internal constant MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_TX = 8;
   uint256 internal constant MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_TX = 4;
   uint256 internal constant NUM_ENCRYPTED_LOGS_HASHES_PER_TX = 1;
   uint256 internal constant NUM_UNENCRYPTED_LOGS_HASHES_PER_TX = 1;
@@ -113,7 +115,7 @@ library Constants {
   uint256 internal constant PARTIAL_STATE_REFERENCE_LENGTH = 6;
   uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 214;
   uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 209;
-  uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 196;
+  uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 200;
   uint256 internal constant STATE_REFERENCE_LENGTH = 8;
   uint256 internal constant TX_CONTEXT_DATA_LENGTH = 4;
   uint256 internal constant TX_REQUEST_LENGTH = 10;

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

@@ -23,7 +23,7 @@ use dep::protocol_types::{
     MAX_NEW_NOTE_HASHES_PER_CALL, MAX_NEW_L2_TO_L1_MSGS_PER_CALL, MAX_NEW_NULLIFIERS_PER_CALL,
     MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL,
     MAX_PUBLIC_DATA_READS_PER_CALL, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL,
-    MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_READ_REQUESTS_PER_CALL,
+    MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL,
     MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_CALL, NUM_FIELDS_PER_SHA256, RETURN_VALUES_LENGTH
 },
     contrakt::{storage_read::StorageRead, storage_update_request::StorageUpdateRequest},
@@ -451,6 +451,7 @@ impl PrivateContext {
                 args_hash: reader.read(),
                 return_values: [0; RETURN_VALUES_LENGTH],
                 nullifier_read_requests: [ReadRequest::empty(); MAX_NULLIFIER_READ_REQUESTS_PER_CALL],
+                nullifier_non_existent_read_requests: [ReadRequest::empty(); MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL],
                 contract_storage_update_requests: [StorageUpdateRequest::empty(); MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL],
                 contract_storage_reads: [StorageRead::empty(); MAX_PUBLIC_DATA_READS_PER_CALL],
                 public_call_stack_hashes: [0; MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL],

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

@@ -15,7 +15,8 @@ use dep::protocol_types::{
     MAX_NEW_NOTE_HASHES_PER_CALL, MAX_NEW_L2_TO_L1_MSGS_PER_CALL, MAX_NEW_NULLIFIERS_PER_CALL,
     MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL, MAX_PUBLIC_DATA_READS_PER_CALL,
     MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL,
-    MAX_NULLIFIER_READ_REQUESTS_PER_CALL, NUM_FIELDS_PER_SHA256, RETURN_VALUES_LENGTH
+    MAX_NULLIFIER_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL,
+    NUM_FIELDS_PER_SHA256, RETURN_VALUES_LENGTH
 },
     contrakt::{storage_read::StorageRead, storage_update_request::StorageUpdateRequest},
     hash::hash_args, header::Header, messaging::l2_to_l1_message::L2ToL1Message, utils::reader::Reader
@@ -29,6 +30,7 @@ struct PublicContext {
     return_values : BoundedVec<Field, RETURN_VALUES_LENGTH>,
 
     nullifier_read_requests: BoundedVec<ReadRequest, MAX_NULLIFIER_READ_REQUESTS_PER_CALL>,
+    nullifier_non_existent_read_requests: BoundedVec<ReadRequest, MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL>,
     contract_storage_update_requests: BoundedVec<StorageUpdateRequest, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL>,
     contract_storage_reads: BoundedVec<StorageRead, MAX_PUBLIC_DATA_READS_PER_CALL>,
     public_call_stack_hashes: BoundedVec<Field, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL>,
@@ -102,6 +104,7 @@ impl PublicContext {
             args_hash,
             return_values: BoundedVec::new(),
             nullifier_read_requests: BoundedVec::new(),
+            nullifier_non_existent_read_requests: BoundedVec::new(),
             contract_storage_update_requests: BoundedVec::new(),
             contract_storage_reads: BoundedVec::new(),
             public_call_stack_hashes: BoundedVec::new(),
@@ -143,6 +146,7 @@ impl PublicContext {
             call_context: self.inputs.call_context, // Done
             args_hash: self.args_hash, // Done
             nullifier_read_requests: self.nullifier_read_requests.storage,
+            nullifier_non_existent_read_requests: self.nullifier_non_existent_read_requests.storage,
             contract_storage_update_requests: self.contract_storage_update_requests.storage,
             contract_storage_reads: self.contract_storage_reads.storage,
             return_values: self.return_values.storage,
@@ -165,6 +169,12 @@ impl PublicContext {
         self.side_effect_counter = self.side_effect_counter + 1;
     }
 
+    pub fn push_nullifier_non_existent_read_request(&mut self, nullifier: Field) {
+        let request = ReadRequest { value: nullifier, counter: self.side_effect_counter };
+        self.nullifier_non_existent_read_requests.push(request);
+        self.side_effect_counter = self.side_effect_counter + 1;
+    }
+
     pub fn message_portal(&mut self, recipient: EthAddress, content: Field) {
         let message = L2ToL1Message { recipient, content };
         self.new_l2_to_l1_msgs.push(message);

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

@@ -20,9 +20,10 @@ use dep::types::{
     hash::{
     compute_constructor_hash, compute_l2_to_l1_hash, compute_logs_hash,
     compute_new_contract_address_hash, function_tree_root_from_siblings, pedersen_hash,
-    private_functions_root_from_siblings, root_from_sibling_path, silo_note_hash, silo_nullifier,
+    private_functions_root_from_siblings, silo_note_hash, silo_nullifier,
     stdlib_recursion_verification_key_compress_native_vk
 },
+    merkle_tree::check_membership,
     utils::{arrays::{array_length, array_to_bounded_vec, validate_array}},
     traits::{is_empty, is_empty_array}
 };
@@ -72,8 +73,14 @@ pub fn validate_note_hash_read_requests(
         // but we use the leaf index as a placeholder to detect a 'pending note read'.
 
         if (read_request != 0) & (witness.is_transient == false) {
-            let root_for_read_request = root_from_sibling_path(read_request, witness.leaf_index, witness.sibling_path);
-            assert(root_for_read_request == historical_note_hash_tree_root, "note hash tree root mismatch");
+            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?
         }

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

@@ -1,12 +1,11 @@
 use crate::common;
 use dep::std::{cmp::Eq, option::Option, unsafe};
-use dep::reset_kernel_lib::{NullifierReadRequestResetHints, reset_read_requests};
+use dep::reset_kernel_lib::{NullifierReadRequestHints, reset_read_requests};
 use dep::types::{
     abis::{
     call_request::CallRequest, nullifier_key_validation_request::NullifierKeyValidationRequestContext,
     kernel_data::{PrivateKernelInnerData, PrivateKernelTailData},
     kernel_circuit_public_inputs::{PrivateKernelCircuitPublicInputsBuilder, PrivateKernelTailCircuitPublicInputs},
-    membership_witness::{MembershipWitness, NullifierMembershipWitness},
     side_effect::{SideEffect, SideEffectLinkedToNoteHash, Ordered}
 },
     constants::{
@@ -26,7 +25,7 @@ struct PrivateKernelTailCircuitPrivateInputs {
     read_commitment_hints: [u64; MAX_NOTE_HASH_READ_REQUESTS_PER_TX],
     sorted_new_nullifiers: [SideEffectLinkedToNoteHash; MAX_NEW_NULLIFIERS_PER_TX],
     sorted_new_nullifiers_indexes: [u64; MAX_NEW_NULLIFIERS_PER_TX],
-    nullifier_read_request_reset_hints: NullifierReadRequestResetHints,
+    nullifier_read_request_hints: NullifierReadRequestHints,
     nullifier_commitment_hints: [u64; MAX_NEW_NULLIFIERS_PER_TX],
     master_nullifier_secret_keys: [GrumpkinPrivateKey; MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_TX],
 }
@@ -43,7 +42,7 @@ impl PrivateKernelTailCircuitPrivateInputs {
 
         let pending_nullifiers = self.previous_kernel.public_inputs.end.new_nullifiers;
 
-        let hints = self.nullifier_read_request_reset_hints;
+        let hints = self.nullifier_read_request_hints;
 
         let nullifier_tree_root = public_inputs.constants.historical_header.state.partial.nullifier_tree.root;
 
@@ -256,7 +255,7 @@ mod tests {
     use dep::std::{cmp::Eq, unsafe};
     use crate::{private_kernel_tail::PrivateKernelTailCircuitPrivateInputs};
     use dep::reset_kernel_lib::{
-        NullifierReadRequestResetHintsBuilder,
+        tests::nullifier_read_request_hints_builder::NullifierReadRequestHintsBuilder,
         read_request_reset::{PendingReadHint, ReadRequestState, ReadRequestStatus}
     };
     use dep::types::constants::{
@@ -277,7 +276,7 @@ mod tests {
         previous_kernel: PreviousKernelDataBuilder,
         read_commitment_hints: [u64; MAX_NOTE_HASH_READ_REQUESTS_PER_TX],
         nullifier_commitment_hints: [u64; MAX_NEW_NULLIFIERS_PER_TX],
-        nullifier_read_request_reset_hints_builder: NullifierReadRequestResetHintsBuilder,
+        nullifier_read_request_hints_builder: NullifierReadRequestHintsBuilder,
     }
 
     impl PrivateKernelTailInputsBuilder {
@@ -286,7 +285,7 @@ mod tests {
                 previous_kernel: PreviousKernelDataBuilder::new(false),
                 read_commitment_hints: [0; MAX_NOTE_HASH_READ_REQUESTS_PER_TX],
                 nullifier_commitment_hints: [0; MAX_NEW_NULLIFIERS_PER_TX],
-                nullifier_read_request_reset_hints_builder: NullifierReadRequestResetHintsBuilder::new(MAX_NULLIFIER_READ_REQUESTS_PER_TX)
+                nullifier_read_request_hints_builder: NullifierReadRequestHintsBuilder::new(MAX_NULLIFIER_READ_REQUESTS_PER_TX)
             }
         }
 
@@ -326,10 +325,10 @@ mod tests {
         pub fn add_nullifier_pending_read(&mut self, nullifier_index_offset_one: u64) {
             let nullifier_index = nullifier_index_offset_one + 1; // + 1 is for the first nullifier
             let read_request_index = self.previous_kernel.add_read_request_for_pending_nullifier(nullifier_index);
-            let hint_index = self.nullifier_read_request_reset_hints_builder.pending_read_hints.len();
+            let hint_index = self.nullifier_read_request_hints_builder.pending_read_hints.len();
             let hint = PendingReadHint { read_request_index, pending_value_index: nullifier_index };
-            self.nullifier_read_request_reset_hints_builder.pending_read_hints.push(hint);
-            self.nullifier_read_request_reset_hints_builder.read_request_statuses[read_request_index] = ReadRequestStatus { state: ReadRequestState.PENDING, hint_index };
+            self.nullifier_read_request_hints_builder.pending_read_hints.push(hint);
+            self.nullifier_read_request_hints_builder.read_request_statuses[read_request_index] = ReadRequestStatus { state: ReadRequestState.PENDING, hint_index };
         }
 
         pub fn nullify_transient_commitment(&mut self, nullifier_index: Field, commitment_index: u64) {
@@ -383,7 +382,7 @@ mod tests {
                 read_commitment_hints: sorted_read_commitment_hints,
                 sorted_new_nullifiers,
                 sorted_new_nullifiers_indexes,
-                nullifier_read_request_reset_hints: self.nullifier_read_request_reset_hints_builder.to_hints(),
+                nullifier_read_request_hints: self.nullifier_read_request_hints_builder.to_hints(),
                 nullifier_commitment_hints: sorted_nullifier_commitment_hints,
                 master_nullifier_secret_keys: unsafe::zeroed()
             };
@@ -480,10 +479,10 @@ mod tests {
 
         builder.append_nullifiers(3);
         builder.add_nullifier_pending_read(1);
-        let mut hint = builder.nullifier_read_request_reset_hints_builder.pending_read_hints.pop();
+        let mut hint = builder.nullifier_read_request_hints_builder.pending_read_hints.pop();
         assert(hint.pending_value_index == 2);
         hint.pending_value_index = 1;
-        builder.nullifier_read_request_reset_hints_builder.pending_read_hints.push(hint);
+        builder.nullifier_read_request_hints_builder.pending_read_hints.push(hint);
 
         builder.failed();
     }

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

@@ -10,9 +10,10 @@ use dep::types::{
     contrakt::{storage_read::StorageRead, storage_update_request::StorageUpdateRequest},
     constants::{
     MAX_NEW_L2_TO_L1_MSGS_PER_CALL, MAX_NEW_NOTE_HASHES_PER_CALL, MAX_NEW_NULLIFIERS_PER_CALL,
-    MAX_NULLIFIER_READ_REQUESTS_PER_CALL, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL,
-    MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, MAX_PUBLIC_DATA_READS_PER_CALL, NUM_FIELDS_PER_SHA256,
-    MAX_REVERTIBLE_PUBLIC_DATA_READS_PER_TX, MAX_REVERTIBLE_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX,
+    MAX_NULLIFIER_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL,
+    MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX,
+    MAX_PUBLIC_DATA_READS_PER_CALL, NUM_FIELDS_PER_SHA256, MAX_REVERTIBLE_PUBLIC_DATA_READS_PER_TX,
+    MAX_REVERTIBLE_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX,
     MAX_NON_REVERTIBLE_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, MAX_NON_REVERTIBLE_PUBLIC_DATA_READS_PER_TX
 },
     hash::{silo_note_hash, silo_nullifier, compute_l2_to_l1_hash, accumulate_sha256},
@@ -89,6 +90,7 @@ pub fn initialize_end_values(
     let start_non_revertible = previous_kernel.public_inputs.end_non_revertible;
     circuit_outputs.end_non_revertible.public_call_stack = array_to_bounded_vec(start_non_revertible.public_call_stack);
     circuit_outputs.end_non_revertible.nullifier_read_requests = array_to_bounded_vec(start_non_revertible.nullifier_read_requests);
+    circuit_outputs.end_non_revertible.nullifier_non_existent_read_requests = array_to_bounded_vec(start_non_revertible.nullifier_non_existent_read_requests);
 }
 
 fn perform_static_call_checks(public_call: PublicCallData) {
@@ -161,6 +163,7 @@ pub fn update_public_end_non_revertible_values(
     circuit_outputs.end_non_revertible.public_call_stack.extend_from_bounded_vec(public_call_requests);
 
     propagate_nullifier_read_requests_non_revertible(public_call, circuit_outputs);
+    propagate_nullifier_non_existent_read_requests_non_revertible(public_call, circuit_outputs);
     propagate_new_nullifiers_non_revertible(public_call, circuit_outputs);
     propagate_new_note_hashes_non_revertible(public_call, circuit_outputs);
     propagate_valid_non_revertible_public_data_update_requests(public_call, circuit_outputs);
@@ -182,6 +185,8 @@ pub fn update_public_end_values(public_call: PublicCallData, circuit_outputs: &m
     circuit_outputs.end.public_call_stack.extend_from_bounded_vec(public_call_requests);
 
     propagate_nullifier_read_requests_revertible(public_call, circuit_outputs);
+    propagate_nullifier_non_existent_read_requests_non_revertible(public_call, circuit_outputs); // TODO - Requests are not revertible and should be propagated to "validation_requests".
+
     propagate_new_nullifiers(public_call, circuit_outputs);
     propagate_new_note_hashes(public_call, circuit_outputs);
 
@@ -224,6 +229,22 @@ fn propagate_nullifier_read_requests_revertible<T>(
     }
 }
 
+fn propagate_nullifier_non_existent_read_requests_non_revertible<T>(
+    public_call: PublicCallData,
+    circuit_outputs: &mut PublicKernelCircuitPublicInputsBuilder
+) {
+    let public_call_public_inputs = public_call.call_stack_item.public_inputs;
+    let nullifier_non_existent_read_requests = public_call_public_inputs.nullifier_non_existent_read_requests;
+    let storage_contract_address = public_call_public_inputs.call_context.storage_contract_address;
+
+    for i in 0..MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL {
+        let request = nullifier_non_existent_read_requests[i];
+        if !is_empty(request) {
+            circuit_outputs.end_non_revertible.nullifier_non_existent_read_requests.push(request.to_context(storage_contract_address));
+        }
+    }
+}
+
 fn propagate_valid_public_data_update_requests(
     public_call: PublicCallData,
     circuit_outputs: &mut PublicKernelCircuitPublicInputsBuilder