From 37d9ee302c4b400fb8309b98a2e7ba614ed18754 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 18 Dec 2019 12:43:18 +0100 Subject: [PATCH] channeldb: take serialized key to fetch lightning node This prevents inefficient key conversions in a follow up commit that change the inner pathfinding loop. --- autopilot/graph.go | 10 +++++++++- channeldb/graph.go | 14 +++++++------- channeldb/graph_test.go | 33 ++++++++++----------------------- routing/pathfind.go | 8 +------- routing/pathfind_test.go | 12 ++---------- routing/router.go | 6 +----- routing/router_test.go | 4 ++-- rpcserver.go | 6 +----- server.go | 7 ++++++- 9 files changed, 39 insertions(+), 61 deletions(-) diff --git a/autopilot/graph.go b/autopilot/graph.go index f02d0a8e..c63d6650 100644 --- a/autopilot/graph.go +++ b/autopilot/graph.go @@ -13,6 +13,7 @@ import ( "github.com/coreos/bbolt" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/routing/route" ) var ( @@ -145,7 +146,14 @@ func (d *databaseChannelGraph) addRandChannel(node1, node2 *btcec.PublicKey, fetchNode := func(pub *btcec.PublicKey) (*channeldb.LightningNode, error) { if pub != nil { - dbNode, err := d.db.FetchLightningNode(pub) + vertex, err := route.NewVertexFromBytes( + pub.SerializeCompressed(), + ) + if err != nil { + return nil, err + } + + dbNode, err := d.db.FetchLightningNode(vertex) switch { case err == channeldb.ErrGraphNodeNotFound: fallthrough diff --git a/channeldb/graph.go b/channeldb/graph.go index abfc5d49..3ec06981 100644 --- a/channeldb/graph.go +++ b/channeldb/graph.go @@ -20,6 +20,7 @@ import ( "github.com/btcsuite/btcutil" "github.com/coreos/bbolt" "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/routing/route" ) var ( @@ -513,7 +514,7 @@ func (c *ChannelGraph) LookupAlias(pub *btcec.PublicKey) (string, error) { // DeleteLightningNode starts a new database transaction to remove a vertex/node // from the database according to the node's public key. -func (c *ChannelGraph) DeleteLightningNode(nodePub *btcec.PublicKey) error { +func (c *ChannelGraph) DeleteLightningNode(nodePub route.Vertex) error { // TODO(roasbeef): ensure dangling edges are removed... return c.db.Update(func(tx *bbolt.Tx) error { nodes := tx.Bucket(nodeBucket) @@ -521,9 +522,7 @@ func (c *ChannelGraph) DeleteLightningNode(nodePub *btcec.PublicKey) error { return ErrGraphNodeNotFound } - return c.deleteLightningNode( - nodes, nodePub.SerializeCompressed(), - ) + return c.deleteLightningNode(nodes, nodePub[:]) }) } @@ -2180,9 +2179,10 @@ func (l *LightningNode) isPublic(tx *bbolt.Tx, sourcePubKey []byte) (bool, error // FetchLightningNode attempts to look up a target node by its identity public // key. If the node isn't found in the database, then ErrGraphNodeNotFound is // returned. -func (c *ChannelGraph) FetchLightningNode(pub *btcec.PublicKey) (*LightningNode, error) { +func (c *ChannelGraph) FetchLightningNode(nodePub route.Vertex) (*LightningNode, + error) { + var node *LightningNode - nodePub := pub.SerializeCompressed() err := c.db.View(func(tx *bbolt.Tx) error { // First grab the nodes bucket which stores the mapping from // pubKey to node information. @@ -2193,7 +2193,7 @@ func (c *ChannelGraph) FetchLightningNode(pub *btcec.PublicKey) (*LightningNode, // If a key for this serialized public key isn't found, then // the target node doesn't exist within the database. - nodeBytes := nodes.Get(nodePub) + nodeBytes := nodes.Get(nodePub[:]) if nodeBytes == nil { return ErrGraphNodeNotFound } diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 0768e829..27e29871 100644 --- a/channeldb/graph_test.go +++ b/channeldb/graph_test.go @@ -20,6 +20,7 @@ import ( "github.com/coreos/bbolt" "github.com/davecgh/go-spew/spew" "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/routing/route" ) var ( @@ -37,6 +38,8 @@ var ( _, _ = testSig.S.SetString("18801056069249825825291287104931333862866033135609736119018462340006816851118", 10) testFeatures = lnwire.NewFeatureVector(nil, lnwire.Features) + + testPub = route.Vertex{2, 202, 4} ) func createLightningNode(db *DB, priv *btcec.PrivateKey) (*LightningNode, error) { @@ -80,7 +83,6 @@ func TestNodeInsertionAndDeletion(t *testing.T) { // We'd like to test basic insertion/deletion for vertexes from the // graph, so we'll create a test vertex to start with. - _, testPub := btcec.PrivKeyFromBytes(btcec.S256(), key[:]) node := &LightningNode{ HaveNodeAnnouncement: true, AuthSigBytes: testSig.Serialize(), @@ -90,9 +92,9 @@ func TestNodeInsertionAndDeletion(t *testing.T) { Features: testFeatures, Addresses: testAddrs, ExtraOpaqueData: []byte("extra new data"), + PubKeyBytes: testPub, db: db, } - copy(node.PubKeyBytes[:], testPub.SerializeCompressed()) // First, insert the node into the graph DB. This should succeed // without any errors. @@ -147,11 +149,10 @@ func TestPartialNode(t *testing.T) { // We want to be able to insert nodes into the graph that only has the // PubKey set. - _, testPub := btcec.PrivKeyFromBytes(btcec.S256(), key[:]) node := &LightningNode{ HaveNodeAnnouncement: false, + PubKeyBytes: testPub, } - copy(node.PubKeyBytes[:], testPub.SerializeCompressed()) if err := graph.AddLightningNode(node); err != nil { t.Fatalf("unable to add node: %v", err) @@ -175,9 +176,9 @@ func TestPartialNode(t *testing.T) { node = &LightningNode{ HaveNodeAnnouncement: false, LastUpdate: time.Unix(0, 0), + PubKeyBytes: testPub, db: db, } - copy(node.PubKeyBytes[:], testPub.SerializeCompressed()) if err := compareNodes(node, dbNode); err != nil { t.Fatalf("nodes don't match: %v", err) @@ -2386,11 +2387,7 @@ func TestPruneGraphNodes(t *testing.T) { // 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 { + if _, err := graph.FetchLightningNode(node3.PubKeyBytes); err == nil { t.Fatalf("node 3 should have been deleted!") } } @@ -2430,18 +2427,9 @@ func TestAddChannelEdgeShellNodes(t *testing.T) { 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) + node1, err = graph.FetchLightningNode(node1.PubKeyBytes) if err != nil { t.Fatalf("unable to fetch node1: %v", err) } @@ -2449,7 +2437,7 @@ func TestAddChannelEdgeShellNodes(t *testing.T) { t.Fatalf("have shell announcement for node1, shouldn't") } - node2, err = graph.FetchLightningNode(node2Pub) + node2, err = graph.FetchLightningNode(node2.PubKeyBytes) if err != nil { t.Fatalf("unable to fetch node2: %v", err) } @@ -2504,8 +2492,7 @@ func TestNodePruningUpdateIndexDeletion(t *testing.T) { // 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 { + if err := graph.DeleteLightningNode(node1.PubKeyBytes); err != nil { t.Fatalf("unable to delete node: %v", err) } diff --git a/routing/pathfind.go b/routing/pathfind.go index ea5315b6..9eca590a 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -7,7 +7,6 @@ import ( "math" "time" - "github.com/btcsuite/btcd/btcec" "github.com/coreos/bbolt" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/feature" @@ -447,12 +446,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // we have for the target node from our graph. features := r.DestFeatures if features == nil { - targetKey, err := btcec.ParsePubKey(target[:], btcec.S256()) - if err != nil { - return nil, err - } - - targetNode, err := g.graph.FetchLightningNode(targetKey) + targetNode, err := g.graph.FetchLightningNode(target) switch { // If the node exists and has features, use them directly. diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index acff0d2b..3a26428c 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -2070,11 +2070,7 @@ func TestPathFindSpecExample(t *testing.T) { // Carol, so we set "B" as the source node so path finding starts from // Bob. bob := ctx.aliases["B"] - bobKey, err := btcec.ParsePubKey(bob[:], btcec.S256()) - if err != nil { - t.Fatal(err) - } - bobNode, err := ctx.graph.FetchLightningNode(bobKey) + bobNode, err := ctx.graph.FetchLightningNode(bob) if err != nil { t.Fatalf("unable to find bob: %v", err) } @@ -2123,11 +2119,7 @@ func TestPathFindSpecExample(t *testing.T) { // Next, we'll set A as the source node so we can assert that we create // the proper route for any queries starting with Alice. alice := ctx.aliases["A"] - aliceKey, err := btcec.ParsePubKey(alice[:], btcec.S256()) - if err != nil { - t.Fatal(err) - } - aliceNode, err := ctx.graph.FetchLightningNode(aliceKey) + aliceNode, err := ctx.graph.FetchLightningNode(alice) if err != nil { t.Fatalf("unable to find alice: %v", err) } diff --git a/routing/router.go b/routing/router.go index ef38788d..87a73937 100644 --- a/routing/router.go +++ b/routing/router.go @@ -2089,11 +2089,7 @@ func (r *ChannelRouter) GetChannelByID(chanID lnwire.ShortChannelID) ( // // NOTE: This method is part of the ChannelGraphSource interface. func (r *ChannelRouter) FetchLightningNode(node route.Vertex) (*channeldb.LightningNode, error) { - pubKey, err := btcec.ParsePubKey(node[:], btcec.S256()) - if err != nil { - return nil, fmt.Errorf("unable to parse raw public key: %v", err) - } - return r.cfg.Graph.FetchLightningNode(pubKey) + return r.cfg.Graph.FetchLightningNode(node) } // ForEachNode is used to iterate over every node in router topology. diff --git a/routing/router_test.go b/routing/router_test.go index e3ae0108..483abec4 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -1319,7 +1319,7 @@ func TestAddEdgeUnknownVertexes(t *testing.T) { t.Fatalf("unable to find any routes: %v", err) } - copy1, err := ctx.graph.FetchLightningNode(priv1.PubKey()) + copy1, err := ctx.graph.FetchLightningNode(pub1) if err != nil { t.Fatalf("unable to fetch node: %v", err) } @@ -1328,7 +1328,7 @@ func TestAddEdgeUnknownVertexes(t *testing.T) { t.Fatalf("fetched node not equal to original") } - copy2, err := ctx.graph.FetchLightningNode(priv2.PubKey()) + copy2, err := ctx.graph.FetchLightningNode(pub2) if err != nil { t.Fatalf("unable to fetch node: %v", err) } diff --git a/rpcserver.go b/rpcserver.go index 239df76d..703ce25a 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -4231,11 +4231,7 @@ func (r *rpcServer) GetNodeInfo(ctx context.Context, // First, parse the hex-encoded public key into a full in-memory public // key object we can work with for querying. - pubKeyBytes, err := hex.DecodeString(in.PubKey) - if err != nil { - return nil, err - } - pubKey, err := btcec.ParsePubKey(pubKeyBytes, btcec.S256()) + pubKey, err := route.NewVertexFromStr(in.PubKey) if err != nil { return nil, err } diff --git a/server.go b/server.go index 46ec00d0..58f8f17b 100644 --- a/server.go +++ b/server.go @@ -3369,7 +3369,12 @@ func computeNextBackoff(currBackoff time.Duration) time.Duration { // fetchNodeAdvertisedAddr attempts to fetch an advertised address of a node. func (s *server) fetchNodeAdvertisedAddr(pub *btcec.PublicKey) (net.Addr, error) { - node, err := s.chanDB.ChannelGraph().FetchLightningNode(pub) + vertex, err := route.NewVertexFromBytes(pub.SerializeCompressed()) + if err != nil { + return nil, err + } + + node, err := s.chanDB.ChannelGraph().FetchLightningNode(vertex) if err != nil { return nil, err }