refactor: counters (#7342)

- Check `min_revertible_side_effect_counter` against private call
requests emitted from any private function.
- Enforce `min_revertible_side_effect_counter` to be set when running
`private_tail_to_public`.
- Remove `side_efffect_counter` from `call_context` and simply refer to
the `start_side_effect_counter` in the private circuit public inputs.

barretenberg/cpp/pil/avm/constants_gen.pil

@@ -15,17 +15,17 @@ namespace constants(256);
     pol ADDRESS_SELECTOR = 1;
     pol STORAGE_ADDRESS_SELECTOR = 1;
     pol FUNCTION_SELECTOR_SELECTOR = 2;
-    pol START_GLOBAL_VARIABLES = 29;
-    pol CHAIN_ID_SELECTOR = 29;
-    pol VERSION_SELECTOR = 30;
-    pol BLOCK_NUMBER_SELECTOR = 31;
-    pol TIMESTAMP_SELECTOR = 32;
-    pol COINBASE_SELECTOR = 33;
-    pol FEE_PER_DA_GAS_SELECTOR = 35;
-    pol FEE_PER_L2_GAS_SELECTOR = 36;
-    pol END_GLOBAL_VARIABLES = 37;
-    pol START_SIDE_EFFECT_COUNTER = 37;
-    pol TRANSACTION_FEE_SELECTOR = 40;
+    pol START_GLOBAL_VARIABLES = 28;
+    pol CHAIN_ID_SELECTOR = 28;
+    pol VERSION_SELECTOR = 29;
+    pol BLOCK_NUMBER_SELECTOR = 30;
+    pol TIMESTAMP_SELECTOR = 31;
+    pol COINBASE_SELECTOR = 32;
+    pol FEE_PER_DA_GAS_SELECTOR = 34;
+    pol FEE_PER_L2_GAS_SELECTOR = 35;
+    pol END_GLOBAL_VARIABLES = 36;
+    pol START_SIDE_EFFECT_COUNTER = 36;
+    pol TRANSACTION_FEE_SELECTOR = 39;
     pol START_NOTE_HASH_EXISTS_WRITE_OFFSET = 0;
     pol START_NULLIFIER_EXISTS_OFFSET = 16;
     pol START_NULLIFIER_NON_EXISTS_OFFSET = 32;

barretenberg/cpp/src/barretenberg/relations/generated/avm/main.hpp

@@ -1227,56 +1227,56 @@ template <typename FF_> class mainImpl {
         // Contribution 118
         {
             Avm_DECLARE_VIEWS(118);
-            auto tmp = (main_sel_op_transaction_fee * (kernel_kernel_in_offset - FF(40)));
+            auto tmp = (main_sel_op_transaction_fee * (kernel_kernel_in_offset - FF(39)));
             tmp *= scaling_factor;
             std::get<118>(evals) += tmp;
         }
         // Contribution 119
         {
             Avm_DECLARE_VIEWS(119);
-            auto tmp = (main_sel_op_chain_id * (kernel_kernel_in_offset - FF(29)));
+            auto tmp = (main_sel_op_chain_id * (kernel_kernel_in_offset - FF(28)));
             tmp *= scaling_factor;
             std::get<119>(evals) += tmp;
         }
         // Contribution 120
         {
             Avm_DECLARE_VIEWS(120);
-            auto tmp = (main_sel_op_version * (kernel_kernel_in_offset - FF(30)));
+            auto tmp = (main_sel_op_version * (kernel_kernel_in_offset - FF(29)));
             tmp *= scaling_factor;
             std::get<120>(evals) += tmp;
         }
         // Contribution 121
         {
             Avm_DECLARE_VIEWS(121);
-            auto tmp = (main_sel_op_block_number * (kernel_kernel_in_offset - FF(31)));
+            auto tmp = (main_sel_op_block_number * (kernel_kernel_in_offset - FF(30)));
             tmp *= scaling_factor;
             std::get<121>(evals) += tmp;
         }
         // Contribution 122
         {
             Avm_DECLARE_VIEWS(122);
-            auto tmp = (main_sel_op_timestamp * (kernel_kernel_in_offset - FF(32)));
+            auto tmp = (main_sel_op_timestamp * (kernel_kernel_in_offset - FF(31)));
             tmp *= scaling_factor;
             std::get<122>(evals) += tmp;
         }
         // Contribution 123
         {
             Avm_DECLARE_VIEWS(123);
-            auto tmp = (main_sel_op_coinbase * (kernel_kernel_in_offset - FF(33)));
+            auto tmp = (main_sel_op_coinbase * (kernel_kernel_in_offset - FF(32)));
             tmp *= scaling_factor;
             std::get<123>(evals) += tmp;
         }
         // Contribution 124
         {
             Avm_DECLARE_VIEWS(124);
-            auto tmp = (main_sel_op_fee_per_da_gas * (kernel_kernel_in_offset - FF(35)));
+            auto tmp = (main_sel_op_fee_per_da_gas * (kernel_kernel_in_offset - FF(34)));
             tmp *= scaling_factor;
             std::get<124>(evals) += tmp;
         }
         // Contribution 125
         {
             Avm_DECLARE_VIEWS(125);
-            auto tmp = (main_sel_op_fee_per_l2_gas * (kernel_kernel_in_offset - FF(36)));
+            auto tmp = (main_sel_op_fee_per_l2_gas * (kernel_kernel_in_offset - FF(35)));
             tmp *= scaling_factor;
             std::get<125>(evals) += tmp;
         }

barretenberg/cpp/src/barretenberg/vm/avm_trace/aztec_constants.hpp

@@ -15,7 +15,7 @@
 #define AZTEC_ADDRESS_LENGTH 1
 #define GAS_FEES_LENGTH 2
 #define GAS_LENGTH 2
-#define CALL_CONTEXT_LENGTH 6
+#define CALL_CONTEXT_LENGTH 5
 #define CONTENT_COMMITMENT_LENGTH 4
 #define CONTRACT_STORAGE_READ_LENGTH 3
 #define CONTRACT_STORAGE_UPDATE_REQUEST_LENGTH 3
@@ -30,23 +30,23 @@
 #define STATE_REFERENCE_LENGTH 8
 #define TOTAL_FEES_LENGTH 1
 #define HEADER_LENGTH 23
-#define PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH 482
-#define PUBLIC_CONTEXT_INPUTS_LENGTH 41
+#define PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH 481
+#define PUBLIC_CONTEXT_INPUTS_LENGTH 40
 #define SENDER_SELECTOR 0
 #define ADDRESS_SELECTOR 1
 #define STORAGE_ADDRESS_SELECTOR 1
 #define FUNCTION_SELECTOR_SELECTOR 2
-#define START_GLOBAL_VARIABLES 29
-#define CHAIN_ID_SELECTOR 29
-#define VERSION_SELECTOR 30
-#define BLOCK_NUMBER_SELECTOR 31
-#define TIMESTAMP_SELECTOR 32
-#define COINBASE_SELECTOR 33
-#define FEE_PER_DA_GAS_SELECTOR 35
-#define FEE_PER_L2_GAS_SELECTOR 36
-#define END_GLOBAL_VARIABLES 37
-#define START_SIDE_EFFECT_COUNTER 37
-#define TRANSACTION_FEE_SELECTOR 40
+#define START_GLOBAL_VARIABLES 28
+#define CHAIN_ID_SELECTOR 28
+#define VERSION_SELECTOR 29
+#define BLOCK_NUMBER_SELECTOR 30
+#define TIMESTAMP_SELECTOR 31
+#define COINBASE_SELECTOR 32
+#define FEE_PER_DA_GAS_SELECTOR 34
+#define FEE_PER_L2_GAS_SELECTOR 35
+#define END_GLOBAL_VARIABLES 36
+#define START_SIDE_EFFECT_COUNTER 36
+#define TRANSACTION_FEE_SELECTOR 39
 #define START_NOTE_HASH_EXISTS_WRITE_OFFSET 0
 #define START_NULLIFIER_EXISTS_OFFSET 16
 #define START_NULLIFIER_NON_EXISTS_OFFSET 32

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

@@ -135,7 +135,7 @@ library Constants {
   uint256 internal constant GAS_FEES_LENGTH = 2;
   uint256 internal constant GAS_LENGTH = 2;
   uint256 internal constant GAS_SETTINGS_LENGTH = 7;
-  uint256 internal constant CALL_CONTEXT_LENGTH = 6;
+  uint256 internal constant CALL_CONTEXT_LENGTH = 5;
   uint256 internal constant CONTENT_COMMITMENT_LENGTH = 4;
   uint256 internal constant CONTRACT_INSTANCE_LENGTH = 5;
   uint256 internal constant CONTRACT_STORAGE_READ_LENGTH = 3;
@@ -164,29 +164,29 @@ library Constants {
   uint256 internal constant NULLIFIER_LENGTH = 3;
   uint256 internal constant SCOPED_NULLIFIER_LENGTH = 4;
   uint256 internal constant CALLER_CONTEXT_LENGTH = 3;
-  uint256 internal constant PRIVATE_CALL_REQUEST_LENGTH = 16;
-  uint256 internal constant SCOPED_PRIVATE_CALL_REQUEST_LENGTH = 17;
+  uint256 internal constant PRIVATE_CALL_REQUEST_LENGTH = 15;
+  uint256 internal constant SCOPED_PRIVATE_CALL_REQUEST_LENGTH = 16;
   uint256 internal constant ROLLUP_VALIDATION_REQUESTS_LENGTH = 2;
   uint256 internal constant STATE_REFERENCE_LENGTH = 8;
   uint256 internal constant TX_CONTEXT_LENGTH = 9;
   uint256 internal constant TX_REQUEST_LENGTH = 13;
   uint256 internal constant TOTAL_FEES_LENGTH = 1;
   uint256 internal constant HEADER_LENGTH = 23;
-  uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 433;
-  uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 482;
-  uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 436;
-  uint256 internal constant PUBLIC_CONTEXT_INPUTS_LENGTH = 41;
+  uint256 internal constant PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH = 428;
+  uint256 internal constant PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 481;
+  uint256 internal constant PRIVATE_CALL_STACK_ITEM_LENGTH = 431;
+  uint256 internal constant PUBLIC_CONTEXT_INPUTS_LENGTH = 40;
   uint256 internal constant AGGREGATION_OBJECT_LENGTH = 16;
   uint256 internal constant SCOPED_READ_REQUEST_LEN = 3;
   uint256 internal constant PUBLIC_DATA_READ_LENGTH = 2;
   uint256 internal constant VALIDATION_REQUESTS_LENGTH = 1026;
   uint256 internal constant PUBLIC_DATA_UPDATE_REQUEST_LENGTH = 3;
   uint256 internal constant COMBINED_ACCUMULATED_DATA_LENGTH = 333;
   uint256 internal constant COMBINED_CONSTANT_DATA_LENGTH = 41;
-  uint256 internal constant PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH = 16;
+  uint256 internal constant PUBLIC_CALL_STACK_ITEM_COMPRESSED_LENGTH = 15;
   uint256 internal constant CALL_REQUEST_LENGTH = 7;
-  uint256 internal constant PRIVATE_ACCUMULATED_DATA_LENGTH = 1168;
-  uint256 internal constant PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2244;
+  uint256 internal constant PRIVATE_ACCUMULATED_DATA_LENGTH = 1160;
+  uint256 internal constant PRIVATE_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 2236;
   uint256 internal constant PUBLIC_ACCUMULATED_DATA_LENGTH = 983;
   uint256 internal constant PUBLIC_KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 3259;
   uint256 internal constant KERNEL_CIRCUIT_PUBLIC_INPUTS_LENGTH = 384;
@@ -211,18 +211,18 @@ library Constants {
   uint256 internal constant ADDRESS_SELECTOR = 1;
   uint256 internal constant STORAGE_ADDRESS_SELECTOR = 1;
   uint256 internal constant FUNCTION_SELECTOR_SELECTOR = 2;
-  uint256 internal constant START_GLOBAL_VARIABLES = 29;
-  uint256 internal constant CHAIN_ID_SELECTOR = 29;
-  uint256 internal constant VERSION_SELECTOR = 30;
-  uint256 internal constant BLOCK_NUMBER_SELECTOR = 31;
-  uint256 internal constant TIMESTAMP_SELECTOR = 32;
-  uint256 internal constant COINBASE_SELECTOR = 33;
-  uint256 internal constant UNUSED_FEE_RECIPIENT_SELECTOR = 34;
-  uint256 internal constant FEE_PER_DA_GAS_SELECTOR = 35;
-  uint256 internal constant FEE_PER_L2_GAS_SELECTOR = 36;
-  uint256 internal constant END_GLOBAL_VARIABLES = 37;
-  uint256 internal constant START_SIDE_EFFECT_COUNTER = 37;
-  uint256 internal constant TRANSACTION_FEE_SELECTOR = 40;
+  uint256 internal constant START_GLOBAL_VARIABLES = 28;
+  uint256 internal constant CHAIN_ID_SELECTOR = 28;
+  uint256 internal constant VERSION_SELECTOR = 29;
+  uint256 internal constant BLOCK_NUMBER_SELECTOR = 30;
+  uint256 internal constant TIMESTAMP_SELECTOR = 31;
+  uint256 internal constant COINBASE_SELECTOR = 32;
+  uint256 internal constant UNUSED_FEE_RECIPIENT_SELECTOR = 33;
+  uint256 internal constant FEE_PER_DA_GAS_SELECTOR = 34;
+  uint256 internal constant FEE_PER_L2_GAS_SELECTOR = 35;
+  uint256 internal constant END_GLOBAL_VARIABLES = 36;
+  uint256 internal constant START_SIDE_EFFECT_COUNTER = 36;
+  uint256 internal constant TRANSACTION_FEE_SELECTOR = 39;
   uint256 internal constant START_NOTE_HASH_EXISTS_WRITE_OFFSET = 0;
   uint256 internal constant START_NULLIFIER_EXISTS_OFFSET = 16;
   uint256 internal constant START_NULLIFIER_NON_EXISTS_OFFSET = 32;

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

@@ -357,7 +357,6 @@ impl PrivateContext {
             is_delegate_call
         );
 
-        assert_eq(item.public_inputs.call_context.side_effect_counter, start_side_effect_counter);
         assert_eq(item.public_inputs.start_side_effect_counter, start_side_effect_counter);
         let end_side_effect_counter = item.public_inputs.end_side_effect_counter;
         self.side_effect_counter = end_side_effect_counter + 1;
@@ -564,7 +563,7 @@ impl PrivateContext {
         assert(contract_address.eq(item.contract_address));
         assert(function_selector.eq(item.function_data.selector));
 
-        assert_eq(item.public_inputs.call_context.side_effect_counter, self.side_effect_counter);
+        assert_eq(item.public_inputs.start_side_effect_counter, self.side_effect_counter);
 
         assert(args_hash == item.public_inputs.args_hash);
 

noir-projects/aztec-nr/aztec/src/oracle/enqueue_public_function_call.nr

@@ -85,6 +85,7 @@ pub fn parse_public_call_stack_item_from_oracle(fields: [Field; ENQUEUE_PUBLIC_F
         function_data: FunctionData { selector: FunctionSelector::from_field(reader.read()), is_private: false },
         public_inputs: PublicCircuitPublicInputs {
             call_context: reader.read_struct(CallContext::deserialize),
+            start_side_effect_counter: reader.read_u32(),
             args_hash: reader.read(),
             returns_hash: 0,
             note_hash_read_requests: [ReadRequest::empty(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL],
@@ -97,7 +98,6 @@ pub fn parse_public_call_stack_item_from_oracle(fields: [Field; ENQUEUE_PUBLIC_F
             note_hashes: [NoteHash::empty(); MAX_NOTE_HASHES_PER_CALL],
             nullifiers: [Nullifier::empty(); MAX_NULLIFIERS_PER_CALL],
             l2_to_l1_msgs: [L2ToL1Message::empty(); MAX_L2_TO_L1_MSGS_PER_CALL],
-            start_side_effect_counter: 0,
             end_side_effect_counter: 0,
             unencrypted_logs_hashes: [LogHash::empty(); MAX_UNENCRYPTED_LOGS_PER_CALL],
             historical_header: Header::empty(),

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

@@ -121,17 +121,6 @@ impl PrivateCallDataValidator {
         let call_context = public_inputs.call_context;
         assert(call_context.is_delegate_call == false, "Users cannot make a delegatecall");
         assert(call_context.is_static_call == false, "Users cannot make a static call");
-
-        let min_revertible_side_effect_counter = public_inputs.min_revertible_side_effect_counter;
-        let first_revertible_index = find_first_revertible_private_call_request_index(public_inputs);
-        // No need to check that the min_revertible_side_effect_counter falls in the counter range of the private call.
-        // It is valid as long as it does not fall in the middle of any nested call.
-        validate_split_ranges(
-            min_revertible_side_effect_counter,
-            first_revertible_index,
-            public_inputs.private_call_requests,
-            self.array_lengths.private_call_requests
-        );
     }
 
     // Confirm that the TxRequest (user's intent) matches the private call being executed.
@@ -263,18 +252,34 @@ impl PrivateCallDataValidator {
     }
 
     fn validate_private_call_requests(self) {
-        let call_requests = self.data.call_stack_item.public_inputs.private_call_requests;
+        let public_inputs = self.data.call_stack_item.public_inputs;
+        let call_requests = public_inputs.private_call_requests;
         let num_requests = self.array_lengths.private_call_requests;
         let mut should_check = true;
         for i in 0..call_requests.len() {
             should_check &= i != num_requests;
             if should_check {
-                validate_caller_context(
-                    call_requests[i].caller_context,
-                    self.data.call_stack_item.public_inputs.call_context
-                );
+                validate_caller_context(call_requests[i].caller_context, public_inputs.call_context);
             }
         }
+
+        // Check that the min_revertible_side_effect_counter does not fall in the middle of any nested calls.
+        // If it is possible, one can create a custom account contract, set the min_revertible_side_effect_counter to a
+        // value that falls in a transfer function, make the tx revert which then preserves the note hashes and discards 
+        // the nullifiers.
+        // 
+        // We don't have to use the aggregated min_revertible_side_effect_counter in the output for the below check.
+        // If the current call's min_revertible_side_effect_counter is 0, it means the counter might be set by another
+        // function. This check for that function guarantees that the counter won't fall into one of its nested calls. 
+        // In this case, we can simply use 0 and make the following check pass.
+        let min_revertible_side_effect_counter = public_inputs.min_revertible_side_effect_counter;
+        let first_revertible_index = find_first_revertible_private_call_request_index(public_inputs);
+        validate_split_ranges(
+            min_revertible_side_effect_counter,
+            first_revertible_index,
+            public_inputs.private_call_requests,
+            self.array_lengths.private_call_requests
+        );
     }
 
     fn validate_public_call_requests(self) {

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

@@ -9,6 +9,8 @@ pub fn split_to_public(
     data: PrivateAccumulatedDataBuilder,
     min_revertible_side_effect_counter: u32
 ) -> (PublicAccumulatedDataBuilder, PublicAccumulatedDataBuilder) {
+    assert(min_revertible_side_effect_counter != 0, "min_revertible_side_effect_counter must not be 0");
+
     let mut non_revertible_builder = PublicAccumulatedDataBuilder::empty();
     let mut revertible_builder = PublicAccumulatedDataBuilder::empty();
 
@@ -26,10 +28,7 @@ pub fn split_to_public(
     }
 
     let nullifiers = data.nullifiers;
-    // First nullifier is always non-revertible.
-    let first_nullifier = nullifiers.get_unchecked(0).expose_to_public();
-    non_revertible_builder.nullifiers.push(first_nullifier);
-    for i in 1..nullifiers.max_len() {
+    for i in 0..nullifiers.max_len() {
         if i < nullifiers.len() {
             let nullifier = nullifiers.get_unchecked(i);
             let public_nullifier = nullifier.expose_to_public();

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

@@ -62,6 +62,8 @@ impl TailToPublicOutputValidator {
 
     fn validate_propagated_sorted_siloed_values(self, hints: TailToPublicOutputHints) {
         let split_counter = self.previous_kernel.min_revertible_side_effect_counter;
+        assert(split_counter != 0, "min_revertible_side_effect_counter must not be 0");
+
         let prev_data = self.previous_kernel.end;
         let output_non_revertible = self.output.end_non_revertible;
         let output_revertible = self.output.end;

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

@@ -77,6 +77,7 @@ mod tests {
             let mut previous_kernel = FixtureBuilder::new().in_vk_tree(PRIVATE_KERNEL_INNER_INDEX);
             previous_kernel.tx_context.gas_settings.gas_limits = Gas::new(1_000_000, 1_000_000);
             previous_kernel.set_first_nullifier();
+            previous_kernel.end_setup();
             previous_kernel.push_public_call_request(1, false);
 
             PrivateKernelTailToPublicInputsBuilder { previous_kernel }
@@ -170,6 +171,13 @@ mod tests {
         builder.failed();
     }
 
+    #[test(should_fail_with="min_revertible_side_effect_counter must not be 0")]
+    fn zero_min_revertible_side_effect_counter_fails() {
+        let mut builder = PrivateKernelTailToPublicInputsBuilder::new();
+        builder.previous_kernel.min_revertible_side_effect_counter = 0;
+        builder.failed();
+    }
+
     #[test(should_fail_with="Must have public calls when exporting public kernel data from the tail circuit")]
     fn no_public_calls_should_fail() {
         let mut builder = PrivateKernelTailToPublicInputsBuilder::new();