fix: Validation requests (#5236)

Data like note hash read requests, nullifier read requests, etc, are not revertible. When a tx reverts, we still need to verify those requests in the kernel. Moving them out of `(Non)RevertibleAccumulcatedData` and creating a new `ValidationRequests` for aggregating these data.
AztecProtocol · Mar 15, 2024 · 25ce33b · 25ce33b
1 parent fed70a1
commit 25ce33b
Show file tree

Hide file tree

Showing 48 changed files with 997 additions and 1,049 deletions.
diff --git a/l1-contracts/slither_output.md b/l1-contracts/slither_output.md
@@ -329,15 +329,15 @@ src/core/messagebridge/Inbox.sol#L148-L153
 Impact: Informational
 Confidence: Medium
  - [ ] ID-36
-Variable [Constants.LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L132) is too similar to [Constants.NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L125)
+Variable [Constants.LOGS_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L130) is too similar to [Constants.NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP](src/core/libraries/ConstantsGen.sol#L123)
 
-src/core/libraries/ConstantsGen.sol#L132
+src/core/libraries/ConstantsGen.sol#L130
 
 
  - [ ] ID-37
-Variable [Constants.L1_TO_L2_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L112) is too similar to [Constants.L2_TO_L1_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L113)
+Variable [Constants.L1_TO_L2_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L110) is too similar to [Constants.L2_TO_L1_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L111)
 
-src/core/libraries/ConstantsGen.sol#L112
+src/core/libraries/ConstantsGen.sol#L110
 
 
  - [ ] ID-38

diff --git a/l1-contracts/src/core/libraries/ConstantsGen.sol b/l1-contracts/src/core/libraries/ConstantsGen.sol
@@ -41,8 +41,6 @@ library Constants {
   uint256 internal constant MAX_NON_REVERTIBLE_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX = 16;
   uint256 internal constant MAX_REVERTIBLE_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX = 16;
   uint256 internal constant MAX_PUBLIC_DATA_READS_PER_TX = 32;
-  uint256 internal constant MAX_NON_REVERTIBLE_PUBLIC_DATA_READS_PER_TX = 16;
-  uint256 internal constant MAX_REVERTIBLE_PUBLIC_DATA_READS_PER_TX = 16;
   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;

diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/common.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/common.nr
@@ -5,8 +5,7 @@ use dep::types::{
     kernel_circuit_public_inputs::PrivateKernelCircuitPublicInputsBuilder,
     membership_witness::NoteHashReadRequestMembershipWitness,
     private_circuit_public_inputs::PrivateCircuitPublicInputs,
-    private_kernel::private_call_data::PrivateCallData,
-    kernel_data::{PrivateKernelInnerData, PrivateKernelTailData},
+    private_kernel::private_call_data::PrivateCallData, kernel_data::{PrivateKernelInnerData},
     side_effect::{SideEffect, SideEffectLinkedToNoteHash}
 },
     address::{AztecAddress, EthAddress, PartialAddress, compute_initialization_hash},
@@ -92,14 +91,15 @@ pub fn initialize_end_values(
     public_inputs.constants = previous_kernel.public_inputs.constants;
     public_inputs.min_revertible_side_effect_counter = previous_kernel.public_inputs.min_revertible_side_effect_counter;
 
+    let start = previous_kernel.public_inputs.validation_requests;
+    public_inputs.validation_requests.note_hash_read_requests = array_to_bounded_vec(start.note_hash_read_requests);
+    public_inputs.validation_requests.nullifier_read_requests = array_to_bounded_vec(start.nullifier_read_requests);
+    public_inputs.validation_requests.nullifier_key_validation_requests = array_to_bounded_vec(start.nullifier_key_validation_requests);
+
     // Ensure the arrays are the same as previously, before we start pushing more data onto them in other
     // functions within this circuit:
     let start = previous_kernel.public_inputs.end;
 
-    public_inputs.end.note_hash_read_requests = array_to_bounded_vec(start.note_hash_read_requests);
-    public_inputs.end.nullifier_read_requests = array_to_bounded_vec(start.nullifier_read_requests);
-    public_inputs.end.nullifier_key_validation_requests = array_to_bounded_vec(start.nullifier_key_validation_requests);
-
     public_inputs.end.new_note_hashes = array_to_bounded_vec(start.new_note_hashes);
     public_inputs.end.new_nullifiers = array_to_bounded_vec(start.new_nullifiers);
 
@@ -194,14 +194,14 @@ pub fn update_end_values(
             )
         }
     }
-    public_inputs.end.note_hash_read_requests.extend_from_bounded_vec(siloed_read_requests);
+    public_inputs.validation_requests.note_hash_read_requests.extend_from_bounded_vec(siloed_read_requests);
 
     // Nullifier read request.
     let nullifier_read_requests = private_call_public_inputs.nullifier_read_requests;
     for i in 0..MAX_NULLIFIER_READ_REQUESTS_PER_CALL {
         let request = nullifier_read_requests[i];
         if !is_empty(request) {
-            public_inputs.end.nullifier_read_requests.push(request.to_context(storage_contract_address));
+            public_inputs.validation_requests.nullifier_read_requests.push(request.to_context(storage_contract_address));
         }
     }
 
@@ -210,7 +210,7 @@ pub fn update_end_values(
     for i in 0..MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_CALL {
         let request = nullifier_key_validation_requests[i];
         if !is_empty(request) {
-            public_inputs.end.nullifier_key_validation_requests.push(request.to_context(storage_contract_address));
+            public_inputs.validation_requests.nullifier_key_validation_requests.push(request.to_context(storage_contract_address));
         }
     }
 

diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_init.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_init.nr
@@ -410,7 +410,7 @@ mod tests {
         assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash());
 
         // non-transient read requests are NOT forwarded
-        assert_eq(array_length(public_inputs.end.note_hash_read_requests), 0);
+        assert_eq(array_length(public_inputs.validation_requests.note_hash_read_requests), 0);
     }
 
     #[test]
@@ -425,7 +425,7 @@ mod tests {
         assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash());
 
         // non-transient read requests are NOT forwarded
-        assert_eq(array_length(public_inputs.end.note_hash_read_requests), 0);
+        assert_eq(array_length(public_inputs.validation_requests.note_hash_read_requests), 0);
     }
 
     #[test]
@@ -440,7 +440,7 @@ mod tests {
         assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash());
 
         // non-transient read requests are NOT forwarded
-        assert_eq(array_length(public_inputs.end.note_hash_read_requests), 0);
+        assert_eq(array_length(public_inputs.validation_requests.note_hash_read_requests), 0);
     }
 
     #[test]
@@ -455,7 +455,7 @@ mod tests {
         assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash());
 
         // non-transient read requests are NOT forwarded
-        assert_eq(array_length(public_inputs.end.note_hash_read_requests), 0);
+        assert_eq(array_length(public_inputs.validation_requests.note_hash_read_requests), 0);
     }
 
     #[test]
@@ -470,7 +470,7 @@ mod tests {
         assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash());
 
         // non-transient read requests are NOT forwarded
-        assert_eq(array_length(public_inputs.end.note_hash_read_requests), 1);
+        assert_eq(array_length(public_inputs.validation_requests.note_hash_read_requests), 1);
     }
 
     #[test]
@@ -486,7 +486,7 @@ mod tests {
         assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash());
 
         // non-transient read requests are NOT forwarded
-        assert_eq(array_length(public_inputs.end.note_hash_read_requests), 1);
+        assert_eq(array_length(public_inputs.validation_requests.note_hash_read_requests), 1);
     }
 
     #[test]
@@ -502,7 +502,7 @@ mod tests {
 
         // non-transient read requests are NOT forwarded
         assert_eq(
-            array_length(public_inputs.end.note_hash_read_requests), MAX_NOTE_HASH_READ_REQUESTS_PER_CALL
+            array_length(public_inputs.validation_requests.note_hash_read_requests), MAX_NOTE_HASH_READ_REQUESTS_PER_CALL
         );
     }
 
@@ -518,7 +518,7 @@ mod tests {
 
         let public_inputs = builder.execute();
 
-        let end_nullifier_read_requests = public_inputs.end.nullifier_read_requests;
+        let end_nullifier_read_requests = public_inputs.validation_requests.nullifier_read_requests;
         assert_eq(array_length(end_nullifier_read_requests), 2);
 
         let request_context = end_nullifier_read_requests[0];
@@ -541,9 +541,9 @@ mod tests {
 
         let public_inputs = builder.execute();
 
-        assert_eq(array_length(public_inputs.end.nullifier_key_validation_requests), 1);
+        assert_eq(array_length(public_inputs.validation_requests.nullifier_key_validation_requests), 1);
 
-        let request_context = public_inputs.end.nullifier_key_validation_requests[0];
+        let request_context = public_inputs.validation_requests.nullifier_key_validation_requests[0];
         assert_eq(request_context.public_key, request.public_key);
         assert_eq(request_context.secret_key, request.secret_key);
         assert_eq(

diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_inner.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_inner.nr
@@ -583,7 +583,7 @@ mod tests {
         let public_inputs = builder.execute();
 
         // non-transient read requests are NOT forwarded
-        assert_eq(array_length(public_inputs.end.note_hash_read_requests), 0);
+        assert_eq(array_length(public_inputs.validation_requests.note_hash_read_requests), 0);
     }
 
     #[test]
@@ -595,7 +595,7 @@ mod tests {
         let public_inputs = builder.execute();
 
         // non-transient read requests are NOT forwarded
-        assert_eq(array_length(public_inputs.end.note_hash_read_requests), 0);
+        assert_eq(array_length(public_inputs.validation_requests.note_hash_read_requests), 0);
     }
 
     #[test]
@@ -607,7 +607,7 @@ mod tests {
         let public_inputs = builder.execute();
 
         // non-transient read requests are NOT forwarded
-        assert_eq(array_length(public_inputs.end.note_hash_read_requests), 0);
+        assert_eq(array_length(public_inputs.validation_requests.note_hash_read_requests), 0);
     }
 
     #[test]
@@ -619,7 +619,7 @@ mod tests {
         let public_inputs = builder.execute();
 
         // non-transient read requests are NOT forwarded
-        assert_eq(array_length(public_inputs.end.note_hash_read_requests), 0);
+        assert_eq(array_length(public_inputs.validation_requests.note_hash_read_requests), 0);
     }
 
     #[test]
@@ -631,7 +631,7 @@ mod tests {
         let public_inputs = builder.execute();
 
         // non-transient read requests are NOT forwarded
-        assert_eq(array_length(public_inputs.end.note_hash_read_requests), 1);
+        assert_eq(array_length(public_inputs.validation_requests.note_hash_read_requests), 1);
     }
 
     #[test]
@@ -645,7 +645,7 @@ mod tests {
         let public_inputs = builder.execute();
 
         // non-transient read requests are NOT forwarded
-        assert_eq(array_length(public_inputs.end.note_hash_read_requests), 1);
+        assert_eq(array_length(public_inputs.validation_requests.note_hash_read_requests), 1);
     }
 
     #[test]
@@ -658,7 +658,7 @@ mod tests {
 
         // non-transient read requests are NOT forwarded
         assert_eq(
-            array_length(public_inputs.end.note_hash_read_requests), MAX_NOTE_HASH_READ_REQUESTS_PER_CALL
+            array_length(public_inputs.validation_requests.note_hash_read_requests), MAX_NOTE_HASH_READ_REQUESTS_PER_CALL
         );
     }
 

diff --git a/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail.nr b/noir-projects/noir-protocol-circuits/crates/private-kernel-lib/src/private_kernel_tail.nr
@@ -4,7 +4,7 @@ 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_data::PrivateKernelInnerData,
     kernel_circuit_public_inputs::{PrivateKernelCircuitPublicInputsBuilder, PrivateKernelTailCircuitPublicInputs},
     side_effect::{SideEffect, SideEffectLinkedToNoteHash, Ordered}
 },
@@ -38,15 +38,15 @@ impl PrivateKernelTailCircuitPrivateInputs {
     }
 
     fn validate_nullifier_read_requests(self, public_inputs: &mut PrivateKernelCircuitPublicInputsBuilder) {
-        let requests = self.previous_kernel.public_inputs.end.nullifier_read_requests;
+        let requests = self.previous_kernel.public_inputs.validation_requests.nullifier_read_requests;
 
         let pending_nullifiers = self.previous_kernel.public_inputs.end.new_nullifiers;
 
         let hints = self.nullifier_read_request_hints;
 
         let nullifier_tree_root = public_inputs.constants.historical_header.state.partial.nullifier_tree.root;
 
-        public_inputs.end.nullifier_read_requests = reset_read_requests(
+        public_inputs.validation_requests.nullifier_read_requests = reset_read_requests(
             requests,
             pending_nullifiers,
             hints.read_request_statuses,
@@ -58,12 +58,12 @@ impl PrivateKernelTailCircuitPrivateInputs {
         // corresponding values are added to public inputs in nested executions.
         // But right now, all the request must be cleared in one go.
         assert(
-            public_inputs.end.nullifier_read_requests.len() == 0, "All nullifier read requests must be verified"
+            public_inputs.validation_requests.nullifier_read_requests.len() == 0, "All nullifier read requests must be verified"
         );
     }
 
     fn validate_nullifier_keys(self, public_inputs: &mut PrivateKernelCircuitPublicInputsBuilder) {
-        let requests = self.previous_kernel.public_inputs.end.nullifier_key_validation_requests;
+        let requests = self.previous_kernel.public_inputs.validation_requests.nullifier_key_validation_requests;
         for i in 0..MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_TX {
             let request = requests[i];
             if !is_empty(request) {
@@ -81,12 +81,12 @@ impl PrivateKernelTailCircuitPrivateInputs {
         }
 
         // Empty out nullifier key validation requests after verifying them.
-        public_inputs.end.nullifier_key_validation_requests = BoundedVec::new();
+        public_inputs.validation_requests.nullifier_key_validation_requests = BoundedVec::new();
     }
 
     fn match_reads_to_commitments(self, public_inputs: &mut PrivateKernelCircuitPublicInputsBuilder) {
         let new_note_hashes = public_inputs.end.new_note_hashes;
-        let read_requests = public_inputs.end.note_hash_read_requests;
+        let read_requests = public_inputs.validation_requests.note_hash_read_requests;
 
         // match reads to commitments from the previous call(s)
         for rr_idx in 0..MAX_NOTE_HASH_READ_REQUESTS_PER_TX {
@@ -103,7 +103,7 @@ impl PrivateKernelTailCircuitPrivateInputs {
         }
 
         // Empty out read requests after matching them to commitments
-        public_inputs.end.note_hash_read_requests = BoundedVec::new();
+        public_inputs.validation_requests.note_hash_read_requests = BoundedVec::new();
     }
 
     fn assert_sorted_counters<T, N>(original: [T; N], sorted: [T; N], indexes: [u64; N]) where T: Eq + Ordered + Empty {
@@ -447,8 +447,8 @@ mod tests {
         builder.append_transient_commitments(1);
         builder.add_transient_read(0);
         // Tweak the read request so that it does not match the hash at index 0;
-        let read_request = builder.previous_kernel.end.note_hash_read_requests.pop();
-        builder.previous_kernel.end.note_hash_read_requests.push(SideEffect { value: read_request.value + 1, counter: 0 });
+        let read_request = builder.previous_kernel.validation_requests.note_hash_read_requests.pop();
+        builder.previous_kernel.validation_requests.note_hash_read_requests.push(SideEffect { value: read_request.value + 1, counter: 0 });
         builder.failed();
     }
 
@@ -494,9 +494,9 @@ mod tests {
         builder.append_nullifiers(3);
         builder.add_nullifier_pending_read(1);
         let nullifier_being_read = builder.get_new_nullifiers()[2];
-        let mut read_request = builder.previous_kernel.end.nullifier_read_requests.pop();
+        let mut read_request = builder.previous_kernel.validation_requests.nullifier_read_requests.pop();
         read_request.counter = nullifier_being_read.counter - 1;
-        builder.previous_kernel.end.nullifier_read_requests.push(read_request);
+        builder.previous_kernel.validation_requests.nullifier_read_requests.push(read_request);
 
         builder.failed();
     }