From 977c139f3c2503776622c093a64dc58aba395e35 Mon Sep 17 00:00:00 2001
From: Wilmer Paulino <wilmer.paulino@gmail.com>
Date: Thu, 1 Aug 2019 14:04:45 -0700
Subject: [PATCH] discovery: handle graph synced status after stalled initial
 historical sync

This ensures that the graph synced status is marked true at some point
once a historical sync has completed. Before this commit, a stalled
historical sync could cause us to never mark the graph as synced.
---
 discovery/sync_manager.go      | 16 ++++++++++-
 discovery/sync_manager_test.go | 52 ++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/discovery/sync_manager.go b/discovery/sync_manager.go
index 6175e44a..b9a18763 100644
--- a/discovery/sync_manager.go
+++ b/discovery/sync_manager.go
@@ -381,7 +381,21 @@ func (m *SyncManager) syncerHandler() {
 		// Our HistoricalSyncTicker has ticked, so we'll randomly select
 		// a peer and force a historical sync with them.
 		case <-m.cfg.HistoricalSyncTicker.Ticks():
-			m.forceHistoricalSync()
+			s := m.forceHistoricalSync()
+
+			// If we've already performed our initial historical
+			// sync, then we have nothing left to do.
+			if m.IsGraphSynced() {
+				continue
+			}
+
+			// Otherwise, we'll track the peer we've performed a
+			// historical sync with in order to handle the case
+			// where our previous historical sync peer did not
+			// respond to our queries and we haven't ingested as
+			// much of the graph as we should.
+			initialHistoricalSyncer = s
+			initialHistoricalSyncSignal = s.ResetSyncedSignal()
 
 		case <-m.quit:
 			return
diff --git a/discovery/sync_manager_test.go b/discovery/sync_manager_test.go
index 7df7d75f..18404979 100644
--- a/discovery/sync_manager_test.go
+++ b/discovery/sync_manager_test.go
@@ -309,6 +309,58 @@ func TestSyncManagerForceHistoricalSync(t *testing.T) {
 	})
 }
 
+// TestSyncManagerGraphSyncedAfterHistoricalSyncReplacement ensures that the
+// sync manager properly marks the graph as synced given that our initial
+// historical sync has stalled, but a replacement has fully completed.
+func TestSyncManagerGraphSyncedAfterHistoricalSyncReplacement(t *testing.T) {
+	t.Parallel()
+
+	syncMgr := newTestSyncManager(0)
+	syncMgr.Start()
+	defer syncMgr.Stop()
+
+	// We should expect to see a QueryChannelRange message with a
+	// FirstBlockHeight of the genesis block, signaling that an initial
+	// historical sync is being attempted.
+	peer := randPeer(t, syncMgr.quit)
+	syncMgr.InitSyncState(peer)
+	assertMsgSent(t, peer, &lnwire.QueryChannelRange{
+		FirstBlockHeight: 0,
+		NumBlocks:        math.MaxUint32,
+	})
+
+	// The graph should not be considered as synced since the initial
+	// historical sync has not finished.
+	if syncMgr.IsGraphSynced() {
+		t.Fatal("expected graph to not be considered as synced")
+	}
+
+	// If an additional peer connects, then another historical sync should
+	// not be attempted.
+	finalHistoricalPeer := randPeer(t, syncMgr.quit)
+	syncMgr.InitSyncState(finalHistoricalPeer)
+	finalHistoricalSyncer := assertSyncerExistence(t, syncMgr, finalHistoricalPeer)
+	assertNoMsgSent(t, finalHistoricalPeer)
+
+	// To simulate that our initial historical sync has stalled, we'll force
+	// a historical sync with the new peer to ensure it is replaced.
+	syncMgr.cfg.HistoricalSyncTicker.(*ticker.Force).Force <- time.Time{}
+
+	// The graph should still not be considered as synced since the
+	// replacement historical sync has not finished.
+	if syncMgr.IsGraphSynced() {
+		t.Fatal("expected graph to not be considered as synced")
+	}
+
+	// Complete the replacement historical sync by transitioning the syncer
+	// to its final chansSynced state. The graph should be considered as
+	// synced after the fact.
+	assertTransitionToChansSynced(t, finalHistoricalSyncer, finalHistoricalPeer)
+	if !syncMgr.IsGraphSynced() {
+		t.Fatal("expected graph to be considered as synced")
+	}
+}
+
 // TestSyncManagerWaitUntilInitialHistoricalSync ensures that no GossipSyncers
 // are initialized as ActiveSync until the initial historical sync has been
 // completed. Once it does, the pending GossipSyncers should be transitioned to