htlcswitch: fix returned failure for insufficient balance

In the scenario where the requested channel does not have enough balance
and another channel towards the same node generates a different failure,
we erroneously returned UnknownNextPeer instead of the expected
TemporaryChannelFailure.

This commit rewrites the non-strict forwarding logic in the switch to
return the proper failure message. Part of this is moving the link
balance check inside the link.

htlcswitch/link.go

@@ -2341,6 +2341,15 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy,
 		return &lnwire.FailExpiryTooFar{}
 	}
 
+	// Check to see if there is enough balance in this channel.
+	if amt > l.Bandwidth() {
+		return l.createFailureWithUpdate(
+			func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage {
+				return lnwire.NewTemporaryChannelFailure(upd)
+			},
+		)
+	}
+
 	return nil
 }
 

htlcswitch/link_test.go

@@ -5406,6 +5406,15 @@ func TestCheckHtlcForward(t *testing.T) {
 		return &lnwire.ChannelUpdate{}, nil
 	}
 
+	testChannel, _, fCleanUp, err := createTestChannel(
+		alicePrivKey, bobPrivKey, 100000, 100000,
+		1000, 1000, lnwire.ShortChannelID{},
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer fCleanUp()
+
 	link := channelLink{
 		cfg: ChannelLinkConfig{
 			FwrdingPolicy: ForwardingPolicy{
@@ -5417,7 +5426,9 @@ func TestCheckHtlcForward(t *testing.T) {
 			FetchLastChannelUpdate: fetchLastChannelUpdate,
 			MaxOutgoingCltvExpiry:  DefaultMaxOutgoingCltvExpiry,
 		},
-		log: log,
+		log:           log,
+		channel:       testChannel.channel,
+		overflowQueue: newPacketQueue(input.MaxHTLCNumber / 2),
 	}
 
 	var hash [32]byte

htlcswitch/switch.go

@@ -790,22 +790,6 @@ func (s *Switch) handleLocalDispatch(pkt *htlcPacket) error {
 			}
 		}
 
-		if link.Bandwidth() < htlc.Amount {
-			err := fmt.Errorf("Link %v has insufficient capacity: "+
-				"need %v, has %v", pkt.outgoingChanID,
-				htlc.Amount, link.Bandwidth())
-			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 link.HandleSwitchPacket(pkt)
 	}
 
@@ -1034,63 +1018,38 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
 		// bandwidth.
 		var destination ChannelLink
 		for _, link := range interfaceLinks {
+			var failure lnwire.FailureMessage
+
 			// We'll skip any links that aren't yet eligible for
 			// forwarding.
 			if !link.EligibleToForward() {
-				continue
+				failure = &lnwire.FailUnknownNextPeer{}
+			} else {
+				// We'll ensure that the HTLC satisfies the
+				// current forwarding conditions of this target
+				// link.
+				currentHeight := atomic.LoadUint32(&s.bestHeight)
+				failure = link.CheckHtlcForward(
+					htlc.PaymentHash, packet.incomingAmount,
+					packet.amount, packet.incomingTimeout,
+					packet.outgoingTimeout, currentHeight,
+				)
 			}
 
-			// Before we check the link's bandwidth, we'll ensure
-			// that the HTLC satisfies the current forwarding
-			// policy of this target link.
-			currentHeight := atomic.LoadUint32(&s.bestHeight)
-			err := link.CheckHtlcForward(
-				htlc.PaymentHash, packet.incomingAmount,
-				packet.amount, packet.incomingTimeout,
-				packet.outgoingTimeout, currentHeight,
-			)
-			if err != nil {
-				linkErrs[link.ShortChanID()] = err
-				continue
-			}
-
-			if link.Bandwidth() >= htlc.Amount {
+			// Stop searching if this link can forward the htlc.
+			if failure == nil {
 				destination = link
-
 				break
 			}
+
+			linkErrs[link.ShortChanID()] = failure
 		}
 
-		switch {
-		// If the channel link we're attempting to forward the update
-		// over has insufficient capacity, and didn't violate any
-		// forwarding policies, then we'll cancel the htlc as the
-		// payment cannot succeed.
-		case destination == nil && len(linkErrs) == 0:
-			// If packet was forwarded from another channel link
-			// than we should notify this link that some error
-			// occurred.
-			var failure lnwire.FailureMessage
-			update, err := s.cfg.FetchLastChannelUpdate(
-				packet.outgoingChanID,
-			)
-			if err != nil {
-				failure = &lnwire.FailTemporaryNodeFailure{}
-			} else {
-				failure = lnwire.NewTemporaryChannelFailure(update)
-			}
-
-			addErr := fmt.Errorf("unable to find appropriate "+
-				"channel link insufficient capacity, need "+
-				"%v towards node=%x", htlc.Amount, targetPeerKey)
-
-			return s.failAddPacket(packet, failure, addErr)
-
 		// If we had a forwarding failure due to the HTLC not
 		// satisfying the current policy, then we'll send back an
 		// error, but ensure we send back the error sourced at the
 		// *target* link.
-		case destination == nil && len(linkErrs) != 0:
+		if destination == nil {
 			// At this point, some or all of the links rejected the
 			// HTLC so we couldn't forward it. So we'll try to look
 			// up the error that came from the source.

htlcswitch/switch_test.go

@@ -1299,10 +1299,10 @@ type multiHopFwdTest struct {
 // to forward any HTLC's.
 func TestSkipIneligibleLinksMultiHopForward(t *testing.T) {
 	tests := []multiHopFwdTest{
-		// None of the channels is eligible or has enough bandwidth.
+		// None of the channels is eligible.
 		{
 			name:          "not eligible",
-			expectedReply: lnwire.CodeTemporaryChannelFailure,
+			expectedReply: lnwire.CodeUnknownNextPeer,
 		},
 
 		// Channel one has a policy failure and the other channel isn't
@@ -1314,25 +1314,23 @@ func TestSkipIneligibleLinksMultiHopForward(t *testing.T) {
 			expectedReply: lnwire.CodeFinalIncorrectCltvExpiry,
 		},
 
-		// The requested channel is not eligible or has insufficient
-		// bandwidth, but the packet is forwarded through the other
-		// channel.
+		// The requested channel is not eligible, but the packet is
+		// forwarded through the other channel.
 		{
 			name:          "non-strict success",
 			eligible2:     true,
 			expectedReply: lnwire.CodeNone,
 		},
 
-		// The requested channel is not eligible or has insufficient
-		// bandwidth and the other channel's policy isn't satisfied.
-		//
-		// NOTE: We expect a temporary channel failure here, but don't
-		// receive it!
+		// 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),
-			expectedReply: lnwire.CodeUnknownNextPeer,
+			expectedReply: lnwire.CodeTemporaryChannelFailure,
 		},
 	}