From c898b68bba89b734c6fb9ced964854be1f9d6760 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Fri, 6 Sep 2024 18:19:49 -0700 Subject: [PATCH 1/2] contractcourt: use t.Run in TestHtlcTimeoutResolver Along the way we refactor the test to eliminate some unnecessary line length. --- contractcourt/htlc_timeout_resolver_test.go | 479 +++++++++++--------- 1 file changed, 253 insertions(+), 226 deletions(-) diff --git a/contractcourt/htlc_timeout_resolver_test.go b/contractcourt/htlc_timeout_resolver_test.go index c551a6f1ce..8ccd4aaaca 100644 --- a/contractcourt/htlc_timeout_resolver_test.go +++ b/contractcourt/htlc_timeout_resolver_test.go @@ -69,11 +69,31 @@ func (m *mockWitnessBeacon) AddPreimages(preimages ...lntypes.Preimage) error { return nil } -// TestHtlcTimeoutResolver tests that the timeout resolver properly handles all -// variations of possible local+remote spends. -func TestHtlcTimeoutResolver(t *testing.T) { - t.Parallel() +type htlcTimeoutTestCase struct { + // name is a human readable description of the test case. + name string + + // remoteCommit denotes if the commitment broadcast was the remote + // commitment or not. + remoteCommit bool + + // timeout denotes if the HTLC should be let timeout, or if the "remote" + // party should sweep it on-chain. This also affects what type of + // resolution message we expect. + timeout bool + + // txToBroadcast is a function closure that should generate the + // transaction that should spend the HTLC output. Test authors can use + // this to customize the witness used when spending to trigger various + // redemption cases. + txToBroadcast func() (*wire.MsgTx, error) + + // outcome is the resolver outcome that we expect to be reported once + // the contract is fully resolved. + outcome channeldb.ResolverOutcome +} +func genHtlcTimeoutTestCases() []htlcTimeoutTestCase { fakePreimageBytes := bytes.Repeat([]byte{1}, lntypes.HashSize) var ( @@ -105,29 +125,7 @@ func TestHtlcTimeoutResolver(t *testing.T) { }, } - testCases := []struct { - // name is a human readable description of the test case. - name string - - // remoteCommit denotes if the commitment broadcast was the - // remote commitment or not. - remoteCommit bool - - // timeout denotes if the HTLC should be let timeout, or if the - // "remote" party should sweep it on-chain. This also affects - // what type of resolution message we expect. - timeout bool - - // txToBroadcast is a function closure that should generate the - // transaction that should spend the HTLC output. Test authors - // can use this to customize the witness used when spending to - // trigger various redemption cases. - txToBroadcast func() (*wire.MsgTx, error) - - // outcome is the resolver outcome that we expect to be reported - // once the contract is fully resolved. - outcome channeldb.ResolverOutcome - }{ + return []htlcTimeoutTestCase{ // Remote commitment is broadcast, we time out the HTLC on // chain, and should expect a fail HTLC resolution. { @@ -149,7 +147,8 @@ func TestHtlcTimeoutResolver(t *testing.T) { // immediately if the witness is already set // correctly. if reflect.DeepEqual( - templateTx.TxIn[0].Witness, witness, + templateTx.TxIn[0].Witness, + witness, ) { return templateTx, nil @@ -219,7 +218,8 @@ func TestHtlcTimeoutResolver(t *testing.T) { // immediately if the witness is already set // correctly. if reflect.DeepEqual( - templateTx.TxIn[0].Witness, witness, + templateTx.TxIn[0].Witness, + witness, ) { return templateTx, nil @@ -253,7 +253,8 @@ func TestHtlcTimeoutResolver(t *testing.T) { // immediately if the witness is already set // correctly. if reflect.DeepEqual( - templateTx.TxIn[0].Witness, witness, + templateTx.TxIn[0].Witness, + witness, ) { return templateTx, nil @@ -265,243 +266,265 @@ func TestHtlcTimeoutResolver(t *testing.T) { outcome: channeldb.ResolverOutcomeClaimed, }, } +} + +func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { + fakePreimageBytes := bytes.Repeat([]byte{1}, lntypes.HashSize) + var fakePreimage lntypes.Preimage + + fakeSignDesc := &input.SignDescriptor{ + Output: &wire.TxOut{}, + } + + copy(fakePreimage[:], fakePreimageBytes) notifier := &mock.ChainNotifier{ EpochChan: make(chan *chainntnfs.BlockEpoch), SpendChan: make(chan *chainntnfs.SpendDetail), ConfChan: make(chan *chainntnfs.TxConfirmation), } - witnessBeacon := newMockWitnessBeacon() - - for _, testCase := range testCases { - t.Logf("Running test case: %v", testCase.name) - - checkPointChan := make(chan struct{}, 1) - incubateChan := make(chan struct{}, 1) - resolutionChan := make(chan ResolutionMsg, 1) - reportChan := make(chan *channeldb.ResolverReport) - - //nolint:lll - chainCfg := ChannelArbitratorConfig{ - ChainArbitratorConfig: ChainArbitratorConfig{ - Notifier: notifier, - PreimageDB: witnessBeacon, - IncubateOutputs: func(wire.OutPoint, - fn.Option[lnwallet.OutgoingHtlcResolution], - fn.Option[lnwallet.IncomingHtlcResolution], - uint32, fn.Option[int32]) error { - - incubateChan <- struct{}{} - return nil - }, - DeliverResolutionMsg: func(msgs ...ResolutionMsg) error { - if len(msgs) != 1 { - return fmt.Errorf("expected 1 "+ - "resolution msg, instead got %v", - len(msgs)) - } - resolutionChan <- msgs[0] - return nil - }, - Budget: *DefaultBudgetConfig(), - QueryIncomingCircuit: func(circuit models.CircuitKey) *models.CircuitKey { - return nil - }, + witnessBeacon := newMockWitnessBeacon() + checkPointChan := make(chan struct{}, 1) + incubateChan := make(chan struct{}, 1) + resolutionChan := make(chan ResolutionMsg, 1) + reportChan := make(chan *channeldb.ResolverReport) + + //nolint:lll + chainCfg := ChannelArbitratorConfig{ + ChainArbitratorConfig: ChainArbitratorConfig{ + Notifier: notifier, + PreimageDB: witnessBeacon, + IncubateOutputs: func(wire.OutPoint, + fn.Option[lnwallet.OutgoingHtlcResolution], + fn.Option[lnwallet.IncomingHtlcResolution], + uint32, fn.Option[int32], + ) error { + incubateChan <- struct{}{} + return nil }, - PutResolverReport: func(_ kvdb.RwTx, - _ *channeldb.ResolverReport) error { + DeliverResolutionMsg: func(msgs ...ResolutionMsg) error { + if len(msgs) != 1 { + return fmt.Errorf("expected 1 "+ + "resolution msg, instead got %v", + len(msgs)) + } + resolutionChan <- msgs[0] return nil }, - } + Budget: *DefaultBudgetConfig(), + QueryIncomingCircuit: func(circuit models.CircuitKey, + ) *models.CircuitKey { + return nil + }, + }, + PutResolverReport: func(_ kvdb.RwTx, + _ *channeldb.ResolverReport, + ) error { + return nil + }, + } - cfg := ResolverConfig{ - ChannelArbitratorConfig: chainCfg, - Checkpoint: func(_ ContractResolver, - reports ...*channeldb.ResolverReport) error { + cfg := ResolverConfig{ + ChannelArbitratorConfig: chainCfg, + Checkpoint: func(_ ContractResolver, + reports ...*channeldb.ResolverReport, + ) error { + checkPointChan <- struct{}{} - checkPointChan <- struct{}{} + // Send all of our reports into the channel. + for _, report := range reports { + reportChan <- report + } - // Send all of our reports into the channel. - for _, report := range reports { - reportChan <- report - } + return nil + }, + } + resolver := &htlcTimeoutResolver{ + htlcResolution: lnwallet.OutgoingHtlcResolution{ + ClaimOutpoint: testChanPoint2, + SweepSignDesc: *fakeSignDesc, + }, + contractResolverKit: *newContractResolverKit( + cfg, + ), + htlc: channeldb.HTLC{ + Amt: testHtlcAmt, + }, + } - return nil - }, - } - resolver := &htlcTimeoutResolver{ - htlcResolution: lnwallet.OutgoingHtlcResolution{ - ClaimOutpoint: testChanPoint2, - SweepSignDesc: *fakeSignDesc, - }, - contractResolverKit: *newContractResolverKit( - cfg, - ), - htlc: channeldb.HTLC{ - Amt: testHtlcAmt, - }, + var reports []*channeldb.ResolverReport + + // If the test case needs the remote commitment to be + // broadcast, then we'll set the timeout commit to a fake + // transaction to force the code path. + if !testCase.remoteCommit { + timeoutTx, err := testCase.txToBroadcast() + require.NoError(t, err) + + resolver.htlcResolution.SignedTimeoutTx = timeoutTx + + if testCase.timeout { + timeoutTxID := timeoutTx.TxHash() + reports = append(reports, &channeldb.ResolverReport{ + OutPoint: timeoutTx.TxIn[0].PreviousOutPoint, + Amount: testHtlcAmt.ToSatoshis(), + ResolverType: channeldb.ResolverTypeOutgoingHtlc, + ResolverOutcome: channeldb.ResolverOutcomeFirstStage, + SpendTxID: &timeoutTxID, + }) } + } - var reports []*channeldb.ResolverReport + // With all the setup above complete, we can initiate the + // resolution process, and the bulk of our test. + var wg sync.WaitGroup + resolveErr := make(chan error, 1) + wg.Add(1) + go func() { + defer wg.Done() - // If the test case needs the remote commitment to be - // broadcast, then we'll set the timeout commit to a fake - // transaction to force the code path. - if !testCase.remoteCommit { - timeoutTx, err := testCase.txToBroadcast() - require.NoError(t, err) - - resolver.htlcResolution.SignedTimeoutTx = timeoutTx - - if testCase.timeout { - timeoutTxID := timeoutTx.TxHash() - reports = append(reports, &channeldb.ResolverReport{ - OutPoint: timeoutTx.TxIn[0].PreviousOutPoint, - Amount: testHtlcAmt.ToSatoshis(), - ResolverType: channeldb.ResolverTypeOutgoingHtlc, - ResolverOutcome: channeldb.ResolverOutcomeFirstStage, - SpendTxID: &timeoutTxID, - }) - } + _, err := resolver.Resolve(false) + if err != nil { + resolveErr <- err } + }() + + // As the output isn't yet in the nursery, we expect that we + // should receive an incubation request. + select { + case <-incubateChan: + case err := <-resolveErr: + t.Fatalf("unable to resolve HTLC: %v", err) + case <-time.After(time.Second * 5): + t.Fatalf("failed to receive incubation request") + } - // With all the setup above complete, we can initiate the - // resolution process, and the bulk of our test. - var wg sync.WaitGroup - resolveErr := make(chan error, 1) - wg.Add(1) - go func() { - defer wg.Done() - - _, err := resolver.Resolve(false) - if err != nil { - resolveErr <- err - } - }() + // Next, the resolver should request a spend notification for + // the direct HTLC output. We'll use the txToBroadcast closure + // for the test case to generate the transaction that we'll + // send to the resolver. + spendingTx, err := testCase.txToBroadcast() + if err != nil { + t.Fatalf("unable to generate tx: %v", err) + } + spendTxHash := spendingTx.TxHash() + + select { + case notifier.SpendChan <- &chainntnfs.SpendDetail{ + SpendingTx: spendingTx, + SpenderTxHash: &spendTxHash, + }: + case <-time.After(time.Second * 5): + t.Fatalf("failed to request spend ntfn") + } - // At the output isn't yet in the nursery, we expect that we - // should receive an incubation request. + if !testCase.timeout { + // If the resolver should settle now, then we'll + // extract the pre-image to be extracted and the + // resolution message sent. select { - case <-incubateChan: - case err := <-resolveErr: - t.Fatalf("unable to resolve HTLC: %v", err) + case newPreimage := <-witnessBeacon.newPreimages: + if newPreimage[0] != fakePreimage { + t.Fatalf("wrong pre-image: "+ + "expected %v, got %v", + fakePreimage, newPreimage) + } + case <-time.After(time.Second * 5): - t.Fatalf("failed to receive incubation request") + t.Fatalf("pre-image not added") } - // Next, the resolver should request a spend notification for - // the direct HTLC output. We'll use the txToBroadcast closure - // for the test case to generate the transaction that we'll - // send to the resolver. - spendingTx, err := testCase.txToBroadcast() - if err != nil { - t.Fatalf("unable to generate tx: %v", err) + // Finally, we should get a resolution message with the + // pre-image set within the message. + select { + case resolutionMsg := <-resolutionChan: + // Once again, the pre-images should match up. + if *resolutionMsg.PreImage != fakePreimage { + t.Fatalf("wrong pre-image: "+ + "expected %v, got %v", + fakePreimage, resolutionMsg.PreImage) + } + case <-time.After(time.Second * 5): + t.Fatalf("resolution not sent") } - spendTxHash := spendingTx.TxHash() + } else { + // Otherwise, the HTLC should now timeout. First, we + // should get a resolution message with a populated + // failure message. select { - case notifier.SpendChan <- &chainntnfs.SpendDetail{ - SpendingTx: spendingTx, - SpenderTxHash: &spendTxHash, - }: + case resolutionMsg := <-resolutionChan: + if resolutionMsg.Failure == nil { + t.Fatalf("expected failure resolution msg") + } case <-time.After(time.Second * 5): - t.Fatalf("failed to request spend ntfn") + t.Fatalf("resolution not sent") } - if !testCase.timeout { - // If the resolver should settle now, then we'll - // extract the pre-image to be extracted and the - // resolution message sent. + // We should also get another request for the spend + // notification of the second-level transaction to + // indicate that it's been swept by the nursery, but + // only if this is a local commitment transaction. + if !testCase.remoteCommit { select { - case newPreimage := <-witnessBeacon.newPreimages: - if newPreimage[0] != fakePreimage { - t.Fatalf("wrong pre-image: "+ - "expected %v, got %v", - fakePreimage, newPreimage) - } - + case notifier.SpendChan <- &chainntnfs.SpendDetail{ + SpendingTx: spendingTx, + SpenderTxHash: &spendTxHash, + }: case <-time.After(time.Second * 5): - t.Fatalf("pre-image not added") + t.Fatalf("failed to request spend ntfn") } + } + } - // Finally, we should get a resolution message with the - // pre-image set within the message. - select { - case resolutionMsg := <-resolutionChan: - // Once again, the pre-images should match up. - if *resolutionMsg.PreImage != fakePreimage { - t.Fatalf("wrong pre-image: "+ - "expected %v, got %v", - fakePreimage, resolutionMsg.PreImage) - } - case <-time.After(time.Second * 5): - t.Fatalf("resolution not sent") - } - } else { + // In any case, before the resolver exits, it should checkpoint + // its final state. + select { + case <-checkPointChan: + case err := <-resolveErr: + t.Fatalf("unable to resolve HTLC: %v", err) + case <-time.After(time.Second * 5): + t.Fatalf("check point not received") + } - // Otherwise, the HTLC should now timeout. First, we - // should get a resolution message with a populated - // failure message. - select { - case resolutionMsg := <-resolutionChan: - if resolutionMsg.Failure == nil { - t.Fatalf("expected failure resolution msg") - } - case <-time.After(time.Second * 5): - t.Fatalf("resolution not sent") - } + // Add a report to our set of expected reports with the outcome + // that the test specifies (either success or timeout). + spendTxID := spendingTx.TxHash() + amt := btcutil.Amount(fakeSignDesc.Output.Value) - // We should also get another request for the spend - // notification of the second-level transaction to - // indicate that it's been swept by the nursery, but - // only if this is a local commitment transaction. - if !testCase.remoteCommit { - select { - case notifier.SpendChan <- &chainntnfs.SpendDetail{ - SpendingTx: spendingTx, - SpenderTxHash: &spendTxHash, - }: - case <-time.After(time.Second * 5): - t.Fatalf("failed to request spend ntfn") - } - } - } + reports = append(reports, &channeldb.ResolverReport{ + OutPoint: testChanPoint2, + Amount: amt, + ResolverType: channeldb.ResolverTypeOutgoingHtlc, + ResolverOutcome: testCase.outcome, + SpendTxID: &spendTxID, + }) - // In any case, before the resolver exits, it should checkpoint - // its final state. - select { - case <-checkPointChan: - case err := <-resolveErr: - t.Fatalf("unable to resolve HTLC: %v", err) - case <-time.After(time.Second * 5): - t.Fatalf("check point not received") - } + for _, report := range reports { + assertResolverReport(t, reportChan, report) + } - // Add a report to our set of expected reports with the outcome - // that the test specifies (either success or timeout). - spendTxID := spendingTx.TxHash() - amt := btcutil.Amount(fakeSignDesc.Output.Value) - - reports = append(reports, &channeldb.ResolverReport{ - OutPoint: testChanPoint2, - Amount: amt, - ResolverType: channeldb.ResolverTypeOutgoingHtlc, - ResolverOutcome: testCase.outcome, - SpendTxID: &spendTxID, - }) + wg.Wait() - for _, report := range reports { - assertResolverReport(t, reportChan, report) - } + // Finally, the resolver should be marked as resolved. + if !resolver.resolved { + t.Fatalf("resolver should be marked as resolved") + } +} + +// TestHtlcTimeoutResolver tests that the timeout resolver properly handles all +// variations of possible local+remote spends. +func TestHtlcTimeoutResolver(t *testing.T) { + t.Parallel() - wg.Wait() + testCases := genHtlcTimeoutTestCases() - // Finally, the resolver should be marked as resolved. - if !resolver.resolved { - t.Fatalf("resolver should be marked as resolved") - } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + testHtlcTimeoutResolver(t, testCase) + }) } } @@ -545,6 +568,7 @@ func TestHtlcTimeoutSingleStage(t *testing.T) { // by the nursery. preCheckpoint: func(ctx *htlcResolverTestContext, _ bool) error { + // The nursery will create and publish a sweep // tx. ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ @@ -653,6 +677,7 @@ func TestHtlcTimeoutSecondStage(t *testing.T) { // that our sweep succeeded. preCheckpoint: func(ctx *htlcResolverTestContext, _ bool) error { + // The nursery will publish the timeout tx. ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{ SpendingTx: timeoutTx, @@ -1296,7 +1321,9 @@ func TestHtlcTimeoutSecondStageSweeperRemoteSpend(t *testing.T) { } func testHtlcTimeout(t *testing.T, resolution lnwallet.OutgoingHtlcResolution, - checkpoints []checkpoint) { + checkpoints []checkpoint, +) { + t.Helper() defer timeout()() From 903c8fc076ccc0c8922d5ce19be0058a58c21209 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 3 Sep 2024 18:58:12 -0700 Subject: [PATCH 2/2] contractcourt: use the sweeper for HTLC offered remote timeout resolution In this commit, we bring the timeout resolver more in line with the success resolver by using the sweeper to handle the HTLC offered remote timeout outputs. These are outputs that we can sweep directly from the remote party's commitment transaction when they broadcast their version of the commitment transaction. With this change, we slim down the scope slightly by only doing this for anchor channels. Non-anchor channels will continue to use the utxonursery for this output type for now. --- contractcourt/htlc_timeout_resolver.go | 94 +++++++++++++++++++-- contractcourt/htlc_timeout_resolver_test.go | 65 +++++++------- 2 files changed, 123 insertions(+), 36 deletions(-) diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index 62ff832071..3227865d19 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -538,7 +538,6 @@ func (h *htlcTimeoutResolver) sweepSecondLevelTx(immediate bool) error { return err } - // TODO(yy): checkpoint here? return err } @@ -562,6 +561,59 @@ func (h *htlcTimeoutResolver) sendSecondLevelTxLegacy() error { return h.Checkpoint(h) } +// sweepDirectHtlcOutput sends the direct spend of the HTLC output to the +// sweeper. This is used when the remote party goes on chain, and we're able to +// sweep an HTLC we offered after a timeout. Only the CLTV encumbered outputs +// are resolved via this path. +func (h *htlcTimeoutResolver) sweepDirectHtlcOutput(immediate bool) error { + var htlcWitnessType input.StandardWitnessType + if h.isTaproot() { + htlcWitnessType = input.TaprootHtlcOfferedRemoteTimeout + } else { + htlcWitnessType = input.HtlcOfferedRemoteTimeout + } + + sweepInput := input.NewCsvInputWithCltv( + &h.htlcResolution.ClaimOutpoint, htlcWitnessType, + &h.htlcResolution.SweepSignDesc, h.broadcastHeight, + h.htlcResolution.CsvDelay, h.htlcResolution.Expiry, + ) + + // Calculate the budget. + // + // TODO(yy): the budget is twice the output's value, which is needed as + // we don't force sweep the output now. To prevent cascading force + // closes, we use all its output value plus a wallet input as the + // budget. This is a temporary solution until we can optionally cancel + // the incoming HTLC, more details in, + // - https://github.com/lightningnetwork/lnd/issues/7969 + budget := calculateBudget( + btcutil.Amount(sweepInput.SignDesc().Output.Value), 2, 0, + ) + + log.Infof("%T(%x): offering offered remote timeout HTLC output to "+ + "sweeper with deadline %v and budget=%v at height=%v", + h, h.htlc.RHash[:], h.incomingHTLCExpiryHeight, budget, + h.broadcastHeight) + + _, err := h.Sweeper.SweepInput( + sweepInput, + sweep.Params{ + Budget: budget, + + // This is an outgoing HTLC, so we want to make sure + // that we sweep it before the incoming HTLC expires. + DeadlineHeight: h.incomingHTLCExpiryHeight, + Immediate: immediate, + }, + ) + if err != nil { + return err + } + + return nil +} + // spendHtlcOutput handles the initial spend of an HTLC output via the timeout // clause. If this is our local commitment, the second-level timeout TX will be // used to spend the output into the next stage. If this is the remote @@ -582,8 +634,18 @@ func (h *htlcTimeoutResolver) spendHtlcOutput( return nil, err } - // If we have no SignDetails, and we haven't already sent the output to - // the utxo nursery, then we'll do so now. + // If this is a remote commitment there's no second level timeout txn, + // and we can just send this directly to the sweeper. + case h.htlcResolution.SignedTimeoutTx == nil && !h.outputIncubating: + if err := h.sweepDirectHtlcOutput(immediate); err != nil { + log.Errorf("Sending direct spend to sweeper: %v", err) + + return nil, err + } + + // If we have a SignedTimeoutTx but no SignDetails, this is a local + // commitment for a non-anchor channel, so we'll send it to the utxo + // nursery. case h.htlcResolution.SignDetails == nil && !h.outputIncubating: if err := h.sendSecondLevelTxLegacy(); err != nil { log.Errorf("Sending timeout tx to nursery: %v", err) @@ -690,6 +752,13 @@ func (h *htlcTimeoutResolver) handleCommitSpend( ) switch { + + // If we swept an HTLC directly off the remote party's commitment + // transaction, then we can exit here as there's no second level sweep + // to do. + case h.htlcResolution.SignedTimeoutTx == nil: + break + // If the sweeper is handling the second level transaction, wait for // the CSV and possible CLTV lock to expire, before sweeping the output // on the second-level. @@ -763,6 +832,7 @@ func (h *htlcTimeoutResolver) handleCommitSpend( h.htlcResolution.CsvDelay, uint32(commitSpend.SpendingHeight), h.htlc.RHash, ) + // Calculate the budget for this sweep. budget := calculateBudget( btcutil.Amount(inp.SignDesc().Output.Value), @@ -800,6 +870,7 @@ func (h *htlcTimeoutResolver) handleCommitSpend( case h.htlcResolution.SignedTimeoutTx != nil: log.Infof("%T(%v): waiting for nursery/sweeper to spend CSV "+ "delayed output", h, claimOutpoint) + sweepTx, err := waitForSpend( &claimOutpoint, h.htlcResolution.SweepSignDesc.Output.PkScript, @@ -866,9 +937,11 @@ func (h *htlcTimeoutResolver) IsResolved() bool { // report returns a report on the resolution state of the contract. func (h *htlcTimeoutResolver) report() *ContractReport { - // If the sign details are nil, the report will be created by handled - // by the nursery. - if h.htlcResolution.SignDetails == nil { + // If we have a SignedTimeoutTx but no SignDetails, this is a local + // commitment for a non-anchor channel, which was handled by the utxo + // nursery. + if h.htlcResolution.SignDetails == nil && h. + htlcResolution.SignedTimeoutTx != nil { return nil } @@ -888,13 +961,20 @@ func (h *htlcTimeoutResolver) initReport() { ) } + // If there's no timeout transaction, then we're already effectively in + // level two. + stage := uint32(1) + if h.htlcResolution.SignedTimeoutTx == nil { + stage = 2 + } + h.currentReport = ContractReport{ Outpoint: h.htlcResolution.ClaimOutpoint, Type: ReportOutputOutgoingHtlc, Amount: finalAmt, MaturityHeight: h.htlcResolution.Expiry, LimboBalance: finalAmt, - Stage: 1, + Stage: stage, } } diff --git a/contractcourt/htlc_timeout_resolver_test.go b/contractcourt/htlc_timeout_resolver_test.go index 8ccd4aaaca..47be71d3ec 100644 --- a/contractcourt/htlc_timeout_resolver_test.go +++ b/contractcourt/htlc_timeout_resolver_test.go @@ -294,12 +294,13 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { chainCfg := ChannelArbitratorConfig{ ChainArbitratorConfig: ChainArbitratorConfig{ Notifier: notifier, + Sweeper: newMockSweeper(), PreimageDB: witnessBeacon, IncubateOutputs: func(wire.OutPoint, fn.Option[lnwallet.OutgoingHtlcResolution], fn.Option[lnwallet.IncomingHtlcResolution], - uint32, fn.Option[int32], - ) error { + uint32, fn.Option[int32]) error { + incubateChan <- struct{}{} return nil }, @@ -311,17 +312,19 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { } resolutionChan <- msgs[0] + return nil }, Budget: *DefaultBudgetConfig(), QueryIncomingCircuit: func(circuit models.CircuitKey, ) *models.CircuitKey { + return nil }, }, PutResolverReport: func(_ kvdb.RwTx, - _ *channeldb.ResolverReport, - ) error { + _ *channeldb.ResolverReport) error { + return nil }, } @@ -329,8 +332,8 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { cfg := ResolverConfig{ ChannelArbitratorConfig: chainCfg, Checkpoint: func(_ ContractResolver, - reports ...*channeldb.ResolverReport, - ) error { + reports ...*channeldb.ResolverReport) error { + checkPointChan <- struct{}{} // Send all of our reports into the channel. @@ -367,13 +370,15 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { if testCase.timeout { timeoutTxID := timeoutTx.TxHash() - reports = append(reports, &channeldb.ResolverReport{ - OutPoint: timeoutTx.TxIn[0].PreviousOutPoint, + report := &channeldb.ResolverReport{ + OutPoint: timeoutTx.TxIn[0].PreviousOutPoint, //nolint:lll Amount: testHtlcAmt.ToSatoshis(), - ResolverType: channeldb.ResolverTypeOutgoingHtlc, - ResolverOutcome: channeldb.ResolverOutcomeFirstStage, + ResolverType: channeldb.ResolverTypeOutgoingHtlc, //nolint:lll + ResolverOutcome: channeldb.ResolverOutcomeFirstStage, //nolint:lll SpendTxID: &timeoutTxID, - }) + } + + reports = append(reports, report) } } @@ -391,10 +396,21 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { } }() - // As the output isn't yet in the nursery, we expect that we - // should receive an incubation request. + // If this is a remote commit, then we expct the outputs should receive + // an incubation request to go through the sweeper, otherwise the + // nursery. + var sweepChan chan input.Input + if testCase.remoteCommit { + mockSweeper, ok := resolver.Sweeper.(*mockSweeper) + require.True(t, ok) + sweepChan = mockSweeper.sweptInputs + } + + // The output should be offered to either the sweeper or + // the nursery. select { case <-incubateChan: + case <-sweepChan: case err := <-resolveErr: t.Fatalf("unable to resolve HTLC: %v", err) case <-time.After(time.Second * 5): @@ -450,7 +466,6 @@ func testHtlcTimeoutResolver(t *testing.T, testCase htlcTimeoutTestCase) { t.Fatalf("resolution not sent") } } else { - // Otherwise, the HTLC should now timeout. First, we // should get a resolution message with a populated // failure message. @@ -559,10 +574,6 @@ func TestHtlcTimeoutSingleStage(t *testing.T) { } checkpoints := []checkpoint{ - { - // Output should be handed off to the nursery. - incubating: true, - }, { // We send a confirmation the sweep tx from published // by the nursery. @@ -594,7 +605,7 @@ func TestHtlcTimeoutSingleStage(t *testing.T) { // After the sweep has confirmed, we expect the // checkpoint to be resolved, and with the above // report. - incubating: true, + incubating: false, resolved: true, reports: []*channeldb.ResolverReport{ claim, @@ -849,9 +860,9 @@ func TestHtlcTimeoutSingleStageRemoteSpend(t *testing.T) { ) } -// TestHtlcTimeoutSecondStageRemoteSpend tests that when a remite commitment -// confirms, and the remote spends the output using the success tx, we -// properly detect this and extract the preimage. +// TestHtlcTimeoutSecondStageRemoteSpend tests that when a remote commitment +// confirms, and the remote spends the output using the success tx, we properly +// detect this and extract the preimage. func TestHtlcTimeoutSecondStageRemoteSpend(t *testing.T) { commitOutpoint := wire.OutPoint{Index: 2} @@ -895,10 +906,6 @@ func TestHtlcTimeoutSecondStageRemoteSpend(t *testing.T) { } checkpoints := []checkpoint{ - { - // Output should be handed off to the nursery. - incubating: true, - }, { // We send a confirmation for the remote's second layer // success transcation. @@ -944,7 +951,7 @@ func TestHtlcTimeoutSecondStageRemoteSpend(t *testing.T) { // After the sweep has confirmed, we expect the // checkpoint to be resolved, and with the above // report. - incubating: true, + incubating: false, resolved: true, reports: []*channeldb.ResolverReport{ claim, @@ -1321,8 +1328,8 @@ func TestHtlcTimeoutSecondStageSweeperRemoteSpend(t *testing.T) { } func testHtlcTimeout(t *testing.T, resolution lnwallet.OutgoingHtlcResolution, - checkpoints []checkpoint, -) { + checkpoints []checkpoint) { + t.Helper() defer timeout()()