From 06d03b55a3bd3a1d4a42dcac41a32ee3088b7fd7 Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Thu, 16 Aug 2018 18:20:51 -0700
Subject: [PATCH 1/2] channeldb: add new TestNodePruningUpdateIndexDeletion
 test

In this commit, we add a new test to expose a lurking bug within the
graph database code. As is, when we go to delete a node from the
database, we don't also remove the entries within the update index. As a
result, if a user attempted to call NodeUpdatesInHorizon (or typically
as part of the p2p handshake), we would error out, as we would try to
read a node that no longer existed in the graph, as it was pruned.
---
 channeldb/graph_test.go | 64 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go
index b5684742..56b4dd38 100644
--- a/channeldb/graph_test.go
+++ b/channeldb/graph_test.go
@@ -2367,6 +2367,70 @@ func TestAddChannelEdgeShellNodes(t *testing.T) {
 	}
 }
 
+// TestNodePruningUpdateIndexDeletion tests that once a node has been removed
+// from the channel graph, we also remove the entry from the update index as
+// well.
+func TestNodePruningUpdateIndexDeletion(t *testing.T) {
+	t.Parallel()
+
+	db, cleanUp, err := makeTestDB()
+	defer cleanUp()
+	if err != nil {
+		t.Fatalf("unable to make test database: %v", err)
+	}
+
+	graph := db.ChannelGraph()
+
+	// We'll first populate our graph with a single node that will be
+	// removed shortly.
+	node1, err := createTestVertex(db)
+	if err != nil {
+		t.Fatalf("unable to create test node: %v", err)
+	}
+	if err := graph.AddLightningNode(node1); err != nil {
+		t.Fatalf("unable to add node: %v", err)
+	}
+
+	// We'll confirm that we can retrieve the node using
+	// NodeUpdatesInHorizon, using a time that's slightly beyond the last
+	// update time of our test node.
+	startTime := time.Unix(9, 0)
+	endTime := node1.LastUpdate.Add(time.Minute)
+	nodesInHorizon, err := graph.NodeUpdatesInHorizon(startTime, endTime)
+	if err != nil {
+		t.Fatalf("unable to fetch nodes in horizon: %v", err)
+	}
+
+	// We should only have a single node, and that node should exactly
+	// match the node we just inserted.
+	if len(nodesInHorizon) != 1 {
+		t.Fatalf("should have 1 nodes instead have: %v",
+			len(nodesInHorizon))
+	}
+	if err := compareNodes(node1, &nodesInHorizon[0]); err != nil {
+		t.Fatalf("nodes don't match: %v", err)
+	}
+
+	// We'll now delete the node from the graph, this should result in it
+	// being removed from the update index as well.
+	nodePub, _ := node1.PubKey()
+	if err := graph.DeleteLightningNode(nodePub); err != nil {
+		t.Fatalf("unable to delete node: %v", err)
+	}
+
+	// Now that the node has been deleted, we'll again query the nodes in
+	// the horizon. This time we should have no nodes at all.
+	nodesInHorizon, err = graph.NodeUpdatesInHorizon(startTime, endTime)
+	if err != nil {
+		t.Fatalf("unable to fetch nodes in horizon: %v", err)
+	}
+
+	if len(nodesInHorizon) != 0 {
+		t.Fatalf("should have zero nodes instead have: %v",
+			len(nodesInHorizon))
+	}
+}
+
 // compareNodes is used to compare two LightningNodes while excluding the
 // Features struct, which cannot be compared as the semantics for reserializing
 // the featuresMap have not been defined.

From 9353ab00b1a68f503928847157e0368d00110f8f Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Thu, 16 Aug 2018 18:21:39 -0700
Subject: [PATCH 2/2] channeldb: ensure that we remove update index entries for
 nodes upon deletion

---
 channeldb/graph.go | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/channeldb/graph.go b/channeldb/graph.go
index cfe00f42..dc481cf7 100644
--- a/channeldb/graph.go
+++ b/channeldb/graph.go
@@ -433,7 +433,36 @@ func (c *ChannelGraph) deleteLightningNode(tx *bolt.Tx,
 	if err := aliases.Delete(compressedPubKey); err != nil {
 		return err
 	}
-	return nodes.Delete(compressedPubKey)
+
+	// Before we delete the node, we'll fetch its current state so we can
+	// determine when its last update was to clear out the node update
+	// index.
+	node, err := fetchLightningNode(nodes, compressedPubKey)
+	if err != nil {
+		return err
+	}
+
+	if err := nodes.Delete(compressedPubKey); err != nil {
+
+		return err
+	}
+
+	// Finally, we'll delete the index entry for the node within the
+	// nodeUpdateIndexBucket as this node is no longer active, so we don't
+	// need to track its last update.
+	nodeUpdateIndex := nodes.Bucket(nodeUpdateIndexBucket)
+	if nodeUpdateIndex == nil {
+		return ErrGraphNodesNotFound
+	}
+
+	// In order to delete the entry, we'll need to reconstruct the key for
+	// its last update.
+	updateUnix := uint64(node.LastUpdate.Unix())
+	var indexKey [8 + 33]byte
+	byteOrder.PutUint64(indexKey[:8], updateUnix)
+	copy(indexKey[8:], compressedPubKey)
+
+	return nodeUpdateIndex.Delete(indexKey[:])
 }
 
 // AddChannelEdge adds a new (undirected, blank) edge to the graph database. An