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

[EPIC] AVM simulator & witgen handle recoverable reverts #9061

Closed
2 tasks done
dbanks12 opened this issue Oct 7, 2024 · 0 comments
Closed
2 tasks done

[EPIC] AVM simulator & witgen handle recoverable reverts #9061

dbanks12 opened this issue Oct 7, 2024 · 0 comments
Labels
C-avm Component: AVM related tickets (aka public VM) T-epic team-bonobos
Milestone

Comments

@dbanks12
Copy link
Contributor

dbanks12 commented Oct 7, 2024

Tasks

Related

@dbanks12 dbanks12 added the C-avm Component: AVM related tickets (aka public VM) label Oct 7, 2024
@dbanks12 dbanks12 added this to A3 Oct 7, 2024
@github-project-automation github-project-automation bot moved this to Todo in A3 Oct 7, 2024
@dbanks12 dbanks12 added T-tracking Type: Tracking Issue. This contains tasklists. T-epic labels Oct 7, 2024
@dbanks12 dbanks12 changed the title AVM (simulator and circuit) properly handles recoverable reverts [EPIC] AVM: simulator and circuit properly handle recoverable reverts Oct 7, 2024
@dbanks12 dbanks12 changed the title [EPIC] AVM: simulator and circuit properly handle recoverable reverts [EPIC] AVM simulator and circuit properly handle recoverable reverts Oct 7, 2024
@dbanks12 dbanks12 added this to the TestNet milestone Oct 7, 2024
@dbanks12 dbanks12 changed the title [EPIC] AVM simulator and circuit properly handle recoverable reverts [EPIC] AVM simulator & witgen handle recoverable reverts Oct 27, 2024
@iAmMichaelConnor iAmMichaelConnor removed the T-tracking Type: Tracking Issue. This contains tasklists. label Oct 28, 2024
fcarreiro added a commit that referenced this issue Oct 29, 2024
This PR
* Introduces RETURNDATASIZE and RETURNDATACOPY (also copies revert data if reverted)
* Fixes a bug in CALL in witgen
* Changes the public context to return slices when calling public functions. This was partly done because templated functions would always be inlined and I wanted to avoid that blowup

Note that the rethrowing hack is still present in the simulator, so the rethrowing branch in the public context is still not used. I will make this change once @sirasistant finishes the string encoding changes.

In a later PR we can remove the returndata from CALL.

Part of #9061.
fcarreiro added a commit that referenced this issue Oct 29, 2024
* (Static)CALL now returns just the success bit
* Properly returns U1 instead of U8
* Removed FunctionSelector

Fixes #8998. Part of #9061.
ludamad pushed a commit that referenced this issue Nov 5, 2024
This PR removes the RethrowableError hack in the AVM simulator, and
relies on the PublicContext's
[rethrow](https://github.com/AztecProtocol/aztec-packages/blob/master/noir-projects/aztec-nr/aztec/src/context/public_context.nr#L88)
to propagate the errors. There are two caveats.

First, because currently Aztec-nr does not keep track of the cause
chain, it would be impossible to have the call stack and original
contract address available, so that the PXE can interpret the error and
show debug information. Solidity has the same problem. I'm introducing a
heuristic to keep track of the call stack for the simple case where the
caller always rethrows: the simulator keeps a running trace in the
machineState, and the caller uses this trace IF the revertData
coincides. That is, if you are (re)throwing the same as what we were
tracking.

Second, while this all works well for errors in user code (e.g.,
`assert` in Noir), because they generate a revertData with an error
selector and data, the "intrinsic" errors from the simulator (aka
exceptional halts) do not work as well. E.g., "division by zero",
"duplicated nullifier", "l1 to l2 blah blah". These errors are
exceptions in typescript and do not have an associated error selector,
and do not add to the revertdata. This _could_ be done with enshrined
error selectors. That's easy in the simulator, but it's not easy in the
circuit for several reasons that are beyond the scope of this
description. The current solution is to propagate the error message (the
user will see the right error) but you cannot "catch and process" the
error in aztec.nr because there is no selector. This is not a limitation
right now because there's no interface in the PublicContext that would
let you catch errors. To be continued.

Part of #9061.
Maddiaa0 pushed a commit that referenced this issue Nov 6, 2024
This PR removes the RethrowableError hack in the AVM simulator, and
relies on the PublicContext's
[rethrow](https://github.com/AztecProtocol/aztec-packages/blob/master/noir-projects/aztec-nr/aztec/src/context/public_context.nr#L88)
to propagate the errors. There are two caveats.

First, because currently Aztec-nr does not keep track of the cause
chain, it would be impossible to have the call stack and original
contract address available, so that the PXE can interpret the error and
show debug information. Solidity has the same problem. I'm introducing a
heuristic to keep track of the call stack for the simple case where the
caller always rethrows: the simulator keeps a running trace in the
machineState, and the caller uses this trace IF the revertData
coincides. That is, if you are (re)throwing the same as what we were
tracking.

Second, while this all works well for errors in user code (e.g.,
`assert` in Noir), because they generate a revertData with an error
selector and data, the "intrinsic" errors from the simulator (aka
exceptional halts) do not work as well. E.g., "division by zero",
"duplicated nullifier", "l1 to l2 blah blah". These errors are
exceptions in typescript and do not have an associated error selector,
and do not add to the revertdata. This _could_ be done with enshrined
error selectors. That's easy in the simulator, but it's not easy in the
circuit for several reasons that are beyond the scope of this
description. The current solution is to propagate the error message (the
user will see the right error) but you cannot "catch and process" the
error in aztec.nr because there is no selector. This is not a limitation
right now because there's no interface in the PublicContext that would
let you catch errors. To be continued.

Part of #9061.
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-avm Component: AVM related tickets (aka public VM) T-epic team-bonobos
Projects
Status: Done
Development

No branches or pull requests

2 participants