diff --git a/channeldb/payment_control.go b/channeldb/payment_control.go index 0bbb344b..c11bc5c9 100644 --- a/channeldb/payment_control.go +++ b/channeldb/payment_control.go @@ -503,7 +503,7 @@ func (p *PaymentControl) Fail(paymentHash lntypes.Hash, return err } - // We mark the payent as failed as long as it is known. This + // We mark the payment as failed as long as it is known. This // lets the last attempt to fail with a terminal write its // failure to the PaymentControl without synchronizing with // other attempts. diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 96f58b45..aa856e7b 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -782,10 +782,13 @@ func (p *shardHandler) handleSendError(attempt *channeldb.HTLCAttemptInfo, if err := p.router.cfg.Control.Fail( p.identifier, *reason); err != nil { - return err + log.Errorf("unable to report failure to control "+ + "tower: %v", err) + + return &internalErrorReason } - return *reason + return reason } // reportFail is a helper closure that reports the failure to the diff --git a/routing/router.go b/routing/router.go index 1f88b02a..e6f4c321 100644 --- a/routing/router.go +++ b/routing/router.go @@ -2,6 +2,7 @@ package routing import ( "bytes" + goErrors "errors" "fmt" "runtime" "strings" @@ -2163,13 +2164,23 @@ func (r *ChannelRouter) SendToRoute(htlcHash lntypes.Hash, rt *route.Route) ( // mark the payment failed with the control tower immediately. Process // the error to check if it maps into a terminal error code, if not use // a generic NO_ROUTE error. - if err := sh.handleSendError(attempt, shardError); err != nil { - return nil, err - } + var failureReason *channeldb.FailureReason + err = sh.handleSendError(attempt, shardError) - err = r.cfg.Control.Fail( - paymentIdentifier, channeldb.FailureReasonNoRoute, - ) + switch { + // If we weren't able to extract a proper failure reason (which can + // happen if the second chance logic is triggered), then we'll use the + // normal no route error. + case err == nil: + err = r.cfg.Control.Fail( + paymentIdentifier, channeldb.FailureReasonNoRoute, + ) + + // If this is a failure reason, then we'll apply the failure directly + // to the control tower, and return the normal response to the caller. + case goErrors.As(err, &failureReason): + err = r.cfg.Control.Fail(paymentIdentifier, *failureReason) + } if err != nil { return nil, err } diff --git a/routing/router_test.go b/routing/router_test.go index 29ad342d..558a54c8 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -2921,44 +2921,64 @@ func TestSendToRouteStructuredError(t *testing.T) { 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.(*mockPaymentAttemptDispatcherOld).setPaymentResult( - func(firstHop lnwire.ShortChannelID) ([32]byte, error) { - return [32]byte{}, htlcswitch.NewForwardingError( - &lnwire.FailFeeInsufficient{ - Update: lnwire.ChannelUpdate{}, - }, 1, + finalHopIndex := len(hops) + testCases := map[int]lnwire.FailureMessage{ + finalHopIndex: lnwire.NewFailIncorrectDetails(payAmt, 100), + 1: &lnwire.FailFeeInsufficient{ + Update: lnwire.ChannelUpdate{}, + }, + } + + for failIndex, errorType := range testCases { + failIndex := failIndex + errorType := errorType + + t.Run(fmt.Sprintf("%T", errorType), func(t *testing.T) { + // We'll modify the SendToSwitch method so that it + // simulates a failed payment with an error originating + // from the final hop in the route. + ctx.router.cfg.Payer.(*mockPaymentAttemptDispatcherOld).setPaymentResult( + func(firstHop lnwire.ShortChannelID) ([32]byte, error) { + return [32]byte{}, htlcswitch.NewForwardingError( + errorType, failIndex, + ) + }, ) + + // 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) + require.True( + t, ok, "expected forwarding error, got: %T", err, + ) + + require.IsType( + t, errorType, fErr.WireMessage(), + "expected type %T got %T", errorType, + fErr.WireMessage(), + ) + + // Check that the correct values were used when + // initiating the payment. + select { + case initVal := <-init: + if initVal.c.Value != payAmt { + t.Fatalf("expected %v, got %v", payAmt, + initVal.c.Value) + } + case <-time.After(100 * time.Millisecond): + t.Fatalf("initPayment not called") + } }) - - // 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.WireMessage().(*lnwire.FailFeeInsufficient); !ok { - t.Fatalf("expected fee insufficient error") - } - - // Check that the correct values were used when initiating the payment. - select { - case initVal := <-init: - if initVal.c.Value != payAmt { - t.Fatalf("expected %v, got %v", payAmt, initVal.c.Value) - } - case <-time.After(100 * time.Millisecond): - t.Fatalf("initPayment not called") } }