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, 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) { 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 diff --git a/routing/router_test.go b/routing/router_test.go index 4d2f6928..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. @@ -818,7 +949,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 +957,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 +993,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()) 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",