From 7ae436e30e5958a144d004ef1f958a8e04743f44 Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Mon, 25 Sep 2017 16:09:48 -0700
Subject: [PATCH] htlcswitch+test: send switch back error on
 lnwallet.ErrInsufficientBalance
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit fixes a bug related to swallowing an error that should go
to the switch in the case of an insufficient balance error when
attempting to add a new HTLC to the channel state machine. In this
case, an error would never be returned back to the client/switch, and
the internal processing within the channelLink would loop forever,
attempting to add an HTLC that can’t be added due to insufficient
balance to state machine itself.

We fix this issue by only treating the lnwallet.ErrMaxHTLCNumber as the
only error that prompts adding an HTLC to the overflow queue rather
than sending the error directly back to the switch.
---
 htlcswitch/link.go   |  4 ++--
 htlcswitch/switch.go |  2 ++
 lnd_test.go          | 20 ++++++++------------
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/htlcswitch/link.go b/htlcswitch/link.go
index 8eca6de7..bd3bdf69 100644
--- a/htlcswitch/link.go
+++ b/htlcswitch/link.go
@@ -483,6 +483,8 @@ out:
 // Switch. Possible messages sent by the switch include requests to forward new
 // HTLCs, timeout previously cleared HTLCs, and finally to settle currently
 // cleared HTLCs with the upstream peer.
+//
+// TODO(roasbeef): add sync ntfn to ensure switch always has consistent view?
 func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) {
 	var isSettle bool
 	switch htlc := pkt.htlc.(type) {
@@ -497,8 +499,6 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) {
 
 			// The channels spare bandwidth is fully allocated, so
 			// we'll put this HTLC into the overflow queue.
-			case lnwallet.ErrInsufficientBalance:
-				fallthrough
 			case lnwallet.ErrMaxHTLCNumber:
 				log.Infof("Downstream htlc add update with "+
 					"payment hash(%x) have been added to "+
diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go
index 4c74ecfc..f1c99c62 100644
--- a/htlcswitch/switch.go
+++ b/htlcswitch/switch.go
@@ -389,6 +389,8 @@ func (s *Switch) handleLocalDispatch(payment *pendingPayment, packet *htlcPacket
 		// we're unable to then we'll bail early.
 		failure, err := payment.deobfuscator.Deobfuscate(htlc.Reason)
 		if err != nil {
+			// TODO(roasbeef): can happen in case of local error in
+			// link pkt handling
 			userErr = errors.Errorf("unable to de-obfuscate "+
 				"onion failure, htlc with hash(%x): %v",
 				payment.paymentHash[:], err)
diff --git a/lnd_test.go b/lnd_test.go
index f14119ee..e30d5a93 100644
--- a/lnd_test.go
+++ b/lnd_test.go
@@ -3133,25 +3133,21 @@ func testAsyncPayments(net *networkHarness, t *harnessTest) {
 		}
 	}
 
-	// We should receive one insufficient capacity error, because we are
-	// sending on one invoice bigger.
+	// We should receive one insufficient capacity error, because we sent
+	// one more payment than we can actually handle with the current
+	// channel capacity.
 	errorReceived := false
 	for i := 0; i < numInvoices; i++ {
 		if resp, err := alicePayStream.Recv(); err != nil {
 			t.Fatalf("payment stream have been closed: %v", err)
 		} else if resp.PaymentError != "" {
-			if strings.Contains(resp.PaymentError,
-				lnwire.CodeTemporaryChannelFailure.String()) {
-				if errorReceived {
-					t.Fatalf("redundant payment "+
-						"error: %v", resp.PaymentError)
-				}
-
-				errorReceived = true
-				continue
+			if errorReceived {
+				t.Fatalf("redundant payment error: %v",
+					resp.PaymentError)
 			}
 
-			t.Fatalf("unable to send payment: %v", resp.PaymentError)
+			errorReceived = true
+			continue
 		}
 	}