From 6f0a342f92b1b4eb117632e86535249275a079dd Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 14 Jan 2020 15:07:29 +0200 Subject: [PATCH] htlcswitch: add ForwardingError constructor Add a constructor for the creation of forwarding errors. A special constructor is added for the case where we have an unknown wire failure, and must set a nil failure message. --- htlcswitch/failure.go | 30 ++++++++--- htlcswitch/mock.go | 5 +- htlcswitch/switch.go | 32 ++++-------- routing/router_test.go | 116 ++++++++++++++++++++--------------------- 4 files changed, 89 insertions(+), 94 deletions(-) diff --git a/htlcswitch/failure.go b/htlcswitch/failure.go index 0d817a07..4352d267 100644 --- a/htlcswitch/failure.go +++ b/htlcswitch/failure.go @@ -40,6 +40,27 @@ func (f *ForwardingError) Error() string { ) } +// NewForwardingError creates a new payment error which wraps a wire error +// with additional metadata. +func NewForwardingError(failure lnwire.FailureMessage, index int, + extraMsg string) *ForwardingError { + + return &ForwardingError{ + FailureSourceIdx: index, + FailureMessage: failure, + ExtraMsg: extraMsg, + } +} + +// NewUnknownForwardingError returns a forwarding error which has a nil failure +// message. This constructor should only be used in the case where we cannot +// decode the failure we have received from a peer. +func NewUnknownForwardingError(index int) *ForwardingError { + return &ForwardingError{ + FailureSourceIdx: index, + } +} + // ErrorDecrypter is an interface that is used to decrypt the onion encrypted // failure reason an extra out a well formed error. type ErrorDecrypter interface { @@ -94,15 +115,10 @@ func (s *SphinxErrorDecrypter) DecryptError(reason lnwire.OpaqueReason) ( r := bytes.NewReader(failure.Message) failureMsg, err := lnwire.DecodeFailure(r, 0) if err != nil { - return &ForwardingError{ - FailureSourceIdx: failure.SenderIdx, - }, nil + return NewUnknownForwardingError(failure.SenderIdx), nil } - return &ForwardingError{ - FailureSourceIdx: failure.SenderIdx, - FailureMessage: failureMsg, - }, nil + return NewForwardingError(failureMsg, failure.SenderIdx, ""), nil } // A compile time check to ensure ErrorDecrypter implements the Deobfuscator diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index 5c905822..828c36b9 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -400,10 +400,7 @@ func (o *mockDeobfuscator) DecryptError(reason lnwire.OpaqueReason) (*Forwarding return nil, err } - return &ForwardingError{ - FailureSourceIdx: 1, - FailureMessage: failure, - }, nil + return NewForwardingError(failure, 1, ""), nil } var _ ErrorDecrypter = (*mockDeobfuscator)(nil) diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index b78f43b1..74276ea8 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -756,10 +756,9 @@ func (s *Switch) handleLocalDispatch(pkt *htlcPacket) error { s.indexMtx.RUnlock() if err != nil { log.Errorf("Link %v not found", pkt.outgoingChanID) - return &ForwardingError{ - FailureSourceIdx: 0, - FailureMessage: &lnwire.FailUnknownNextPeer{}, - } + return NewForwardingError( + &lnwire.FailUnknownNextPeer{}, 0, "", + ) } if !link.EligibleToForward() { @@ -770,11 +769,7 @@ func (s *Switch) handleLocalDispatch(pkt *htlcPacket) error { // The update does not need to be populated as the error // will be returned back to the router. htlcErr := lnwire.NewTemporaryChannelFailure(nil) - return &ForwardingError{ - FailureSourceIdx: 0, - ExtraMsg: err.Error(), - FailureMessage: htlcErr, - } + return NewForwardingError(htlcErr, 0, err.Error()) } // Ensure that the htlc satisfies the outgoing channel policy. @@ -788,10 +783,7 @@ func (s *Switch) handleLocalDispatch(pkt *htlcPacket) error { log.Errorf("Link %v policy for local forward not "+ "satisfied", pkt.outgoingChanID) - return &ForwardingError{ - FailureSourceIdx: 0, - FailureMessage: htlcErr, - } + return NewForwardingError(htlcErr, 0, "") } return link.HandleSwitchPacket(pkt) @@ -930,11 +922,7 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter, failureMsg = lnwire.NewTemporaryChannelFailure(nil) } - return &ForwardingError{ - FailureSourceIdx: 0, - ExtraMsg: userErr, - FailureMessage: failureMsg, - } + return NewForwardingError(failureMsg, 0, userErr) // A payment had to be timed out on chain before it got past // the first hop. In this case, we'll report a permanent @@ -945,11 +933,9 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter, "on-chain, then canceled back (hash=%v, pid=%d)", paymentHash, paymentID) - return &ForwardingError{ - FailureSourceIdx: 0, - ExtraMsg: userErr, - FailureMessage: &lnwire.FailPermanentChannelFailure{}, - } + return NewForwardingError( + &lnwire.FailPermanentChannelFailure{}, 0, userErr, + ) // A regular multi-hop payment error that we'll need to // decrypt. diff --git a/routing/router_test.go b/routing/router_test.go index 6fd9a630..f62473a3 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -288,11 +288,12 @@ func TestSendPaymentRouteFailureFallback(t *testing.T) { roasbeefSongoku := lnwire.NewShortChanIDFromInt(12345) if firstHop == roasbeefSongoku { - return [32]byte{}, &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - // TODO(roasbeef): temp node failure should be? - FailureMessage: &lnwire.FailTemporaryChannelFailure{}, - } + return [32]byte{}, htlcswitch.NewForwardingError( + // TODO(roasbeef): temp node failure + // should be? + &lnwire.FailTemporaryChannelFailure{}, + 1, "", + ) } return preImage, nil @@ -420,12 +421,12 @@ func TestChannelUpdateValidation(t *testing.T) { // The unsigned channel update is attached to the failure message. ctx.router.cfg.Payer.(*mockPaymentAttemptDispatcher).setPaymentResult( func(firstHop lnwire.ShortChannelID) ([32]byte, error) { - return [32]byte{}, &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - FailureMessage: &lnwire.FailFeeInsufficient{ + return [32]byte{}, htlcswitch.NewForwardingError( + &lnwire.FailFeeInsufficient{ Update: errChanUpdate, }, - } + 1, "", + ) }) // The payment parameter is mostly redundant in SendToRoute. Can be left @@ -542,16 +543,15 @@ func TestSendPaymentErrorRepeatedFeeInsufficient(t *testing.T) { roasbeefSongoku := lnwire.NewShortChanIDFromInt(chanID) if firstHop == roasbeefSongoku { - return [32]byte{}, &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - - // Within our error, we'll add a channel update - // which is meant to reflect he new fee - // schedule for the node/channel. - FailureMessage: &lnwire.FailFeeInsufficient{ + return [32]byte{}, htlcswitch.NewForwardingError( + // Within our error, we'll add a + // channel update which is meant to + // reflect the new fee schedule for the + // node/channel. + &lnwire.FailFeeInsufficient{ Update: errChanUpdate, - }, - } + }, 1, "", + ) } return preImage, nil @@ -646,12 +646,11 @@ func TestSendPaymentErrorNonFinalTimeLockErrors(t *testing.T) { func(firstHop lnwire.ShortChannelID) ([32]byte, error) { if firstHop == roasbeefSongoku { - return [32]byte{}, &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - FailureMessage: &lnwire.FailExpiryTooSoon{ + return [32]byte{}, htlcswitch.NewForwardingError( + &lnwire.FailExpiryTooSoon{ Update: errChanUpdate, - }, - } + }, 1, "", + ) } return preImage, nil @@ -700,12 +699,11 @@ func TestSendPaymentErrorNonFinalTimeLockErrors(t *testing.T) { func(firstHop lnwire.ShortChannelID) ([32]byte, error) { if firstHop == roasbeefSongoku { - return [32]byte{}, &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - FailureMessage: &lnwire.FailIncorrectCltvExpiry{ + return [32]byte{}, htlcswitch.NewForwardingError( + &lnwire.FailIncorrectCltvExpiry{ Update: errChanUpdate, - }, - } + }, 1, "", + ) } return preImage, nil @@ -763,20 +761,19 @@ func TestSendPaymentErrorPathPruning(t *testing.T) { // We'll first simulate an error from the first // hop to simulate the channel from songoku to // sophon not having enough capacity. - return [32]byte{}, &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - FailureMessage: &lnwire.FailTemporaryChannelFailure{}, - } + return [32]byte{}, htlcswitch.NewForwardingError( + &lnwire.FailTemporaryChannelFailure{}, + 1, "", + ) } // Next, we'll create an error from phan nuwen to // indicate that the sophon node is not longer online, // which should prune out the rest of the routes. if firstHop == roasbeefPhanNuwen { - return [32]byte{}, &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - FailureMessage: &lnwire.FailUnknownNextPeer{}, - } + return [32]byte{}, htlcswitch.NewForwardingError( + &lnwire.FailUnknownNextPeer{}, 1, "", + ) } return preImage, nil @@ -805,10 +802,10 @@ func TestSendPaymentErrorPathPruning(t *testing.T) { func(firstHop lnwire.ShortChannelID) ([32]byte, error) { if firstHop == roasbeefSongoku { - return [32]byte{}, &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - FailureMessage: &lnwire.FailUnknownNextPeer{}, - } + failure := htlcswitch.NewForwardingError( + &lnwire.FailUnknownNextPeer{}, 1, "", + ) + return [32]byte{}, failure } return preImage, nil @@ -851,10 +848,10 @@ func TestSendPaymentErrorPathPruning(t *testing.T) { // We'll first simulate an error from the first // outgoing link to simulate the channel from luo ji to // roasbeef not having enough capacity. - return [32]byte{}, &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - FailureMessage: &lnwire.FailTemporaryChannelFailure{}, - } + return [32]byte{}, htlcswitch.NewForwardingError( + &lnwire.FailTemporaryChannelFailure{}, + 1, "", + ) } return preImage, nil }) @@ -2539,9 +2536,7 @@ func TestUnknownErrorSource(t *testing.T) { // couldn't be decoded (FailureMessage is nil). if firstHop.ToUint64() == 2 { return [32]byte{}, - &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - } + htlcswitch.NewUnknownForwardingError(1) } // Otherwise the payment succeeds. @@ -3105,10 +3100,10 @@ func TestRouterPaymentStateMachine(t *testing.T) { // called, and we respond with a forwarding error case sendToSwitchResultFailure: select { - case sendResult <- &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - FailureMessage: &lnwire.FailTemporaryChannelFailure{}, - }: + case sendResult <- htlcswitch.NewForwardingError( + &lnwire.FailTemporaryChannelFailure{}, + 1, "", + ): case <-time.After(1 * time.Second): t.Fatalf("unable to send result") } @@ -3129,12 +3124,14 @@ func TestRouterPaymentStateMachine(t *testing.T) { // to be called, and we respond with a forwarding // error, indicating that the router should retry. case getPaymentResultFailure: + failure := htlcswitch.NewForwardingError( + &lnwire.FailTemporaryChannelFailure{}, + 1, "", + ) + select { case getPaymentResult <- &htlcswitch.PaymentResult{ - Error: &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - FailureMessage: &lnwire.FailTemporaryChannelFailure{}, - }, + Error: failure, }: case <-time.After(1 * time.Second): t.Fatalf("unable to get result") @@ -3302,12 +3299,11 @@ func TestSendToRouteStructuredError(t *testing.T) { // The unsigned channel update is attached to the failure message. ctx.router.cfg.Payer.(*mockPaymentAttemptDispatcher).setPaymentResult( func(firstHop lnwire.ShortChannelID) ([32]byte, error) { - return [32]byte{}, &htlcswitch.ForwardingError{ - FailureSourceIdx: 1, - FailureMessage: &lnwire.FailFeeInsufficient{ + return [32]byte{}, htlcswitch.NewForwardingError( + &lnwire.FailFeeInsufficient{ Update: lnwire.ChannelUpdate{}, - }, - } + }, 1, "", + ) }) // The payment parameter is mostly redundant in SendToRoute. Can be left