Merge branch '0-18-4-branch-rc1-9062' into 0-18-4-branch-rc1

contractcourt/htlc_timeout_resolver.go

@@ -549,7 +549,6 @@ func (h *htlcTimeoutResolver) sweepSecondLevelTx(immediate bool) error {
 		return err
 	}
 
-	// TODO(yy): checkpoint here?
 	return err
 }
 
@@ -573,6 +572,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
@@ -593,8 +645,18 @@ func (h *htlcTimeoutResolver) spendHtlcOutput(
 			return nil, err
 		}
 
-	// If we have no SignDetails, and we haven't already sent the output to
+	// If this is a remote commitment there's no second level timeout txn,
-	// the utxo nursery, then we'll do so now.
+	// 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)
@@ -701,6 +763,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.
@@ -774,6 +843,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),
@@ -811,6 +881,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,
@@ -877,9 +948,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
+	// If we have a SignedTimeoutTx but no SignDetails, this is a local
-	// by the nursery.
+	// commitment for a non-anchor channel, which was handled by the utxo
-	if h.htlcResolution.SignDetails == nil {
+	// nursery.
+	if h.htlcResolution.SignDetails == nil && h.
+		htlcResolution.SignedTimeoutTx != nil {
 		return nil
 	}
 
@@ -899,13 +972,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,
 	}
 }
 

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
+type htlcTimeoutTestCase struct {
-// variations of possible local+remote spends.
+	// name is a human readable description of the test case.
-func TestHtlcTimeoutResolver(t *testing.T) {
+	name string
-	t.Parallel()
 
+	// 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 {
+	return []htlcTimeoutTestCase{
-		// 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
-	}{
 		// 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,17 +266,25 @@ 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)
@@ -285,6 +294,7 @@ func TestHtlcTimeoutResolver(t *testing.T) {
 	chainCfg := ChannelArbitratorConfig{
 		ChainArbitratorConfig: ChainArbitratorConfig{
 			Notifier:   notifier,
+			Sweeper:    newMockSweeper(),
 			PreimageDB: witnessBeacon,
 			IncubateOutputs: func(wire.OutPoint,
 				fn.Option[lnwallet.OutgoingHtlcResolution],
@@ -302,10 +312,13 @@ func TestHtlcTimeoutResolver(t *testing.T) {
 				}
 
 				resolutionChan <- msgs[0]
+
 				return nil
 			},
 			Budget: *DefaultBudgetConfig(),
-				QueryIncomingCircuit: func(circuit models.CircuitKey) *models.CircuitKey {
+			QueryIncomingCircuit: func(circuit models.CircuitKey,
+			) *models.CircuitKey {
+
 				return nil
 			},
 		},
@@ -357,13 +370,15 @@ func TestHtlcTimeoutResolver(t *testing.T) {
 
 		if testCase.timeout {
 			timeoutTxID := timeoutTx.TxHash()
-				reports = append(reports, &channeldb.ResolverReport{
+			report := &channeldb.ResolverReport{
-					OutPoint:        timeoutTx.TxIn[0].PreviousOutPoint,
+				OutPoint:        timeoutTx.TxIn[0].PreviousOutPoint, //nolint:lll
 				Amount:          testHtlcAmt.ToSatoshis(),
-					ResolverType:    channeldb.ResolverTypeOutgoingHtlc,
+				ResolverType:    channeldb.ResolverTypeOutgoingHtlc,  //nolint:lll
-					ResolverOutcome: channeldb.ResolverOutcomeFirstStage,
+				ResolverOutcome: channeldb.ResolverOutcomeFirstStage, //nolint:lll
 				SpendTxID:       &timeoutTxID,
-				})
+			}
+
+			reports = append(reports, report)
 		}
 	}
 
@@ -381,10 +396,21 @@ func TestHtlcTimeoutResolver(t *testing.T) {
 		}
 	}()
 
-		// At the output isn't yet in the nursery, we expect that we
+	// If this is a remote commit, then we expct the outputs should receive
-		// should receive an incubation request.
+	// 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):
@@ -440,7 +466,6 @@ func TestHtlcTimeoutResolver(t *testing.T) {
 			t.Fatalf("resolution not sent")
 		}
 	} else {
-
 		// Otherwise, the HTLC should now timeout.  First, we
 		// should get a resolution message with a populated
 		// failure message.
@@ -503,6 +528,19 @@ func TestHtlcTimeoutResolver(t *testing.T) {
 		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()
+
+	testCases := genHtlcTimeoutTestCases()
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			testHtlcTimeoutResolver(t, testCase)
+		})
+	}
 }
 
 // NOTE: the following tests essentially checks many of the same scenarios as
@@ -536,15 +574,12 @@ 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.
 			preCheckpoint: func(ctx *htlcResolverTestContext,
 				_ bool) error {
+
 				// The nursery will create and publish a sweep
 				// tx.
 				ctx.notifier.SpendChan <- &chainntnfs.SpendDetail{
@@ -570,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,
@@ -653,6 +688,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,
@@ -824,9 +860,9 @@ func TestHtlcTimeoutSingleStageRemoteSpend(t *testing.T) {
 	)
 }
 
-// TestHtlcTimeoutSecondStageRemoteSpend tests that when a remite commitment
+// TestHtlcTimeoutSecondStageRemoteSpend tests that when a remote commitment
-// confirms, and the remote spends the output using the success tx, we
+// confirms, and the remote spends the output using the success tx, we properly
-// properly detect this and extract the preimage.
+// detect this and extract the preimage.
 func TestHtlcTimeoutSecondStageRemoteSpend(t *testing.T) {
 	commitOutpoint := wire.OutPoint{Index: 2}
 
@@ -870,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.
@@ -919,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,
@@ -1298,6 +1330,8 @@ func TestHtlcTimeoutSecondStageSweeperRemoteSpend(t *testing.T) {
 func testHtlcTimeout(t *testing.T, resolution lnwallet.OutgoingHtlcResolution,
 	checkpoints []checkpoint) {
 
+	t.Helper()
+
 	defer timeout()()
 
 	// We first run the resolver from start to finish, ensuring it gets