diff --git a/contractcourt/briefcase_test.go b/contractcourt/briefcase_test.go index 3dfc155e4..c86bffb38 100644 --- a/contractcourt/briefcase_test.go +++ b/contractcourt/briefcase_test.go @@ -278,9 +278,9 @@ func assertResolversEqual(t *testing.T, originalResolver ContractResolver, t.Fatalf("expected %v, got %v", ogRes.resolved.Load(), diskRes.resolved.Load()) } - if ogRes.broadcastHeight != diskRes.broadcastHeight { + if ogRes.confirmHeight != diskRes.confirmHeight { t.Fatalf("expected %v, got %v", - ogRes.broadcastHeight, diskRes.broadcastHeight) + ogRes.confirmHeight, diskRes.confirmHeight) } if ogRes.chanPoint != diskRes.chanPoint { t.Fatalf("expected %v, got %v", ogRes.chanPoint, @@ -341,8 +341,8 @@ func TestContractInsertionRetrieval(t *testing.T) { SelfOutputSignDesc: testSignDesc, MaturityDelay: 99, }, - broadcastHeight: 109, - chanPoint: testChanPoint1, + confirmHeight: 109, + chanPoint: testChanPoint1, } commitResolver.resolved.Store(false) diff --git a/contractcourt/commit_sweep_resolver.go b/contractcourt/commit_sweep_resolver.go index 0f2cb6b24..d8c8c3903 100644 --- a/contractcourt/commit_sweep_resolver.go +++ b/contractcourt/commit_sweep_resolver.go @@ -38,10 +38,10 @@ type commitSweepResolver struct { // this HTLC on-chain. commitResolution lnwallet.CommitOutputResolution - // broadcastHeight is the height that the original contract was - // broadcast to the main-chain at. We'll use this value to bound any - // historical queries to the chain for spends/confirmations. - broadcastHeight uint32 + // confirmHeight is the block height that the commitment transaction was + // confirmed at. We'll use this value to bound any historical queries to + // the chain for spends/confirmations. + confirmHeight uint32 // chanPoint is the channel point of the original contract. chanPoint wire.OutPoint @@ -74,13 +74,13 @@ type commitSweepResolver struct { // newCommitSweepResolver instantiates a new direct commit output resolver. func newCommitSweepResolver(res lnwallet.CommitOutputResolution, - broadcastHeight uint32, chanPoint wire.OutPoint, + confirmHeight uint32, chanPoint wire.OutPoint, resCfg ResolverConfig) *commitSweepResolver { r := &commitSweepResolver{ contractResolverKit: *newContractResolverKit(resCfg), commitResolution: res, - broadcastHeight: broadcastHeight, + confirmHeight: confirmHeight, chanPoint: chanPoint, } @@ -123,37 +123,6 @@ func waitForSpend(op *wire.OutPoint, pkScript []byte, heightHint uint32, } } -// getCommitTxConfHeight waits for confirmation of the commitment tx and -// returns the confirmation height. -func (c *commitSweepResolver) getCommitTxConfHeight() (uint32, error) { - txID := c.commitResolution.SelfOutPoint.Hash - signDesc := c.commitResolution.SelfOutputSignDesc - pkScript := signDesc.Output.PkScript - - const confDepth = 1 - - confChan, err := c.Notifier.RegisterConfirmationsNtfn( - &txID, pkScript, confDepth, c.broadcastHeight, - ) - if err != nil { - return 0, err - } - defer confChan.Cancel() - - select { - case txConfirmation, ok := <-confChan.Confirmed: - if !ok { - return 0, fmt.Errorf("cannot get confirmation "+ - "for commit tx %v", txID) - } - - return txConfirmation.BlockHeight, nil - - case <-c.quit: - return 0, errResolverShuttingDown - } -} - // Resolve instructs the contract resolver to resolve the output on-chain. Once // the output has been *fully* resolved, the function should return immediately // with a nil ContractResolver value for the first return value. In the case @@ -268,7 +237,7 @@ func (c *commitSweepResolver) Encode(w io.Writer) error { if err := binary.Write(w, endian, c.IsResolved()); err != nil { return err } - if err := binary.Write(w, endian, c.broadcastHeight); err != nil { + if err := binary.Write(w, endian, c.confirmHeight); err != nil { return err } if _, err := w.Write(c.chanPoint.Hash[:]); err != nil { @@ -308,7 +277,7 @@ func newCommitSweepResolverFromReader(r io.Reader, resCfg ResolverConfig) ( c.markResolved() } - if err := binary.Read(r, endian, &c.broadcastHeight); err != nil { + if err := binary.Read(r, endian, &c.confirmHeight); err != nil { return nil, err } _, err := io.ReadFull(r, c.chanPoint.Hash[:]) @@ -381,19 +350,14 @@ func (c *commitSweepResolver) Launch() error { return nil } - confHeight, err := c.getCommitTxConfHeight() - if err != nil { - return err - } - // Wait up until the CSV expires, unless we also have a CLTV that // expires after. - unlockHeight := confHeight + c.commitResolution.MaturityDelay + unlockHeight := c.confirmHeight + c.commitResolution.MaturityDelay if c.hasCLTV() { unlockHeight = max(unlockHeight, c.leaseExpiry) } - // Update report now that we learned the confirmation height. + // Update report with the calculated maturity height. c.reportLock.Lock() c.currentReport.MaturityHeight = unlockHeight c.reportLock.Unlock() @@ -412,7 +376,7 @@ func (c *commitSweepResolver) Launch() error { inp = input.NewCsvInputWithCltv( &c.commitResolution.SelfOutPoint, witnessType, &c.commitResolution.SelfOutputSignDesc, - c.broadcastHeight, c.commitResolution.MaturityDelay, + c.confirmHeight, c.commitResolution.MaturityDelay, c.leaseExpiry, input.WithResolutionBlob( c.commitResolution.ResolutionBlob, ), @@ -421,7 +385,7 @@ func (c *commitSweepResolver) Launch() error { inp = input.NewCsvInput( &c.commitResolution.SelfOutPoint, witnessType, &c.commitResolution.SelfOutputSignDesc, - c.broadcastHeight, c.commitResolution.MaturityDelay, + c.confirmHeight, c.commitResolution.MaturityDelay, input.WithResolutionBlob( c.commitResolution.ResolutionBlob, ), diff --git a/contractcourt/commit_sweep_resolver_test.go b/contractcourt/commit_sweep_resolver_test.go index 6855fddcd..5c660e100 100644 --- a/contractcourt/commit_sweep_resolver_test.go +++ b/contractcourt/commit_sweep_resolver_test.go @@ -18,6 +18,10 @@ import ( "github.com/stretchr/testify/require" ) +const ( + testCommitSweepConfHeight = 99 +) + type commitSweepResolverTestContext struct { resolver *commitSweepResolver notifier *mock.ChainNotifier @@ -27,7 +31,8 @@ type commitSweepResolverTestContext struct { } func newCommitSweepResolverTestContext(t *testing.T, - resolution *lnwallet.CommitOutputResolution) *commitSweepResolverTestContext { + resolution *lnwallet.CommitOutputResolution, + confirmHeight uint32) *commitSweepResolverTestContext { notifier := &mock.ChainNotifier{ EpochChan: make(chan *chainntnfs.BlockEpoch), @@ -68,7 +73,7 @@ func newCommitSweepResolverTestContext(t *testing.T, } resolver := newCommitSweepResolver( - *resolution, 0, wire.OutPoint{}, cfg, + *resolution, confirmHeight, wire.OutPoint{}, cfg, ) return &commitSweepResolverTestContext{ @@ -178,7 +183,9 @@ func TestCommitSweepResolverNoDelay(t *testing.T) { }, } - ctx := newCommitSweepResolverTestContext(t, &res) + ctx := newCommitSweepResolverTestContext( + t, &res, testCommitSweepConfHeight, + ) // Replace our checkpoint with one which will push reports into a // channel for us to consume. We replace this function on the resolver @@ -197,15 +204,12 @@ func TestCommitSweepResolverNoDelay(t *testing.T) { ctx.resolve() - spendTx := &wire.MsgTx{} - spendHash := spendTx.TxHash() - ctx.notifier.ConfChan <- &chainntnfs.TxConfirmation{ - Tx: spendTx, - } - // No csv delay, so the input should be swept immediately. <-ctx.sweeper.sweptInputs + spendTx := &wire.MsgTx{} + spendHash := spendTx.TxHash() + amt := btcutil.Amount(res.SelfOutputSignDesc.Output.Value) expectedReport := &channeldb.ResolverReport{ OutPoint: wire.OutPoint{}, @@ -242,7 +246,10 @@ func testCommitSweepResolverDelay(t *testing.T, sweepErr error) { SelfOutPoint: outpoint, } - ctx := newCommitSweepResolverTestContext(t, &res) + // Use confirmHeight = 99, so maturityHeight = 99 + 3 = 102. + ctx := newCommitSweepResolverTestContext( + t, &res, testCommitSweepConfHeight, + ) // Replace our checkpoint with one which will push reports into a // channel for us to consume. We replace this function on the resolver @@ -270,25 +277,18 @@ func testCommitSweepResolverDelay(t *testing.T, sweepErr error) { Amount: btcutil.Amount(amt), LimboBalance: btcutil.Amount(amt), } - if *report != expectedReport { - t.Fatalf("unexpected resolver report. want=%v got=%v", - expectedReport, report) - } + require.Equal(t, expectedReport, *report) ctx.resolve() - ctx.notifier.ConfChan <- &chainntnfs.TxConfirmation{ - BlockHeight: testInitialBlockHeight - 1, - } - - // Allow resolver to process confirmation. + // Allow resolver to launch and update the report. time.Sleep(sweepProcessInterval) // Expect report to be updated. + // confirmHeight(99) + maturityDelay(3) = 102. report = ctx.resolver.report() - if report.MaturityHeight != testInitialBlockHeight+2 { - t.Fatal("report maturity height incorrect") - } + expectedMaturity := testCommitSweepConfHeight + res.MaturityDelay + require.Equal(t, expectedMaturity, report.MaturityHeight) // Notify initial block height. Although the csv lock is still in // effect, we expect the input being sent to the sweeper before the csv @@ -325,13 +325,10 @@ func testCommitSweepResolverDelay(t *testing.T, sweepErr error) { Outpoint: outpoint, Type: ReportOutputUnencumbered, Amount: btcutil.Amount(amt), - MaturityHeight: testInitialBlockHeight + 2, + MaturityHeight: testCommitSweepConfHeight + res.MaturityDelay, RecoveredBalance: expectedRecoveredBalance, } - if *report != expectedReport { - t.Fatalf("unexpected resolver report. want=%v got=%v", - expectedReport, report) - } + require.Equal(t, expectedReport, *report) } // TestCommitSweepResolverDelay tests resolution of a direct commitment output diff --git a/docs/release-notes/release-notes-0.20.1.md b/docs/release-notes/release-notes-0.20.1.md index 1963db61b..7dbcd31ea 100644 --- a/docs/release-notes/release-notes-0.20.1.md +++ b/docs/release-notes/release-notes-0.20.1.md @@ -24,6 +24,10 @@ * Fix bug where channels with both [policies disabled at startup could never be used for routing](https://github.com/lightningnetwork/lnd/pull/10378) +* [Fix a case where resolving the + to_local/to_remote output](https://github.com/lightningnetwork/lnd/pull/10387) + might take too long. + # New Features ## Functional Enhancements