From 8ee65d9c4eb12b55b3e7a246c6464a8c170a0e08 Mon Sep 17 00:00:00 2001
From: Damilola Edwards <damilolaedwards@gmail.com>
Date: Tue, 18 Aug 2026 17:38:43 +0100
Subject: [PATCH] Fix Stop() not stopping the cron scheduler

Start built the job scheduler into a local variable and started it,
but never assigned it to the node so Stop() had nothing to call.
Every cron job kept running after Stop, including health checks,
sync status, node version and peer polling. Now the scheduler is
assigned before it starts so Stop can actually stop it.
---
 pkg/beacon/beacon.go      |   2 +
 pkg/beacon/beacon_test.go | 108 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+)

diff --git a/pkg/beacon/beacon.go b/pkg/beacon/beacon.go
index 0b1eb83..6e479cc 100644
--- a/pkg/beacon/beacon.go
+++ b/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!")
diff --git a/pkg/beacon/beacon_test.go b/pkg/beacon/beacon_test.go
index e840886..18745fa 100644
--- a/pkg/beacon/beacon_test.go
+++ b/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")
+	}
+}