From 3a465c64b5dcc16f3114fb00af03500604115143 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sat, 21 Jul 2018 19:49:08 -0700 Subject: [PATCH 1/8] channeldb: modify pruneGraphNodes to prune nodes if no edges exist --- channeldb/graph.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/channeldb/graph.go b/channeldb/graph.go index 920193aa..6ab7b35d 100644 --- a/channeldb/graph.go +++ b/channeldb/graph.go @@ -734,7 +734,11 @@ func (c *ChannelGraph) pruneGraphNodes(tx *bolt.Tx, nodes *bolt.Bucket, numChansLeft++ return nil }) - if err != nil { + + // If we're unable read the node, or no edges exist in the + // graph atm (so all the nodes are unconnected), then we'll + // just skip this node all together. + if err != nil && err != ErrGraphNoEdgesFound { continue } From 0645261e5e7bea75a96a626a4043e4005839297e Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sat, 21 Jul 2018 19:49:54 -0700 Subject: [PATCH 2/8] channeldb: add nwe PruneGraphNodes method to prune unconnected nodes --- channeldb/graph.go | 39 +++++++++++++++++++++++++++-- channeldb/graph_test.go | 55 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/channeldb/graph.go b/channeldb/graph.go index 6ab7b35d..f418ddfb 100644 --- a/channeldb/graph.go +++ b/channeldb/graph.go @@ -10,12 +10,12 @@ import ( "net" "time" - "github.com/coreos/bbolt" - "github.com/lightningnetwork/lnd/lnwire" "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" + "github.com/coreos/bbolt" + "github.com/lightningnetwork/lnd/lnwire" ) var ( @@ -698,6 +698,41 @@ func (c *ChannelGraph) PruneGraph(spentOutputs []*wire.OutPoint, return chansClosed, nil } +// PruneGraphNodes is a garbage collection method which attempts to prune out +// any nodes from the channel graph that are currently unconnected. This ensure +// that we only maintain a graph of reachable nodes. In the event that a pruned +// node gains more channels, it will be re-added back to the graph. +func (c *ChannelGraph) PruneGraphNodes() error { + // We'll use this map to collect a + nodesToMaybePrune := make(map[[33]byte]struct{}) + + return c.db.Update(func(tx *bolt.Tx) error { + nodes, err := tx.CreateBucketIfNotExists(nodeBucket) + if err != nil { + return err + } + + err = nodes.ForEach(func(pubKey, nodeBytes []byte) error { + // Skip anything that may actually be a sub-bucket. + if len(pubKey) != 33 { + return nil + } + + var nodePub [33]byte + copy(nodePub[:], pubKey) + + nodesToMaybePrune[nodePub] = struct{}{} + + return nil + }) + if err != nil { + return err + } + + return c.pruneGraphNodes(tx, nodes, nodesToMaybePrune) + }) +} + // pruneGraphNodes attempts to remove any nodes from the graph who have had a // channel closed within the current block. If the node still has existing // channels in the graph, this will act as a no-op. diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 2533a66a..6e340f2f 100644 --- a/channeldb/graph_test.go +++ b/channeldb/graph_test.go @@ -14,12 +14,12 @@ import ( "testing" "time" - "github.com/coreos/bbolt" - "github.com/davecgh/go-spew/spew" - "github.com/lightningnetwork/lnd/lnwire" "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/wire" + "github.com/coreos/bbolt" + "github.com/davecgh/go-spew/spew" + "github.com/lightningnetwork/lnd/lnwire" ) var ( @@ -2058,6 +2058,55 @@ func TestChannelEdgePruningUpdateIndexDeletion(t *testing.T) { } } +// TestPruneGraphNodes tests that unconnected vertexes are pruned via the +// PruneSyncState method. +func TestPruneGraphNodes(t *testing.T) { + t.Parallel() + + db, cleanUp, err := makeTestDB() + defer cleanUp() + if err != nil { + t.Fatalf("unable to make test database: %v", err) + } + + // We'll start off by inserting our source node, to ensure that it's + // the only node left after we prune the graph. + graph := db.ChannelGraph() + sourceNode, err := createTestVertex(db) + if err != nil { + t.Fatalf("unable to create source node: %v", err) + } + if err := graph.SetSourceNode(sourceNode); err != nil { + t.Fatalf("unable to set source node: %v", err) + } + + // With the source node inserted, we'll now add two nodes into the + // channel graph, as they don't have any channels they should be + // removed from the graph at the end. + 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) + } + node2, err := createTestVertex(db) + if err != nil { + t.Fatalf("unable to create test node: %v", err) + } + if err := graph.AddLightningNode(node2); err != nil { + t.Fatalf("unable to add node: %v", err) + } + + if err := graph.PruneGraphNodes(); err != nil { + t.Fatalf("unable to prune graph nodes: %v", err) + } + + // There should only be a single node left at this point, the source + // node. + assertNumNodes(t, graph, 1) +} + // 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 f8cbe34e936049a1ef0695bd91d12a021d397383 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sat, 21 Jul 2018 19:52:21 -0700 Subject: [PATCH 3/8] routing: prune nodes from the channel graph on start up --- routing/router.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/routing/router.go b/routing/router.go index 4d62de28..de465325 100644 --- a/routing/router.go +++ b/routing/router.go @@ -9,6 +9,9 @@ import ( "sync/atomic" "time" + "github.com/btcsuite/btcd/btcec" + "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcutil" "github.com/coreos/bbolt" "github.com/davecgh/go-spew/spew" "github.com/lightningnetwork/lnd/channeldb" @@ -17,9 +20,6 @@ import ( "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/multimutex" "github.com/lightningnetwork/lnd/routing/chainview" - "github.com/btcsuite/btcd/btcec" - "github.com/btcsuite/btcd/wire" - "github.com/btcsuite/btcutil" "crypto/sha256" @@ -382,6 +382,13 @@ func (r *ChannelRouter) Start() error { return err } + // Finally, before we proceed, we'll prune any unconnected nodes from + // the graph in order to ensure we maintain a tight graph of "useful" + // nodes. + if err := r.cfg.Graph.PruneGraphNodes(); err != nil { + return err + } + r.wg.Add(1) go r.networkHandler() From 131eea4960e78628aeb7d13b6405f9c692661d1c Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sun, 22 Jul 2018 18:57:10 -0700 Subject: [PATCH 4/8] channeldb: modify TestPruneGraphNodes to test nodes w/ single edge In this commit, we extend the TestPruneGraphNodes test to also test the case of when a node is involved in a channel, but only a single edge for that channel has been advertised. In order to test this, we add an additional node to the graph, and also a new channel. However, this channel will only have a single edge advertised. As result, when we prune the set of edges, the only node remaining should be the node that didn't have any edges at all. --- channeldb/graph_test.go | 49 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 6e340f2f..5325704f 100644 --- a/channeldb/graph_test.go +++ b/channeldb/graph_test.go @@ -2080,9 +2080,9 @@ func TestPruneGraphNodes(t *testing.T) { t.Fatalf("unable to set source node: %v", err) } - // With the source node inserted, we'll now add two nodes into the - // channel graph, as they don't have any channels they should be - // removed from the graph at the end. + // With the source node inserted, we'll now add three nodes to the + // channel graph, at the end of the scenario, only two of these nodes + // should still be in the graph. node1, err := createTestVertex(db) if err != nil { t.Fatalf("unable to create test node: %v", err) @@ -2097,14 +2097,51 @@ func TestPruneGraphNodes(t *testing.T) { if err := graph.AddLightningNode(node2); err != nil { t.Fatalf("unable to add node: %v", err) } + node3, err := createTestVertex(db) + if err != nil { + t.Fatalf("unable to create test node: %v", err) + } + if err := graph.AddLightningNode(node3); err != nil { + t.Fatalf("unable to add node: %v", err) + } + // We'll now add a new edge to the graph, but only actually advertise + // the edge of *one* of the nodes. + edgeInfo, chanID := createEdge(100, 0, 0, 0, node1, node2) + if err := graph.AddChannelEdge(&edgeInfo); err != nil { + t.Fatalf("unable to add edge: %v", err) + } + + // We'll now insert an advertised edge, but it'll only be the edge that + // points from the first to the second node. + edge1 := randEdgePolicy(chanID.ToUint64(), edgeInfo.ChannelPoint, db) + edge1.Flags = 0 + edge1.Node = node1 + edge1.SigBytes = testSig.Serialize() + if err := graph.UpdateEdgePolicy(edge1); err != nil { + t.Fatalf("unable to update edge: %v", err) + } + + // We'll now initiate a around of graph pruning. if err := graph.PruneGraphNodes(); err != nil { t.Fatalf("unable to prune graph nodes: %v", err) } - // There should only be a single node left at this point, the source - // node. - assertNumNodes(t, graph, 1) + // At this point, there should be 3 nodes left in the graph still: the + // source node (which can't be pruned), and node 1+2. Nodes 1 and two + // should still be left in the graph as there's half of an advertised + // edge between them. + assertNumNodes(t, graph, 3) + + // Finally, we'll ensure that node3, the only fully unconnected node as + // properly deleted from the graph and not another node in its place. + node3Pub, err := node3.PubKey() + if err != nil { + t.Fatalf("unable to fetch the pubkey of node3: %v", err) + } + if _, err := graph.FetchLightningNode(node3Pub); err == nil { + t.Fatalf("node 3 should have been deleted!") + } } // compareNodes is used to compare two LightningNodes while excluding the From 6bf15e8f2b594e455988886765a5adaa3dc0e388 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sun, 22 Jul 2018 18:59:28 -0700 Subject: [PATCH 5/8] channeldb: fix bug in pruneGraphNodes by switching to using ref counting In this commit, we fix an existing bug in the pruneGraphNodes method. Before this commit, if a node was involved in a channel, but only one of the edges was advertised, then either it, or the other node would be erroneously pruned from the graph. They shouldn't be pruned as there's still an edge connecting the, although only 1/2 of the edge is actually advertised. In order to fix this, we'll now do two passes: the first pass will populate a ref count map of all known nodes in the graph, the second pass will increment the ref count each time a node is found in the graph. With this two pass method, we ensure that nodes are only deleted if there are absolutely no edges pointing to them within the graph. --- channeldb/graph.go | 135 +++++++++++++++++++++++---------------------- 1 file changed, 70 insertions(+), 65 deletions(-) diff --git a/channeldb/graph.go b/channeldb/graph.go index f418ddfb..8be7bf15 100644 --- a/channeldb/graph.go +++ b/channeldb/graph.go @@ -29,7 +29,7 @@ var ( // traversals. The graph is formed as a star-graph with the source node // at the center. // - // maps: pubKey -> nofInfo + // maps: pubKey -> nodeInfo // maps: source -> selfPubKey nodeBucket = []byte("graph-node") @@ -477,7 +477,7 @@ func (c *ChannelGraph) AddChannelEdge(edge *ChannelEdgeInfo) error { } // HasChannelEdge returns true if the database knows of a channel edge with the -// passed channel ID, and false otherwise. If the an edge with that ID is found +// passed channel ID, and false otherwise. If an edge with that ID is found // within the graph, then two time stamps representing the last time the edge // was updated for both directed edges are returned along with the boolean. func (c *ChannelGraph) HasChannelEdge(chanID uint64) (time.Time, time.Time, bool, error) { @@ -588,13 +588,6 @@ func (c *ChannelGraph) PruneGraph(spentOutputs []*wire.OutPoint, var chansClosed []*ChannelEdgeInfo - // nodesWithChansClosed is the set of nodes, each identified by their - // compressed public key, who had a channel closed within the latest - // block. We'll use this later on to determine whether we should prune - // them from the channel graph due to no longer having any other open - // channels. - nodesWithChansClosed := make(map[[33]byte]struct{}) - err := c.db.Update(func(tx *bolt.Tx) error { // First grab the edges bucket which houses the information // we'd like to delete @@ -655,11 +648,7 @@ func (c *ChannelGraph) PruneGraph(spentOutputs []*wire.OutPoint, return err } - // Include this channel in our list of closed channels - // and collect the node public keys at each end. chansClosed = append(chansClosed, &edgeInfo) - nodesWithChansClosed[edgeInfo.NodeKey1Bytes] = struct{}{} - nodesWithChansClosed[edgeInfo.NodeKey2Bytes] = struct{}{} } metaBucket, err := tx.CreateBucketIfNotExists(graphMetaBucket) @@ -689,7 +678,7 @@ func (c *ChannelGraph) PruneGraph(spentOutputs []*wire.OutPoint, // Now that the graph has been pruned, we'll also attempt to // prune any nodes that have had a channel closed within the // latest block. - return c.pruneGraphNodes(tx, nodes, nodesWithChansClosed) + return c.pruneGraphNodes(tx, nodes, edgeIndex) }) if err != nil { return nil, err @@ -703,33 +692,21 @@ func (c *ChannelGraph) PruneGraph(spentOutputs []*wire.OutPoint, // that we only maintain a graph of reachable nodes. In the event that a pruned // node gains more channels, it will be re-added back to the graph. func (c *ChannelGraph) PruneGraphNodes() error { - // We'll use this map to collect a - nodesToMaybePrune := make(map[[33]byte]struct{}) - return c.db.Update(func(tx *bolt.Tx) error { nodes, err := tx.CreateBucketIfNotExists(nodeBucket) if err != nil { return err } - - err = nodes.ForEach(func(pubKey, nodeBytes []byte) error { - // Skip anything that may actually be a sub-bucket. - if len(pubKey) != 33 { - return nil - } - - var nodePub [33]byte - copy(nodePub[:], pubKey) - - nodesToMaybePrune[nodePub] = struct{}{} - - return nil - }) - if err != nil { - return err + edges := tx.Bucket(edgeBucket) + if edges == nil { + return ErrGraphNotFound + } + edgeIndex := edges.Bucket(edgeIndexBucket) + if edgeIndex == nil { + return ErrGraphNoEdgesFound } - return c.pruneGraphNodes(tx, nodes, nodesToMaybePrune) + return c.pruneGraphNodes(tx, nodes, edgeIndex) }) } @@ -737,7 +714,7 @@ func (c *ChannelGraph) PruneGraphNodes() error { // channel closed within the current block. If the node still has existing // channels in the graph, this will act as a no-op. func (c *ChannelGraph) pruneGraphNodes(tx *bolt.Tx, nodes *bolt.Bucket, - nodePubKeys map[[33]byte]struct{}) error { + edgeIndex *bolt.Bucket) error { log.Trace("Pruning nodes from graph with no open channels") @@ -748,41 +725,69 @@ func (c *ChannelGraph) pruneGraphNodes(tx *bolt.Tx, nodes *bolt.Bucket, return err } - // We'll now iterate over every node which had a channel closed and - // check whether they have any other open channels left within the - // graph. If they don't, they'll be pruned from the channel graph. - for nodePubKey := range nodePubKeys { - if bytes.Equal(nodePubKey[:], sourceNode.PubKeyBytes[:]) { - continue - } - - node, err := fetchLightningNode(nodes, nodePubKey[:]) - if err != nil { - continue - } - node.db = c.db - - numChansLeft := 0 - err = node.ForEachChannel(tx, func(*bolt.Tx, *ChannelEdgeInfo, - *ChannelEdgePolicy, *ChannelEdgePolicy) error { - - numChansLeft++ + // We'll use this map to keep count the number of references to a node + // in the graph. A node should only be removed once it has no more + // references in the graph. + nodeRefCounts := make(map[[33]byte]int) + err = nodes.ForEach(func(pubKey, nodeBytes []byte) error { + // If this is the source key, then we skip this + // iteration as the value for this key is a pubKey + // rather than raw node information. + if bytes.Equal(pubKey, sourceKey) || len(pubKey) != 33 { return nil - }) + } - // If we're unable read the node, or no edges exist in the - // graph atm (so all the nodes are unconnected), then we'll - // just skip this node all together. - if err != nil && err != ErrGraphNoEdgesFound { + var nodePub [33]byte + copy(nodePub[:], pubKey) + nodeRefCounts[nodePub] = 0 + + return nil + }) + if err != nil { + return err + } + + // To ensure we never delete the source node, we'll start off by + // bumping its ref count to 1. + nodeRefCounts[sourceNode.PubKeyBytes] = 1 + + // Next, we'll run through the edgeIndex which maps a channel ID to the + // edge info. We'll use this scan to populate our reference count map + // above. + err = edgeIndex.ForEach(func(chanID, edgeInfoBytes []byte) error { + // The first 66 bytes of the edge info contain the pubkeys of + // the nodes that this edge attaches. We'll extract them, and + // add them to the ref count map. + var node1, node2 [33]byte + copy(node1[:], edgeInfoBytes[:33]) + copy(node2[:], edgeInfoBytes[33:]) + + // With the nodes extracted, we'll increase the ref count of + // each of the nodes. + nodeRefCounts[node1] += 1 + nodeRefCounts[node2] += 1 + + return nil + }) + if err != nil { + return err + } + + // Finally, we'll make a second pass over the set of nodes, and delete + // any nodes that have a ref count of zero. + for nodePubKey, refCount := range nodeRefCounts { + // If the ref count of the node isn't zero, then we can safely + // skip it as it still has edges to or from it within the + // graph. + if refCount != 0 { continue } - if numChansLeft == 0 { - err := c.deleteLightningNode(tx, nodePubKey[:]) - if err != nil { - log.Tracef("Unable to prune node %x from the "+ - "graph: %v", nodePubKey, err) - } + // If we reach this point, then there are no longer any edges + // that connect this node, so we can delete it. + if err := c.deleteLightningNode(tx, nodePubKey[:]); err != nil { + log.Warnf("Unable to prune node %x from the "+ + "graph: %v", nodePubKey, err) } } From 8519ea869d3f58b29e1414b867ec9b9740bbcf64 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sun, 22 Jul 2018 21:00:43 -0700 Subject: [PATCH 6/8] channeldb: add logging for nodes pruned from the channel graph --- channeldb/graph.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/channeldb/graph.go b/channeldb/graph.go index 8be7bf15..11431192 100644 --- a/channeldb/graph.go +++ b/channeldb/graph.go @@ -775,6 +775,7 @@ func (c *ChannelGraph) pruneGraphNodes(tx *bolt.Tx, nodes *bolt.Bucket, // Finally, we'll make a second pass over the set of nodes, and delete // any nodes that have a ref count of zero. + var numNodesPruned int for nodePubKey, refCount := range nodeRefCounts { // If the ref count of the node isn't zero, then we can safely // skip it as it still has edges to or from it within the @@ -788,7 +789,18 @@ func (c *ChannelGraph) pruneGraphNodes(tx *bolt.Tx, nodes *bolt.Bucket, if err := c.deleteLightningNode(tx, nodePubKey[:]); err != nil { log.Warnf("Unable to prune node %x from the "+ "graph: %v", nodePubKey, err) + continue } + + log.Infof("Pruned unconnected node %x from channel graph", + nodePubKey[:]) + + numNodesPruned++ + } + + if numNodesPruned > 0 { + log.Infof("Pruned %v unconnected nodes from the channel graph", + numNodesPruned) } return nil From 2e75499787f284d491e1fe533bc2a03882664997 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sun, 22 Jul 2018 21:02:46 -0700 Subject: [PATCH 7/8] channeldb+routing: move adding shell nodes into db txn of AddChannelEdge In this commit, we fix a slight race condition that can occur when we go to add a shell node for a node announcement, but then right afterwards, a new block arrives that causes us to prune an unconnected node. To ensure this doesn't happen, we now add shell nodes within the same db transaction as AddChannelEdge. This ensures that the state is fully consistent and shell nodes will be added atomically along with the new channel edge. As a result of this change, we no longer need to add shell nodes within the ChannelRouter, as the database will take care of this operation as it should. --- channeldb/graph.go | 56 +++++++++++++++++++++++++++++++++++++++++----- routing/router.go | 28 ----------------------- 2 files changed, 51 insertions(+), 33 deletions(-) diff --git a/channeldb/graph.go b/channeldb/graph.go index 11431192..49b73aad 100644 --- a/channeldb/graph.go +++ b/channeldb/graph.go @@ -439,6 +439,10 @@ func (c *ChannelGraph) AddChannelEdge(edge *ChannelEdgeInfo) error { binary.BigEndian.PutUint64(chanKey[:], edge.ChannelID) return c.db.Update(func(tx *bolt.Tx) error { + nodes, err := tx.CreateBucketIfNotExists(nodeBucket) + if err != nil { + return err + } edges, err := tx.CreateBucketIfNotExists(edgeBucket) if err != nil { return err @@ -459,6 +463,45 @@ func (c *ChannelGraph) AddChannelEdge(edge *ChannelEdgeInfo) error { return ErrEdgeAlreadyExist } + // Before we insert the channel into the database, we'll ensure + // that both nodes already exist in the channel graph. If + // either node doesn't, then we'll insert a "shell" node that + // just includes its public key, so subsequent validation and + // queries can work properly. + _, node1Err := fetchLightningNode(nodes, edge.NodeKey1Bytes[:]) + switch { + case node1Err == ErrGraphNodeNotFound: + node1Shell := LightningNode{ + PubKeyBytes: edge.NodeKey1Bytes, + HaveNodeAnnouncement: false, + } + err := addLightningNode(tx, &node1Shell) + if err != nil { + return fmt.Errorf("unable to create shell node "+ + "for: %x", edge.NodeKey1Bytes) + + } + case node1Err != nil: + return err + } + + _, node2Err := fetchLightningNode(nodes, edge.NodeKey2Bytes[:]) + switch { + case node2Err == ErrGraphNodeNotFound: + node2Shell := LightningNode{ + PubKeyBytes: edge.NodeKey2Bytes, + HaveNodeAnnouncement: false, + } + err := addLightningNode(tx, &node2Shell) + if err != nil { + return fmt.Errorf("unable to create shell node "+ + "for: %x", edge.NodeKey2Bytes) + + } + case node2Err != nil: + return err + } + // If the edge hasn't been created yet, then we'll first add it // to the edge index in order to associate the edge between two // nodes and also store the static components of the channel. @@ -2229,8 +2272,9 @@ func (c *ChannelGraph) FetchChannelEdgesByOutpoint(op *wire.OutPoint) (*ChannelE // Once we have the information about the channels' parameters, // we'll fetch the routing policies for each for the directed // edges. - e1, e2, err := fetchChanEdgePolicies(edgeIndex, edges, nodes, - chanID, c.db) + e1, e2, err := fetchChanEdgePolicies( + edgeIndex, edges, nodes, chanID, c.db, + ) if err != nil { return err } @@ -2288,8 +2332,9 @@ func (c *ChannelGraph) FetchChannelEdgesByID(chanID uint64) (*ChannelEdgeInfo, * } edgeInfo = &edge - e1, e2, err := fetchChanEdgePolicies(edgeIndex, edges, nodes, - channelID[:], c.db) + e1, e2, err := fetchChanEdgePolicies( + edgeIndex, edges, nodes, channelID[:], c.db, + ) if err != nil { return err } @@ -2889,7 +2934,8 @@ func deserializeChanEdgePolicy(r io.Reader, node, err := fetchLightningNode(nodes, pub[:]) if err != nil { - return nil, err + return nil, fmt.Errorf("unable to fetch node: %x, %v", + pub[:], err) } edge.Node = &node diff --git a/routing/router.go b/routing/router.go index de465325..0e5b6e24 100644 --- a/routing/router.go +++ b/routing/router.go @@ -956,34 +956,6 @@ func (r *ChannelRouter) processUpdate(msg interface{}) error { "chan_id=%v", msg.ChannelID) } - // Query the database for the existence of the two nodes in this - // channel. If not found, add a partial node to the database, - // containing only the node keys. - _, exists, _ = r.cfg.Graph.HasLightningNode(msg.NodeKey1Bytes) - if !exists { - node1 := &channeldb.LightningNode{ - PubKeyBytes: msg.NodeKey1Bytes, - HaveNodeAnnouncement: false, - } - err := r.cfg.Graph.AddLightningNode(node1) - if err != nil { - return errors.Errorf("unable to add node %v to"+ - " the graph: %v", node1.PubKeyBytes, err) - } - } - _, exists, _ = r.cfg.Graph.HasLightningNode(msg.NodeKey2Bytes) - if !exists { - node2 := &channeldb.LightningNode{ - PubKeyBytes: msg.NodeKey2Bytes, - HaveNodeAnnouncement: false, - } - err := r.cfg.Graph.AddLightningNode(node2) - if err != nil { - return errors.Errorf("unable to add node %v to"+ - " the graph: %v", node2.PubKeyBytes, err) - } - } - // Before we can add the channel to the channel graph, we need // to obtain the full funding outpoint that's encoded within // the channel ID. From ef4c7d2a780ecfaa30b6c01ba862d61b4477ef82 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sun, 22 Jul 2018 21:05:40 -0700 Subject: [PATCH 8/8] channeldb: add new TestAddChannelEdgeShellNodes test --- channeldb/graph_test.go | 63 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 5325704f..cc3055b3 100644 --- a/channeldb/graph_test.go +++ b/channeldb/graph_test.go @@ -2144,6 +2144,69 @@ func TestPruneGraphNodes(t *testing.T) { } } +// TestAddChannelEdgeShellNodes tests that when we attempt to add a ChannelEdge +// to the graph, one or both of the nodes the edge involves aren't found in the +// database, then shell edges are created for each node if needed. +func TestAddChannelEdgeShellNodes(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() + + // To start, we'll create two nodes, and only add one of them to the + // channel graph. + 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) + } + node2, err := createTestVertex(db) + if err != nil { + t.Fatalf("unable to create test node: %v", err) + } + + // We'll now create an edge between the two nodes, as a result, node2 + // should be inserted into the database as a shell node. + edgeInfo, _ := createEdge(100, 0, 0, 0, node1, node2) + if err := graph.AddChannelEdge(&edgeInfo); err != nil { + t.Fatalf("unable to add edge: %v", err) + } + + node1Pub, err := node1.PubKey() + if err != nil { + t.Fatalf("unable to parse node 1 pub: %v", err) + } + node2Pub, err := node2.PubKey() + if err != nil { + t.Fatalf("unable to parse node 2 pub: %v", err) + } + + // Ensure that node1 was inserted as a full node, while node2 only has + // a shell node present. + node1, err = graph.FetchLightningNode(node1Pub) + if err != nil { + t.Fatalf("unable to fetch node1: %v", err) + } + if !node1.HaveNodeAnnouncement { + t.Fatalf("have shell announcement for node1, shouldn't") + } + + node2, err = graph.FetchLightningNode(node2Pub) + if err != nil { + t.Fatalf("unable to fetch node2: %v", err) + } + if node2.HaveNodeAnnouncement { + t.Fatalf("should have shell announcement for node2, but is full") + } +} + // 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.