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

@@ -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{
+		return NewUnknownForwardingError(failure.SenderIdx), nil
-			FailureSourceIdx: failure.SenderIdx,
-		}, nil
 	}
 
-	return &ForwardingError{
+	return NewForwardingError(failureMsg, failure.SenderIdx, ""), nil
-		FailureSourceIdx: failure.SenderIdx,
-		FailureMessage:   failureMsg,
-	}, nil
 }
 
 // A compile time check to ensure ErrorDecrypter implements the Deobfuscator

htlcswitch/mock.go

@@ -400,10 +400,7 @@ func (o *mockDeobfuscator) DecryptError(reason lnwire.OpaqueReason) (*Forwarding
 		return nil, err
 	}
 
-	return &ForwardingError{
+	return NewForwardingError(failure, 1, ""), nil
-		FailureSourceIdx: 1,
-		FailureMessage:   failure,
-	}, nil
 }
 
 var _ ErrorDecrypter = (*mockDeobfuscator)(nil)

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{
+			return NewForwardingError(
-				FailureSourceIdx: 0,
+				&lnwire.FailUnknownNextPeer{}, 0, "",
-				FailureMessage:   &lnwire.FailUnknownNextPeer{},
+			)
-			}
 		}
 
 		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{
+			return NewForwardingError(htlcErr, 0, err.Error())
-				FailureSourceIdx: 0,
-				ExtraMsg:         err.Error(),
-				FailureMessage:   htlcErr,
-			}
 		}
 
 		// 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{
+			return NewForwardingError(htlcErr, 0, "")
-				FailureSourceIdx: 0,
-				FailureMessage:   htlcErr,
-			}
 		}
 
 		return link.HandleSwitchPacket(pkt)
@@ -930,11 +922,7 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter,
 			failureMsg = lnwire.NewTemporaryChannelFailure(nil)
 		}
 
-		return &ForwardingError{
+		return NewForwardingError(failureMsg, 0, userErr)
-			FailureSourceIdx: 0,
-			ExtraMsg:         userErr,
-			FailureMessage:   failureMsg,
-		}
 
 	// 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{
+		return NewForwardingError(
-			FailureSourceIdx: 0,
+			&lnwire.FailPermanentChannelFailure{}, 0, userErr,
-			ExtraMsg:         userErr,
+		)
-			FailureMessage:   &lnwire.FailPermanentChannelFailure{},
-		}
 
 	// A regular multi-hop payment error that we'll need to
 	// decrypt.

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{
+				return [32]byte{}, htlcswitch.NewForwardingError(
-					FailureSourceIdx: 1,
+					// TODO(roasbeef): temp node failure
-					// TODO(roasbeef): temp node failure should be?
+					//  should be?
-					FailureMessage: &lnwire.FailTemporaryChannelFailure{},
+					&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{
+			return [32]byte{}, htlcswitch.NewForwardingError(
-				FailureSourceIdx: 1,
+				&lnwire.FailFeeInsufficient{
-				FailureMessage: &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{
+				return [32]byte{}, htlcswitch.NewForwardingError(
-					FailureSourceIdx: 1,
+					// Within our error, we'll add a
-
+					// channel update which is meant to
-					// Within our error, we'll add a channel update
+					// reflect the new fee schedule for the
-					// which is meant to reflect he new fee
+					// node/channel.
-					// schedule for the node/channel.
+					&lnwire.FailFeeInsufficient{
-					FailureMessage: &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{
+				return [32]byte{}, htlcswitch.NewForwardingError(
-					FailureSourceIdx: 1,
+					&lnwire.FailExpiryTooSoon{
-					FailureMessage: &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{
+				return [32]byte{}, htlcswitch.NewForwardingError(
-					FailureSourceIdx: 1,
+					&lnwire.FailIncorrectCltvExpiry{
-					FailureMessage: &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{
+				return [32]byte{}, htlcswitch.NewForwardingError(
-					FailureSourceIdx: 1,
+					&lnwire.FailTemporaryChannelFailure{},
-					FailureMessage:   &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{
+				return [32]byte{}, htlcswitch.NewForwardingError(
-					FailureSourceIdx: 1,
+					&lnwire.FailUnknownNextPeer{}, 1, "",
-					FailureMessage:   &lnwire.FailUnknownNextPeer{},
+				)
-				}
 			}
 
 			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{
+				failure := htlcswitch.NewForwardingError(
-					FailureSourceIdx: 1,
+					&lnwire.FailUnknownNextPeer{}, 1, "",
-					FailureMessage:   &lnwire.FailUnknownNextPeer{},
+				)
-				}
+				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{
+				return [32]byte{}, htlcswitch.NewForwardingError(
-					FailureSourceIdx: 1,
+					&lnwire.FailTemporaryChannelFailure{},
-					FailureMessage:   &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{
+					htlcswitch.NewUnknownForwardingError(1)
-						FailureSourceIdx: 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{
+				case sendResult <- htlcswitch.NewForwardingError(
-					FailureSourceIdx: 1,
+					&lnwire.FailTemporaryChannelFailure{},
-					FailureMessage:   &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{
+					Error: failure,
-						FailureSourceIdx: 1,
-						FailureMessage:   &lnwire.FailTemporaryChannelFailure{},
-					},
 				}:
 				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{
+			return [32]byte{}, htlcswitch.NewForwardingError(
-				FailureSourceIdx: 1,
+				&lnwire.FailFeeInsufficient{
-				FailureMessage: &lnwire.FailFeeInsufficient{
 					Update: lnwire.ChannelUpdate{},
-				},
+				}, 1, "",
-			}
+			)
 		})
 
 	// The payment parameter is mostly redundant in SendToRoute. Can be left