diff --git a/htlcswitch/failure.go b/htlcswitch/failure.go index 9734a2e0..37326338 100644 --- a/htlcswitch/failure.go +++ b/htlcswitch/failure.go @@ -76,7 +76,7 @@ func (l *LinkError) Error() string { return l.msg.Error() } - return fmt.Sprintf("%v: %v", l.msg.Error(), l.FailureDetail) + return l.FailureDetail.FailureString() } // ForwardingError wraps an lnwire.FailureMessage in a struct that also diff --git a/htlcswitch/failure_detail.go b/htlcswitch/failure_detail.go index 9c068ceb..341688d1 100644 --- a/htlcswitch/failure_detail.go +++ b/htlcswitch/failure_detail.go @@ -42,6 +42,18 @@ const ( // to forward a htlc through our node which arrives and leaves on the // same channel. OutgoingFailureCircularRoute + + // OutgoingFailureIncompleteForward is returned when we cancel an incomplete + // forward. + OutgoingFailureIncompleteForward + + // OutgoingFailureDownstreamHtlcAdd is returned when we fail to add a + // downstream htlc to our outgoing link. + OutgoingFailureDownstreamHtlcAdd + + // OutgoingFailureForwardsDisabled is returned when the switch is + // configured to disallow forwards. + OutgoingFailureForwardsDisabled ) // FailureString returns the string representation of a failure detail. @@ -70,6 +82,15 @@ func (fd OutgoingFailure) FailureString() string { case OutgoingFailureCircularRoute: return "same incoming and outgoing channel" + case OutgoingFailureIncompleteForward: + return "failed after detecting incomplete forward" + + case OutgoingFailureDownstreamHtlcAdd: + return "could not add downstream htlc" + + case OutgoingFailureForwardsDisabled: + return "node configured to disallow forwards" + default: return "unknown failure detail" } diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 94773f2c..484d0e07 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -1314,6 +1314,10 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) { reason lnwire.OpaqueReason ) + // Create a temporary channel failure which we + // will send back to our peer if this is a + // forward, or report to the user if the failed + // payment was locally initiated. failure := l.createFailureWithUpdate( func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage { return lnwire.NewTemporaryChannelFailure( @@ -1322,28 +1326,43 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) { }, ) - // Encrypt the error back to the source unless - // the payment was generated locally. + // If the payment was locally initiated (which + // is indicated by a nil obfuscator), we do + // not need to encrypt it back to the sender. if pkt.obfuscator == nil { var b bytes.Buffer err := lnwire.EncodeFailure(&b, failure, 0) if err != nil { - l.log.Errorf("unable to encode failure: %v", err) + l.log.Errorf("unable to "+ + "encode failure: %v", err) l.mailBox.AckPacket(pkt.inKey()) return } reason = lnwire.OpaqueReason(b.Bytes()) localFailure = true } else { + // If the packet is part of a forward, + // (identified by a non-nil obfuscator) + // we need to encrypt the error back to + // the source. var err error reason, err = pkt.obfuscator.EncryptFirstHop(failure) if err != nil { - l.log.Errorf("unable to obfuscate error: %v", err) + l.log.Errorf("unable to "+ + "obfuscate error: %v", err) l.mailBox.AckPacket(pkt.inKey()) return } } + // Create a link error containing the temporary + // channel failure and a detail which indicates + // the we failed to add the htlc. + linkError := NewDetailedLinkError( + failure, + OutgoingFailureDownstreamHtlcAdd, + ) + failPkt := &htlcPacket{ incomingChanID: pkt.incomingChanID, incomingHTLCID: pkt.incomingHTLCID, @@ -1351,6 +1370,7 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) { sourceRef: pkt.sourceRef, hasSource: true, localFailure: localFailure, + linkFailure: linkError, htlc: &lnwire.UpdateFailHTLC{ Reason: reason, }, @@ -2461,7 +2481,9 @@ func (l *channelLink) processRemoteSettleFails(fwdPkg *channeldb.FwdPkg, } // Fetch the reason the HTLC was canceled so we can - // continue to propagate it. + // continue to propagate it. This failure originated + // from another node, so the linkFailure field is not + // set on the packet. failPacket := &htlcPacket{ outgoingChanID: l.ShortChanID(), outgoingHTLCID: pd.ParentIndex, diff --git a/htlcswitch/packet.go b/htlcswitch/packet.go index 3e0816e6..e0aa7527 100644 --- a/htlcswitch/packet.go +++ b/htlcswitch/packet.go @@ -54,6 +54,11 @@ type htlcPacket struct { // encrypted with any shared secret. localFailure bool + // linkFailure is non-nil for htlcs that fail at our node. This may + // occur for our own payments which fail on the outgoing link, + // or for forwards which fail in the switch or on the outgoing link. + linkFailure *LinkError + // convertedError is set to true if this is an HTLC fail that was // created using an UpdateFailMalformedHTLC from the remote party. If // this is true, then when forwarding this failure packet, we'll need diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index 7ca56223..f4370635 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -45,11 +45,6 @@ var ( // through the switch and is locked into another commitment txn. ErrDuplicateAdd = errors.New("duplicate add HTLC detected") - // ErrIncompleteForward is used when an htlc was already forwarded - // through the switch, but did not get locked into another commitment - // txn. - ErrIncompleteForward = errors.New("incomplete forward detected") - // ErrUnknownErrorDecryptor signals that we were unable to locate the // error decryptor for this payment. This is likely due to restarting // the daemon. @@ -496,9 +491,12 @@ func (s *Switch) forward(packet *htlcPacket) error { } else { failure = lnwire.NewTemporaryChannelFailure(update) } - addErr := ErrIncompleteForward - return s.failAddPacket(packet, failure, addErr) + linkError := NewDetailedLinkError( + failure, OutgoingFailureIncompleteForward, + ) + + return s.failAddPacket(packet, linkError) } packet.circuit = circuit @@ -647,14 +645,14 @@ func (s *Switch) ForwardPackets(linkQuit chan struct{}, } else { failure = lnwire.NewTemporaryChannelFailure(update) } + linkError := NewDetailedLinkError( + failure, OutgoingFailureIncompleteForward, + ) for _, packet := range failedPackets { - addErr := errors.New("failing packet after " + - "detecting incomplete forward") - // We don't handle the error here since this method // always returns an error. - s.failAddPacket(packet, failure, addErr) + _ = s.failAddPacket(packet, linkError) } } @@ -979,10 +977,12 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { // Check if the node is set to reject all onward HTLCs and also make // sure that HTLC is not from the source node. if s.cfg.RejectHTLC && packet.incomingChanID != hop.Source { - failure := &lnwire.FailChannelDisabled{} - addErr := fmt.Errorf("unable to forward any htlcs") + failure := NewDetailedLinkError( + &lnwire.FailChannelDisabled{}, + OutgoingFailureForwardsDisabled, + ) - return s.failAddPacket(packet, failure, addErr) + return s.failAddPacket(packet, failure) } if packet.incomingChanID == hop.Source { @@ -1002,9 +1002,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { s.cfg.AllowCircularRoute, htlc.PaymentHash, ) if linkErr != nil { - return s.failAddPacket( - packet, linkErr.WireMessage(), linkErr, - ) + return s.failAddPacket(packet, linkErr) } s.indexMtx.RLock() @@ -1012,14 +1010,17 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { if err != nil { s.indexMtx.RUnlock() + log.Debugf("unable to find link with "+ + "destination %v", packet.outgoingChanID) + // If packet was forwarded from another channel link // than we should notify this link that some error // occurred. - failure := &lnwire.FailUnknownNextPeer{} - addErr := fmt.Errorf("unable to find link with "+ - "destination %v", packet.outgoingChanID) + linkError := NewLinkError( + &lnwire.FailUnknownNextPeer{}, + ) - return s.failAddPacket(packet, failure, addErr) + return s.failAddPacket(packet, linkError) } targetPeerKey := targetLink.Peer().PubKey() interfaceLinks, _ := s.getLinks(targetPeerKey) @@ -1088,12 +1089,12 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { })) } - addErr := fmt.Errorf("incoming HTLC(%x) violated "+ + log.Tracef("incoming HTLC(%x) violated "+ "target outgoing link (id=%v) policy: %v", htlc.PaymentHash[:], packet.outgoingChanID, linkErr) - return s.failAddPacket(packet, linkErr.WireMessage(), addErr) + return s.failAddPacket(packet, linkErr) } // Send the packet to the destination channel link which @@ -1225,13 +1226,11 @@ func checkCircularForward(incoming, outgoing lnwire.ShortChannelID, // failAddPacket encrypts a fail packet back to an add packet's source. // The ciphertext will be derived from the failure message proivded by context. // This method returns the failErr if all other steps complete successfully. -func (s *Switch) failAddPacket(packet *htlcPacket, - failure lnwire.FailureMessage, failErr error) error { - +func (s *Switch) failAddPacket(packet *htlcPacket, failure *LinkError) error { // Encrypt the failure so that the sender will be able to read the error // message. Since we failed this packet, we use EncryptFirstHop to // obfuscate the failure for their eyes only. - reason, err := packet.obfuscator.EncryptFirstHop(failure) + reason, err := packet.obfuscator.EncryptFirstHop(failure.WireMessage()) if err != nil { err := fmt.Errorf("unable to obfuscate "+ "error: %v", err) @@ -1239,13 +1238,14 @@ func (s *Switch) failAddPacket(packet *htlcPacket, return err } - log.Error(failErr) + log.Error(failure.Error()) failPkt := &htlcPacket{ sourceRef: packet.sourceRef, incomingChanID: packet.incomingChanID, incomingHTLCID: packet.incomingHTLCID, circuit: packet.circuit, + linkFailure: failure, htlc: &lnwire.UpdateFailHTLC{ Reason: reason, }, @@ -1261,7 +1261,7 @@ func (s *Switch) failAddPacket(packet *htlcPacket, return err } - return failErr + return failure } // closeCircuit accepts a settle or fail htlc and the associated htlc packet and @@ -1869,8 +1869,11 @@ func (s *Switch) reforwardSettleFails(fwdPkgs []*channeldb.FwdPkg) { // commitment state, so we'll forward this to the switch so the // backwards undo can continue. case lnwallet.Fail: - // Fetch the reason the HTLC was canceled so we can - // continue to propagate it. + // Fetch the reason the HTLC was canceled so + // we can continue to propagate it. This + // failure originated from another node, so + // the linkFailure field is not set on this + // packet. failPacket := &htlcPacket{ outgoingChanID: fwdPkg.Source, outgoingHTLCID: pd.ParentIndex, diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index a098967b..adc19649 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -211,10 +211,13 @@ func TestSwitchSendPending(t *testing.T) { // Send the ADD packet, this should not be forwarded out to the link // since there are no eligible links. err = s.forward(packet) - expErr := fmt.Sprintf("unable to find link with destination %v", - aliceChanID) - if err != nil && err.Error() != expErr { - t.Fatalf("expected forward failure: %v", err) + linkErr, ok := err.(*LinkError) + if !ok { + t.Fatalf("expected link error, got: %T", err) + } + if linkErr.WireMessage().Code() != lnwire.CodeUnknownNextPeer { + t.Fatalf("expected fail unknown next peer, got: %T", + linkErr.WireMessage().Code()) } // No message should be sent, since the packet was failed. @@ -1070,9 +1073,13 @@ func TestSwitchForwardFailAfterHalfAdd(t *testing.T) { // Resend the failed htlc, it should be returned to alice since the // switch will detect that it has been half added previously. err = s2.forward(ogPacket) - if err != ErrIncompleteForward { - t.Fatal("unexpected error when reforwarding a "+ - "failed packet", err) + linkErr, ok := err.(*LinkError) + if !ok { + t.Fatalf("expected link error, got: %T", err) + } + if linkErr.FailureDetail != OutgoingFailureIncompleteForward { + t.Fatalf("expected incomplete forward, got: %v", + linkErr.FailureDetail) } // After detecting an incomplete forward, the fail packet should have