From 21097feb858a3517bcf97bf46b7e7e38d17fd78e Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 4 Aug 2022 02:36:12 +0800 Subject: [PATCH] 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 | 35 ++++++- lntemp/harness_assertion.go | 24 +++++ lntemp/rpc/lnd.go | 10 +- lntemp/utils.go | 14 +++ lntest/itest/list_on_test.go | 4 + lntest/itest/lnd_funding_test.go | 126 +++++++++----------------- lntest/itest/lnd_misc_test.go | 6 +- lntest/itest/lnd_test_list_on_test.go | 4 - 8 files changed, 127 insertions(+), 96 deletions(-) diff --git a/lntemp/harness.go b/lntemp/harness.go index 397ba3524..36d03a94c 100644 --- a/lntemp/harness.go +++ b/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 diff --git a/lntemp/harness_assertion.go b/lntemp/harness_assertion.go index 41912d355..1b9c29a9c 100644 --- a/lntemp/harness_assertion.go +++ b/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 +} diff --git a/lntemp/rpc/lnd.go b/lntemp/rpc/lnd.go index 7f7261233..5cf5d5b2b 100644 --- a/lntemp/rpc/lnd.go +++ b/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 diff --git a/lntemp/utils.go b/lntemp/utils.go index d6d279765..590380423 100644 --- a/lntemp/utils.go +++ b/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 +} diff --git a/lntest/itest/list_on_test.go b/lntest/itest/list_on_test.go index f6b10af85..cc875f805 100644 --- a/lntest/itest/list_on_test.go +++ b/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, + }, } diff --git a/lntest/itest/lnd_funding_test.go b/lntest/itest/lnd_funding_test.go index 486f9525c..5ad24f948 100644 --- a/lntest/itest/lnd_funding_test.go +++ b/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 diff --git a/lntest/itest/lnd_misc_test.go b/lntest/itest/lnd_misc_test.go index 65847e989..9d80c9b3e 100644 --- a/lntest/itest/lnd_misc_test.go +++ b/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 diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index 9c65aca59..4d069d853 100644 --- a/lntest/itest/lnd_test_list_on_test.go +++ b/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,