From 0d4ee08372aaeee42f4baffd2dc5200e2b42f388 Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Thu, 3 Aug 2017 16:57:56 -0700
Subject: [PATCH] htlcswitch: asynchronously handle channel close requests

This commit modifies how the htlcswitch handles close requests.
Previously it could be the case that a new channel was added, but at
the same time a channel was requested to be closed. This would result
in a circular waiting dependency: the peer contacts the switch, who
tries to contact the peer.

We eliminate this possibility by ensuring that the switch handles all
close requests asynchronously. With this, the switch won't block
indefinitely in the scenario described above.
---
 htlcswitch/switch.go | 50 ++++++++++++++++----------------------------
 1 file changed, 18 insertions(+), 32 deletions(-)

diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go
index 201819404..3ef4aaa83 100644
--- a/htlcswitch/switch.go
+++ b/htlcswitch/switch.go
@@ -149,7 +149,7 @@ type Switch struct {
 	// the channel close handler.
 	chanCloseRequests chan *ChanClose
 
-	// linkControl is a channel used to propogate add/remove/get htlc
+	// linkControl is a channel used to propagate add/remove/get htlc
 	// switch handler commands.
 	linkControl chan interface{}
 }
@@ -512,7 +512,9 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
 
 		// If this is failure than we need to obfuscate the error.
 		if htlc, ok := htlc.(*lnwire.UpdateFailHTLC); ok && !packet.isObfuscated {
-			htlc.Reason = circuit.Obfuscator.BackwardObfuscate(htlc.Reason)
+			htlc.Reason = circuit.Obfuscator.BackwardObfuscate(
+				htlc.Reason,
+			)
 		}
 
 		// Propagating settle/fail htlc back to src of add htlc packet.
@@ -542,7 +544,7 @@ func (s *Switch) CloseLink(chanPoint *wire.OutPoint,
 	closeType ChannelCloseType) (chan *lnrpc.CloseStatusUpdate, chan error) {
 
 	// TODO(roasbeef) abstract out the close updates.
-	updateChan := make(chan *lnrpc.CloseStatusUpdate, 1)
+	updateChan := make(chan *lnrpc.CloseStatusUpdate, 2)
 	errChan := make(chan error, 1)
 
 	command := &ChanClose{
@@ -564,34 +566,6 @@ func (s *Switch) CloseLink(chanPoint *wire.OutPoint,
 	}
 }
 
-// handleCloseLink sends a message to the peer responsible for the target
-// channel point, instructing it to initiate a cooperative channel closure.
-func (s *Switch) handleChanelClose(req *ChanClose) {
-	targetChanID := lnwire.NewChanIDFromOutPoint(req.ChanPoint)
-
-	var link ChannelLink
-	for chanID, l := range s.linkIndex {
-		if chanID == targetChanID {
-			link = l
-		}
-	}
-
-	if link == nil {
-		req.Err <- errors.Errorf("channel with ChannelID(%v) not "+
-			"found", targetChanID)
-		return
-	}
-
-	log.Debugf("requesting local channel close, peer(%v) channel(%v)",
-		link.Peer(), targetChanID)
-
-	// TODO(roasbeef): if type was CloseBreach initiate force closure with
-	// all other channels (if any) we have with the remote peer.
-	peerPub := link.Peer().PubKey()
-	s.cfg.LocalChannelClose(peerPub[:], req)
-	return
-}
-
 // htlcForwarder is responsible for optimally forwarding (and possibly
 // fragmenting) incoming/outgoing HTLCs amongst all active interfaces and their
 // links. The duties of the forwarder are similar to that of a network switch,
@@ -630,7 +604,19 @@ func (s *Switch) htlcForwarder() {
 		// relevant link (if it exists) so the channel can be
 		// cooperatively closed (if possible).
 		case req := <-s.chanCloseRequests:
-			s.handleChanelClose(req)
+			chanID := lnwire.NewChanIDFromOutPoint(req.ChanPoint)
+			link, ok := s.linkIndex[chanID]
+			if !ok {
+				req.Err <- errors.Errorf("channel with "+
+					"chan_id=%v not found", chanID[:])
+				continue
+			}
+
+			peerPub := link.Peer().PubKey()
+			log.Debugf("Requesting local channel close: peer=%v, "+
+				"chan_id=%x", link.Peer(), chanID[:])
+
+			go s.cfg.LocalChannelClose(peerPub[:], req)
 
 		// A new packet has arrived for forwarding, we'll interpret the
 		// packet concretely, then either forward it along, or