From f935bd0baf6925b2e37d7f2cd8694043de7df90f Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 22 Mar 2018 17:34:42 -0700 Subject: [PATCH 1/6] routing: add vertex pruning for non-final CLTV related errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In this commit, we add vertex pruning for any non-final CLTV error. Before this commit, we assumed that any source of this error was due to the local node setting the incorrect time lock. However, it’s been recently noticed on main net that there’re a set of nodes that seem to not be properly scanned to the chain. Without this patch, users aren’t able to route successfully as atm, we’ll stop all path finding attempts if we encounter this. --- routing/router.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/routing/router.go b/routing/router.go index 1947f9e5..be92e668 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1670,15 +1670,20 @@ func (r *ChannelRouter) SendPayment(payment *LightningPayment) ([32]byte, *Route return preImage, nil, sendError // If we get a notice that the expiry was too soon for - // an intermediate node, then we'll exit early as the - // expected block height as shifted from underneath us. + // an intermediate node, then we'll prune out the node + // that sent us this error, as it doesn't now what the + // correct block height is. case *lnwire.FailExpiryTooSoon: update := onionErr.Update if err := r.applyChannelUpdate(&update); err != nil { log.Errorf("unable to apply channel "+ "update for onion error: %v", err) } - return preImage, nil, sendError + + pruneVertexFailure( + paySession, route, errSource, false, + ) + continue // If we hit an instance of onion payload corruption or // an invalid version, then we'll exit early as this @@ -1734,6 +1739,10 @@ func (r *ChannelRouter) SendPayment(payment *LightningPayment) ([32]byte, *Route errFailedFeeChans[chanID] = struct{}{} continue + // If we get the failure for an intermediate node that + // disagrees with our time lock values, then we'll + // prune it out for now, and continue with path + // finding. case *lnwire.FailIncorrectCltvExpiry: update := onionErr.Update if err := r.applyChannelUpdate(&update); err != nil { @@ -1741,7 +1750,10 @@ func (r *ChannelRouter) SendPayment(payment *LightningPayment) ([32]byte, *Route "update for onion error: %v", err) } - return preImage, nil, sendError + pruneVertexFailure( + paySession, route, errSource, false, + ) + continue // The outgoing channel that this node was meant to // forward one is currently disabled, so we'll apply From 1365392d2ba7ce21ab5bb6e755058d4d7a1f632a Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 22 Mar 2018 17:35:20 -0700 Subject: [PATCH 2/6] routing: don't traverse links that advertise a timelock delta of zero --- routing/pathfind.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/routing/pathfind.go b/routing/pathfind.go index 5584ad2b..88002aa4 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -543,7 +543,8 @@ func findPath(tx *bolt.Tx, graph *channeldb.ChannelGraph, // amount to our relaxation condition. if tempDist < distance[v].dist && edgeInfo.Capacity >= amt.ToSatoshis() && - amt >= outEdge.MinHTLC { + amt >= outEdge.MinHTLC && + outEdge.TimeLockDelta != 0 { distance[v] = nodeWithDist{ dist: tempDist, From de1150b78ba196c36613b8c78368cbb1213e62a0 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 22 Mar 2018 17:38:49 -0700 Subject: [PATCH 3/6] routing: add new vertex and to new channels to basic_graph.json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In this commit, we add a new node to the current default test graph that we use for our path finding tests. This new node connects roasbeef to sophon via a new route with very high fees. With this new node and the two channels it adds, we can properly test that we’ll route around failures that we run into during payment routing. --- routing/testdata/basic_graph.json | 56 ++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/routing/testdata/basic_graph.json b/routing/testdata/basic_graph.json index c5fa8647..48958796 100644 --- a/routing/testdata/basic_graph.json +++ b/routing/testdata/basic_graph.json @@ -27,7 +27,8 @@ " ▼ │ ", " ┌──────────┐ │ ", " │ roasbeef │◀─────────────────────────────────────┘ ", -" └──────────┘ 100k satoshis " +" └──────────┘ 100k satoshis ", +" the graph also includes a channel from roasbeef to sophon via pham nuwen" ], "nodes": [ { @@ -54,9 +55,62 @@ "source": false, "pubkey": "036264734b40c9e91d3d990a8cdfbbe23b5b0b7ad3cd0e080a25dcd05d39eeb7eb", "alias": "sophon" + }, + { + "source": false, + "pubkey": "02a1d2856be336a58af08989aea0d8c41e072ccc392c46f8ce0e6e069f002035f3", + "alias": "phamnuwen" } ], "edges": [ + { + "node_1": "02a1d2856be336a58af08989aea0d8c41e072ccc392c46f8ce0e6e069f002035f3", + "node_2": "0367cec75158a4129177bfb8b269cb586efe93d751b43800d456485e81c2620ca6", + "channel_id": 999991, + "channel_point": "48a0e8b856fef01d9feda7d25a4fac6dae48749e28ba356b92d712ab7f5bd2d0:0", + "flags": 1, + "expiry": 1, + "min_htlc": 1000, + "fee_base_msat": 10000, + "fee_rate": 1000000, + "capacity": 100000 + }, + { + "node_1": "02a1d2856be336a58af08989aea0d8c41e072ccc392c46f8ce0e6e069f002035f3", + "node_2": "0367cec75158a4129177bfb8b269cb586efe93d751b43800d456485e81c2620ca6", + "channel_id": 999991, + "channel_point": "48a0e8b856fef01d9feda7d25a4fac6dae48749e28ba356b92d712ab7f5bd2d0:0", + "flags": 0, + "expiry": 1, + "min_htlc": 1000, + "fee_base_msat": 10000, + "fee_rate": 1000000, + "capacity": 100000 + }, + { + "node_1": "02a1d2856be336a58af08989aea0d8c41e072ccc392c46f8ce0e6e069f002035f3", + "node_2": "036264734b40c9e91d3d990a8cdfbbe23b5b0b7ad3cd0e080a25dcd05d39eeb7eb", + "channel_id": 99999, + "channel_point": "05ffda8890d0a4fffe0ddca0b1932ba0415b1d5868a99515384a4e7883d96b88:0", + "flags": 1, + "expiry": 1, + "min_htlc": 1000, + "fee_base_msat": 10000, + "fee_rate": 1000000, + "capacity": 100000 + }, + { + "node_1": "02a1d2856be336a58af08989aea0d8c41e072ccc392c46f8ce0e6e069f002035f3", + "node_2": "036264734b40c9e91d3d990a8cdfbbe23b5b0b7ad3cd0e080a25dcd05d39eeb7eb", + "channel_id": 99999, + "channel_point": "05ffda8890d0a4fffe0ddca0b1932ba0415b1d5868a99515384a4e7883d96b88:0", + "flags": 0, + "expiry": 1, + "min_htlc": 1000, + "fee_base_msat": 10000, + "fee_rate": 1000000, + "capacity": 100000 + }, { "node_1": "0367cec75158a4129177bfb8b269cb586efe93d751b43800d456485e81c2620ca6", "node_2": "032b480de5d002f1a8fd1fe1bbf0a0f1b07760f65f052e66d56f15d71097c01add", From d5721165de90268f865964350d9ea042a51caa6a Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 22 Mar 2018 17:39:21 -0700 Subject: [PATCH 4/6] routing: fix TestPathInsufficientCapacity to actually send 1 BTC --- routing/pathfind_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index f4d7db3f..0a4d4eeb 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -654,7 +654,7 @@ func TestPathInsufficientCapacity(t *testing.T) { // though we have a 2-hop link. target := aliases["sophon"] - const payAmt = btcutil.SatoshiPerBitcoin + payAmt := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin) _, err = findPath(nil, graph, sourceNode, target, ignoredVertexes, ignoredEdges, payAmt) if !IsError(err, ErrNoPathFound) { From 62b8ddb839f9d4e6da117eb0ccf253a514b012b5 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 22 Mar 2018 17:39:47 -0700 Subject: [PATCH 5/6] routing: update TestAddEdgeUnknownVertexes due to additional node in test graph --- routing/router_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/routing/router_test.go b/routing/router_test.go index 4d2f6928..b5f2610d 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -818,7 +818,7 @@ func TestAddEdgeUnknownVertexes(t *testing.T) { t.Fatalf("unable to update edge policy: %v", err) } - // We should now be able to find one route to node 2. + // We should now be able to find two routes to node 2. paymentAmt := lnwire.NewMSatFromSatoshis(100) targetNode := priv2.PubKey() routes, err := ctx.router.FindRoutes(targetNode, paymentAmt, @@ -826,8 +826,8 @@ func TestAddEdgeUnknownVertexes(t *testing.T) { if err != nil { t.Fatalf("unable to find any routes: %v", err) } - if len(routes) != 1 { - t.Fatalf("expected to find 1 route, found: %v", len(routes)) + if len(routes) != 2 { + t.Fatalf("expected to find 2 route, found: %v", len(routes)) } // Now check that we can update the node info for the partial node @@ -862,15 +862,15 @@ func TestAddEdgeUnknownVertexes(t *testing.T) { t.Fatalf("could not add node: %v", err) } - // Should still be able to find the route, and the info should be + // Should still be able to find the routes, and the info should be // updated. routes, err = ctx.router.FindRoutes(targetNode, paymentAmt, defaultNumRoutes, DefaultFinalCLTVDelta) if err != nil { t.Fatalf("unable to find any routes: %v", err) } - if len(routes) != 1 { - t.Fatalf("expected to find 1 route, found: %v", len(routes)) + if len(routes) != 2 { + t.Fatalf("expected to find 2 route, found: %v", len(routes)) } copy1, err := ctx.graph.FetchLightningNode(priv1.PubKey()) From 1a05b48080ac483718b4d285df977d9db73bf568 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Thu, 22 Mar 2018 17:40:07 -0700 Subject: [PATCH 6/6] routing: add new test case to ensure we route around time lock failures --- routing/router_test.go | 131 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) diff --git a/routing/router_test.go b/routing/router_test.go index b5f2610d..2471654b 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -379,6 +379,137 @@ func TestSendPaymentErrorRepeatedFeeInsufficient(t *testing.T) { } } +// TestSendPaymentErrorNonFinalTimeLockErrors tests that if we receive either +// an ExpiryTooSoon or a IncorrectCltvExpiry error from a node, then we prune +// that node from the available graph witin a mission control session. This +// test ensures that we'll route around errors due to nodes not knowing the +// current block height. +func TestSendPaymentErrorNonFinalTimeLockErrors(t *testing.T) { + t.Parallel() + + const startingBlockHeight = 101 + ctx, cleanUp, err := createTestCtx(startingBlockHeight, basicGraphFilePath) + defer cleanUp() + if err != nil { + t.Fatalf("unable to create router: %v", err) + } + + // Craft a LightningPayment struct that'll send a payment from roasbeef + // to sophon for 1k satoshis. + var payHash [32]byte + payment := LightningPayment{ + Target: ctx.aliases["sophon"], + Amount: lnwire.NewMSatFromSatoshis(1000), + PaymentHash: payHash, + } + + var preImage [32]byte + copy(preImage[:], bytes.Repeat([]byte{9}, 32)) + + // We'll also fetch the first outgoing channel edge from roasbeef to + // son goku. This edge will be included in the time lock related expiry + // errors that we'll get back due to disagrements in what the current + // block height is. + chanID := uint64(3495345) + _, _, edgeUpateToFail, err := ctx.graph.FetchChannelEdgesByID(chanID) + if err != nil { + t.Fatalf("unable to fetch chan id: %v", err) + } + + errChanUpdate := lnwire.ChannelUpdate{ + ShortChannelID: lnwire.NewShortChanIDFromInt(chanID), + Timestamp: uint32(edgeUpateToFail.LastUpdate.Unix()), + Flags: edgeUpateToFail.Flags, + TimeLockDelta: edgeUpateToFail.TimeLockDelta, + HtlcMinimumMsat: edgeUpateToFail.MinHTLC, + BaseFee: uint32(edgeUpateToFail.FeeBaseMSat), + FeeRate: uint32(edgeUpateToFail.FeeProportionalMillionths), + } + + // The error will be returned by Son Goku. + sourceNode := ctx.aliases["songoku"] + + // We'll now modify the SendToSwitch method to return an error for the + // outgoing channel to son goku. Since this is a time lock related + // error, we should fail the payment flow all together, as Goku is the + // only channel to Sophon. + ctx.router.cfg.SendToSwitch = func(n [33]byte, + _ *lnwire.UpdateAddHTLC, _ *sphinx.Circuit) ([32]byte, error) { + + if bytes.Equal(sourceNode.SerializeCompressed(), n[:]) { + return [32]byte{}, &htlcswitch.ForwardingError{ + ErrorSource: sourceNode, + FailureMessage: &lnwire.FailExpiryTooSoon{ + Update: errChanUpdate, + }, + } + } + + return preImage, nil + } + + // assertExpectedPath is a helper function that asserts the returned + // route properly routes around the failure we've introduced in the + // graph. + assertExpectedPath := func(retPreImage [32]byte, route *Route) { + // The route selected should have two hops + if len(route.Hops) != 2 { + t.Fatalf("incorrect route length: expected %v got %v", 2, + len(route.Hops)) + } + + // The preimage should match up with the once created above. + if !bytes.Equal(retPreImage[:], preImage[:]) { + t.Fatalf("incorrect preimage used: expected %x got %x", + preImage[:], retPreImage[:]) + } + + // The route should have satoshi as the first hop. + if route.Hops[0].Channel.Node.Alias != "phamnuwen" { + t.Fatalf("route should go through phamnuwen as first hop, "+ + "instead passes through: %v", + route.Hops[0].Channel.Node.Alias) + } + } + + // Send off the payment request to the router, this payment should + // suceed as we should actually go through Pham Nuwen in order to get + // to Sophon, even though he has higher fees. + paymentPreImage, route, err := ctx.router.SendPayment(&payment) + if err != nil { + t.Fatalf("unable to send payment: %v", err) + } + + assertExpectedPath(paymentPreImage, route) + + // We'll now modify the error return an IncorrectCltvExpiry error + // instead, this should result in the same behavior of roasbeef routing + // around the faulty Son Goku node. + ctx.router.cfg.SendToSwitch = func(n [33]byte, + _ *lnwire.UpdateAddHTLC, _ *sphinx.Circuit) ([32]byte, error) { + + if bytes.Equal(sourceNode.SerializeCompressed(), n[:]) { + return [32]byte{}, &htlcswitch.ForwardingError{ + ErrorSource: sourceNode, + FailureMessage: &lnwire.FailIncorrectCltvExpiry{ + Update: errChanUpdate, + }, + } + } + + return preImage, nil + } + + // Once again, Roasbeef should route around Goku since they disagree + // w.r.t to the block height, and instead go through Pham Nuwen. + paymentPreImage, route, err = ctx.router.SendPayment(&payment) + if err != nil { + t.Fatalf("unable to send payment: %v", err) + } + + assertExpectedPath(paymentPreImage, route) +} + // TestSendPaymentErrorPathPruning tests that the send of candidate routes // properly gets pruned in response to ForwardingError response from the // underlying SendToSwitch function.