Merge pull request #5148 from yyforyongyu/4215-deadline-aware

sweeper+contractcourt: deadline aware in sweeping anchors

contractcourt/chain_arbitrator.go

@@ -241,7 +241,7 @@ type arbChannel struct {
 // commitment transactions.
 //
 // NOTE: Part of the ArbChannel interface.
-func (a *arbChannel) NewAnchorResolutions() ([]*lnwallet.AnchorResolution,
+func (a *arbChannel) NewAnchorResolutions() (*lnwallet.AnchorResolutions,
 	error) {
 
 	// Get a fresh copy of the database state to base the anchor resolutions

contractcourt/channel_arbitrator.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"math"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -31,8 +32,10 @@ var (
 
 const (
 	// anchorSweepConfTarget is the conf target used when sweeping
-	// commitment anchors.
+	// commitment anchors. This value is only used when the commitment
-	anchorSweepConfTarget = 6
+	// transaction has no valid HTLCs for determining a confirmation
+	// deadline.
+	anchorSweepConfTarget = 144
 
 	// arbitratorBlockBufferSize is the size of the buffer we give to each
 	// channel arbitrator.
@@ -91,7 +94,7 @@ type ArbChannel interface {
 
 	// NewAnchorResolutions returns the anchor resolutions for currently
 	// valid commitment transactions.
-	NewAnchorResolutions() ([]*lnwallet.AnchorResolution, error)
+	NewAnchorResolutions() (*lnwallet.AnchorResolutions, error)
 }
 
 // ChannelArbitratorConfig contains all the functionality that the
@@ -1087,14 +1090,24 @@ func (c *ChannelArbitrator) stateStep(
 // sweepAnchors offers all given anchor resolutions to the sweeper. It requests
 // sweeping at the minimum fee rate. This fee rate can be upped manually by the
 // user via the BumpFee rpc.
-func (c *ChannelArbitrator) sweepAnchors(anchors []*lnwallet.AnchorResolution,
+func (c *ChannelArbitrator) sweepAnchors(anchors *lnwallet.AnchorResolutions,
 	heightHint uint32) error {
 
 	// Use the chan id as the exclusive group. This prevents any of the
 	// anchors from being batched together.
 	exclusiveGroup := c.cfg.ShortChanID.ToUint64()
 
-	for _, anchor := range anchors {
+	// sweepWithDeadline is a helper closure that takes an anchor
+	// resolution and sweeps it with its corresponding deadline.
+	sweepWithDeadline := func(anchor *lnwallet.AnchorResolution,
+		htlcs htlcSet) error {
+
+		// Find the deadline for this specific anchor.
+		deadline, err := c.findCommitmentDeadline(heightHint, htlcs)
+		if err != nil {
+			return err
+		}
+
 		log.Debugf("ChannelArbitrator(%v): pre-confirmation sweep of "+
 			"anchor of tx %v", c.cfg.ChanPoint, anchor.CommitAnchor)
 
@@ -1118,11 +1131,11 @@ func (c *ChannelArbitrator) sweepAnchors(anchors []*lnwallet.AnchorResolution,
 		// Also signal that this is a force sweep, so that the anchor
 		// will be swept even if it isn't economical purely based on the
 		// anchor value.
-		_, err := c.cfg.Sweeper.SweepInput(
+		_, err = c.cfg.Sweeper.SweepInput(
 			&anchorInput,
 			sweep.Params{
 				Fee: sweep.FeePreference{
-					ConfTarget: anchorSweepConfTarget,
+					ConfTarget: deadline,
 				},
 				Force:          true,
 				ExclusiveGroup: &exclusiveGroup,
@@ -1131,11 +1144,129 @@ func (c *ChannelArbitrator) sweepAnchors(anchors []*lnwallet.AnchorResolution,
 		if err != nil {
 			return err
 		}
+
+		return nil
+	}
+
+	// Sweep anchors based on different HTLC sets. Notice the HTLC sets may
+	// differ across commitments, thus their deadline values could vary.
+	for htlcSet, htlcs := range c.activeHTLCs {
+		switch {
+		case htlcSet == LocalHtlcSet && anchors.Local != nil:
+			err := sweepWithDeadline(anchors.Local, htlcs)
+			if err != nil {
+				return err
+			}
+
+		case htlcSet == RemoteHtlcSet && anchors.Remote != nil:
+			err := sweepWithDeadline(anchors.Remote, htlcs)
+			if err != nil {
+				return err
+			}
+
+		case htlcSet == RemotePendingHtlcSet &&
+			anchors.RemotePending != nil:
+
+			err := sweepWithDeadline(anchors.RemotePending, htlcs)
+			if err != nil {
+				return err
+			}
+		}
 	}
 
 	return nil
 }
 
+// findCommitmentDeadline finds the deadline (relative block height) for a
+// commitment transaction by extracting the minimum CLTV from its HTLCs. From
+// our PoV, the deadline is defined to be the smaller of,
+//   - the least CLTV from outgoing HTLCs,  or,
+//   - the least CLTV from incoming HTLCs if the preimage is available.
+//
+// Note: when the deadline turns out to be 0 blocks, we will replace it with 1
+// block because our fee estimator doesn't allow a 0 conf target. This also
+// means we've left behind and should increase our fee to make the transaction
+// confirmed asap.
+func (c *ChannelArbitrator) findCommitmentDeadline(heightHint uint32,
+	htlcs htlcSet) (uint32, error) {
+
+	deadlineMinHeight := uint32(math.MaxUint32)
+
+	// First, iterate through the outgoingHTLCs to find the lowest CLTV
+	// value.
+	for _, htlc := range htlcs.outgoingHTLCs {
+		// Skip if the HTLC is dust.
+		if htlc.OutputIndex < 0 {
+			log.Debugf("ChannelArbitrator(%v): skipped deadline "+
+				"for dust htlc=%x",
+				c.cfg.ChanPoint, htlc.RHash[:])
+
+			continue
+		}
+
+		if htlc.RefundTimeout < deadlineMinHeight {
+			deadlineMinHeight = htlc.RefundTimeout
+		}
+	}
+
+	// Then going through the incomingHTLCs, and update the minHeight when
+	// conditions met.
+	for _, htlc := range htlcs.incomingHTLCs {
+		// Skip if the HTLC is dust.
+		if htlc.OutputIndex < 0 {
+			log.Debugf("ChannelArbitrator(%v): skipped deadline "+
+				"for dust htlc=%x",
+				c.cfg.ChanPoint, htlc.RHash[:])
+
+			continue
+		}
+
+		// Since it's an HTLC sent to us, check if we have preimage for
+		// this HTLC.
+		preimageAvailable, err := c.isPreimageAvailable(htlc.RHash)
+		if err != nil {
+			return 0, err
+		}
+
+		if !preimageAvailable {
+			continue
+		}
+
+		if htlc.RefundTimeout < deadlineMinHeight {
+			deadlineMinHeight = htlc.RefundTimeout
+		}
+	}
+
+	// Calculate the deadline. There are two cases to be handled here,
+	//   - when the deadlineMinHeight never gets updated, which could
+	//     happen when we have no outgoing HTLCs, and, for incoming HTLCs,
+	//       * either we have none, or,
+	//       * none of the HTLCs are preimageAvailable.
+	//   - when our deadlineMinHeight is no greater than the heightHint,
+	//     which means we are behind our schedule.
+	deadline := deadlineMinHeight - heightHint
+	switch {
+	// When we couldn't find a deadline height from our HTLCs, we will fall
+	// back to the default value.
+	case deadlineMinHeight == math.MaxUint32:
+		deadline = anchorSweepConfTarget
+
+	// When the deadline is passed, we will fall back to the smallest conf
+	// target (1 block).
+	case deadlineMinHeight <= heightHint:
+		log.Warnf("ChannelArbitrator(%v): deadline is passed with "+
+			"deadlineMinHeight=%d, heightHint=%d",
+			c.cfg.ChanPoint, deadlineMinHeight, heightHint)
+		deadline = 1
+	}
+
+	log.Debugf("ChannelArbitrator(%v): calculated deadline: %d, "+
+		"using deadlineMinHeight=%d, heightHint=%d",
+		c.cfg.ChanPoint, deadline, deadlineMinHeight, heightHint)
+
+	return deadline, nil
+}
+
 // launchResolvers updates the activeResolvers list and starts the resolvers.
 func (c *ChannelArbitrator) launchResolvers(resolvers []ContractResolver) {
 	c.activeResolversLock.Lock()

contractcourt/channel_arbitrator_test.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"sort"
 	"sync"
 	"testing"
 	"time"
@@ -20,8 +21,10 @@ import (
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/kvdb"
 	"github.com/lightningnetwork/lnd/lntest/mock"
+	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/lnwire"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -2093,6 +2096,254 @@ func TestRemoteCloseInitiator(t *testing.T) {
 	}
 }
 
+// TestFindCommitmentDeadline tests the logic used to determine confirmation
+// deadline is implemented as expected.
+func TestFindCommitmentDeadline(t *testing.T) {
+	// Create a testing channel arbitrator.
+	log := &mockArbitratorLog{
+		state:     StateDefault,
+		newStates: make(chan ArbitratorState, 5),
+	}
+	chanArbCtx, err := createTestChannelArbitrator(t, log)
+	require.NoError(t, err, "unable to create ChannelArbitrator")
+
+	// Add a dummy payment hash to the preimage lookup.
+	rHash := [lntypes.PreimageSize]byte{1, 2, 3}
+	mockPreimageDB := newMockWitnessBeacon()
+	mockPreimageDB.lookupPreimage[rHash] = rHash
+
+	// Attack a mock PreimageDB and Registry to channel arbitrator.
+	chanArb := chanArbCtx.chanArb
+	chanArb.cfg.PreimageDB = mockPreimageDB
+	chanArb.cfg.Registry = &mockRegistry{}
+
+	htlcIndexBase := uint64(99)
+	heightHint := uint32(1000)
+	htlcExpiryBase := heightHint + uint32(10)
+
+	// Create four testing HTLCs.
+	htlcDust := channeldb.HTLC{
+		HtlcIndex:     htlcIndexBase + 1,
+		RefundTimeout: htlcExpiryBase + 1,
+		OutputIndex:   -1,
+	}
+	htlcSmallExipry := channeldb.HTLC{
+		HtlcIndex:     htlcIndexBase + 2,
+		RefundTimeout: htlcExpiryBase + 2,
+	}
+
+	htlcPreimage := channeldb.HTLC{
+		HtlcIndex:     htlcIndexBase + 3,
+		RefundTimeout: htlcExpiryBase + 3,
+		RHash:         rHash,
+	}
+	htlcLargeExpiry := channeldb.HTLC{
+		HtlcIndex:     htlcIndexBase + 4,
+		RefundTimeout: htlcExpiryBase + 100,
+	}
+	htlcExpired := channeldb.HTLC{
+		HtlcIndex:     htlcIndexBase + 5,
+		RefundTimeout: heightHint,
+	}
+
+	makeHTLCSet := func(incoming, outgoing channeldb.HTLC) htlcSet {
+		return htlcSet{
+			incomingHTLCs: map[uint64]channeldb.HTLC{
+				incoming.HtlcIndex: incoming,
+			},
+			outgoingHTLCs: map[uint64]channeldb.HTLC{
+				outgoing.HtlcIndex: outgoing,
+			},
+		}
+	}
+
+	testCases := []struct {
+		name     string
+		htlcs    htlcSet
+		err      error
+		deadline uint32
+	}{
+		{
+			// When we have no HTLCs, the default value should be
+			// used.
+			name:     "use default conf target",
+			htlcs:    htlcSet{},
+			err:      nil,
+			deadline: anchorSweepConfTarget,
+		},
+		{
+			// When we have a preimage available in the local HTLC
+			// set, its CLTV should be used.
+			name:     "use htlc with preimage available",
+			htlcs:    makeHTLCSet(htlcPreimage, htlcLargeExpiry),
+			err:      nil,
+			deadline: htlcPreimage.RefundTimeout - heightHint,
+		},
+		{
+			// When the HTLC in the local set is not preimage
+			// available, we should not use its CLTV even its value
+			// is smaller.
+			name:     "use htlc with no preimage available",
+			htlcs:    makeHTLCSet(htlcSmallExipry, htlcLargeExpiry),
+			err:      nil,
+			deadline: htlcLargeExpiry.RefundTimeout - heightHint,
+		},
+		{
+			// When we have dust HTLCs, their CLTVs should NOT be
+			// used even the values are smaller.
+			name:     "ignore dust HTLCs",
+			htlcs:    makeHTLCSet(htlcPreimage, htlcDust),
+			err:      nil,
+			deadline: htlcPreimage.RefundTimeout - heightHint,
+		},
+		{
+			// When we've reached our deadline, use conf target of
+			// 1 as our deadline.
+			name:     "use conf target 1",
+			htlcs:    makeHTLCSet(htlcPreimage, htlcExpired),
+			err:      nil,
+			deadline: 1,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			deadline, err := chanArb.findCommitmentDeadline(
+				heightHint, tc.htlcs,
+			)
+
+			require.Equal(t, tc.err, err)
+			require.Equal(t, tc.deadline, deadline)
+		})
+	}
+
+}
+
+// TestSweepAnchors checks the sweep transactions are created using the
+// expected deadlines for different anchor resolutions.
+func TestSweepAnchors(t *testing.T) {
+	// Create a testing channel arbitrator.
+	log := &mockArbitratorLog{
+		state:     StateDefault,
+		newStates: make(chan ArbitratorState, 5),
+	}
+	chanArbCtx, err := createTestChannelArbitrator(t, log)
+	require.NoError(t, err, "unable to create ChannelArbitrator")
+
+	// Add a dummy payment hash to the preimage lookup.
+	rHash := [lntypes.PreimageSize]byte{1, 2, 3}
+	mockPreimageDB := newMockWitnessBeacon()
+	mockPreimageDB.lookupPreimage[rHash] = rHash
+
+	// Attack a mock PreimageDB and Registry to channel arbitrator.
+	chanArb := chanArbCtx.chanArb
+	chanArb.cfg.PreimageDB = mockPreimageDB
+	chanArb.cfg.Registry = &mockRegistry{}
+
+	// Set current block height.
+	heightHint := uint32(1000)
+	chanArbCtx.chanArb.blocks <- int32(heightHint)
+
+	htlcIndexBase := uint64(99)
+	htlcExpiryBase := heightHint + uint32(10)
+
+	// Create three testing HTLCs.
+	htlcDust := channeldb.HTLC{
+		HtlcIndex:     htlcIndexBase + 1,
+		RefundTimeout: htlcExpiryBase + 1,
+		OutputIndex:   -1,
+	}
+	htlcWithPreimage := channeldb.HTLC{
+		HtlcIndex:     htlcIndexBase + 2,
+		RefundTimeout: htlcExpiryBase + 2,
+		RHash:         rHash,
+	}
+	htlcSmallExipry := channeldb.HTLC{
+		HtlcIndex:     htlcIndexBase + 3,
+		RefundTimeout: htlcExpiryBase + 3,
+	}
+
+	// Setup our local HTLC set such that we will use the HTLC's CLTV from
+	// the incoming HTLC set.
+	expectedLocalDeadline := htlcWithPreimage.RefundTimeout - heightHint
+	chanArb.activeHTLCs[LocalHtlcSet] = htlcSet{
+		incomingHTLCs: map[uint64]channeldb.HTLC{
+			htlcWithPreimage.HtlcIndex: htlcWithPreimage,
+		},
+		outgoingHTLCs: map[uint64]channeldb.HTLC{
+			htlcDust.HtlcIndex: htlcDust,
+		},
+	}
+
+	// Setup our remote HTLC set such that no valid HTLCs can be used, thus
+	// we default to anchorSweepConfTarget.
+	expectedRemoteDeadline := anchorSweepConfTarget
+	chanArb.activeHTLCs[RemoteHtlcSet] = htlcSet{
+		incomingHTLCs: map[uint64]channeldb.HTLC{
+			htlcSmallExipry.HtlcIndex: htlcSmallExipry,
+		},
+		outgoingHTLCs: map[uint64]channeldb.HTLC{
+			htlcDust.HtlcIndex: htlcDust,
+		},
+	}
+
+	// Setup out pending remote HTLC set such that we will use the HTLC's
+	// CLTV from the outgoing HTLC set.
+	expectedPendingDeadline := htlcSmallExipry.RefundTimeout - heightHint
+	chanArb.activeHTLCs[RemotePendingHtlcSet] = htlcSet{
+		incomingHTLCs: map[uint64]channeldb.HTLC{
+			htlcDust.HtlcIndex: htlcDust,
+		},
+		outgoingHTLCs: map[uint64]channeldb.HTLC{
+			htlcSmallExipry.HtlcIndex: htlcSmallExipry,
+		},
+	}
+
+	// Create AnchorResolutions.
+	anchors := &lnwallet.AnchorResolutions{
+		Local: &lnwallet.AnchorResolution{
+			AnchorSignDescriptor: input.SignDescriptor{
+				Output: &wire.TxOut{Value: 1},
+			},
+		},
+		Remote: &lnwallet.AnchorResolution{
+			AnchorSignDescriptor: input.SignDescriptor{
+				Output: &wire.TxOut{Value: 1},
+			},
+		},
+		RemotePending: &lnwallet.AnchorResolution{
+			AnchorSignDescriptor: input.SignDescriptor{
+				Output: &wire.TxOut{Value: 1},
+			},
+		},
+	}
+
+	// Sweep anchors and check there's no error.
+	err = chanArb.sweepAnchors(anchors, heightHint)
+	require.NoError(t, err)
+
+	// Verify deadlines are used as expected.
+	deadlines := chanArbCtx.sweeper.deadlines
+	// Since there's no guarantee of the deadline orders, we sort it here
+	// so they can be compared.
+	sort.Ints(deadlines) // [12, 13, 144]
+	require.EqualValues(
+		t, expectedLocalDeadline, deadlines[0],
+		"local deadline not matched",
+	)
+	require.EqualValues(
+		t, expectedPendingDeadline, deadlines[1],
+		"pending remote deadline not matched",
+	)
+	require.EqualValues(
+		t, expectedRemoteDeadline, deadlines[2],
+		"remote deadline not matched",
+	)
+
+}
+
 // TestChannelArbitratorAnchors asserts that the commitment tx anchor is swept.
 func TestChannelArbitratorAnchors(t *testing.T) {
 	log := &mockArbitratorLog{
@@ -2113,14 +2364,29 @@ func TestChannelArbitratorAnchors(t *testing.T) {
 		reports,
 	)
 
+	// Add a dummy payment hash to the preimage lookup.
+	rHash := [lntypes.PreimageSize]byte{1, 2, 3}
+	mockPreimageDB := newMockWitnessBeacon()
+	mockPreimageDB.lookupPreimage[rHash] = rHash
+
+	// Attack a mock PreimageDB and Registry to channel arbitrator.
 	chanArb := chanArbCtx.chanArb
-	chanArb.cfg.PreimageDB = newMockWitnessBeacon()
+	chanArb.cfg.PreimageDB = mockPreimageDB
 	chanArb.cfg.Registry = &mockRegistry{}
 
 	// Setup two pre-confirmation anchor resolutions on the mock channel.
 	chanArb.cfg.Channel.(*mockChannel).anchorResolutions =
-		[]*lnwallet.AnchorResolution{
+		&lnwallet.AnchorResolutions{
-			{}, {},
+			Local: &lnwallet.AnchorResolution{
+				AnchorSignDescriptor: input.SignDescriptor{
+					Output: &wire.TxOut{Value: 1},
+				},
+			},
+			Remote: &lnwallet.AnchorResolution{
+				AnchorSignDescriptor: input.SignDescriptor{
+					Output: &wire.TxOut{Value: 1},
+				},
+			},
 		}
 
 	if err := chanArb.Start(nil); err != nil {
@@ -2141,6 +2407,41 @@ func TestChannelArbitratorAnchors(t *testing.T) {
 	}
 	chanArb.UpdateContractSignals(signals)
 
+	// Set current block height.
+	heightHint := uint32(1000)
+	chanArbCtx.chanArb.blocks <- int32(heightHint)
+
+	// Create testing HTLCs.
+	htlcExpiryBase := heightHint + uint32(10)
+	htlcWithPreimage := channeldb.HTLC{
+		HtlcIndex:     99,
+		RefundTimeout: htlcExpiryBase + 2,
+		RHash:         rHash,
+		Incoming:      true,
+	}
+	htlc := channeldb.HTLC{
+		HtlcIndex:     100,
+		RefundTimeout: htlcExpiryBase + 3,
+	}
+
+	// We now send two HTLC updates, one for local HTLC set and the other
+	// for remote HTLC set.
+	htlcUpdates <- &ContractUpdate{
+		HtlcKey: LocalHtlcSet,
+		// This will make the deadline of the local anchor resolution
+		// to be htlcWithPreimage's CLTV minus heightHint since the
+		// incoming HTLC (toLocalHTLCs) has a lower CLTV value and is
+		// preimage available.
+		Htlcs: []channeldb.HTLC{htlc, htlcWithPreimage},
+	}
+	htlcUpdates <- &ContractUpdate{
+		HtlcKey: RemoteHtlcSet,
+		// This will make the deadline of the remote anchor resolution
+		// to be htlcWithPreimage's CLTV minus heightHint because the
+		// incoming HTLC (toRemoteHTLCs) has a lower CLTV.
+		Htlcs: []channeldb.HTLC{htlc, htlcWithPreimage},
+	}
+
 	errChan := make(chan error, 1)
 	respChan := make(chan *wire.MsgTx, 1)
 
@@ -2256,6 +2557,20 @@ func TestChannelArbitratorAnchors(t *testing.T) {
 	}
 
 	assertResolverReport(t, reports, expectedReport)
+
+	// We expect two anchor inputs, the local and the remote to be swept.
+	// Thus we should expect there are two deadlines used, both are equal
+	// to htlcWithPreimage's CLTV minus current block height.
+	require.Equal(t, 2, len(chanArbCtx.sweeper.deadlines))
+	require.EqualValues(t,
+		htlcWithPreimage.RefundTimeout-heightHint,
+		chanArbCtx.sweeper.deadlines[0],
+	)
+	require.EqualValues(t,
+		htlcWithPreimage.RefundTimeout-heightHint,
+		chanArbCtx.sweeper.deadlines[1],
+	)
+
 }
 
 // putResolverReportInChannel returns a put report function which will pipe
@@ -2286,13 +2601,16 @@ func assertResolverReport(t *testing.T, reports chan *channeldb.ResolverReport,
 }
 
 type mockChannel struct {
-	anchorResolutions []*lnwallet.AnchorResolution
+	anchorResolutions *lnwallet.AnchorResolutions
 }
 
-func (m *mockChannel) NewAnchorResolutions() ([]*lnwallet.AnchorResolution,
+func (m *mockChannel) NewAnchorResolutions() (*lnwallet.AnchorResolutions,
 	error) {
+	if m.anchorResolutions != nil {
+		return m.anchorResolutions, nil
+	}
 
-	return m.anchorResolutions, nil
+	return &lnwallet.AnchorResolutions{}, nil
 }
 
 func (m *mockChannel) ForceCloseChan() (*lnwallet.LocalForceCloseSummary, error) {

contractcourt/commit_sweep_resolver_test.go

@@ -108,14 +108,17 @@ type mockSweeper struct {
 	sweepTx           *wire.MsgTx
 	sweepErr          error
 	createSweepTxChan chan *wire.MsgTx
+
+	deadlines []int
 }
 
 func newMockSweeper() *mockSweeper {
 	return &mockSweeper{
-		sweptInputs:       make(chan input.Input),
+		sweptInputs:       make(chan input.Input, 3),
 		updatedInputs:     make(chan wire.OutPoint),
 		sweepTx:           &wire.MsgTx{},
 		createSweepTxChan: make(chan *wire.MsgTx),
+		deadlines:         []int{},
 	}
 }
 
@@ -124,6 +127,11 @@ func (s *mockSweeper) SweepInput(input input.Input, params sweep.Params) (
 
 	s.sweptInputs <- input
 
+	// Update the deadlines used if it's set.
+	if params.Fee.ConfTarget != 0 {
+		s.deadlines = append(s.deadlines, int(params.Fee.ConfTarget))
+	}
+
 	result := make(chan sweep.Result, 1)
 	result <- sweep.Result{
 		Tx:  s.sweepTx,

docs/release-notes/release-notes-0.13.1.md

@@ -37,6 +37,17 @@ The [`monitoring` build tag is now on by
 default](https://github.com/lightningnetwork/lnd/pull/5399) for all routine
 builds.
 
+## Deadline Aware in Anchor Sweeping
+Anchor sweeping is now [deadline
+aware](https://github.com/lightningnetwork/lnd/pull/5148). Previously, all
+anchor sweepings use a default conf target of 6, which is likely to cause
+overpaying miner fees since the CLTV values of the HTLCs are far in the future.
+Brought by this update, the anchor sweeping (particularly local force close)
+will construct a deadline from its set of HTLCs, and use it as the conf target
+when estimating miner fees. The previous default conf target 6 is now changed
+to 144, and it's only used when there are no eligible HTLCs for deadline
+construction.
+
 ## Bug Fixes
 
 An optimization intended to speed up the payment critical path by

lntest/fee_service.go

@@ -15,7 +15,7 @@ const (
 	// feeServiceTarget is the confirmation target for which a fee estimate
 	// is returned. Requests for higher confirmation targets will fall back
 	// to this.
-	feeServiceTarget = 2
+	feeServiceTarget = 1
 )
 
 // feeService runs a web service that provides fee estimation information.
@@ -100,3 +100,11 @@ func (f *feeService) setFee(fee chainfee.SatPerKWeight) {
 
 	f.Fees[feeServiceTarget] = uint32(fee.FeePerKVByte())
 }
+
+// setFeeWithConf sets a fee for the given confirmation target.
+func (f *feeService) setFeeWithConf(fee chainfee.SatPerKWeight, conf uint32) {
+	f.lock.Lock()
+	defer f.lock.Unlock()
+
+	f.Fees[conf] = uint32(fee.FeePerKVByte())
+}

lntest/fee_service_test.go

@@ -34,6 +34,6 @@ func TestFeeService(t *testing.T) {
 	require.NoError(t, err)
 
 	require.Equal(
-		t, "{\"fee_by_block_target\":{\"2\":20000}}", string(body),
+		t, "{\"fee_by_block_target\":{\"1\":20000}}", string(body),
 	)
 }

lntest/harness.go

@@ -1512,6 +1512,12 @@ func (n *NetworkHarness) SetFeeEstimate(fee chainfee.SatPerKWeight) {
 	n.feeService.setFee(fee)
 }
 
+func (n *NetworkHarness) SetFeeEstimateWithConf(
+	fee chainfee.SatPerKWeight, conf uint32) {
+
+	n.feeService.setFeeWithConf(fee, conf)
+}
+
 // CopyFile copies the file src to dest.
 func CopyFile(dest, src string) error {
 	s, err := os.Open(src)

lntest/itest/lnd_channel_force_close.go

@@ -0,0 +1,222 @@
+package itest
+
+import (
+	"context"
+	"testing"
+
+	"github.com/btcsuite/btcd/blockchain"
+	"github.com/btcsuite/btcd/integration/rpctest"
+	"github.com/btcsuite/btcd/wire"
+	"github.com/btcsuite/btcutil"
+	"github.com/lightningnetwork/lnd/lnrpc"
+	"github.com/lightningnetwork/lnd/lnrpc/routerrpc"
+	"github.com/lightningnetwork/lnd/lntest"
+	"github.com/lightningnetwork/lnd/lntest/wait"
+	"github.com/lightningnetwork/lnd/lnwallet"
+	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
+	"github.com/lightningnetwork/lnd/routing"
+	"github.com/stretchr/testify/require"
+)
+
+// TODO(yy): move channel force closed related tests into this file.
+
+// testCommitmentTransactionDeadline tests that the anchor sweep transaction is
+// taking account of the deadline of the commitment transaction. It tests two
+// scenarios:
+//   1) when the CPFP is skipped, checks that the deadline is not used.
+//   2) when the CPFP is used, checks that the deadline is applied.
+// Note that whether the deadline is used or not is implicitly checked by its
+// corresponding fee rates.
+func testCommitmentTransactionDeadline(net *lntest.NetworkHarness,
+	t *harnessTest) {
+
+	// Get the default max fee rate used in sweeping the commitment
+	// transaction.
+	defaultMax := lnwallet.DefaultAnchorsCommitMaxFeeRateSatPerVByte
+	maxPerKw := chainfee.SatPerKVByte(defaultMax * 1000).FeePerKWeight()
+
+	const (
+		// feeRateConfDefault(sat/kw) is used when no conf target is
+		// set. This value will be returned by the fee estimator but
+		// won't be used because our commitment fee rate is capped by
+		// DefaultAnchorsCommitMaxFeeRateSatPerVByte.
+		feeRateDefault = 20000
+
+		// finalCTLV is used when Alice sends payment to Bob.
+		finalCTLV = 144
+
+		// deadline is used when Alice sweep the anchor. Notice there
+		// is a block padding of 3 added, such that the value of
+		// deadline is 147.
+		deadline = uint32(finalCTLV + routing.BlockPadding)
+	)
+
+	// feeRateSmall(sat/kw) is used when we want to skip the CPFP
+	// on anchor transactions. When the fee rate is smaller than
+	// the parent's (commitment transaction) fee rate, the CPFP
+	// will be skipped. Atm, the parent tx's fee rate is roughly
+	// 2500 sat/kw in this test.
+	feeRateSmall := maxPerKw / 2
+
+	// feeRateLarge(sat/kw) is used when we want to use the anchor
+	// transaction to CPFP our commitment transaction.
+	feeRateLarge := maxPerKw * 2
+
+	ctxt, cancel := context.WithTimeout(
+		context.Background(), defaultTimeout,
+	)
+	defer cancel()
+
+	// Before we start, set up the default fee rate and we will test the
+	// actual fee rate against it to decide whether we are using the
+	// deadline to perform fee estimation.
+	net.SetFeeEstimate(feeRateDefault)
+
+	// setupNode creates a new node and sends 1 btc to the node.
+	setupNode := func(name string) *lntest.HarnessNode {
+		// Create the node.
+		args := []string{"--hodl.exit-settle"}
+		args = append(args, commitTypeAnchors.Args()...)
+		node := net.NewNode(t.t, name, args)
+
+		// Send some coins to the node.
+		net.SendCoins(ctxt, t.t, btcutil.SatoshiPerBitcoin, node)
+		return node
+	}
+
+	// calculateSweepFeeRate runs multiple steps to calculate the fee rate
+	// used in sweeping the transactions.
+	calculateSweepFeeRate := func(expectedSweepTxNum int) int64 {
+		// Create two nodes, Alice and Bob.
+		alice := setupNode("Alice")
+		defer shutdownAndAssert(net, t, alice)
+
+		bob := setupNode("Bob")
+		defer shutdownAndAssert(net, t, bob)
+
+		// Connect Alice to Bob.
+		net.ConnectNodes(ctxt, t.t, alice, bob)
+
+		// Open a channel between Alice and Bob.
+		chanPoint := openChannelAndAssert(
+			ctxt, t, net, alice, bob,
+			lntest.OpenChannelParams{
+				Amt:     10e6,
+				PushAmt: 5e6,
+			},
+		)
+
+		// Send a payment with a specified finalCTLVDelta, which will
+		// be used as our deadline later on when Alice force closes the
+		// channel.
+		_, err := alice.RouterClient.SendPaymentV2(
+			ctxt,
+			&routerrpc.SendPaymentRequest{
+				Dest:           bob.PubKey[:],
+				Amt:            10e4,
+				PaymentHash:    makeFakePayHash(t),
+				FinalCltvDelta: finalCTLV,
+				TimeoutSeconds: 60,
+				FeeLimitMsat:   noFeeLimitMsat,
+			},
+		)
+		require.NoError(t.t, err, "unable to send alice htlc")
+
+		// Once the HTLC has cleared, all the nodes in our mini network
+		// should show that the HTLC has been locked in.
+		nodes := []*lntest.HarnessNode{alice, bob}
+		err = wait.NoError(func() error {
+			return assertNumActiveHtlcs(nodes, 1)
+		}, defaultTimeout)
+		require.NoError(t.t, err, "htlc mismatch")
+
+		// Alice force closes the channel.
+		_, _, err = net.CloseChannel(ctxt, alice, chanPoint, true)
+		require.NoError(t.t, err, "unable to force close channel")
+
+		// Now that the channel has been force closed, it should show
+		// up in the PendingChannels RPC under the waiting close
+		// section.
+		pendingChansRequest := &lnrpc.PendingChannelsRequest{}
+		pendingChanResp, err := alice.PendingChannels(
+			ctxt, pendingChansRequest,
+		)
+		require.NoError(
+			t.t, err, "unable to query for pending channels",
+		)
+		require.NoError(
+			t.t, checkNumWaitingCloseChannels(pendingChanResp, 1),
+		)
+
+		// We should see only one sweep transaction because the anchor
+		// sweep is skipped.
+		sweepTxns, err := getNTxsFromMempool(
+			net.Miner.Client,
+			expectedSweepTxNum, minerMempoolTimeout,
+		)
+		require.NoError(
+			t.t, err, "failed to find commitment tx in mempool",
+		)
+
+		// Mine a block to confirm these transactions such that they
+		// don't remain in the mempool for any subsequent tests.
+		_, err = net.Miner.Client.Generate(1)
+		require.NoError(t.t, err, "unable to mine blocks")
+
+		// Calculate the fee rate used.
+		feeRate := calculateTxnsFeeRate(t.t, net.Miner, sweepTxns)
+
+		return feeRate
+	}
+
+	// Setup our fee estimation for the deadline. Because the fee rate is
+	// smaller than the parent tx's fee rate, this value won't be used and
+	// we should see only one sweep tx in the mempool.
+	net.SetFeeEstimateWithConf(feeRateSmall, deadline)
+
+	// Calculate fee rate used.
+	feeRate := calculateSweepFeeRate(1)
+
+	// We expect the default max fee rate is used. Allow some deviation
+	// because weight estimates during tx generation are estimates.
+	require.InEpsilonf(
+		t.t, int64(maxPerKw), feeRate, 0.01,
+		"expected fee rate:%d, got fee rate:%d", maxPerKw, feeRate,
+	)
+
+	// Setup our fee estimation for the deadline. Because the fee rate is
+	// greater than the parent tx's fee rate, this value will be used to
+	// sweep the anchor transaction and we should see two sweep
+	// transactions in the mempool.
+	net.SetFeeEstimateWithConf(feeRateLarge, deadline)
+
+	// Calculate fee rate used.
+	feeRate = calculateSweepFeeRate(2)
+
+	// We expect the anchor to be swept with the deadline, which has the
+	// fee rate of feeRateLarge.
+	require.InEpsilonf(
+		t.t, int64(feeRateLarge), feeRate, 0.01,
+		"expected fee rate:%d, got fee rate:%d", feeRateLarge, feeRate,
+	)
+}
+
+// calculateTxnsFeeRate takes a list of transactions and estimates the fee rate
+// used to sweep them.
+func calculateTxnsFeeRate(t *testing.T,
+	miner *rpctest.Harness, txns []*wire.MsgTx) int64 {
+
+	var totalWeight, totalFee int64
+	for _, tx := range txns {
+		utx := btcutil.NewTx(tx)
+		totalWeight += blockchain.GetTransactionWeight(utx)
+
+		fee, err := getTxFee(miner.Client, tx)
+		require.NoError(t, err)
+
+		totalFee += int64(fee)
+	}
+	feeRate := totalFee * 1000 / totalWeight
+
+	return feeRate
+}

lntest/itest/lnd_test_list_on_test.go

@@ -111,7 +111,6 @@ var allTestCases = []*testCase{
 		name: "private channel update policy",
 		test: testUpdateChannelPolicyForPrivateChannel,
 	},
-
 	{
 		name: "invoice routing hints",
 		test: testInvoiceRoutingHints,
@@ -239,6 +238,10 @@ var allTestCases = []*testCase{
 		name: "hold invoice force close",
 		test: testHoldInvoiceForceClose,
 	},
+	{
+		name: "commitment deadline",
+		test: testCommitmentTransactionDeadline,
+	},
 	{
 		name: "cpfp",
 		test: testCPFP,

lnwallet/chainfee/estimator.go

@@ -19,12 +19,12 @@ const (
 	// a WebAPIEstimator will cache fees for. This number is chosen
 	// because it's the highest number of confs bitcoind will return a fee
 	// estimate for.
-	maxBlockTarget uint32 = 1009
+	maxBlockTarget uint32 = 1008
 
 	// minBlockTarget is the lowest number of blocks confirmations that
 	// a WebAPIEstimator will cache fees for. Requesting an estimate for
 	// less than this will result in an error.
-	minBlockTarget uint32 = 2
+	minBlockTarget uint32 = 1
 
 	// minFeeUpdateTimeout represents the minimum interval in which a
 	// WebAPIEstimator will request fresh fees from its API.
@@ -375,7 +375,16 @@ func (b *BitcoindEstimator) Stop() error {
 // confirmation and returns the estimated fee expressed in sat/kw.
 //
 // NOTE: This method is part of the Estimator interface.
-func (b *BitcoindEstimator) EstimateFeePerKW(numBlocks uint32) (SatPerKWeight, error) {
+func (b *BitcoindEstimator) EstimateFeePerKW(
+	numBlocks uint32) (SatPerKWeight, error) {
+
+	if numBlocks > maxBlockTarget {
+		log.Debugf("conf target %d exceeds the max value, "+
+			"use %d instead.", numBlocks, maxBlockTarget,
+		)
+		numBlocks = maxBlockTarget
+	}
+
 	feeEstimate, err := b.fetchEstimate(numBlocks)
 	switch {
 	// If the estimator doesn't have enough data, or returns an error, then

lnwallet/chainfee/estimator_test.go

@@ -172,7 +172,7 @@ func TestWebAPIFeeEstimator(t *testing.T) {
 		est    uint32
 		err    string
 	}{
-		{"target_below_min", 1, 12345, 12345, "too low, minimum"},
+		{"target_below_min", 0, 12345, 12345, "too low, minimum"},
 		{"target_w_too-low_fee", 10, 42, feeFloor, ""},
 		{"API-omitted_target", 2, 0, 0, "web API does not include"},
 		{"valid_target", 20, 54321, 54321, ""},

lnwallet/channel.go

@@ -27,8 +27,6 @@ import (
 	"github.com/lightningnetwork/lnd/lnwire"
 )
 
-var zeroHash chainhash.Hash
-
 var (
 	// ErrChanClosing is returned when a caller attempts to close a channel
 	// that has already been closed or is in the process of being closed.
@@ -6406,16 +6404,33 @@ func (lc *LightningChannel) CompleteCooperativeClose(
 	return closeTx, ourBalance, nil
 }
 
-// NewAnchorResolutions returns the anchor resolutions for all currently valid
+// AnchorResolutions is a set of anchor resolutions that's being used when
-// commitment transactions. Because we have no view on the mempool, we can only
+// sweeping anchors during local channel force close.
-// blindly anchor all of these txes down.
+type AnchorResolutions struct {
-func (lc *LightningChannel) NewAnchorResolutions() ([]*AnchorResolution,
+	// Local is the anchor resolution for the local commitment tx.
+	Local *AnchorResolution
+
+	// Remote is the anchor resolution for the remote commitment tx.
+	Remote *AnchorResolution
+
+	// RemotePending is the anchor resolution for the remote pending
+	// commitment tx. The value will be non-nil iff we've created a new
+	// commitment tx for the remote party which they haven't ACKed yet.
+	RemotePending *AnchorResolution
+}
+
+// NewAnchorResolutions returns a set of anchor resolutions wrapped in the
+// struct AnchorResolutions. Because we have no view on the mempool, we can
+// only blindly anchor all of these txes down. Caller needs to check the
+// returned values against nil to decide whether there exists an anchor
+// resolution for local/remote/pending remote commitment txes.
+func (lc *LightningChannel) NewAnchorResolutions() (*AnchorResolutions,
 	error) {
 
 	lc.Lock()
 	defer lc.Unlock()
 
-	var resolutions []*AnchorResolution
+	resolutions := &AnchorResolutions{}
 
 	// Add anchor for local commitment tx, if any.
 	localRes, err := NewAnchorResolution(
@@ -6424,9 +6439,7 @@ func (lc *LightningChannel) NewAnchorResolutions() ([]*AnchorResolution,
 	if err != nil {
 		return nil, err
 	}
-	if localRes != nil {
+	resolutions.Local = localRes
-		resolutions = append(resolutions, localRes)
-	}
 
 	// Add anchor for remote commitment tx, if any.
 	remoteRes, err := NewAnchorResolution(
@@ -6435,9 +6448,7 @@ func (lc *LightningChannel) NewAnchorResolutions() ([]*AnchorResolution,
 	if err != nil {
 		return nil, err
 	}
-	if remoteRes != nil {
+	resolutions.Remote = remoteRes
-		resolutions = append(resolutions, remoteRes)
-	}
 
 	// Add anchor for remote pending commitment tx, if any.
 	remotePendingCommit, err := lc.channelState.RemoteCommitChainTip()
@@ -6453,10 +6464,7 @@ func (lc *LightningChannel) NewAnchorResolutions() ([]*AnchorResolution,
 		if err != nil {
 			return nil, err
 		}
-
+		resolutions.RemotePending = remotePendingRes
-		if remotePendingRes != nil {
-			resolutions = append(resolutions, remotePendingRes)
-		}
 	}
 
 	return resolutions, nil

lnwallet/channel_test.go

@@ -917,14 +917,19 @@ func testForceClose(t *testing.T, testCase *forceCloseTestCase) {
 		}
 
 		// Check the pre-confirmation resolutions.
-		resList, err := aliceChannel.NewAnchorResolutions()
+		res, err := aliceChannel.NewAnchorResolutions()
 		if err != nil {
 			t.Fatalf("pre-confirmation resolution error: %v", err)
 		}
 
-		if len(resList) != 2 {
+		// Check we have the expected anchor resolutions.
-			t.Fatal("expected two resolutions")
+		require.NotNil(t, res.Local, "expected local anchor resolution")
-		}
+		require.NotNil(t,
+			res.Remote, "expected remote anchor resolution",
+		)
+		require.Nil(t,
+			res.RemotePending, "expected no anchor resolution",
+		)
 	}
 
 	// The SelfOutputSignDesc should be non-nil since the output to-self is