lntemp+itest: refactor testRouteFeeCutoff

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 {

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,
+	},
 }

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) {
+func testRouteFeeCutoff(ht *lntemp.HarnessTest) {
-	ctxb := context.Background()
-
 	// 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(
+	alice, bob := ht.Alice, ht.Bob
-		t, net, net.Alice, net.Bob,
+	chanPointAliceBob := ht.OpenChannel(
-		lntest.OpenChannelParams{
+		alice, bob, lntemp.OpenChannelParams{Amt: chanAmt},
-			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)
+	carol := ht.NewNode("Carol", nil)
-	defer shutdownAndAssert(net, t, carol)
+	ht.ConnectNodes(carol, alice)
+	ht.FundCoins(btcutil.SatoshiPerBitcoin, carol)
 
-	net.ConnectNodes(t.t, carol, net.Alice)
+	chanPointAliceCarol := ht.OpenChannel(
-	net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, carol)
+		alice, carol, lntemp.OpenChannelParams{Amt: chanAmt},
-
-	chanPointAliceCarol := openChannelAndAssert(
-		t, net, net.Alice, carol,
-		lntest.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)
+	dave := ht.NewNode("Dave", nil)
-	defer shutdownAndAssert(net, t, dave)
+	ht.ConnectNodes(dave, bob)
-
+	chanPointBobDave := ht.OpenChannel(
-	net.ConnectNodes(t.t, dave, net.Bob)
+		bob, dave, lntemp.OpenChannelParams{Amt: chanAmt},
-	chanPointBobDave := openChannelAndAssert(
-		t, net, net.Bob, dave,
-		lntest.OpenChannelParams{
-			Amt: chanAmt,
-		},
 	)
 
 	// Open a channel between Carol and Dave.
-	net.ConnectNodes(t.t, carol, dave)
+	ht.ConnectNodes(carol, dave)
-	chanPointCarolDave := openChannelAndAssert(
+	chanPointCarolDave := ht.OpenChannel(
-		t, net, carol, dave,
+		carol, dave, lntemp.OpenChannelParams{Amt: chanAmt},
-		lntest.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}
+	nodes := []*node.HarnessNode{alice, bob, carol, dave}
-	nodeNames := []string{"alice", "bob", "carol", "dave"}
 	networkChans := []*lnrpc.ChannelPoint{
 		chanPointAliceBob, chanPointAliceCarol, chanPointBobDave,
 		chanPointCarolDave,
 	}
 	for _, chanPoint := range networkChans {
-		for i, node := range nodes {
+		for _, node := range nodes {
-			txid, err := lnrpc.GetChanPointFundingTxid(chanPoint)
+			ht.AssertTopologyChannelOpen(node, 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)
-			}
 		}
 	}
 
@@ -1184,56 +1149,35 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) {
 			ChanPoint: chanPointCarolDave,
 		},
 	}
-	ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
+	carol.RPC.UpdateChannelPolicy(updateFeeReq)
-	if _, err := carol.UpdateChannelPolicy(ctxt, updateFeeReq); err != nil {
-		t.Fatalf("unable to update chan policy: %v", err)
-	}
 
 	// Wait for Alice to receive the channel update from Carol.
-	assertChannelPolicyUpdate(
+	ht.AssertChannelPolicyUpdate(
-		t.t, net.Alice, carol.PubKeyStr,
+		alice, carol, expectedPolicy, chanPointCarolDave, false,
-		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{}
+	channel := ht.QueryChannelByChanPoint(bob, chanPointAliceBob)
-	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+	aliceBobChanID := channel.ChanId
-	listResp, err := net.Bob.ListChannels(ctxt, listReq)
+	require.NotZero(ht, aliceBobChanID,
-	if err != nil {
+		"channel between alice and bob not found")
-		t.Fatalf("unable to retrieve bob's channels: %v", err)
-	}
 
-	var aliceBobChanID, bobDaveChanID uint64
+	channel = ht.QueryChannelByChanPoint(bob, chanPointBobDave)
-	for _, channel := range listResp.Channels {
+	bobDaveChanID := channel.ChanId
-		switch channel.RemotePubkey {
+	require.NotZero(ht, bobDaveChanID,
-		case net.Alice.PubKeyStr:
+		"channel between bob and dave not found")
-			aliceBobChanID = channel.ChanId
-		case dave.PubKeyStr:
-			bobDaveChanID = channel.ChanId
-		}
-	}
 
-	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 {
+		require.Len(ht, route.Hops, 2, "expected two hops")
-			t.Fatalf("expected two hops, got %d", len(route.Hops))
-		}
 
 		for i, hop := range route.Hops {
-			if hop.ChanId != hopChanIDs[i] {
+			require.Equal(ht, hopChanIDs[i], hop.ChanId,
-				t.Fatalf("expected chan id %d, got %d",
+				"hop chan id not match")
-					hopChanIDs[i], hop.ChanId)
-			}
 		}
 	}
 
@@ -1252,20 +1196,12 @@ func testRouteFeeCutoff(net *lntest.NetworkHarness, t *harnessTest) {
 			Amt:      paymentAmt,
 			FeeLimit: feeLimit,
 		}
-		ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+		routesResp := alice.RPC.QueryRoutes(queryRoutesReq)
-		routesResp, err := net.Alice.QueryRoutes(ctxt, queryRoutesReq)
-		if err != nil {
-			t.Fatalf("unable to get routes: %v", err)
-		}
 
 		checkRoute(routesResp.Routes[0])
 
 		invoice := &lnrpc.Invoice{Value: paymentAmt}
-		ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+		invoiceResp := dave.RPC.AddInvoice(invoice)
-		invoiceResp, err := dave.AddInvoice(ctxt, invoice)
-		if err != nil {
-			t.Fatalf("unable to create invoice: %v", err)
-		}
 
 		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)
+	ht.CloseChannel(alice, chanPointAliceBob)
-	closeChannelAndAssert(t, net, net.Alice, chanPointAliceCarol, false)
+	ht.CloseChannel(alice, chanPointAliceCarol)
-	closeChannelAndAssert(t, net, net.Bob, chanPointBobDave, false)
+	ht.CloseChannel(bob, chanPointBobDave)
-	closeChannelAndAssert(t, net, carol, chanPointCarolDave, false)
+	ht.CloseChannel(carol, chanPointCarolDave)
 }
 
 // computeFee calculates the payment fee as specified in BOLT07.

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,