Skip to content

Commit

Permalink
feat: basic public reverts (#4870)
Browse files Browse the repository at this point in the history
Fix #4971 , Parent issue is #4096

This PR adds basic support for reversions of public functions.

A public function may be simulated locally before the TX is submitted.
In this case, the PXE calls out to the node, requesting public
simulation.

When this is done, the node will throw an error if the public function
would revert.

However, if the function is called with `skipPublicSimulation`, the TX
is submitted, and the sequencer will run the public processor, which
catches `SimulationErrors`, and includes them in the rollup.

Generally this is accomplished by adding a `reverted` boolean to the
`PublicCircuitPublicInputs` and `PublicKernelCircuitPublicInputs`.

It is expected that the AVM will set its `reverted` flag to true. In the
meantime, while simulating with the acvm, if it throws, we catch, and
set the flag on the output to `true`.

There is also a `revertedReason`, which is useful for debugging (e.g.
when you simulate public functions locally, you want the node to return
a meaningful error to you)

The meat of the logic changes are in `public_kernel_app_logic.nr`,
`abstract_phase_manager.ts`, and `app_logic_phase_manager.ts`.

The primary test is in `e2e_fees.test.ts`.

In the app logic circuit, we now check to see if we have reverted, and
forward the flag if so. Additionally, if we have reverted, we do not
propagate forward any revertible side effects, and instead leave them
all to zero.

Note further, if we get a simulation error in a **nested** public
execution, we **do throw**, and allow the parent/top-level call to catch
the error and return gracefully.

I also added a bunch of inspect functions, and assertions about hashes
for sanity checking.

# Future work

Presently if a tx reverts but is included, it is not distinguished from
other transactions in the block which did not revert. This will be fixed
as part of AztecProtocol/aztec-packages#4972

Further, to test the flow where we **privately** pay the fee but then
have a revert, we need to first tackle
AztecProtocol/aztec-packages#4712 so that we
have "non-revertible" logs (unshield is used in fee prep in this case,
which emits encrypted logs, which are presently dropped if the
transaction reverts).

Additionally, as mentioned in comments below, we do not yet insist that
the non-revertible parts of public execution in fact do not revert. This
will be done in
AztecProtocol/aztec-packages#5038.

All of those are tracked as part of the parent issue #4096

We will also want to optimize the public kernel circuits, but I am in
favor of adding the bare minimum feature set before we start optimizing.
  • Loading branch information
just-mitch authored and AztecBot committed Mar 19, 2024
1 parent 58d77a6 commit 188be5c
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
3 changes: 2 additions & 1 deletion aztec/src/context/private_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,8 @@ impl PrivateContext {
unencrypted_logs_hash: [0; NUM_FIELDS_PER_SHA256],
unencrypted_log_preimages_length: 0,
historical_header: Header::empty(),
prover_address: AztecAddress::zero()
prover_address: AztecAddress::zero(),
reverted: false
},
is_execution_request: true
};
Expand Down
3 changes: 2 additions & 1 deletion aztec/src/context/public_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ impl PublicContext {
unencrypted_logs_hash,
unencrypted_log_preimages_length,
historical_header: self.inputs.historical_header,
prover_address: self.prover_address
prover_address: self.prover_address,
reverted: false
};
pub_circuit_pub_inputs
}
Expand Down

0 comments on commit 188be5c

Please sign in to comment.