sweeper: relax anchor sweeping when there's no deadline pressure

contractcourt/channel_arbitrator.go

@@ -1309,9 +1309,23 @@ func (c *ChannelArbitrator) sweepAnchors(anchors *lnwallet.AnchorResolutions,
 			return err
 		}
 
+		// Create a force flag that's used to indicate whether we
+		// should force sweeping this anchor.
+		var force bool
+
+		// Check the deadline against the default value. If it's less
+		// than the default value of 144, it means there is a deadline
+		// and we will perform a CPFP for this commitment tx.
+		if deadline < anchorSweepConfTarget {
+			// 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.
+			force = true
+		}
+
 		log.Debugf("ChannelArbitrator(%v): pre-confirmation sweep of "+
-			"anchor of %s commit tx %v", c.cfg.ChanPoint,
-			anchorPath, anchor.CommitAnchor)
+			"anchor of %s commit tx %v, force=%v", c.cfg.ChanPoint,
+			anchorPath, anchor.CommitAnchor, force)
 
 		witnessType := input.CommitmentAnchor
 
@@ -1337,20 +1351,17 @@ func (c *ChannelArbitrator) sweepAnchors(anchors *lnwallet.AnchorResolutions,
 		)
 
 		// Sweep anchor output with a confirmation target fee
-		// preference. Because this is a cpfp-operation, the anchor will
-		// only be attempted to sweep when the current fee estimate for
-		// the confirmation target exceeds the commit fee rate.
-		//
-		// 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.
+		// preference. Because this is a cpfp-operation, the anchor
+		// will only be attempted to sweep when the current fee
+		// estimate for the confirmation target exceeds the commit fee
+		// rate.
 		_, err = c.cfg.Sweeper.SweepInput(
 			&anchorInput,
 			sweep.Params{
 				Fee: sweep.FeePreference{
 					ConfTarget: deadline,
 				},
-				Force:          true,
+				Force:          force,
 				ExclusiveGroup: &exclusiveGroup,
 			},
 		)
@@ -1427,6 +1438,9 @@ func (c *ChannelArbitrator) findCommitmentDeadline(heightHint uint32,
 
 		if htlc.RefundTimeout < deadlineMinHeight {
 			deadlineMinHeight = htlc.RefundTimeout
+			log.Tracef("ChannelArbitrator(%v): outgoing HTLC has "+
+				"deadline: %v", c.cfg.ChanPoint,
+				deadlineMinHeight)
 		}
 	}
 
@@ -1455,6 +1469,9 @@ func (c *ChannelArbitrator) findCommitmentDeadline(heightHint uint32,
 
 		if htlc.RefundTimeout < deadlineMinHeight {
 			deadlineMinHeight = htlc.RefundTimeout
+			log.Tracef("ChannelArbitrator(%v): incoming HTLC has "+
+				"deadline: %v", c.cfg.ChanPoint,
+				deadlineMinHeight)
 		}
 	}
 

docs/release-notes/release-notes-0.17.1.md

@@ -25,6 +25,16 @@
 
 # New Features
 ## Functional Enhancements
+
+- Previously, when a channel was force closed locally, its anchor was always
+  force-swept when the CPFP requirements are met. This is now
+  [changed](https://github.com/lightningnetwork/lnd/pull/7965) to only attempt
+  CPFP based on the deadline of the commitment transaction. The anchor output
+  will still be offered to the sweeper during channel force close, while the
+  actual sweeping won't be forced(CPFP) unless a relevant HTLC will timeout in
+  144 blocks. If CPFP before this deadline is needed, user can use `BumpFee`
+  instead.
+
 ## RPC Additions
 ## lncli Additions
 
@@ -44,5 +54,5 @@
 ## Tooling and Documentation
 
 # Contributors (Alphabetical Order)
-
-* Eugene Siegel
+* Eugene Siegel
+* Yong Yu

itest/lnd_channel_backup_test.go

@@ -783,16 +783,15 @@ func runChanRestoreScenarioForceClose(ht *lntest.HarnessTest, zeroConf bool) {
 		ht, crs.password, crs.mnemonic, multi, dave,
 	)
 
-	// We now wait until both Dave's closing tx and sweep tx have shown in
-	// mempool.
-	ht.Miner.AssertNumTxsInMempool(2)
+	// We now wait until both Dave's closing tx.
+	ht.Miner.AssertNumTxsInMempool(1)
 
 	// Now that we're able to make our restored now, we'll shutdown the old
 	// Dave node as we'll be storing it shortly below.
 	ht.Shutdown(dave)
 
 	// Mine a block to confirm the closing tx from Dave.
-	ht.MineBlocksAndAssertNumTxes(1, 2)
+	ht.MineBlocksAndAssertNumTxes(1, 1)
 
 	// To make sure the channel state is advanced correctly if the channel
 	// peer is not online at first, we also shutdown Carol.
@@ -1412,8 +1411,9 @@ func chanRestoreViaRPC(ht *lntest.HarnessTest, password []byte,
 func assertTimeLockSwept(ht *lntest.HarnessTest, carol, dave *node.HarnessNode,
 	carolStartingBalance, daveStartingBalance int64) {
 
-	// We expect Carol to sweep her funds and also the anchor tx.
-	expectedTxes := 2
+	// We expect Carol to sweep her funds and also the anchor tx. In
+	// addition, Dave will also sweep his anchor output.
+	expectedTxes := 3
 
 	// Carol should sweep her funds immediately, as they are not
 	// timelocked.
@@ -1495,11 +1495,7 @@ func assertDLPExecuted(ht *lntest.HarnessTest,
 
 	// Upon reconnection, the nodes should detect that Dave is out of sync.
 	// Carol should force close the channel using her latest commitment.
-	expectedTxes := 1
-	if lntest.CommitTypeHasAnchors(commitType) {
-		expectedTxes = 2
-	}
-	ht.Miner.AssertNumTxsInMempool(expectedTxes)
+	ht.Miner.AssertNumTxsInMempool(1)
 
 	// Channel should be in the state "waiting close" for Carol since she
 	// broadcasted the force close tx.
@@ -1515,7 +1511,8 @@ func assertDLPExecuted(ht *lntest.HarnessTest,
 	ht.RestartNode(dave)
 
 	// Generate a single block, which should confirm the closing tx.
-	ht.MineBlocksAndAssertNumTxes(1, expectedTxes)
+	ht.MineBlocksAndAssertNumTxes(1, 1)
+	blocksMined := uint32(1)
 
 	// Dave should consider the channel pending force close (since he is
 	// waiting for his sweep to confirm).
@@ -1532,14 +1529,22 @@ func assertDLPExecuted(ht *lntest.HarnessTest,
 
 		// Mine Dave's anchor sweep tx.
 		ht.MineBlocksAndAssertNumTxes(1, 1)
+		blocksMined++
+
+		// The above block will trigger Carol's sweeper to reconsider
+		// the anchor sweeping. Because we are now sweeping at the fee
+		// rate floor, the sweeper will consider this input has
+		// positive yield thus attempts the sweeping.
+		ht.MineBlocksAndAssertNumTxes(1, 1)
+		blocksMined++
 
 		// After Carol's output matures, she should also reclaim her
 		// funds.
 		//
 		// The commit sweep resolver publishes the sweep tx at
 		// defaultCSV-1 and we already mined one block after the
 		// commitmment was published, so take that into account.
-		ht.MineBlocks(defaultCSV - 1 - 1)
+		ht.MineBlocks(defaultCSV - blocksMined)
 		ht.MineBlocksAndAssertNumTxes(1, 1)
 
 		// Now the channel should be fully closed also from Carol's POV.
@@ -1561,21 +1566,33 @@ func assertDLPExecuted(ht *lntest.HarnessTest,
 		// Dave should sweep his funds immediately, as they are not
 		// timelocked. We also expect Dave to sweep his anchor, if
 		// present.
-		ht.Miner.AssertNumTxsInMempool(expectedTxes)
+		if lntest.CommitTypeHasAnchors(commitType) {
+			ht.MineBlocksAndAssertNumTxes(1, 2)
+		} else {
+			ht.MineBlocksAndAssertNumTxes(1, 1)
+		}
 
-		// Mine the sweep tx.
-		ht.MineBlocksAndAssertNumTxes(1, expectedTxes)
+		blocksMined++
 
 		// Now Dave should consider the channel fully closed.
 		ht.AssertNumPendingForceClose(dave, 0)
 
+		// The above block will trigger Carol's sweeper to reconsider
+		// the anchor sweeping. Because we are now sweeping at the fee
+		// rate floor, the sweeper will consider this input has
+		// positive yield thus attempts the sweeping.
+		if lntest.CommitTypeHasAnchors(commitType) {
+			ht.MineBlocksAndAssertNumTxes(1, 1)
+			blocksMined++
+		}
+
 		// After Carol's output matures, she should also reclaim her
 		// funds.
 		//
 		// The commit sweep resolver publishes the sweep tx at
-		// defaultCSV-1 and we already mined one block after the
+		// defaultCSV-1 and we already have blocks mined after the
 		// commitmment was published, so take that into account.
-		ht.MineBlocks(defaultCSV - 1 - 1)
+		ht.MineBlocks(defaultCSV - blocksMined)
 		ht.MineBlocksAndAssertNumTxes(1, 1)
 
 		// Now the channel should be fully closed also from Carol's

itest/lnd_channel_force_close_test.go

@@ -23,10 +23,13 @@ import (
 )
 
 // testCommitmentTransactionDeadline tests that the anchor sweep transaction is
-// taking account of the deadline of the commitment transaction. It tests two
+// taking account of the deadline of the commitment transaction. It tests three
 // 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.
+//  2. when the CPFP is used, checks that the deadline is NOT applied when it's
+//     larger than 144.
+//  3. when the CPFP is used, checks that the deadline is applied when it's
+//     less than 144.
 //
 // Note that whether the deadline is used or not is implicitly checked by its
 // corresponding fee rates.
@@ -43,13 +46,13 @@ func testCommitmentTransactionDeadline(ht *lntest.HarnessTest) {
 		// DefaultAnchorsCommitMaxFeeRateSatPerVByte.
 		feeRateDefault = 20000
 
-		// finalCTLV is used when Alice sends payment to Bob.
-		finalCTLV = 144
+		// defaultDeadline is the anchorSweepConfTarget, which is used
+		// when the commitment has no deadline pressure.
+		defaultDeadline = 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)
+		// deadline is one block below the default deadline. A forced
+		// anchor sweep will be performed when seeing this value.
+		deadline = defaultDeadline - 1
 	)
 
 	// feeRateSmall(sat/kw) is used when we want to skip the CPFP
@@ -91,7 +94,7 @@ func testCommitmentTransactionDeadline(ht *lntest.HarnessTest) {
 
 	// calculateSweepFeeRate runs multiple steps to calculate the fee rate
 	// used in sweeping the transactions.
-	calculateSweepFeeRate := func(expectedSweepTxNum int) int64 {
+	calculateSweepFeeRate := func(expectedSweepTxNum, deadline int) int64 {
 		// Create two nodes, Alice and Bob.
 		alice := setupNode("Alice")
 		defer ht.Shutdown(alice)
@@ -110,14 +113,18 @@ func testCommitmentTransactionDeadline(ht *lntest.HarnessTest) {
 			},
 		)
 
+		// Calculate the final ctlv delta based on the expected
+		// deadline.
+		finalCltvDelta := int32(deadline - int(routing.BlockPadding))
+
 		// Send a payment with a specified finalCTLVDelta, which will
 		// be used as our deadline later on when Alice force closes the
 		// channel.
 		req := &routerrpc.SendPaymentRequest{
 			Dest:           bob.PubKey[:],
 			Amt:            10e4,
 			PaymentHash:    ht.Random32Bytes(),
-			FinalCltvDelta: finalCTLV,
+			FinalCltvDelta: finalCltvDelta,
 			TimeoutSeconds: 60,
 			FeeLimitMsat:   noFeeLimitMsat,
 		}
@@ -154,8 +161,27 @@ func testCommitmentTransactionDeadline(ht *lntest.HarnessTest) {
 	// we should see only one sweep tx in the mempool.
 	ht.SetFeeEstimateWithConf(feeRateSmall, deadline)
 
-	// Calculate fee rate used.
-	feeRate := calculateSweepFeeRate(1)
+	// Calculate fee rate used and assert only the force close tx is
+	// broadcast.
+	feeRate := calculateSweepFeeRate(1, deadline)
+
+	// We expect the default max fee rate is used. Allow some deviation
+	// because weight estimates during tx generation are estimates.
+	require.InEpsilonf(
+		ht, 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. However, due to the default value
+	// being used, we should not attempt CPFP here because we are not force
+	// sweeping the anchor output.
+	ht.SetFeeEstimateWithConf(feeRateLarge, defaultDeadline)
+
+	// Calculate fee rate used and assert only the force close tx is
+	// broadcast.
+	feeRate = calculateSweepFeeRate(1, defaultDeadline)
 
 	// We expect the default max fee rate is used. Allow some deviation
 	// because weight estimates during tx generation are estimates.
@@ -170,8 +196,9 @@ func testCommitmentTransactionDeadline(ht *lntest.HarnessTest) {
 	// transactions in the mempool.
 	ht.SetFeeEstimateWithConf(feeRateLarge, deadline)
 
-	// Calculate fee rate used.
-	feeRate = calculateSweepFeeRate(2)
+	// Calculate fee rate used and assert both the force close tx and the
+	// anchor sweeping tx are broadcast.
+	feeRate = calculateSweepFeeRate(2, deadline)
 
 	// We expect the anchor to be swept with the deadline, which has the
 	// fee rate of feeRateLarge.