From 0e273a5731fc595367ae30432e991b37c927ded0 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 23 May 2019 21:17:16 +0200 Subject: [PATCH] routing: return structured error for send to route --- routing/payment_lifecycle.go | 40 +++++++++++--- routing/router.go | 25 ++++++--- routing/router_test.go | 104 +++++++++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+), 16 deletions(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 38883ca8..46993d2a 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -12,6 +12,20 @@ import ( "github.com/lightningnetwork/lnd/routing/route" ) +// errNoRoute is returned when all routes from the payment session have been +// attempted. +type errNoRoute struct { + // lastError is the error encountered during the last payment attempt, + // if at least one attempt has been made. + lastError *htlcswitch.ForwardingError +} + +// Error returns a string representation of the error. +func (e errNoRoute) Error() string { + return fmt.Sprintf("unable to route payment to destination: %v", + e.lastError) +} + // paymentLifecycle holds all information about the current state of a payment // needed to resume if from any point. type paymentLifecycle struct { @@ -23,7 +37,7 @@ type paymentLifecycle struct { finalCLTVDelta uint16 attempt *channeldb.PaymentAttemptInfo circuit *sphinx.Circuit - lastError error + lastError *htlcswitch.ForwardingError } // resumePayment resumes the paymentLifecycle from the current state. @@ -218,10 +232,8 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, // payment, we'll return that. if p.lastError != nil { return lnwire.ShortChannelID{}, nil, - fmt.Errorf("unable to route payment to "+ - "destination: %v", p.lastError) + errNoRoute{lastError: p.lastError} } - // Terminal state, return. return lnwire.ShortChannelID{}, nil, err } @@ -326,9 +338,21 @@ func (p *paymentLifecycle) sendPaymentAttempt(firstHop lnwire.ShortChannelID, // handleSendError inspects the given error from the Switch and determines // whether we should make another payment attempt. func (p *paymentLifecycle) handleSendError(sendErr error) error { - finalOutcome := p.router.processSendError( - p.paySession, &p.attempt.Route, sendErr, - ) + var finalOutcome bool + + // If an internal, non-forwarding error occurred, we can stop trying. + fErr, ok := sendErr.(*htlcswitch.ForwardingError) + if !ok { + finalOutcome = true + } else { + finalOutcome = p.router.processSendError( + p.paySession, &p.attempt.Route, fErr, + ) + + // Save the forwarding error so it can be returned if this turns + // out to be the last attempt. + p.lastError = fErr + } if finalOutcome { log.Errorf("Payment %x failed with final outcome: %v", @@ -348,7 +372,5 @@ func (p *paymentLifecycle) handleSendError(sendErr error) error { return sendErr } - // We get ready to make another payment attempt. - p.lastError = sendErr return nil } diff --git a/routing/router.go b/routing/router.go index 6e85a096..539bdb4e 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1658,7 +1658,23 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( // Since this is the first time this payment is being made, we pass nil // for the existing attempt. preimage, _, err := r.sendPayment(nil, payment, paySession) - return preimage, err + if err != nil { + // SendToRoute should return a structured error. In case the + // provided route fails, payment lifecycle will return a + // noRouteError with the structured error embedded. + if noRouteError, ok := err.(errNoRoute); ok { + if noRouteError.lastError == nil { + return lntypes.Preimage{}, + errors.New("failure message missing") + } + + return lntypes.Preimage{}, noRouteError.lastError + } + + return lntypes.Preimage{}, err + } + + return preimage, nil } // sendPayment attempts to send a payment as described within the passed @@ -1740,12 +1756,7 @@ func (r *ChannelRouter) sendPayment( // to continue with an alternative route. This is indicated by the boolean // return value. func (r *ChannelRouter) processSendError(paySession PaymentSession, - rt *route.Route, err error) bool { - - fErr, ok := err.(*htlcswitch.ForwardingError) - if !ok { - return true - } + rt *route.Route, fErr *htlcswitch.ForwardingError) bool { errSource := fErr.ErrorSource errVertex := route.NewVertex(errSource) diff --git a/routing/router_test.go b/routing/router_test.go index 9dd2e85e..f90dbd8c 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -3136,3 +3136,107 @@ func TestRouterPaymentStateMachine(t *testing.T) { } } } + +// TestSendToRouteStructuredError asserts that SendToRoute returns a structured +// error. +func TestSendToRouteStructuredError(t *testing.T) { + t.Parallel() + + // Setup a three node network. + chanCapSat := btcutil.Amount(100000) + testChannels := []*testChannel{ + symmetricTestChannel("a", "b", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 400, + MinHTLC: 1, + MaxHTLC: lnwire.NewMSatFromSatoshis(chanCapSat), + }, 1), + symmetricTestChannel("b", "c", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 400, + MinHTLC: 1, + MaxHTLC: lnwire.NewMSatFromSatoshis(chanCapSat), + }, 2), + } + + testGraph, err := createTestGraphFromChannels(testChannels) + 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() + + // Setup a route from source a to destination c. The route will be used + // in a call to SendToRoute. SendToRoute also applies channel updates, + // but it saves us from including RequestRoute in the test scope too. + hop1 := ctx.aliases["b"] + hop2 := ctx.aliases["c"] + + hops := []*route.Hop{ + { + ChannelID: 1, + PubKeyBytes: hop1, + }, + { + ChannelID: 2, + PubKeyBytes: hop2, + }, + } + + rt, err := route.NewRouteFromHops( + lnwire.MilliSatoshi(10000), 100, + ctx.aliases["a"], hops, + ) + if err != nil { + t.Fatalf("unable to create route: %v", err) + } + + // We'll modify the SendToSwitch method so that it simulates a failed + // payment with an error originating from the first hop of the route. + // The unsigned channel update is attached to the failure message. + ctx.router.cfg.Payer.(*mockPaymentAttemptDispatcher).setPaymentResult( + func(firstHop lnwire.ShortChannelID) ([32]byte, error) { + + v := ctx.aliases["b"] + source, err := btcec.ParsePubKey( + v[:], btcec.S256(), + ) + if err != nil { + t.Fatal(err) + } + + return [32]byte{}, &htlcswitch.ForwardingError{ + ErrorSource: source, + FailureMessage: &lnwire.FailFeeInsufficient{ + Update: lnwire.ChannelUpdate{}, + }, + } + }) + + // The payment parameter is mostly redundant in SendToRoute. Can be left + // empty for this test. + var payment lntypes.Hash + + // Send off the payment request to the router. The specified route + // should be attempted and the channel update should be received by + // router and ignored because it is missing a valid signature. + _, err = ctx.router.SendToRoute(payment, rt) + + fErr, ok := err.(*htlcswitch.ForwardingError) + if !ok { + t.Fatalf("expected forwarding error") + } + + if _, ok := fErr.FailureMessage.(*lnwire.FailFeeInsufficient); !ok { + t.Fatalf("expected fee insufficient error") + } +}