Fix Stop() not stopping the cron scheduler

pkg/beacon/beacon.go

@@ -298,6 +298,8 @@ func (n *node) Start(ctx context.Context) error {
 		return err
 	}
 
+	n.crons = s
+
 	s.StartAsync()
 
 	n.log.Info("Beacon started!")

pkg/beacon/beacon_test.go

@@ -6,6 +6,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/chuckpreslar/emission"
+	"github.com/ethpandaops/beacon/pkg/beacon/api/types"
+	eapi "github.com/ethpandaops/go-eth2-client/api"
+	v1 "github.com/ethpandaops/go-eth2-client/api/v1"
 	"github.com/sirupsen/logrus"
 )
 
@@ -123,3 +127,107 @@ func TestLifecycleStartStopSequence(t *testing.T) {
 		t.Error("context was not cancelled after Stop")
 	}
 }
+
+// schedulerFakeClient implements just enough of eth2client.Service to get
+// through ensureClients and bootstrap without a real beacon node.
+type schedulerFakeClient struct{}
+
+func (f *schedulerFakeClient) Name() string    { return "fake" }
+func (f *schedulerFakeClient) Address() string { return "fake://" }
+func (f *schedulerFakeClient) IsActive() bool  { return true }
+func (f *schedulerFakeClient) IsSynced() bool  { return true }
+
+func (f *schedulerFakeClient) NodeSyncing(
+	_ context.Context, _ *eapi.NodeSyncingOpts,
+) (*eapi.Response[*v1.SyncState], error) {
+	return &eapi.Response[*v1.SyncState]{Data: &v1.SyncState{}}, nil
+}
+
+func (f *schedulerFakeClient) Spec(
+	_ context.Context, _ *eapi.SpecOpts,
+) (*eapi.Response[map[string]any], error) {
+	return &eapi.Response[map[string]any]{
+		Data: map[string]any{
+			"SECONDS_PER_SLOT": "12",
+			"SLOTS_PER_EPOCH":  "32",
+		},
+	}, nil
+}
+
+func (f *schedulerFakeClient) Genesis(
+	_ context.Context, _ *eapi.GenesisOpts,
+) (*eapi.Response[*v1.Genesis], error) {
+	return &eapi.Response[*v1.Genesis]{Data: &v1.Genesis{GenesisTime: time.Now()}}, nil
+}
+
+func (f *schedulerFakeClient) NodeVersion(
+	_ context.Context, _ *eapi.NodeVersionOpts,
+) (*eapi.Response[string], error) {
+	return &eapi.Response[string]{Data: "fake/v0.0.0"}, nil
+}
+
+// schedulerFakeAPI implements api.ConsensusClient. Start()'s cron jobs run
+// once immediately when the scheduler starts, so FetchPeers needs this to
+// avoid a nil pointer dereference on n.api.
+type schedulerFakeAPI struct{ types.Peers }
+
+func (f *schedulerFakeAPI) NodePeer(context.Context, string) (types.Peer, error) {
+	return types.Peer{}, nil
+}
+func (f *schedulerFakeAPI) NodePeers(context.Context) (types.Peers, error) { return nil, nil }
+func (f *schedulerFakeAPI) NodePeerCount(context.Context) (types.PeerCount, error) {
+	return types.PeerCount{}, nil
+}
+func (f *schedulerFakeAPI) RawBlock(context.Context, string, string) ([]byte, error) {
+	return nil, nil
+}
+func (f *schedulerFakeAPI) RawDebugBeaconState(context.Context, string, string) ([]byte, error) {
+	return nil, nil
+}
+func (f *schedulerFakeAPI) DepositSnapshot(context.Context) (*types.DepositSnapshot, error) {
+	return nil, nil
+}
+func (f *schedulerFakeAPI) NodeIdentity(context.Context) (*types.Identity, error) {
+	return nil, nil
+}
+
+// TestStopStopsScheduler runs a real Start()/Stop() cycle and checks that
+// the cron scheduler Start() creates is actually assigned to n.crons and
+// stopped by Stop(). Not run under -race: go-eth2-client's wallclock
+// dependency has its own unrelated internal race between its ticker
+// goroutine and listener registration that triggers on any Start() call,
+// independent of this fix, and this repo's CI does not run tests with
+// -race either.
+func TestStopStopsScheduler(t *testing.T) {
+	n := &node{
+		log:     logrus.New(),
+		options: &Options{HealthCheck: DefaultHealthCheckOptions()},
+		config:  &Config{},
+		stat:    NewStatus(1, 1),
+		broker:  emission.NewEmitter(),
+		client:  &schedulerFakeClient{},
+		api:     &schedulerFakeAPI{},
+	}
+
+	ctx := context.Background()
+
+	if err := n.Start(ctx); err != nil {
+		t.Fatalf("Start failed: %v", err)
+	}
+
+	if n.crons == nil {
+		t.Fatal("expected crons to be assigned after Start")
+	}
+
+	if !n.crons.IsRunning() {
+		t.Fatal("expected scheduler to be running after Start")
+	}
+
+	if err := n.Stop(ctx); err != nil {
+		t.Fatalf("Stop failed: %v", err)
+	}
+
+	if n.crons.IsRunning() {
+		t.Fatal("expected scheduler to be stopped after Stop, but it is still running")
+	}
+}