contractcourt: unify the lease specific HTLC sweeping logic

In this commit, we consolidate the _lease specific_ logic for the
success and timeout HTLC resolvers. We do this with the addition of a
new struct which is then composed via struct embedding with the two
existing structs. This fixes a flake in the integration tests by
ensuring the height is set up front, rather than eventually once the
height matches the lock time.

contractcourt/channel_arbitrator.go

@@ -2146,6 +2146,9 @@ func (c *ChannelArbitrator) prepContractResolutions(
 				resolver := newSuccessResolver(
 					resolution, height, htlc, resolverCfg,
 				)
+				if chanState != nil {
+					resolver.SupplementState(chanState)
+				}
 				htlcResolvers = append(htlcResolvers, resolver)
 			}
 
@@ -2204,6 +2207,9 @@ func (c *ChannelArbitrator) prepContractResolutions(
 					resolution, height, htlc,
 					resolverCfg,
 				)
+				if chanState != nil {
+					resolver.SupplementState(chanState)
+				}
 				htlcResolvers = append(htlcResolvers, resolver)
 			}
 

contractcourt/htlc_incoming_contest_resolver.go

@@ -439,13 +439,6 @@ func (h *htlcIncomingContestResolver) Supplement(htlc channeldb.HTLC) {
 	h.htlc = htlc
 }
 
-// SupplementState allows the user of a ContractResolver to supplement it with
-// state required for the proper resolution of a contract.
-//
-// NOTE: Part of the ContractResolver interface.
-func (h *htlcIncomingContestResolver) SupplementState(_ *channeldb.OpenChannel) {
-}
-
 // decodePayload (re)decodes the hop payload of a received htlc.
 func (h *htlcIncomingContestResolver) decodePayload() (*hop.Payload,
 	[]byte, error) {

contractcourt/htlc_lease_resolver.go

@@ -0,0 +1,84 @@
+package contractcourt
+
+import (
+	"math"
+
+	"github.com/btcsuite/btcd/wire"
+	"github.com/lightningnetwork/lnd/chainntnfs"
+	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/lightningnetwork/lnd/input"
+)
+
+// htlcLeaseResolver is a struct that houses the lease specific HTLC resolution
+// logic. This includes deriving the _true_ waiting height, as well as the
+// input to offer to the sweeper.
+type htlcLeaseResolver struct {
+	// channelInitiator denotes whether the party responsible for resolving
+	// the contract initiated the channel.
+	channelInitiator bool
+
+	// leaseExpiry denotes the additional waiting period the contract must
+	// hold until it can be resolved. This waiting period is known as the
+	// expiration of a script-enforced leased channel and only applies to
+	// the channel initiator.
+	//
+	// NOTE: This value should only be set when the contract belongs to a
+	// leased channel.
+	leaseExpiry uint32
+}
+
+// hasCLTV denotes whether the resolver must wait for an additional CLTV to
+// expire before resolving the contract.
+func (h *htlcLeaseResolver) hasCLTV() bool {
+	return h.channelInitiator && h.leaseExpiry > 0
+}
+
+// deriveWaitHeight computes the height the resolver needs to wait until it can
+// sweep the input.
+func (h *htlcLeaseResolver) deriveWaitHeight(csvDelay uint32,
+	commitSpend *chainntnfs.SpendDetail) uint32 {
+
+	waitHeight := uint32(commitSpend.SpendingHeight) + csvDelay - 1
+	if h.hasCLTV() {
+		waitHeight = uint32(math.Max(
+			float64(waitHeight), float64(h.leaseExpiry),
+		))
+	}
+
+	return waitHeight
+}
+
+// makeSweepInput constructs the type of input (either just csv or csv+ctlv) to
+// send to the sweeper so the output can ultimately be swept.
+func (h *htlcLeaseResolver) makeSweepInput(op *wire.OutPoint,
+	wType, cltvWtype input.StandardWitnessType,
+	signDesc *input.SignDescriptor,
+	csvDelay, broadcastHeight uint32, payHash [32]byte) *input.BaseInput {
+
+	if h.hasCLTV() {
+		log.Infof("%T(%x): CSV and CLTV locks expired, offering "+
+			"second-layer output to sweeper: %v", h, payHash, op)
+
+		return input.NewCsvInputWithCltv(
+			op, cltvWtype, signDesc,
+			broadcastHeight, csvDelay,
+			h.leaseExpiry,
+		)
+	}
+
+	log.Infof("%T(%x): CSV lock expired, offering second-layer output to "+
+		"sweeper: %v", h, payHash, op)
+
+	return input.NewCsvInput(op, wType, signDesc, broadcastHeight, csvDelay)
+}
+
+// SupplementState allows the user of a ContractResolver to supplement it with
+// state required for the proper resolution of a contract.
+//
+// NOTE: Part of the ContractResolver interface.
+func (h *htlcLeaseResolver) SupplementState(state *channeldb.OpenChannel) {
+	if state.ChanType.HasLeaseExpiration() {
+		h.leaseExpiry = state.ThawHeight
+	}
+	h.channelInitiator = state.IsInitiator
+}

contractcourt/htlc_outgoing_contest_resolver.go

@@ -190,14 +190,6 @@ func (h *htlcOutgoingContestResolver) IsResolved() bool {
 	return h.resolved
 }
 
-// SupplementState allows the user of a ContractResolver to supplement it with
-// state required for the proper resolution of a contract.
-//
-// NOTE: Part of the ContractResolver interface.
-func (h *htlcOutgoingContestResolver) SupplementState(state *channeldb.OpenChannel) {
-	h.htlcTimeoutResolver.SupplementState(state)
-}
-
 // Encode writes an encoded version of the ContractResolver into the passed
 // Writer.
 //

contractcourt/htlc_success_resolver.go

@@ -67,6 +67,8 @@ type htlcSuccessResolver struct {
 	reportLock sync.Mutex
 
 	contractResolverKit
+
+	htlcLeaseResolver
 }
 
 // newSuccessResolver instanties a new htlc success resolver.
@@ -292,9 +294,12 @@ func (h *htlcSuccessResolver) broadcastReSignedSuccessTx() (
 		}
 	}
 
-	// The HTLC success tx has a CSV lock that we must wait for.
+	// The HTLC success tx has a CSV lock that we must wait for, and if
-	waitHeight := uint32(commitSpend.SpendingHeight) +
+	// this is a lease enforced channel and we're the imitator, we may need
-		h.htlcResolution.CsvDelay - 1
+	// to wait for longer.
+	waitHeight := h.deriveWaitHeight(
+		h.htlcResolution.CsvDelay, commitSpend,
+	)
 
 	// Now that the sweeper has broadcasted the second-level transaction,
 	// it has confirmed, and we have checkpointed our state, we'll sweep
@@ -305,8 +310,14 @@ func (h *htlcSuccessResolver) broadcastReSignedSuccessTx() (
 	h.currentReport.MaturityHeight = waitHeight
 	h.reportLock.Unlock()
 
-	log.Infof("%T(%x): waiting for CSV lock to expire at height %v",
+	if h.hasCLTV() {
-		h, h.htlc.RHash[:], waitHeight)
+		log.Infof("%T(%x): waiting for CSV and CLTV lock to "+
+			"expire at height %v", h, h.htlc.RHash[:],
+			waitHeight)
+	} else {
+		log.Infof("%T(%x): waiting for CSV lock to expire at "+
+			"height %v", h, h.htlc.RHash[:], waitHeight)
+	}
 
 	err := waitForHeight(waitHeight, h.Notifier, h.quit)
 	if err != nil {
@@ -327,10 +338,14 @@ func (h *htlcSuccessResolver) broadcastReSignedSuccessTx() (
 	log.Infof("%T(%x): CSV lock expired, offering second-layer "+
 		"output to sweeper: %v", h, h.htlc.RHash[:], op)
 
-	inp := input.NewCsvInput(
+	// Let the sweeper sweep the second-level output now that the
+	// CSV/CLTV locks have expired.
+	inp := h.makeSweepInput(
 		op, input.HtlcAcceptedSuccessSecondLevel,
-		&h.htlcResolution.SweepSignDesc, h.broadcastHeight,
+		input.LeaseHtlcAcceptedSuccessSecondLevel,
-		h.htlcResolution.CsvDelay,
+		&h.htlcResolution.SweepSignDesc,
+		h.htlcResolution.CsvDelay, h.broadcastHeight,
+		h.htlc.RHash,
 	)
 	_, err = h.Sweeper.SweepInput(
 		inp,
@@ -626,13 +641,6 @@ func (h *htlcSuccessResolver) Supplement(htlc channeldb.HTLC) {
 	h.htlc = htlc
 }
 
-// SupplementState allows the user of a ContractResolver to supplement it with
-// state required for the proper resolution of a contract.
-//
-// NOTE: Part of the ContractResolver interface.
-func (h *htlcSuccessResolver) SupplementState(_ *channeldb.OpenChannel) {
-}
-
 // HtlcPoint returns the htlc's outpoint on the commitment tx.
 //
 // NOTE: Part of the htlcContractResolver interface.

contractcourt/htlc_timeout_resolver.go

@@ -4,7 +4,6 @@ import (
 	"encoding/binary"
 	"fmt"
 	"io"
-	"math"
 	"sync"
 
 	"github.com/btcsuite/btcd/btcutil"
@@ -48,19 +47,6 @@ type htlcTimeoutResolver struct {
 	// htlc contains information on the htlc that we are resolving on-chain.
 	htlc channeldb.HTLC
 
-	// channelInitiator denotes whether the party responsible for resolving
-	// the contract initiated the channel.
-	channelInitiator bool
-
-	// leaseExpiry denotes the additional waiting period the contract must
-	// hold until it can be resolved. This waiting period is known as the
-	// expiration of a script-enforced leased channel and only applies to
-	// the channel initiator.
-	//
-	// NOTE: This value should only be set when the contract belongs to a
-	// leased channel.
-	leaseExpiry uint32
-
 	// currentReport stores the current state of the resolver for reporting
 	// over the rpc interface. This should only be reported in case we have
 	// a non-nil SignDetails on the htlcResolution, otherwise the nursery
@@ -71,6 +57,8 @@ type htlcTimeoutResolver struct {
 	reportLock sync.Mutex
 
 	contractResolverKit
+
+	htlcLeaseResolver
 }
 
 // newTimeoutResolver instantiates a new timeout htlc resolver.
@@ -444,13 +432,9 @@ func (h *htlcTimeoutResolver) handleCommitSpend(
 	// the CSV and possible CLTV lock to expire, before sweeping the output
 	// on the second-level.
 	case h.htlcResolution.SignDetails != nil:
-		waitHeight := uint32(commitSpend.SpendingHeight) +
+		waitHeight := h.deriveWaitHeight(
-			h.htlcResolution.CsvDelay - 1
+			h.htlcResolution.CsvDelay, commitSpend,
-		if h.hasCLTV() {
+		)
-			waitHeight = uint32(math.Max(
-				float64(waitHeight), float64(h.leaseExpiry),
-			))
-		}
 
 		h.reportLock.Lock()
 		h.currentReport.Stage = 2
@@ -483,27 +467,13 @@ func (h *htlcTimeoutResolver) handleCommitSpend(
 
 		// Let the sweeper sweep the second-level output now that the
 		// CSV/CLTV locks have expired.
-		var inp *input.BaseInput
+		inp := h.makeSweepInput(
-		if h.hasCLTV() {
+			op, input.HtlcOfferedTimeoutSecondLevel,
-			log.Infof("%T(%x): CSV and CLTV locks expired, offering "+
+			input.LeaseHtlcOfferedTimeoutSecondLevel,
-				"second-layer output to sweeper: %v", h,
+			&h.htlcResolution.SweepSignDesc,
-				h.htlc.RHash[:], op)
+			h.htlcResolution.CsvDelay, h.broadcastHeight,
-			inp = input.NewCsvInputWithCltv(
+			h.htlc.RHash,
-				op, input.LeaseHtlcOfferedTimeoutSecondLevel,
+		)
-				&h.htlcResolution.SweepSignDesc,
-				h.broadcastHeight, h.htlcResolution.CsvDelay,
-				h.leaseExpiry,
-			)
-		} else {
-			log.Infof("%T(%x): CSV lock expired, offering "+
-				"second-layer output to sweeper: %v", h,
-				h.htlc.RHash[:], op)
-			inp = input.NewCsvInput(
-				op, input.HtlcOfferedTimeoutSecondLevel,
-				&h.htlcResolution.SweepSignDesc,
-				h.broadcastHeight, h.htlcResolution.CsvDelay,
-			)
-		}
 		_, err = h.Sweeper.SweepInput(
 			inp,
 			sweep.Params{
@@ -716,23 +686,6 @@ func (h *htlcTimeoutResolver) Supplement(htlc channeldb.HTLC) {
 	h.htlc = htlc
 }
 
-// SupplementState allows the user of a ContractResolver to supplement it with
-// state required for the proper resolution of a contract.
-//
-// NOTE: Part of the ContractResolver interface.
-func (h *htlcTimeoutResolver) SupplementState(state *channeldb.OpenChannel) {
-	if state.ChanType.HasLeaseExpiration() {
-		h.leaseExpiry = state.ThawHeight
-	}
-	h.channelInitiator = state.IsInitiator
-}
-
-// hasCLTV denotes whether the resolver must wait for an additional CLTV to
-// expire before resolving the contract.
-func (h *htlcTimeoutResolver) hasCLTV() bool {
-	return h.channelInitiator && h.leaseExpiry > 0
-}
-
 // HtlcPoint returns the htlc's outpoint on the commitment tx.
 //
 // NOTE: Part of the htlcContractResolver interface.

docs/release-notes/release-notes-0.15.0.md

@@ -125,12 +125,6 @@ compact filters and block/block headers.
 * [Fixed deadlock in the invoice registry](
   https://github.com/lightningnetwork/lnd/pull/6600)
 
-## Neutrino
-
-* [New neutrino sub-server](https://github.com/lightningnetwork/lnd/pull/5652)
-  capable of status checks, adding, disconnecting and listing
-  peers, fetching compact filters and block/block headers.
-
 * [Added signature length
   validation](https://github.com/lightningnetwork/lnd/pull/6314) when calling
   `NewSigFromRawSignature`.
@@ -187,6 +181,8 @@ from occurring that would result in an erroneous force close.](https://github.co
 * [Fixed a wrong channel status inheritance used in `migration26` and
   `migration27`](https://github.com/lightningnetwork/lnd/pull/6563).
 
+* [Fixes an issue related to HTLCs on lease enforced channels that can lead to itest flakes](https://github.com/lightningnetwork/lnd/pull/6605/files)
+
 ## Routing
 
 * [Add a new `time_pref` parameter to the QueryRoutes and SendPayment APIs](https://github.com/lightningnetwork/lnd/pull/6024) that