itest: refactor testGarbageCollectLinkNodes

lntest/itest/list_on_test.go

@@ -91,4 +91,8 @@ var allTestCasesTemp = []*lntemp.TestCase{
 		Name:     "max pending channel",
 		TestFunc: testMaxPendingChannels,
 	},
+	{
+		Name:     "garbage collect link nodes",
+		TestFunc: testGarbageCollectLinkNodes,
+	},
 }

lntest/itest/lnd_misc_test.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"strings"
-	"time"
 
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/btcsuite/btcwallet/wallet"
@@ -20,6 +19,7 @@ import (
 	"github.com/lightningnetwork/lnd/lnrpc/routerrpc"
 	"github.com/lightningnetwork/lnd/lnrpc/walletrpc"
 	"github.com/lightningnetwork/lnd/lntemp"
+	"github.com/lightningnetwork/lnd/lntemp/node"
 	"github.com/lightningnetwork/lnd/lntest"
 	"github.com/lightningnetwork/lnd/lntest/wait"
 	"github.com/lightningnetwork/lnd/lnwallet"
@@ -416,32 +416,30 @@ func testMaxPendingChannels(ht *lntemp.HarnessTest) {
 	}
 }
 
-// testGarbageCollectLinkNodes tests that we properly garbage collect link nodes
+// testGarbageCollectLinkNodes tests that we properly garbage collect link
-// from the database and the set of persistent connections within the server.
+// nodes from the database and the set of persistent connections within the
-func testGarbageCollectLinkNodes(net *lntest.NetworkHarness, t *harnessTest) {
+// server.
-	ctxb := context.Background()
+func testGarbageCollectLinkNodes(ht *lntemp.HarnessTest) {
+	const chanAmt = 1000000
 
-	const (
+	alice, bob := ht.Alice, ht.Bob
-		chanAmt = 1000000
-	)
 
 	// Open a channel between Alice and Bob which will later be
 	// cooperatively closed.
-	coopChanPoint := openChannelAndAssert(
+	coopChanPoint := ht.OpenChannel(
-		t, net, net.Alice, net.Bob, lntest.OpenChannelParams{
+		alice, bob, lntemp.OpenChannelParams{
 			Amt: chanAmt,
 		},
 	)
 
 	// Create Carol's node and connect Alice to her.
-	carol := net.NewNode(t.t, "Carol", nil)
+	carol := ht.NewNode("Carol", nil)
-	defer shutdownAndAssert(net, t, carol)
+	ht.ConnectNodes(alice, carol)
-	net.ConnectNodes(t.t, net.Alice, carol)
 
 	// Open a channel between Alice and Carol which will later be force
 	// closed.
-	forceCloseChanPoint := openChannelAndAssert(
+	forceCloseChanPoint := ht.OpenChannel(
-		t, net, net.Alice, carol, lntest.OpenChannelParams{
+		alice, carol, lntemp.OpenChannelParams{
 			Amt: chanAmt,
 		},
 	)
@@ -449,200 +447,85 @@ func testGarbageCollectLinkNodes(net *lntest.NetworkHarness, t *harnessTest) {
 	// Now, create Dave's a node and also open a channel between Alice and
 	// him. This link will serve as the only persistent link throughout
 	// restarts in this test.
-	dave := net.NewNode(t.t, "Dave", nil)
+	dave := ht.NewNode("Dave", nil)
-	defer shutdownAndAssert(net, t, dave)
 
-	net.ConnectNodes(t.t, net.Alice, dave)
+	ht.ConnectNodes(alice, dave)
-	persistentChanPoint := openChannelAndAssert(
+	persistentChanPoint := ht.OpenChannel(
-		t, net, net.Alice, dave, lntest.OpenChannelParams{
+		alice, dave, lntemp.OpenChannelParams{
 			Amt: chanAmt,
 		},
 	)
 
-	// isConnected is a helper closure that checks if a peer is connected to
-	// Alice.
-	isConnected := func(pubKey string) bool {
-		req := &lnrpc.ListPeersRequest{}
-		ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
-		resp, err := net.Alice.ListPeers(ctxt, req)
-		if err != nil {
-			t.Fatalf("unable to retrieve alice's peers: %v", err)
-		}
-
-		for _, peer := range resp.Peers {
-			if peer.PubKey == pubKey {
-				return true
-			}
-		}
-
-		return false
-	}
-
 	// Restart both Bob and Carol to ensure Alice is able to reconnect to
 	// them.
-	if err := net.RestartNode(net.Bob, nil); err != nil {
+	ht.RestartNode(bob)
-		t.Fatalf("unable to restart bob's node: %v", err)
+	ht.RestartNode(carol)
-	}
-	if err := net.RestartNode(carol, nil); err != nil {
-		t.Fatalf("unable to restart carol's node: %v", err)
-	}
 
-	require.Eventually(t.t, func() bool {
+	ht.AssertConnected(alice, bob)
-		return isConnected(net.Bob.PubKeyStr)
+	ht.AssertConnected(alice, carol)
-	}, defaultTimeout, 20*time.Millisecond)
-	require.Eventually(t.t, func() bool {
-		return isConnected(carol.PubKeyStr)
-	}, defaultTimeout, 20*time.Millisecond)
 
 	// We'll also restart Alice to ensure she can reconnect to her peers
 	// with open channels.
-	if err := net.RestartNode(net.Alice, nil); err != nil {
+	ht.RestartNode(alice)
-		t.Fatalf("unable to restart alice's node: %v", err)
-	}
 
-	require.Eventually(t.t, func() bool {
+	ht.AssertConnected(alice, bob)
-		return isConnected(net.Bob.PubKeyStr)
+	ht.AssertConnected(alice, carol)
-	}, defaultTimeout, 20*time.Millisecond)
+	ht.AssertConnected(alice, dave)
-	require.Eventually(t.t, func() bool {
-		return isConnected(carol.PubKeyStr)
-	}, defaultTimeout, 20*time.Millisecond)
-	require.Eventually(t.t, func() bool {
-		return isConnected(dave.PubKeyStr)
-	}, defaultTimeout, 20*time.Millisecond)
-	err := wait.Predicate(func() bool {
-		return isConnected(dave.PubKeyStr)
-	}, defaultTimeout)
 
 	// testReconnection is a helper closure that restarts the nodes at both
 	// ends of a channel to ensure they do not reconnect after restarting.
 	// When restarting Alice, we'll first need to ensure she has
 	// reestablished her connection with Dave, as they still have an open
 	// channel together.
-	testReconnection := func(node *lntest.HarnessNode) {
+	testReconnection := func(node *node.HarnessNode) {
 		// Restart both nodes, to trigger the pruning logic.
-		if err := net.RestartNode(node, nil); err != nil {
+		ht.RestartNode(node)
-			t.Fatalf("unable to restart %v's node: %v",
+		ht.RestartNode(alice)
-				node.Name(), err)
-		}
-
-		if err := net.RestartNode(net.Alice, nil); err != nil {
-			t.Fatalf("unable to restart alice's node: %v", err)
-		}
 
 		// Now restart both nodes and make sure they don't reconnect.
-		if err := net.RestartNode(node, nil); err != nil {
+		ht.RestartNode(node)
-			t.Fatalf("unable to restart %v's node: %v", node.Name(),
+		ht.AssertNotConnected(alice, node)
-				err)
-		}
-		err = wait.Invariant(func() bool {
-			return !isConnected(node.PubKeyStr)
-		}, 5*time.Second)
-		if err != nil {
-			t.Fatalf("alice reconnected to %v", node.Name())
-		}
 
-		if err := net.RestartNode(net.Alice, nil); err != nil {
+		ht.RestartNode(alice)
-			t.Fatalf("unable to restart alice's node: %v", err)
+		ht.AssertConnected(alice, dave)
-		}
+		ht.AssertNotConnected(alice, node)
-		err = wait.Predicate(func() bool {
-			return isConnected(dave.PubKeyStr)
-		}, defaultTimeout)
-		if err != nil {
-			t.Fatalf("alice didn't reconnect to Dave")
-		}
-
-		err = wait.Invariant(func() bool {
-			return !isConnected(node.PubKeyStr)
-		}, 5*time.Second)
-		if err != nil {
-			t.Fatalf("alice reconnected to %v", node.Name())
-		}
 	}
 
 	// Now, we'll close the channel between Alice and Bob and ensure there
 	// is no reconnection logic between the both once the channel is fully
 	// closed.
-	closeChannelAndAssert(t, net, net.Alice, coopChanPoint, false)
+	ht.CloseChannel(alice, coopChanPoint)
 
-	testReconnection(net.Bob)
+	testReconnection(bob)
 
 	// We'll do the same with Alice and Carol, but this time we'll force
 	// close the channel instead.
-	closeChannelAndAssert(t, net, net.Alice, forceCloseChanPoint, true)
+	ht.ForceCloseChannel(alice, forceCloseChanPoint)
-
-	// Cleanup by mining the force close and sweep transaction.
-	cleanupForceClose(t, net, net.Alice, forceCloseChanPoint)
 
 	// We'll need to mine some blocks in order to mark the channel fully
 	// closed.
-	_, err = net.Miner.Client.Generate(chainreg.DefaultBitcoinTimeLockDelta - defaultCSV)
+	ht.MineBlocks(
-	if err != nil {
+		chainreg.DefaultBitcoinTimeLockDelta - defaultCSV,
-		t.Fatalf("unable to generate blocks: %v", err)
+	)
-	}
 
 	// Before we test reconnection, we'll ensure that the channel has been
 	// fully cleaned up for both Carol and Alice.
-	var predErr error
+	ht.AssertNumPendingForceClose(alice, 0)
-	pendingChansRequest := &lnrpc.PendingChannelsRequest{}
+	ht.AssertNumPendingForceClose(carol, 0)
-	err = wait.Predicate(func() bool {
-		ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
-		pendingChanResp, err := net.Alice.PendingChannels(
-			ctxt, pendingChansRequest,
-		)
-		if err != nil {
-			predErr = fmt.Errorf("unable to query for pending "+
-				"channels: %v", err)
-			return false
-		}
-
-		predErr = checkNumForceClosedChannels(pendingChanResp, 0)
-		if predErr != nil {
-			return false
-		}
-
-		ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
-		pendingChanResp, err = carol.PendingChannels(
-			ctxt, pendingChansRequest,
-		)
-		if err != nil {
-			predErr = fmt.Errorf("unable to query for pending "+
-				"channels: %v", err)
-			return false
-		}
-
-		predErr = checkNumForceClosedChannels(pendingChanResp, 0)
-
-		return predErr == nil
-	}, defaultTimeout)
-	if err != nil {
-		t.Fatalf("channels not marked as fully resolved: %v", predErr)
-	}
 
 	testReconnection(carol)
 
 	// Finally, we'll ensure that Bob and Carol no longer show in Alice's
 	// channel graph.
-	describeGraphReq := &lnrpc.ChannelGraphRequest{
+	req := &lnrpc.ChannelGraphRequest{IncludeUnannounced: true}
-		IncludeUnannounced: true,
+	channelGraph := alice.RPC.DescribeGraph(req)
-	}
+	require.NotContains(ht, channelGraph.Nodes, bob.PubKeyStr,
-	ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
+		"did not expect to find bob in the channel graph, but did")
-	channelGraph, err := net.Alice.DescribeGraph(ctxt, describeGraphReq)
+	require.NotContains(ht, channelGraph.Nodes, carol.PubKeyStr,
-	if err != nil {
+		"did not expect to find carol in the channel graph, but did")
-		t.Fatalf("unable to query for alice's channel graph: %v", err)
-	}
-	for _, node := range channelGraph.Nodes {
-		if node.PubKey == net.Bob.PubKeyStr {
-			t.Fatalf("did not expect to find bob in the channel " +
-				"graph, but did")
-		}
-		if node.PubKey == carol.PubKeyStr {
-			t.Fatalf("did not expect to find carol in the channel " +
-				"graph, but did")
-		}
-	}
 
 	// Now that the test is done, we can also close the persistent link.
-	closeChannelAndAssert(t, net, net.Alice, persistentChanPoint, false)
+	ht.CloseChannel(alice, persistentChanPoint)
 }
 
 // testRejectHTLC tests that a node can be created with the flag --rejecthtlc.

lntest/itest/lnd_test_list_on_test.go

@@ -179,10 +179,6 @@ var allTestCases = []*testCase{
 		name: "failing link",
 		test: testFailingChannel,
 	},
-	{
-		name: "garbage collect link nodes",
-		test: testGarbageCollectLinkNodes,
-	},
 	{
 		name: "abandonchannel",
 		test: testAbandonChannel,