Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add logs to non-revertible set #4712

Closed
Tracked by #4096
just-mitch opened this issue Feb 22, 2024 · 1 comment · Fixed by #7076 or #7088
Closed
Tracked by #4096

Add logs to non-revertible set #4712

just-mitch opened this issue Feb 22, 2024 · 1 comment · Fixed by #7076 or #7088

Comments

@just-mitch
Copy link
Contributor

just-mitch commented Feb 22, 2024

We don't presently support logs that are emitted as part of TX setup.

To do so, we first need #5017 which adds side effect counters to logs.

@github-project-automation github-project-automation bot moved this to Todo in A3 Feb 22, 2024
This was referenced Mar 6, 2024
@just-mitch just-mitch added this to the ITN - Fees milestone Mar 6, 2024
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.
AztecBot pushed a commit to AztecProtocol/aztec-nr that referenced this issue Mar 19, 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 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.
@alexghr
Copy link
Contributor

alexghr commented Apr 17, 2024

The sequencer needs to be able to be able to identify contracts deployed in the non-revertible phase of a tx and deploy them still if the app-logic reverts.

MirandaWood added a commit that referenced this issue May 16, 2024
Closes #1641

## This PR:

### Logs as `SideEffect`s -> `LogHash` or `NoteLogHash`
Adds new structs to track logs in the kernel circuits, so we can track
individual log lengths (and assign gas accordingly) and separate logs
linked to notes from generic logs.
The length changes contribute to #4712 but this PR does not close it -
we need some more work in ts to ensure logs from the non-revertible side
actually persist to the tx.

### Adds `note_encrypted_logs`
To aid the kernels/pxe/note dbs in managing note encrypted logs, most of
the changed files are adding a new array of note logs to anything
already containing `.encrypted_logs_<hashes/hash>`.
Since all our current contracts/tests only emit encrypted logs for
notes, the values expected in `.encrypted_logs` have basically moved
over to `.note_encrypted_logs`. A later PR will add code for generic
encrypted logs (see below).
Includes adding logs to the note cache in `client_execution_context` and
removing `LogsCache`, as the note cache handles note specific logs.

### Squashes transient note logs
Removes any logs linked to note hashes nullified in the same tx. This
involves adding note logs to transient hints for the private kernel tail
and chopping them inside the circuit.

### TODOs/Notes

- This PR ended up pretty large, so while we can silo log hashes now, it
would be cleaner to do in a follow up PR.
- There are some small changes to public/AVM contexts which are mostly
to replace `SideEffect` with `LogHash` and a couple of things addressed
in comments below.
- Currently, the only encrypted logs that are emitted are linked to
notes. To allow for generic values emitted as encrypted logs, I'll need
to add a new oracle call and methods, which makes sense to do once we
encrypt inside the circuit (#1139).
- Total logs length (e.g. `encrypted_log_preimages_length` is no longer
required now we track indvidual lengths (it's also not trusted since it
comes from contexts), so it can be removed from the kernels.
- We originally tried to track log lengths the same way as `ts`, which
adds 4 bytes to each nested 'group' of logs:
`L2BlockL2Logs.txLogs = TxL2Logs -> .functionLogs = FunctionL2Logs ->
.logs = <Un>EncryptedL2Log[]`
but this never actually matched up because we accumulate and sort logs
regardless of what call they came from. E.g. 3 groups of
`FunctionL2Logs` would each have +4 length when added to a tx, but we
take the contents, sort them, then add to a single `FunctionL2Logs.logs`
array before assigning to a tx, so end up with a single +4 length. This
length value is only used on L1 to destructure the blob of bytes
representing logs. Any incorrect lengths would lead to a logs hash
mismatch, so we don't need to match them in the circuit anyway. For this
reason, I've removed a few unnecessary +4s from the circuits.
just-mitch added a commit that referenced this issue Jun 21, 2024
…tible logs. (#7076)

Tx objects need to look at the kernel outputs to remove logs that
reverted.
Add test of private fee payment method with revert.

fix #4712
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 Jun 21, 2024
sirasistant pushed a commit that referenced this issue Jun 26, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.44.0</summary>

##
[0.44.0](aztec-package-v0.43.0...aztec-package-v0.44.0)
(2024-06-26)


### Features

* Add OpenTelemetry to node
([#7102](#7102))
([6bf2b72](6bf2b72))
* Devnet deployments
([#7024](#7024))
([fa70876](fa70876))
* Track spans
([#7129](#7129))
([924c3f8](924c3f8))
</details>

<details><summary>barretenberg.js: 0.44.0</summary>

##
[0.44.0](barretenberg.js-v0.43.0...barretenberg.js-v0.44.0)
(2024-06-26)


### Bug Fixes

* False decryption fix
([#7066](#7066))
([48d9df4](48d9df4))
</details>

<details><summary>aztec-packages: 0.44.0</summary>

##
[0.44.0](aztec-packages-v0.43.0...aztec-packages-v0.44.0)
(2024-06-26)


### ⚠ BREAKING CHANGES

* make note_getter return BoundedVec instead of an Option array
([#7050](#7050))
* TXE
([#6985](#6985))

### Features

* Add macro impls for events
([#7081](#7081))
([c13dd9f](c13dd9f))
* Add OpenTelemetry to node
([#7102](#7102))
([6bf2b72](6bf2b72))
* Added prove_output_all flow for honk
([#6869](#6869))
([7bd7c66](7bd7c66))
* **avm:** Add ECC ops to avm_proving_test
([#7058](#7058))
([7f62a90](7f62a90))
* **avm:** Cpp msm changes
([#7056](#7056))
([f9c8f20](f9c8f20))
* **avm:** Include bb-pilcom in monorepo
([#7098](#7098))
([0442158](0442158))
* Constrain event encryption and unify note and event emit api
([#7171](#7171))
([5c3772f](5c3772f)),
closes
[#7160](#7160)
* Conventional lookups using log-deriv
([#7020](#7020))
([6f1212f](6f1212f))
* Devnet deployments
([#7024](#7024))
([fa70876](fa70876))
* Do not discard logs on revert since the kernel has pruned revertible
logs.
([#7076](#7076))
([366fb21](366fb21)),
closes
[#4712](#4712)
* **docs:** Publish PDF of protocol specs + remove links to pages in
item lists in protocol specs
([#6684](#6684))
([367e3cf](367e3cf))
* Enable merge recursive verifier in Goblin recursive verifier
([#7182](#7182))
([9b4f56c](9b4f56c))
* Flamegraph helper script
([#7077](#7077))
([8630c8f](8630c8f))
* Full test skips public simulation
([#7186](#7186))
([4c1997f](4c1997f))
* Make note_getter return BoundedVec instead of an Option array
([#7050](#7050))
([f9ac0fc](f9ac0fc))
* **p2p:** More comprehensive peer management, dial retries, persistence
fix
([#6953](#6953))
([cdd1cbd](cdd1cbd))
* Private authwit with static call
([#7073](#7073))
([9c52d47](9c52d47))
* Several updates in SMT verification module
([#7105](#7105))
([41b21f1](41b21f1))
* Shplonk revival in ECCVM
([#7164](#7164))
([34eb5a0](34eb5a0))
* Throwing errors in `BufferReader` when out of bounds
([#7149](#7149))
([bf4a986](bf4a986))
* Track spans
([#7129](#7129))
([924c3f8](924c3f8))
* TXE
([#6985](#6985))
([109624f](109624f))
* TXE 2: Electric boogaloo
([#7154](#7154))
([bb38246](bb38246))


### Bug Fixes

* **avm:** Fix unencryptedlog c++ deser
([#7194](#7194))
([89a99af](89a99af))
* **avm:** Re-enable ext call test
([#7147](#7147))
([33ccf1b](33ccf1b))
* **avm:** Reenable tag error sload
([#7153](#7153))
([fd92d46](fd92d46))
* **avm:** Update codegen
([#7178](#7178))
([1d29708](1d29708))
* Bug fixing bench prover test
([#7135](#7135))
([13678be](13678be)),
closes
[#7080](#7080)
* **ci:** Don't run npm_deploy l1-contracts
([#7187](#7187))
([80d26d8](80d26d8))
* **ci:** Move osxcross from build image
([#7151](#7151))
([7746363](7746363))
* Enable log filtering with the DEBUG variable
([#7150](#7150))
([33798b6](33798b6))
* Export event selector and replace function selector with event
selector where appropriate
([#7095](#7095))
([fcc15fa](fcc15fa)),
closes
[#7089](#7089)
* False decryption fix
([#7066](#7066))
([48d9df4](48d9df4))
* Fix bug for a unit test in full proving mode repated to MSM
([#7104](#7104))
([e37809b](e37809b))


### Miscellaneous

* `destroy_note(...)` optimization
([#7103](#7103))
([0770011](0770011))
* Add avm team as codeowners to more repo files
([#7196](#7196))
([9be0ad6](9be0ad6))
* **avm:** Remove avm prefix from pil and executor
([#7099](#7099))
([b502fcd](b502fcd))
* **avm:** Renamings and comments
([#7128](#7128))
([ed2f98e](ed2f98e))
* **avm:** Separate some fixed tables
([#7163](#7163))
([1d4a9a2](1d4a9a2))
* **ci:** Add new e2e base target
([#7179](#7179))
([26fc599](26fc599))
* Create workflow for full AVM tests
([#7051](#7051))
([a0b9c4b](a0b9c4b)),
closes
[#6643](#6643)
* **docs:** Fix migration notes
([#7195](#7195))
([88efda0](88efda0))
* **docs:** Moving tutorials and quick starts around, spinning off
codespaces page
([#6777](#6777))
([1542fa6](1542fa6))
* Fix migration notes
([#7133](#7133))
([14917d3](14917d3))
* Fix noir-projects dockerfile for CircleCI
([#7093](#7093))
([52ce25d](52ce25d))
* Increase the timeout of the runner for full AVM workflow to 70 minutes
([#7183](#7183))
([9aabc32](9aabc32))
* Indirects and read/write slices
([#7082](#7082))
([d5e80ee](d5e80ee))
* Note hashes cleanup + optimization
([#7132](#7132))
([edd6d3f](edd6d3f))
* Note hashing gate optimizations
([#7130](#7130))
([81a2580](81a2580))
* **powdr:** Update to latest and add logging
([#7152](#7152))
([f500f2e](f500f2e))
* Reads the return data
([#6669](#6669))
([ef85542](ef85542))
* Refactor AVM simulator's side-effect tracing
([#7091](#7091))
([9495413](9495413))
* Remove stray files
([#7158](#7158))
([29398de](29398de))
* Remove unneeded public input folding
([#7094](#7094))
([c30dc38](c30dc38))
* Replace relative paths to noir-protocol-circuits
([f7e4392](f7e4392))
* Replace relative paths to noir-protocol-circuits
([886f7b1](886f7b1))
* Replace relative paths to noir-protocol-circuits
([b1081f8](b1081f8))
* Replace relative paths to noir-protocol-circuits
([c0989eb](c0989eb))
* Replace relative paths to noir-protocol-circuits
([525bbe7](525bbe7))
* Replace relative paths to noir-protocol-circuits
([67bcd82](67bcd82))
* Take the PCS out of Zeromorph and refactor tests
([#7078](#7078))
([e192678](e192678))
* Track avm proving time
([#7084](#7084))
([59df722](59df722))
* Ultra flavor cleanup
([#7070](#7070))
([77761c6](77761c6))
</details>

<details><summary>barretenberg: 0.44.0</summary>

##
[0.44.0](barretenberg-v0.43.0...barretenberg-v0.44.0)
(2024-06-26)


### Features

* Added prove_output_all flow for honk
([#6869](#6869))
([7bd7c66](7bd7c66))
* **avm:** Add ECC ops to avm_proving_test
([#7058](#7058))
([7f62a90](7f62a90))
* **avm:** Cpp msm changes
([#7056](#7056))
([f9c8f20](f9c8f20))
* **avm:** Include bb-pilcom in monorepo
([#7098](#7098))
([0442158](0442158))
* Conventional lookups using log-deriv
([#7020](#7020))
([6f1212f](6f1212f))
* Enable merge recursive verifier in Goblin recursive verifier
([#7182](#7182))
([9b4f56c](9b4f56c))
* Several updates in SMT verification module
([#7105](#7105))
([41b21f1](41b21f1))
* Shplonk revival in ECCVM
([#7164](#7164))
([34eb5a0](34eb5a0))


### Bug Fixes

* **avm:** Fix unencryptedlog c++ deser
([#7194](#7194))
([89a99af](89a99af))
* **avm:** Re-enable ext call test
([#7147](#7147))
([33ccf1b](33ccf1b))
* **avm:** Reenable tag error sload
([#7153](#7153))
([fd92d46](fd92d46))
* **avm:** Update codegen
([#7178](#7178))
([1d29708](1d29708))
* Bug fixing bench prover test
([#7135](#7135))
([13678be](13678be)),
closes
[#7080](#7080)
* Fix bug for a unit test in full proving mode repated to MSM
([#7104](#7104))
([e37809b](e37809b))


### Miscellaneous

* **avm:** Remove avm prefix from pil and executor
([#7099](#7099))
([b502fcd](b502fcd))
* **avm:** Renamings and comments
([#7128](#7128))
([ed2f98e](ed2f98e))
* **avm:** Separate some fixed tables
([#7163](#7163))
([1d4a9a2](1d4a9a2))
* Create workflow for full AVM tests
([#7051](#7051))
([a0b9c4b](a0b9c4b)),
closes
[#6643](#6643)
* Indirects and read/write slices
([#7082](#7082))
([d5e80ee](d5e80ee))
* Reads the return data
([#6669](#6669))
([ef85542](ef85542))
* Remove unneeded public input folding
([#7094](#7094))
([c30dc38](c30dc38))
* Take the PCS out of Zeromorph and refactor tests
([#7078](#7078))
([e192678](e192678))
* Ultra flavor cleanup
([#7070](#7070))
([77761c6](77761c6))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
AztecBot added a commit to AztecProtocol/barretenberg that referenced this issue Jun 27, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.44.0</summary>

##
[0.44.0](AztecProtocol/aztec-packages@aztec-package-v0.43.0...aztec-package-v0.44.0)
(2024-06-26)


### Features

* Add OpenTelemetry to node
([#7102](AztecProtocol/aztec-packages#7102))
([6bf2b72](AztecProtocol/aztec-packages@6bf2b72))
* Devnet deployments
([#7024](AztecProtocol/aztec-packages#7024))
([fa70876](AztecProtocol/aztec-packages@fa70876))
* Track spans
([#7129](AztecProtocol/aztec-packages#7129))
([924c3f8](AztecProtocol/aztec-packages@924c3f8))
</details>

<details><summary>barretenberg.js: 0.44.0</summary>

##
[0.44.0](AztecProtocol/aztec-packages@barretenberg.js-v0.43.0...barretenberg.js-v0.44.0)
(2024-06-26)


### Bug Fixes

* False decryption fix
([#7066](AztecProtocol/aztec-packages#7066))
([48d9df4](AztecProtocol/aztec-packages@48d9df4))
</details>

<details><summary>aztec-packages: 0.44.0</summary>

##
[0.44.0](AztecProtocol/aztec-packages@aztec-packages-v0.43.0...aztec-packages-v0.44.0)
(2024-06-26)


### ⚠ BREAKING CHANGES

* make note_getter return BoundedVec instead of an Option array
([#7050](AztecProtocol/aztec-packages#7050))
* TXE
([#6985](AztecProtocol/aztec-packages#6985))

### Features

* Add macro impls for events
([#7081](AztecProtocol/aztec-packages#7081))
([c13dd9f](AztecProtocol/aztec-packages@c13dd9f))
* Add OpenTelemetry to node
([#7102](AztecProtocol/aztec-packages#7102))
([6bf2b72](AztecProtocol/aztec-packages@6bf2b72))
* Added prove_output_all flow for honk
([#6869](AztecProtocol/aztec-packages#6869))
([7bd7c66](AztecProtocol/aztec-packages@7bd7c66))
* **avm:** Add ECC ops to avm_proving_test
([#7058](AztecProtocol/aztec-packages#7058))
([7f62a90](AztecProtocol/aztec-packages@7f62a90))
* **avm:** Cpp msm changes
([#7056](AztecProtocol/aztec-packages#7056))
([f9c8f20](AztecProtocol/aztec-packages@f9c8f20))
* **avm:** Include bb-pilcom in monorepo
([#7098](AztecProtocol/aztec-packages#7098))
([0442158](AztecProtocol/aztec-packages@0442158))
* Constrain event encryption and unify note and event emit api
([#7171](AztecProtocol/aztec-packages#7171))
([5c3772f](AztecProtocol/aztec-packages@5c3772f)),
closes
[#7160](AztecProtocol/aztec-packages#7160)
* Conventional lookups using log-deriv
([#7020](AztecProtocol/aztec-packages#7020))
([6f1212f](AztecProtocol/aztec-packages@6f1212f))
* Devnet deployments
([#7024](AztecProtocol/aztec-packages#7024))
([fa70876](AztecProtocol/aztec-packages@fa70876))
* Do not discard logs on revert since the kernel has pruned revertible
logs.
([#7076](AztecProtocol/aztec-packages#7076))
([366fb21](AztecProtocol/aztec-packages@366fb21)),
closes
[#4712](AztecProtocol/aztec-packages#4712)
* **docs:** Publish PDF of protocol specs + remove links to pages in
item lists in protocol specs
([#6684](AztecProtocol/aztec-packages#6684))
([367e3cf](AztecProtocol/aztec-packages@367e3cf))
* Enable merge recursive verifier in Goblin recursive verifier
([#7182](AztecProtocol/aztec-packages#7182))
([9b4f56c](AztecProtocol/aztec-packages@9b4f56c))
* Flamegraph helper script
([#7077](AztecProtocol/aztec-packages#7077))
([8630c8f](AztecProtocol/aztec-packages@8630c8f))
* Full test skips public simulation
([#7186](AztecProtocol/aztec-packages#7186))
([4c1997f](AztecProtocol/aztec-packages@4c1997f))
* Make note_getter return BoundedVec instead of an Option array
([#7050](AztecProtocol/aztec-packages#7050))
([f9ac0fc](AztecProtocol/aztec-packages@f9ac0fc))
* **p2p:** More comprehensive peer management, dial retries, persistence
fix
([#6953](AztecProtocol/aztec-packages#6953))
([cdd1cbd](AztecProtocol/aztec-packages@cdd1cbd))
* Private authwit with static call
([#7073](AztecProtocol/aztec-packages#7073))
([9c52d47](AztecProtocol/aztec-packages@9c52d47))
* Several updates in SMT verification module
([#7105](AztecProtocol/aztec-packages#7105))
([41b21f1](AztecProtocol/aztec-packages@41b21f1))
* Shplonk revival in ECCVM
([#7164](AztecProtocol/aztec-packages#7164))
([34eb5a0](AztecProtocol/aztec-packages@34eb5a0))
* Throwing errors in `BufferReader` when out of bounds
([#7149](AztecProtocol/aztec-packages#7149))
([bf4a986](AztecProtocol/aztec-packages@bf4a986))
* Track spans
([#7129](AztecProtocol/aztec-packages#7129))
([924c3f8](AztecProtocol/aztec-packages@924c3f8))
* TXE
([#6985](AztecProtocol/aztec-packages#6985))
([109624f](AztecProtocol/aztec-packages@109624f))
* TXE 2: Electric boogaloo
([#7154](AztecProtocol/aztec-packages#7154))
([bb38246](AztecProtocol/aztec-packages@bb38246))


### Bug Fixes

* **avm:** Fix unencryptedlog c++ deser
([#7194](AztecProtocol/aztec-packages#7194))
([89a99af](AztecProtocol/aztec-packages@89a99af))
* **avm:** Re-enable ext call test
([#7147](AztecProtocol/aztec-packages#7147))
([33ccf1b](AztecProtocol/aztec-packages@33ccf1b))
* **avm:** Reenable tag error sload
([#7153](AztecProtocol/aztec-packages#7153))
([fd92d46](AztecProtocol/aztec-packages@fd92d46))
* **avm:** Update codegen
([#7178](AztecProtocol/aztec-packages#7178))
([1d29708](AztecProtocol/aztec-packages@1d29708))
* Bug fixing bench prover test
([#7135](AztecProtocol/aztec-packages#7135))
([13678be](AztecProtocol/aztec-packages@13678be)),
closes
[#7080](AztecProtocol/aztec-packages#7080)
* **ci:** Don't run npm_deploy l1-contracts
([#7187](AztecProtocol/aztec-packages#7187))
([80d26d8](AztecProtocol/aztec-packages@80d26d8))
* **ci:** Move osxcross from build image
([#7151](AztecProtocol/aztec-packages#7151))
([7746363](AztecProtocol/aztec-packages@7746363))
* Enable log filtering with the DEBUG variable
([#7150](AztecProtocol/aztec-packages#7150))
([33798b6](AztecProtocol/aztec-packages@33798b6))
* Export event selector and replace function selector with event
selector where appropriate
([#7095](AztecProtocol/aztec-packages#7095))
([fcc15fa](AztecProtocol/aztec-packages@fcc15fa)),
closes
[#7089](AztecProtocol/aztec-packages#7089)
* False decryption fix
([#7066](AztecProtocol/aztec-packages#7066))
([48d9df4](AztecProtocol/aztec-packages@48d9df4))
* Fix bug for a unit test in full proving mode repated to MSM
([#7104](AztecProtocol/aztec-packages#7104))
([e37809b](AztecProtocol/aztec-packages@e37809b))


### Miscellaneous

* `destroy_note(...)` optimization
([#7103](AztecProtocol/aztec-packages#7103))
([0770011](AztecProtocol/aztec-packages@0770011))
* Add avm team as codeowners to more repo files
([#7196](AztecProtocol/aztec-packages#7196))
([9be0ad6](AztecProtocol/aztec-packages@9be0ad6))
* **avm:** Remove avm prefix from pil and executor
([#7099](AztecProtocol/aztec-packages#7099))
([b502fcd](AztecProtocol/aztec-packages@b502fcd))
* **avm:** Renamings and comments
([#7128](AztecProtocol/aztec-packages#7128))
([ed2f98e](AztecProtocol/aztec-packages@ed2f98e))
* **avm:** Separate some fixed tables
([#7163](AztecProtocol/aztec-packages#7163))
([1d4a9a2](AztecProtocol/aztec-packages@1d4a9a2))
* **ci:** Add new e2e base target
([#7179](AztecProtocol/aztec-packages#7179))
([26fc599](AztecProtocol/aztec-packages@26fc599))
* Create workflow for full AVM tests
([#7051](AztecProtocol/aztec-packages#7051))
([a0b9c4b](AztecProtocol/aztec-packages@a0b9c4b)),
closes
[#6643](AztecProtocol/aztec-packages#6643)
* **docs:** Fix migration notes
([#7195](AztecProtocol/aztec-packages#7195))
([88efda0](AztecProtocol/aztec-packages@88efda0))
* **docs:** Moving tutorials and quick starts around, spinning off
codespaces page
([#6777](AztecProtocol/aztec-packages#6777))
([1542fa6](AztecProtocol/aztec-packages@1542fa6))
* Fix migration notes
([#7133](AztecProtocol/aztec-packages#7133))
([14917d3](AztecProtocol/aztec-packages@14917d3))
* Fix noir-projects dockerfile for CircleCI
([#7093](AztecProtocol/aztec-packages#7093))
([52ce25d](AztecProtocol/aztec-packages@52ce25d))
* Increase the timeout of the runner for full AVM workflow to 70 minutes
([#7183](AztecProtocol/aztec-packages#7183))
([9aabc32](AztecProtocol/aztec-packages@9aabc32))
* Indirects and read/write slices
([#7082](AztecProtocol/aztec-packages#7082))
([d5e80ee](AztecProtocol/aztec-packages@d5e80ee))
* Note hashes cleanup + optimization
([#7132](AztecProtocol/aztec-packages#7132))
([edd6d3f](AztecProtocol/aztec-packages@edd6d3f))
* Note hashing gate optimizations
([#7130](AztecProtocol/aztec-packages#7130))
([81a2580](AztecProtocol/aztec-packages@81a2580))
* **powdr:** Update to latest and add logging
([#7152](AztecProtocol/aztec-packages#7152))
([f500f2e](AztecProtocol/aztec-packages@f500f2e))
* Reads the return data
([#6669](AztecProtocol/aztec-packages#6669))
([ef85542](AztecProtocol/aztec-packages@ef85542))
* Refactor AVM simulator's side-effect tracing
([#7091](AztecProtocol/aztec-packages#7091))
([9495413](AztecProtocol/aztec-packages@9495413))
* Remove stray files
([#7158](AztecProtocol/aztec-packages#7158))
([29398de](AztecProtocol/aztec-packages@29398de))
* Remove unneeded public input folding
([#7094](AztecProtocol/aztec-packages#7094))
([c30dc38](AztecProtocol/aztec-packages@c30dc38))
* Replace relative paths to noir-protocol-circuits
([f7e4392](AztecProtocol/aztec-packages@f7e4392))
* Replace relative paths to noir-protocol-circuits
([886f7b1](AztecProtocol/aztec-packages@886f7b1))
* Replace relative paths to noir-protocol-circuits
([b1081f8](AztecProtocol/aztec-packages@b1081f8))
* Replace relative paths to noir-protocol-circuits
([c0989eb](AztecProtocol/aztec-packages@c0989eb))
* Replace relative paths to noir-protocol-circuits
([525bbe7](AztecProtocol/aztec-packages@525bbe7))
* Replace relative paths to noir-protocol-circuits
([67bcd82](AztecProtocol/aztec-packages@67bcd82))
* Take the PCS out of Zeromorph and refactor tests
([#7078](AztecProtocol/aztec-packages#7078))
([e192678](AztecProtocol/aztec-packages@e192678))
* Track avm proving time
([#7084](AztecProtocol/aztec-packages#7084))
([59df722](AztecProtocol/aztec-packages@59df722))
* Ultra flavor cleanup
([#7070](AztecProtocol/aztec-packages#7070))
([77761c6](AztecProtocol/aztec-packages@77761c6))
</details>

<details><summary>barretenberg: 0.44.0</summary>

##
[0.44.0](AztecProtocol/aztec-packages@barretenberg-v0.43.0...barretenberg-v0.44.0)
(2024-06-26)


### Features

* Added prove_output_all flow for honk
([#6869](AztecProtocol/aztec-packages#6869))
([7bd7c66](AztecProtocol/aztec-packages@7bd7c66))
* **avm:** Add ECC ops to avm_proving_test
([#7058](AztecProtocol/aztec-packages#7058))
([7f62a90](AztecProtocol/aztec-packages@7f62a90))
* **avm:** Cpp msm changes
([#7056](AztecProtocol/aztec-packages#7056))
([f9c8f20](AztecProtocol/aztec-packages@f9c8f20))
* **avm:** Include bb-pilcom in monorepo
([#7098](AztecProtocol/aztec-packages#7098))
([0442158](AztecProtocol/aztec-packages@0442158))
* Conventional lookups using log-deriv
([#7020](AztecProtocol/aztec-packages#7020))
([6f1212f](AztecProtocol/aztec-packages@6f1212f))
* Enable merge recursive verifier in Goblin recursive verifier
([#7182](AztecProtocol/aztec-packages#7182))
([9b4f56c](AztecProtocol/aztec-packages@9b4f56c))
* Several updates in SMT verification module
([#7105](AztecProtocol/aztec-packages#7105))
([41b21f1](AztecProtocol/aztec-packages@41b21f1))
* Shplonk revival in ECCVM
([#7164](AztecProtocol/aztec-packages#7164))
([34eb5a0](AztecProtocol/aztec-packages@34eb5a0))


### Bug Fixes

* **avm:** Fix unencryptedlog c++ deser
([#7194](AztecProtocol/aztec-packages#7194))
([89a99af](AztecProtocol/aztec-packages@89a99af))
* **avm:** Re-enable ext call test
([#7147](AztecProtocol/aztec-packages#7147))
([33ccf1b](AztecProtocol/aztec-packages@33ccf1b))
* **avm:** Reenable tag error sload
([#7153](AztecProtocol/aztec-packages#7153))
([fd92d46](AztecProtocol/aztec-packages@fd92d46))
* **avm:** Update codegen
([#7178](AztecProtocol/aztec-packages#7178))
([1d29708](AztecProtocol/aztec-packages@1d29708))
* Bug fixing bench prover test
([#7135](AztecProtocol/aztec-packages#7135))
([13678be](AztecProtocol/aztec-packages@13678be)),
closes
[#7080](AztecProtocol/aztec-packages#7080)
* Fix bug for a unit test in full proving mode repated to MSM
([#7104](AztecProtocol/aztec-packages#7104))
([e37809b](AztecProtocol/aztec-packages@e37809b))


### Miscellaneous

* **avm:** Remove avm prefix from pil and executor
([#7099](AztecProtocol/aztec-packages#7099))
([b502fcd](AztecProtocol/aztec-packages@b502fcd))
* **avm:** Renamings and comments
([#7128](AztecProtocol/aztec-packages#7128))
([ed2f98e](AztecProtocol/aztec-packages@ed2f98e))
* **avm:** Separate some fixed tables
([#7163](AztecProtocol/aztec-packages#7163))
([1d4a9a2](AztecProtocol/aztec-packages@1d4a9a2))
* Create workflow for full AVM tests
([#7051](AztecProtocol/aztec-packages#7051))
([a0b9c4b](AztecProtocol/aztec-packages@a0b9c4b)),
closes
[#6643](AztecProtocol/aztec-packages#6643)
* Indirects and read/write slices
([#7082](AztecProtocol/aztec-packages#7082))
([d5e80ee](AztecProtocol/aztec-packages@d5e80ee))
* Reads the return data
([#6669](AztecProtocol/aztec-packages#6669))
([ef85542](AztecProtocol/aztec-packages@ef85542))
* Remove unneeded public input folding
([#7094](AztecProtocol/aztec-packages#7094))
([c30dc38](AztecProtocol/aztec-packages@c30dc38))
* Take the PCS out of Zeromorph and refactor tests
([#7078](AztecProtocol/aztec-packages#7078))
([e192678](AztecProtocol/aztec-packages@e192678))
* Ultra flavor cleanup
([#7070](AztecProtocol/aztec-packages#7070))
([77761c6](AztecProtocol/aztec-packages@77761c6))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
superstar0402 added a commit to superstar0402/aztec-nr that referenced this issue Aug 16, 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 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
2 participants