multi: refactor testFundingPersistence

This commit refactors the test `testFundingPersistence`. In addition, it
also changes the old `OpenChannelAssertPending` method and adds a new
method `OpenChannelAssertStream` for clarity.

lntemp/harness.go

@@ -734,8 +734,9 @@ func (h *HarnessTest) prepareOpenChannel(srcNode, destNode *node.HarnessNode,
 // destNode with the passed channel funding parameters. Once the `OpenChannel`
 // is called, it will consume the first event it receives from the open channel
 // client and asserts it's a channel pending event.
-func (h *HarnessTest) OpenChannelAssertPending(srcNode,
-	destNode *node.HarnessNode, p OpenChannelParams) rpc.OpenChanClient {
+func (h *HarnessTest) openChannelAssertPending(srcNode,
+	destNode *node.HarnessNode,
+	p OpenChannelParams) (*lnrpc.PendingUpdate, rpc.OpenChanClient) {
 
 	// Prepare the request and open the channel.
 	openReq := h.prepareOpenChannel(srcNode, destNode, p)
@@ -747,11 +748,35 @@ func (h *HarnessTest) OpenChannelAssertPending(srcNode,
 	resp := h.ReceiveOpenChannelUpdate(respStream)
 
 	// Check that the update is channel pending.
-	_, ok := resp.Update.(*lnrpc.OpenStatusUpdate_ChanPending)
+	update, ok := resp.Update.(*lnrpc.OpenStatusUpdate_ChanPending)
 	require.Truef(h, ok, "expected channel pending: update, instead got %v",
 		resp)
 
-	return respStream
+	return update.ChanPending, respStream
+}
+
+// OpenChannelAssertPending attempts to open a channel between srcNode and
+// destNode with the passed channel funding parameters. Once the `OpenChannel`
+// is called, it will consume the first event it receives from the open channel
+// client and asserts it's a channel pending event. It returns the
+// `PendingUpdate`.
+func (h *HarnessTest) OpenChannelAssertPending(srcNode,
+	destNode *node.HarnessNode, p OpenChannelParams) *lnrpc.PendingUpdate {
+
+	resp, _ := h.openChannelAssertPending(srcNode, destNode, p)
+	return resp
+}
+
+// OpenChannelAssertStream attempts to open a channel between srcNode and
+// destNode with the passed channel funding parameters. Once the `OpenChannel`
+// is called, it will consume the first event it receives from the open channel
+// client and asserts it's a channel pending event. It returns the open channel
+// stream.
+func (h *HarnessTest) OpenChannelAssertStream(srcNode,
+	destNode *node.HarnessNode, p OpenChannelParams) rpc.OpenChanClient {
+
+	_, stream := h.openChannelAssertPending(srcNode, destNode, p)
+	return stream
 }
 
 // OpenChannel attempts to open a channel with the specified parameters
@@ -763,7 +788,7 @@ func (h *HarnessTest) OpenChannelAssertPending(srcNode,
 func (h *HarnessTest) OpenChannel(alice, bob *node.HarnessNode,
 	p OpenChannelParams) *lnrpc.ChannelPoint {
 
-	chanOpenUpdate := h.OpenChannelAssertPending(alice, bob, p)
+	chanOpenUpdate := h.OpenChannelAssertStream(alice, bob, p)
 
 	// Mine 6 blocks, then wait for Alice's node to notify us that the
 	// channel has been opened. The funding transaction should be found

lntemp/harness_assertion.go

@@ -1479,3 +1479,27 @@ func (h *HarnessTest) AssertZombieChannel(hn *node.HarnessNode, chanID uint64) {
 	}, DefaultTimeout)
 	require.NoError(h, err, "timeout while checking zombie channel")
 }
+
+// AssertTxAtHeight gets all of the transactions that a node's wallet has a
+// record of at the target height, and finds and returns the tx with the target
+// txid, failing if it is not found.
+func (h *HarnessTest) AssertTxAtHeight(hn *node.HarnessNode, height int32,
+	txid *chainhash.Hash) *lnrpc.Transaction {
+
+	req := &lnrpc.GetTransactionsRequest{
+		StartHeight: height,
+		EndHeight:   height,
+	}
+	txns := hn.RPC.GetTransactions(req)
+
+	for _, tx := range txns.Transactions {
+		if tx.TxHash == txid.String() {
+			return tx
+		}
+	}
+
+	require.Failf(h, "fail to find tx", "tx:%v not found at height:%v",
+		txid, height)
+
+	return nil
+}

lntemp/rpc/lnd.go

@@ -346,11 +346,17 @@ func (h *HarnessRPC) SendCoinsAssertErr(req *lnrpc.SendCoinsRequest) {
 }
 
 // GetTransactions makes a RPC call to GetTransactions and asserts.
-func (h *HarnessRPC) GetTransactions() *lnrpc.TransactionDetails {
+func (h *HarnessRPC) GetTransactions(
+	req *lnrpc.GetTransactionsRequest) *lnrpc.TransactionDetails {
+
 	ctxt, cancel := context.WithTimeout(h.runCtx, DefaultTimeout)
 	defer cancel()
 
-	resp, err := h.LN.GetTransactions(ctxt, &lnrpc.GetTransactionsRequest{})
+	if req == nil {
+		req = &lnrpc.GetTransactionsRequest{}
+	}
+
+	resp, err := h.LN.GetTransactions(ctxt, req)
 	require.NoErrorf(h, err, "failed to GetTransactions for %s", h.Name)
 
 	return resp

lntemp/utils.go

@@ -8,6 +8,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/lightningnetwork/lnd/lnrpc"
 	"github.com/lightningnetwork/lnd/lntest"
 )
 
@@ -92,3 +93,16 @@ func ParseDerivationPath(path string) ([]uint32, error) {
 
 	return indices, nil
 }
+
+// ChanPointFromPendingUpdate constructs a channel point from a lnrpc pending
+// update.
+func ChanPointFromPendingUpdate(pu *lnrpc.PendingUpdate) *lnrpc.ChannelPoint {
+	chanPoint := &lnrpc.ChannelPoint{
+		FundingTxid: &lnrpc.ChannelPoint_FundingTxidBytes{
+			FundingTxidBytes: pu.Txid,
+		},
+		OutputIndex: pu.OutputIndex,
+	}
+
+	return chanPoint
+}

lntest/itest/list_on_test.go

@@ -127,4 +127,8 @@ var allTestCasesTemp = []*lntemp.TestCase{
 		Name:     "unconfirmed channel funding",
 		TestFunc: testUnconfirmedChannelFunding,
 	},
+	{
+		Name:     "funding flow persistence",
+		TestFunc: testChannelFundingPersistence,
+	},
 }

lntest/itest/lnd_funding_test.go

@@ -237,7 +237,7 @@ func testUnconfirmedChannelFunding(ht *lntemp.HarnessTest) {
 	// as she doesn't have any other funds since it's a new node.
 	ht.ConnectNodes(carol, alice)
 
-	chanOpenUpdate := ht.OpenChannelAssertPending(
+	chanOpenUpdate := ht.OpenChannelAssertStream(
 		carol, alice, lntemp.OpenChannelParams{
 			Amt:              chanAmt,
 			PushAmt:          pushAmt,
@@ -391,7 +391,7 @@ func testChannelFundingInputTypes(ht *lntemp.HarnessTest) {
 		// We'll send her some confirmed funds.
 		funder(chanAmt*2, carol)
 
-		chanOpenUpdate := ht.OpenChannelAssertPending(
+		chanOpenUpdate := ht.OpenChannelAssertStream(
 			carol, alice, lntemp.OpenChannelParams{
 				Amt: chanAmt,
 			},
@@ -572,7 +572,7 @@ func testExternalFundingChanPoint(ht *lntemp.HarnessTest) {
 // representation of channels if the system is restarted or disconnected.
 // testFundingPersistence mirrors testBasicChannelFunding, but adds restarts
 // and checks for the state of channels with unconfirmed funding transactions.
-func testChannelFundingPersistence(net *lntest.NetworkHarness, t *harnessTest) {
+func testChannelFundingPersistence(ht *lntemp.HarnessTest) {
 	chanAmt := funding.MaxBtcFundingAmount
 	pushAmt := btcutil.Amount(0)
 
@@ -580,140 +580,102 @@ func testChannelFundingPersistence(net *lntest.NetworkHarness, t *harnessTest) {
 	// confirmation before it's open, with the current set of defaults,
 	// we'll need to create a new node instance.
 	const numConfs = 5
-	carolArgs := []string{fmt.Sprintf("--bitcoin.defaultchanconfs=%v", numConfs)}
-	carol := net.NewNode(t.t, "Carol", carolArgs)
+	carolArgs := []string{
+		fmt.Sprintf("--bitcoin.defaultchanconfs=%v", numConfs),
+	}
+	carol := ht.NewNode("Carol", carolArgs)
 
-	// Clean up carol's node when the test finishes.
-	defer shutdownAndAssert(net, t, carol)
-
-	net.ConnectNodes(t.t, net.Alice, carol)
+	alice := ht.Alice
+	ht.ConnectNodes(alice, carol)
 
 	// Create a new channel that requires 5 confs before it's considered
 	// open, then broadcast the funding transaction
-	pendingUpdate, err := net.OpenPendingChannel(
-		net.Alice, carol, chanAmt, pushAmt,
-	)
-	if err != nil {
-		t.Fatalf("unable to open channel: %v", err)
+	param := lntemp.OpenChannelParams{
+		Amt:     chanAmt,
+		PushAmt: pushAmt,
 	}
+	update := ht.OpenChannelAssertPending(alice, carol, param)
 
 	// At this point, the channel's funding transaction will have been
 	// broadcast, but not confirmed. Alice and Bob's nodes should reflect
 	// this when queried via RPC.
-	assertNumOpenChannelsPending(t, net.Alice, carol, 1)
+	ht.AssertNumPendingOpenChannels(alice, 1)
+	ht.AssertNumPendingOpenChannels(carol, 1)
 
 	// Restart both nodes to test that the appropriate state has been
 	// persisted and that both nodes recover gracefully.
-	if err := net.RestartNode(net.Alice, nil); err != nil {
-		t.Fatalf("Node restart failed: %v", err)
-	}
-	if err := net.RestartNode(carol, nil); err != nil {
-		t.Fatalf("Node restart failed: %v", err)
-	}
+	ht.RestartNode(alice)
+	ht.RestartNode(carol)
 
-	fundingTxID, err := chainhash.NewHash(pendingUpdate.Txid)
-	if err != nil {
-		t.Fatalf("unable to convert funding txid into chainhash.Hash:"+
-			" %v", err)
-	}
-	fundingTxStr := fundingTxID.String()
+	fundingTxID, err := chainhash.NewHash(update.Txid)
+	require.NoError(ht, err, "unable to convert funding txid "+
+		"into chainhash.Hash")
 
 	// Mine a block, then wait for Alice's node to notify us that the
 	// channel has been opened. The funding transaction should be found
 	// within the newly mined block.
-	block := mineBlocks(t, net, 1, 1)[0]
-	assertTxInBlock(t, block, fundingTxID)
+	block := ht.MineBlocksAndAssertNumTxes(1, 1)[0]
+	ht.Miner.AssertTxInBlock(block, fundingTxID)
 
 	// Get the height that our transaction confirmed at.
-	_, height, err := net.Miner.Client.GetBestBlock()
-	require.NoError(t.t, err, "could not get best block")
+	_, height := ht.Miner.GetBestBlock()
 
 	// Restart both nodes to test that the appropriate state has been
 	// persisted and that both nodes recover gracefully.
-	if err := net.RestartNode(net.Alice, nil); err != nil {
-		t.Fatalf("Node restart failed: %v", err)
-	}
-	if err := net.RestartNode(carol, nil); err != nil {
-		t.Fatalf("Node restart failed: %v", err)
-	}
+	ht.RestartNode(alice)
+	ht.RestartNode(carol)
 
 	// The following block ensures that after both nodes have restarted,
 	// they have reconnected before the execution of the next test.
-	net.EnsureConnected(t.t, net.Alice, carol)
+	ht.EnsureConnected(alice, carol)
 
 	// Next, mine enough blocks s.t the channel will open with a single
 	// additional block mined.
-	if _, err := net.Miner.Client.Generate(3); err != nil {
-		t.Fatalf("unable to mine blocks: %v", err)
-	}
+	ht.MineBlocks(3)
 
 	// Assert that our wallet has our opening transaction with a label
 	// that does not have a channel ID set yet, because we have not
 	// reached our required confirmations.
-	tx := findTxAtHeight(t, height, fundingTxStr, net.Alice)
+	tx := ht.AssertTxAtHeight(alice, height, fundingTxID)
 
 	// At this stage, we expect the transaction to be labelled, but not with
 	// our channel ID because our transaction has not yet confirmed.
 	label := labels.MakeLabel(labels.LabelTypeChannelOpen, nil)
-	require.Equal(t.t, label, tx.Label, "open channel label wrong")
+	require.Equal(ht, label, tx.Label, "open channel label wrong")
 
 	// Both nodes should still show a single channel as pending.
-	time.Sleep(time.Second * 1)
-	assertNumOpenChannelsPending(t, net.Alice, carol, 1)
+	ht.AssertNumPendingOpenChannels(alice, 1)
+	ht.AssertNumPendingOpenChannels(carol, 1)
 
 	// Finally, mine the last block which should mark the channel as open.
-	if _, err := net.Miner.Client.Generate(1); err != nil {
-		t.Fatalf("unable to mine blocks: %v", err)
-	}
+	ht.MineBlocks(1)
 
 	// At this point, the channel should be fully opened and there should
 	// be no pending channels remaining for either node.
-	time.Sleep(time.Second * 1)
-	assertNumOpenChannelsPending(t, net.Alice, carol, 0)
+	ht.AssertNumPendingOpenChannels(alice, 0)
+	ht.AssertNumPendingOpenChannels(carol, 0)
 
 	// The channel should be listed in the peer information returned by
 	// both peers.
-	outPoint := wire.OutPoint{
-		Hash:  *fundingTxID,
-		Index: pendingUpdate.OutputIndex,
-	}
+	chanPoint := lntemp.ChanPointFromPendingUpdate(update)
 
 	// Re-lookup our transaction in the block that it confirmed in.
-	tx = findTxAtHeight(t, height, fundingTxStr, net.Alice)
+	tx = ht.AssertTxAtHeight(alice, height, fundingTxID)
+
+	// Check both nodes to ensure that the channel is ready for operation.
+	chanAlice := ht.AssertChannelExists(alice, chanPoint)
+	ht.AssertChannelExists(carol, chanPoint)
 
 	// Create an additional check for our channel assertion that will
 	// check that our label is as expected.
-	check := func(channel *lnrpc.Channel) {
-		shortChanID := lnwire.NewShortChanIDFromInt(
-			channel.ChanId,
-		)
-
-		label := labels.MakeLabel(
-			labels.LabelTypeChannelOpen, &shortChanID,
-		)
-		require.Equal(t.t, label, tx.Label,
-			"open channel label not updated")
-	}
-
-	// Check both nodes to ensure that the channel is ready for operation.
-	err = net.AssertChannelExists(net.Alice, &outPoint, check)
-	if err != nil {
-		t.Fatalf("unable to assert channel existence: %v", err)
-	}
-	if err := net.AssertChannelExists(carol, &outPoint); err != nil {
-		t.Fatalf("unable to assert channel existence: %v", err)
-	}
+	shortChanID := lnwire.NewShortChanIDFromInt(chanAlice.ChanId)
+	label = labels.MakeLabel(labels.LabelTypeChannelOpen, &shortChanID)
+	require.Equal(ht, label, tx.Label, "open channel label not updated")
 
 	// Finally, immediately close the channel. This function will also
 	// block until the channel is closed and will additionally assert the
 	// relevant channel closing post conditions.
-	chanPoint := &lnrpc.ChannelPoint{
-		FundingTxid: &lnrpc.ChannelPoint_FundingTxidBytes{
-			FundingTxidBytes: pendingUpdate.Txid,
-		},
-		OutputIndex: pendingUpdate.OutputIndex,
-	}
-	closeChannelAndAssert(t, net, net.Alice, chanPoint, false)
+	ht.CloseChannel(alice, chanPoint)
 }
 
 // testBatchChanFunding makes sure multiple channels can be opened in one batch

lntest/itest/lnd_misc_test.go

@@ -53,7 +53,7 @@ func testDisconnectingTargetPeer(ht *lntemp.HarnessTest) {
 		Amt:     chanAmt,
 		PushAmt: pushAmt,
 	}
-	stream := ht.OpenChannelAssertPending(alice, bob, p)
+	stream := ht.OpenChannelAssertStream(alice, bob, p)
 
 	// At this point, the channel's funding transaction will have been
 	// broadcast, but not confirmed. Alice and Bob's nodes should reflect
@@ -361,7 +361,7 @@ func testMaxPendingChannels(ht *lntemp.HarnessTest) {
 		[]lnrpc.Lightning_OpenChannelClient, maxPendingChannels,
 	)
 	for i := 0; i < maxPendingChannels; i++ {
-		stream := ht.OpenChannelAssertPending(
+		stream := ht.OpenChannelAssertStream(
 			alice, carol, lntemp.OpenChannelParams{
 				Amt: amount,
 			},
@@ -823,7 +823,7 @@ func testSweepAllCoins(ht *lntemp.HarnessTest) {
 	assertTxLabel := func(targetTx, label string) error {
 		// List all transactions relevant to our wallet, and find the
 		// tx so that we can check the correct label has been set.
-		txResp := ainz.RPC.GetTransactions()
+		txResp := ainz.RPC.GetTransactions(nil)
 
 		var target *lnrpc.Transaction
 

lntest/itest/lnd_test_list_on_test.go

@@ -24,10 +24,6 @@ var allTestCases = []*testCase{
 		name: "graph topology notifications",
 		test: testGraphTopologyNotifications,
 	},
-	{
-		name: "funding flow persistence",
-		test: testChannelFundingPersistence,
-	},
 	{
 		name: "channel force closure",
 		test: testChannelForceClosure,