Merge pull request #3892 from matheusdtech/fix-lingering-contract

contractcourt: fix lingering contract after local breach

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.Warnf("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()
 

contractcourt/commit_sweep_resolver_test.go

@@ -1,7 +1,6 @@
 package contractcourt
 
 import (
-	"reflect"
 	"testing"
 	"time"
 
@@ -96,6 +95,7 @@ func (i *commitSweepResolverTestContext) waitForResult() {
 type mockSweeper struct {
 	sweptInputs   chan input.Input
 	updatedInputs chan wire.OutPoint
+	sweepErr      error
 }
 
 func newMockSweeper() *mockSweeper {
@@ -112,7 +112,8 @@ func (s *mockSweeper) SweepInput(input input.Input, params sweep.Params) (
 
 	result := make(chan sweep.Result, 1)
 	result <- sweep.Result{
-		Tx: &wire.MsgTx{},
+		Tx:  &wire.MsgTx{},
+		Err: s.sweepErr,
 	}
 	return result, nil
 }
@@ -167,12 +168,13 @@ func TestCommitSweepResolverNoDelay(t *testing.T) {
 	ctx.waitForResult()
 }
 
-// TestCommitSweepResolverDelay tests resolution of a direct commitment output
+// testCommitSweepResolverDelay tests resolution of a direct commitment output
-// that is encumbered by a time lock.
+// that is encumbered by a time lock. sweepErr indicates whether the local node
-func TestCommitSweepResolverDelay(t *testing.T) {
+// fails to sweep the output.
-	t.Parallel()
+func testCommitSweepResolverDelay(t *testing.T, sweepErr error) {
 	defer timeout(t)()
 
+	const sweepProcessInterval = 100 * time.Millisecond
 	amt := int64(100)
 	outpoint := wire.OutPoint{
 		Index: 5,
@@ -190,14 +192,20 @@ func TestCommitSweepResolverDelay(t *testing.T) {
 
 	ctx := newCommitSweepResolverTestContext(t, &res)
 
+	// Setup whether we expect the sweeper to receive a sweep error in this
+	// test case.
+	ctx.sweeper.sweepErr = sweepErr
+
 	report := ctx.resolver.report()
-	if !reflect.DeepEqual(report, &ContractReport{
+	expectedReport := ContractReport{
 		Outpoint:     outpoint,
 		Type:         ReportOutputUnencumbered,
 		Amount:       btcutil.Amount(amt),
 		LimboBalance: btcutil.Amount(amt),
-	}) {
+	}
-		t.Fatal("unexpected resolver report")
+	if *report != expectedReport {
+		t.Fatalf("unexpected resolver report. want=%v got=%v",
+			expectedReport, report)
 	}
 
 	ctx.resolve()
@@ -207,7 +215,7 @@ func TestCommitSweepResolverDelay(t *testing.T) {
 	}
 
 	// Allow resolver to process confirmation.
-	time.Sleep(100 * time.Millisecond)
+	time.Sleep(sweepProcessInterval)
 
 	// Expect report to be updated.
 	report = ctx.resolver.report()
@@ -222,7 +230,7 @@ func TestCommitSweepResolverDelay(t *testing.T) {
 	select {
 	case <-ctx.sweeper.sweptInputs:
 		t.Fatal("no sweep expected")
-	case <-time.After(100 * time.Millisecond):
+	case <-time.After(sweepProcessInterval):
 	}
 
 	// A new block arrives. The commit tx confirmed at height -1 and the csv
@@ -233,14 +241,52 @@ func TestCommitSweepResolverDelay(t *testing.T) {
 
 	ctx.waitForResult()
 
+	// If this test case generates a sweep error, we don't expect to be
+	// able to recover anything. This might happen if the local commitment
+	// output was swept by a justice transaction by the remote party.
+	expectedRecoveredBalance := btcutil.Amount(amt)
+	if sweepErr != nil {
+		expectedRecoveredBalance = 0
+	}
+
 	report = ctx.resolver.report()
-	if !reflect.DeepEqual(report, &ContractReport{
+	expectedReport = ContractReport{
 		Outpoint:         outpoint,
 		Type:             ReportOutputUnencumbered,
 		Amount:           btcutil.Amount(amt),
-		RecoveredBalance: btcutil.Amount(amt),
 		MaturityHeight:   testInitialBlockHeight + 2,
-	}) {
+		RecoveredBalance: expectedRecoveredBalance,
-		t.Fatal("unexpected resolver report")
+	}
+	if *report != expectedReport {
+		t.Fatalf("unexpected resolver report. want=%v got=%v",
+			expectedReport, report)
+	}
+
+}
+
+// TestCommitSweepResolverDelay tests resolution of a direct commitment output
+// that is encumbered by a time lock.
+func TestCommitSweepResolverDelay(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name     string
+		sweepErr error
+	}{{
+		name:     "success",
+		sweepErr: nil,
+	}, {
+		name:     "remote spend",
+		sweepErr: sweep.ErrRemoteSpend,
+	}}
+
+	for _, tc := range testCases {
+		tc := tc
+		ok := t.Run(tc.name, func(t *testing.T) {
+			testCommitSweepResolverDelay(t, tc.sweepErr)
+		})
+		if !ok {
+			break
+		}
 	}
 }

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