From 025e569f07bca55dada71c2c21ba151ba832be9c Mon Sep 17 00:00:00 2001
From: Keagan McClelland <keagan.mcclelland@gmail.com>
Date: Sun, 26 Nov 2023 13:29:48 -0800
Subject: [PATCH] peer: fix local close requests to shutdown via link lifecycle
 hooks

In order to handle shutdown requests when there are still HTLCs on
the link, we have to manage the shutdown process via the link's
lifecycle hooks. This means we can't use the simple `tryLinkShutdown`
anymore and instead queue a `Shutdown` message at the next opportunity
to do so -- when we send our next `CommitSig`
---
 peer/brontide.go | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/peer/brontide.go b/peer/brontide.go
index 25499c75c..fa8b53cc1 100644
--- a/peer/brontide.go
+++ b/peer/brontide.go
@@ -2957,17 +2957,6 @@ func (p *Brontide) handleLocalCloseReq(req *htlcswitch.ChanClose) {
 			}
 		}
 
-		// Optimistically try a link shutdown, erroring out if it
-		// failed.
-		if err := p.tryLinkShutdown(chanID); err != nil {
-			p.log.Errorf("failed link shutdown: %v", err)
-
-			req.Err <- fmt.Errorf("failed handling co-op closing "+
-				"request with (try force closing "+
-				"it instead): %w", err)
-			return
-		}
-
 		chanCloser, err := p.createChanCloser(
 			channel, deliveryScript, req.TargetFeePerKw, req, true,
 		)
@@ -2994,7 +2983,28 @@ func (p *Brontide) handleLocalCloseReq(req *htlcswitch.ChanClose) {
 			return
 		}
 
-		p.queueMsg(shutdownMsg, nil)
+		link := p.fetchLinkFromKeyAndCid(chanID)
+		if link == nil {
+			// If the link is nil then it means it was already
+			// removed from the switch or it never existed in the
+			// first place. The latter case is handled at the
+			// beginning of this function, so in the case where it
+			// has already been removed, we can skip adding the
+			// commit hook to queue a Shutdown message.
+			p.log.Warnf("link not found during attempted closure: "+
+				"%v", chanID)
+			return
+		}
+
+		link.OnCommitOnce(htlcswitch.Outgoing, func() {
+			err := link.DisableAdds(htlcswitch.Outgoing)
+			if err != nil {
+				p.log.Warnf("outgoing link adds already "+
+					"disabled: %v", link.ChanID())
+			}
+
+			p.queueMsg(shutdownMsg, nil)
+		})
 
 	// A type of CloseBreach indicates that the counterparty has breached
 	// the channel therefore we need to clean up our local state.
@@ -3647,8 +3657,8 @@ func (p *Brontide) handleCloseMsg(msg *closeMsg) {
 		if link != nil {
 			err := link.DisableAdds(htlcswitch.Incoming)
 			if err != nil {
-				p.log.Warnf("incoming link adds already disabled: %v",
-					link.ChanID())
+				p.log.Warnf("incoming link adds already "+
+					"disabled: %v", link.ChanID())
 			}
 		}