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

Refactor chain manager subnets #2711

Merged
merged 10 commits into from
Feb 12, 2024
Merged

Conversation

joshua-kim
Copy link
Contributor

@joshua-kim joshua-kim commented Feb 7, 2024

Why this should be merged

Needed as a pre-requisite to remove vm.Initialize. This refactors the currently running subnets into its own struct, so it can be shared with a future chains.Factory struct which will be responsible for creating chain instances (currently this is created internally in the chain manager). Ideally we'll have a chain factory to create chains and a chain manager that holds the chains that are currently running. This way, the chain manager doesn't need to know about the p-chain at all since the caller can choose arguments passed into the chain factory instead and the p-chain logic can be in the caller instead of in chains/manager

How this works

Refactors currently running subnets into its own struct

How this was tested

Added unit tests

@joshua-kim joshua-kim force-pushed the refactor-chains-subnets branch 4 times, most recently from 357bc93 to bed8d13 Compare February 7, 2024 16:06
@joshua-kim joshua-kim changed the title refactor chain manager subnets Refactor chain manager subnets Feb 7, 2024
@joshua-kim joshua-kim force-pushed the refactor-chains-subnets branch 4 times, most recently from b220924 to a8e7e35 Compare February 7, 2024 16:37
@joshua-kim joshua-kim self-assigned this Feb 7, 2024
@joshua-kim joshua-kim added the cleanup Code quality improvement label Feb 7, 2024
@joshua-kim joshua-kim marked this pull request as ready for review February 7, 2024 16:41
@joshua-kim joshua-kim added this to the v1.11.0 milestone Feb 7, 2024
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Copy link
Collaborator

@aaronbuchwald aaronbuchwald left a comment

Choose a reason for hiding this comment

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

LGTM with optional nits

chains/subnets.go Outdated Show resolved Hide resolved
chains/subnets.go Outdated Show resolved Hide resolved
chains/manager.go Outdated Show resolved Hide resolved
chains/subnets.go Outdated Show resolved Hide resolved
joshua-kim and others added 3 commits February 7, 2024 13:00
Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Copy link
Contributor

@StephenButtolph StephenButtolph left a comment

Choose a reason for hiding this comment

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

Running locally resulted in:

[02-10|13:01:49.389] INFO node/node.go:899 initializing chains
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x1028a12c8]

goroutine 1 [running]:
github.com/ava-labs/avalanchego/chains.(*Subnets).Get(0x0?, {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...})
        /Users/stephen/go/src/github.com/ava-labs/avalanchego/chains/subnets.go:50 +0x28
github.com/ava-labs/avalanchego/chains.(*manager).StartChainCreator(0x14002a64380, {{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...}, ...})
        /Users/stephen/go/src/github.com/ava-labs/avalanchego/chains/manager.go:1337 +0x4c
github.com/ava-labs/avalanchego/node.(*Node).initChains(0x1400024b200, {0x1400175f800, 0x1bc2, 0x3800})
        /Users/stephen/go/src/github.com/ava-labs/avalanchego/node/node.go:910 +0x12c
github.com/ava-labs/avalanchego/node.New(0x140000d7600, {0x1039e9b18, 0x14000362380}, {0x1039f0320, 0x14000398080})
        /Users/stephen/go/src/github.com/ava-labs/avalanchego/node/node.go:261 +0x14e4
github.com/ava-labs/avalanchego/app.New({{{0x6fc23ac00, 0x6fc23ac00, 0x6fc23ac00, 0x1bf08eb000}, {{0x0, {...}}, {0x0, 0x0}, {0x0, {...}, ...}, ...}, ...}, ...})
        /Users/stephen/go/src/github.com/ava-labs/avalanchego/app/app.go:73 +0x3a0
main.main()
        /Users/stephen/go/src/github.com/ava-labs/avalanchego/main/main.go:48 +0x2cc

(which is presumably why all the E2E tests are failing).

chains/manager.go Outdated Show resolved Hide resolved
chains/subnets.go Outdated Show resolved Hide resolved
chains/subnets.go Outdated Show resolved Hide resolved
chains/manager.go Outdated Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
joshua-kim and others added 4 commits February 12, 2024 12:24
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Co-authored-by: Stephen Buttolph <stephen@avalabs.org>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
chains/subnets_test.go Outdated Show resolved Hide resolved
chains/subnets_test.go Outdated Show resolved Hide resolved
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
@StephenButtolph StephenButtolph added this pull request to the merge queue Feb 12, 2024
Merged via the queue into master with commit f1f88ac Feb 12, 2024
17 checks passed
@StephenButtolph StephenButtolph deleted the refactor-chains-subnets branch February 12, 2024 23:28
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.

None yet

4 participants