feat: non revertible effects and tx phases (#4629)

There are 3 primary changes in this PR.

1. Within the private kernel tail, split the side effects that have been
accumulated during private execution into `end`, and
`end_non_revertible` based on their side effect counters compared with
`min_revertible_side_effect_counter`.
2. Examine those structs on the sequencer and run circuits for setup,
app logic, and teardown accordingly.
3. Recombine `end` and `end_non_revertible` at the beginning of the
rollup base.

To accomplish this, new structs are introduced.

`CombinedAccumulatedData` is unchanged, and holds side effects that are
both revertible and non-revertible.

For this reason, it is part of `PrivateKernelInnerCircuitPublicInputs`,
and `RollupKernelCircuitPublicInputs`.

`PrivateKernelTailCircuitPublicInputs` has been updated to hold 
```
    end_non_revertible: PrivateAccumulatedNonRevertibleData,
    end: PrivateAccumulatedRevertibleData,
```
and `PublicKernelCircuitPublicInputs` has been updated to hold
```
    end_non_revertible: PublicAccumulatedNonRevertibleData,
    end: PublicAccumulatedRevertibleData,
```
The difference between `PrivateAccumulated` and `PublicAccumulated` is
that the `Private` flavors cannot hold `public_data_update_requests` or
`public_data_reads`.

Additionally, for the types of side effects that can be non-revertible
(which at present are commitments, nullifiers, public data updates,
public data reads), there are new constants defined, e.g.
`MAX_NON_REVERTIBLE_COMMITMENTS_PER_TX` and
`MAX_REVERTIBLE_COMMITMENTS_PER_TX`.

These two sizes must sum to `MAX_NEW_COMMITMENTS_PER_TX`, and similarly
for the other types.

Thus, the bulk of the added work done by circuits is performed in
`combined_accumulated_data.nr`. We expect there are significant
optimizations that can be made to our initial implementation of
splitting and recombining.

Note, the decision was made to recombine in the base rollup to reduce
conditional logic and reuse existing machinery that wants to look at a
single `CombinedAccumulatedData`. Further, it made architectural sense
in that the rollup should not need to know/care that certain e.g.
nullifiers submitted were `nonRevertible`: it received the nullifiers,
thus they must not have reverted. The recombine could have been
performed by the public kernel, but we didn't want to add an obligatory
circuit for the sequencer if, e.g. they were submitted a fully private
transaction (since they would always be obliged to recombine the TX
nullifier which is now part of the non-revertible side effects).

To handle the non revertible side effects, we repurposed the public
kernel circuits to correspond with the "phases" of a transaction:
1. setup
2. app logic
3. teardown

In order to ensure that the phases are correctly followed, we introduce
3 more members to `PrivateKernelTailCircuitPublicInputs` and
`PublicKernelCircuitPublicInputs`:

```
    needs_setup: bool, // true if there is more than 1 enqueued non-revertible public call
    needs_app_logic: bool, // true if there is more than 0 enqueued revertible public calls
    needs_teardown: bool, // true if there is more than 0 enqueued non-revertible public calls
```

The semantic is that for enqueued non-revertible public calls, the call
that is enqueued last is for teardown (e.g. pay the fee), and all other
are for setup (e.g. engage with some fee payment contract). See
`AbstractPhaseManager.extractEnqueuedPublicCallsByPhase`.

Note further that we haven't added new e2e tests here, as the interface
for using this is sitting in [this
PR](#4543)

Co-authored-by: Alex Gherghisan <[email protected]>

l1-contracts/slither_output.md

@@ -353,9 +353,9 @@ src/core/messagebridge/Inbox.sol#L148-L153
 Impact: Informational
 Confidence: Medium
  - [ ] ID-41
-Variable [Constants.L1_TO_L2_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L96) is too similar to [Constants.L2_TO_L1_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L97)
+Variable [Constants.L1_TO_L2_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L103) is too similar to [Constants.L2_TO_L1_MESSAGE_LENGTH](src/core/libraries/ConstantsGen.sol#L104)
 
-src/core/libraries/ConstantsGen.sol#L96
+src/core/libraries/ConstantsGen.sol#L103
 
 
  - [ ] ID-42

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

@@ -26,20 +26,27 @@ library Constants {
   uint256 internal constant MAX_READ_REQUESTS_PER_CALL = 32;
   uint256 internal constant MAX_NULLIFIER_KEY_VALIDATION_REQUESTS_PER_CALL = 1;
   uint256 internal constant MAX_NEW_COMMITMENTS_PER_TX = 64;
+  uint256 internal constant MAX_NON_REVERTIBLE_COMMITMENTS_PER_TX = 8;
+  uint256 internal constant MAX_REVERTIBLE_COMMITMENTS_PER_TX = 56;
   uint256 internal constant MAX_NEW_NULLIFIERS_PER_TX = 64;
+  uint256 internal constant MAX_NON_REVERTIBLE_NULLIFIERS_PER_TX = 8;
+  uint256 internal constant MAX_REVERTIBLE_NULLIFIERS_PER_TX = 56;
   uint256 internal constant MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX = 8;
   uint256 internal constant MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX = 8;
-  uint256 internal constant MAX_NEW_L2_TO_L1_MSGS_PER_TX = 2;
+  uint256 internal constant MAX_NON_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX = 3;
+  uint256 internal constant MAX_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX = 5;
   uint256 internal constant MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX = 16;
+  uint256 internal constant MAX_NON_REVERTIBLE_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX = 1;
+  uint256 internal constant MAX_REVERTIBLE_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX = 15;
   uint256 internal constant MAX_PUBLIC_DATA_READS_PER_TX = 16;
+  uint256 internal constant MAX_NON_REVERTIBLE_PUBLIC_DATA_READS_PER_TX = 1;
+  uint256 internal constant MAX_REVERTIBLE_PUBLIC_DATA_READS_PER_TX = 15;
+  uint256 internal constant MAX_NEW_L2_TO_L1_MSGS_PER_TX = 2;
   uint256 internal constant MAX_NEW_CONTRACTS_PER_TX = 1;
   uint256 internal constant MAX_READ_REQUESTS_PER_TX = 128;
   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;
-  uint256 internal constant MAX_NEW_COMMITMENTS_PER_TX_META = 8;
-  uint256 internal constant MAX_NEW_NULLIFIERS_PER_TX_META = 8;
-  uint256 internal constant MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX_META = 2;
   uint256 internal constant NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP = 16;
   uint256 internal constant VK_TREE_HEIGHT = 3;
   uint256 internal constant FUNCTION_TREE_HEIGHT = 5;

noir-projects/aztec-nr/authwit/src/account.nr

@@ -61,7 +61,7 @@ impl AccountActions {
         let fee_hash = fee_payload.hash();
         assert(valid_fn(private_context, fee_hash));
         fee_payload.execute_calls(private_context);
-        private_context.capture_max_non_revertible_side_effect_counter();
+        private_context.capture_min_revertible_side_effect_counter();
 
         let app_hash = app_payload.hash();
         assert(valid_fn(private_context, app_hash));

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

@@ -4,7 +4,8 @@ use crate::{
     oracle::{
     arguments, call_private_function::call_private_function_internal,
     enqueue_public_function_call::enqueue_public_function_call_internal, context::get_portal_address,
-    header::get_header_at, nullifier_key::{get_nullifier_key_pair, NullifierKeyPair}
+    header::get_header_at, nullifier_key::{get_nullifier_key_pair, NullifierKeyPair},
+    debug_log::debug_log
 }
 };
 use dep::protocol_types::{
@@ -40,7 +41,7 @@ struct PrivateContext {
     inputs: PrivateContextInputs,
     side_effect_counter: u32,
 
-    max_non_revertible_side_effect_counter: u32,
+    min_revertible_side_effect_counter: u32,
 
     args_hash : Field,
     return_values : BoundedVec<Field, RETURN_VALUES_LENGTH>,
@@ -68,10 +69,16 @@ struct PrivateContext {
 
 impl PrivateContext {
     pub fn new(inputs: PrivateContextInputs, args_hash: Field) -> PrivateContext {
+        let side_effect_counter = inputs.call_context.start_side_effect_counter;
+        let mut min_revertible_side_effect_counter = 0;
+        // Note. The side effect counter is 2 when this is the initial call
+        if (side_effect_counter == 2) {
+            min_revertible_side_effect_counter = side_effect_counter;
+        }
         PrivateContext {
             inputs,
-            side_effect_counter: inputs.call_context.start_side_effect_counter,
-            max_non_revertible_side_effect_counter: 0,
+            side_effect_counter,
+            min_revertible_side_effect_counter,
             args_hash,
             return_values: BoundedVec::new(0),
             read_requests: BoundedVec::new(SideEffect::empty()),
@@ -136,7 +143,12 @@ impl PrivateContext {
             call_context: self.inputs.call_context,
             args_hash: self.args_hash,
             return_values: self.return_values.storage,
-            max_non_revertible_side_effect_counter: self.max_non_revertible_side_effect_counter,
+            // TODO(fees): start this from 0 and test the following:
+            // - in the private circuit init that it gets set correctly
+            // - in the private circuit inner that it remains 0
+            // I've had to initialize the counter here so that it would work for contract deployments
+            // the above checks should be doable after we figure out fee payments for contract deployments
+            min_revertible_side_effect_counter: self.min_revertible_side_effect_counter,
             read_requests: self.read_requests.storage,
             nullifier_key_validation_requests: self.nullifier_key_validation_requests.storage,
             new_commitments: self.new_commitments.storage,
@@ -157,11 +169,8 @@ impl PrivateContext {
         priv_circuit_pub_inputs
     }
 
-    pub fn capture_max_non_revertible_side_effect_counter(&mut self) {
-        assert(
-            self.max_non_revertible_side_effect_counter == 0, "Already captured the non-revertible side effect counter"
-        );
-        self.max_non_revertible_side_effect_counter = self.side_effect_counter;
+    pub fn capture_min_revertible_side_effect_counter(&mut self) {
+        self.min_revertible_side_effect_counter = self.side_effect_counter;
     }
 
     pub fn push_read_request(&mut self, read_request: Field) {

noir-projects/noir-protocol-circuits/package.json

@@ -11,10 +11,10 @@
     "build": "yarn clean && yarn noir:build",
     "clean": "rm -rf ./dest src/target",
     "noir:build": "cd src && ../../../noir/target/release/nargo compile --silence-warnings",
-    "test": "cd src && ../../../noir/target/release/nargo test"
+    "test": "cd src && ../../../noir/target/release/nargo test --silence-warnings"
   },
   "files": [
     "dest",
     "src"
   ]
-}
+}

noir-projects/noir-protocol-circuits/src/Nargo.toml

@@ -13,6 +13,8 @@ members = [
     "crates/public-kernel-setup-simulated",
     "crates/public-kernel-app-logic",
     "crates/public-kernel-app-logic-simulated",
+    "crates/public-kernel-teardown",
+    "crates/public-kernel-teardown-simulated",
     "crates/rollup-lib",
     "crates/rollup-merge",
     "crates/rollup-base",

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

@@ -86,6 +86,7 @@ pub fn initialize_end_values(
     public_inputs: &mut PrivateKernelCircuitPublicInputsBuilder
 ) {
     public_inputs.constants = previous_kernel.public_inputs.constants;
+    public_inputs.min_revertible_side_effect_counter = previous_kernel.public_inputs.min_revertible_side_effect_counter;
 
     // Ensure the arrays are the same as previously, before we start pushing more data onto them in other
     // functions within this circuit:

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

@@ -26,7 +26,7 @@ impl PrivateKernelInitCircuitPrivateInputs {
             historical_header: self.private_call.call_stack_item.public_inputs.historical_header,
             tx_context: self.tx_request.tx_context,
         };
-        public_inputs.max_non_revertible_side_effect_counter = self.private_call.call_stack_item.public_inputs.max_non_revertible_side_effect_counter;
+        public_inputs.min_revertible_side_effect_counter = self.private_call.call_stack_item.public_inputs.min_revertible_side_effect_counter;
     }
 
     // Confirm that the TxRequest (user's intent) 

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

@@ -103,7 +103,7 @@ mod tests {
 
     impl PrivateKernelInnerInputsBuilder {
         pub fn new() -> Self {
-            let previous_kernel = PreviousKernelDataBuilder::new();
+            let previous_kernel = PreviousKernelDataBuilder::new(false);
             let private_call = PrivateCallDataBuilder::new(false);
 
             PrivateKernelInnerInputsBuilder { previous_kernel, private_call }