From 37d9ee302c4b400fb8309b98a2e7ba614ed18754 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 18 Dec 2019 12:43:18 +0100 Subject: [PATCH 1/8] 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 f02d0a8e6..c63d66502 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 abfc5d496..3ec069818 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 0768e8293..27e298717 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 ea5315b6a..9eca590ae 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 acff0d2bb..3a26428c2 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 ef38788dc..87a739375 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 e3ae01081..483abec4d 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 239df76de..703ce25a8 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 46ec00d0f..58f8f17bd 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 } From 95ddab57faa611ecde2d3ab6402d1854ae42b0e0 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Fri, 20 Dec 2019 10:14:13 +0100 Subject: [PATCH 2/8] channeldb: add tx argument for FetchLightningNode To allow execution within an existing tx. --- autopilot/graph.go | 2 +- channeldb/graph.go | 21 +++++++++++++++++---- channeldb/graph_test.go | 15 ++++++++------- routing/pathfind.go | 2 +- routing/pathfind_test.go | 4 ++-- routing/router.go | 2 +- routing/router_test.go | 4 ++-- rpcserver.go | 2 +- server.go | 2 +- 9 files changed, 34 insertions(+), 20 deletions(-) diff --git a/autopilot/graph.go b/autopilot/graph.go index c63d66502..5792443f8 100644 --- a/autopilot/graph.go +++ b/autopilot/graph.go @@ -153,7 +153,7 @@ func (d *databaseChannelGraph) addRandChannel(node1, node2 *btcec.PublicKey, return nil, err } - dbNode, err := d.db.FetchLightningNode(vertex) + dbNode, err := d.db.FetchLightningNode(nil, vertex) switch { case err == channeldb.ErrGraphNodeNotFound: fallthrough diff --git a/channeldb/graph.go b/channeldb/graph.go index 3ec069818..61ca7a5fe 100644 --- a/channeldb/graph.go +++ b/channeldb/graph.go @@ -2179,11 +2179,17 @@ 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(nodePub route.Vertex) (*LightningNode, - error) { +// +// If the caller wishes to re-use an existing boltdb transaction, then it +// should be passed as the first argument. Otherwise the first argument should +// be nil and a fresh transaction will be created to execute the graph +// traversal. +func (c *ChannelGraph) FetchLightningNode(tx *bbolt.Tx, nodePub route.Vertex) ( + *LightningNode, error) { var node *LightningNode - err := c.db.View(func(tx *bbolt.Tx) error { + + fetchNode := func(tx *bbolt.Tx) error { // First grab the nodes bucket which stores the mapping from // pubKey to node information. nodes := tx.Bucket(nodeBucket) @@ -2210,7 +2216,14 @@ func (c *ChannelGraph) FetchLightningNode(nodePub route.Vertex) (*LightningNode, node = &n return nil - }) + } + + var err error + if tx == nil { + err = c.db.View(fetchNode) + } else { + err = fetchNode(tx) + } if err != nil { return nil, err } diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 27e298717..d79f15dd1 100644 --- a/channeldb/graph_test.go +++ b/channeldb/graph_test.go @@ -104,7 +104,7 @@ func TestNodeInsertionAndDeletion(t *testing.T) { // Next, fetch the node from the database to ensure everything was // serialized properly. - dbNode, err := graph.FetchLightningNode(testPub) + dbNode, err := graph.FetchLightningNode(nil, testPub) if err != nil { t.Fatalf("unable to locate node: %v", err) } @@ -128,7 +128,7 @@ func TestNodeInsertionAndDeletion(t *testing.T) { // Finally, attempt to fetch the node again. This should fail as the // node should have been deleted from the database. - _, err = graph.FetchLightningNode(testPub) + _, err = graph.FetchLightningNode(nil, testPub) if err != ErrGraphNodeNotFound { t.Fatalf("fetch after delete should fail!") } @@ -160,7 +160,7 @@ func TestPartialNode(t *testing.T) { // Next, fetch the node from the database to ensure everything was // serialized properly. - dbNode, err := graph.FetchLightningNode(testPub) + dbNode, err := graph.FetchLightningNode(nil, testPub) if err != nil { t.Fatalf("unable to locate node: %v", err) } @@ -192,7 +192,7 @@ func TestPartialNode(t *testing.T) { // Finally, attempt to fetch the node again. This should fail as the // node should have been deleted from the database. - _, err = graph.FetchLightningNode(testPub) + _, err = graph.FetchLightningNode(nil, testPub) if err != ErrGraphNodeNotFound { t.Fatalf("fetch after delete should fail!") } @@ -2387,7 +2387,8 @@ 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. - if _, err := graph.FetchLightningNode(node3.PubKeyBytes); err == nil { + _, err = graph.FetchLightningNode(nil, node3.PubKeyBytes) + if err == nil { t.Fatalf("node 3 should have been deleted!") } } @@ -2429,7 +2430,7 @@ func TestAddChannelEdgeShellNodes(t *testing.T) { // Ensure that node1 was inserted as a full node, while node2 only has // a shell node present. - node1, err = graph.FetchLightningNode(node1.PubKeyBytes) + node1, err = graph.FetchLightningNode(nil, node1.PubKeyBytes) if err != nil { t.Fatalf("unable to fetch node1: %v", err) } @@ -2437,7 +2438,7 @@ func TestAddChannelEdgeShellNodes(t *testing.T) { t.Fatalf("have shell announcement for node1, shouldn't") } - node2, err = graph.FetchLightningNode(node2.PubKeyBytes) + node2, err = graph.FetchLightningNode(nil, node2.PubKeyBytes) if err != nil { t.Fatalf("unable to fetch node2: %v", err) } diff --git a/routing/pathfind.go b/routing/pathfind.go index 9eca590ae..1049e5da1 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -446,7 +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 { - targetNode, err := g.graph.FetchLightningNode(target) + targetNode, err := g.graph.FetchLightningNode(tx, 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 3a26428c2..1bec640a5 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -2070,7 +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"] - bobNode, err := ctx.graph.FetchLightningNode(bob) + bobNode, err := ctx.graph.FetchLightningNode(nil, bob) if err != nil { t.Fatalf("unable to find bob: %v", err) } @@ -2119,7 +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"] - aliceNode, err := ctx.graph.FetchLightningNode(alice) + aliceNode, err := ctx.graph.FetchLightningNode(nil, alice) if err != nil { t.Fatalf("unable to find alice: %v", err) } diff --git a/routing/router.go b/routing/router.go index 87a739375..085db61f7 100644 --- a/routing/router.go +++ b/routing/router.go @@ -2089,7 +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) { - return r.cfg.Graph.FetchLightningNode(node) + return r.cfg.Graph.FetchLightningNode(nil, 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 483abec4d..26803492c 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(pub1) + copy1, err := ctx.graph.FetchLightningNode(nil, 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(pub2) + copy2, err := ctx.graph.FetchLightningNode(nil, pub2) if err != nil { t.Fatalf("unable to fetch node: %v", err) } diff --git a/rpcserver.go b/rpcserver.go index 703ce25a8..1ae6f2ad9 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -4239,7 +4239,7 @@ func (r *rpcServer) GetNodeInfo(ctx context.Context, // With the public key decoded, attempt to fetch the node corresponding // to this public key. If the node cannot be found, then an error will // be returned. - node, err := graph.FetchLightningNode(pubKey) + node, err := graph.FetchLightningNode(nil, pubKey) if err != nil { return nil, err } diff --git a/server.go b/server.go index 58f8f17bd..dd97b2be9 100644 --- a/server.go +++ b/server.go @@ -3374,7 +3374,7 @@ func (s *server) fetchNodeAdvertisedAddr(pub *btcec.PublicKey) (net.Addr, error) return nil, err } - node, err := s.chanDB.ChannelGraph().FetchLightningNode(vertex) + node, err := s.chanDB.ChannelGraph().FetchLightningNode(nil, vertex) if err != nil { return nil, err } From e546a2c42ba7cb91c44d803864a95b13d4ca5597 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 18 Dec 2019 08:16:53 +0100 Subject: [PATCH 3/8] routing: more efficient feature validation Also prepares for payload size tracking during pathfinding --- routing/pathfind.go | 92 ++++++++++++++++++++++++++++++--------------- 1 file changed, 62 insertions(+), 30 deletions(-) diff --git a/routing/pathfind.go b/routing/pathfind.go index 1049e5da1..2f64e311a 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -670,36 +670,6 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, edge.ChannelID) } - switch { - - // If this edge takes us to the final hop, we'll set the node - // features to those determined above. These are either taken - // from the destination features, e.g. virtual or invoice - // features, or loaded as a fallback from the graph. The - // transitive dependencies were already validated above, so no - // need to do so now. - // - // NOTE: This may overwrite features loaded from the graph if - // destination features were provided. This is fine though, - // since our route construction does not care where the features - // are actually taken from. In the future we may wish to do - // route construction within findPath, and avoid using - // ChannelEdgePolicy altogether. - case edge.Node.PubKeyBytes == target: - edge.Node.Features = features - - // Otherwise, this is some other intermediary node. Verify the - // transitive feature dependencies for this node, and skip the - // channel if they are invalid. - default: - err := feature.ValidateDeps(edge.Node.Features) - if err != nil { - log.Tracef("Node %x has invalid features", - edge.Node.PubKeyBytes) - return - } - } - // All conditions are met and this new tentative distance is // better than the current best known distance to this node. // The new better distance is recorded, and also our "next hop" @@ -724,6 +694,44 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // TODO(roasbeef): also add path caching // * similar to route caching, but doesn't factor in the amount + // Cache features because we visit nodes multiple times. + featureCache := make(map[route.Vertex]*lnwire.FeatureVector) + + // getGraphFeatures returns (cached) node features from the graph. + getGraphFeatures := func(node route.Vertex) (*lnwire.FeatureVector, + error) { + + // Check cache for features of the fromNode. + fromFeatures, ok := featureCache[node] + if !ok { + targetNode, err := g.graph.FetchLightningNode(tx, node) + switch { + + // If the node exists and has valid features, use them. + case err == nil: + err := feature.ValidateDeps(targetNode.Features) + if err == nil { + fromFeatures = targetNode.Features + } + + // If an error other than the node not existing is hit, + // abort. + case err != channeldb.ErrGraphNodeNotFound: + return nil, err + + // Otherwise, we couldn't find a node announcement, + // populate a blank feature vector. + default: + fromFeatures = lnwire.EmptyFeatureVector() + } + + // Update cache. + featureCache[node] = fromFeatures + } + + return fromFeatures, nil + } + routeToSelf := source == target for { nodesVisited++ @@ -771,6 +779,17 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, continue } + // Get feature vector for fromNode. + fromFeatures, err := getGraphFeatures(fromNode) + if err != nil { + return nil, err + } + + // If there are no valid features, skip this node. + if fromFeatures == nil { + continue + } + // Check if this candidate node is better than what we // already have. processEdge(fromNode, policy, partialPath) @@ -818,6 +837,19 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, } } + // For the final hop, we'll set the node features to those determined + // above. These are either taken from the destination features, e.g. + // virtual or invoice features, or loaded as a fallback from the graph. + // The transitive dependencies were already validated above, so no need + // to do so now. + // + // NOTE: This may overwrite features loaded from the graph if + // destination features were provided. This is fine though, since our + // route construction does not care where the features are actually + // taken from. In the future we may wish to do route construction within + // findPath, and avoid using ChannelEdgePolicy altogether. + pathEdges[len(pathEdges)-1].Node.Features = features + // The route is invalid if it spans more than 20 hops. The current // Sphinx (onion routing) implementation can only encode up to 20 hops // as the entire packet is fixed size. If this route is more than 20 From 72a63839753e11cfb5a6ead66c142f16f13d681c Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Tue, 17 Dec 2019 11:55:03 +0100 Subject: [PATCH 4/8] routing: use absolute final expiry in pathfinding --- routing/heap.go | 7 ++++--- routing/pathfind.go | 16 ++++++++++------ routing/pathfind_test.go | 34 +++++++++++++++++---------------- routing/payment_session.go | 4 +++- routing/payment_session_test.go | 4 ++-- routing/router.go | 18 +++++++++-------- routing/router_test.go | 2 +- 7 files changed, 48 insertions(+), 37 deletions(-) diff --git a/routing/heap.go b/routing/heap.go index 39a6f4ed9..e7e1b5878 100644 --- a/routing/heap.go +++ b/routing/heap.go @@ -24,9 +24,10 @@ type nodeWithDist struct { // amount that includes also the fees for subsequent hops. amountToReceive lnwire.MilliSatoshi - // incomingCltv is the expected cltv value for the incoming htlc of this - // node. This value does not include the final cltv. - incomingCltv uint32 + // incomingCltv is the expected absolute expiry height for the incoming + // htlc of this node. This value should already include the final cltv + // delta. + incomingCltv int32 // probability is the probability that from this node onward the route // is successful. diff --git a/routing/pathfind.go b/routing/pathfind.go index 2f64e311a..f74d00d95 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -47,7 +47,8 @@ const ( // pathFinder defines the interface of a path finding algorithm. type pathFinder = func(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, source, target route.Vertex, - amt lnwire.MilliSatoshi) ([]*channeldb.ChannelEdgePolicy, error) + amt lnwire.MilliSatoshi, finalHtlcExpiry int32) ( + []*channeldb.ChannelEdgePolicy, error) var ( // DefaultPaymentAttemptPenalty is the virtual cost in path finding weight @@ -411,7 +412,7 @@ func getMaxOutgoingAmt(node route.Vertex, outgoingChan *uint64, // that need to be paid along the path and accurately check the amount // to forward at every node against the available bandwidth. func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, - source, target route.Vertex, amt lnwire.MilliSatoshi) ( + source, target route.Vertex, amt lnwire.MilliSatoshi, finalHtlcExpiry int32) ( []*channeldb.ChannelEdgePolicy, error) { // Pathfinding can be a significant portion of the total payment @@ -542,10 +543,14 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, weight: 0, node: target, amountToReceive: amt, - incomingCltv: 0, + incomingCltv: finalHtlcExpiry, probability: 1, } + // Calculate the absolute cltv limit. Use uint64 to prevent an overflow + // if the cltv limit is MaxUint32. + absoluteCltvLimit := uint64(r.CltvLimit) + uint64(finalHtlcExpiry) + // processEdge is a helper closure that will be used to make sure edges // satisfy our specific requirements. processEdge := func(fromVertex route.Vertex, @@ -590,11 +595,10 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, timeLockDelta = edge.TimeLockDelta } - incomingCltv := toNodeDist.incomingCltv + - uint32(timeLockDelta) + incomingCltv := toNodeDist.incomingCltv + int32(timeLockDelta) // Check that we are within our CLTV limit. - if incomingCltv > r.CltvLimit { + if uint64(incomingCltv) > absoluteCltvLimit { return } diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 1bec640a5..c47f73486 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -823,6 +823,7 @@ func testBasicGraphPathFindingCase(t *testing.T, graphInstance *testGraphInstanc }, testPathFindingConfig, sourceNode.PubKeyBytes, target, paymentAmt, + startingHeight+finalHopCLTV, ) if test.expectFailureNoPath { if err == nil { @@ -1012,6 +1013,7 @@ func TestPathFindingWithAdditionalEdges(t *testing.T) { }, r, testPathFindingConfig, sourceNode.PubKeyBytes, doge.PubKeyBytes, paymentAmt, + 0, ) } @@ -1383,7 +1385,7 @@ func TestNewRoutePathTooLong(t *testing.T) { graph: graph.graph, }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, paymentAmt, + sourceNode.PubKeyBytes, target, paymentAmt, 0, ) if err != nil { t.Fatalf("path should have been found") @@ -1397,7 +1399,7 @@ func TestNewRoutePathTooLong(t *testing.T) { graph: graph.graph, }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, paymentAmt, + sourceNode.PubKeyBytes, target, paymentAmt, 0, ) if err == nil { t.Fatalf("should not have been able to find path, supposed to be "+ @@ -1437,7 +1439,7 @@ func TestPathNotAvailable(t *testing.T) { graph: graph.graph, }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, unknownNode, 100, + sourceNode.PubKeyBytes, unknownNode, 100, 0, ) if err != errNoPathFound { t.Fatalf("path shouldn't have been found: %v", err) @@ -1495,7 +1497,7 @@ func TestDestTLVGraphFallback(t *testing.T) { graph: ctx.graphParams.graph, }, r, testPathFindingConfig, - sourceNode.PubKeyBytes, target, 100, + sourceNode.PubKeyBytes, target, 100, 0, ) } @@ -1604,7 +1606,7 @@ func TestMissingFeatureDep(t *testing.T) { graph: ctx.graphParams.graph, }, r, testPathFindingConfig, - sourceNode.PubKeyBytes, target, 100, + sourceNode.PubKeyBytes, target, 100, 0, ) } @@ -1682,7 +1684,7 @@ func TestDestPaymentAddr(t *testing.T) { graph: ctx.graphParams.graph, }, r, testPathFindingConfig, - sourceNode.PubKeyBytes, target, 100, + sourceNode.PubKeyBytes, target, 100, 0, ) } @@ -1743,7 +1745,7 @@ func TestPathInsufficientCapacity(t *testing.T) { graph: graph.graph, }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, payAmt, + sourceNode.PubKeyBytes, target, payAmt, 0, ) if err != errNoPathFound { t.Fatalf("graph shouldn't be able to support payment: %v", err) @@ -1776,7 +1778,7 @@ func TestRouteFailMinHTLC(t *testing.T) { graph: graph.graph, }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, payAmt, + sourceNode.PubKeyBytes, target, payAmt, 0, ) if err != errNoPathFound { t.Fatalf("graph shouldn't be able to support payment: %v", err) @@ -1875,7 +1877,7 @@ func TestRouteFailDisabledEdge(t *testing.T) { graph: graph.graph, }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, payAmt, + sourceNode.PubKeyBytes, target, payAmt, 0, ) if err != nil { t.Fatalf("unable to find path: %v", err) @@ -1903,7 +1905,7 @@ func TestRouteFailDisabledEdge(t *testing.T) { graph: graph.graph, }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, payAmt, + sourceNode.PubKeyBytes, target, payAmt, 0, ) if err != nil { t.Fatalf("unable to find path: %v", err) @@ -1928,7 +1930,7 @@ func TestRouteFailDisabledEdge(t *testing.T) { graph: graph.graph, }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, payAmt, + sourceNode.PubKeyBytes, target, payAmt, 0, ) if err != errNoPathFound { t.Fatalf("graph shouldn't be able to support payment: %v", err) @@ -1962,7 +1964,7 @@ func TestPathSourceEdgesBandwidth(t *testing.T) { graph: graph.graph, }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, payAmt, + sourceNode.PubKeyBytes, target, payAmt, 0, ) if err != nil { t.Fatalf("unable to find path: %v", err) @@ -1986,7 +1988,7 @@ func TestPathSourceEdgesBandwidth(t *testing.T) { bandwidthHints: bandwidths, }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, payAmt, + sourceNode.PubKeyBytes, target, payAmt, 0, ) if err != errNoPathFound { t.Fatalf("graph shouldn't be able to support payment: %v", err) @@ -2004,7 +2006,7 @@ func TestPathSourceEdgesBandwidth(t *testing.T) { bandwidthHints: bandwidths, }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, payAmt, + sourceNode.PubKeyBytes, target, payAmt, 0, ) if err != nil { t.Fatalf("unable to find path: %v", err) @@ -2035,7 +2037,7 @@ func TestPathSourceEdgesBandwidth(t *testing.T) { bandwidthHints: bandwidths, }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, payAmt, + sourceNode.PubKeyBytes, target, payAmt, 0, ) if err != nil { t.Fatalf("unable to find path: %v", err) @@ -2872,7 +2874,7 @@ func (c *pathFindingTestContext) findPath(target route.Vertex, return findPath( &c.graphParams, &c.restrictParams, &c.pathFindingConfig, - c.source, target, amt, + c.source, target, amt, 0, ) } diff --git a/routing/payment_session.go b/routing/payment_session.go index e5474ddfc..47732e01a 100644 --- a/routing/payment_session.go +++ b/routing/payment_session.go @@ -113,6 +113,8 @@ func (p *paymentSession) RequestRoute(payment *LightningPayment, return nil, err } + finalHtlcExpiry := int32(height) + int32(finalCltvDelta) + path, err := p.pathFinder( &graphParams{ graph: ss.Graph, @@ -121,7 +123,7 @@ func (p *paymentSession) RequestRoute(payment *LightningPayment, }, restrictions, &ss.PathFindingConfig, ss.SelfNode.PubKeyBytes, payment.Target, - payment.Amount, + payment.Amount, finalHtlcExpiry, ) if err != nil { return nil, err diff --git a/routing/payment_session_test.go b/routing/payment_session_test.go index 14f98449a..55549c448 100644 --- a/routing/payment_session_test.go +++ b/routing/payment_session_test.go @@ -15,8 +15,8 @@ func TestRequestRoute(t *testing.T) { findPath := func(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, source, target route.Vertex, - amt lnwire.MilliSatoshi) ([]*channeldb.ChannelEdgePolicy, - error) { + amt lnwire.MilliSatoshi, finalHtlcExpiry int32) ( + []*channeldb.ChannelEdgePolicy, error) { // We expect find path to receive a cltv limit excluding the // final cltv delta (including the block padding). diff --git a/routing/router.go b/routing/router.go index 085db61f7..55cefaae9 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1431,27 +1431,29 @@ func (r *ChannelRouter) FindRoute(source, target route.Vertex, return nil, err } + // We'll fetch the current block height so we can properly calculate the + // required HTLC time locks within the route. + _, currentHeight, err := r.cfg.Chain.GetBestBlock() + if err != nil { + return nil, err + } + // Now that we know the destination is reachable within the graph, we'll // execute our path finding algorithm. + finalHtlcExpiry := currentHeight + int32(finalCLTVDelta) + path, err := findPath( &graphParams{ graph: r.cfg.Graph, bandwidthHints: bandwidthHints, }, restrictions, &r.cfg.PathFindingConfig, - source, target, amt, + source, target, amt, finalHtlcExpiry, ) if err != nil { return nil, err } - // We'll fetch the current block height so we can properly calculate the - // required HTLC time locks within the route. - _, currentHeight, err := r.cfg.Chain.GetBestBlock() - if err != nil { - return nil, err - } - // Create the route with absolute time lock values. route, err := newRoute( source, path, uint32(currentHeight), diff --git a/routing/router_test.go b/routing/router_test.go index 26803492c..6fd9a6303 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -2174,7 +2174,7 @@ func TestFindPathFeeWeighting(t *testing.T) { }, noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, amt, + sourceNode.PubKeyBytes, target, amt, 0, ) if err != nil { t.Fatalf("unable to find path: %v", err) From e8fd05e8e3f57d3afbf1031031dcc622a41759a1 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 16 Dec 2019 21:34:29 +0100 Subject: [PATCH 5/8] tlv+record+routing: add payload size calculation --- record/mpp.go | 5 +++ routing/route/route.go | 47 +++++++++++++++++++++++++++ routing/route/route_test.go | 63 ++++++++++++++++++++++++++++++++++++- tlv/varint.go | 7 +++++ 4 files changed, 121 insertions(+), 1 deletion(-) diff --git a/record/mpp.go b/record/mpp.go index 38877caf7..6e260d545 100644 --- a/record/mpp.go +++ b/record/mpp.go @@ -98,6 +98,11 @@ func (r *MPP) Record() tlv.Record { ) } +// PayloadSize returns the size this record takes up in encoded form. +func (r *MPP) PayloadSize() uint64 { + return 32 + tlv.SizeTUint64(uint64(r.totalMsat)) +} + // String returns a human-readable representation of the mpp payload field. func (r *MPP) String() string { return fmt.Sprintf("total=%v, addr=%x", r.totalMsat, r.paymentAddr) diff --git a/routing/route/route.go b/routing/route/route.go index ff043ef89..90a407e38 100644 --- a/routing/route/route.go +++ b/routing/route/route.go @@ -184,6 +184,53 @@ func (h *Hop) PackHopPayload(w io.Writer, nextChanID uint64) error { return tlvStream.Encode(w) } +// Size returns the total size this hop's payload would take up in the onion +// packet. +func (h *Hop) PayloadSize(nextChanID uint64) uint64 { + if h.LegacyPayload { + return sphinx.LegacyHopDataSize + } + + var payloadSize uint64 + + addRecord := func(tlvType tlv.Type, length uint64) { + payloadSize += tlv.VarIntSize(uint64(tlvType)) + + tlv.VarIntSize(length) + length + } + + // Add amount size. + addRecord(record.AmtOnionType, tlv.SizeTUint64(uint64(h.AmtToForward))) + + // Add lock time size. + addRecord( + record.LockTimeOnionType, + tlv.SizeTUint64(uint64(h.OutgoingTimeLock)), + ) + + // Add next hop if present. + if nextChanID != 0 { + addRecord(record.NextHopOnionType, 8) + } + + // Add mpp if present. + if h.MPP != nil { + addRecord(record.MPPOnionType, h.MPP.PayloadSize()) + } + + // Add custom records. + for k, v := range h.CustomRecords { + addRecord(tlv.Type(k), uint64(len(v))) + } + + // Add the size required to encode the payload length. + payloadSize += tlv.VarIntSize(payloadSize) + + // Add HMAC. + payloadSize += sphinx.HMACSize + + return payloadSize +} + // Route represents a path through the channel graph which runs over one or // more channels in succession. This struct carries all the information // required to craft the Sphinx onion packet, and send the payment along the diff --git a/routing/route/route_test.go b/routing/route/route_test.go index 2894cbdc0..6c32d8be3 100644 --- a/routing/route/route_test.go +++ b/routing/route/route_test.go @@ -2,12 +2,20 @@ package route import ( "bytes" + "encoding/hex" "testing" + "github.com/btcsuite/btcd/btcec" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/record" ) +var ( + testPrivKeyBytes, _ = hex.DecodeString("e126f68f7eafcc8b74f54d269fe206be715000f94dac067d1c04a8ca3b2db734") + _, testPubKey = btcec.PrivKeyFromBytes(btcec.S256(), testPrivKeyBytes) + testPubKeyBytes, _ = NewVertexFromBytes(testPubKey.SerializeCompressed()) +) + // TestRouteTotalFees checks that a route reports the expected total fee. func TestRouteTotalFees(t *testing.T) { t.Parallel() @@ -56,7 +64,6 @@ func TestRouteTotalFees(t *testing.T) { if r.TotalFees() != fee { t.Fatalf("expected %v fees, got %v", fee, r.TotalFees()) } - } var ( @@ -93,3 +100,57 @@ func TestMPPHop(t *testing.T) { t.Fatalf("expected err: %v, got: %v", nil, err) } } + +// TestPayloadSize tests the payload size calculation that is provided by Hop +// structs. +func TestPayloadSize(t *testing.T) { + hops := []*Hop{ + { + PubKeyBytes: testPubKeyBytes, + AmtToForward: 1000, + OutgoingTimeLock: 600000, + ChannelID: 3432483437438, + LegacyPayload: true, + }, + { + PubKeyBytes: testPubKeyBytes, + AmtToForward: 1200, + OutgoingTimeLock: 700000, + ChannelID: 63584534844, + }, + { + PubKeyBytes: testPubKeyBytes, + AmtToForward: 1200, + OutgoingTimeLock: 700000, + MPP: record.NewMPP(500, [32]byte{}), + CustomRecords: map[uint64][]byte{ + 100000: {1, 2, 3}, + 1000000: {4, 5}, + }, + }, + } + + rt := Route{ + Hops: hops, + } + path, err := rt.ToSphinxPath() + if err != nil { + t.Fatal(err) + } + + for i, onionHop := range path[:path.TrueRouteLength()] { + hop := hops[i] + var nextChan uint64 + if i < len(hops)-1 { + nextChan = hops[i+1].ChannelID + } + + expected := uint64(onionHop.HopPayload.NumBytes()) + actual := hop.PayloadSize(nextChan) + if expected != actual { + t.Fatalf("unexpected payload size at hop %v: "+ + "expected %v, got %v", + i, expected, actual) + } + } +} diff --git a/tlv/varint.go b/tlv/varint.go index 3888bfcb4..38c7a7cd6 100644 --- a/tlv/varint.go +++ b/tlv/varint.go @@ -4,6 +4,8 @@ import ( "encoding/binary" "errors" "io" + + "github.com/btcsuite/btcd/wire" ) // ErrVarIntNotCanonical signals that the decoded varint was not minimally encoded. @@ -107,3 +109,8 @@ func WriteVarInt(w io.Writer, val uint64, buf *[8]byte) error { _, err := w.Write(buf[:length]) return err } + +// VarIntSize returns the required number of bytes to encode a var int. +func VarIntSize(val uint64) uint64 { + return uint64(wire.VarIntSerializeSize(val)) +} From 513341516eebc1290dc109d57e0aaf2ca1fbd2a5 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 18 Dec 2019 10:28:21 +0100 Subject: [PATCH 6/8] routing: fix max route length test Enable the test again and use a programmatically built network. --- routing/pathfind_test.go | 73 ++--- routing/testdata/excessive_hops.json | 410 --------------------------- 2 files changed, 31 insertions(+), 452 deletions(-) delete mode 100644 routing/testdata/excessive_hops.json diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index c47f73486..bddb5fbfd 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -1360,53 +1360,42 @@ func TestNewRoute(t *testing.T) { } func TestNewRoutePathTooLong(t *testing.T) { - t.Skip() + t.Parallel() - // Ensure that potential paths which are over the maximum hop-limit are - // rejected. - graph, err := parseTestGraph(excessiveHopsGraphFilePath) + var testChannels []*testChannel + + // Setup a linear network of 21 hops. + fromNode := "start" + for i := 0; i < 21; i++ { + toNode := fmt.Sprintf("node-%v", i+1) + c := symmetricTestChannel(fromNode, toNode, 100000, &testChannelPolicy{ + Expiry: 144, + FeeRate: 400, + MinHTLC: 1, + MaxHTLC: 100000001, + }) + testChannels = append(testChannels, c) + + fromNode = toNode + } + + ctx := newPathFindingTestContext(t, testChannels, "start") + defer ctx.cleanup() + + // Assert that we can find 20 hop routes. + node20 := ctx.keyFromAlias("node-20") + payAmt := lnwire.MilliSatoshi(100001) + _, err := ctx.findPath(node20, payAmt) if err != nil { - t.Fatalf("unable to create graph: %v", err) - } - defer graph.cleanUp() - - sourceNode, err := graph.graph.SourceNode() - if err != nil { - t.Fatalf("unable to fetch source node: %v", err) + t.Fatalf("unexpected pathfinding failure: %v", err) } - paymentAmt := lnwire.NewMSatFromSatoshis(100) - - // We start by confirming that routing a payment 20 hops away is - // possible. Alice should be able to find a valid route to ursula. - target := graph.aliasMap["ursula"] - _, err = findPath( - &graphParams{ - graph: graph.graph, - }, - noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, paymentAmt, 0, - ) - if err != nil { - t.Fatalf("path should have been found") + // Assert that finding a 21 hop route fails. + node21 := ctx.keyFromAlias("node-21") + _, err = ctx.findPath(node21, payAmt) + if err != errMaxHopsExceeded { + t.Fatalf("expected route too long, but got %v", err) } - - // Vincent is 21 hops away from Alice, and thus no valid route should be - // presented to Alice. - target = graph.aliasMap["vincent"] - path, err := findPath( - &graphParams{ - graph: graph.graph, - }, - noRestrictions, testPathFindingConfig, - sourceNode.PubKeyBytes, target, paymentAmt, 0, - ) - if err == nil { - t.Fatalf("should not have been able to find path, supposed to be "+ - "greater than 20 hops, found route with %v hops", - len(path)) - } - } func TestPathNotAvailable(t *testing.T) { diff --git a/routing/testdata/excessive_hops.json b/routing/testdata/excessive_hops.json deleted file mode 100644 index 3094f1b47..000000000 --- a/routing/testdata/excessive_hops.json +++ /dev/null @@ -1,410 +0,0 @@ -{ - "nodes": [ - { - "source": true, - "pubkey": "021b96642e723592ee0b095983fe3a26c8b40b8926968d8b7510e51c9429d4562c", - "alias": "alice" - }, - { - "source": false, - "pubkey": "022096b2b0ac083e708074a5ab57288bc821b6bef7b964185b307e073772c3748f", - "alias": "bob" - }, - { - "source": false, - "pubkey": "022a190ce901ab2b6f349483f18b28a1d72c64a7bccb8057291f25784c0899840f", - "alias": "carol" - }, - { - "source": false, - "pubkey": "022d855d09971dd047b7ecf929b23c6f147b568d4668af67fb2226eb8c15c4660d", - "alias": "dave" - }, - { - "source": false, - "pubkey": "024ca436834b0d38d9dc7ee4d95aa21db321c45598dc5921a4a52304a8e0dd2952", - "alias": "eve" - }, - { - "source": false, - "pubkey": "025234a0c44cbf1b20c18e2c397107ad731376831e1c43ddb360b41dbb98c10266", - "alias": "fez" - }, - { - "source": false, - "pubkey": "0253e9d03030f2ff08d3a7f1d824ad6d8c0dae422f324e72d5bb313e3f2a2d45a8", - "alias": "gabby" - }, - { - "source": false, - "pubkey": "0263d4f2baca258ff3bd5bce86c2754e95daaea27f268ae1a048c1253ff20de56e", - "alias": "harold" - }, - { - "source": false, - "pubkey": "02650db8e44302f75e265e9427264bc0d7e2337831d6b9ceb7c58ed1e725d4576a", - "alias": "inez" - }, - { - "source": false, - "pubkey": "02727bfd298aa055a6419404931dfc1ccb4f0eb7c9660a7df346b93d0025df3ba1", - "alias": "jake" - }, - { - "source": false, - "pubkey": "0280c83b3eded413dcec12f7952410e2738f079bd9cbc9a7c462e32ed4d74bd5b7", - "alias": "karen" - }, - { - "source": false, - "pubkey": "0290bf454f4b95baf9227801301b331e35d477c6b6e7f36a599983ae58747b3828", - "alias": "liam" - }, - { - "source": false, - "pubkey": "0297c8de635d17e3dd5775edfa2797be0874c53b0026f69009787cecd2fa577de8", - "alias": "maggie" - }, - { - "source": false, - "pubkey": "02a27227113c71eab0c8609ac0cdc7e76791fc3163c16e643cb4658d1080c7e336", - "alias": "nick" - }, - { - "source": false, - "pubkey": "02f5f6bb6373fc60528118003f803557b916fbecd90c3a0c5df4c86c6a6e962fd1", - "alias": "ophelia" - }, - { - "source": false, - "pubkey": "02fd7a5f04d550cf0ba8af6053a20e0080d956f41b1221357a35fab3a363e5f78e", - "alias": "patrick" - }, - { - "source": false, - "pubkey": "030da942ed7cfc7d3096811b3264e15115778e692eaacb2b7a76fb27a58cbb5359", - "alias": "quinn" - }, - { - "source": false, - "pubkey": "0319d6b038e26ac89802e856d7e78f293e9d109c414614f98e3fa5c626f20934be", - "alias": "rick" - }, - { - "source": false, - "pubkey": "03384439e78e87d168fecabe8d88218dfd5983c5e14fd8fa6dc89caeb3cc0fb171", - "alias": "sarah" - }, - { - "source": false, - "pubkey": "0362002b8fbc1a799c839c8bcea43fce38a147467a00bc450414bbeab5c7a19efe", - "alias": "tim" - }, - { - "source": false, - "pubkey": "0369bca64993fce966745d32c09b882f668958d9bd7aabb60ba35ef1884013be1d", - "alias": "ursula" - }, - { - "source": false, - "pubkey": "0367cec75158a4129177bfb8b269cb586efe93d751b43800d456485e81c2620ca6", - "alias": "vincent" - } - ], - "edges": [ - { - "node_1": "021b96642e723592ee0b095983fe3a26c8b40b8926968d8b7510e51c9429d4562c", - "node_2": "022096b2b0ac083e708074a5ab57288bc821b6bef7b964185b307e073772c3748f", - "channel_id": 12345, - "channel_point": "99dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "022096b2b0ac083e708074a5ab57288bc821b6bef7b964185b307e073772c3748f", - "node_2": "022a190ce901ab2b6f349483f18b28a1d72c64a7bccb8057291f25784c0899840f", - "channel_id": 12346, - "channel_point": "79dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "022a190ce901ab2b6f349483f18b28a1d72c64a7bccb8057291f25784c0899840f", - "node_2": "022d855d09971dd047b7ecf929b23c6f147b568d4668af67fb2226eb8c15c4660d", - "channel_id": 12347, - "channel_point": "69dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "022d855d09971dd047b7ecf929b23c6f147b568d4668af67fb2226eb8c15c4660d", - "node_2": "024ca436834b0d38d9dc7ee4d95aa21db321c45598dc5921a4a52304a8e0dd2952", - "channel_id": 12348, - "channel_point": "59dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "024ca436834b0d38d9dc7ee4d95aa21db321c45598dc5921a4a52304a8e0dd2952", - "node_2": "025234a0c44cbf1b20c18e2c397107ad731376831e1c43ddb360b41dbb98c10266", - "channel_id": 12349, - "channel_point": "49dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "025234a0c44cbf1b20c18e2c397107ad731376831e1c43ddb360b41dbb98c10266", - "node_2": "0253e9d03030f2ff08d3a7f1d824ad6d8c0dae422f324e72d5bb313e3f2a2d45a8", - "channel_id": 12340, - "channel_point": "39dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "0253e9d03030f2ff08d3a7f1d824ad6d8c0dae422f324e72d5bb313e3f2a2d45a8", - "node_2": "0263d4f2baca258ff3bd5bce86c2754e95daaea27f268ae1a048c1253ff20de56e", - "channel_id": 12344, - "channel_point": "29dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "0263d4f2baca258ff3bd5bce86c2754e95daaea27f268ae1a048c1253ff20de56e", - "node_2": "02650db8e44302f75e265e9427264bc0d7e2337831d6b9ceb7c58ed1e725d4576a", - "channel_id": 12343, - "channel_point": "19dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "02650db8e44302f75e265e9427264bc0d7e2337831d6b9ceb7c58ed1e725d4576a", - "node_2": "02727bfd298aa055a6419404931dfc1ccb4f0eb7c9660a7df346b93d0025df3ba1", - "channel_id": 12342, - "channel_point": "88dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "02727bfd298aa055a6419404931dfc1ccb4f0eb7c9660a7df346b93d0025df3ba1", - "node_2": "0280c83b3eded413dcec12f7952410e2738f079bd9cbc9a7c462e32ed4d74bd5b7", - "channel_id": 12341, - "channel_point": "87dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "0280c83b3eded413dcec12f7952410e2738f079bd9cbc9a7c462e32ed4d74bd5b7", - "node_2": "0290bf454f4b95baf9227801301b331e35d477c6b6e7f36a599983ae58747b3828", - "channel_id": 12355, - "channel_point": "86dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "0290bf454f4b95baf9227801301b331e35d477c6b6e7f36a599983ae58747b3828", - "node_2": "0297c8de635d17e3dd5775edfa2797be0874c53b0026f69009787cecd2fa577de8", - "channel_id": 12365, - "channel_point": "85dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "0297c8de635d17e3dd5775edfa2797be0874c53b0026f69009787cecd2fa577de8", - "node_2": "02a27227113c71eab0c8609ac0cdc7e76791fc3163c16e643cb4658d1080c7e336", - "channel_id": 12375, - "channel_point": "84dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "02a27227113c71eab0c8609ac0cdc7e76791fc3163c16e643cb4658d1080c7e336", - "node_2": "02f5f6bb6373fc60528118003f803557b916fbecd90c3a0c5df4c86c6a6e962fd1", - "channel_id": 12385, - "channel_point": "83dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "02f5f6bb6373fc60528118003f803557b916fbecd90c3a0c5df4c86c6a6e962fd1", - "node_2": "02fd7a5f04d550cf0ba8af6053a20e0080d956f41b1221357a35fab3a363e5f78e", - "channel_id": 12395, - "channel_point": "82dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "02fd7a5f04d550cf0ba8af6053a20e0080d956f41b1221357a35fab3a363e5f78e", - "node_2": "030da942ed7cfc7d3096811b3264e15115778e692eaacb2b7a76fb27a58cbb5359", - "channel_id": 12305, - "channel_point": "81dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "030da942ed7cfc7d3096811b3264e15115778e692eaacb2b7a76fb27a58cbb5359", - "node_2": "0319d6b038e26ac89802e856d7e78f293e9d109c414614f98e3fa5c626f20934be", - "channel_id": 12335, - "channel_point": "80dc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "0319d6b038e26ac89802e856d7e78f293e9d109c414614f98e3fa5c626f20934be", - "node_2": "03384439e78e87d168fecabe8d88218dfd5983c5e14fd8fa6dc89caeb3cc0fb171", - "channel_id": 12325, - "channel_point": "89ec56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "03384439e78e87d168fecabe8d88218dfd5983c5e14fd8fa6dc89caeb3cc0fb171", - "node_2": "0362002b8fbc1a799c839c8bcea43fce38a147467a00bc450414bbeab5c7a19efe", - "channel_id": 12315, - "channel_point": "89fc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "0362002b8fbc1a799c839c8bcea43fce38a147467a00bc450414bbeab5c7a19efe", - "node_2": "0369bca64993fce966745d32c09b882f668958d9bd7aabb60ba35ef1884013be1d", - "channel_id": 12445, - "channel_point": "89cc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - }, - { - "node_1": "0369bca64993fce966745d32c09b882f668958d9bd7aabb60ba35ef1884013be1d", - "node_2": "0367cec75158a4129177bfb8b269cb586efe93d751b43800d456485e81c2620ca6", - "channel_id": 12545, - "channel_point": "89bc56859c6a082d15ba1a7f6cb6be3fea62e1746e2cb8497b1189155c21a233:0", - "channel_flags": 0, - "message_flags": 1, - "expiry": 1, - "min_htlc": 1, - "max_htlc": 100000000, - "fee_base_msat": 10, - "fee_rate": 0.001, - "capacity": 100000 - } - ] -} From b760b252290a3e2d0cae0dd1dc6502eeea6e632d Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 16 Dec 2019 14:22:42 +0100 Subject: [PATCH 7/8] routing: limit routing info size during pathfinding Also the max hop count check can be removed, because the real bound is the payload size. By moving the check inside the search loop, we now also backtrack when we hit the limit. --- routing/heap.go | 4 ++ routing/pathfind.go | 72 +++++++++++++++++++++++++----------- routing/pathfind_test.go | 15 +++++++- routing/payment_lifecycle.go | 1 - 4 files changed, 68 insertions(+), 24 deletions(-) diff --git a/routing/heap.go b/routing/heap.go index e7e1b5878..f6869663c 100644 --- a/routing/heap.go +++ b/routing/heap.go @@ -40,6 +40,10 @@ type nodeWithDist struct { // nextHop is the edge this route comes from. nextHop *channeldb.ChannelEdgePolicy + + // routingInfoSize is the total size requirement for the payloads field + // in the onion packet from this hop towards the final destination. + routingInfoSize uint64 } // distanceHeap is a min-distance heap that's used within our path finding diff --git a/routing/pathfind.go b/routing/pathfind.go index f74d00d95..1c1eea9fb 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -8,6 +8,7 @@ import ( "time" "github.com/coreos/bbolt" + sphinx "github.com/lightningnetwork/lightning-onion" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/feature" "github.com/lightningnetwork/lnd/lnwire" @@ -16,12 +17,6 @@ import ( ) const ( - // HopLimit is the maximum number hops that is permissible as a route. - // Any potential paths found that lie above this limit will be rejected - // with an error. This value is computed using the current fixed-size - // packet length of the Sphinx construction. - HopLimit = 20 - // infinity is used as a starting distance in our shortest path search. infinity = math.MaxInt64 @@ -79,10 +74,6 @@ var ( // not exist in the graph. errNoPathFound = errors.New("unable to find a path to destination") - // errMaxHopsExceeded is returned when a candidate path is found, but - // the length of that path exceeds HopLimit. - errMaxHopsExceeded = errors.New("potential path has too many hops") - // errInsufficientLocalBalance is returned when none of the local // channels have enough balance for the payment. errInsufficientBalance = errors.New("insufficient local balance") @@ -529,6 +520,23 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, } } + // Build a preliminary destination hop structure to obtain the payload + // size. + var mpp *record.MPP + if r.PaymentAddr != nil { + mpp = record.NewMPP(amt, *r.PaymentAddr) + } + + finalHop := route.Hop{ + AmtToForward: amt, + OutgoingTimeLock: uint32(finalHtlcExpiry), + CustomRecords: r.DestCustomRecords, + LegacyPayload: !features.HasFeature( + lnwire.TLVOnionPayloadOptional, + ), + MPP: mpp, + } + // We can't always assume that the end destination is publicly // advertised to the network so we'll manually include the target node. // The target node charges no fee. Distance is set to 0, because this is @@ -545,6 +553,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, amountToReceive: amt, incomingCltv: finalHtlcExpiry, probability: 1, + routingInfoSize: finalHop.PayloadSize(0), } // Calculate the absolute cltv limit. Use uint64 to prevent an overflow @@ -554,6 +563,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // processEdge is a helper closure that will be used to make sure edges // satisfy our specific requirements. processEdge := func(fromVertex route.Vertex, + fromFeatures *lnwire.FeatureVector, edge *channeldb.ChannelEdgePolicy, toNodeDist *nodeWithDist) { edgesExpanded++ @@ -674,6 +684,34 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, edge.ChannelID) } + // Calculate the total routing info size if this hop were to be + // included. If we are coming from the source hop, the payload + // size is zero, because the original htlc isn't in the onion + // blob. + var payloadSize uint64 + if fromVertex != source { + supportsTlv := fromFeatures.HasFeature( + lnwire.TLVOnionPayloadOptional, + ) + + hop := route.Hop{ + AmtToForward: amountToSend, + OutgoingTimeLock: uint32( + toNodeDist.incomingCltv, + ), + LegacyPayload: !supportsTlv, + } + + payloadSize = hop.PayloadSize(edge.ChannelID) + } + + routingInfoSize := toNodeDist.routingInfoSize + payloadSize + + // Skip paths that would exceed the maximum routing info size. + if routingInfoSize > sphinx.MaxPayloadSize { + return + } + // All conditions are met and this new tentative distance is // better than the current best known distance to this node. // The new better distance is recorded, and also our "next hop" @@ -686,6 +724,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, incomingCltv: incomingCltv, probability: probability, nextHop: edge, + routingInfoSize: routingInfoSize, } distance[fromVertex] = withDist @@ -796,7 +835,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // Check if this candidate node is better than what we // already have. - processEdge(fromNode, policy, partialPath) + processEdge(fromNode, fromFeatures, policy, partialPath) } if nodeHeap.Len() == 0 { @@ -854,17 +893,8 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // findPath, and avoid using ChannelEdgePolicy altogether. pathEdges[len(pathEdges)-1].Node.Features = features - // The route is invalid if it spans more than 20 hops. The current - // Sphinx (onion routing) implementation can only encode up to 20 hops - // as the entire packet is fixed size. If this route is more than 20 - // hops, then it's invalid. - numEdges := len(pathEdges) - if numEdges > HopLimit { - return nil, errMaxHopsExceeded - } - log.Debugf("Found route: probability=%v, hops=%v, fee=%v\n", - distance[source].probability, numEdges, + distance[source].probability, len(pathEdges), distance[source].amountToReceive-amt) return pathEdges, nil diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index bddb5fbfd..910d13db4 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -1393,8 +1393,19 @@ func TestNewRoutePathTooLong(t *testing.T) { // Assert that finding a 21 hop route fails. node21 := ctx.keyFromAlias("node-21") _, err = ctx.findPath(node21, payAmt) - if err != errMaxHopsExceeded { - t.Fatalf("expected route too long, but got %v", err) + if err != errNoPathFound { + t.Fatalf("not route error expected, but got %v", err) + } + + // Assert that we can't find a 20 hop route if custom records make it + // exceed the maximum payload size. + ctx.restrictParams.DestFeatures = tlvFeatures + ctx.restrictParams.DestCustomRecords = map[uint64][]byte{ + 100000: bytes.Repeat([]byte{1}, 100), + } + _, err = ctx.findPath(node20, payAmt) + if err != errNoPathFound { + t.Fatalf("not route error expected, but got %v", err) } } diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index b5203671c..2283ca057 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -196,7 +196,6 @@ func errorToPaymentFailure(err error) channeldb.FailureReason { errNoTlvPayload, errNoPaymentAddr, errNoPathFound, - errMaxHopsExceeded, errPrebuiltRouteTried: return channeldb.FailureReasonNoRoute From c84e57a5228897dd62c7adcd05024be90ec4c65a Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Fri, 20 Dec 2019 10:28:48 +0100 Subject: [PATCH 8/8] routing: remove unnecessary newline --- routing/pathfind.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routing/pathfind.go b/routing/pathfind.go index 1c1eea9fb..a6bd58785 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -893,7 +893,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // findPath, and avoid using ChannelEdgePolicy altogether. pathEdges[len(pathEdges)-1].Node.Features = features - log.Debugf("Found route: probability=%v, hops=%v, fee=%v\n", + log.Debugf("Found route: probability=%v, hops=%v, fee=%v", distance[source].probability, len(pathEdges), distance[source].amountToReceive-amt)