From 31fca44371859e57b89e77897f8cb7dc9f061b55 Mon Sep 17 00:00:00 2001 From: just-mitch <68168980+just-mitch@users.noreply.github.com> Date: Wed, 20 Mar 2024 14:47:15 -0400 Subject: [PATCH] feat!: Mark transactions as reverted on L1 (#5226) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Introduction Presently, if a transaction is reverted, the base rollup does not mark it as reverted: it merely has its revertible side effects dropped. This issue will update the base rollup to mark reverted transactions. # Solution Design ## New `RevertCode` in `TxEffect` Create a new class in `circuits.js` to hold a `RevertCode`. It will be a safe wrapper around an `Fr` that will be used to store the revert code of a transaction. `0` will indicate success, and any other value will indicate failure. Presently, only `1` is used to indicate general failure, but we'll leave the door open for future expansion. `TxEffect` will be updated to include a `RevertCode`. ### Ethereum `status` Ethereum stores their `status` in a `uint64`, but only currently use `0` to indicate failure and `1` for success. [More info](https://github.com/ethereum/EIPs/issues/98) (Thanks @spalladino !) ### Size considerations We'll eventually want to add `gasUsed` and `gasPrice` to `TxEffect`. Using a full field is wasteful, but using a smaller size will make the encoding and hashing more complex (see below). We'll implement with a full field and have a stacked PR to optimize the size. ## Kernel Constraints The `RevertCode` will become part of the content commitment for a transaction. The content commitment is computed by the base rollup in `compute_tx_effects_hash`, which: - accepts a `CombinedAccumulatedData` - operates over `Field`s We will therefore update the `CombinedAccumulatedData` to include the `RevertCode`. A fallout from that is we will move the current `reverted` flag from `PublicKernelCircuitPublicInputs` to be part of: - `PrivateAccumulatedNonRevertibleData` - `PublicAccumulatedNonRevertibleData` This is because we: 1. build up a `CombinedAccumulatedData` during private execution 2. split them into `PrivateAccumulatedNonRevertibleData` and `PrivateAccumulatedRevertibleData` in the private kernel tail 3. convert those `PrivateAccumulated...` into `PublicAccumulated...` during public kernel execution as needed 4. recombine them back into `CombinedAccumulatedData` before feeding back into base rollup ## Encoding The `RevertCode` will come first in the encoded `TxEffect`. That is we will publish: ``` || 32 bytes for RevertCode || ... Existing PublishedTxEffect ... || ``` ## L1 Tx Decoder We'll need to update the availability oracle to compensate for the new flag. ## `TxReceipt` We'll add a new `TxStatus` that is `REVERTED = 'reverted'`. We need to update `getSettledTxReceipt` to inspect the `TxEffect` and set the `status` accordingly. # Test Plan ## Unit Tests - Serde of `TxEffect` in TS and solidity. ## E2E Tests - Add checks for tx statuses of `REVERTED` in the `e2e_fees`, `e2e_deploy_contract`, and `dapp_testing`. # Documentation Plan Will start a page in the "Data publication and availability" section of the yellow paper describing the format of the block submitted. # Especially Relevant Parties - @PhilWindle - @benesjan - @alexghr - @LeilaWang - @LHerskind # Plan Approvals **_👋 Please add a +1 or comment to express your approval or disapproval of the plan above._** --- aztec/src/context/private_context.nr | 2 +- aztec/src/context/public_context.nr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/aztec/src/context/private_context.nr b/aztec/src/context/private_context.nr index 23b3c1c3..18b52ca5 100644 --- a/aztec/src/context/private_context.nr +++ b/aztec/src/context/private_context.nr @@ -465,7 +465,7 @@ impl PrivateContext { unencrypted_log_preimages_length: 0, historical_header: Header::empty(), prover_address: AztecAddress::zero(), - reverted: false + reverted: 0 }, is_execution_request: true }; diff --git a/aztec/src/context/public_context.nr b/aztec/src/context/public_context.nr index c33aa633..4013e4b5 100644 --- a/aztec/src/context/public_context.nr +++ b/aztec/src/context/public_context.nr @@ -143,7 +143,7 @@ impl PublicContext { unencrypted_log_preimages_length, historical_header: self.inputs.historical_header, prover_address: self.prover_address, - reverted: false + reverted: 0 }; pub_circuit_pub_inputs }