Do not throw on public simulation error #4971
Assignees
Comments
Closed
just-mitch
added a commit
that referenced
this issue
Mar 7, 2024
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 #4972 Further, to test the flow where we **privately** pay the fee but then have a revert, we need to first tackle #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 #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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Presently, we always throw whenever there is an error while simulating public functions. We want to include transactions that revert during app logic, so this must be changed to gracefully handle public reverts.
The text was updated successfully, but these errors were encountered: