diff --git a/contractcourt/commit_sweep_resolver.go b/contractcourt/commit_sweep_resolver.go index fe09dc09..5c499077 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.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() diff --git a/contractcourt/commit_sweep_resolver_test.go b/contractcourt/commit_sweep_resolver_test.go index 279cc6e8..866cb22b 100644 --- a/contractcourt/commit_sweep_resolver_test.go +++ b/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 -// that is encumbered by a time lock. -func TestCommitSweepResolverDelay(t *testing.T) { - t.Parallel() +// testCommitSweepResolverDelay tests resolution of a direct commitment output +// that is encumbered by a time lock. sweepErr indicates whether the local node +// fails to sweep the output. +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, - }) { - t.Fatal("unexpected resolver report") + RecoveredBalance: expectedRecoveredBalance, + } + 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 + } } } diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 6c48ddc8..475507ba 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