From d71a4ee03342fe34222353a1d81ec0df4daef8b8 Mon Sep 17 00:00:00 2001
From: Matheus Degiovani <opensource@matheusd.com>
Date: Wed, 8 Jan 2020 09:06:59 -0300
Subject: [PATCH] contractcourt+itest: fix lingering pending channel after
 breach

This fixes an issue where the contract court could leave a completely
swept commit tx unresolved if it was swept by the remote party.

This could happen if (our) commit tx just published was actually a
previously revoked state, in which case the remote party would claim the
funds via a justice transaction.

This manifested itself in the testRevokedCloseRetribution integration
test where at the end of the test Bob was left with a pending channel
that never resolved itself.
---
 contractcourt/commit_sweep_resolver.go | 26 +++++++++++++++++++++-----
 lntest/itest/lnd_test.go               |  9 +++++++++
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/contractcourt/commit_sweep_resolver.go b/contractcourt/commit_sweep_resolver.go
index fe09dc09f..c184010a1 100644
--- a/contractcourt/commit_sweep_resolver.go
+++ b/contractcourt/commit_sweep_resolver.go
@@ -239,24 +239,40 @@ func (c *commitSweepResolver) Resolve() (ContractResolver, error) {
 	// possible and publish the sweep tx. When the sweep tx
 	// confirms, it signals us through the result channel with the
 	// outcome. Wait for this to happen.
+	recovered := true
 	select {
 	case sweepResult := <-resultChan:
-		if sweepResult.Err != nil {
+		switch sweepResult.Err {
+		case sweep.ErrRemoteSpend:
+			// If the remote party was able to sweep this output
+			// it's likely what we sent was actually a revoked
+			// commitment. Report the error and continue to wrap up
+			// the contract.
+			c.log.Errorf("local commitment output was swept by "+
+				"remote party via %v", sweepResult.Tx.TxHash())
+			recovered = false
+		case nil:
+			// No errors, therefore continue processing.
+			c.log.Infof("local commitment output fully resolved by "+
+				"sweep tx: %v", sweepResult.Tx.TxHash())
+		default:
+			// Unknown errors.
 			c.log.Errorf("unable to sweep input: %v",
 				sweepResult.Err)
 
 			return nil, sweepResult.Err
 		}
-
-		c.log.Infof("commit tx fully resolved by sweep tx: %v",
-			sweepResult.Tx.TxHash())
 	case <-c.quit:
 		return nil, errResolverShuttingDown
 	}
 
 	// Funds have been swept and balance is no longer in limbo.
 	c.reportLock.Lock()
-	c.currentReport.RecoveredBalance = c.currentReport.LimboBalance
+	if recovered {
+		// We only record the balance as recovered if it actually came
+		// back to us.
+		c.currentReport.RecoveredBalance = c.currentReport.LimboBalance
+	}
 	c.currentReport.LimboBalance = 0
 	c.reportLock.Unlock()
 
diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go
index 6c48ddc8b..475507bab 100644
--- a/lntest/itest/lnd_test.go
+++ b/lntest/itest/lnd_test.go
@@ -7586,6 +7586,15 @@ func testRevokedCloseRetribution(net *lntest.NetworkHarness, t *harnessTest) {
 	}
 
 	assertNodeNumChannels(t, carol, 0)
+
+	// Mine enough blocks for Bob's channel arbitrator to wrap up the
+	// references to the breached channel. The chanarb waits for commitment
+	// tx's confHeight+CSV-1 blocks and since we've already mined one that
+	// included the justice tx we only need to mine extra DefaultCSV-2
+	// blocks to unlock it.
+	mineBlocks(t, net, lntest.DefaultCSV-2, 0)
+
+	assertNumPendingChannels(t, net.Bob, 0, 0)
 }
 
 // testRevokedCloseRetributionZeroValueRemoteOutput tests that Dave is able