From 578df81fb27cf2e4a9f946f3742525763aff7d46 Mon Sep 17 00:00:00 2001
From: Wilmer Paulino <wilmer.paulino@gmail.com>
Date: Mon, 26 Aug 2019 18:17:30 -0700
Subject: [PATCH] lntest/itest: resolve mempool flake in multi-hop htlc local
 timeout test

The test assumed that transactions would be broadcast and confirmed at
incorrect heights. Due to timing issues, it was possible for the test to
still succeed, resulting in a flake.

The test assumes that Bob will sweep a pending outgoing HTLC and commit
output back to their wallet. This commit ensures that these operations
are done when expected, i.e.:

1. Bob force closes the channel due to the HTLC timing out.
2. Once the channel is confirmed, Bob broadcasts their HTLC timeout
transaction.
3. Bob broadcasts their commit output sweep transaction once its CSV
expires.
4. Bob broadcasts their second layer sweep transaction for the timed out
HTLC once its CSV expires.
---
 lntest/itest/lnd_test.go | 64 +++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 33 deletions(-)

diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go
index 0d2797d8..4ee67480 100644
--- a/lntest/itest/lnd_test.go
+++ b/lntest/itest/lnd_test.go
@@ -10153,6 +10153,10 @@ func testMultiHopHtlcLocalTimeout(net *lntest.NetworkHarness, t *harnessTest) {
 	if err != nil {
 		t.Fatalf("unable to get txid: %v", err)
 	}
+	closeTxid, err := waitForTxInMempool(net.Miner.Node, minerMempoolTimeout)
+	if err != nil {
+		t.Fatalf("unable to find closing txid: %v", err)
+	}
 	assertSpendingTxInMempool(
 		t, net.Miner.Node, minerMempoolTimeout, wire.OutPoint{
 			Hash:  *bobFundingTxid,
@@ -10179,24 +10183,22 @@ func testMultiHopHtlcLocalTimeout(net *lntest.NetworkHarness, t *harnessTest) {
 		t.Fatalf("htlc mismatch: %v", predErr)
 	}
 
-	// We'll mine defaultCSV blocks in order to generate the sweep
-	// transaction of Bob's funding output. This will also bring us to the
-	// maturity height of the htlc tx output.
-	if _, err := net.Miner.Node.Generate(defaultCSV); err != nil {
-		t.Fatalf("unable to generate blocks: %v", err)
+	// With the closing transaction confirmed, we should expect Bob's HTLC
+	// timeout transaction to be broadcast due to the expiry being reached.
+	htlcTimeout, err := waitForTxInMempool(net.Miner.Node, minerMempoolTimeout)
+	if err != nil {
+		t.Fatalf("unable to find bob's htlc timeout tx: %v", err)
 	}
 
-	_, err = waitForTxInMempool(net.Miner.Node, minerMempoolTimeout)
-	if err != nil {
-		t.Fatalf("unable to find bob's funding output sweep tx: %v", err)
-	}
-
-	// The second layer HTLC timeout transaction should now have been
-	// broadcast on-chain.
-	secondLayerHash, err := waitForTxInMempool(net.Miner.Node, minerMempoolTimeout)
-	if err != nil {
-		t.Fatalf("unable to find bob's second layer transaction")
-	}
+	// We'll mine the remaining blocks in order to generate the sweep
+	// transaction of Bob's commitment output.
+	mineBlocks(t, net, defaultCSV, 1)
+	assertSpendingTxInMempool(
+		t, net.Miner.Node, minerMempoolTimeout, wire.OutPoint{
+			Hash:  *closeTxid,
+			Index: 1,
+		},
+	)
 
 	// Bob's pending channel report should show that he has a commitment
 	// output awaiting sweeping, and also that there's an outgoing HTLC
@@ -10220,14 +10222,20 @@ func testMultiHopHtlcLocalTimeout(net *lntest.NetworkHarness, t *harnessTest) {
 		t.Fatalf("bob should have pending htlc but doesn't")
 	}
 
-	// Now we'll mine an additional block, which should include the second
-	// layer sweep tx.
-	block := mineBlocks(t, net, 1, 1)[0]
+	// Now we'll mine an additional block, which should confirm Bob's commit
+	// sweep. This block should also prompt Bob to broadcast their second
+	// layer sweep due to the CSV on the HTLC timeout output.
+	mineBlocks(t, net, 1, 1)
+	assertSpendingTxInMempool(
+		t, net.Miner.Node, minerMempoolTimeout, wire.OutPoint{
+			Hash:  *htlcTimeout,
+			Index: 0,
+		},
+	)
 
-	// The block should have confirmed Bob's second layer sweeping
-	// transaction. Therefore, at this point, there should be no active
-	// HTLC's on the commitment transaction from Alice -> Bob.
-	assertTxInBlock(t, block, secondLayerHash)
+	// The block should have confirmed Bob's HTLC timeout transaction.
+	// Therefore, at this point, there should be no active HTLC's on the
+	// commitment transaction from Alice -> Bob.
 	nodes = []*lntest.HarnessNode{net.Alice}
 	err = lntest.WaitPredicate(func() bool {
 		predErr = assertNumActiveHtlcs(nodes, 0)
@@ -10252,16 +10260,6 @@ func testMultiHopHtlcLocalTimeout(net *lntest.NetworkHarness, t *harnessTest) {
 		t.Fatalf("bob's htlc should have advanced to the second stage: %v", err)
 	}
 
-	// We'll now mine four more blocks. After the 4th block, a transaction
-	// sweeping the HTLC output should be broadcast.
-	if _, err := net.Miner.Node.Generate(4); err != nil {
-		t.Fatalf("unable to generate blocks: %v", err)
-	}
-	_, err = waitForTxInMempool(net.Miner.Node, minerMempoolTimeout)
-	if err != nil {
-		t.Fatalf("unable to find bob's sweeping transaction: %v", err)
-	}
-
 	// Next, we'll mine a final block that should confirm the second-layer
 	// sweeping transaction.
 	if _, err := net.Miner.Node.Generate(1); err != nil {