diff --git a/lntemp/harness.go b/lntemp/harness.go index 33134cac8..3f4754417 100644 --- a/lntemp/harness.go +++ b/lntemp/harness.go @@ -1439,6 +1439,14 @@ func (h *HarnessTest) SendPaymentAssertFail(hn *node.HarnessNode, return payment } +// SendPaymentAssertSettled sends a payment from the passed node and asserts the +// payment is settled. +func (h *HarnessTest) SendPaymentAssertSettled(hn *node.HarnessNode, + req *routerrpc.SendPaymentRequest) *lnrpc.Payment { + + return h.SendPaymentAndAssertStatus(hn, req, lnrpc.Payment_SUCCEEDED) +} + // OpenChannelRequest is used to open a channel using the method // OpenMultiChannelsAsync. type OpenChannelRequest struct { diff --git a/lntest/itest/list_on_test.go b/lntest/itest/list_on_test.go index 986464284..ca6ca11e4 100644 --- a/lntest/itest/list_on_test.go +++ b/lntest/itest/list_on_test.go @@ -337,4 +337,8 @@ var allTestCasesTemp = []*lntemp.TestCase{ Name: "query routes", TestFunc: testQueryRoutes, }, + { + Name: "route fee cutoff", + TestFunc: testRouteFeeCutoff, + }, } diff --git a/lntest/itest/lnd_routing_test.go b/lntest/itest/lnd_routing_test.go index 22a1d2956..a27300337 100644 --- a/lntest/itest/lnd_routing_test.go +++ b/lntest/itest/lnd_routing_test.go @@ -1,20 +1,17 @@ package itest import ( - "context" "encoding/hex" "fmt" "testing" "time" "github.com/btcsuite/btcd/btcutil" - "github.com/btcsuite/btcd/wire" "github.com/lightningnetwork/lnd/chainreg" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lnrpc/routerrpc" "github.com/lightningnetwork/lnd/lntemp" "github.com/lightningnetwork/lnd/lntemp/node" - "github.com/lightningnetwork/lnd/lntest" "github.com/lightningnetwork/lnd/lntest/wait" "github.com/lightningnetwork/lnd/lnwire" "github.com/stretchr/testify/require" @@ -1068,9 +1065,7 @@ func testMissionControlImport(ht *lntemp.HarnessTest, hn *node.HarnessNode, // testRouteFeeCutoff tests that we are able to prevent querying routes and // sending payments that incur a fee higher than the fee limit. -func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { - ctxb := context.Background() - +func testRouteFeeCutoff(ht *lntemp.HarnessTest) { // For this test, we'll create the following topology: // // --- Bob --- @@ -1085,75 +1080,45 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { const chanAmt = btcutil.Amount(100000) // Open a channel between Alice and Bob. - chanPointAliceBob := openChannelAndAssert( - t, net, net.Alice, net.Bob, - lntest.OpenChannelParams{ - Amt: chanAmt, - }, + alice, bob := ht.Alice, ht.Bob + chanPointAliceBob := ht.OpenChannel( + alice, bob, lntemp.OpenChannelParams{Amt: chanAmt}, ) // Create Carol's node and open a channel between her and Alice with // Alice being the funder. - carol := net.NewNode(t.t, "Carol", nil) - defer shutdownAndAssert(net, t, carol) + carol := ht.NewNode("Carol", nil) + ht.ConnectNodes(carol, alice) + ht.FundCoins(btcutil.SatoshiPerBitcoin, carol) - net.ConnectNodes(t.t, carol, net.Alice) - net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, carol) - - chanPointAliceCarol := openChannelAndAssert( - t, net, net.Alice, carol, - lntest.OpenChannelParams{ - Amt: chanAmt, - }, + chanPointAliceCarol := ht.OpenChannel( + alice, carol, lntemp.OpenChannelParams{Amt: chanAmt}, ) // Create Dave's node and open a channel between him and Bob with Bob // being the funder. - dave := net.NewNode(t.t, "Dave", nil) - defer shutdownAndAssert(net, t, dave) - - net.ConnectNodes(t.t, dave, net.Bob) - chanPointBobDave := openChannelAndAssert( - t, net, net.Bob, dave, - lntest.OpenChannelParams{ - Amt: chanAmt, - }, + dave := ht.NewNode("Dave", nil) + ht.ConnectNodes(dave, bob) + chanPointBobDave := ht.OpenChannel( + bob, dave, lntemp.OpenChannelParams{Amt: chanAmt}, ) // Open a channel between Carol and Dave. - net.ConnectNodes(t.t, carol, dave) - chanPointCarolDave := openChannelAndAssert( - t, net, carol, dave, - lntest.OpenChannelParams{ - Amt: chanAmt, - }, + ht.ConnectNodes(carol, dave) + chanPointCarolDave := ht.OpenChannel( + carol, dave, lntemp.OpenChannelParams{Amt: chanAmt}, ) // Now that all the channels were set up, we'll wait for all the nodes // to have seen all the channels. - nodes := []*lntest.HarnessNode{net.Alice, net.Bob, carol, dave} - nodeNames := []string{"alice", "bob", "carol", "dave"} + nodes := []*node.HarnessNode{alice, bob, carol, dave} networkChans := []*lnrpc.ChannelPoint{ chanPointAliceBob, chanPointAliceCarol, chanPointBobDave, chanPointCarolDave, } for _, chanPoint := range networkChans { - for i, node := range nodes { - txid, err := lnrpc.GetChanPointFundingTxid(chanPoint) - if err != nil { - t.Fatalf("unable to get txid: %v", err) - } - outpoint := wire.OutPoint{ - Hash: *txid, - Index: chanPoint.OutputIndex, - } - - err = node.WaitForNetworkChannelOpen(chanPoint) - if err != nil { - t.Fatalf("%s(%d) timed out waiting for "+ - "channel(%s) open: %v", nodeNames[i], - node.NodeID, outpoint, err) - } + for _, node := range nodes { + ht.AssertTopologyChannelOpen(node, chanPoint) } } @@ -1184,56 +1149,35 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { ChanPoint: chanPointCarolDave, }, } - ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) - if _, err := carol.UpdateChannelPolicy(ctxt, updateFeeReq); err != nil { - t.Fatalf("unable to update chan policy: %v", err) - } + carol.RPC.UpdateChannelPolicy(updateFeeReq) // Wait for Alice to receive the channel update from Carol. - assertChannelPolicyUpdate( - t.t, net.Alice, carol.PubKeyStr, - expectedPolicy, chanPointCarolDave, false, + ht.AssertChannelPolicyUpdate( + alice, carol, expectedPolicy, chanPointCarolDave, false, ) // We'll also need the channel IDs for Bob's channels in order to // confirm the route of the payments. - listReq := &lnrpc.ListChannelsRequest{} - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - listResp, err := net.Bob.ListChannels(ctxt, listReq) - if err != nil { - t.Fatalf("unable to retrieve bob's channels: %v", err) - } + channel := ht.QueryChannelByChanPoint(bob, chanPointAliceBob) + aliceBobChanID := channel.ChanId + require.NotZero(ht, aliceBobChanID, + "channel between alice and bob not found") - var aliceBobChanID, bobDaveChanID uint64 - for _, channel := range listResp.Channels { - switch channel.RemotePubkey { - case net.Alice.PubKeyStr: - aliceBobChanID = channel.ChanId - case dave.PubKeyStr: - bobDaveChanID = channel.ChanId - } - } + channel = ht.QueryChannelByChanPoint(bob, chanPointBobDave) + bobDaveChanID := channel.ChanId + require.NotZero(ht, bobDaveChanID, + "channel between bob and dave not found") - if aliceBobChanID == 0 { - t.Fatalf("channel between alice and bob not found") - } - if bobDaveChanID == 0 { - t.Fatalf("channel between bob and dave not found") - } hopChanIDs := []uint64{aliceBobChanID, bobDaveChanID} // checkRoute is a helper closure to ensure the route contains the // correct intermediate hops. checkRoute := func(route *lnrpc.Route) { - if len(route.Hops) != 2 { - t.Fatalf("expected two hops, got %d", len(route.Hops)) - } + require.Len(ht, route.Hops, 2, "expected two hops") for i, hop := range route.Hops { - if hop.ChanId != hopChanIDs[i] { - t.Fatalf("expected chan id %d, got %d", - hopChanIDs[i], hop.ChanId) - } + require.Equal(ht, hopChanIDs[i], hop.ChanId, + "hop chan id not match") } } @@ -1252,20 +1196,12 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { Amt: paymentAmt, FeeLimit: feeLimit, } - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - routesResp, err := net.Alice.QueryRoutes(ctxt, queryRoutesReq) - if err != nil { - t.Fatalf("unable to get routes: %v", err) - } + routesResp := alice.RPC.QueryRoutes(queryRoutesReq) checkRoute(routesResp.Routes[0]) invoice := &lnrpc.Invoice{Value: paymentAmt} - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - invoiceResp, err := dave.AddInvoice(ctxt, invoice) - if err != nil { - t.Fatalf("unable to create invoice: %v", err) - } + invoiceResp := dave.RPC.AddInvoice(invoice) sendReq := &routerrpc.SendPaymentRequest{ PaymentRequest: invoiceResp.PaymentRequest, @@ -1279,7 +1215,7 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { sendReq.FeeLimitMsat = 1000 * paymentAmt * limit.Percent / 100 } - result := sendAndAssertSuccess(t, net.Alice, sendReq) + result := ht.SendPaymentAssertSettled(alice, sendReq) checkRoute(result.Htlcs[0].Route) } @@ -1304,12 +1240,21 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) { } testFeeCutoff(feeLimitFixed) + // TODO(yy): remove the sleep once the following bug is fixed. When the + // payment is reported as settled by Carol, 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) + // Once we're done, close the channels and shut down the nodes created // throughout this test. - closeChannelAndAssert(t, net, net.Alice, chanPointAliceBob, false) - closeChannelAndAssert(t, net, net.Alice, chanPointAliceCarol, false) - closeChannelAndAssert(t, net, net.Bob, chanPointBobDave, false) - closeChannelAndAssert(t, net, carol, chanPointCarolDave, false) + ht.CloseChannel(alice, chanPointAliceBob) + ht.CloseChannel(alice, chanPointAliceCarol) + ht.CloseChannel(bob, chanPointBobDave) + ht.CloseChannel(carol, chanPointCarolDave) } // computeFee calculates the payment fee as specified in BOLT07. diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index ad207a3d7..31f8217a2 100644 --- a/lntest/itest/lnd_test_list_on_test.go +++ b/lntest/itest/lnd_test_list_on_test.go @@ -48,10 +48,6 @@ var allTestCases = []*testCase{ name: "switch offline delivery outgoing offline", test: testSwitchOfflineDeliveryOutgoingOffline, }, - { - name: "route fee cutoff", - test: testRouteFeeCutoff, - }, { name: "cpfp", test: testCPFP,