lntemp+itest: refactor testSendToRouteMultiPath

This commit also introduces `mppTestScenario` which replaces the old
`mppmppTestContext` to reduce the total blocks mined during the test.
For each test case it will save 35 blocks.

lntemp/rpc/router.go

@@ -150,3 +150,15 @@ func (h *HarnessRPC) XImportMissionControlAssertErr(
 	_, err := h.Router.XImportMissionControl(ctxt, req)
 	require.Error(h, err, "expect an error from x import mission control")
 }
+
+// BuildRoute makes a RPC call to the node's RouterClient and asserts.
+func (h *HarnessRPC) BuildRoute(
+	req *routerrpc.BuildRouteRequest) *routerrpc.BuildRouteResponse {
+
+	ctxt, cancel := context.WithTimeout(h.runCtx, DefaultTimeout)
+	defer cancel()
+
+	resp, err := h.Router.BuildRoute(ctxt, req)
+	h.NoError(err, "BuildRoute")
+	return resp
+}

lntest/itest/list_on_test.go

@@ -385,4 +385,8 @@ var allTestCasesTemp = []*lntemp.TestCase{
 		Name:     "switch offline delivery outgoing offline",
 		TestFunc: testSwitchOfflineDeliveryOutgoingOffline,
 	},
+	{
+		Name:     "sendtoroute multi path payment",
+		TestFunc: testSendToRouteMultiPath,
+	},
 }

lntest/itest/lnd_mpp_test.go

@@ -1,7 +1,6 @@
 package itest
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"time"
@@ -11,17 +10,18 @@ import (
 	"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/lntypes"
 	"github.com/lightningnetwork/lnd/routing/route"
+	"github.com/stretchr/testify/require"
 )
 
 // testSendToRouteMultiPath tests that we are able to successfully route a
 // payment using multiple shards across different paths, by using SendToRoute.
-func testSendToRouteMultiPath(net *lntest.NetworkHarness, t *harnessTest) {
+func testSendToRouteMultiPath(ht *lntemp.HarnessTest) {
-	ctxb := context.Background()
+	mts := newMppTestScenario(ht)
-
-	ctx := newMppTestContext(t, net)
-	defer ctx.shutdownNodes()
 
 	// To ensure the payment goes through separate paths, we'll set a
 	// channel size that can only carry one shard at a time. We'll divide
@@ -39,55 +39,41 @@ func testSendToRouteMultiPath(net *lntest.NetworkHarness, t *harnessTest) {
 	//      \              /
 	//       \__ Dave ____/
 	//
-	ctx.openChannel(ctx.carol, ctx.bob, chanAmt)
+	req := &mppOpenChannelRequest{
-	ctx.openChannel(ctx.dave, ctx.bob, chanAmt)
+		// Since the channel Alice-> Carol will have to carry two
-	ctx.openChannel(ctx.alice, ctx.dave, chanAmt)
+		// shards, we make it larger.
-	ctx.openChannel(ctx.eve, ctx.bob, chanAmt)
+		amtAliceCarol: chanAmt + shardAmt,
-	ctx.openChannel(ctx.carol, ctx.eve, chanAmt)
+		amtAliceDave:  chanAmt,
-
+		amtCarolBob:   chanAmt,
-	// Since the channel Alice-> Carol will have to carry two
+		amtCarolEve:   chanAmt,
-	// shards, we make it larger.
+		amtDaveBob:    chanAmt,
-	ctx.openChannel(ctx.alice, ctx.carol, chanAmt+shardAmt)
+		amtEveBob:     chanAmt,
-
+	}
-	defer ctx.closeChannels()
+	mts.openChannels(req)
-
-	ctx.waitForChannels()
 
 	// Make Bob create an invoice for Alice to pay.
-	payReqs, rHashes, invoices, err := createPayReqs(
+	payReqs, rHashes, invoices := ht.CreatePayReqs(mts.bob, paymentAmt, 1)
-		ctx.bob, paymentAmt, 1,
-	)
-	if err != nil {
-		t.Fatalf("unable to create pay reqs: %v", err)
-	}
 
 	rHash := rHashes[0]
 	payReq := payReqs[0]
 
-	ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
+	decodeResp := mts.bob.RPC.DecodePayReq(payReq)
-	decodeResp, err := ctx.bob.DecodePayReq(
-		ctxt, &lnrpc.PayReqString{PayReq: payReq},
-	)
-	if err != nil {
-		t.Fatalf("decode pay req: %v", err)
-	}
-
 	payAddr := decodeResp.PaymentAddr
 
+	// Subscribe the invoice.
+	stream := mts.bob.RPC.SubscribeSingleInvoice(rHash)
+
 	// We'll send shards along three routes from Alice.
-	sendRoutes := [][]*lntest.HarnessNode{
+	sendRoutes := [][]*node.HarnessNode{
-		{ctx.carol, ctx.bob},
+		{mts.carol, mts.bob},
-		{ctx.dave, ctx.bob},
+		{mts.dave, mts.bob},
-		{ctx.carol, ctx.eve, ctx.bob},
+		{mts.carol, mts.eve, mts.bob},
 	}
 
 	responses := make(chan *lnrpc.HTLCAttempt, len(sendRoutes))
 	for _, hops := range sendRoutes {
 		// Build a route for the specified hops.
-		r, err := ctx.buildRoute(ctxb, shardAmt, ctx.alice, hops)
+		r := mts.buildRoute(shardAmt, mts.alice, hops)
-		if err != nil {
-			t.Fatalf("unable to build route: %v", err)
-		}
 
 		// Set the MPP records to indicate this is a payment shard.
 		hop := r.Hops[len(r.Hops)-1]
@@ -103,62 +89,44 @@ func testSendToRouteMultiPath(net *lntest.NetworkHarness, t *harnessTest) {
 			Route:       r,
 		}
 
-		// We'll send all shards in their own goroutine, since SendToRoute will
+		// We'll send all shards in their own goroutine, since
-		// block as long as the payment is in flight.
+		// SendToRoute will block as long as the payment is in flight.
 		go func() {
-			ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
+			resp := mts.alice.RPC.SendToRouteV2(sendReq)
-			resp, err := ctx.alice.RouterClient.SendToRouteV2(ctxt, sendReq)
-			if err != nil {
-				t.Fatalf("unable to send payment: %v", err)
-			}
-
 			responses <- resp
 		}()
 	}
 
 	// Wait for all responses to be back, and check that they all
 	// succeeded.
+	timer := time.After(defaultTimeout)
 	for range sendRoutes {
 		var resp *lnrpc.HTLCAttempt
 		select {
 		case resp = <-responses:
-		case <-time.After(defaultTimeout):
+		case <-timer:
-			t.Fatalf("response not received")
+			require.Fail(ht, "response not received")
 		}
 
-		if resp.Failure != nil {
+		require.Nil(ht, resp.Failure, "received payment failure")
-			t.Fatalf("received payment failure : %v", resp.Failure)
-		}
 
 		// All shards should come back with the preimage.
-		if !bytes.Equal(resp.Preimage, invoices[0].RPreimage) {
+		require.Equal(ht, resp.Preimage, invoices[0].RPreimage,
-			t.Fatalf("preimage doesn't match")
+			"preimage doesn't match")
-		}
 	}
 
 	// assertNumHtlcs is a helper that checks the node's latest payment,
 	// and asserts it was split into num shards.
-	assertNumHtlcs := func(node *lntest.HarnessNode, num int) {
+	assertNumHtlcs := func(hn *node.HarnessNode, num int) {
-		req := &lnrpc.ListPaymentsRequest{
+		var preimage lntypes.Preimage
-			IncludeIncomplete: true,
+		copy(preimage[:], invoices[0].RPreimage)
-		}
-		ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
-		paymentsResp, err := node.ListPayments(ctxt, req)
-		if err != nil {
-			t.Fatalf("error when obtaining payments: %v",
-				err)
-		}
 
-		payments := paymentsResp.Payments
+		payment := ht.AssertPaymentStatus(
-		if len(payments) == 0 {
+			hn, preimage, lnrpc.Payment_SUCCEEDED,
-			t.Fatalf("no payments found")
+		)
-		}
 
-		payment := payments[len(payments)-1]
 		htlcs := payment.Htlcs
-		if len(htlcs) == 0 {
+		require.NotEmpty(ht, htlcs, "no htlcs")
-			t.Fatalf("no htlcs")
-		}
 
 		succeeded := 0
 		for _, htlc := range htlcs {
@@ -166,78 +134,32 @@ func testSendToRouteMultiPath(net *lntest.NetworkHarness, t *harnessTest) {
 				succeeded++
 			}
 		}
-
+		require.Equal(ht, num, succeeded, "HTLCs not matched")
-		if succeeded != num {
-			t.Fatalf("expected %v succussful HTLCs, got %v", num,
-				succeeded)
-		}
 	}
 
 	// assertSettledInvoice checks that the invoice for the given payment
 	// hash is settled, and has been paid using num HTLCs.
-	assertSettledInvoice := func(node *lntest.HarnessNode, rhash []byte,
+	assertSettledInvoice := func(rhash []byte, num int) {
-		num int) {
+		var payHash lntypes.Hash
+		copy(payHash[:], rhash)
+		inv := ht.AssertInvoiceState(stream, lnrpc.Invoice_SETTLED)
 
-		found := false
+		// Assert that the amount paid to the invoice is correct.
-		offset := uint64(0)
+		require.EqualValues(ht, paymentAmt, inv.AmtPaidSat,
-		for !found {
+			"incorrect payment amt")
-			ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
-			invoicesResp, err := node.ListInvoices(
-				ctxt, &lnrpc.ListInvoiceRequest{
-					IndexOffset: offset,
-				},
-			)
-			if err != nil {
-				t.Fatalf("error when obtaining payments: %v",
-					err)
-			}
 
-			if len(invoicesResp.Invoices) == 0 {
+		require.Len(ht, inv.Htlcs, num, "wrong num of HTLCs")
-				break
-			}
-
-			for _, inv := range invoicesResp.Invoices {
-				if !bytes.Equal(inv.RHash, rhash) {
-					continue
-				}
-
-				// Assert that the amount paid to the invoice is
-				// correct.
-				if inv.AmtPaidSat != int64(paymentAmt) {
-					t.Fatalf("incorrect payment amt for "+
-						"invoicewant: %d, got %d",
-						paymentAmt, inv.AmtPaidSat)
-				}
-
-				if inv.State != lnrpc.Invoice_SETTLED {
-					t.Fatalf("Invoice not settled: %v",
-						inv.State)
-				}
-
-				if len(inv.Htlcs) != num {
-					t.Fatalf("expected invoice to be "+
-						"settled with %v HTLCs, had %v",
-						num, len(inv.Htlcs))
-				}
-
-				found = true
-				break
-			}
-
-			offset = invoicesResp.LastIndexOffset
-		}
-
-		if !found {
-			t.Fatalf("invoice not found")
-		}
 	}
 
 	// Finally check that the payment shows up with three settled HTLCs in
 	// Alice's list of payments...
-	assertNumHtlcs(ctx.alice, 3)
+	assertNumHtlcs(mts.alice, 3)
 
 	// ...and in Bob's list of paid invoices.
-	assertSettledInvoice(ctx.bob, rHash, 3)
+	assertSettledInvoice(rHash, 3)
+
+	// Finally, close all channels.
+	mts.closeChannels()
 }
 
 type mppTestContext struct {
@@ -371,3 +293,216 @@ func (c *mppTestContext) buildRoute(ctxb context.Context, amt btcutil.Amount,
 
 	return routeResp.Route, nil
 }
+
+// mppTestScenario defines a test scenario used for testing MPP-related tests.
+// It has two standby nodes, alice and bob, and three new nodes, carol, dave,
+// and eve.
+type mppTestScenario struct {
+	ht *lntemp.HarnessTest
+
+	alice, bob, carol, dave, eve *node.HarnessNode
+	nodes                        []*node.HarnessNode
+
+	// Keep a list of all our active channels.
+	channelPoints []*lnrpc.ChannelPoint
+}
+
+// newMppTestScenario initializes a new mpp test scenario with five funded
+// nodes and connects them to have the following topology,
+//
+//	            _ Eve _
+//	           /       \
+//	Alice -- Carol ---- Bob
+//	    \              /
+//	     \__ Dave ____/
+func newMppTestScenario(ht *lntemp.HarnessTest) *mppTestScenario {
+	alice, bob := ht.Alice, ht.Bob
+	ht.RestartNodeWithExtraArgs(bob, []string{
+		"--maxpendingchannels=2",
+		"--accept-amp",
+	})
+
+	// Create a five-node context consisting of Alice, Bob and three new
+	// nodes.
+	carol := ht.NewNode("carol", []string{"--maxpendingchannels=2"})
+	dave := ht.NewNode("dave", nil)
+	eve := ht.NewNode("eve", nil)
+
+	// Connect nodes to ensure propagation of channels.
+	ht.EnsureConnected(alice, carol)
+	ht.EnsureConnected(alice, dave)
+	ht.EnsureConnected(carol, bob)
+	ht.EnsureConnected(carol, eve)
+	ht.EnsureConnected(dave, bob)
+	ht.EnsureConnected(eve, bob)
+
+	// Send coins to the nodes and mine 1 blocks to confirm them.
+	for i := 0; i < 2; i++ {
+		ht.FundCoinsUnconfirmed(btcutil.SatoshiPerBitcoin, carol)
+		ht.FundCoinsUnconfirmed(btcutil.SatoshiPerBitcoin, dave)
+		ht.FundCoinsUnconfirmed(btcutil.SatoshiPerBitcoin, eve)
+		ht.MineBlocks(1)
+	}
+
+	mts := &mppTestScenario{
+		ht:    ht,
+		alice: alice,
+		bob:   bob,
+		carol: carol,
+		dave:  dave,
+		eve:   eve,
+		nodes: []*node.HarnessNode{alice, bob, carol, dave, eve},
+	}
+
+	return mts
+}
+
+// mppOpenChannelRequest defines the amounts used for each channel opening.
+type mppOpenChannelRequest struct {
+	// Channel Alice=>Carol.
+	amtAliceCarol btcutil.Amount
+
+	// Channel Alice=>Dave.
+	amtAliceDave btcutil.Amount
+
+	// Channel Carol=>Bob.
+	amtCarolBob btcutil.Amount
+
+	// Channel Carol=>Eve.
+	amtCarolEve btcutil.Amount
+
+	// Channel Dave=>Bob.
+	amtDaveBob btcutil.Amount
+
+	// Channel Eve=>Bob.
+	amtEveBob btcutil.Amount
+}
+
+// openChannels is a helper to open channels that sets up a network topology
+// with three different paths Alice <-> Bob as following,
+//
+//		 _ Eve _
+//		/       \
+//	 Alice -- Carol ---- Bob
+//		\              /
+//		 \__ Dave ____/
+//
+// NOTE: all the channels are open together to save blocks mined.
+func (m *mppTestScenario) openChannels(r *mppOpenChannelRequest) {
+	reqs := []*lntemp.OpenChannelRequest{
+		{
+			Local:  m.alice,
+			Remote: m.carol,
+			Param:  lntemp.OpenChannelParams{Amt: r.amtAliceCarol},
+		},
+		{
+			Local:  m.alice,
+			Remote: m.dave,
+			Param:  lntemp.OpenChannelParams{Amt: r.amtAliceDave},
+		},
+		{
+			Local:  m.carol,
+			Remote: m.bob,
+			Param:  lntemp.OpenChannelParams{Amt: r.amtCarolBob},
+		},
+		{
+			Local:  m.carol,
+			Remote: m.eve,
+			Param:  lntemp.OpenChannelParams{Amt: r.amtCarolEve},
+		},
+		{
+			Local:  m.dave,
+			Remote: m.bob,
+			Param:  lntemp.OpenChannelParams{Amt: r.amtDaveBob},
+		},
+		{
+			Local:  m.eve,
+			Remote: m.bob,
+			Param:  lntemp.OpenChannelParams{Amt: r.amtEveBob},
+		},
+	}
+
+	m.channelPoints = m.ht.OpenMultiChannelsAsync(reqs)
+
+	// Make sure every node has heard every channel.
+	for _, hn := range m.nodes {
+		for _, cp := range m.channelPoints {
+			m.ht.AssertTopologyChannelOpen(hn, cp)
+		}
+	}
+}
+
+// closeChannels closes all the open channels from `openChannels`.
+func (m *mppTestScenario) closeChannels() {
+	if m.ht.Failed() {
+		m.ht.Log("Skipped closing channels for failed test")
+		return
+	}
+
+	// Close all channels without mining the closing transactions.
+	m.ht.CloseChannelAssertPending(m.alice, m.channelPoints[0], false)
+	m.ht.CloseChannelAssertPending(m.alice, m.channelPoints[1], false)
+	m.ht.CloseChannelAssertPending(m.carol, m.channelPoints[2], false)
+	m.ht.CloseChannelAssertPending(m.carol, m.channelPoints[3], false)
+	m.ht.CloseChannelAssertPending(m.dave, m.channelPoints[4], false)
+	m.ht.CloseChannelAssertPending(m.eve, m.channelPoints[5], false)
+
+	// Now mine a block to include all the closing transactions.
+	m.ht.MineBlocks(1)
+
+	// Assert that the channels are closed.
+	for _, hn := range m.nodes {
+		m.ht.AssertNumWaitingClose(hn, 0)
+	}
+}
+
+// Helper function for Alice to build a route from pubkeys.
+func (m *mppTestScenario) buildRoute(amt btcutil.Amount,
+	sender *node.HarnessNode, hops []*node.HarnessNode) *lnrpc.Route {
+
+	rpcHops := make([][]byte, 0, len(hops))
+	for _, hop := range hops {
+		k := hop.PubKeyStr
+		pubkey, err := route.NewVertexFromStr(k)
+		require.NoErrorf(m.ht, err, "error parsing %v: %v", k, err)
+		rpcHops = append(rpcHops, pubkey[:])
+	}
+
+	req := &routerrpc.BuildRouteRequest{
+		AmtMsat:        int64(amt * 1000),
+		FinalCltvDelta: chainreg.DefaultBitcoinTimeLockDelta,
+		HopPubkeys:     rpcHops,
+	}
+	routeResp := sender.RPC.BuildRoute(req)
+
+	return routeResp.Route
+}
+
+// updatePolicy updates a Dave's global channel policy and returns the expected
+// policy for further check.
+func (m *mppTestScenario) updateDaveGlobalPolicy() *lnrpc.RoutingPolicy {
+	const (
+		baseFeeMsat = 500_000
+		feeRate     = 0.001
+		maxHtlcMsat = 133_650_000
+	)
+
+	expectedPolicy := &lnrpc.RoutingPolicy{
+		FeeBaseMsat:      baseFeeMsat,
+		FeeRateMilliMsat: feeRate * testFeeBase,
+		TimeLockDelta:    40,
+		MinHtlc:          1000, // default value
+		MaxHtlcMsat:      maxHtlcMsat,
+	}
+
+	updateFeeReq := &lnrpc.PolicyUpdateRequest{
+		BaseFeeMsat:   baseFeeMsat,
+		FeeRate:       feeRate,
+		TimeLockDelta: 40,
+		Scope:         &lnrpc.PolicyUpdateRequest_Global{Global: true},
+		MaxHtlcMsat:   maxHtlcMsat,
+	}
+	m.dave.RPC.UpdateChannelPolicy(updateFeeReq)
+
+	return expectedPolicy
+}

lntest/itest/lnd_test_list_on_test.go

@@ -40,10 +40,6 @@ var allTestCases = []*testCase{
 		name: "sign psbt",
 		test: testSignPsbt,
 	},
-	{
-		name: "sendtoroute multi path payment",
-		test: testSendToRouteMultiPath,
-	},
 	{
 		name: "sendtoroute amp",
 		test: testSendToRouteAMP,