From 8502bffec521df1b327fcd7abedd65dbb2a85c81 Mon Sep 17 00:00:00 2001 From: superstar0402 Date: Thu, 7 Mar 2024 12:29:02 -0500 Subject: [PATCH] feat: basic public reverts (#4870) 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 https://github.com/AztecProtocol/aztec-packages/issues/4972 Further, to test the flow where we **privately** pay the fee but then have a revert, we need to first tackle https://github.com/AztecProtocol/aztec-packages/issues/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 https://github.com/AztecProtocol/aztec-packages/issues/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. --- aztec/src/context/private_context.nr | 3 ++- aztec/src/context/public_context.nr | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/aztec/src/context/private_context.nr b/aztec/src/context/private_context.nr index 30c6d14..dffe714 100644 --- a/aztec/src/context/private_context.nr +++ b/aztec/src/context/private_context.nr @@ -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 }; diff --git a/aztec/src/context/public_context.nr b/aztec/src/context/public_context.nr index 50a25dd..f8816f2 100644 --- a/aztec/src/context/public_context.nr +++ b/aztec/src/context/public_context.nr @@ -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 }