From b5a2d75465f3b027fb556c9ec5dcd3bb3c214127 Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 14 Jan 2020 15:07:42 +0200 Subject: [PATCH] htlcswitch+routing: type check on ClearTextError Update the type check used for checking local payment failures to check on the ClearTextError interface rather than on the ForwardingError type. This change prepares for splitting payment errors up into Link and Forwarding errors. --- htlcswitch/link_test.go | 90 +++++++++++++++++--------------- htlcswitch/switch_test.go | 18 ++++--- lnrpc/routerrpc/router_server.go | 15 ++++-- routing/mock_test.go | 4 +- routing/router.go | 29 +++++++--- 5 files changed, 94 insertions(+), 62 deletions(-) diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 0bee52715..16bd92e7d 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -574,12 +574,12 @@ func TestExitNodeTimelockPayloadMismatch(t *testing.T) { t.Fatalf("payment should have failed but didn't") } - ferr, ok := err.(*ForwardingError) + rtErr, ok := err.(ClearTextError) if !ok { - t.Fatalf("expected a ForwardingError, instead got: %T", err) + t.Fatalf("expected a ClearTextError, instead got: %T", err) } - switch ferr.WireMessage().(type) { + switch rtErr.WireMessage().(type) { case *lnwire.FailFinalIncorrectCltvExpiry: default: t.Fatalf("incorrect error, expected incorrect cltv expiry, "+ @@ -674,12 +674,12 @@ func TestLinkForwardTimelockPolicyMismatch(t *testing.T) { t.Fatalf("payment should have failed but didn't") } - ferr, ok := err.(*ForwardingError) + rtErr, ok := err.(ClearTextError) if !ok { - t.Fatalf("expected a ForwardingError, instead got: %T", err) + t.Fatalf("expected a ClearTextError, instead got: %T", err) } - switch ferr.WireMessage().(type) { + switch rtErr.WireMessage().(type) { case *lnwire.FailIncorrectCltvExpiry: default: t.Fatalf("incorrect error, expected incorrect cltv expiry, "+ @@ -732,12 +732,12 @@ func TestLinkForwardFeePolicyMismatch(t *testing.T) { t.Fatalf("payment should have failed but didn't") } - ferr, ok := err.(*ForwardingError) + rtErr, ok := err.(ClearTextError) if !ok { - t.Fatalf("expected a ForwardingError, instead got: %T", err) + t.Fatalf("expected a ClearTextError, instead got: %T", err) } - switch ferr.WireMessage().(type) { + switch rtErr.WireMessage().(type) { case *lnwire.FailFeeInsufficient: default: t.Fatalf("incorrect error, expected fee insufficient, "+ @@ -790,12 +790,12 @@ func TestLinkForwardMinHTLCPolicyMismatch(t *testing.T) { t.Fatalf("payment should have failed but didn't") } - ferr, ok := err.(*ForwardingError) + rtErr, ok := err.(ClearTextError) if !ok { - t.Fatalf("expected a ForwardingError, instead got: %T", err) + t.Fatalf("expected a ClearTextError, instead got: %T", err) } - switch ferr.WireMessage().(type) { + switch rtErr.WireMessage().(type) { case *lnwire.FailAmountBelowMinimum: default: t.Fatalf("incorrect error, expected amount below minimum, "+ @@ -857,12 +857,12 @@ func TestLinkForwardMaxHTLCPolicyMismatch(t *testing.T) { t.Fatalf("payment should have failed but didn't") } - ferr, ok := err.(*ForwardingError) + rtErr, ok := err.(ClearTextError) if !ok { - t.Fatalf("expected a ForwardingError, instead got: %T", err) + t.Fatalf("expected a ClearTextError, instead got: %T", err) } - switch ferr.WireMessage().(type) { + switch rtErr.WireMessage().(type) { case *lnwire.FailTemporaryChannelFailure: default: t.Fatalf("incorrect error, expected temporary channel failure, "+ @@ -964,11 +964,12 @@ func TestUpdateForwardingPolicy(t *testing.T) { t.Fatalf("payment should've been rejected") } - ferr, ok := err.(*ForwardingError) + rtErr, ok := err.(ClearTextError) if !ok { - t.Fatalf("expected a ForwardingError, instead got (%T): %v", err, err) + t.Fatalf("expected a ClearTextError, instead got (%T): %v", err, err) } - switch ferr.WireMessage().(type) { + + switch rtErr.WireMessage().(type) { case *lnwire.FailFeeInsufficient: default: t.Fatalf("expected FailFeeInsufficient instead got: %v", err) @@ -1003,12 +1004,13 @@ func TestUpdateForwardingPolicy(t *testing.T) { t.Fatalf("payment should've been rejected") } - ferr, ok = err.(*ForwardingError) + rtErr, ok = err.(ClearTextError) if !ok { - t.Fatalf("expected a ForwardingError, instead got (%T): %v", + t.Fatalf("expected a ClearTextError, instead got (%T): %v", err, err) } - switch ferr.WireMessage().(type) { + + switch rtErr.WireMessage().(type) { case *lnwire.FailTemporaryChannelFailure: default: t.Fatalf("expected TemporaryChannelFailure, instead got: %v", @@ -1249,13 +1251,14 @@ func TestChannelLinkMultiHopUnknownNextHop(t *testing.T) { if err == nil { t.Fatal("error haven't been received") } - fErr, ok := err.(*ForwardingError) + rtErr, ok := err.(ClearTextError) if !ok { - t.Fatalf("expected ForwardingError") + t.Fatalf("expected ClearTextError") } - if _, ok = fErr.WireMessage().(*lnwire.FailUnknownNextPeer); !ok { + + if _, ok = rtErr.WireMessage().(*lnwire.FailUnknownNextPeer); !ok { t.Fatalf("wrong error has been received: %T", - fErr.WireMessage()) + rtErr.WireMessage()) } // Wait for Alice to receive the revocation. @@ -1364,12 +1367,12 @@ func TestChannelLinkMultiHopDecodeError(t *testing.T) { t.Fatal("error haven't been received") } - ferr, ok := err.(*ForwardingError) + rtErr, ok := err.(ClearTextError) if !ok { - t.Fatalf("expected a ForwardingError, instead got: %T", err) + t.Fatalf("expected a ClearTextError, instead got: %T", err) } - switch ferr.WireMessage().(type) { + switch rtErr.WireMessage().(type) { case *lnwire.FailInvalidOnionVersion: default: t.Fatalf("wrong error have been received: %v", err) @@ -1456,13 +1459,13 @@ func TestChannelLinkExpiryTooSoonExitNode(t *testing.T) { "time lock value") } - ferr, ok := err.(*ForwardingError) + rtErr, ok := err.(ClearTextError) if !ok { - t.Fatalf("expected a ForwardingError, instead got: %T %v", - err, err) + t.Fatalf("expected a ClearTextError, instead got: %T %v", + rtErr, err) } - switch ferr.WireMessage().(type) { + switch rtErr.WireMessage().(type) { case *lnwire.FailIncorrectDetails: default: t.Fatalf("expected incorrect_or_unknown_payment_details, "+ @@ -1519,12 +1522,13 @@ func TestChannelLinkExpiryTooSoonMidNode(t *testing.T) { "time lock value") } - ferr, ok := err.(*ForwardingError) + rtErr, ok := err.(ClearTextError) if !ok { - t.Fatalf("expected a ForwardingError, instead got: %T: %v", err, err) + t.Fatalf("expected a ClearTextError, instead got: %T: %v", + rtErr, err) } - switch ferr.WireMessage().(type) { + switch rtErr.WireMessage().(type) { case *lnwire.FailExpiryTooSoon: default: t.Fatalf("incorrect error, expected final time lock too "+ @@ -5632,11 +5636,11 @@ func TestChannelLinkCanceledInvoice(t *testing.T) { // Because the invoice is canceled, we expect an unknown payment hash // result. - fErr, ok := err.(*ForwardingError) + rtErr, ok := err.(ClearTextError) if !ok { - t.Fatalf("expected ForwardingError, but got %v", err) + t.Fatalf("expected ClearTextError, but got %v", err) } - _, ok = fErr.WireMessage().(*lnwire.FailIncorrectDetails) + _, ok = rtErr.WireMessage().(*lnwire.FailIncorrectDetails) if !ok { t.Fatalf("expected unknown payment hash, but got %v", err) } @@ -6213,16 +6217,16 @@ func TestChannelLinkReceiveEmptySig(t *testing.T) { aliceLink.Stop() } -// assertFailureCode asserts that an error is of type ForwardingError and that +// assertFailureCode asserts that an error is of type ClearTextError and that // the failure code is as expected. func assertFailureCode(t *testing.T, err error, code lnwire.FailCode) { - fErr, ok := err.(*ForwardingError) + rtErr, ok := err.(ClearTextError) if !ok { - t.Fatalf("expected ForwardingError but got %T", err) + t.Fatalf("expected ClearTextError but got %T", err) } - if fErr.WireMessage().Code() != code { + if rtErr.WireMessage().Code() != code { t.Fatalf("expected %v but got %v", - code, fErr.WireMessage().Code()) + code, rtErr.WireMessage().Code()) } } diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 7f78bfe4e..0f6ab9e01 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -2178,11 +2178,11 @@ func TestUpdateFailMalformedHTLCErrorConversion(t *testing.T) { t.Fatalf("unable to send payment: %v", err) } - fwdingErr := err.(*ForwardingError) - failureMsg := fwdingErr.WireMessage() + routingErr := err.(ClearTextError) + failureMsg := routingErr.WireMessage() if _, ok := failureMsg.(*lnwire.FailInvalidOnionKey); !ok { t.Fatalf("expected onion failure instead got: %v", - fwdingErr.WireMessage()) + routingErr.WireMessage()) } } @@ -2441,14 +2441,18 @@ func TestInvalidFailure(t *testing.T) { select { case result := <-resultChan: - fErr, ok := result.Error.(*ForwardingError) + rtErr, ok := result.Error.(ClearTextError) if !ok { - t.Fatal("expected ForwardingError") + t.Fatal("expected ClearTextError") } - if fErr.FailureSourceIdx != 2 { + source, ok := rtErr.(*ForwardingError) + if !ok { + t.Fatalf("expected forwarding error, got: %T", rtErr) + } + if source.FailureSourceIdx != 2 { t.Fatal("unexpected error source index") } - if fErr.WireMessage() != nil { + if rtErr.WireMessage() != nil { t.Fatal("expected empty failure message") } diff --git a/lnrpc/routerrpc/router_server.go b/lnrpc/routerrpc/router_server.go index f761883c4..93cbeff0b 100644 --- a/lnrpc/routerrpc/router_server.go +++ b/lnrpc/routerrpc/router_server.go @@ -326,12 +326,12 @@ func marshallError(sendError error) (*Failure, error) { return response, nil } - fErr, ok := sendError.(*htlcswitch.ForwardingError) + rtErr, ok := sendError.(htlcswitch.ClearTextError) if !ok { return nil, sendError } - switch onionErr := fErr.WireMessage().(type) { + switch onionErr := rtErr.WireMessage().(type) { case *lnwire.FailIncorrectDetails: response.Code = Failure_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS @@ -425,7 +425,16 @@ func marshallError(sendError error) (*Failure, error) { return nil, fmt.Errorf("cannot marshall failure %T", onionErr) } - response.FailureSourceIndex = uint32(fErr.FailureSourceIdx) + // If the ClearTextError received is a ForwardingError, the error + // originated from a node along the route, not locally on our outgoing + // link. We set failureSourceIdx to the index of the node where the + // failure occurred. If the error is not a ForwardingError, the failure + // occurred at our node, so we leave the index as 0 to indicate that + // we failed locally. + fErr, ok := rtErr.(*htlcswitch.ForwardingError) + if ok { + response.FailureSourceIndex = uint32(fErr.FailureSourceIdx) + } return response, nil } diff --git a/routing/mock_test.go b/routing/mock_test.go index d2f7448c7..fac67aa9f 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -35,12 +35,12 @@ func (m *mockPaymentAttemptDispatcher) SendHTLC(firstHop lnwire.ShortChannelID, var result *htlcswitch.PaymentResult preimage, err := m.onPayment(firstHop) if err != nil { - fwdErr, ok := err.(*htlcswitch.ForwardingError) + rtErr, ok := err.(htlcswitch.ClearTextError) if !ok { return err } result = &htlcswitch.PaymentResult{ - Error: fwdErr, + Error: rtErr, } } else { result = &htlcswitch.PaymentResult{Preimage: preimage} diff --git a/routing/router.go b/routing/router.go index 0b2ab2846..ae090144c 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1910,18 +1910,33 @@ func (r *ChannelRouter) processSendError(paymentID uint64, rt *route.Route, return reportFail(nil, nil) } - // If an internal, non-forwarding error occurred, we can stop - // trying. - fErr, ok := sendErr.(*htlcswitch.ForwardingError) + + // If the error is a ClearTextError, we have received a valid wire + // failure message, either from our own outgoing link or from a node + // down the route. If the error is not related to the propagation of + // our payment, we can stop trying because an internal error has + // occurred. + rtErr, ok := sendErr.(htlcswitch.ClearTextError) if !ok { return &internalErrorReason } - failureMessage := fErr.WireMessage() - failureSourceIdx := fErr.FailureSourceIdx + // failureSourceIdx is the index of the node that the failure occurred + // at. If the ClearTextError received is not a ForwardingError the + // payment error occurred at our node, so we leave this value as 0 + // to indicate that the failure occurred locally. If the error is a + // ForwardingError, it did not originate at our node, so we set + // failureSourceIdx to the index of the node where the failure occurred. + failureSourceIdx := 0 + source, ok := rtErr.(*htlcswitch.ForwardingError) + if ok { + failureSourceIdx = source.FailureSourceIdx + } - // Apply channel update if the error contains one. For unknown - // failures, failureMessage is nil. + // Extract the wire failure and apply channel update if it contains one. + // If we received an unknown failure message from a node along the + // route, the failure message will be nil. + failureMessage := rtErr.WireMessage() if failureMessage != nil { err := r.tryApplyChannelUpdate( rt, failureSourceIdx, failureMessage,