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

Snow VM #1831

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

Snow VM #1831

wants to merge 16 commits into from

Conversation

aaronbuchwald
Copy link
Collaborator

This refactor is intended to separate out maintenance of the AvalancheGo specific invariants and initialization from the VM package.

Prior to this PR, the VM package:

  • initializes all of the required components of the chain with the params supplied by AvalancheGo
  • Implements the snowman.Block, block.ChainVM, and block.StateSyncableVM interfaces

This PR aims to reduce the responsibilities of the VM package as much as possible. Note: #1744 moved the StatefulBlock type out of the chain package and into VM. This continues to remove AvalancheGo specific functionality from the HyperVM specific components.

The goal of this refactor is to get to a point where:

  • snow package implements snowman.Block and block.ChainVM and maintains invariants required by consensus engine
  • snow package can be tested independently of HyperSDK chain logic that it correctly maintains those invariants
  • snow package "decomposes" the AvalancheGo VM interface into components with given defaults and can be overridden by an individual chain (currently Options, but should be renamed as it doesn't quite fit under this pattern)
  • rename vm package to hypervm
  • vm package plays the role of initializing all of the required components and implementing a simpler interface that handles the required block state transitions
  • can write integration tests for the vm package that do not need to cover consensus engine edge cases
  • makes it possible to implement both DSMR and non-DSMR version of HyperSDK (use DSMR block type and produce chain.OutputBlock after accept that can be sent to relevant APIs)

@aaronbuchwald aaronbuchwald added the DO NOT MERGE This PR is not meant to be merged in its current state label Dec 10, 2024
snow/vm.go Fixed Show fixed Hide fixed
vm/vm.go Fixed Show fixed Hide fixed
vm/vm.go Fixed Show fixed Hide fixed
vm/vm.go Fixed Show fixed Hide fixed
vm/vm.go Fixed Show fixed Hide fixed
vm/vm.go Fixed Show fixed Hide fixed
vm/vm.go Fixed Show fixed Hide fixed
vm/vm.go Fixed Show fixed Hide fixed
vm/vm.go Fixed Show fixed Hide fixed
vm/vm.go Fixed Show fixed Hide fixed
@aaronbuchwald
Copy link
Collaborator Author

aaronbuchwald commented Dec 11, 2024

Next steps:

  1. Clean up dynamic state sync and write snow dynamic state sync tests
  2. Move general purpose config out of snow package and replace with "context" that can either be a singleton or passed around similar to require.Equal(t, a, b) or r.Equal(a, b)
  3. Update out of date comments
  4. Replace "Options" misnomer (Configuration?)
  5. Handle cyclic dependency with chain index (chain must currently return last accepted block and potentially re-process blocks)
  6. Update VM package + rename to HyperVM + add HyperVM integration tests

Most importantly break this PR up

}
vm.snowApp.WithCloser(chainStoreDB.Close)
vm.chainStore, err = chainstore.New[*chain.ExecutionBlock](vm.snowInput.Context, vm.chain, chainStoreDB)

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This definition of err is never used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE This PR is not meant to be merged in its current state
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant