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

AVM: cleanup simulator #5818

Open
26 of 40 tasks
dbanks12 opened this issue Apr 17, 2024 · 1 comment
Open
26 of 40 tasks

AVM: cleanup simulator #5818

dbanks12 opened this issue Apr 17, 2024 · 1 comment
Labels
C-avm Component: AVM related tickets (aka public VM)

Comments

@dbanks12
Copy link
Contributor

dbanks12 commented Apr 17, 2024

Journal

  • Rename journal file to match class name. Rename all instances of journal.
  • Rename flush function to be more meaningful (and JournalData type) (do we even need this function & type?)
  • Make any members private that can be made so
  • Add max-array checks in trace class (AVM: limit max state accesses / side-effects in simulator #4805)
  • Consider whether CallPointer and EndLifetime belong in trace at this stage
  • Remove transitional logic (eventually)
  • What needs to be tracked in a trace array vs journal array?
  • Split nullifier checks into non-existent and existent
  • Add L2ToL1Message read requests to PublicExecutionResult and public kernel
  • Rename Nullifiers type (bad name)
  • Stop committing to public storage DB in executor.ts and make stateManager's publicStorage private

Contracts and tests

AvmContextInputs

  • Decide if we'll want it long term or not (we don't)
  • Remove 'selector' and create opcode?
  • Remove 'is_static' and create opcode?
  • Remove args_hash? Do this in Noir in the macros?

Transition-related

  • Clean up noir macros
  • Remove public_execution_context
  • Clean up ACVM stuff in executor.ts and abstract_phase_manager
  • Cleanup/simplify interaction of PublicExecution(Result) inside the simulator and in executor.ts and abstract_phase_manager
  • Remove transitional_adaptors
  • Remove AVM_MAGIC_SUFFIX and related (from transpiler as well) (not removing)
  • Clean up ACIR public simulator tests in simulator/public
  • Clean up (Public)ContextInterfaces

Misc

  • AVM: fix instruction -> AvmContext -> decodeFromBytecode dependency cycle in simulator #4359
  • re-group instructions into files that match YP categories (rename when necessary)
  • rename avm_message_call_results file
  • types in transpile_contract.rs should be renamed to match their corresponding names in nargo/src/artifacts/{program.rs,contract.rs} (or to just import the types themselves instead of recreating them - but they might not be exported)
  • can transpiler ever actually encounter a Const with bit_size: 254, or can we remove that special case?
  • Gas handling could use a simplification
  • Update references to AvmContext in docs/docs/protocol-specs/public-vm/context.mdx
  • Consider removing header from the environment: feat(avm): add Header to AvmExecutionEnvironment #5592
@dbanks12 dbanks12 added the C-avm Component: AVM related tickets (aka public VM) label Apr 17, 2024
@dbanks12 dbanks12 added this to the AVM Simulator Phase 2 (full) milestone Apr 17, 2024
@dbanks12 dbanks12 added this to A3 Apr 17, 2024
@github-project-automation github-project-automation bot moved this to Todo in A3 Apr 17, 2024
@dbanks12
Copy link
Contributor Author

@fcarreiro pinging you so you're aware this ticket exists. We can probably knock many of these out in a single PR. Some of them I have procrastinated on for a while.

We might want to make this ticket more general to "simulator cleanup" or make another such ticket to track simulator cleanup tasks.

@dbanks12 dbanks12 changed the title chore(avm-simulator): cleanup journal chore(avm-simulator): cleanup simulator May 9, 2024
dbanks12 added a commit that referenced this issue May 21, 2024
This PR migrates the public (execution) environment to use the AVM
simulator. The idea of this PR is to be as minimal as possible, as to
enable easy rollbacks if needed. If things go well, there is a lot of
cleanup to do afterwards, which we are tracking in [this
issue](#5818).

Major Changes
* `PublicContext` gets replaced with what was the `AvmContext`.
* Noir Aztec macros now always take the AVM path for public.
* Migration notes are added.

Other changes
* Delegate call tests are disabled, since the AVM doesn't support
delegate calls.
* ACIR public execution tests are disabled.
* Fees were changed in `bench_tx_size_fees` since the cost is now
different (bytecode and L2 gas changes).

---------

Co-authored-by: dbanks12 <[email protected]>
AztecBot pushed a commit to AztecProtocol/aztec-nr that referenced this issue May 22, 2024
This PR migrates the public (execution) environment to use the AVM
simulator. The idea of this PR is to be as minimal as possible, as to
enable easy rollbacks if needed. If things go well, there is a lot of
cleanup to do afterwards, which we are tracking in [this
issue](AztecProtocol/aztec-packages#5818).

Major Changes
* `PublicContext` gets replaced with what was the `AvmContext`.
* Noir Aztec macros now always take the AVM path for public.
* Migration notes are added.

Other changes
* Delegate call tests are disabled, since the AVM doesn't support
delegate calls.
* ACIR public execution tests are disabled.
* Fees were changed in `bench_tx_size_fees` since the cost is now
different (bytecode and L2 gas changes).

---------

Co-authored-by: dbanks12 <[email protected]>
fcarreiro added a commit that referenced this issue May 31, 2024
fcarreiro added a commit that referenced this issue May 31, 2024
Remove `public_execution_context` & move a few things around. Nice
cleanup in `executor.ts`.

I'd like to get rid of `execution` in the result, but that'll require a
bit more effort.

Part of #5818.
fcarreiro added a commit that referenced this issue Jun 7, 2024
fcarreiro added a commit that referenced this issue Jun 12, 2024
fcarreiro added a commit that referenced this issue Jun 12, 2024
fcarreiro added a commit that referenced this issue Jun 12, 2024
fcarreiro added a commit that referenced this issue Jun 12, 2024
@dbanks12 dbanks12 changed the title chore(avm-simulator): cleanup simulator AVM: cleanup simulator Jun 19, 2024
@dbanks12 dbanks12 removed this from the AVM Simulator Phase 2 (full) milestone Oct 7, 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)
Projects
Status: Todo
Development

No branches or pull requests

1 participant