lntemp+itest: refactor testReconnectAfterIPChange

lntemp/harness_assertion.go

@@ -1624,3 +1624,16 @@ func (h *HarnessTest) AssertNumPayments(hn *node.HarnessNode,
 
 	return payments
 }
+
+// AssertNumNodeAnns asserts that a given number of node announcements has been
+// seen in the specified node.
+func (h *HarnessTest) AssertNumNodeAnns(hn *node.HarnessNode,
+	pubkey string, num int) []*lnrpc.NodeUpdate {
+
+	// We will get the current number of channel updates first and add it
+	// to our expected number of newly created channel updates.
+	anns, err := hn.Watcher.WaitForNumNodeUpdates(pubkey, num)
+	require.NoError(h, err, "failed to assert num of channel updates")
+
+	return anns
+}

lntemp/node/watcher.go

@@ -124,10 +124,12 @@ func (nw *nodeWatcher) WaitForNumChannelUpdates(op wire.OutPoint,
 // WaitForNumNodeUpdates will block until a given number of node updates has
 // been seen in the node's network topology.
 func (nw *nodeWatcher) WaitForNumNodeUpdates(pubkey string,
-	expected int) error {
+	expected int) ([]*lnrpc.NodeUpdate, error) {
 
+	updates := make([]*lnrpc.NodeUpdate, 0)
 	checkNumUpdates := func() error {
-		num := len(nw.GetNodeUpdates(pubkey))
+		updates = nw.GetNodeUpdates(pubkey)
+		num := len(updates)
 		if num >= expected {
 			return nil
 		}
@@ -136,7 +138,9 @@ func (nw *nodeWatcher) WaitForNumNodeUpdates(pubkey string,
 			"want %d, got %d", expected, num)
 	}
 
-	return wait.NoError(checkNumUpdates, DefaultTimeout)
+	err := wait.NoError(checkNumUpdates, DefaultTimeout)
+
+	return updates, err
 }
 
 // WaitForChannelOpen will block until a channel with the target outpoint is

lntest/itest/list_on_test.go

@@ -155,4 +155,8 @@ var allTestCasesTemp = []*lntemp.TestCase{
 		Name:     "connection timeout",
 		TestFunc: testNetworkConnectionTimeout,
 	},
+	{
+		Name:     "reconnect after ip change",
+		TestFunc: testReconnectAfterIPChange,
+	},
 }

lntest/itest/lnd_network_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/lightningnetwork/lnd/lntemp"
 	"github.com/lightningnetwork/lnd/lntemp/node"
 	"github.com/lightningnetwork/lnd/lntest"
+	"github.com/lightningnetwork/lnd/lntest/wait"
 	"github.com/stretchr/testify/require"
 )
 
@@ -78,7 +79,7 @@ func testNetworkConnectionTimeout(ht *lntemp.HarnessTest) {
 
 // testReconnectAfterIPChange verifies that if a persistent inbound node changes
 // its listening address then it's peer will still be able to reconnect to it.
-func testReconnectAfterIPChange(net *lntest.NetworkHarness, t *harnessTest) {
+func testReconnectAfterIPChange(ht *lntemp.HarnessTest) {
 	// In this test, the following network will be set up. A single
 	// dash line represents a peer connection and a double dash line
 	// represents a channel.
@@ -104,115 +105,81 @@ func testReconnectAfterIPChange(net *lntest.NetworkHarness, t *harnessTest) {
 	// reconnect.
 
 	// Create a new node, Charlie.
-	charlie := net.NewNode(t.t, "Charlie", nil)
+	charlie := ht.NewNode("Charlie", nil)
-	defer shutdownAndAssert(net, t, charlie)
 
-	// We derive two ports for Dave, and we initialise his node with
+	// We derive an extra port for Dave, and we initialise his node with
-	// these ports advertised as `--externalip` arguments.
+	// the port advertised as `--externalip` arguments.
-	ip1 := lntest.NextAvailablePort()
 	ip2 := lntest.NextAvailablePort()
 
+	// Create a new node, Dave, which will initialize a P2P port for him.
+	daveArgs := []string{fmt.Sprintf("--externalip=127.0.0.1:%d", ip2)}
+	dave := ht.NewNode("Dave", daveArgs)
+
+	// We now have two ports, the initial P2P port from creating the node,
+	// and the `externalip` specified above.
 	advertisedAddrs := []string{
-		fmt.Sprintf("127.0.0.1:%d", ip1),
+		fmt.Sprintf("127.0.0.1:%d", dave.Cfg.P2PPort),
 		fmt.Sprintf("127.0.0.1:%d", ip2),
 	}
 
-	var daveArgs []string
-	for _, addr := range advertisedAddrs {
-		daveArgs = append(daveArgs, "--externalip="+addr)
-	}
-
-	// withP2PPort is a helper closure used to set the P2P port that a node
-	// should use.
-	var withP2PPort = func(port int) lntest.NodeOption {
-		return func(cfg *lntest.BaseNodeConfig) {
-			cfg.P2PPort = port
-		}
-	}
-
-	// Create a new node, Dave, and ensure that his initial P2P port is
-	// ip1 derived above.
-	dave := net.NewNode(t.t, "Dave", daveArgs, withP2PPort(ip1))
-	defer shutdownAndAssert(net, t, dave)
-
-	// Subscribe to graph notifications from Charlie so that we can tell
-	// when he receives Dave's NodeAnnouncements.
-	ctxb := context.Background()
-	charlieSub := subscribeGraphNotifications(ctxb, t, charlie)
-	defer close(charlieSub.quit)
-
 	// Connect Alice to Dave and Charlie.
-	net.ConnectNodes(t.t, net.Alice, dave)
+	alice := ht.Alice
-	net.ConnectNodes(t.t, net.Alice, charlie)
+	ht.ConnectNodes(alice, dave)
+	ht.ConnectNodes(alice, charlie)
 
 	// We'll then go ahead and open a channel between Alice and Dave. This
 	// ensures that Charlie receives the node announcement from Alice as
 	// part of the announcement broadcast.
-	chanPoint := openChannelAndAssert(
+	chanPoint := ht.OpenChannel(
-		t, net, net.Alice, dave, lntest.OpenChannelParams{
+		alice, dave, lntemp.OpenChannelParams{Amt: 1000000},
-			Amt: 1000000,
-		},
 	)
-	defer closeChannelAndAssert(t, net, net.Alice, chanPoint, false)
 
 	// waitForNodeAnnouncement is a closure used to wait on the given graph
 	// subscription for a node announcement from a node with the given
 	// public key. It also waits for the node announcement that advertises
 	// a particular set of addresses.
-	waitForNodeAnnouncement := func(graphSub graphSubscription,
+	waitForNodeAnnouncement := func(nodePubKey string, addrs []string) {
-		nodePubKey string, addrs []string) {
+		err := wait.NoError(func() error {
+			// Expect to have at least 1 node announcement now.
+			updates := ht.AssertNumNodeAnns(charlie, nodePubKey, 1)
 
-		for {
+			// Get latest node update from the node.
-			select {
+			update := updates[len(updates)-1]
-			case graphUpdate := <-graphSub.updateChan:
-			nextUpdate:
-				for _, update := range graphUpdate.NodeUpdates {
-					if update.IdentityKey != nodePubKey {
-						continue
-					}
 
 			addrMap := make(map[string]bool)
 			for _, addr := range update.NodeAddresses {
 				addrMap[addr.GetAddr()] = true
 			}
 
+			// Check that our wanted addresses can be found from
+			// the node update.
 			for _, addr := range addrs {
 				if !addrMap[addr] {
-							continue nextUpdate
+					return fmt.Errorf("address %s not "+
+						"found", addr)
 				}
 			}
 
-					return
+			return nil
-				}
+		}, defaultTimeout)
-
+		require.NoError(ht, err, "timeout checking node ann")
-			case err := <-graphSub.errChan:
-				t.Fatalf("unable to recv graph update: %v", err)
-
-			case <-time.After(defaultTimeout):
-				t.Fatalf("did not receive node ann update")
-			}
-		}
 	}
 
 	// Wait for Charlie to receive Dave's initial NodeAnnouncement.
-	waitForNodeAnnouncement(charlieSub, dave.PubKeyStr, advertisedAddrs)
+	waitForNodeAnnouncement(dave.PubKeyStr, advertisedAddrs)
 
-	// Now create a persistent connection between Charlie and Bob with no
+	// Now create a persistent connection between Charlie and Dave with no
-	// channels. Charlie is the outbound node and Bob is the inbound node.
+	// channels. Charlie is the outbound node and Dave is the inbound node.
-	net.ConnectNodesPerm(t.t, charlie, dave)
+	ht.ConnectNodesPerm(charlie, dave)
-
-	// Assert that Dave and Charlie are connected
-	assertConnected(t, dave, charlie)
 
 	// Change Dave's P2P port to the second IP address that he advertised
 	// and restart his node.
 	dave.Cfg.P2PPort = ip2
-	err := net.RestartNode(dave, nil)
+	ht.RestartNode(dave)
-	require.NoError(t.t, err)
 
 	// assert that Dave and Charlie reconnect successfully after Dave
 	// changes to his second advertised address.
-	assertConnected(t, dave, charlie)
+	ht.AssertConnected(dave, charlie)
 
 	// Next we test the case where Dave changes his listening address to one
 	// that was not listed in his original advertised addresses. The desired
@@ -227,20 +194,22 @@ func testReconnectAfterIPChange(net *lntest.NetworkHarness, t *harnessTest) {
 			"--externalip=127.0.0.1:%d", dave.Cfg.P2PPort,
 		),
 	}
-	err = net.RestartNode(dave, nil)
+	ht.RestartNode(dave)
-	require.NoError(t.t, err)
 
 	// Show that Charlie does receive Dave's new listening address in
 	// a Node Announcement.
 	waitForNodeAnnouncement(
-		charlieSub, dave.PubKeyStr,
+		dave.PubKeyStr,
 		[]string{fmt.Sprintf("127.0.0.1:%d", dave.Cfg.P2PPort)},
 	)
 
 	// assert that Dave and Charlie do reconnect after Dave changes his P2P
 	// address to one not listed in Dave's original advertised list of
 	// addresses.
-	assertConnected(t, dave, charlie)
+	ht.AssertConnected(dave, charlie)
+
+	// Finally, close the channel.
+	ht.CloseChannel(alice, chanPoint)
 }
 
 func connect(ctxt context.Context, node *lntest.HarnessNode,

lntest/itest/lnd_test_list_on_test.go

@@ -8,10 +8,6 @@ var allTestCases = []*testCase{
 		name: "open channel reorg test",
 		test: testOpenChannelAfterReorg,
 	},
-	{
-		name: "reconnect after ip change",
-		test: testReconnectAfterIPChange,
-	},
 	{
 		name: "graph topology notifications",
 		test: testGraphTopologyNotifications,