Skip to content

Refactor feature extensions out of VMManager #2578

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

Merged
merged 6 commits into from
Jan 19, 2024

Conversation

joshua-kim
Copy link
Contributor

@joshua-kim joshua-kim commented Jan 3, 2024

Why this should be merged

Currently, all feature extension code is registered as a VM (even though they aren't actually VMs) under the VMManager. This leads to strange hacks like returning the opaque any type since we can return either a vm factory or an fx factory, which leads to us also having to perform more type casting to sanity check that we're actually using a vm instead of a potential fx.

This PR hard-codes the fxs that we're using to just be a map, effectively deprecating plugin feature extensions that aren't used anyways. Only the AVM as far as I am aware is using these.

How this was tested

CI passes. Calling the GetVMs api additionally now only returns vms, instead of previously returning the feature extensions as well.

➜  subnet-evm git:(master) curl -sX POST --data '{
    "jsonrpc":"2.0",
    "id"     :1,
    "method" :"info.getVMs",
    "params" :{}
}' -H 'content-type:application/json;' 127.0.0.1:9650/ext/info  | jq
{
  "jsonrpc": "2.0",
  "result": {
    "vms": {
      "jvYyfQTxGMJLuGWa55kdP2p2zSUYsQ5Raupu4TW34ZAUBAbtq": [
        "avm"
      ],
      "mgj786NP7uDwBCcq6YwThhaN8FLyybkCa4zBWTQbNgmK6k9A6": [
        "evm"
      ],
      "rWhpuQPF1kb72esV2momhMuTYGkEb1oL29pt2EBXWmSy4kxnT": [
        "platform"
      ]
    }
  },
  "id": 1
}

@joshua-kim joshua-kim force-pushed the refactor-fx-factory branch from 53b9485 to 4763258 Compare January 3, 2024 06:47
@joshua-kim joshua-kim self-assigned this Jan 17, 2024
@joshua-kim joshua-kim added the cleanup Code quality improvement label Jan 17, 2024
@joshua-kim joshua-kim marked this pull request as ready for review January 17, 2024 04:55

// Factory returns an instance of a feature extension
type Factory interface {
New() any
Copy link
Contributor Author

@joshua-kim joshua-kim Jan 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another idea is to return an fx.Interface interface here, but we're already currently returning the any type so I'm going to punt this off to a follow-up since my focus is to currently just refactor the vm code.

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
@joshua-kim joshua-kim changed the base branch from dev to master January 19, 2024 18:30
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
@StephenButtolph StephenButtolph added this to the v1.10.19 milestone Jan 19, 2024
@StephenButtolph StephenButtolph added this pull request to the merge queue Jan 19, 2024
Merged via the queue into master with commit 7cea467 Jan 19, 2024
@StephenButtolph StephenButtolph deleted the refactor-fx-factory branch January 19, 2024 23:12
chand1012 pushed a commit to multisig-labs/avalanchego that referenced this pull request Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code quality improvement
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants