From b24edb28330d8e6e49a166b68a2ac520707a17d0 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 6 Jul 2021 18:35:20 -0700 Subject: [PATCH] routing: modify TestSendToRouteStructuredError to return non-second chance error In this commit, we modify the existing `TestSendToRouteStructuredError` test to return an error that doesn't trigger the second chance logic. Otherwise, we'll get a nil failure result from the mission control interpretation, meaning we won't exercise the full code path. Instead, we use a terminal error to ensure that the expected code path is hit. As is, this test will fail as a recent refactoring causes us to return a `channeldb.FailureReason` error, since the newly added `handleSendError` code path in the `SendToRoute` method will return the raw error, rather than the `shardError`, which is of the expected type. --- routing/router_test.go | 92 +++++++++++++++++++++++++----------------- 1 file changed, 56 insertions(+), 36 deletions(-) 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") } }