diff --git a/go.mod b/go.mod index e94a27ecd..947870913 100644 --- a/go.mod +++ b/go.mod @@ -34,7 +34,7 @@ require ( github.com/juju/version v0.0.0-20180108022336-b64dbd566305 // indirect github.com/kkdai/bstream v0.0.0-20181106074824-b3251f7901ec github.com/lightninglabs/neutrino v0.11.0 - github.com/lightningnetwork/lightning-onion v0.0.0-20190909101754-850081b08b6a + github.com/lightningnetwork/lightning-onion v0.0.0-20191214001659-f34e9dc1651d github.com/lightningnetwork/lnd/cert v1.0.0 github.com/lightningnetwork/lnd/queue v1.0.2 github.com/lightningnetwork/lnd/ticker v1.0.0 diff --git a/go.sum b/go.sum index 26dd211ae..c34fd1a1b 100644 --- a/go.sum +++ b/go.sum @@ -132,8 +132,8 @@ github.com/lightninglabs/gozmq v0.0.0-20191113021534-d20a764486bf h1:HZKvJUHlcXI github.com/lightninglabs/gozmq v0.0.0-20191113021534-d20a764486bf/go.mod h1:vxmQPeIQxPf6Jf9rM8R+B4rKBqLA2AjttNxkFBL2Plk= github.com/lightninglabs/neutrino v0.11.0 h1:lPpYFCtsfJX2W5zI4pWycPmbbBdr7zU+BafYdLoD6k0= github.com/lightninglabs/neutrino v0.11.0/go.mod h1:CuhF0iuzg9Sp2HO6ZgXgayviFTn1QHdSTJlMncK80wg= -github.com/lightningnetwork/lightning-onion v0.0.0-20190909101754-850081b08b6a h1:GoWPN4i4jTKRxhVNh9a2vvBBO1Y2seiJB+SopUYoKyo= -github.com/lightningnetwork/lightning-onion v0.0.0-20190909101754-850081b08b6a/go.mod h1:rigfi6Af/KqsF7Za0hOgcyq2PNH4AN70AaMRxcJkff4= +github.com/lightningnetwork/lightning-onion v0.0.0-20191214001659-f34e9dc1651d h1:U50MHOOeL6gR3Ee/l0eMvZMpmRo+ydzmlQuIruCyCsA= +github.com/lightningnetwork/lightning-onion v0.0.0-20191214001659-f34e9dc1651d/go.mod h1:rigfi6Af/KqsF7Za0hOgcyq2PNH4AN70AaMRxcJkff4= github.com/ltcsuite/ltcd v0.0.0-20190101042124-f37f8bf35796 h1:sjOGyegMIhvgfq5oaue6Td+hxZuf3tDC8lAPrFldqFw= github.com/ltcsuite/ltcd v0.0.0-20190101042124-f37f8bf35796/go.mod h1:3p7ZTf9V1sNPI5H8P3NkTFF4LuwMdPl2DodF60qAKqY= github.com/ltcsuite/ltcutil v0.0.0-20181217130922-17f3b04680b6/go.mod h1:8Vg/LTOO0KYa/vlHWJ6XZAevPQThGH5sufO0Hrou/lA= diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index a15f88c98..093ee8bc9 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -74,7 +74,9 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { } } else { // If this was a resumed attempt, we must regenerate the - // circuit. + // circuit. We don't need to check for errors resulting + // from an invalid route, because the sphinx packet has + // been successfully generated before. _, c, err := generateSphinxPacket( &p.attempt.Route, p.payment.PaymentHash[:], p.attempt.SessionKey, @@ -239,7 +241,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, } // Create a new payment attempt from the given payment session. - route, err := p.paySession.RequestRoute( + rt, err := p.paySession.RequestRoute( p.payment, uint32(p.currentHeight), p.finalCLTVDelta, ) if err != nil { @@ -279,8 +281,26 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, // with the htlcAdd message that we send directly to the // switch. onionBlob, c, err := generateSphinxPacket( - route, p.payment.PaymentHash[:], sessionKey, + rt, p.payment.PaymentHash[:], sessionKey, ) + + // With SendToRoute, it can happen that the route exceeds protocol + // constraints. Mark the payment as failed with an internal error. + if err == route.ErrMaxRouteHopsExceeded || + err == sphinx.ErrMaxRoutingInfoSizeExceeded { + + log.Debugf("Invalid route provided for payment %x: %v", + p.payment.PaymentHash, err) + + controlErr := p.router.cfg.Control.Fail( + p.payment.PaymentHash, channeldb.FailureReasonError, + ) + if controlErr != nil { + return lnwire.ShortChannelID{}, nil, controlErr + } + } + + // In any case, don't continue if there is an error. if err != nil { return lnwire.ShortChannelID{}, nil, err } @@ -293,8 +313,8 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, // metadata within this packet will be used to route the // payment through the network, starting with the first-hop. htlcAdd := &lnwire.UpdateAddHTLC{ - Amount: route.TotalAmount, - Expiry: route.TotalTimeLock, + Amount: rt.TotalAmount, + Expiry: rt.TotalTimeLock, PaymentHash: p.payment.PaymentHash, } copy(htlcAdd.OnionBlob[:], onionBlob) @@ -303,7 +323,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, // the payment. If this attempt fails, then we'll continue on // to the next available route. firstHop := lnwire.NewShortChanIDFromInt( - route.Hops[0].ChannelID, + rt.Hops[0].ChannelID, ) // We generate a new, unique payment ID that we will use for @@ -318,7 +338,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, p.attempt = &channeldb.PaymentAttemptInfo{ PaymentID: paymentID, SessionKey: sessionKey, - Route: *route, + Route: *rt, } // Before sending this HTLC to the switch, we checkpoint the diff --git a/routing/route/route.go b/routing/route/route.go index 1444b9df2..ff043ef89 100644 --- a/routing/route/route.go +++ b/routing/route/route.go @@ -26,6 +26,10 @@ var ( // each route. ErrNoRouteHopsProvided = fmt.Errorf("empty route hops provided") + // ErrMaxRouteHopsExceeded is returned when a caller attempts to + // construct a new sphinx packet, but provides too many hops. + ErrMaxRouteHopsExceeded = fmt.Errorf("route has too many hops") + // ErrIntermediateMPPHop is returned when a hop tries to deliver an MPP // record to an intermediate hop, only final hops can receive MPP // records. @@ -267,6 +271,16 @@ func NewRouteFromHops(amtToSend lnwire.MilliSatoshi, timeLock uint32, func (r *Route) ToSphinxPath() (*sphinx.PaymentPath, error) { var path sphinx.PaymentPath + // We can only construct a route if there are hops provided. + if len(r.Hops) == 0 { + return nil, ErrNoRouteHopsProvided + } + + // Check maximum route length. + if len(r.Hops) > sphinx.NumMaxHops { + return nil, ErrMaxRouteHopsExceeded + } + // For each hop encoded within the route, we'll convert the hop struct // to an OnionHop with matching per-hop payload within the path as used // by the sphinx package. diff --git a/routing/router.go b/routing/router.go index f157b7bda..c8d4bebd1 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1487,13 +1487,6 @@ func generateNewSessionKey() (*btcec.PrivateKey, error) { func generateSphinxPacket(rt *route.Route, paymentHash []byte, sessionKey *btcec.PrivateKey) ([]byte, *sphinx.Circuit, error) { - // As a sanity check, we'll ensure that the set of hops has been - // properly filled in, otherwise, we won't actually be able to - // construct a route. - if len(rt.Hops) == 0 { - return nil, nil, route.ErrNoRouteHopsProvided - } - // Now that we know we have an actual route, we'll map the route into a // sphinx payument path which includes per-hop paylods for each hop // that give each node within the route the necessary information diff --git a/routing/router_test.go b/routing/router_test.go index 9ea9737c0..e3ae01081 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -3339,6 +3339,74 @@ func TestSendToRouteStructuredError(t *testing.T) { } } +// TestSendToRouteMaxHops asserts that SendToRoute fails when using a route that +// exceeds the maximum number of hops. +func TestSendToRouteMaxHops(t *testing.T) { + t.Parallel() + + // Setup a two node network. + chanCapSat := btcutil.Amount(100000) + testChannels := []*testChannel{ + symmetricTestChannel("a", "b", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 400, + MinHTLC: 1, + MaxHTLC: lnwire.NewMSatFromSatoshis(chanCapSat), + }, 1), + } + + testGraph, err := createTestGraphFromChannels(testChannels, "a") + if err != nil { + t.Fatalf("unable to create graph: %v", err) + } + defer testGraph.cleanUp() + + const startingBlockHeight = 101 + + ctx, cleanUp, err := createTestCtxFromGraphInstance( + startingBlockHeight, testGraph, + ) + if err != nil { + t.Fatalf("unable to create router: %v", err) + } + defer cleanUp() + + // Create a 30 hop route that exceeds the maximum hop limit. + const payAmt = lnwire.MilliSatoshi(10000) + hopA := ctx.aliases["a"] + hopB := ctx.aliases["b"] + + var hops []*route.Hop + for i := 0; i < 15; i++ { + hops = append(hops, &route.Hop{ + ChannelID: 1, + PubKeyBytes: hopB, + AmtToForward: payAmt, + LegacyPayload: true, + }) + + hops = append(hops, &route.Hop{ + ChannelID: 1, + PubKeyBytes: hopA, + AmtToForward: payAmt, + LegacyPayload: true, + }) + } + + rt, err := route.NewRouteFromHops(payAmt, 100, ctx.aliases["a"], hops) + if err != nil { + t.Fatalf("unable to create route: %v", err) + } + + // Send off the payment request to the router. We expect an error back + // indicating that the route is too long. + var payment lntypes.Hash + _, err = ctx.router.SendToRoute(payment, rt) + if err != route.ErrMaxRouteHopsExceeded { + t.Fatalf("expected ErrMaxRouteHopsExceeded, but got %v", err) + } +} + // TestBuildRoute tests whether correct routes are built. func TestBuildRoute(t *testing.T) { // Setup a three node network.