Merge pull request #8148 from yyforyongyu/sweeper-rbf-aware

[4/5]sweep: deepen `Aggregator` interface

cmd/lncli/walletrpc_active.go

@@ -286,8 +286,6 @@ var bumpCloseFeeCommand = cli.Command{
 	to sweep the anchor outputs of the closing transaction at the requested
 	fee rate or confirmation target. The specified fee rate will be the
 	effective fee rate taking the parent fee into account.
-	Depending on the sweeper configuration (batchwindowduration) the sweeptx
-	will not be published immediately.
 	NOTE: This cmd is DEPRECATED please use bumpforceclosefee instead.
 	`,
 	Flags: []cli.Flag{
@@ -321,8 +319,6 @@ var bumpForceCloseFeeCommand = cli.Command{
 	to sweep the anchor outputs of the closing transaction at the requested
 	fee rate or confirmation target. The specified fee rate will be the
 	effective fee rate taking the parent fee into account.
-	Depending on the sweeper configuration (batchwindowduration) the sweeptx
-	will not be published immediately.
 	`,
 	Flags: []cli.Flag{
 		cli.Uint64Flag{

config.go

@@ -689,8 +689,7 @@ func DefaultConfig() Config {
 			Timeout: lncfg.DefaultRemoteSignerRPCTimeout,
 		},
 		Sweeper: &lncfg.Sweeper{
-			BatchWindowDuration: sweep.DefaultBatchWindowDuration,
-			MaxFeeRate:          sweep.DefaultMaxFeeRate,
+			MaxFeeRate: sweep.DefaultMaxFeeRate,
 		},
 		Htlcswitch: &lncfg.Htlcswitch{
 			MailboxDeliveryTimeout: htlcswitch.DefaultMailboxDeliveryTimeout,

itest/lnd_channel_backup_test.go

@@ -1266,6 +1266,12 @@ func testDataLossProtection(ht *lntest.HarnessTest) {
 	// information Dave needs to sweep his funds.
 	require.NoError(ht, restartDave(), "unable to restart Eve")
 
+	// Dave should have a pending sweep.
+	ht.AssertNumPendingSweeps(dave, 1)
+
+	// Mine a block to trigger the sweep.
+	ht.MineBlocks(1)
+
 	// Dave should sweep his funds.
 	ht.Miner.AssertNumTxsInMempool(1)
 
@@ -1417,6 +1423,11 @@ func assertTimeLockSwept(ht *lntest.HarnessTest, carol, dave *node.HarnessNode,
 
 	// Carol should sweep her funds immediately, as they are not
 	// timelocked.
+	ht.AssertNumPendingSweeps(carol, 2)
+	ht.AssertNumPendingSweeps(dave, 1)
+
+	// Mine a block to trigger the sweeps.
+	ht.MineBlocks(1)
 	ht.Miner.AssertNumTxsInMempool(expectedTxes)
 
 	// Carol should consider the channel pending force close (since she is
@@ -1444,9 +1455,13 @@ func assertTimeLockSwept(ht *lntest.HarnessTest, carol, dave *node.HarnessNode,
 	// After the Dave's output matures, he should reclaim his funds.
 	//
 	// The commit sweep resolver publishes the sweep tx at defaultCSV-1 and
-	// we already mined one block after the commitment was published, so
-	// take that into account.
-	ht.MineBlocks(defaultCSV - 1 - 1)
+	// we already mined one block after the commitment was published, and
+	// one block to trigger Carol's sweeps, so take that into account.
+	ht.MineBlocks(1)
+	ht.AssertNumPendingSweeps(dave, 1)
+
+	// Mine a block to trigger the sweeps.
+	ht.MineBlocks(1)
 	daveSweep := ht.Miner.AssertNumTxsInMempool(1)[0]
 	block := ht.MineBlocksAndAssertNumTxes(1, 1)[0]
 	ht.Miner.AssertTxInBlock(block, daveSweep)
@@ -1526,6 +1541,12 @@ func assertDLPExecuted(ht *lntest.HarnessTest,
 		// Dave should sweep his anchor only, since he still has the
 		// lease CLTV constraint on his commitment output. We'd also
 		// see Carol's anchor sweep here.
+		ht.AssertNumPendingSweeps(dave, 1)
+		ht.AssertNumPendingSweeps(carol, 1)
+
+		// Mine a block to trigger the sweeps.
+		ht.MineBlocks(1)
+		blocksMined++
 		ht.Miner.AssertNumTxsInMempool(2)
 
 		// Mine anchor sweep txes for Carol and Dave.
@@ -1539,6 +1560,10 @@ func assertDLPExecuted(ht *lntest.HarnessTest,
 		// defaultCSV-1 and we already mined one block after the
 		// commitmment was published, so take that into account.
 		ht.MineBlocks(defaultCSV - blocksMined)
+		ht.AssertNumPendingSweeps(carol, 1)
+
+		// Mine a block to trigger the sweep.
+		ht.MineBlocks(1)
 		ht.MineBlocksAndAssertNumTxes(1, 1)
 
 		// Now the channel should be fully closed also from Carol's POV.
@@ -1552,14 +1577,33 @@ func assertDLPExecuted(ht *lntest.HarnessTest,
 		require.Positive(ht, blocksTilMaturity)
 
 		ht.MineBlocks(uint32(blocksTilMaturity))
+		ht.AssertNumPendingSweeps(dave, 1)
+
+		// Mine a block to trigger the sweep.
+		ht.MineBlocks(1)
 		ht.MineBlocksAndAssertNumTxes(1, 1)
 
 		// Now Dave should consider the channel fully closed.
 		ht.AssertNumPendingForceClose(dave, 0)
 	} else {
 		// Dave should sweep his funds immediately, as they are not
 		// timelocked. We also expect Carol and Dave sweep their
-		// anchors.
+		// anchors if it's an anchor channel.
+		if lntest.CommitTypeHasAnchors(commitType) {
+			ht.AssertNumPendingSweeps(carol, 1)
+			ht.AssertNumPendingSweeps(dave, 2)
+		} else {
+			ht.AssertNumPendingSweeps(dave, 1)
+		}
+
+		// Mine one block to trigger the sweeper to sweep.
+		ht.MineBlocks(1)
+		blocksMined++
+
+		// For anchor channels, we expect three txns,
+		// 1. the anchor sweeping tx from Dave.
+		// 2. the anchor sweeping tx from Carol.
+		// 3. the commitment sweep from Dave.
 		if lntest.CommitTypeHasAnchors(commitType) {
 			ht.MineBlocksAndAssertNumTxes(1, 3)
 		} else {
@@ -1578,6 +1622,12 @@ func assertDLPExecuted(ht *lntest.HarnessTest,
 		// defaultCSV-1 and we already have blocks mined after the
 		// commitmment was published, so take that into account.
 		ht.MineBlocks(defaultCSV - blocksMined)
+
+		// Mine one block to trigger the sweeper to sweep.
+		ht.MineBlocks(1)
+		ht.AssertNumPendingSweeps(carol, 1)
+
+		// Assert the sweeping tx is mined.
 		ht.MineBlocksAndAssertNumTxes(1, 1)
 
 		// Now the channel should be fully closed also from Carol's

itest/lnd_channel_force_close_test.go

@@ -179,12 +179,17 @@ func testCommitmentTransactionDeadline(ht *lntest.HarnessTest) {
 
 		// Bob should now sweep his to_local output and anchor output.
 		expectedNumTxes = 2
+		ht.AssertNumPendingSweeps(bob, 2)
 
 		// If Alice's anchor is not swept above, we should see it here.
 		if !expectAnchor {
 			expectedNumTxes = 3
+			ht.AssertNumPendingSweeps(alice, 1)
 		}
 
+		// Mine one block to trigger the sweeps.
+		ht.MineBlocks(1)
+
 		// Mine one more block to assert the sweep transactions.
 		ht.MineBlocksAndAssertNumTxes(1, expectedNumTxes)
 
@@ -386,16 +391,6 @@ func channelForceClosureTest(ht *lntest.HarnessTest,
 		)
 	)
 
-	// If we are dealing with an anchor channel type, the sweeper will
-	// sweep the HTLC second level output one block earlier (than the
-	// nursery that waits an additional block, and handles non-anchor
-	// channels). So we set a maturity height that is one less.
-	if lntest.CommitTypeHasAnchors(channelType) {
-		htlcCsvMaturityHeight = padCLTV(
-			startHeight + defaultCLTV + defaultCSV,
-		)
-	}
-
 	aliceChan := ht.QueryChannelByChanPoint(alice, chanPoint)
 	require.NotZero(ht, aliceChan.NumUpdates,
 		"alice should see at least one update to her channel")
@@ -523,6 +518,13 @@ func channelForceClosureTest(ht *lntest.HarnessTest,
 	// (the "kindergarten" bucket.)
 	ht.RestartNode(alice)
 
+	// Carol should have pending sweeps now.
+	ht.AssertNumPendingSweeps(carol, expectedTxes)
+
+	// Mine a block to trigger the sweep transactions.
+	blocksMined := int32(1)
+	ht.MineBlocks(1)
+
 	// Carol's sweep tx should be in the mempool already, as her output is
 	// not timelocked. If there are anchors, we also expect Carol's anchor
 	// sweep now.
@@ -560,7 +562,8 @@ func channelForceClosureTest(ht *lntest.HarnessTest,
 	// For the persistence test, we generate two blocks, then trigger
 	// a restart and then generate the final block that should trigger
 	// the creation of the sweep transaction.
-	ht.MineBlocks(defaultCSV - 2)
+	ht.MineBlocks(1)
+	blocksMined++
 
 	// The following restart checks to ensure that outputs in the
 	// kindergarten bucket are persisted while waiting for the required
@@ -592,7 +595,8 @@ func channelForceClosureTest(ht *lntest.HarnessTest,
 		// outputs should also reflect that this many blocks have
 		// passed.
 		err = checkCommitmentMaturity(
-			forceClose, commCsvMaturityHeight, 2,
+			forceClose, commCsvMaturityHeight,
+			defaultCSV-blocksMined,
 		)
 		if err != nil {
 			return err
@@ -621,8 +625,13 @@ func channelForceClosureTest(ht *lntest.HarnessTest,
 	ht.MineBlocks(1)
 
 	// At this point, the CSV will expire in the next block, meaning that
-	// the sweeping transaction should now be broadcast. So we fetch the
-	// node's mempool to ensure it has been properly broadcast.
+	// the output should be offered to the sweeper.
+	ht.AssertNumPendingSweeps(alice, 1)
+
+	// Mine one block and the sweeping transaction should now be broadcast.
+	// So we fetch the node's mempool to ensure it has been properly
+	// broadcast.
+	ht.MineBlocks(1)
 	sweepingTXID := ht.Miner.AssertNumTxsInMempool(1)[0]
 
 	// Fetch the sweep transaction, all input it's spending should be from
@@ -729,7 +738,16 @@ func channelForceClosureTest(ht *lntest.HarnessTest,
 	// number of blocks we have generated since adding it to the nursery,
 	// and take an additional block off so that we end up one block shy of
 	// the expiry height, and add the block padding.
-	cltvHeightDelta := padCLTV(defaultCLTV - defaultCSV - 1 - 1)
+	cltvHeightDelta := padCLTV(defaultCLTV - defaultCSV - 1 - 1 - 1)
+
+	// NOTE: this rest of the test would only pass if we remove the `Force`
+	// flag used in sweeping HTLCs, otherwise an immediate sweep will be
+	// attempted due to being forced. This flag will be removed once we can
+	// conditionally cancel back upstream htlcs to avoid cascading FCs.
+	ht.Shutdown(alice)
+	ht.Shutdown(carol)
+	ht.MineBlocksAndAssertNumTxes(1, 0)
+	ht.Skip("Skipping due until force flags are removed")
 
 	// Advance the blockchain until just before the CLTV expires, nothing
 	// exciting should have happened during this time.
@@ -773,20 +791,24 @@ func channelForceClosureTest(ht *lntest.HarnessTest,
 	}, defaultTimeout)
 	require.NoError(ht, err, "timeout while checking force closed channel")
 
-	// Now, generate the block which will cause Alice to broadcast the
-	// presigned htlc timeout txns.
+	// Now, generate the block which will cause Alice to offer the
+	// presigned htlc timeout txns to the sweeper.
 	ht.MineBlocks(1)
 
 	// Since Alice had numInvoices (6) htlcs extended to Carol before force
 	// closing, we expect Alice to broadcast an htlc timeout txn for each
 	// one.
 	expectedTxes = numInvoices
+	ht.AssertNumPendingSweeps(alice, numInvoices)
 
 	// In case of anchors, the timeout txs will be aggregated into one.
 	if lntest.CommitTypeHasAnchors(channelType) {
 		expectedTxes = 1
 	}
 
+	// Mine a block to trigger the sweeps.
+	ht.MineBlocks(1)
+
 	// Wait for them all to show up in the mempool.
 	htlcTxIDs := ht.Miner.AssertNumTxsInMempool(expectedTxes)
 
@@ -905,7 +927,7 @@ func channelForceClosureTest(ht *lntest.HarnessTest,
 	ht.RestartNode(alice)
 
 	// Advance the chain until just before the 2nd-layer CSV delays expire.
-	// For anchor channels thhis is one block earlier.
+	// For anchor channels this is one block earlier.
 	numBlocks := uint32(defaultCSV - 1)
 	if lntest.CommitTypeHasAnchors(channelType) {
 		numBlocks = defaultCSV - 2
@@ -935,6 +957,10 @@ func channelForceClosureTest(ht *lntest.HarnessTest,
 	// Generate a block that causes Alice to sweep the htlc outputs in the
 	// kindergarten bucket.
 	ht.MineBlocks(1)
+	ht.AssertNumPendingSweeps(alice, 6)
+
+	// Mine a block to trigger the sweep.
+	ht.MineBlocks(1)
 
 	// Wait for the single sweep txn to appear in the mempool.
 	htlcSweepTxID := ht.Miner.AssertNumTxsInMempool(1)[0]
@@ -1009,7 +1035,7 @@ func channelForceClosureTest(ht *lntest.HarnessTest,
 		}
 
 		err = checkPendingHtlcStageAndMaturity(
-			forceClose, 2, htlcCsvMaturityHeight, 0,
+			forceClose, 2, htlcCsvMaturityHeight, -1,
 		)
 		if err != nil {
 			return err
@@ -1133,6 +1159,10 @@ func testFailingChannel(ht *lntest.HarnessTest) {
 	// Mine enough blocks for Alice to sweep her funds from the force
 	// closed channel.
 	ht.MineBlocks(defaultCSV - 1)
+	ht.AssertNumPendingSweeps(alice, 1)
+
+	// Mine a block to trigger the sweep.
+	ht.MineBlocks(1)
 
 	// Wait for the sweeping tx to be broadcast.
 	ht.Miner.AssertNumTxsInMempool(1)