Merge pull request #1739 from Roasbeef/graph-private-chan-node-cleanup

channeldb: ensure we remove update index entries for nodes upon deletion

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

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.