From bd775b9bb38994af0c15ed198731f8fae9e92e4f Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Tue, 21 Feb 2017 22:12:46 -0800
Subject: [PATCH] lnwallet: ensure reservation state is cleaned up in case of
 Cancel
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit fixes a bug in the wallet’s internal reservation manager
that prevented it from cleaning up the resources used by a reservation
after it was finished running through the workflow.

We fix this issue by ensuring the reservations context is deleted from
the funding limbo.

It is the callers responsibility to properly .Cancel() a reservation in
the case of an error during the funding workflow.
---
 lnwallet/interface_test.go | 13 +++++++++++++
 lnwallet/wallet.go         | 12 ++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go
index 6b8eb21ad..29c1b5603 100644
--- a/lnwallet/interface_test.go
+++ b/lnwallet/interface_test.go
@@ -96,6 +96,12 @@ func assertChannelOpen(t *testing.T, miner *rpctest.Harness, numConfs uint32,
 	}
 }
 
+func assertReservationDeleted(res *lnwallet.ChannelReservation, t *testing.T) {
+	if err := res.Cancel(); err == nil {
+		t.Fatalf("reservation wasn't deleted from wallet")
+	}
+}
+
 // bobNode represents the other party involved as a node within LN. Bob is our
 // only "default-route", we have a direct connection with him.
 type bobNode struct {
@@ -526,6 +532,10 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness, wallet *lnwallet
 	if err := wallet.PublishTransaction(bobCloseTx); err != nil {
 		t.Fatalf("broadcast of close tx rejected: %v", err)
 	}
+
+	// Now that the reservation has conclued, ensure that the wallet has
+	// cleaned up the state allocated to the reservation.
+	assertReservationDeleted(chanReservation, t)
 }
 
 func testFundingTransactionLockedOutputs(miner *rpctest.Harness,
@@ -774,6 +784,8 @@ func testSingleFunderReservationWorkflowInitiator(miner *rpctest.Harness,
 		lnChan <- openDetails.Channel
 	}()
 	assertChannelOpen(t, miner, uint32(numReqConfs), lnChan)
+
+	assertReservationDeleted(chanReservation, t)
 }
 
 func testSingleFunderReservationWorkflowResponder(miner *rpctest.Harness,
@@ -934,6 +946,7 @@ func testSingleFunderReservationWorkflowResponder(miner *rpctest.Harness,
 	}
 
 	// TODO(roasbeef): bob verify alice's sig
+	assertReservationDeleted(chanReservation, t)
 }
 
 func testListTransactionDetails(miner *rpctest.Harness, wallet *lnwallet.LightningWallet, t *testing.T) {
diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go
index 70f898868..f7f21b095 100644
--- a/lnwallet/wallet.go
+++ b/lnwallet/wallet.go
@@ -1242,6 +1242,12 @@ func (l *LightningWallet) handleChannelOpen(req *channelOpenMsg) {
 	res.chanOpen <- &openChanDetails{
 		channel: channel,
 	}
+
+	// As this reservation has concluded, we can now safely remove the
+	// reservation from the limbo map.
+	l.limboMtx.Lock()
+	delete(l.fundingLimbo, req.pendingFundingID)
+	l.limboMtx.Unlock()
 }
 
 // openChannelAfterConfirmations creates, and opens a payment channel after
@@ -1299,6 +1305,12 @@ out:
 		blockHeight: confDetails.BlockHeight,
 		txIndex:     confDetails.TxIndex,
 	}
+
+	// As this reservation has concluded, we can now safely remove the
+	// reservation from the limbo map.
+	l.limboMtx.Lock()
+	delete(l.fundingLimbo, res.reservationID)
+	l.limboMtx.Unlock()
 }
 
 // selectCoinsAndChange performs coin selection in order to obtain witness