diff --git a/htlcswitch/failure.go b/htlcswitch/failure.go index 20ba6d018..f3aa6ad2f 100644 --- a/htlcswitch/failure.go +++ b/htlcswitch/failure.go @@ -33,6 +33,9 @@ type LinkError struct { // 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. @@ -42,6 +45,17 @@ 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, @@ -56,7 +70,13 @@ func (l *LinkError) WireMessage() lnwire.FailureMessage { // // Note this is part of the ClearTextError interface. func (l *LinkError) Error() string { - return l.msg.Error() + // 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 diff --git a/htlcswitch/failure_detail.go b/htlcswitch/failure_detail.go new file mode 100644 index 000000000..92b25510e --- /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 bb34b283f..1281e9515 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 0bba3f656..92ecba65c 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 16bd92e7d..16d64d70e 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -5541,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") } }) @@ -5549,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") } }) @@ -5557,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") } }) @@ -5565,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") } }) @@ -5573,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") } @@ -5583,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") } }) diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index 828c36b92..e39c60732 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -645,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 @@ -707,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 74276ea8c..c08bdc0cf 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -756,20 +756,19 @@ func (s *Switch) handleLocalDispatch(pkt *htlcPacket) error { s.indexMtx.RUnlock() if err != nil { log.Errorf("Link %v not found", pkt.outgoingChanID) - return NewForwardingError( - &lnwire.FailUnknownNextPeer{}, 0, "", - ) + 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 NewForwardingError(htlcErr, 0, err.Error()) + return NewDetailedLinkError( + lnwire.NewTemporaryChannelFailure(nil), + FailureDetailLinkNotEligible, + ) } // Ensure that the htlc satisfies the outgoing channel policy. @@ -782,8 +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 NewForwardingError(htlcErr, 0, "") + return htlcErr } return link.HandleSwitchPacket(pkt) @@ -907,35 +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 NewForwardingError(failureMsg, 0, userErr) + // 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 NewForwardingError( - &lnwire.FailPermanentChannelFailure{}, 0, userErr, - ) + return linkError // A regular multi-hop payment error that we'll need to // decrypt. @@ -1002,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 @@ -1048,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 { @@ -1061,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 0f6ab9e01..69425dc21 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) }