From c986e52da74683d397c99a28464336a1ab3ecd6f Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Sun, 26 Nov 2017 13:25:26 -0600
Subject: [PATCH] funding+server: ensure we cancel all reservations when a peer
 disconnects
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In this commit, we fix an existing issue that could at times cause an
inconsistent view between the set of total coins, and the set of segwit
coins in the wallet of the node. This could be caused by initiating a
funding flow, but then the funding negotiation breaking down somewhere
along the lines. In this case, us or the other peer will disconnect.
When we initiate funding flows, we lock coins exclusively, to ensure
that concurrent funding flows don’t end up double spending the same
coin. Before this commit, we wouldn’t ever unlock those coins. As a
result, our view of available coins would be skewed.

The walletbalance call would show all the coins, but when adding the
—witness_only flag, some coins would be missing, or gone all together.
This is because the former call actually scans the txstore and manually
tallies the amount of available coins, while the latter looks at the
sent of available outputs, which is filtered based on which coins are
locked.

To remedy this, we now ensure that when a peer disconnects, we wipe all
existing reservations which will return any locked outputs to the set
of available outputs for funding flows.
---
 fundingmanager.go | 39 +++++++++++++++++++++++++++++++++++++--
 server.go         |  6 ++++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/fundingmanager.go b/fundingmanager.go
index d2c670c0..e5cf8b24 100644
--- a/fundingmanager.go
+++ b/fundingmanager.go
@@ -575,6 +575,41 @@ func (f *fundingManager) PendingChannels() ([]*pendingChannel, error) {
 	return <-respChan, <-errChan
 }
 
+// CancelPeerReservations cancels all active reservations associated with the
+// passed node. This will ensure any outputs which have been pre committed,
+// (and thus locked from coin selection), are properly freed.
+func (f *fundingManager) CancelPeerReservations(nodePub [33]byte) {
+
+	fndgLog.Debugf("Cancelling all reservations for peer %x", nodePub[:])
+
+	f.resMtx.Lock()
+	defer f.resMtx.Unlock()
+
+	// We'll attempt to look up this node in the set of active
+	// reservations.  If they don't have any, then there's no further work
+	// to be done.
+	nodeReservations, ok := f.activeReservations[nodePub]
+	if !ok {
+		fndgLog.Debugf("No active reservations for node: %x", nodePub[:])
+		return
+	}
+
+	// If they do have any active reservations, then we'll cancel all of
+	// them (which releases any locked UTXO's), and also delete it from the
+	// reservation map.
+	for pendingID, resCtx := range nodeReservations {
+		if err := resCtx.reservation.Cancel(); err != nil {
+			fndgLog.Errorf("unable to cancel reservation for "+
+				"node=%x: %v", nodePub[:], err)
+		}
+
+		delete(nodeReservations, pendingID)
+	}
+
+	// Finally, we'll delete the node itself from the set of reservations.
+	delete(f.activeReservations, nodePub)
+}
+
 // failFundingFlow will fail the active funding flow with the target peer,
 // identified by it's unique temporary channel ID. This method is send an error
 // to the remote peer, and also remove the reservation from our set of pending
@@ -1089,8 +1124,8 @@ func (f *fundingManager) handleFundingCreated(fmsg *fundingCreatedMsg) {
 	f.newChanBarriers[channelID] = make(chan struct{})
 	f.barrierMtx.Unlock()
 
-	fndgLog.Infof("sending signComplete for pendingID(%x) over ChannelPoint(%v)",
-		pendingChanID[:], fundingOut)
+	fndgLog.Infof("sending FundingSigned for pendingID(%x) over "+
+		"ChannelPoint(%v)", pendingChanID[:], fundingOut)
 
 	// With their signature for our version of the commitment transaction
 	// verified, we can now send over our signature to the remote peer.
diff --git a/server.go b/server.go
index 994f4e05..b2553703 100644
--- a/server.go
+++ b/server.go
@@ -1036,6 +1036,12 @@ func (s *server) peerTerminationWatcher(p *peer) {
 		return
 	}
 
+	// Next, we'll cancel all pending funding reservations with this node.
+	// If we tried to initiate any funding flows that haven't yet finished,
+	// then we need to unlock those committed outputs so they're still
+	// available for use.
+	s.fundingMgr.CancelPeerReservations(p.PubKey())
+
 	// Tell the switch to remove all links associated with this peer.
 	// Passing nil as the target link indicates that all links associated
 	// with this interface should be closed.