diff --git a/lntemp/harness.go b/lntemp/harness.go index f15fb443a..ea75edd5f 100644 --- a/lntemp/harness.go +++ b/lntemp/harness.go @@ -6,6 +6,7 @@ import ( "fmt" "sync" "testing" + "time" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg/chainhash" @@ -1344,3 +1345,100 @@ func (h *HarnessTest) SendPaymentAssertFail(hn *node.HarnessNode, return payment } + +// OpenChannelRequest is used to open a channel using the method +// OpenMultiChannelsAsync. +type OpenChannelRequest struct { + // Local is the funding node. + Local *node.HarnessNode + + // Remote is the receiving node. + Remote *node.HarnessNode + + // Param is the open channel params. + Param OpenChannelParams + + // stream is the client created after calling OpenChannel RPC. + stream rpc.OpenChanClient + + // result is a channel used to send the channel point once the funding + // has succeeded. + result chan *lnrpc.ChannelPoint +} + +// OpenMultiChannelsAsync takes a list of OpenChannelRequest and opens them in +// batch. The channel points are returned in same the order of the requests +// once all of the channel open succeeded. +// +// NOTE: compared to open multiple channel sequentially, this method will be +// faster as it doesn't need to mine 6 blocks for each channel open. However, +// it does make debugging the logs more difficult as messages are intertwined. +func (h *HarnessTest) OpenMultiChannelsAsync( + reqs []*OpenChannelRequest) []*lnrpc.ChannelPoint { + + // openChannel opens a channel based on the request. + openChannel := func(req *OpenChannelRequest) { + stream := h.OpenChannelAssertStream( + req.Local, req.Remote, req.Param, + ) + req.stream = stream + } + + // assertChannelOpen is a helper closure that asserts a channel is + // open. + assertChannelOpen := func(req *OpenChannelRequest) { + // Wait for the channel open event from the stream. + cp := h.WaitForChannelOpenEvent(req.stream) + + // Check that both alice and bob have seen the channel + // from their channel watch request. + h.AssertTopologyChannelOpen(req.Local, cp) + h.AssertTopologyChannelOpen(req.Remote, cp) + + // Finally, check that the channel can be seen in their + // ListChannels. + h.AssertChannelExists(req.Local, cp) + h.AssertChannelExists(req.Remote, cp) + + req.result <- cp + } + + // Go through the requests and make the OpenChannel RPC call. + for _, r := range reqs { + openChannel(r) + } + + // Mine one block to confirm all the funding transactions. + h.MineBlocksAndAssertNumTxes(1, len(reqs)) + + // Mine 5 more blocks so all the public channels are announced to the + // network. + h.MineBlocks(numBlocksOpenChannel - 1) + + // Once the blocks are mined, we fire goroutines for each of the + // request to watch for the channel openning. + for _, r := range reqs { + r.result = make(chan *lnrpc.ChannelPoint, 1) + go assertChannelOpen(r) + } + + // Finally, collect the results. + channelPoints := make([]*lnrpc.ChannelPoint, 0) + for _, r := range reqs { + select { + case cp := <-r.result: + channelPoints = append(channelPoints, cp) + + case <-time.After(lntest.ChannelOpenTimeout): + require.Failf(h, "timeout", "wait channel point "+ + "timeout for channel %s=>%s", r.Local.Name(), + r.Remote.Name()) + } + } + + // Assert that we have the expected num of channel points. + require.Len(h, channelPoints, len(reqs), + "returned channel points not match") + + return channelPoints +} diff --git a/lntemp/harness_assertion.go b/lntemp/harness_assertion.go index 87704dc05..6a9f753da 100644 --- a/lntemp/harness_assertion.go +++ b/lntemp/harness_assertion.go @@ -4,6 +4,7 @@ import ( "context" "crypto/rand" "encoding/hex" + "encoding/json" "fmt" "math" "strings" @@ -78,6 +79,22 @@ func (h *HarnessTest) ConnectNodes(a, b *node.HarnessNode) { h.AssertPeerConnected(a, b) } +// ConnectNodesPerm creates a persistent connection between the two nodes and +// asserts the connection is succeeded. +func (h *HarnessTest) ConnectNodesPerm(a, b *node.HarnessNode) { + bobInfo := b.RPC.GetInfo() + + req := &lnrpc.ConnectPeerRequest{ + Addr: &lnrpc.LightningAddress{ + Pubkey: bobInfo.IdentityPubkey, + Host: b.Cfg.P2PAddr(), + }, + Perm: true, + } + a.RPC.ConnectPeer(req) + h.AssertPeerConnected(a, b) +} + // DisconnectNodes disconnects the given two nodes and asserts the // disconnection is succeeded. The request is made from node a and sent to node // b. @@ -1544,3 +1561,38 @@ func (h *HarnessTest) AssertChannelPolicy(hn *node.HarnessNode, err = node.CheckChannelPolicy(policy, expectedPolicy) require.NoErrorf(h, err, "%s: check policy failed", hn.Name()) } + +// AssertNumPolicyUpdates asserts that a given number of channel policy updates +// has been seen in the specified node. +func (h *HarnessTest) AssertNumPolicyUpdates(hn *node.HarnessNode, + chanPoint *lnrpc.ChannelPoint, + advertisingNode *node.HarnessNode, num int) { + + op := h.OutPointFromChannelPoint(chanPoint) + + var policies []*node.PolicyUpdateInfo + + err := wait.NoError(func() error { + policyMap := hn.Watcher.GetPolicyUpdates(op) + nodePolicy, ok := policyMap[advertisingNode.PubKeyStr] + if ok { + policies = nodePolicy + } + + if len(policies) == num { + return nil + } + + p, err := json.MarshalIndent(policies, "", "\t") + require.NoError(h, err, "encode policy err") + + return fmt.Errorf("expected to find %d policy updates, "+ + "instead got: %d, chanPoint: %v, "+ + "advertisingNode: %s:%s, policy: %s", num, + len(policies), op, advertisingNode.Name(), + advertisingNode.PubKeyStr, p) + }, DefaultTimeout) + + require.NoError(h, err, "%s: timeout waiting for num of policy updates", + hn.Name()) +} diff --git a/lntest/itest/list_on_test.go b/lntest/itest/list_on_test.go index 515f2ae99..f41b00b64 100644 --- a/lntest/itest/list_on_test.go +++ b/lntest/itest/list_on_test.go @@ -139,4 +139,8 @@ var allTestCasesTemp = []*lntemp.TestCase{ Name: "update channel policy", TestFunc: testUpdateChannelPolicy, }, + { + Name: "send update disable channel", + TestFunc: testSendUpdateDisableChannel, + }, } diff --git a/lntest/itest/lnd_channel_policy_test.go b/lntest/itest/lnd_channel_policy_test.go index 2db663742..d38e25a5b 100644 --- a/lntest/itest/lnd_channel_policy_test.go +++ b/lntest/itest/lnd_channel_policy_test.go @@ -2,6 +2,7 @@ package itest import ( "context" + "fmt" "math" "time" @@ -435,71 +436,92 @@ func testUpdateChannelPolicy(ht *lntemp.HarnessTest) { // testSendUpdateDisableChannel ensures that a channel update with the disable // flag set is sent once a channel has been either unilaterally or cooperatively // closed. -func testSendUpdateDisableChannel(net *lntest.NetworkHarness, t *harnessTest) { - const ( - chanAmt = 100000 - ) +// +// NOTE: this test can be flaky as we are testing the chan-enable-timeout and +// chan-disable-timeout flags here. For instance, if some operations take more +// than 6 seconds to finish, the channel will be marked as disabled, thus a +// following operation will fail if it relies on the channel being enabled. +func testSendUpdateDisableChannel(ht *lntemp.HarnessTest) { + const chanAmt = 100000 - // Open a channel between Alice and Bob and Alice and Carol. These will - // be closed later on in order to trigger channel update messages - // marking the channels as disabled. - chanPointAliceBob := openChannelAndAssert( - t, net, net.Alice, net.Bob, - lntest.OpenChannelParams{ - Amt: chanAmt, - }, - ) + alice, bob := ht.Alice, ht.Bob - carol := net.NewNode( - t.t, "Carol", []string{ - "--minbackoff=10s", - "--chan-enable-timeout=1.5s", - "--chan-disable-timeout=3s", - "--chan-status-sample-interval=.5s", - }) - defer shutdownAndAssert(net, t, carol) + // Create a new node Eve, which will be restarted later with a config + // that has an inactive channel timeout of just 6 seconds (down from + // the default 20m). It will be used to test channel updates for + // channels going inactive. + // + // NOTE: we don't create Eve with the chan-disable-timeout here because + // the following channel openings might take longer than that timeout + // value, which will cause the channel Eve=>Carol being marked as + // disabled. + eve := ht.NewNode("Eve", nil) - net.ConnectNodes(t.t, net.Alice, carol) - chanPointAliceCarol := openChannelAndAssert( - t, net, net.Alice, carol, - lntest.OpenChannelParams{ - Amt: chanAmt, - }, - ) - - // We create a new node Eve that has an inactive channel timeout of - // just 2 seconds (down from the default 20m). It will be used to test - // channel updates for channels going inactive. - eve := net.NewNode( - t.t, "Eve", []string{ - "--minbackoff=10s", - "--chan-enable-timeout=1.5s", - "--chan-disable-timeout=3s", - "--chan-status-sample-interval=.5s", - }) - defer shutdownAndAssert(net, t, eve) - - // Give Eve some coins. - net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, eve) - - // Connect Eve to Carol and Bob, and open a channel to carol. - net.ConnectNodes(t.t, eve, carol) - net.ConnectNodes(t.t, eve, net.Bob) - - chanPointEveCarol := openChannelAndAssert( - t, net, eve, carol, - lntest.OpenChannelParams{ - Amt: chanAmt, - }, - ) + // Create a new node Carol, which will later be restarted with the same + // config as Eve's. + carol := ht.NewNode("Carol", nil) // Launch a node for Dave which will connect to Bob in order to receive // graph updates from. This will ensure that the channel updates are // propagated throughout the network. - dave := net.NewNode(t.t, "Dave", nil) - defer shutdownAndAssert(net, t, dave) + dave := ht.NewNode("Dave", nil) - net.ConnectNodes(t.t, net.Bob, dave) + // We will start our test by creating the following topology, + // Alice --- Bob --- Dave + // | | + // Carol --- Eve + ht.EnsureConnected(alice, bob) + ht.ConnectNodes(alice, carol) + ht.ConnectNodes(bob, dave) + ht.ConnectNodes(eve, carol) + + // Connect Eve and Bob using a persistent connection. Later after Eve + // is restarted, they will connect again automatically. + ht.ConnectNodesPerm(bob, eve) + + // Give Eve some coins. + ht.FundCoins(btcutil.SatoshiPerBitcoin, eve) + + // We now proceed to open channels: Alice=>Bob, Alice=>Carol and + // Eve=>Carol. + p := lntemp.OpenChannelParams{Amt: chanAmt} + reqs := []*lntemp.OpenChannelRequest{ + {Local: alice, Remote: bob, Param: p}, + {Local: alice, Remote: carol, Param: p}, + {Local: eve, Remote: carol, Param: p}, + } + resp := ht.OpenMultiChannelsAsync(reqs) + + // Extract channel points from the response. + chanPointAliceBob := resp[0] + chanPointAliceCarol := resp[1] + chanPointEveCarol := resp[2] + + // We will use 10 seconds as the disable timeout. + chanDisableTimeout := 10 + chanEnableTimeout := 5 + + // waitChanDisabled is a helper closure to wait the chanDisableTimeout + // seconds such that the channel disable logic is taking effect. + waitChanDisabled := func() { + time.Sleep(time.Duration(chanDisableTimeout) * time.Second) + } + + // With the channels open, we now restart Carol and Eve to use + // customized timeout values. + nodeCfg := []string{ + "--minbackoff=60s", + fmt.Sprintf("--chan-enable-timeout=%ds", chanEnableTimeout), + fmt.Sprintf("--chan-disable-timeout=%ds", chanDisableTimeout), + "--chan-status-sample-interval=.5s", + } + ht.RestartNodeWithExtraArgs(carol, nodeCfg) + ht.RestartNodeWithExtraArgs(eve, nodeCfg) + + // Dave should know all the channels. + ht.AssertTopologyChannelOpen(dave, chanPointAliceBob) + ht.AssertTopologyChannelOpen(dave, chanPointAliceCarol) + ht.AssertTopologyChannelOpen(dave, chanPointEveCarol) // We should expect to see a channel update with the default routing // policy, except that it should indicate the channel is disabled. @@ -514,110 +536,132 @@ func testSendUpdateDisableChannel(net *lntest.NetworkHarness, t *harnessTest) { // assertPolicyUpdate checks that the required policy update has // happened on the given node. - assertPolicyUpdate := func(node *lntest.HarnessNode, - policy *lnrpc.RoutingPolicy, chanPoint *lnrpc.ChannelPoint) { + assertPolicyUpdate := func(node *node.HarnessNode, + policy *lnrpc.RoutingPolicy, chanPoint *lnrpc.ChannelPoint, + numUpdates int) { - require.NoError( - t.t, dave.WaitForChannelPolicyUpdate( - node.PubKeyStr, policy, chanPoint, false, - ), "error while waiting for channel update", + ht.AssertNumPolicyUpdates(dave, chanPoint, node, numUpdates) + ht.AssertChannelPolicyUpdate( + dave, node, policy, chanPoint, false, ) } - // Let Carol go offline. Since Eve has an inactive timeout of 2s, we + // Let Carol go offline. Since Eve has an inactive timeout of 6s, we // expect her to send an update disabling the channel. - restartCarol, err := net.SuspendNode(carol) - if err != nil { - t.Fatalf("unable to suspend carol: %v", err) - } + restartCarol := ht.SuspendNode(carol) - assertPolicyUpdate(eve, expectedPolicy, chanPointEveCarol) + // We expect to see a total of 2 channel policy updates from the + // channel Carol <-> Eve and advertised by Eve using the route + // Eve->Bob->Dave. + waitChanDisabled() + assertPolicyUpdate(eve, expectedPolicy, chanPointEveCarol, 2) // We restart Carol. Since the channel now becomes active again, Eve // should send a ChannelUpdate setting the channel no longer disabled. - if err := restartCarol(); err != nil { - t.Fatalf("unable to restart carol: %v", err) - } + require.NoError(ht, restartCarol(), "unable to restart carol") expectedPolicy.Disabled = false - assertPolicyUpdate(eve, expectedPolicy, chanPointEveCarol) + // We expect to see a total of 3 channel policy updates from the + // channel Carol <-> Eve and advertised by Eve using the route + // Eve->Bob->Dave. + assertPolicyUpdate(eve, expectedPolicy, chanPointEveCarol, 3) // Wait until Carol and Eve are reconnected before we disconnect them // again. - net.EnsureConnected(t.t, eve, carol) + ht.EnsureConnected(eve, carol) // Now we'll test a long disconnection. Disconnect Carol and Eve and // ensure they both detect each other as disabled. Their min backoffs // are high enough to not interfere with disabling logic. - if err := net.DisconnectNodes(carol, eve); err != nil { - t.Fatalf("unable to disconnect Carol from Eve: %v", err) - } + ht.DisconnectNodes(carol, eve) // Wait for a disable from both Carol and Eve to come through. expectedPolicy.Disabled = true - assertPolicyUpdate(eve, expectedPolicy, chanPointEveCarol) - assertPolicyUpdate(carol, expectedPolicy, chanPointEveCarol) + // We expect to see a total of 4 channel policy updates from the + // channel Carol <-> Eve and advertised by Eve using the route + // Eve->Bob->Dave. + waitChanDisabled() + assertPolicyUpdate(eve, expectedPolicy, chanPointEveCarol, 4) + + // Because Carol has restarted twice before, depending on how much time + // it has taken, she might mark the channel disabled and enable it + // multiple times. Thus we could see a total of 2 or 4 or 6 channel + // policy updates from the channel Carol <-> Eve and advertised by + // Carol using the route Carol->Alice->Bob->Dave. + // + // Assume there are 2 channel policy updates from Carol, and update it + // if more has found + numCarol := 2 + op := ht.OutPointFromChannelPoint(chanPointEveCarol) + policyMap := dave.Watcher.GetPolicyUpdates(op) + nodePolicy, ok := policyMap[carol.PubKeyStr] + switch { + case !ok: + break + case len(nodePolicy) > 2: + numCarol = 4 + case len(nodePolicy) > 4: + numCarol = 6 + } + assertPolicyUpdate(carol, expectedPolicy, chanPointEveCarol, numCarol) // Reconnect Carol and Eve, this should cause them to reenable the // channel from both ends after a short delay. - net.EnsureConnected(t.t, carol, eve) + ht.EnsureConnected(carol, eve) expectedPolicy.Disabled = false - assertPolicyUpdate(eve, expectedPolicy, chanPointEveCarol) - assertPolicyUpdate(carol, expectedPolicy, chanPointEveCarol) + // We expect to see a total of 5 channel policy updates from the + // channel Carol <-> Eve and advertised by Eve using the route + // Eve->Bob->Dave. + assertPolicyUpdate(eve, expectedPolicy, chanPointEveCarol, 5) + // We expect to see a total of 3 or 5 channel policy updates from the + // channel Carol <-> Eve and advertised by Carol using the route + // Carol->Alice->Bob->Dave. + numCarol++ + assertPolicyUpdate(carol, expectedPolicy, chanPointEveCarol, numCarol) // Now we'll test a short disconnection. Disconnect Carol and Eve, then // reconnect them after one second so that their scheduled disables are // aborted. One second is twice the status sample interval, so this // should allow for the disconnect to be detected, but still leave time - // to cancel the announcement before the 3 second inactive timeout is + // to cancel the announcement before the 6 second inactive timeout is // hit. - if err := net.DisconnectNodes(carol, eve); err != nil { - t.Fatalf("unable to disconnect Carol from Eve: %v", err) - } + ht.DisconnectNodes(carol, eve) time.Sleep(time.Second) - net.EnsureConnected(t.t, eve, carol) + ht.EnsureConnected(eve, carol) - // Since the disable should have been canceled by both Carol and Eve, we - // expect no channel updates to appear on the network, which means we - // expect the polices stay unchanged(Disable == false). - assertPolicyUpdate(eve, expectedPolicy, chanPointEveCarol) - assertPolicyUpdate(carol, expectedPolicy, chanPointEveCarol) + // Since the disable should have been canceled by both Carol and Eve, + // we expect no channel updates to appear on the network, which means + // we expect the polices stay unchanged(Disable == false). + assertPolicyUpdate(eve, expectedPolicy, chanPointEveCarol, 5) + assertPolicyUpdate(carol, expectedPolicy, chanPointEveCarol, numCarol) // Close Alice's channels with Bob and Carol cooperatively and - // unilaterally respectively. - _, _, err = net.CloseChannel(net.Alice, chanPointAliceBob, false) - if err != nil { - t.Fatalf("unable to close channel: %v", err) - } - - _, _, err = net.CloseChannel(net.Alice, chanPointAliceCarol, true) - if err != nil { - t.Fatalf("unable to close channel: %v", err) - } + // unilaterally respectively. Note that the CloseChannel will mine a + // block and check that the closing transaction can be found in both + // the mempool and the block. + ht.CloseChannel(alice, chanPointAliceBob) + ht.ForceCloseChannel(alice, chanPointAliceCarol) // Now that the channel close processes have been started, we should // receive an update marking each as disabled. expectedPolicy.Disabled = true - assertPolicyUpdate(net.Alice, expectedPolicy, chanPointAliceBob) - assertPolicyUpdate(net.Alice, expectedPolicy, chanPointAliceCarol) - - // Finally, close the channels by mining the closing transactions. - mineBlocks(t, net, 1, 2) + // We expect to see a total of 2 channel policy updates from the + // channel Alice <-> Bob and advertised by Alice using the route + // Alice->Bob->Dave. + assertPolicyUpdate(alice, expectedPolicy, chanPointAliceBob, 2) + // We expect to see a total of 2 channel policy updates from the + // channel Alice <-> Carol and advertised by Alice using the route + // Alice->Bob->Dave. + assertPolicyUpdate(alice, expectedPolicy, chanPointAliceCarol, 2) // Also do this check for Eve's channel with Carol. - _, _, err = net.CloseChannel(eve, chanPointEveCarol, false) - if err != nil { - t.Fatalf("unable to close channel: %v", err) - } + ht.CloseChannel(eve, chanPointEveCarol) - assertPolicyUpdate(eve, expectedPolicy, chanPointEveCarol) - - mineBlocks(t, net, 1, 1) - - // And finally, clean up the force closed channel by mining the - // sweeping transaction. - cleanupForceClose(t, net, net.Alice, chanPointAliceCarol) + // We expect to see a total of 5 channel policy updates from the + // channel Carol <-> Eve and advertised by Eve using the route + // Eve->Bob->Dave. + assertPolicyUpdate(eve, expectedPolicy, chanPointEveCarol, 6) } // testUpdateChannelPolicyForPrivateChannel tests when a private channel diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index 0341d7ac9..5c1b90fc3 100644 --- a/lntest/itest/lnd_test_list_on_test.go +++ b/lntest/itest/lnd_test_list_on_test.go @@ -163,10 +163,6 @@ var allTestCases = []*testCase{ name: "route fee cutoff", test: testRouteFeeCutoff, }, - { - name: "send update disable channel", - test: testSendUpdateDisableChannel, - }, { name: "streaming channel backup update", test: testChannelBackupUpdates,