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

Combine APIs + Controller #1224

Closed
aaronbuchwald opened this issue Jul 30, 2024 · 1 comment
Closed

Combine APIs + Controller #1224

aaronbuchwald opened this issue Jul 30, 2024 · 1 comment
Assignees

Comments

@aaronbuchwald
Copy link
Collaborator

The original controller implementation bundled together multiple components following a similar design of VMs in Avalanche, where the VM implements the full interface required by the parent.

Now that we've broken down the controller into individual components, the only thing that will remain after the genesis/upgrade bytes has been factored out w/ #1217 will be the APIs.

This ticket is to simplify the controller down to exactly what's needed by the API with the dependencies supplied directly by the HyperSDK (as laid out in #1217).

We should update from using the controller interface as it's used now to an API factory that produces the API handlers and takes in the exact arguments that a VM should need in order to implement their APIs. In general, there are three types of APIs that a VM may want to support:

  1. write activity (issue a transaction)
  2. current state reads (needs reference to latest state)
  3. historical queries (needs to implement its own indexing w/ AcceptedSubscriber Add Tx Indexer Extension #1143)

The new API/controller refactor should pass all of the requirements in to enable a VM to select which standard APIs to import from the HyperSDK and which to implement itself (ie current state APIs that require an understanding of the VM's state layout).

This should look approximately like:

type CurrentStateReader interface {
	// Returns a view of the current state
	// Currently the VM can only perform a single batch of atomic reads (holds a lock on the merkledb)
	// an improvement on this would be to pass a full view of the current state that can perform a
	// sequence of reads without worrying about the state changing underneath it (should be possible with merkledb)
	// Need to handle garbage collection of this view correctly.
	// First pass implementation can just use the same locked read function that the VM currently uses
	GetCurrentState() (View, error)
}

type AcceptedSubscriberRegistry interface {
	Register(subscriber indexer.AcceptedSubscriber) error
}

type Mempool interface {
	IssueTx(tx *Tx) error
}

type APIFactory interface {
	New(genesis Genesis, currentStateReader CurrentStateReader, registry AcceptedSubscriberRegistry, mempool Mempool) http.Handler
}

Currently, the HyperSDK/example VMs share the responsibility of implementing user APIs. It can be a bit unintuitive what API should be part of the HyperSDK vs. part of the VM. I suggest that as part of this refactor we move all of the APIs to be served by the VM. To reduce code duplication, we can offer a helper function that pre-supplies the standard set of APIs the HyperSDK would otherwise implement, so that the VM only needs to implement its own custom APIs.

@joshua-kim joshua-kim self-assigned this Jul 30, 2024
@joshua-kim
Copy link
Contributor

Related: #1225

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants