Simplify common.Engine API

Halt() is only used for the bootstrapper engine, and not invoked on other engine types. Context() is only used in tests. Therefore, this commit removes these methods from the common engine API to simplify it. Signed-off-by: Yacov Manevich <[email protected]>
ava-labs · Sep 24, 2024 · 3cacae5 · 3cacae5
1 parent c55a011
commit 3cacae5
Show file tree

Hide file tree

Showing 20 changed files with 90 additions and 129 deletions.
diff --git a/chains/manager.go b/chains/manager.go
@@ -888,6 +888,8 @@ func (m *manager) createAvalancheChain(
 		return nil, err
 	}
 
+	var halter common.Halter
+
 	// Asynchronously passes messages from the network to the consensus engine
 	h, err := handler.New(
 		ctx,
@@ -900,6 +902,7 @@ func (m *manager) createAvalancheChain(
 		connectedValidators,
 		peerTracker,
 		handlerReg,
+		func() {},
 	)
 	if err != nil {
 		return nil, fmt.Errorf("error initializing network handler: %w", err)
@@ -950,6 +953,7 @@ func (m *manager) createAvalancheChain(
 
 	// create bootstrap gear
 	bootstrapCfg := smbootstrap.Config{
+		ShouldHalt:                     halter.Halted,
 		NonVerifyingParse:              block.ParseFunc(proposerVM.ParseLocalBlock),
 		AllGetsServer:                  snowGetHandler,
 		Ctx:                            ctx,
@@ -1012,17 +1016,19 @@ func (m *manager) createAvalancheChain(
 		avalancheBootstrapperConfig.StopVertexID = m.Upgrades.CortinaXChainStopVertexID
 	}
 
-	avalancheBootstrapper, err := avbootstrap.New(
+	var avalancheBootstrapper common.BootstrapableEngine
+	avBoot, err := avbootstrap.New(
 		avalancheBootstrapperConfig,
 		snowmanBootstrapper.Start,
 		avalancheMetrics,
 	)
+	avalancheBootstrapper = avBoot
 	if err != nil {
 		return nil, fmt.Errorf("error initializing avalanche bootstrapper: %w", err)
 	}
 
 	if m.TracingEnabled {
-		avalancheBootstrapper = common.TraceBootstrapableEngine(avalancheBootstrapper, m.Tracer)
+		avalancheBootstrapper = common.TraceBootstrapableEngine(avBoot, m.Tracer)
 	}
 
 	h.SetEngineManager(&handler.EngineManager{
@@ -1277,6 +1283,8 @@ func (m *manager) createSnowmanChain(
 		return nil, err
 	}
 
+	var halter common.Halter
+
 	// Asynchronously passes messages from the network to the consensus engine
 	h, err := handler.New(
 		ctx,
@@ -1289,6 +1297,7 @@ func (m *manager) createSnowmanChain(
 		connectedValidators,
 		peerTracker,
 		handlerReg,
+		func() {},
 	)
 	if err != nil {
 		return nil, fmt.Errorf("couldn't initialize message handler: %w", err)
@@ -1340,6 +1349,7 @@ func (m *manager) createSnowmanChain(
 
 	// create bootstrap gear
 	bootstrapCfg := smbootstrap.Config{
+		ShouldHalt:                     halter.Halted,
 		NonVerifyingParse:              block.ParseFunc(proposerVM.ParseLocalBlock),
 		AllGetsServer:                  snowGetHandler,
 		Ctx:                            ctx,

diff --git a/snow/engine/avalanche/bootstrap/bootstrapper.go b/snow/engine/avalanche/bootstrap/bootstrapper.go
@@ -49,7 +49,7 @@ func New(
 	config Config,
 	onFinished func(ctx context.Context, lastReqID uint32) error,
 	reg prometheus.Registerer,
-) (common.BootstrapableEngine, error) {
+) (*bootstrapper, error) {
 	b := &bootstrapper{
 		Config: config,
 

diff --git a/snow/engine/common/engine.go b/snow/engine/common/engine.go
@@ -9,7 +9,6 @@ import (
 
 	"github.com/ava-labs/avalanchego/api/health"
 	"github.com/ava-labs/avalanchego/ids"
-	"github.com/ava-labs/avalanchego/snow"
 	"github.com/ava-labs/avalanchego/snow/validators"
 	"github.com/ava-labs/avalanchego/utils/set"
 )
@@ -23,9 +22,6 @@ import (
 type Engine interface {
 	Handler
 
-	// Return the context of the chain this engine is working on
-	Context() *snow.ConsensusContext
-
 	// Start engine operations from given request ID
 	Start(ctx context.Context, startReqID uint32) error
 
@@ -425,14 +421,6 @@ type InternalHandler interface {
 	// Gossip to the network a container on the accepted frontier
 	Gossip(context.Context) error
 
-	// Halt this engine.
-	//
-	// This function will be called before the environment starts exiting. This
-	// function is special, in that it does not expect the chain's context lock
-	// to be held before calling this function. This function also does not
-	// require the engine to have been started.
-	Halt(context.Context)
-
 	// Shutdown this engine.
 	//
 	// This function will be called when the environment is exiting.

diff --git a/snow/engine/common/halter.go b/snow/engine/common/halter.go
@@ -3,23 +3,20 @@
 
 package common
 
-import (
-	"context"
-	"sync/atomic"
-)
+import "sync/atomic"
 
 var _ Haltable = (*Halter)(nil)
 
 type Haltable interface {
-	Halt(context.Context)
+	Halt()
 	Halted() bool
 }
 
 type Halter struct {
 	halted uint32
 }
 
-func (h *Halter) Halt(context.Context) {
+func (h *Halter) Halt() {
 	atomic.StoreUint32(&h.halted, 1)
 }
 

diff --git a/snow/engine/common/no_ops_handlers.go b/snow/engine/common/no_ops_handlers.go
@@ -355,13 +355,6 @@ func (nop *noOpInternalHandler) Gossip(context.Context) error {
 	return nil
 }
 
-func (nop *noOpInternalHandler) Halt(context.Context) {
-	nop.log.Debug("dropping request",
-		zap.String("reason", "unhandled by this gear"),
-		zap.String("messageOp", "halt"),
-	)
-}
-
 func (nop *noOpInternalHandler) Shutdown(context.Context) error {
 	nop.log.Debug("dropping request",
 		zap.String("reason", "unhandled by this gear"),

diff --git a/snow/engine/common/traced_engine.go b/snow/engine/common/traced_engine.go
@@ -10,7 +10,6 @@ import (
 	"go.opentelemetry.io/otel/attribute"
 
 	"github.com/ava-labs/avalanchego/ids"
-	"github.com/ava-labs/avalanchego/snow"
 	"github.com/ava-labs/avalanchego/trace"
 	"github.com/ava-labs/avalanchego/utils/set"
 	"github.com/ava-labs/avalanchego/version"
@@ -344,13 +343,6 @@ func (e *tracedEngine) Gossip(ctx context.Context) error {
 	return e.engine.Gossip(ctx)
 }
 
-func (e *tracedEngine) Halt(ctx context.Context) {
-	ctx, span := e.tracer.Start(ctx, "tracedEngine.Halt")
-	defer span.End()
-
-	e.engine.Halt(ctx)
-}
-
 func (e *tracedEngine) Shutdown(ctx context.Context) error {
 	ctx, span := e.tracer.Start(ctx, "tracedEngine.Shutdown")
 	defer span.End()
@@ -367,10 +359,6 @@ func (e *tracedEngine) Notify(ctx context.Context, msg Message) error {
 	return e.engine.Notify(ctx, msg)
 }
 
-func (e *tracedEngine) Context() *snow.ConsensusContext {
-	return e.engine.Context()
-}
-
 func (e *tracedEngine) Start(ctx context.Context, startReqID uint32) error {
 	ctx, span := e.tracer.Start(ctx, "tracedEngine.Start", oteltrace.WithAttributes(
 		attribute.Int64("requestID", int64(startReqID)),

diff --git a/snow/engine/enginetest/engine.go b/snow/engine/enginetest/engine.go
@@ -189,19 +189,6 @@ func (e *Engine) Start(ctx context.Context, startReqID uint32) error {
 	return errStart
 }
 
-func (e *Engine) Context() *snow.ConsensusContext {
-	if e.ContextF != nil {
-		return e.ContextF()
-	}
-	if !e.CantContext {
-		return nil
-	}
-	if e.T != nil {
-		require.FailNow(e.T, "Unexpectedly called Context")
-	}
-	return nil
-}
-
 func (e *Engine) Timeout(ctx context.Context) error {
 	if e.TimeoutF != nil {
 		return e.TimeoutF(ctx)
@@ -228,19 +215,6 @@ func (e *Engine) Gossip(ctx context.Context) error {
 	return errGossip
 }
 
-func (e *Engine) Halt(ctx context.Context) {
-	if e.HaltF != nil {
-		e.HaltF(ctx)
-		return
-	}
-	if !e.CantHalt {
-		return
-	}
-	if e.T != nil {
-		require.FailNow(e.T, "Unexpectedly called Halt")
-	}
-}
-
 func (e *Engine) Shutdown(ctx context.Context) error {
 	if e.ShutdownF != nil {
 		return e.ShutdownF(ctx)

diff --git a/snow/engine/snowman/bootstrap/bootstrapper.go b/snow/engine/snowman/bootstrap/bootstrapper.go
@@ -68,7 +68,7 @@ var (
 // called, so it must be guaranteed the VM is not used until after Start.
 type Bootstrapper struct {
 	Config
-	common.Halter
+	shouldHalt func() bool
 	*metrics
 
 	// list of NoOpsHandler for messages dropped by bootstrapper
@@ -120,6 +120,7 @@ type Bootstrapper struct {
 func New(config Config, onFinished func(ctx context.Context, lastReqID uint32) error) (*Bootstrapper, error) {
 	metrics, err := newMetrics(config.Ctx.Registerer)
 	return &Bootstrapper{
+		shouldHalt:                  config.ShouldHalt,
 		nonVerifyingParser:          config.NonVerifyingParse,
 		Config:                      config,
 		metrics:                     metrics,
@@ -649,7 +650,7 @@ func (b *Bootstrapper) tryStartExecuting(ctx context.Context) error {
 	numToExecute := b.tree.Len()
 	err = execute(
 		ctx,
-		b,
+		b.ShouldHalt,
 		log,
 		b.DB,
 		&parseAcceptor{
@@ -673,7 +674,7 @@ func (b *Bootstrapper) tryStartExecuting(ctx context.Context) error {
 			lastAccepted.Height(),
 		)
 	}
-	if b.Halted() {
+	if b.shouldHalt() {
 		return nil
 	}