diff --git a/htlcswitch/failure.go b/htlcswitch/failure.go index 0d817a07..35d78590 100644 --- a/htlcswitch/failure.go +++ b/htlcswitch/failure.go @@ -9,6 +9,76 @@ import ( "github.com/lightningnetwork/lnd/lnwire" ) +// ClearTextError is an interface which is implemented by errors that occur +// when we know the underlying wire failure message. These errors are the +// opposite to opaque errors which are onion-encrypted blobs only understandable +// to the initiating node. ClearTextErrors are used when we fail a htlc at our +// node, or one of our initiated payments failed and we can decrypt the onion +// encrypted error fully. +type ClearTextError interface { + error + + // WireMessage extracts a valid wire failure message from an internal + // error which may contain additional metadata (which should not be + // exposed to the network). This value may be nil in the case where + // an unknown wire error is returned by one of our peers. + WireMessage() lnwire.FailureMessage +} + +// LinkError is an implementation of the ClearTextError interface which +// represents failures that occur on our incoming or outgoing link. +type LinkError struct { + // msg returns the wire failure associated with the error. + // This value should *not* be nil, because we should always + // know the failure type for failures which occur at our own + // node. + msg lnwire.FailureMessage + + // FailureDetail enriches the wire error with additional information. + FailureDetail +} + +// NewLinkError returns a LinkError with the failure message provided. +// The failure message provided should *not* be nil, because we should +// always know the failure type for failures which occur at our own node. +func NewLinkError(msg lnwire.FailureMessage) *LinkError { + return &LinkError{msg: msg} +} + +// NewDetailedLinkError returns a link error that enriches a wire message with +// a failure detail. +func NewDetailedLinkError(msg lnwire.FailureMessage, + detail FailureDetail) *LinkError { + + return &LinkError{ + msg: msg, + FailureDetail: detail, + } +} + +// WireMessage extracts a valid wire failure message from an internal +// error which may contain additional metadata (which should not be +// exposed to the network). This value should never be nil for LinkErrors, +// because we are the ones failing the htlc. +// +// Note this is part of the ClearTextError interface. +func (l *LinkError) WireMessage() lnwire.FailureMessage { + return l.msg +} + +// Error returns the string representation of a link error. +// +// Note this is part of the ClearTextError interface. +func (l *LinkError) Error() string { + // If the link error has no failure detail, return the wire message's + // error. + if l.FailureDetail == FailureDetailNone { + return l.msg.Error() + } + + return fmt.Sprintf("%v: %v", l.msg.Error(), l.FailureDetail) +} + // ForwardingError wraps an lnwire.FailureMessage in a struct that also // includes the source of the error. type ForwardingError struct { @@ -18,28 +88,51 @@ type ForwardingError struct { // zero is the self node. FailureSourceIdx int - // ExtraMsg is an additional error message that callers can provide in - // order to provide context specific error details. - ExtraMsg string + // msg is the wire message associated with the error. This value may + // be nil in the case where we fail to decode failure message sent by + // a peer. + msg lnwire.FailureMessage +} - lnwire.FailureMessage +// WireMessage extracts a valid wire failure message from an internal +// error which may contain additional metadata (which should not be +// exposed to the network). This value may be nil in the case where +// an unknown wire error is returned by one of our peers. +// +// Note this is part of the ClearTextError interface. +func (f *ForwardingError) WireMessage() lnwire.FailureMessage { + return f.msg } // Error implements the built-in error interface. We use this method to allow // the switch or any callers to insert additional context to the error message // returned. func (f *ForwardingError) Error() string { - if f.ExtraMsg == "" { - return fmt.Sprintf( - "%v@%v", f.FailureMessage, f.FailureSourceIdx, - ) - } - return fmt.Sprintf( - "%v@%v: %v", f.FailureMessage, f.FailureSourceIdx, f.ExtraMsg, + "%v@%v", f.msg, f.FailureSourceIdx, ) } +// NewForwardingError creates a new payment error which wraps a wire error +// with additional metadata. +func NewForwardingError(failure lnwire.FailureMessage, + index int) *ForwardingError { + + return &ForwardingError{ + FailureSourceIdx: index, + msg: failure, + } +} + +// 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 +187,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/failure_detail.go b/htlcswitch/failure_detail.go new file mode 100644 index 00000000..92b25510 --- /dev/null +++ b/htlcswitch/failure_detail.go @@ -0,0 +1,58 @@ +package htlcswitch + +// FailureDetail is an enum which is used to enrich failures with +// additional information. +type FailureDetail int + +const ( + // FailureDetailNone is returned when the wire message contains + // sufficient information. + FailureDetailNone = iota + + // FailureDetailOnionDecode indicates that we could not decode an + // onion error. + FailureDetailOnionDecode + + // FailureDetailLinkNotEligible indicates that a routing attempt was + // made over a link that is not eligible for routing. + FailureDetailLinkNotEligible + + // FailureDetailOnChainTimeout indicates that a payment had to be timed + // out on chain before it got past the first hop by us or the remote + // party. + FailureDetailOnChainTimeout + + // FailureDetailHTLCExceedsMax is returned when a htlc exceeds our + // policy's maximum htlc amount. + FailureDetailHTLCExceedsMax + + // FailureDetailInsufficientBalance is returned when we cannot route a + // htlc due to insufficient outgoing capacity. + FailureDetailInsufficientBalance +) + +// String returns the string representation of a failure detail. +func (fd FailureDetail) String() string { + switch fd { + case FailureDetailNone: + return "no failure detail" + + case FailureDetailOnionDecode: + return "could not decode onion" + + case FailureDetailLinkNotEligible: + return "link not eligible" + + case FailureDetailOnChainTimeout: + return "payment was resolved on-chain, then canceled back" + + case FailureDetailHTLCExceedsMax: + return "htlc exceeds maximum policy amount" + + case FailureDetailInsufficientBalance: + return "insufficient bandwidth to route htlc" + + default: + return "unknown failure detail" + } +} diff --git a/htlcswitch/interfaces.go b/htlcswitch/interfaces.go index bb34b283..1281e951 100644 --- a/htlcswitch/interfaces.go +++ b/htlcswitch/interfaces.go @@ -102,20 +102,21 @@ type ChannelLink interface { // CheckHtlcForward should return a nil error if the passed HTLC details // satisfy the current forwarding policy fo the target link. Otherwise, - // a valid protocol failure message should be returned in order to - // signal to the source of the HTLC, the policy consistency issue. + // a LinkError with a valid protocol failure message should be returned + // in order to signal to the source of the HTLC, the policy consistency + // issue. CheckHtlcForward(payHash [32]byte, incomingAmt lnwire.MilliSatoshi, amtToForward lnwire.MilliSatoshi, incomingTimeout, outgoingTimeout uint32, - heightNow uint32) lnwire.FailureMessage + heightNow uint32) *LinkError // CheckHtlcTransit should return a nil error if the passed HTLC details - // satisfy the current channel policy. Otherwise, a valid protocol - // failure message should be returned in order to signal the violation. - // This call is intended to be used for locally initiated payments for - // which there is no corresponding incoming htlc. + // satisfy the current channel policy. Otherwise, a LinkError with a + // valid protocol failure message should be returned in order to signal + // the violation. This call is intended to be used for locally initiated + // payments for which there is no corresponding incoming htlc. CheckHtlcTransit(payHash [32]byte, amt lnwire.MilliSatoshi, - timeout uint32, heightNow uint32) lnwire.FailureMessage + timeout uint32, heightNow uint32) *LinkError // Bandwidth returns the amount of milli-satoshis which current link // might pass through channel link. The value returned from this method diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 0bba3f65..92ecba65 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -2135,16 +2135,17 @@ func (l *channelLink) UpdateForwardingPolicy(newPolicy ForwardingPolicy) { l.cfg.FwrdingPolicy = newPolicy } -// CheckHtlcForward should return a nil error if the passed HTLC details satisfy -// the current forwarding policy fo the target link. Otherwise, a valid -// protocol failure message should be returned in order to signal to the source -// of the HTLC, the policy consistency issue. +// CheckHtlcForward should return a nil error if the passed HTLC details +// satisfy the current forwarding policy fo the target link. Otherwise, +// a LinkError with a valid protocol failure message should be returned +// in order to signal to the source of the HTLC, the policy consistency +// issue. // // NOTE: Part of the ChannelLink interface. func (l *channelLink) CheckHtlcForward(payHash [32]byte, incomingHtlcAmt, amtToForward lnwire.MilliSatoshi, incomingTimeout, outgoingTimeout uint32, - heightNow uint32) lnwire.FailureMessage { + heightNow uint32) *LinkError { l.RLock() policy := l.cfg.FwrdingPolicy @@ -2176,14 +2177,14 @@ func (l *channelLink) CheckHtlcForward(payHash [32]byte, // As part of the returned error, we'll send our latest routing // policy so the sending node obtains the most up to date data. - - return l.createFailureWithUpdate( + failure := l.createFailureWithUpdate( func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage { return lnwire.NewFeeInsufficient( amtToForward, *upd, ) }, ) + return NewLinkError(failure) } // Finally, we'll ensure that the time-lock on the outgoing HTLC meets @@ -2198,26 +2199,27 @@ func (l *channelLink) CheckHtlcForward(payHash [32]byte, // Grab the latest routing policy so the sending node is up to // date with our current policy. - return l.createFailureWithUpdate( + failure := l.createFailureWithUpdate( func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage { return lnwire.NewIncorrectCltvExpiry( incomingTimeout, *upd, ) }, ) + return NewLinkError(failure) } return nil } -// CheckHtlcTransit should return a nil error if the passed HTLC details satisfy the -// current channel policy. Otherwise, a valid protocol failure message should -// be returned in order to signal the violation. This call is intended to be -// used for locally initiated payments for which there is no corresponding -// incoming htlc. +// CheckHtlcTransit should return a nil error if the passed HTLC details +// satisfy the current channel policy. Otherwise, a LinkError with a +// valid protocol failure message should be returned in order to signal +// the violation. This call is intended to be used for locally initiated +// payments for which there is no corresponding incoming htlc. func (l *channelLink) CheckHtlcTransit(payHash [32]byte, amt lnwire.MilliSatoshi, timeout uint32, - heightNow uint32) lnwire.FailureMessage { + heightNow uint32) *LinkError { l.RLock() policy := l.cfg.FwrdingPolicy @@ -2232,7 +2234,7 @@ func (l *channelLink) CheckHtlcTransit(payHash [32]byte, // the channel's amount and time lock constraints. func (l *channelLink) canSendHtlc(policy ForwardingPolicy, payHash [32]byte, amt lnwire.MilliSatoshi, timeout uint32, - heightNow uint32) lnwire.FailureMessage { + heightNow uint32) *LinkError { // As our first sanity check, we'll ensure that the passed HTLC isn't // too small for the next hop. If so, then we'll cancel the HTLC @@ -2244,13 +2246,14 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy, // As part of the returned error, we'll send our latest routing // policy so the sending node obtains the most up to date data. - return l.createFailureWithUpdate( + failure := l.createFailureWithUpdate( func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage { return lnwire.NewAmountBelowMinimum( amt, *upd, ) }, ) + return NewLinkError(failure) } // Next, ensure that the passed HTLC isn't too large. If so, we'll @@ -2261,11 +2264,12 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy, // As part of the returned error, we'll send our latest routing // policy so the sending node obtains the most up-to-date data. - return l.createFailureWithUpdate( + failure := l.createFailureWithUpdate( func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage { return lnwire.NewTemporaryChannelFailure(upd) }, ) + return NewDetailedLinkError(failure, FailureDetailHTLCExceedsMax) } // We want to avoid offering an HTLC which will expire in the near @@ -2275,12 +2279,12 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy, l.log.Errorf("htlc(%x) has an expiry that's too soon: "+ "outgoing_expiry=%v, best_height=%v", payHash[:], timeout, heightNow) - - return l.createFailureWithUpdate( + failure := l.createFailureWithUpdate( func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage { return lnwire.NewExpiryTooSoon(*upd) }, ) + return NewLinkError(failure) } // Check absolute max delta. @@ -2289,16 +2293,19 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy, "the future: got %v, but maximum is %v", payHash[:], timeout-heightNow, l.cfg.MaxOutgoingCltvExpiry) - return &lnwire.FailExpiryTooFar{} + return NewLinkError(&lnwire.FailExpiryTooFar{}) } // Check to see if there is enough balance in this channel. if amt > l.Bandwidth() { - return l.createFailureWithUpdate( + failure := l.createFailureWithUpdate( func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage { return lnwire.NewTemporaryChannelFailure(upd) }, ) + return NewDetailedLinkError( + failure, FailureDetailInsufficientBalance, + ) } return nil diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 9efe89dc..16d64d70 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.FailureMessage.(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.FailureMessage.(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.FailureMessage.(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.FailureMessage.(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.FailureMessage.(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.FailureMessage.(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.FailureMessage.(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.FailureMessage.(*lnwire.FailUnknownNextPeer); !ok { + + if _, ok = rtErr.WireMessage().(*lnwire.FailUnknownNextPeer); !ok { t.Fatalf("wrong error has been received: %T", - fErr.FailureMessage) + 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.FailureMessage.(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.FailureMessage.(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.FailureMessage.(type) { + switch rtErr.WireMessage().(type) { case *lnwire.FailExpiryTooSoon: default: t.Fatalf("incorrect error, expected final time lock too "+ @@ -5537,7 +5541,7 @@ func TestCheckHtlcForward(t *testing.T) { t.Run("below minhtlc", func(t *testing.T) { result := link.CheckHtlcForward(hash, 100, 50, 200, 150, 0) - if _, ok := result.(*lnwire.FailAmountBelowMinimum); !ok { + if _, ok := result.WireMessage().(*lnwire.FailAmountBelowMinimum); !ok { t.Fatalf("expected FailAmountBelowMinimum failure code") } }) @@ -5545,7 +5549,7 @@ func TestCheckHtlcForward(t *testing.T) { t.Run("above maxhtlc", func(t *testing.T) { result := link.CheckHtlcForward(hash, 1500, 1200, 200, 150, 0) - if _, ok := result.(*lnwire.FailTemporaryChannelFailure); !ok { + if _, ok := result.WireMessage().(*lnwire.FailTemporaryChannelFailure); !ok { t.Fatalf("expected FailTemporaryChannelFailure failure code") } }) @@ -5553,7 +5557,7 @@ func TestCheckHtlcForward(t *testing.T) { t.Run("insufficient fee", func(t *testing.T) { result := link.CheckHtlcForward(hash, 1005, 1000, 200, 150, 0) - if _, ok := result.(*lnwire.FailFeeInsufficient); !ok { + if _, ok := result.WireMessage().(*lnwire.FailFeeInsufficient); !ok { t.Fatalf("expected FailFeeInsufficient failure code") } }) @@ -5561,7 +5565,7 @@ func TestCheckHtlcForward(t *testing.T) { t.Run("expiry too soon", func(t *testing.T) { result := link.CheckHtlcForward(hash, 1500, 1000, 200, 150, 190) - if _, ok := result.(*lnwire.FailExpiryTooSoon); !ok { + if _, ok := result.WireMessage().(*lnwire.FailExpiryTooSoon); !ok { t.Fatalf("expected FailExpiryTooSoon failure code") } }) @@ -5569,7 +5573,7 @@ func TestCheckHtlcForward(t *testing.T) { t.Run("incorrect cltv expiry", func(t *testing.T) { result := link.CheckHtlcForward(hash, 1500, 1000, 200, 190, 0) - if _, ok := result.(*lnwire.FailIncorrectCltvExpiry); !ok { + if _, ok := result.WireMessage().(*lnwire.FailIncorrectCltvExpiry); !ok { t.Fatalf("expected FailIncorrectCltvExpiry failure code") } @@ -5579,7 +5583,7 @@ func TestCheckHtlcForward(t *testing.T) { // Check that expiry isn't too far in the future. result := link.CheckHtlcForward(hash, 1500, 1000, 10200, 10100, 0) - if _, ok := result.(*lnwire.FailExpiryTooFar); !ok { + if _, ok := result.WireMessage().(*lnwire.FailExpiryTooFar); !ok { t.Fatalf("expected FailExpiryTooFar failure code") } }) @@ -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.FailureMessage.(*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.FailureMessage.Code() != code { + if rtErr.WireMessage().Code() != code { t.Fatalf("expected %v but got %v", - code, fErr.FailureMessage.Code()) + code, rtErr.WireMessage().Code()) } } diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index 5c905822..02737a71 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) @@ -648,9 +645,9 @@ type mockChannelLink struct { htlcID uint64 - checkHtlcTransitResult lnwire.FailureMessage + checkHtlcTransitResult *LinkError - checkHtlcForwardResult lnwire.FailureMessage + checkHtlcForwardResult *LinkError } // completeCircuit is a helper method for adding the finalized payment circuit @@ -710,14 +707,14 @@ func (f *mockChannelLink) HandleChannelUpdate(lnwire.Message) { func (f *mockChannelLink) UpdateForwardingPolicy(_ ForwardingPolicy) { } func (f *mockChannelLink) CheckHtlcForward([32]byte, lnwire.MilliSatoshi, - lnwire.MilliSatoshi, uint32, uint32, uint32) lnwire.FailureMessage { + lnwire.MilliSatoshi, uint32, uint32, uint32) *LinkError { return f.checkHtlcForwardResult } func (f *mockChannelLink) CheckHtlcTransit(payHash [32]byte, amt lnwire.MilliSatoshi, timeout uint32, - heightNow uint32) lnwire.FailureMessage { + heightNow uint32) *LinkError { return f.checkHtlcTransitResult } diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index b78f43b1..c08bdc0c 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -756,25 +756,19 @@ 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 NewLinkError(&lnwire.FailUnknownNextPeer{}) } if !link.EligibleToForward() { - err := fmt.Errorf("Link %v is not available to forward", + log.Errorf("Link %v is not available to forward", pkt.outgoingChanID) - log.Error(err) // 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 NewDetailedLinkError( + lnwire.NewTemporaryChannelFailure(nil), + FailureDetailLinkNotEligible, + ) } // Ensure that the htlc satisfies the outgoing channel policy. @@ -787,11 +781,7 @@ func (s *Switch) handleLocalDispatch(pkt *htlcPacket) error { if htlcErr != nil { log.Errorf("Link %v policy for local forward not "+ "satisfied", pkt.outgoingChanID) - - return &ForwardingError{ - FailureSourceIdx: 0, - FailureMessage: htlcErr, - } + return htlcErr } return link.HandleSwitchPacket(pkt) @@ -915,41 +905,43 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter, // decrypt the error, simply decode it them report back to the // user. case unencrypted: - var userErr string r := bytes.NewReader(htlc.Reason) failureMsg, err := lnwire.DecodeFailure(r, 0) if err != nil { - userErr = fmt.Sprintf("unable to decode onion "+ - "failure (hash=%v, pid=%d): %v", - paymentHash, paymentID, err) - log.Error(userErr) + // If we could not decode the failure reason, return a link + // error indicating that we failed to decode the onion. + linkError := NewDetailedLinkError( + // As this didn't even clear the link, we don't + // need to apply an update here since it goes + // directly to the router. + lnwire.NewTemporaryChannelFailure(nil), + FailureDetailOnionDecode, + ) - // As this didn't even clear the link, we don't need to - // apply an update here since it goes directly to the - // router. - failureMsg = lnwire.NewTemporaryChannelFailure(nil) + log.Errorf("%v: (hash=%v, pid=%d): %v", + linkError.FailureDetail, paymentHash, paymentID, + err) + + return linkError } - return &ForwardingError{ - FailureSourceIdx: 0, - ExtraMsg: userErr, - FailureMessage: failureMsg, - } + // If we successfully decoded the failure reason, return it. + return NewLinkError(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 // channel failure as this means us, or the remote party had to // go on chain. case isResolution && htlc.Reason == nil: - userErr := fmt.Sprintf("payment was resolved "+ - "on-chain, then canceled back (hash=%v, pid=%d)", + linkError := NewDetailedLinkError( + &lnwire.FailPermanentChannelFailure{}, + FailureDetailOnChainTimeout, + ) + + log.Info("%v: hash=%v, pid=%d", linkError.FailureDetail, paymentHash, paymentID) - return &ForwardingError{ - FailureSourceIdx: 0, - ExtraMsg: userErr, - FailureMessage: &lnwire.FailPermanentChannelFailure{}, - } + return linkError // A regular multi-hop payment error that we'll need to // decrypt. @@ -1016,18 +1008,21 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { // selection process. This way we can return the error for // precise link that the sender selected, while optimistically // trying all links to utilize our available bandwidth. - linkErrs := make(map[lnwire.ShortChannelID]lnwire.FailureMessage) + linkErrs := make(map[lnwire.ShortChannelID]*LinkError) // Try to find destination channel link with appropriate // bandwidth. var destination ChannelLink for _, link := range interfaceLinks { - var failure lnwire.FailureMessage + var failure *LinkError // We'll skip any links that aren't yet eligible for // forwarding. if !link.EligibleToForward() { - failure = &lnwire.FailUnknownNextPeer{} + failure = NewDetailedLinkError( + &lnwire.FailUnknownNextPeer{}, + FailureDetailLinkNotEligible, + ) } else { // We'll ensure that the HTLC satisfies the // current forwarding conditions of this target @@ -1062,7 +1057,9 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { // If we can't find the error of the source, // then we'll return an unknown next peer, // though this should never happen. - linkErr = &lnwire.FailUnknownNextPeer{} + linkErr = NewLinkError( + &lnwire.FailUnknownNextPeer{}, + ) log.Warnf("unable to find err source for "+ "outgoing_link=%v, errors=%v", packet.outgoingChanID, newLogClosure(func() string { @@ -1075,7 +1072,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { htlc.PaymentHash[:], packet.outgoingChanID, linkErr) - return s.failAddPacket(packet, linkErr, addErr) + return s.failAddPacket(packet, linkErr.WireMessage(), addErr) } // Send the packet to the destination channel link which diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 28493c83..69425dc2 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -1290,7 +1290,7 @@ func TestSwitchForwardCircuitPersistence(t *testing.T) { type multiHopFwdTest struct { name string eligible1, eligible2 bool - failure1, failure2 lnwire.FailureMessage + failure1, failure2 *LinkError expectedReply lnwire.FailCode } @@ -1308,9 +1308,11 @@ func TestSkipIneligibleLinksMultiHopForward(t *testing.T) { // Channel one has a policy failure and the other channel isn't // available. { - name: "policy fail", - eligible1: true, - failure1: lnwire.NewFinalIncorrectCltvExpiry(0), + name: "policy fail", + eligible1: true, + failure1: NewLinkError( + lnwire.NewFinalIncorrectCltvExpiry(0), + ), expectedReply: lnwire.CodeFinalIncorrectCltvExpiry, }, @@ -1325,11 +1327,16 @@ func TestSkipIneligibleLinksMultiHopForward(t *testing.T) { // The requested channel has insufficient bandwidth and the // other channel's policy isn't satisfied. { - name: "non-strict policy fail", - eligible1: true, - failure1: lnwire.NewTemporaryChannelFailure(nil), - eligible2: true, - failure2: lnwire.NewFinalIncorrectCltvExpiry(0), + name: "non-strict policy fail", + eligible1: true, + failure1: NewDetailedLinkError( + lnwire.NewTemporaryChannelFailure(nil), + FailureDetailInsufficientBalance, + ), + eligible2: true, + failure2: NewLinkError( + lnwire.NewFinalIncorrectCltvExpiry(0), + ), expectedReply: lnwire.CodeTemporaryChannelFailure, }, } @@ -1482,7 +1489,9 @@ func testSkipLinkLocalForward(t *testing.T, eligible bool, aliceChannelLink := newMockChannelLink( s, chanID1, aliceChanID, alicePeer, eligible, ) - aliceChannelLink.checkHtlcTransitResult = policyResult + aliceChannelLink.checkHtlcTransitResult = NewLinkError( + policyResult, + ) if err := s.AddLink(aliceChannelLink); err != nil { t.Fatalf("unable to add alice link: %v", err) } @@ -2178,11 +2187,11 @@ func TestUpdateFailMalformedHTLCErrorConversion(t *testing.T) { t.Fatalf("unable to send payment: %v", err) } - fwdingErr := err.(*ForwardingError) - failureMsg := fwdingErr.FailureMessage + routingErr := err.(ClearTextError) + failureMsg := routingErr.WireMessage() if _, ok := failureMsg.(*lnwire.FailInvalidOnionKey); !ok { t.Fatalf("expected onion failure instead got: %v", - fwdingErr.FailureMessage) + routingErr.WireMessage()) } } @@ -2441,14 +2450,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.FailureMessage != 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 0a42e9fc..a1b7948c 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.FailureMessage.(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 d2f7448c..fac67aa9 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 463dfaca..8efa63e0 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1905,18 +1905,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.FailureMessage - 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, diff --git a/routing/router_test.go b/routing/router_test.go index 90255f60..b3470d1c 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 @@ -3324,7 +3320,7 @@ func TestSendToRouteStructuredError(t *testing.T) { t.Fatalf("expected forwarding error") } - if _, ok := fErr.FailureMessage.(*lnwire.FailFeeInsufficient); !ok { + if _, ok := fErr.WireMessage().(*lnwire.FailFeeInsufficient); !ok { t.Fatalf("expected fee insufficient error") }