discovery/test: add comprehensive tests for state handler error exits

Add comprehensive test coverage to verify that state handler errors cause
the channelGraphSyncer goroutine to exit cleanly without entering endless
retry loops. These tests use mutation testing principles to ensure they
would fail if the fixes were removed.

TestGossipSyncerStateHandlerErrors is a table-driven test covering four
scenarios: context cancellation and peer disconnect during syncingChans
state, and context cancellation and network errors during queryNewChannels
state. Each test case verifies both attempt count (no endless loop) and
clean shutdown (no deadlock).

TestGossipSyncerProcessChanRangeReplyError verifies that errors from
processChanRangeReply in the waitingQueryRangeReply state cause clean
exit. This test sends multiple malformed messages and checks that only
the first is processed before the goroutine exits, using channel queue
depth to detect if the goroutine is still running.

All tests are race-detector clean and use mutation testing validation:
removing any of the error return statements causes the corresponding
tests to fail, confirming the tests properly verify the fixes.

Author: Roasbeef

discovery/syncer_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
 	"github.com/davecgh/go-spew/spew"
 	graphdb "github.com/lightningnetwork/lnd/graph/db"
+	"github.com/lightningnetwork/lnd/lnpeer"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/stretchr/testify/require"
 )
@@ -232,6 +233,106 @@ func newTestSyncer(hID lnwire.ShortChannelID,
 	return msgChan, syncer, cfg.channelSeries.(*mockChannelGraphTimeSeries)
 }
 
+// errorInjector provides thread-safe error injection for test syncers and
+// tracks the number of send attempts to detect endless loops.
+type errorInjector struct {
+	mu           sync.Mutex
+	err          error
+	attemptCount int
+}
+
+// setError sets the error that will be returned by sendMsg calls.
+func (ei *errorInjector) setError(err error) {
+	ei.mu.Lock()
+	defer ei.mu.Unlock()
+	ei.err = err
+}
+
+// getError retrieves the current error in a thread-safe manner and increments
+// the attempt counter.
+func (ei *errorInjector) getError() error {
+	ei.mu.Lock()
+	defer ei.mu.Unlock()
+	ei.attemptCount++
+	return ei.err
+}
+
+// getAttemptCount returns the number of times sendMsg was called.
+func (ei *errorInjector) getAttemptCount() int {
+	ei.mu.Lock()
+	defer ei.mu.Unlock()
+	return ei.attemptCount
+}
+
+// newErrorInjectingSyncer creates a GossipSyncer with controllable error
+// injection for testing error handling. The returned errorInjector can be used
+// to inject errors into sendMsg calls.
+func newErrorInjectingSyncer(hID lnwire.ShortChannelID,
+	chunkSize int32) (*GossipSyncer, *errorInjector, chan []lnwire.Message) {
+
+	ei := &errorInjector{}
+	msgChan := make(chan []lnwire.Message, 20)
+
+	cfg := gossipSyncerCfg{
+		channelSeries:          newMockChannelGraphTimeSeries(hID),
+		encodingType:           defaultEncoding,
+		chunkSize:              chunkSize,
+		batchSize:              chunkSize,
+		noSyncChannels:         false,
+		noReplyQueries:         true,
+		noTimestampQueryOption: false,
+		sendMsg: func(_ context.Context, _ bool,
+			msgs ...lnwire.Message) error {
+
+			// Check if we should inject an error.
+			if err := ei.getError(); err != nil {
+				return err
+			}
+
+			msgChan <- msgs
+			return nil
+		},
+		bestHeight: func() uint32 {
+			return latestKnownHeight
+		},
+		markGraphSynced:          func() {},
+		maxQueryChanRangeReplies: maxQueryChanRangeReplies,
+		timestampQueueSize:       10,
+	}
+
+	syncerSema := make(chan struct{}, 1)
+	syncerSema <- struct{}{}
+
+	syncer := newGossipSyncer(cfg, syncerSema)
+
+	return syncer, ei, msgChan
+}
+
+// assertSyncerExitsCleanly verifies that a syncer stops cleanly within the
+// given timeout. This is used to ensure error handling doesn't cause endless
+// loops.
+func assertSyncerExitsCleanly(t *testing.T, syncer *GossipSyncer,
+	timeout time.Duration) {
+
+	t.Helper()
+
+	stopChan := make(chan struct{})
+	go func() {
+		syncer.Stop()
+		close(stopChan)
+	}()
+
+	select {
+	case <-stopChan:
+		// Success - syncer stopped cleanly.
+	case <-time.After(timeout):
+		t.Fatal(
+			"syncer did not stop within timeout - possible " +
+				"endless loop",
+		)
+	}
+}
+
 // TestGossipSyncerFilterGossipMsgsNoHorizon tests that if the remote peer
 // doesn't have a horizon set, then we won't send any incoming messages to it.
 func TestGossipSyncerFilterGossipMsgsNoHorizon(t *testing.T) {
@@ -2411,3 +2512,200 @@ func TestGossipSyncerMaxChannelRangeReplies(t *testing.T) {
 		},
 	}, nil))
 }
+
+// TestGossipSyncerStateHandlerErrors tests that errors in state handlers cause
+// the channelGraphSyncer goroutine to exit cleanly without endless retry loops.
+// This is a table-driven test covering various error types and states.
+func TestGossipSyncerStateHandlerErrors(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		state       syncerState
+		setupState  func(*GossipSyncer)
+		chunkSize   int32
+		injectedErr error
+	}{
+		{
+			name:        "context cancel during syncingChans",
+			state:       syncingChans,
+			chunkSize:   defaultChunkSize,
+			injectedErr: context.Canceled,
+			setupState:  func(s *GossipSyncer) {},
+		},
+		{
+			name:        "peer exit during syncingChans",
+			state:       syncingChans,
+			chunkSize:   defaultChunkSize,
+			injectedErr: lnpeer.ErrPeerExiting,
+			setupState:  func(s *GossipSyncer) {},
+		},
+		{
+			name:        "context cancel during queryNewChannels",
+			state:       queryNewChannels,
+			chunkSize:   2,
+			injectedErr: context.Canceled,
+			setupState: func(s *GossipSyncer) {
+				s.newChansToQuery = []lnwire.ShortChannelID{
+					lnwire.NewShortChanIDFromInt(1),
+					lnwire.NewShortChanIDFromInt(2),
+					lnwire.NewShortChanIDFromInt(3),
+				}
+			},
+		},
+		{
+			name:        "network error during queryNewChannels",
+			state:       queryNewChannels,
+			chunkSize:   2,
+			injectedErr: errors.New("connection closed"),
+			setupState: func(s *GossipSyncer) {
+				s.newChansToQuery = []lnwire.ShortChannelID{
+					lnwire.NewShortChanIDFromInt(1),
+					lnwire.NewShortChanIDFromInt(2),
+				}
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			// Create syncer with error injection capability.
+			hID := lnwire.NewShortChanIDFromInt(10)
+			syncer, errInj, _ := newErrorInjectingSyncer(
+				hID, tt.chunkSize,
+			)
+
+			// Set up the initial state and any required state data.
+			syncer.setSyncState(tt.state)
+			tt.setupState(syncer)
+
+			// Inject the error that should cause the goroutine to
+			// exit.
+			errInj.setError(tt.injectedErr)
+
+			// Start the syncer which spawns the channelGraphSyncer
+			// goroutine.
+			syncer.Start()
+
+			// Wait long enough that an endless loop would
+			// accumulate many attempts. With the fix, we should
+			// only see 1-3 attempts. Without the fix, we'd see
+			// 50-100+ attempts.
+			time.Sleep(500 * time.Millisecond)
+
+			// Check how many send attempts were made. This verifies
+			// that the state handler doesn't loop endlessly.
+			attemptCount := errInj.getAttemptCount()
+			require.GreaterOrEqual(
+				t, attemptCount, 1,
+				"state handler was not called - test "+
+					"setup issue",
+			)
+			require.LessOrEqual(
+				t, attemptCount, 5,
+				"too many attempts (%d) - endless loop "+
+					"not fixed",
+				attemptCount,
+			)
+
+			// Verify the syncer exits cleanly without hanging.
+			assertSyncerExitsCleanly(t, syncer, 2*time.Second)
+		})
+	}
+}
+
+// TestGossipSyncerProcessChanRangeReplyError tests that errors from
+// processChanRangeReply cause the goroutine to exit cleanly. This test uses
+// a different approach than the state handler tests because the
+// waitingQueryRangeReply state waits in a select rather than looping.
+func TestGossipSyncerProcessChanRangeReplyError(t *testing.T) {
+	t.Parallel()
+
+	hID := lnwire.NewShortChanIDFromInt(10)
+	msgChan := make(chan []lnwire.Message, 20)
+
+	// Create custom channel series.
+	chanSeries := newMockChannelGraphTimeSeries(hID)
+
+	cfg := gossipSyncerCfg{
+		channelSeries:          chanSeries,
+		encodingType:           defaultEncoding,
+		chunkSize:              defaultChunkSize,
+		batchSize:              defaultChunkSize,
+		noSyncChannels:         false,
+		noReplyQueries:         true,
+		noTimestampQueryOption: false,
+		sendMsg: func(_ context.Context, _ bool,
+			msgs ...lnwire.Message) error {
+
+			msgChan <- msgs
+			return nil
+		},
+		bestHeight: func() uint32 {
+			return latestKnownHeight
+		},
+		markGraphSynced:          func() {},
+		maxQueryChanRangeReplies: maxQueryChanRangeReplies,
+		timestampQueueSize:       10,
+	}
+
+	syncerSema := make(chan struct{}, 1)
+	syncerSema <- struct{}{}
+
+	syncer := newGossipSyncer(cfg, syncerSema)
+
+	// Start the syncer which will send a QueryChannelRange message.
+	syncer.Start()
+	defer syncer.Stop()
+
+	// Wait for the syncer to send its QueryChannelRange message.
+	var query *lnwire.QueryChannelRange
+	select {
+	case msgs := <-msgChan:
+		require.Len(t, msgs, 1)
+		var ok bool
+		query, ok = msgs[0].(*lnwire.QueryChannelRange)
+		require.True(t, ok, "expected QueryChannelRange message")
+	case <-time.After(time.Second):
+		t.Fatal("expected query message")
+	}
+
+	// Send 10 malformed replies. With the fix, only the first one should
+	// be processed before the goroutine exits. Without the fix, all would
+	// be processed (goroutine stays in waitingQueryRangeReply state).
+	for i := 0; i < 10; i++ {
+		malformedReply := &lnwire.ReplyChannelRange{
+			ChainHash: query.ChainHash,
+			// FirstBlockHeight is before query range (invalid).
+			FirstBlockHeight: query.FirstBlockHeight - 100,
+			NumBlocks:        query.NumBlocks,
+			Complete:         1,
+			ShortChanIDs:     []lnwire.ShortChannelID{},
+		}
+
+		// Send the malformed reply.
+		err := syncer.ProcessQueryMsg(malformedReply, nil)
+		require.NoError(t, err)
+
+		// Brief delay to let goroutine process.
+		time.Sleep(20 * time.Millisecond)
+	}
+
+	// With the fix: goroutine exits after first error, so remaining 9
+	// messages stay queued in gossipMsgs channel (buffer size 50).
+	// Without fix: goroutine processes all 10 messages.
+	//
+	// Check channel length. If goroutine exited, we should have ~9 messages
+	// queued (first was processed, rest are stuck).
+	channelLen := len(syncer.gossipMsgs)
+	require.GreaterOrEqual(t, channelLen, 7,
+		"expected messages queued (goroutine exited), but found %d - "+
+			"goroutine may still be processing", channelLen)
+
+	// The channelGraphSyncer goroutine should have exited. Verify stop
+	// completes quickly.
+	assertSyncerExitsCleanly(t, syncer, 2*time.Second)
+}