Skip to content

Commit

Permalink
feat: non revertible effects and tx phases (#4629)
Browse files Browse the repository at this point in the history
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](AztecProtocol/aztec-packages#4543)

Co-authored-by: Alex Gherghisan <alexghr@users.noreply.github.com>
  • Loading branch information
2 people authored and AztecBot committed Mar 19, 2024
1 parent 24fb8a6 commit 871f8de
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 11 deletions.
2 changes: 1 addition & 1 deletion authwit/src/account.nr
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
29 changes: 19 additions & 10 deletions aztec/src/context/private_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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()),
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down

0 comments on commit 871f8de

Please sign in to comment.