lntemp+itest: refactor testGraphTopologyNotifications

lntemp/harness_assertion.go

@@ -1637,3 +1637,13 @@ func (h *HarnessTest) AssertNumNodeAnns(hn *node.HarnessNode,
 
 	return anns
 }
+
+// AssertNumChannelUpdates asserts that a given number of channel updates has
+// been seen in the specified node's network topology.
+func (h *HarnessTest) AssertNumChannelUpdates(hn *node.HarnessNode,
+	chanPoint *lnrpc.ChannelPoint, num int) {
+
+	op := h.OutPointFromChannelPoint(chanPoint)
+	err := hn.Watcher.WaitForNumChannelUpdates(op, num)
+	require.NoError(h, err, "failed to assert num of channel updates")
+}

lntest/itest/assertions.go

@@ -1171,28 +1171,6 @@ func assertNodeNumChannels(t *harnessTest, node *lntest.HarnessNode,
 	)
 }
 
-func assertSyncType(t *harnessTest, node *lntest.HarnessNode,
-	peer string, syncType lnrpc.Peer_SyncType) {
-
-	t.t.Helper()
-
-	ctxb := context.Background()
-	ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
-	resp, err := node.ListPeers(ctxt, &lnrpc.ListPeersRequest{})
-	require.NoError(t.t, err)
-
-	for _, rpcPeer := range resp.Peers {
-		if rpcPeer.PubKey != peer {
-			continue
-		}
-
-		require.Equal(t.t, syncType, rpcPeer.SyncType)
-		return
-	}
-
-	t.t.Fatalf("unable to find peer: %s", peer)
-}
-
 // assertActiveHtlcs makes sure all the passed nodes have the _exact_ HTLCs
 // matching payHashes on _all_ their channels.
 func assertActiveHtlcs(nodes []*lntest.HarnessNode, payHashes ...[]byte) error {

lntest/itest/list_on_test.go

@@ -167,4 +167,8 @@ var allTestCasesTemp = []*lntemp.TestCase{
 		Name:     "unannounced channels",
 		TestFunc: testUnannouncedChannels,
 	},
+	{
+		Name:     "graph topology notifications",
+		TestFunc: testGraphTopologyNotifications,
+	},
 }

lntest/itest/lnd_channel_graph_test.go

@@ -1,7 +1,6 @@
 package itest
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"io"
@@ -257,32 +256,30 @@ func testUnannouncedChannels(ht *lntemp.HarnessTest) {
 	ht.CloseChannel(alice, fundingChanPoint)
 }
 
-func testGraphTopologyNotifications(net *lntest.NetworkHarness, t *harnessTest) {
+func testGraphTopologyNotifications(ht *lntemp.HarnessTest) {
-	t.t.Run("pinned", func(t *testing.T) {
+	ht.Run("pinned", func(t *testing.T) {
-		ht := newHarnessTest(t, net)
+		subT := ht.Subtest(t)
-		testGraphTopologyNtfns(net, ht, true)
+		testGraphTopologyNtfns(subT, true)
 	})
-	t.t.Run("unpinned", func(t *testing.T) {
+	ht.Run("unpinned", func(t *testing.T) {
-		ht := newHarnessTest(t, net)
+		subT := ht.Subtest(t)
-		testGraphTopologyNtfns(net, ht, false)
+		testGraphTopologyNtfns(subT, false)
 	})
 }
 
-func testGraphTopologyNtfns(net *lntest.NetworkHarness, t *harnessTest, pinned bool) {
+func testGraphTopologyNtfns(ht *lntemp.HarnessTest, pinned bool) {
-	ctxb := context.Background()
-
 	const chanAmt = funding.MaxBtcFundingAmount
 
 	// Spin up Bob first, since we will need to grab his pubkey when
 	// starting Alice to test pinned syncing.
-	bob := net.NewNode(t.t, "bob", nil)
+	bob := ht.Bob
-	defer shutdownAndAssert(net, t, bob)
+	bobInfo := bob.RPC.GetInfo()
-
-	ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
-	bobInfo, err := bob.GetInfo(ctxt, &lnrpc.GetInfoRequest{})
-	require.NoError(t.t, err)
 	bobPubkey := bobInfo.IdentityPubkey
 
+	// Restart Bob as he may have leftover announcements from previous
+	// tests, causing the graph to be unsynced.
+	ht.RestartNodeWithExtraArgs(bob, nil)
+
 	// For unpinned syncing, start Alice as usual. Otherwise grab Bob's
 	// pubkey to include in his pinned syncer set.
 	var aliceArgs []string
@@ -293,169 +290,64 @@ func testGraphTopologyNtfns(net *lntest.NetworkHarness, t *harnessTest, pinned b
 		}
 	}
 
-	alice := net.NewNode(t.t, "alice", aliceArgs)
+	alice := ht.Alice
-	defer shutdownAndAssert(net, t, alice)
+	ht.RestartNodeWithExtraArgs(alice, aliceArgs)
 
 	// Connect Alice and Bob.
-	net.EnsureConnected(t.t, alice, bob)
+	ht.EnsureConnected(alice, bob)
-
-	// Alice stimmy.
-	net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, alice)
-
-	// Bob stimmy.
-	net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, bob)
 
 	// Assert that Bob has the correct sync type before proceeding.
 	if pinned {
-		assertSyncType(t, alice, bobPubkey, lnrpc.Peer_PINNED_SYNC)
+		assertSyncType(ht, alice, bobPubkey, lnrpc.Peer_PINNED_SYNC)
 	} else {
-		assertSyncType(t, alice, bobPubkey, lnrpc.Peer_ACTIVE_SYNC)
+		assertSyncType(ht, alice, bobPubkey, lnrpc.Peer_ACTIVE_SYNC)
 	}
 
 	// Regardless of syncer type, ensure that both peers report having
 	// completed their initial sync before continuing to make a channel.
-	waitForGraphSync(t, alice)
+	ht.WaitForGraphSync(alice)
-
-	// Let Alice subscribe to graph notifications.
-	graphSub := subscribeGraphNotifications(ctxb, t, alice)
-	defer close(graphSub.quit)
 
 	// Open a new channel between Alice and Bob.
-	chanPoint := openChannelAndAssert(
+	chanPoint := ht.OpenChannel(
-		t, net, alice, bob,
+		alice, bob, lntemp.OpenChannelParams{Amt: chanAmt},
-		lntest.OpenChannelParams{
-			Amt: chanAmt,
-		},
 	)
 
 	// The channel opening above should have triggered a few notifications
 	// sent to the notification client. We'll expect two channel updates,
 	// and two node announcements.
-	var numChannelUpds int
+	ht.AssertNumChannelUpdates(alice, chanPoint, 2)
-	var numNodeAnns int
+	ht.AssertNumNodeAnns(alice, alice.PubKeyStr, 1)
-	for numChannelUpds < 2 && numNodeAnns < 2 {
+	ht.AssertNumNodeAnns(alice, bob.PubKeyStr, 1)
-		select {
-		// Ensure that a new update for both created edges is properly
-		// dispatched to our registered client.
-		case graphUpdate := <-graphSub.updateChan:
-			// Process all channel updates presented in this update
-			// message.
-			for _, chanUpdate := range graphUpdate.ChannelUpdates {
-				switch chanUpdate.AdvertisingNode {
-				case alice.PubKeyStr:
-				case bob.PubKeyStr:
-				default:
-					t.Fatalf("unknown advertising node: %v",
-						chanUpdate.AdvertisingNode)
-				}
-				switch chanUpdate.ConnectingNode {
-				case alice.PubKeyStr:
-				case bob.PubKeyStr:
-				default:
-					t.Fatalf("unknown connecting node: %v",
-						chanUpdate.ConnectingNode)
-				}
 
-				if chanUpdate.Capacity != int64(chanAmt) {
+	_, blockHeight := ht.Miner.GetBestBlock()
-					t.Fatalf("channel capacities mismatch:"+
-						" expected %v, got %v", chanAmt,
-						btcutil.Amount(chanUpdate.Capacity))
-				}
-				numChannelUpds++
-			}
 
-			for _, nodeUpdate := range graphUpdate.NodeUpdates {
+	// Now we'll test that updates are properly sent after channels are
-				switch nodeUpdate.IdentityKey {
+	// closed within the network.
-				case alice.PubKeyStr:
+	ht.CloseChannel(alice, chanPoint)
-				case bob.PubKeyStr:
-				default:
-					t.Fatalf("unknown node: %v",
-						nodeUpdate.IdentityKey)
-				}
-				numNodeAnns++
-			}
-		case err := <-graphSub.errChan:
-			t.Fatalf("unable to recv graph update: %v", err)
-		case <-time.After(time.Second * 10):
-			t.Fatalf("timeout waiting for graph notifications, "+
-				"only received %d/2 chanupds and %d/2 nodeanns",
-				numChannelUpds, numNodeAnns)
-		}
-	}
-
-	_, blockHeight, err := net.Miner.Client.GetBestBlock()
-	if err != nil {
-		t.Fatalf("unable to get current blockheight %v", err)
-	}
-
-	// Now we'll test that updates are properly sent after channels are closed
-	// within the network.
-	closeChannelAndAssert(t, net, alice, chanPoint, false)
 
 	// Now that the channel has been closed, we should receive a
 	// notification indicating so.
-out:
+	closedChan := ht.AssertTopologyChannelClosed(alice, chanPoint)
-	for {
-		select {
-		case graphUpdate := <-graphSub.updateChan:
-			if len(graphUpdate.ClosedChans) != 1 {
-				continue
-			}
 
-			closedChan := graphUpdate.ClosedChans[0]
+	require.Equal(ht, uint32(blockHeight+1), closedChan.ClosedHeight,
-			if closedChan.ClosedHeight != uint32(blockHeight+1) {
+		"close heights of channel mismatch")
-				t.Fatalf("close heights of channel mismatch: "+
-					"expected %v, got %v", blockHeight+1,
-					closedChan.ClosedHeight)
-			}
-			chanPointTxid, err := lnrpc.GetChanPointFundingTxid(chanPoint)
-			if err != nil {
-				t.Fatalf("unable to get txid: %v", err)
-			}
-			closedChanTxid, err := lnrpc.GetChanPointFundingTxid(
-				closedChan.ChanPoint,
-			)
-			if err != nil {
-				t.Fatalf("unable to get txid: %v", err)
-			}
-			if !bytes.Equal(closedChanTxid[:], chanPointTxid[:]) {
-				t.Fatalf("channel point hash mismatch: "+
-					"expected %v, got %v", chanPointTxid,
-					closedChanTxid)
-			}
-			if closedChan.ChanPoint.OutputIndex != chanPoint.OutputIndex {
-				t.Fatalf("output index mismatch: expected %v, "+
-					"got %v", chanPoint.OutputIndex,
-					closedChan.ChanPoint)
-			}
 
-			break out
+	fundingTxid := ht.OutPointFromChannelPoint(chanPoint)
-
+	closeTxid := ht.OutPointFromChannelPoint(closedChan.ChanPoint)
-		case err := <-graphSub.errChan:
+	require.EqualValues(ht, fundingTxid, closeTxid,
-			t.Fatalf("unable to recv graph update: %v", err)
+		"channel point hash mismatch")
-		case <-time.After(time.Second * 10):
-			t.Fatalf("notification for channel closure not " +
-				"sent")
-		}
-	}
 
 	// For the final portion of the test, we'll ensure that once a new node
 	// appears in the network, the proper notification is dispatched. Note
 	// that a node that does not have any channels open is ignored, so first
 	// we disconnect Alice and Bob, open a channel between Bob and Carol,
 	// and finally connect Alice to Bob again.
-	if err := net.DisconnectNodes(alice, bob); err != nil {
+	ht.DisconnectNodes(alice, bob)
-		t.Fatalf("unable to disconnect alice and bob: %v", err)
-	}
-	carol := net.NewNode(t.t, "Carol", nil)
-	defer shutdownAndAssert(net, t, carol)
 
-	net.ConnectNodes(t.t, bob, carol)
+	carol := ht.NewNode("Carol", nil)
-	chanPoint = openChannelAndAssert(
+	ht.ConnectNodes(bob, carol)
-		t, net, bob, carol,
+	chanPoint = ht.OpenChannel(
-		lntest.OpenChannelParams{
+		bob, carol, lntemp.OpenChannelParams{Amt: chanAmt},
-			Amt: chanAmt,
-		},
 	)
 
 	// Reconnect Alice and Bob. This should result in the nodes syncing up
@@ -464,60 +356,15 @@ out:
 	// and Carol. Note that we will also receive a node announcement from
 	// Bob, since a node will update its node announcement after a new
 	// channel is opened.
-	net.EnsureConnected(t.t, alice, bob)
+	ht.EnsureConnected(alice, bob)
 
 	// We should receive an update advertising the newly connected node,
 	// Bob's new node announcement, and the channel between Bob and Carol.
-	numNodeAnns = 0
+	ht.AssertNumChannelUpdates(alice, chanPoint, 2)
-	numChannelUpds = 0
+	ht.AssertNumNodeAnns(alice, bob.PubKeyStr, 1)
-	for numChannelUpds < 2 && numNodeAnns < 1 {
-		select {
-		case graphUpdate := <-graphSub.updateChan:
-			for _, nodeUpdate := range graphUpdate.NodeUpdates {
-				switch nodeUpdate.IdentityKey {
-				case carol.PubKeyStr:
-				case bob.PubKeyStr:
-				default:
-					t.Fatalf("unknown node update pubey: %v",
-						nodeUpdate.IdentityKey)
-				}
-				numNodeAnns++
-			}
-
-			for _, chanUpdate := range graphUpdate.ChannelUpdates {
-				switch chanUpdate.AdvertisingNode {
-				case carol.PubKeyStr:
-				case bob.PubKeyStr:
-				default:
-					t.Fatalf("unknown advertising node: %v",
-						chanUpdate.AdvertisingNode)
-				}
-				switch chanUpdate.ConnectingNode {
-				case carol.PubKeyStr:
-				case bob.PubKeyStr:
-				default:
-					t.Fatalf("unknown connecting node: %v",
-						chanUpdate.ConnectingNode)
-				}
-
-				if chanUpdate.Capacity != int64(chanAmt) {
-					t.Fatalf("channel capacities mismatch:"+
-						" expected %v, got %v", chanAmt,
-						btcutil.Amount(chanUpdate.Capacity))
-				}
-				numChannelUpds++
-			}
-		case err := <-graphSub.errChan:
-			t.Fatalf("unable to recv graph update: %v", err)
-		case <-time.After(time.Second * 10):
-			t.Fatalf("timeout waiting for graph notifications, "+
-				"only received %d/2 chanupds and %d/2 nodeanns",
-				numChannelUpds, numNodeAnns)
-		}
-	}
 
 	// Close the channel between Bob and Carol.
-	closeChannelAndAssert(t, net, bob, chanPoint, false)
+	ht.CloseChannel(bob, chanPoint)
 }
 
 // testNodeAnnouncement ensures that when a node is started with one or more
@@ -939,3 +786,23 @@ func testUpdateNodeAnnouncement(net *lntest.NetworkHarness, t *harnessTest) {
 	// Close the channel between Bob and Dave.
 	closeChannelAndAssert(t, net, net.Bob, chanPoint, false)
 }
+
+// assertSyncType asserts that the peer has an expected syncType.
+//
+// NOTE: only made for tests in this file.
+func assertSyncType(ht *lntemp.HarnessTest, hn *node.HarnessNode,
+	peer string, syncType lnrpc.Peer_SyncType) {
+
+	resp := hn.RPC.ListPeers()
+	for _, rpcPeer := range resp.Peers {
+		if rpcPeer.PubKey != peer {
+			continue
+		}
+
+		require.Equal(ht, syncType, rpcPeer.SyncType)
+
+		return
+	}
+
+	ht.Fatalf("unable to find peer: %s", peer)
+}

lntest/itest/lnd_test_list_on_test.go

@@ -8,10 +8,6 @@ var allTestCases = []*testCase{
 		name: "open channel reorg test",
 		test: testOpenChannelAfterReorg,
 	},
-	{
-		name: "graph topology notifications",
-		test: testGraphTopologyNotifications,
-	},
 	{
 		name: "channel force closure",
 		test: testChannelForceClosure,