From a62538691f896d68bdbe8a214216b6d792042eeb Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Wed, 7 Feb 2024 10:50:47 -0500 Subject: [PATCH 01/10] refactor chain manager subnets Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- chains/manager.go | 73 ++++------------- chains/subnets.go | 87 ++++++++++++++++++++ chains/subnets_test.go | 177 +++++++++++++++++++++++++++++++++++++++++ node/node.go | 99 ++++++++++++----------- 4 files changed, 335 insertions(+), 101 deletions(-) create mode 100644 chains/subnets.go create mode 100644 chains/subnets_test.go diff --git a/chains/manager.go b/chains/manager.go index 48f2fee35704..59f68001e9c4 100644 --- a/chains/manager.go +++ b/chains/manager.go @@ -94,11 +94,11 @@ var ( // Bootstrapping prefixes for ChainVMs ChainBootstrappingDBPrefix = []byte("bs") - errUnknownVMType = errors.New("the vm should have type avalanche.DAGVM or snowman.ChainVM") - errCreatePlatformVM = errors.New("attempted to create a chain running the PlatformVM") - errNotBootstrapped = errors.New("subnets not bootstrapped") - errNoPrimaryNetworkConfig = errors.New("no subnet config for primary network found") - errPartialSyncAsAValidator = errors.New("partial sync should not be configured for a validator") + errUnknownVMType = errors.New("the vm should have type avalanche.DAGVM or snowman.ChainVM") + errCreatePlatformVM = errors.New("attempted to create a chain running the PlatformVM") + errNotBootstrapped = errors.New("subnets not bootstrapped") + errPrimaryNetworkNotRunning = errors.New("node is not running the primary network") + errPartialSyncAsAValidator = errors.New("partial sync should not be configured for a validator") fxs = map[ids.ID]fx.Factory{ secp256k1fx.ID: &secp256k1fx.Factory{}, @@ -256,10 +256,7 @@ type manager struct { chainCreatorShutdownCh chan struct{} chainCreatorExited sync.WaitGroup - subnetsLock sync.RWMutex - // Key: Subnet's ID - // Value: Subnet description - subnets map[ids.ID]subnets.Subnet + subnets *Subnets chainsLock sync.Mutex // Key: Chain's ID @@ -271,13 +268,13 @@ type manager struct { } // New returns a new Manager -func New(config *ManagerConfig) Manager { +func New(config *ManagerConfig, subnets *Subnets) Manager { return &manager{ Aliaser: ids.NewAliaser(), ManagerConfig: *config, stakingSigner: config.StakingTLSCert.PrivateKey.(crypto.Signer), stakingCert: staking.CertificateFromX509(config.StakingTLSCert.Leaf), - subnets: make(map[ids.ID]subnets.Subnet), + subnets: subnets, chains: make(map[ids.ID]handler.Handler), chainsQueue: buffer.NewUnboundedBlockingDeque[ChainParameters](initialQueueSize), unblockChainCreatorCh: make(chan struct{}), @@ -288,25 +285,11 @@ func New(config *ManagerConfig) Manager { // QueueChainCreation queues a chain creation request // Invariant: Tracked Subnet must be checked before calling this function func (m *manager) QueueChainCreation(chainParams ChainParameters) { - m.subnetsLock.Lock() - subnetID := chainParams.SubnetID - sb, exists := m.subnets[subnetID] - if !exists { - sbConfig, ok := m.SubnetConfigs[subnetID] - if !ok { - // default to primary subnet config - sbConfig = m.SubnetConfigs[constants.PrimaryNetworkID] - } - sb = subnets.New(m.NodeID, sbConfig) - m.subnets[chainParams.SubnetID] = sb - } - addedChain := sb.AddChain(chainParams.ID) - m.subnetsLock.Unlock() - - if !addedChain { + _ = m.subnets.Add(chainParams.SubnetID) + if sb, _ := m.subnets.Get(chainParams.SubnetID); !sb.AddChain(chainParams.ID) { m.Log.Debug("skipping chain creation", zap.String("reason", "chain already staged"), - zap.Stringer("subnetID", subnetID), + zap.Stringer("subnetID", chainParams.SubnetID), zap.Stringer("chainID", chainParams.ID), zap.Stringer("vmID", chainParams.VMID), ) @@ -316,7 +299,7 @@ func (m *manager) QueueChainCreation(chainParams ChainParameters) { if ok := m.chainsQueue.PushRight(chainParams); !ok { m.Log.Warn("skipping chain creation", zap.String("reason", "couldn't enqueue chain"), - zap.Stringer("subnetID", subnetID), + zap.Stringer("subnetID", chainParams.SubnetID), zap.Stringer("chainID", chainParams.ID), zap.Stringer("vmID", chainParams.VMID), ) @@ -334,9 +317,7 @@ func (m *manager) createChain(chainParams ChainParameters) { zap.Stringer("vmID", chainParams.VMID), ) - m.subnetsLock.RLock() - sb := m.subnets[chainParams.SubnetID] - m.subnetsLock.RUnlock() + sb, _ := m.subnets.Get(chainParams.SubnetID) // Note: buildChain builds all chain's relevant objects (notably engine and handler) // but does not start their operations. Starting of the handler (which could potentially @@ -1307,23 +1288,9 @@ func (m *manager) IsBootstrapped(id ids.ID) bool { return chain.Context().State.Get().State == snow.NormalOp } -func (m *manager) subnetsNotBootstrapped() []ids.ID { - m.subnetsLock.RLock() - defer m.subnetsLock.RUnlock() - - subnetsBootstrapping := make([]ids.ID, 0, len(m.subnets)) - for subnetID, subnet := range m.subnets { - if !subnet.IsBootstrapped() { - subnetsBootstrapping = append(subnetsBootstrapping, subnetID) - } - } - return subnetsBootstrapping -} - func (m *manager) registerBootstrappedHealthChecks() error { bootstrappedCheck := health.CheckerFunc(func(context.Context) (interface{}, error) { - subnetIDs := m.subnetsNotBootstrapped() - if len(subnetIDs) != 0 { + if subnetIDs := m.subnets.Bootstrapping(); len(subnetIDs) != 0 { return subnetIDs, errNotBootstrapped } return []ids.ID{}, nil @@ -1365,18 +1332,12 @@ func (m *manager) registerBootstrappedHealthChecks() error { // Starts chain creation loop to process queued chains func (m *manager) StartChainCreator(platformParams ChainParameters) error { - // Get the Primary Network's subnet config. If it wasn't registered, then we - // throw a fatal error. - sbConfig, ok := m.SubnetConfigs[constants.PrimaryNetworkID] + // Add the P-Chain to the Primary Network + sb, ok := m.subnets.Get(constants.PrimaryNetworkID) if !ok { - return errNoPrimaryNetworkConfig + return errPrimaryNetworkNotRunning } - - sb := subnets.New(m.NodeID, sbConfig) - m.subnetsLock.Lock() - m.subnets[platformParams.SubnetID] = sb sb.AddChain(platformParams.ID) - m.subnetsLock.Unlock() // The P-chain is created synchronously to ensure that `VM.Initialize` has // finished before returning from this function. This is required because diff --git a/chains/subnets.go b/chains/subnets.go new file mode 100644 index 000000000000..5017ba76fffe --- /dev/null +++ b/chains/subnets.go @@ -0,0 +1,87 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package chains + +import ( + "errors" + "sync" + + "github.com/ava-labs/avalanchego/ids" + "github.com/ava-labs/avalanchego/subnets" + "github.com/ava-labs/avalanchego/utils/constants" +) + +var ErrNoPrimaryNetworkConfig = errors.New("no subnet config for primary network found") + +func NewSubnets( + nodeID ids.NodeID, + configs map[ids.ID]subnets.Config, +) (*Subnets, error) { + if _, ok := configs[constants.PrimaryNetworkID]; !ok { + return nil, ErrNoPrimaryNetworkConfig + } + + s := &Subnets{ + nodeID: nodeID, + configs: configs, + subnets: make(map[ids.ID]subnets.Subnet), + } + + _ = s.Add(constants.PrimaryNetworkID) + return s, nil +} + +// Subnets holds the currently running subnets on this node +type Subnets struct { + nodeID ids.NodeID + configs map[ids.ID]subnets.Config + + lock sync.Mutex + subnets map[ids.ID]subnets.Subnet +} + +// Add a subnet that is being run on this node +func (s *Subnets) Add(subnetID ids.ID) bool { + s.lock.Lock() + defer s.lock.Unlock() + + if _, ok := s.subnets[subnetID]; ok { + return false + } + + // Default to the primary network config if a subnet config was not + // specified + config, ok := s.configs[subnetID] + if !ok { + config = s.configs[constants.PrimaryNetworkID] + } + + s.subnets[subnetID] = subnets.New(s.nodeID, config) + return true +} + +// Get returns a subnet if it is being run on this node +func (s *Subnets) Get(subnetID ids.ID) (subnets.Subnet, bool) { + s.lock.Lock() + defer s.lock.Unlock() + + subnet, ok := s.subnets[subnetID] + return subnet, ok +} + +// Bootstrapping returns subnets that have any chains that are still +// bootstrapping. +func (s *Subnets) Bootstrapping() []ids.ID { + s.lock.Lock() + defer s.lock.Unlock() + + subnetsBootstrapping := make([]ids.ID, 0, len(s.subnets)) + for subnetID, subnet := range s.subnets { + if !subnet.IsBootstrapped() { + subnetsBootstrapping = append(subnetsBootstrapping, subnetID) + } + } + + return subnetsBootstrapping +} diff --git a/chains/subnets_test.go b/chains/subnets_test.go new file mode 100644 index 000000000000..5fb12660e15a --- /dev/null +++ b/chains/subnets_test.go @@ -0,0 +1,177 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package chains + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/ava-labs/avalanchego/ids" + "github.com/ava-labs/avalanchego/subnets" + "github.com/ava-labs/avalanchego/utils/constants" +) + +func TestNewSubnets(t *testing.T) { + require := require.New(t) + config := map[ids.ID]subnets.Config{ + constants.PrimaryNetworkID: {}, + } + + subnets, err := NewSubnets(ids.EmptyNodeID, config) + require.NoError(err) + + subnet, ok := subnets.Get(constants.PrimaryNetworkID) + require.True(ok) + require.Equal(subnet.Config(), config[constants.PrimaryNetworkID]) +} + +func TestNewSubnets_NoPrimaryNetworkConfig(t *testing.T) { + require := require.New(t) + config := map[ids.ID]subnets.Config{} + + _, err := NewSubnets(ids.EmptyNodeID, config) + require.ErrorIs(err, ErrNoPrimaryNetworkConfig) +} + +func TestSubnetsAdd(t *testing.T) { + testSubnetID := ids.GenerateTestID() + + type add struct { + subnetID ids.ID + want bool + } + + tests := []struct { + name string + adds []add + }{ + { + name: "adding duplicate subnet is a noop", + adds: []add{ + { + subnetID: testSubnetID, + want: true, + }, + { + subnetID: testSubnetID, + }, + }, + }, + { + name: "adding unique subnets succeeds", + adds: []add{ + { + subnetID: ids.GenerateTestID(), + want: true, + }, + { + subnetID: ids.GenerateTestID(), + want: true, + }, + { + subnetID: ids.GenerateTestID(), + want: true, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + config := map[ids.ID]subnets.Config{ + constants.PrimaryNetworkID: {}, + } + subnets, err := NewSubnets(ids.EmptyNodeID, config) + require.NoError(err) + + for _, add := range tt.adds { + got := subnets.Add(add.subnetID) + require.Equal(got, add.want) + } + }) + } +} + +func TestSubnetConfigs(t *testing.T) { + testSubnetID := ids.GenerateTestID() + + tests := []struct { + name string + config map[ids.ID]subnets.Config + subnetID ids.ID + want subnets.Config + }{ + { + name: "default to primary network config", + config: map[ids.ID]subnets.Config{ + constants.PrimaryNetworkID: {}, + }, + subnetID: testSubnetID, + want: subnets.Config{}, + }, + { + name: "use subnet config", + config: map[ids.ID]subnets.Config{ + constants.PrimaryNetworkID: {}, + testSubnetID: { + GossipConfig: subnets.GossipConfig{ + AcceptedFrontierValidatorSize: 123456789, + }, + }, + }, + subnetID: testSubnetID, + want: subnets.Config{ + GossipConfig: subnets.GossipConfig{ + AcceptedFrontierValidatorSize: 123456789, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + + subnets, err := NewSubnets(ids.EmptyNodeID, tt.config) + require.NoError(err) + + ok := subnets.Add(tt.subnetID) + require.True(ok) + + subnet, ok := subnets.Get(tt.subnetID) + require.True(ok) + + require.Equal(tt.want, subnet.Config()) + }) + } +} + +func TestSubnetsBootstrapping(t *testing.T) { + require := require.New(t) + + config := map[ids.ID]subnets.Config{ + constants.PrimaryNetworkID: {}, + } + + subnets, err := NewSubnets(ids.EmptyNodeID, config) + require.NoError(err) + + subnetID := ids.GenerateTestID() + chainID := ids.GenerateTestID() + + subnets.Add(subnetID) + subnet, ok := subnets.Get(subnetID) + require.True(ok) + + // Start bootstrapping + subnet.AddChain(chainID) + bootstrapping := subnets.Bootstrapping() + require.Contains(bootstrapping, subnetID) + + // Finish bootstrapping + subnet.Bootstrapped(chainID) + require.Empty(subnets.Bootstrapping()) +} diff --git a/node/node.go b/node/node.go index d7ef070fd434..457ce2147ffc 100644 --- a/node/node.go +++ b/node/node.go @@ -304,6 +304,8 @@ type Node struct { // Manages network timeouts timeoutManager timeout.Manager + subnets *chains.Subnets + // Manages creation of blockchains and routing messages to them chainManager chains.Manager @@ -1117,51 +1119,58 @@ func (n *Node) initChainManager(avaxAssetID ids.ID) error { return fmt.Errorf("couldn't initialize chain router: %w", err) } - n.chainManager = chains.New(&chains.ManagerConfig{ - SybilProtectionEnabled: n.Config.SybilProtectionEnabled, - StakingTLSCert: n.Config.StakingTLSCert, - StakingBLSKey: n.Config.StakingSigningKey, - Log: n.Log, - LogFactory: n.LogFactory, - VMManager: n.VMManager, - BlockAcceptorGroup: n.BlockAcceptorGroup, - TxAcceptorGroup: n.TxAcceptorGroup, - VertexAcceptorGroup: n.VertexAcceptorGroup, - DB: n.DB, - MsgCreator: n.msgCreator, - Router: n.chainRouter, - Net: n.Net, - Validators: n.vdrs, - PartialSyncPrimaryNetwork: n.Config.PartialSyncPrimaryNetwork, - NodeID: n.ID, - NetworkID: n.Config.NetworkID, - Server: n.APIServer, - Keystore: n.keystore, - AtomicMemory: n.sharedMemory, - AVAXAssetID: avaxAssetID, - XChainID: xChainID, - CChainID: cChainID, - CriticalChains: criticalChains, - TimeoutManager: n.timeoutManager, - Health: n.health, - ShutdownNodeFunc: n.Shutdown, - MeterVMEnabled: n.Config.MeterVMEnabled, - Metrics: n.MetricsGatherer, - SubnetConfigs: n.Config.SubnetConfigs, - ChainConfigs: n.Config.ChainConfigs, - FrontierPollFrequency: n.Config.FrontierPollFrequency, - ConsensusAppConcurrency: n.Config.ConsensusAppConcurrency, - BootstrapMaxTimeGetAncestors: n.Config.BootstrapMaxTimeGetAncestors, - BootstrapAncestorsMaxContainersSent: n.Config.BootstrapAncestorsMaxContainersSent, - BootstrapAncestorsMaxContainersReceived: n.Config.BootstrapAncestorsMaxContainersReceived, - ApricotPhase4Time: version.GetApricotPhase4Time(n.Config.NetworkID), - ApricotPhase4MinPChainHeight: version.ApricotPhase4MinPChainHeight[n.Config.NetworkID], - ResourceTracker: n.resourceTracker, - StateSyncBeacons: n.Config.StateSyncIDs, - TracingEnabled: n.Config.TraceConfig.Enabled, - Tracer: n.tracer, - ChainDataDir: n.Config.ChainDataDir, - }) + n.subnets, err = chains.NewSubnets(n.ID, n.Config.SubnetConfigs) + if err != nil { + return fmt.Errorf("failed to initialize subnets: %w", err) + } + n.chainManager = chains.New( + &chains.ManagerConfig{ + SybilProtectionEnabled: n.Config.SybilProtectionEnabled, + StakingTLSCert: n.Config.StakingTLSCert, + StakingBLSKey: n.Config.StakingSigningKey, + Log: n.Log, + LogFactory: n.LogFactory, + VMManager: n.VMManager, + BlockAcceptorGroup: n.BlockAcceptorGroup, + TxAcceptorGroup: n.TxAcceptorGroup, + VertexAcceptorGroup: n.VertexAcceptorGroup, + DB: n.DB, + MsgCreator: n.msgCreator, + Router: n.chainRouter, + Net: n.Net, + Validators: n.vdrs, + PartialSyncPrimaryNetwork: n.Config.PartialSyncPrimaryNetwork, + NodeID: n.ID, + NetworkID: n.Config.NetworkID, + Server: n.APIServer, + Keystore: n.keystore, + AtomicMemory: n.sharedMemory, + AVAXAssetID: avaxAssetID, + XChainID: xChainID, + CChainID: cChainID, + CriticalChains: criticalChains, + TimeoutManager: n.timeoutManager, + Health: n.health, + ShutdownNodeFunc: n.Shutdown, + MeterVMEnabled: n.Config.MeterVMEnabled, + Metrics: n.MetricsGatherer, + SubnetConfigs: n.Config.SubnetConfigs, + ChainConfigs: n.Config.ChainConfigs, + FrontierPollFrequency: n.Config.FrontierPollFrequency, + ConsensusAppConcurrency: n.Config.ConsensusAppConcurrency, + BootstrapMaxTimeGetAncestors: n.Config.BootstrapMaxTimeGetAncestors, + BootstrapAncestorsMaxContainersSent: n.Config.BootstrapAncestorsMaxContainersSent, + BootstrapAncestorsMaxContainersReceived: n.Config.BootstrapAncestorsMaxContainersReceived, + ApricotPhase4Time: version.GetApricotPhase4Time(n.Config.NetworkID), + ApricotPhase4MinPChainHeight: version.ApricotPhase4MinPChainHeight[n.Config.NetworkID], + ResourceTracker: n.resourceTracker, + StateSyncBeacons: n.Config.StateSyncIDs, + TracingEnabled: n.Config.TraceConfig.Enabled, + Tracer: n.tracer, + ChainDataDir: n.Config.ChainDataDir, + }, + n.subnets, + ) // Notify the API server when new chains are created n.chainManager.AddRegistrant(n.APIServer) From 523d7d8b02dc62aa7a61e9df2c47d3f08343d08c Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Wed, 7 Feb 2024 13:00:25 -0500 Subject: [PATCH 02/10] Update chains/subnets.go Co-authored-by: aaronbuchwald Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- chains/subnets.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chains/subnets.go b/chains/subnets.go index 5017ba76fffe..5816d5c09a47 100644 --- a/chains/subnets.go +++ b/chains/subnets.go @@ -70,7 +70,7 @@ func (s *Subnets) Get(subnetID ids.ID) (subnets.Subnet, bool) { return subnet, ok } -// Bootstrapping returns subnets that have any chains that are still +// Bootstrapping returns the subnetIDs of any chains that are still // bootstrapping. func (s *Subnets) Bootstrapping() []ids.ID { s.lock.Lock() From 6ebfe5dc70d7061436cac19dbeaa76ee6ba6dae2 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Wed, 7 Feb 2024 14:08:52 -0500 Subject: [PATCH 03/10] nit Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- chains/manager.go | 5 +++-- chains/subnets.go | 42 ++++++++++++++++++++++-------------------- node/node.go | 2 +- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/chains/manager.go b/chains/manager.go index 59f68001e9c4..cbefd9b57b56 100644 --- a/chains/manager.go +++ b/chains/manager.go @@ -233,6 +233,8 @@ type ManagerConfig struct { StateSyncBeacons []ids.NodeID ChainDataDir string + + Subnets *Subnets } type manager struct { @@ -268,13 +270,12 @@ type manager struct { } // New returns a new Manager -func New(config *ManagerConfig, subnets *Subnets) Manager { +func New(config *ManagerConfig) Manager { return &manager{ Aliaser: ids.NewAliaser(), ManagerConfig: *config, stakingSigner: config.StakingTLSCert.PrivateKey.(crypto.Signer), stakingCert: staking.CertificateFromX509(config.StakingTLSCert.Leaf), - subnets: subnets, chains: make(map[ids.ID]handler.Handler), chainsQueue: buffer.NewUnboundedBlockingDeque[ChainParameters](initialQueueSize), unblockChainCreatorCh: make(chan struct{}), diff --git a/chains/subnets.go b/chains/subnets.go index 5816d5c09a47..e84e124c4f9f 100644 --- a/chains/subnets.go +++ b/chains/subnets.go @@ -14,24 +14,6 @@ import ( var ErrNoPrimaryNetworkConfig = errors.New("no subnet config for primary network found") -func NewSubnets( - nodeID ids.NodeID, - configs map[ids.ID]subnets.Config, -) (*Subnets, error) { - if _, ok := configs[constants.PrimaryNetworkID]; !ok { - return nil, ErrNoPrimaryNetworkConfig - } - - s := &Subnets{ - nodeID: nodeID, - configs: configs, - subnets: make(map[ids.ID]subnets.Subnet), - } - - _ = s.Add(constants.PrimaryNetworkID) - return s, nil -} - // Subnets holds the currently running subnets on this node type Subnets struct { nodeID ids.NodeID @@ -41,7 +23,8 @@ type Subnets struct { subnets map[ids.ID]subnets.Subnet } -// Add a subnet that is being run on this node +// Add a subnet that is being run on this node. Returns if the node was added +// or not. func (s *Subnets) Add(subnetID ids.ID) bool { s.lock.Lock() defer s.lock.Unlock() @@ -61,7 +44,8 @@ func (s *Subnets) Add(subnetID ids.ID) bool { return true } -// Get returns a subnet if it is being run on this node +// Get returns a subnet if it is being run on this node. Returns the subnet +// if it was present. func (s *Subnets) Get(subnetID ids.ID) (subnets.Subnet, bool) { s.lock.Lock() defer s.lock.Unlock() @@ -85,3 +69,21 @@ func (s *Subnets) Bootstrapping() []ids.ID { return subnetsBootstrapping } + +func NewSubnets( + nodeID ids.NodeID, + configs map[ids.ID]subnets.Config, +) (*Subnets, error) { + if _, ok := configs[constants.PrimaryNetworkID]; !ok { + return nil, ErrNoPrimaryNetworkConfig + } + + s := &Subnets{ + nodeID: nodeID, + configs: configs, + subnets: make(map[ids.ID]subnets.Subnet), + } + + _ = s.Add(constants.PrimaryNetworkID) + return s, nil +} diff --git a/node/node.go b/node/node.go index 457ce2147ffc..8b492199b608 100644 --- a/node/node.go +++ b/node/node.go @@ -1168,8 +1168,8 @@ func (n *Node) initChainManager(avaxAssetID ids.ID) error { TracingEnabled: n.Config.TraceConfig.Enabled, Tracer: n.tracer, ChainDataDir: n.Config.ChainDataDir, + Subnets: n.subnets, }, - n.subnets, ) // Notify the API server when new chains are created From c01e41509c53246509ff5cca32dc28b5712d1afe Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Wed, 7 Feb 2024 14:10:11 -0500 Subject: [PATCH 04/10] nit Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- chains/subnets.go | 1 + 1 file changed, 1 insertion(+) diff --git a/chains/subnets.go b/chains/subnets.go index e84e124c4f9f..5838556a418f 100644 --- a/chains/subnets.go +++ b/chains/subnets.go @@ -70,6 +70,7 @@ func (s *Subnets) Bootstrapping() []ids.ID { return subnetsBootstrapping } +// NewSubnets returns an instance of Subnets func NewSubnets( nodeID ids.NodeID, configs map[ids.ID]subnets.Config, From c3127c1370c850a72bee74b5c00e93504d13ab05 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Mon, 12 Feb 2024 12:24:19 -0500 Subject: [PATCH 05/10] fix Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- chains/manager.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/chains/manager.go b/chains/manager.go index cbefd9b57b56..0535232ed79f 100644 --- a/chains/manager.go +++ b/chains/manager.go @@ -258,8 +258,6 @@ type manager struct { chainCreatorShutdownCh chan struct{} chainCreatorExited sync.WaitGroup - subnets *Subnets - chainsLock sync.Mutex // Key: Chain's ID // Value: The chain @@ -286,8 +284,8 @@ func New(config *ManagerConfig) Manager { // QueueChainCreation queues a chain creation request // Invariant: Tracked Subnet must be checked before calling this function func (m *manager) QueueChainCreation(chainParams ChainParameters) { - _ = m.subnets.Add(chainParams.SubnetID) - if sb, _ := m.subnets.Get(chainParams.SubnetID); !sb.AddChain(chainParams.ID) { + _ = m.Subnets.Add(chainParams.SubnetID) + if sb, _ := m.Subnets.Get(chainParams.SubnetID); !sb.AddChain(chainParams.ID) { m.Log.Debug("skipping chain creation", zap.String("reason", "chain already staged"), zap.Stringer("subnetID", chainParams.SubnetID), @@ -318,7 +316,7 @@ func (m *manager) createChain(chainParams ChainParameters) { zap.Stringer("vmID", chainParams.VMID), ) - sb, _ := m.subnets.Get(chainParams.SubnetID) + sb, _ := m.Subnets.Get(chainParams.SubnetID) // Note: buildChain builds all chain's relevant objects (notably engine and handler) // but does not start their operations. Starting of the handler (which could potentially @@ -1291,7 +1289,7 @@ func (m *manager) IsBootstrapped(id ids.ID) bool { func (m *manager) registerBootstrappedHealthChecks() error { bootstrappedCheck := health.CheckerFunc(func(context.Context) (interface{}, error) { - if subnetIDs := m.subnets.Bootstrapping(); len(subnetIDs) != 0 { + if subnetIDs := m.Subnets.Bootstrapping(); len(subnetIDs) != 0 { return subnetIDs, errNotBootstrapped } return []ids.ID{}, nil @@ -1334,7 +1332,7 @@ func (m *manager) registerBootstrappedHealthChecks() error { // Starts chain creation loop to process queued chains func (m *manager) StartChainCreator(platformParams ChainParameters) error { // Add the P-Chain to the Primary Network - sb, ok := m.subnets.Get(constants.PrimaryNetworkID) + sb, ok := m.Subnets.Get(constants.PrimaryNetworkID) if !ok { return errPrimaryNetworkNotRunning } From 400ff82e1ddc542bc24710f0168ab54b50b455a3 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Mon, 12 Feb 2024 12:24:39 -0500 Subject: [PATCH 06/10] Update chains/subnets.go Co-authored-by: Stephen Buttolph Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- chains/subnets.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chains/subnets.go b/chains/subnets.go index 5838556a418f..0eaa42d63c05 100644 --- a/chains/subnets.go +++ b/chains/subnets.go @@ -23,7 +23,7 @@ type Subnets struct { subnets map[ids.ID]subnets.Subnet } -// Add a subnet that is being run on this node. Returns if the node was added +// Add a subnet that is being run on this node. Returns if the subnet was added // or not. func (s *Subnets) Add(subnetID ids.ID) bool { s.lock.Lock() From aa4528087d8f5aaa3b372ee4283db39df8e2d2d0 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Mon, 12 Feb 2024 12:25:18 -0500 Subject: [PATCH 07/10] nit Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- chains/subnets.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/chains/subnets.go b/chains/subnets.go index 0eaa42d63c05..1c83def34aca 100644 --- a/chains/subnets.go +++ b/chains/subnets.go @@ -19,7 +19,7 @@ type Subnets struct { nodeID ids.NodeID configs map[ids.ID]subnets.Config - lock sync.Mutex + lock sync.RWMutex subnets map[ids.ID]subnets.Subnet } @@ -47,8 +47,8 @@ func (s *Subnets) Add(subnetID ids.ID) bool { // Get returns a subnet if it is being run on this node. Returns the subnet // if it was present. func (s *Subnets) Get(subnetID ids.ID) (subnets.Subnet, bool) { - s.lock.Lock() - defer s.lock.Unlock() + s.lock.RLock() + defer s.lock.RUnlock() subnet, ok := s.subnets[subnetID] return subnet, ok @@ -57,8 +57,8 @@ func (s *Subnets) Get(subnetID ids.ID) (subnets.Subnet, bool) { // Bootstrapping returns the subnetIDs of any chains that are still // bootstrapping. func (s *Subnets) Bootstrapping() []ids.ID { - s.lock.Lock() - defer s.lock.Unlock() + s.lock.RLock() + defer s.lock.RUnlock() subnetsBootstrapping := make([]ids.ID, 0, len(s.subnets)) for subnetID, subnet := range s.subnets { From 54cdfc2aabe82688774e39563adf781a851d0416 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Mon, 12 Feb 2024 12:26:23 -0500 Subject: [PATCH 08/10] nit Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- node/node.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/node/node.go b/node/node.go index 8b492199b608..437f505a5e0a 100644 --- a/node/node.go +++ b/node/node.go @@ -304,8 +304,6 @@ type Node struct { // Manages network timeouts timeoutManager timeout.Manager - subnets *chains.Subnets - // Manages creation of blockchains and routing messages to them chainManager chains.Manager @@ -1119,7 +1117,7 @@ func (n *Node) initChainManager(avaxAssetID ids.ID) error { return fmt.Errorf("couldn't initialize chain router: %w", err) } - n.subnets, err = chains.NewSubnets(n.ID, n.Config.SubnetConfigs) + subnets, err := chains.NewSubnets(n.ID, n.Config.SubnetConfigs) if err != nil { return fmt.Errorf("failed to initialize subnets: %w", err) } @@ -1168,7 +1166,7 @@ func (n *Node) initChainManager(avaxAssetID ids.ID) error { TracingEnabled: n.Config.TraceConfig.Enabled, Tracer: n.tracer, ChainDataDir: n.Config.ChainDataDir, - Subnets: n.subnets, + Subnets: subnets, }, ) From de6a89f206b4a1ef4d1c375bef6b80617353ee0e Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Mon, 12 Feb 2024 16:37:36 -0500 Subject: [PATCH 09/10] nit Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- chains/manager.go | 10 +++------- chains/subnets.go | 26 +++++++++----------------- chains/subnets_test.go | 32 ++++++++++++++------------------ 3 files changed, 26 insertions(+), 42 deletions(-) diff --git a/chains/manager.go b/chains/manager.go index 0535232ed79f..4dfa506eca1f 100644 --- a/chains/manager.go +++ b/chains/manager.go @@ -284,8 +284,7 @@ func New(config *ManagerConfig) Manager { // QueueChainCreation queues a chain creation request // Invariant: Tracked Subnet must be checked before calling this function func (m *manager) QueueChainCreation(chainParams ChainParameters) { - _ = m.Subnets.Add(chainParams.SubnetID) - if sb, _ := m.Subnets.Get(chainParams.SubnetID); !sb.AddChain(chainParams.ID) { + if sb, _ := m.Subnets.GetOrCreate(chainParams.SubnetID); !sb.AddChain(chainParams.ID) { m.Log.Debug("skipping chain creation", zap.String("reason", "chain already staged"), zap.Stringer("subnetID", chainParams.SubnetID), @@ -316,7 +315,7 @@ func (m *manager) createChain(chainParams ChainParameters) { zap.Stringer("vmID", chainParams.VMID), ) - sb, _ := m.Subnets.Get(chainParams.SubnetID) + sb, _ := m.Subnets.GetOrCreate(chainParams.SubnetID) // Note: buildChain builds all chain's relevant objects (notably engine and handler) // but does not start their operations. Starting of the handler (which could potentially @@ -1332,10 +1331,7 @@ func (m *manager) registerBootstrappedHealthChecks() error { // Starts chain creation loop to process queued chains func (m *manager) StartChainCreator(platformParams ChainParameters) error { // Add the P-Chain to the Primary Network - sb, ok := m.Subnets.Get(constants.PrimaryNetworkID) - if !ok { - return errPrimaryNetworkNotRunning - } + sb, _ := m.Subnets.GetOrCreate(constants.PrimaryNetworkID) sb.AddChain(platformParams.ID) // The P-chain is created synchronously to ensure that `VM.Initialize` has diff --git a/chains/subnets.go b/chains/subnets.go index 1c83def34aca..fda66a7eded2 100644 --- a/chains/subnets.go +++ b/chains/subnets.go @@ -23,14 +23,14 @@ type Subnets struct { subnets map[ids.ID]subnets.Subnet } -// Add a subnet that is being run on this node. Returns if the subnet was added -// or not. -func (s *Subnets) Add(subnetID ids.ID) bool { +// GetOrCreate returns a subnet running on this node, or creates one if it was +// not running before. Returns the subnet and if the subnet was created. +func (s *Subnets) GetOrCreate(subnetID ids.ID) (subnets.Subnet, bool) { s.lock.Lock() defer s.lock.Unlock() - if _, ok := s.subnets[subnetID]; ok { - return false + if subnet, ok := s.subnets[subnetID]; ok { + return subnet, false } // Default to the primary network config if a subnet config was not @@ -40,18 +40,10 @@ func (s *Subnets) Add(subnetID ids.ID) bool { config = s.configs[constants.PrimaryNetworkID] } - s.subnets[subnetID] = subnets.New(s.nodeID, config) - return true -} - -// Get returns a subnet if it is being run on this node. Returns the subnet -// if it was present. -func (s *Subnets) Get(subnetID ids.ID) (subnets.Subnet, bool) { - s.lock.RLock() - defer s.lock.RUnlock() + subnet := subnets.New(s.nodeID, config) + s.subnets[subnetID] = subnet - subnet, ok := s.subnets[subnetID] - return subnet, ok + return subnet, true } // Bootstrapping returns the subnetIDs of any chains that are still @@ -85,6 +77,6 @@ func NewSubnets( subnets: make(map[ids.ID]subnets.Subnet), } - _ = s.Add(constants.PrimaryNetworkID) + _, _ = s.GetOrCreate(constants.PrimaryNetworkID) return s, nil } diff --git a/chains/subnets_test.go b/chains/subnets_test.go index 5fb12660e15a..231a8f970a15 100644 --- a/chains/subnets_test.go +++ b/chains/subnets_test.go @@ -22,12 +22,12 @@ func TestNewSubnets(t *testing.T) { subnets, err := NewSubnets(ids.EmptyNodeID, config) require.NoError(err) - subnet, ok := subnets.Get(constants.PrimaryNetworkID) - require.True(ok) - require.Equal(subnet.Config(), config[constants.PrimaryNetworkID]) + subnet, ok := subnets.GetOrCreate(constants.PrimaryNetworkID) + require.False(ok) + require.Equal(config[constants.PrimaryNetworkID], subnet.Config()) } -func TestNewSubnets_NoPrimaryNetworkConfig(t *testing.T) { +func TestNewSubnetsNoPrimaryNetworkConfig(t *testing.T) { require := require.New(t) config := map[ids.ID]subnets.Config{} @@ -35,21 +35,21 @@ func TestNewSubnets_NoPrimaryNetworkConfig(t *testing.T) { require.ErrorIs(err, ErrNoPrimaryNetworkConfig) } -func TestSubnetsAdd(t *testing.T) { +func TestSubnetsGetOrCreate(t *testing.T) { testSubnetID := ids.GenerateTestID() - type add struct { + type args struct { subnetID ids.ID want bool } tests := []struct { name string - adds []add + args []args }{ { name: "adding duplicate subnet is a noop", - adds: []add{ + args: []args{ { subnetID: testSubnetID, want: true, @@ -61,7 +61,7 @@ func TestSubnetsAdd(t *testing.T) { }, { name: "adding unique subnets succeeds", - adds: []add{ + args: []args{ { subnetID: ids.GenerateTestID(), want: true, @@ -87,9 +87,9 @@ func TestSubnetsAdd(t *testing.T) { subnets, err := NewSubnets(ids.EmptyNodeID, config) require.NoError(err) - for _, add := range tt.adds { - got := subnets.Add(add.subnetID) - require.Equal(got, add.want) + for _, arg := range tt.args { + _, got := subnets.GetOrCreate(arg.subnetID) + require.Equal(arg.want, got) } }) } @@ -138,10 +138,7 @@ func TestSubnetConfigs(t *testing.T) { subnets, err := NewSubnets(ids.EmptyNodeID, tt.config) require.NoError(err) - ok := subnets.Add(tt.subnetID) - require.True(ok) - - subnet, ok := subnets.Get(tt.subnetID) + subnet, ok := subnets.GetOrCreate(tt.subnetID) require.True(ok) require.Equal(tt.want, subnet.Config()) @@ -162,8 +159,7 @@ func TestSubnetsBootstrapping(t *testing.T) { subnetID := ids.GenerateTestID() chainID := ids.GenerateTestID() - subnets.Add(subnetID) - subnet, ok := subnets.Get(subnetID) + subnet, ok := subnets.GetOrCreate(subnetID) require.True(ok) // Start bootstrapping From 32f74d6028fc53d7afe4ada8b2807060ba425978 Mon Sep 17 00:00:00 2001 From: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Date: Mon, 12 Feb 2024 16:42:52 -0500 Subject: [PATCH 10/10] nit Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> --- chains/manager.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/chains/manager.go b/chains/manager.go index 4dfa506eca1f..3c016fe7c18f 100644 --- a/chains/manager.go +++ b/chains/manager.go @@ -94,11 +94,10 @@ var ( // Bootstrapping prefixes for ChainVMs ChainBootstrappingDBPrefix = []byte("bs") - errUnknownVMType = errors.New("the vm should have type avalanche.DAGVM or snowman.ChainVM") - errCreatePlatformVM = errors.New("attempted to create a chain running the PlatformVM") - errNotBootstrapped = errors.New("subnets not bootstrapped") - errPrimaryNetworkNotRunning = errors.New("node is not running the primary network") - errPartialSyncAsAValidator = errors.New("partial sync should not be configured for a validator") + errUnknownVMType = errors.New("the vm should have type avalanche.DAGVM or snowman.ChainVM") + errCreatePlatformVM = errors.New("attempted to create a chain running the PlatformVM") + errNotBootstrapped = errors.New("subnets not bootstrapped") + errPartialSyncAsAValidator = errors.New("partial sync should not be configured for a validator") fxs = map[ids.ID]fx.Factory{ secp256k1fx.ID: &secp256k1fx.Factory{},