From e1c5a3eccf4a1ca5c0d48e8de9d17ccf87df91cf Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Fri, 9 Apr 2021 16:38:26 -0400 Subject: [PATCH] change RegisterChain structure; move GetVM to common.Engine --- api/server/server.go | 90 +++++++++++++++-------------- snow/engine/avalanche/engine.go | 4 -- snow/engine/avalanche/transitive.go | 2 +- snow/engine/common/engine.go | 3 + snow/engine/common/test_engine.go | 14 ++++- snow/engine/snowman/engine.go | 4 -- 6 files changed, 63 insertions(+), 54 deletions(-) diff --git a/api/server/server.go b/api/server/server.go index 4b2a9f3e827a..d226e7da1455 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -110,53 +110,55 @@ func (s *Server) DispatchTLS(certFile, keyFile string) error { // and at the same time the server's lock is held due to an API call and is trying // to grab the P-Chain's lock. func (s *Server) RegisterChain(chainName string, ctx *snow.Context, engineIntf interface{}) { - go func() { - var ( - handlers map[string]*common.HTTPHandler - err error - ) - - switch engine := engineIntf.(type) { - case snowman.Engine: - ctx.Lock.Lock() - handlers, err = engine.GetVM().CreateHandlers() - ctx.Lock.Unlock() - case avalanche.Engine: - ctx.Lock.Lock() - handlers, err = engine.GetVM().CreateHandlers() - ctx.Lock.Unlock() - default: - s.log.Error("engine has unexpected type %T", engineIntf) - return - } - if err != nil { - s.log.Error("failed to create %s handlers: %s", chainName, err) - return - } + go s.registerChain(chainName, ctx, engineIntf) +} - httpLogger, err := s.factory.MakeChain(chainName, "http") - if err != nil { - s.log.Error("failed to create new http logger: %s", err) - } +func (s *Server) registerChain(chainName string, ctx *snow.Context, engineIntf interface{}) { + var ( + handlers map[string]*common.HTTPHandler + err error + ) + + switch engine := engineIntf.(type) { + case snowman.Engine: + ctx.Lock.Lock() + handlers, err = engine.GetVM().CreateHandlers() + ctx.Lock.Unlock() + case avalanche.Engine: + ctx.Lock.Lock() + handlers, err = engine.GetVM().CreateHandlers() + ctx.Lock.Unlock() + default: + s.log.Error("engine has unexpected type %T", engineIntf) + return + } + if err != nil { + s.log.Error("failed to create %s handlers: %s", chainName, err) + return + } + + httpLogger, err := s.factory.MakeChain(chainName, "http") + if err != nil { + s.log.Error("failed to create new http logger: %s", err) + } - s.log.Verbo("About to add API endpoints for chain with ID %s", ctx.ChainID) - // all subroutes to a chain begin with "bc/" - defaultEndpoint := "bc/" + ctx.ChainID.String() - - // Register each endpoint - for extension, handler := range handlers { - // Validate that the route being added is valid - // e.g. "/foo" and "" are ok but "\n" is not - _, err := url.ParseRequestURI(extension) - if extension != "" && err != nil { - s.log.Error("could not add route to chain's API handler because route is malformed: %s", err) - continue - } - if err := s.AddChainRoute(handler, ctx, defaultEndpoint, extension, httpLogger); err != nil { - s.log.Error("error adding route: %s", err) - } + s.log.Verbo("About to add API endpoints for chain with ID %s", ctx.ChainID) + // all subroutes to a chain begin with "bc/" + defaultEndpoint := "bc/" + ctx.ChainID.String() + + // Register each endpoint + for extension, handler := range handlers { + // Validate that the route being added is valid + // e.g. "/foo" and "" are ok but "\n" is not + _, err := url.ParseRequestURI(extension) + if extension != "" && err != nil { + s.log.Error("could not add route to chain's API handler because route is malformed: %s", err) + continue + } + if err := s.AddChainRoute(handler, ctx, defaultEndpoint, extension, httpLogger); err != nil { + s.log.Error("error adding route: %s", err) } - }() + } } // AddChainRoute registers a route to a chain's handler diff --git a/snow/engine/avalanche/engine.go b/snow/engine/avalanche/engine.go index 06b4872a1d62..da5583816120 100644 --- a/snow/engine/avalanche/engine.go +++ b/snow/engine/avalanche/engine.go @@ -6,7 +6,6 @@ package avalanche import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/consensus/avalanche" - "github.com/ava-labs/avalanchego/snow/engine/avalanche/vertex" "github.com/ava-labs/avalanchego/snow/engine/common" ) @@ -20,7 +19,4 @@ type Engine interface { // GetVtx returns a vertex by its ID. // Returns an error if unknown. GetVtx(vtxID ids.ID) (avalanche.Vertex, error) - - // GetVM returns this engine's VM - GetVM() vertex.DAGVM } diff --git a/snow/engine/avalanche/transitive.go b/snow/engine/avalanche/transitive.go index 1860ba052d9e..6824a55cd314 100644 --- a/snow/engine/avalanche/transitive.go +++ b/snow/engine/avalanche/transitive.go @@ -658,6 +658,6 @@ func (t *Transitive) GetVtx(vtxID ids.ID) (avalanche.Vertex, error) { return t.Manager.Get(vtxID) } -func (t *Transitive) GetVM() vertex.DAGVM { +func (t *Transitive) GetVM() common.VM { return t.VM } diff --git a/snow/engine/common/engine.go b/snow/engine/common/engine.go index 9eba2ab72370..845e6bc3df9b 100644 --- a/snow/engine/common/engine.go +++ b/snow/engine/common/engine.go @@ -22,6 +22,9 @@ type Engine interface { // Returns nil if the engine is healthy. // Periodically called and reported through the health API health.Checkable + + // GetVM returns this engine's VM + GetVM() VM } // Handler defines the functions that are acted on the node diff --git a/snow/engine/common/test_engine.go b/snow/engine/common/test_engine.go index 064bc19da54d..c6af6e65d0da 100644 --- a/snow/engine/common/test_engine.go +++ b/snow/engine/common/test_engine.go @@ -50,7 +50,7 @@ type EngineTest struct { CantHealth, - CantGetVtx bool + CantGetVtx, CantGetVM bool IsBootstrappedF func() bool ContextF func() *snow.Context @@ -65,6 +65,7 @@ type EngineTest struct { ConnectedF, DisconnectedF func(validatorID ids.ShortID) error HealthF func() (interface{}, error) GetVtxF func() (avalanche.Vertex, error) + GetVMF func() VM } var _ Engine = &EngineTest{} @@ -107,6 +108,7 @@ func (e *EngineTest) Default(cant bool) { e.CantHealth = cant e.CantGetVtx = cant + e.CantGetVM = cant } func (e *EngineTest) Context() *snow.Context { @@ -435,3 +437,13 @@ func (e *EngineTest) GetVtx() (avalanche.Vertex, error) { } return nil, errors.New("unexpectedly called GetVtx") } + +func (e *EngineTest) GetVM() VM { + if e.GetVMF != nil { + return e.GetVMF() + } + if e.CantGetVM && e.T != nil { + e.T.Fatalf("Unexpectedly called GetVM") + } + return nil +} diff --git a/snow/engine/snowman/engine.go b/snow/engine/snowman/engine.go index 733ec8614993..bcc684b42324 100644 --- a/snow/engine/snowman/engine.go +++ b/snow/engine/snowman/engine.go @@ -7,7 +7,6 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/consensus/snowman" "github.com/ava-labs/avalanchego/snow/engine/common" - "github.com/ava-labs/avalanchego/snow/engine/snowman/block" ) // Engine describes the events that can occur to a Snowman instance. @@ -27,7 +26,4 @@ type Engine interface { // GetBlock returns a block by its ID. // Returns an error if unknown. GetBlock(blkID ids.ID) (snowman.Block, error) - - // GetVM returns this engine's VM - GetVM() block.ChainVM }