From 6b82676ca2f074469415d56b527459b21521cef7 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 11 Aug 2022 07:15:33 +0800 Subject: [PATCH] lntemp+itest: refactor `testOptionScidAlias` --- lntemp/harness.go | 94 ++++++++++++++++++--- lntest/itest/list_on_test.go | 4 + lntest/itest/lnd_test_list_on_test.go | 4 - lntest/itest/lnd_zero_conf_test.go | 117 ++++++++++---------------- 4 files changed, 132 insertions(+), 87 deletions(-) diff --git a/lntemp/harness.go b/lntemp/harness.go index 137107e0d..f366805f1 100644 --- a/lntemp/harness.go +++ b/lntemp/harness.go @@ -885,24 +885,76 @@ func (h *HarnessTest) OpenChannelAssertStream(srcNode, } // OpenChannel attempts to open a channel with the specified parameters -// extended from Alice to Bob. Additionally, the following items are asserted, -// - 6 blocks will be mined so the channel will be announced if it's public. -// - the funding transaction should be found in the first block. +// extended from Alice to Bob. Additionally, for public channels, it will mine +// extra blocks so they are announced to the network. In specific, the +// following items are asserted, +// - for non-zero conf channel, 1 blocks will be mined to confirm the funding +// tx. // - both nodes should see the channel edge update in their network graph. // - both nodes can report the status of the new channel from ListChannels. +// - extra blocks are mined if it's a public channel. func (h *HarnessTest) OpenChannel(alice, bob *node.HarnessNode, p OpenChannelParams) *lnrpc.ChannelPoint { + // First, open the channel without announcing it. + cp := h.OpenChannelNoAnnounce(alice, bob, p) + + // If this is a private channel, there's no need to mine extra blocks + // since it will never be announced to the network. + if p.Private { + return cp + } + + // Mine extra blocks to announce the channel. + if p.ZeroConf { + // For a zero-conf channel, no blocks have been mined so we + // need to mine 6 blocks. + // + // Mine 1 block to confirm the funding transaction. + h.MineBlocksAndAssertNumTxes(numBlocksOpenChannel, 1) + } else { + // For a regular channel, 1 block has already been mined to + // confirm the funding transaction, so we mine 5 blocks. + h.MineBlocks(numBlocksOpenChannel - 1) + } + + return cp +} + +// OpenChannelNoAnnounce attempts to open a channel with the specified +// parameters extended from Alice to Bob without mining the necessary blocks to +// announce the channel. Additionally, the following items are asserted, +// - for non-zero conf channel, 1 blocks will be mined to confirm the funding +// tx. +// - both nodes should see the channel edge update in their network graph. +// - both nodes can report the status of the new channel from ListChannels. +func (h *HarnessTest) OpenChannelNoAnnounce(alice, bob *node.HarnessNode, + p OpenChannelParams) *lnrpc.ChannelPoint { + 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 - // within the first newly mined block. We mine 6 blocks so that in the - // case that the channel is public, it is announced to the network. - block := h.MineBlocksAndAssertNumTxes(numBlocksOpenChannel, 1)[0] + // Open a zero conf channel. + if p.ZeroConf { + return h.openChannelZeroConf(alice, bob, chanOpenUpdate) + } + + // Open a non-zero conf channel. + return h.openChannel(alice, bob, chanOpenUpdate) +} + +// openChannel attempts to open a channel with the specified parameters +// extended from Alice to Bob. Additionally, the following items are asserted, +// - 1 block is mined and the funding transaction should be found in it. +// - both nodes should see the channel edge update in their network graph. +// - both nodes can report the status of the new channel from ListChannels. +func (h *HarnessTest) openChannel(alice, bob *node.HarnessNode, + stream rpc.OpenChanClient) *lnrpc.ChannelPoint { + + // Mine 1 block to confirm the funding transaction. + block := h.MineBlocksAndAssertNumTxes(1, 1)[0] // Wait for the channel open event. - fundingChanPoint := h.WaitForChannelOpenEvent(chanOpenUpdate) + fundingChanPoint := h.WaitForChannelOpenEvent(stream) // Check that the funding tx is found in the first block. fundingTxID := h.GetChanPointFundingTxid(fundingChanPoint) @@ -917,9 +969,27 @@ func (h *HarnessTest) OpenChannel(alice, bob *node.HarnessNode, h.AssertChannelExists(alice, fundingChanPoint) h.AssertChannelExists(bob, fundingChanPoint) - // Finally, check the blocks are synced. - h.WaitForBlockchainSync(alice) - h.WaitForBlockchainSync(bob) + return fundingChanPoint +} + +// openChannelZeroConf attempts to open a channel with the specified parameters +// extended from Alice to Bob. Additionally, the following items are asserted, +// - both nodes should see the channel edge update in their network graph. +// - both nodes can report the status of the new channel from ListChannels. +func (h *HarnessTest) openChannelZeroConf(alice, bob *node.HarnessNode, + stream rpc.OpenChanClient) *lnrpc.ChannelPoint { + + // Wait for the channel open event. + fundingChanPoint := h.WaitForChannelOpenEvent(stream) + + // Check that both alice and bob have seen the channel from their + // network topology. + h.AssertTopologyChannelOpen(alice, fundingChanPoint) + h.AssertTopologyChannelOpen(bob, fundingChanPoint) + + // Finally, check that the channel can be seen in their ListChannels. + h.AssertChannelExists(alice, fundingChanPoint) + h.AssertChannelExists(bob, fundingChanPoint) return fundingChanPoint } diff --git a/lntest/itest/list_on_test.go b/lntest/itest/list_on_test.go index 894869ea0..4dfcee189 100644 --- a/lntest/itest/list_on_test.go +++ b/lntest/itest/list_on_test.go @@ -421,4 +421,8 @@ var allTestCasesTemp = []*lntemp.TestCase{ Name: "zero conf channel open", TestFunc: testZeroConfChannelOpen, }, + { + Name: "option scid alias", + TestFunc: testOptionScidAlias, + }, } diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index 7b9016ee3..e43ff763f 100644 --- a/lntest/itest/lnd_test_list_on_test.go +++ b/lntest/itest/lnd_test_list_on_test.go @@ -56,10 +56,6 @@ var allTestCases = []*testCase{ name: "taproot", test: testTaproot, }, - { - name: "option scid alias", - test: testOptionScidAlias, - }, { name: "scid alias channel update", test: testUpdateChannelPolicyScidAlias, diff --git a/lntest/itest/lnd_zero_conf_test.go b/lntest/itest/lnd_zero_conf_test.go index ebc6c69dd..17a5fa6c4 100644 --- a/lntest/itest/lnd_zero_conf_test.go +++ b/lntest/itest/lnd_zero_conf_test.go @@ -189,7 +189,7 @@ func testZeroConfChannelOpen(ht *lntemp.HarnessTest) { // testOptionScidAlias checks that opening an option_scid_alias channel-type // channel or w/o the channel-type works properly. -func testOptionScidAlias(net *lntest.NetworkHarness, t *harnessTest) { +func testOptionScidAlias(ht *lntemp.HarnessTest) { type scidTestCase struct { name string @@ -220,10 +220,10 @@ func testOptionScidAlias(net *lntest.NetworkHarness, t *harnessTest) { for _, testCase := range testCases { testCase := testCase - success := t.t.Run(testCase.name, func(t *testing.T) { - h := newHarnessTest(t, net) + success := ht.Run(testCase.name, func(t *testing.T) { + st := ht.Subtest(t) optionScidAliasScenario( - net, h, testCase.chantype, testCase.private, + st, testCase.chantype, testCase.private, ) }) if !success { @@ -232,110 +232,89 @@ func testOptionScidAlias(net *lntest.NetworkHarness, t *harnessTest) { } } -func optionScidAliasScenario(net *lntest.NetworkHarness, t *harnessTest, - chantype, private bool) { - - ctxb := context.Background() - +func optionScidAliasScenario(ht *lntemp.HarnessTest, chantype, private bool) { // Option-scid-alias is opt-in, as is anchors. scidAliasArgs := []string{ "--protocol.option-scid-alias", "--protocol.anchors", } - carol := net.NewNode(t.t, "Carol", scidAliasArgs) - defer shutdownAndAssert(net, t, carol) - - dave := net.NewNode(t.t, "Dave", scidAliasArgs) - defer shutdownAndAssert(net, t, dave) + bob := ht.Bob + carol := ht.NewNode("Carol", scidAliasArgs) + dave := ht.NewNode("Dave", scidAliasArgs) // Ensure Bob, Carol are connected. - net.EnsureConnected(t.t, net.Bob, carol) + ht.EnsureConnected(bob, carol) // Ensure Carol, Dave are connected. - net.EnsureConnected(t.t, carol, dave) + ht.EnsureConnected(carol, dave) // Give Carol some coins so she can open the channel. - net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, carol) + ht.FundCoins(btcutil.SatoshiPerBitcoin, carol) chanAmt := btcutil.Amount(1_000_000) - params := lntest.OpenChannelParams{ + params := lntemp.OpenChannelParams{ Amt: chanAmt, Private: private, CommitmentType: lnrpc.CommitmentType_ANCHORS, ScidAlias: chantype, } - fundingPoint := openChannelAndAssert(t, net, carol, dave, params) + fundingPoint := ht.OpenChannel(carol, dave, params) + // Make sure Bob knows this channel if it's public. if !private { - err := net.Bob.WaitForNetworkChannelOpen(fundingPoint) - require.NoError(t.t, err, "bob didn't report channel") + ht.AssertTopologyChannelOpen(bob, fundingPoint) } - err := carol.WaitForNetworkChannelOpen(fundingPoint) - require.NoError(t.t, err, "carol didn't report channel") - err = dave.WaitForNetworkChannelOpen(fundingPoint) - require.NoError(t.t, err, "dave didn't report channel") - // Assert that a payment from Carol to Dave works as expected. daveInvoiceParams := &lnrpc.Invoice{ Value: int64(10_000), Private: true, } - daveInvoiceResp, err := dave.AddInvoice(ctxb, daveInvoiceParams) - require.NoError(t.t, err) - _ = sendAndAssertSuccess( - t, carol, &routerrpc.SendPaymentRequest{ - PaymentRequest: daveInvoiceResp.PaymentRequest, - TimeoutSeconds: 60, - FeeLimitMsat: noFeeLimitMsat, - }, + daveInvoiceResp := dave.RPC.AddInvoice(daveInvoiceParams) + ht.CompletePaymentRequests( + carol, []string{daveInvoiceResp.PaymentRequest}, ) // We'll now open a regular public channel between Bob and Carol and // assert that Bob can pay Dave. We'll also assert that the invoice // Dave issues has the startingAlias as a hop hint. - fundingPoint2 := openChannelAndAssert( - t, net, net.Bob, carol, - lntest.OpenChannelParams{ - Amt: chanAmt, - }, - ) + p := lntemp.OpenChannelParams{ + Amt: chanAmt, + } + fundingPoint2 := ht.OpenChannel(bob, carol, p) - err = net.Bob.WaitForNetworkChannelOpen(fundingPoint2) - require.NoError(t.t, err) - err = carol.WaitForNetworkChannelOpen(fundingPoint2) - require.NoError(t.t, err) + defer func() { + // TODO(yy): remove the sleep once the following bug is fixed. + // When the payment is reported as settled by Bob, it's + // expected the commitment dance is finished and all subsequent + // states have been updated. Yet we'd receive the error `cannot + // co-op close channel with active htlcs` or `link failed to + // shutdown` if we close the channel. We need to investigate + // the order of settling the payments and updating commitments + // to understand and fix. + time.Sleep(2 * time.Second) + + // Close standby node's channels. + ht.CloseChannel(bob, fundingPoint2) + }() // Wait until Dave receives the Bob<->Carol channel. - err = dave.WaitForNetworkChannelOpen(fundingPoint2) - require.NoError(t.t, err) + ht.AssertTopologyChannelOpen(dave, fundingPoint2) - daveInvoiceResp2, err := dave.AddInvoice(ctxb, daveInvoiceParams) - require.NoError(t.t, err) - davePayReq := &lnrpc.PayReqString{ - PayReq: daveInvoiceResp2.PaymentRequest, - } - - decodedReq, err := dave.DecodePayReq(ctxb, davePayReq) - require.NoError(t.t, err) + daveInvoiceResp2 := dave.RPC.AddInvoice(daveInvoiceParams) + decodedReq := dave.RPC.DecodePayReq(daveInvoiceResp2.PaymentRequest) if !private { - require.Equal(t.t, 0, len(decodedReq.RouteHints)) + require.Len(ht, decodedReq.RouteHints, 0) payReq := daveInvoiceResp2.PaymentRequest - _ = sendAndAssertSuccess( - t, net.Bob, &routerrpc.SendPaymentRequest{ - PaymentRequest: payReq, - TimeoutSeconds: 60, - FeeLimitMsat: noFeeLimitMsat, - }, - ) + ht.CompletePaymentRequests(bob, []string{payReq}) return } - require.Equal(t.t, 1, len(decodedReq.RouteHints)) - require.Equal(t.t, 1, len(decodedReq.RouteHints[0].HopHints)) + require.Len(ht, decodedReq.RouteHints, 1) + require.Len(ht, decodedReq.RouteHints[0].HopHints, 1) startingAlias := lnwire.ShortChannelID{ BlockHeight: 16_000_000, @@ -344,14 +323,10 @@ func optionScidAliasScenario(net *lntest.NetworkHarness, t *harnessTest, } daveHopHint := decodedReq.RouteHints[0].HopHints[0].ChanId - require.Equal(t.t, startingAlias.ToUint64(), daveHopHint) + require.Equal(ht, startingAlias.ToUint64(), daveHopHint) - _ = sendAndAssertSuccess( - t, net.Bob, &routerrpc.SendPaymentRequest{ - PaymentRequest: daveInvoiceResp2.PaymentRequest, - TimeoutSeconds: 60, - FeeLimitMsat: noFeeLimitMsat, - }, + ht.CompletePaymentRequests( + bob, []string{daveInvoiceResp2.PaymentRequest}, ) }