lntemp+itest: refactor testListPayments

lntemp/harness_assertion.go

@@ -1606,6 +1606,7 @@ func (h *HarnessTest) AssertNumPayments(hn *node.HarnessNode,
 
 	req := &lnrpc.ListPaymentsRequest{
 		IncludeIncomplete: true,
+		IndexOffset:       hn.State.Payment.LastIndexOffset,
 	}
 
 	var payments []*lnrpc.Payment

lntest/itest/list_on_test.go

@@ -179,4 +179,8 @@ var allTestCasesTemp = []*lntemp.TestCase{
 		Name:     "update node announcement rpc",
 		TestFunc: testUpdateNodeAnnouncement,
 	},
+	{
+		Name:     "list outgoing payments",
+		TestFunc: testListPayments,
+	},
 }

lntest/itest/lnd_payment_test.go

@@ -6,49 +6,29 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"fmt"
-	"reflect"
 	"time"
 
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/lnrpc"
 	"github.com/lightningnetwork/lnd/lnrpc/routerrpc"
+	"github.com/lightningnetwork/lnd/lntemp"
 	"github.com/lightningnetwork/lnd/lntest"
 	"github.com/lightningnetwork/lnd/lntest/wait"
 	"github.com/stretchr/testify/require"
 )
 
-func testListPayments(net *lntest.NetworkHarness, t *harnessTest) {
-	ctxb := context.Background()
-
-	// First start by deleting all payments that Alice knows of. This will
-	// allow us to execute the test with a clean state for Alice.
-	delPaymentsReq := &lnrpc.DeleteAllPaymentsRequest{}
-	ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
-	if _, err := net.Alice.DeleteAllPayments(ctxt, delPaymentsReq); err != nil {
-		t.Fatalf("unable to delete payments: %v", err)
-	}
+func testListPayments(ht *lntemp.HarnessTest) {
+	alice, bob := ht.Alice, ht.Bob
 
 	// Check that there are no payments before test.
-	reqInit := &lnrpc.ListPaymentsRequest{}
-	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
-	paymentsRespInit, err := net.Alice.ListPayments(ctxt, reqInit)
-	if err != nil {
-		t.Fatalf("error when obtaining Alice payments: %v", err)
-	}
-	if len(paymentsRespInit.Payments) != 0 {
-		t.Fatalf("incorrect number of payments, got %v, want %v",
-			len(paymentsRespInit.Payments), 0)
-	}
+	ht.AssertNumPayments(alice, 0)
 
 	// Open a channel with 100k satoshis between Alice and Bob with Alice
 	// being the sole funder of the channel.
 	chanAmt := btcutil.Amount(100000)
-	chanPoint := openChannelAndAssert(
-		t, net, net.Alice, net.Bob,
-		lntest.OpenChannelParams{
-			Amt: chanAmt,
-		},
+	chanPoint := ht.OpenChannel(
+		alice, bob, lntemp.OpenChannelParams{Amt: chanAmt},
 	)
 
 	// Now that the channel is open, create an invoice for Bob which
@@ -61,101 +41,58 @@ func testListPayments(net *lntest.NetworkHarness, t *harnessTest) {
 		RPreimage: preimage,
 		Value:     paymentAmt,
 	}
-	addInvoiceCtxt, _ := context.WithTimeout(ctxb, defaultTimeout)
-	invoiceResp, err := net.Bob.AddInvoice(addInvoiceCtxt, invoice)
-	if err != nil {
-		t.Fatalf("unable to add invoice: %v", err)
-	}
-
-	// Wait for Alice to recognize and advertise the new channel generated
-	// above.
-	if err = net.Alice.WaitForNetworkChannelOpen(chanPoint); err != nil {
-		t.Fatalf("alice didn't advertise channel before "+
-			"timeout: %v", err)
-	}
-	if err = net.Bob.WaitForNetworkChannelOpen(chanPoint); err != nil {
-		t.Fatalf("bob didn't advertise channel before "+
-			"timeout: %v", err)
-	}
+	invoiceResp := bob.RPC.AddInvoice(invoice)
 
 	// With the invoice for Bob added, send a payment towards Alice paying
 	// to the above generated invoice.
-	sendAndAssertSuccess(
-		t, net.Alice, &routerrpc.SendPaymentRequest{
-			PaymentRequest: invoiceResp.PaymentRequest,
-			TimeoutSeconds: 60,
-			FeeLimitSat:    1000000,
-		},
-	)
+	payReqs := []string{invoiceResp.PaymentRequest}
+	ht.CompletePaymentRequests(alice, payReqs)
 
 	// Grab Alice's list of payments, she should show the existence of
 	// exactly one payment.
-	req := &lnrpc.ListPaymentsRequest{}
-	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
-	paymentsResp, err := net.Alice.ListPayments(ctxt, req)
-	if err != nil {
-		t.Fatalf("error when obtaining Alice payments: %v", err)
-	}
-	if len(paymentsResp.Payments) != 1 {
-		t.Fatalf("incorrect number of payments, got %v, want %v",
-			len(paymentsResp.Payments), 1)
-	}
-	p := paymentsResp.Payments[0] // nolint:staticcheck
+	p := ht.AssertNumPayments(alice, 1)[0]
 	path := p.Htlcs[len(p.Htlcs)-1].Route.Hops
 
 	// Ensure that the stored path shows a direct payment to Bob with no
 	// other nodes in-between.
-	if len(path) != 1 || path[0].PubKey != net.Bob.PubKeyStr {
-		t.Fatalf("incorrect path")
-	}
+	require.Len(ht, path, 1, "wrong number of routes in path")
+	require.Equal(ht, bob.PubKeyStr, path[0].PubKey, "wrong pub key")
 
 	// The payment amount should also match our previous payment directly.
-	if p.Value != paymentAmt { // nolint:staticcheck
-		t.Fatalf("incorrect amount, got %v, want %v",
-			p.Value, paymentAmt) // nolint:staticcheck
-	}
+	require.EqualValues(ht, paymentAmt, p.ValueSat, "incorrect sat amount")
+	require.EqualValues(ht, paymentAmt*1000, p.ValueMsat,
+		"incorrect msat amount")
 
 	// The payment hash (or r-hash) should have been stored correctly.
 	correctRHash := hex.EncodeToString(invoiceResp.RHash)
-	if !reflect.DeepEqual(p.PaymentHash, correctRHash) {
-		t.Fatalf("incorrect RHash, got %v, want %v",
-			p.PaymentHash, correctRHash)
-	}
+	require.Equal(ht, correctRHash, p.PaymentHash, "incorrect RHash")
 
-	// As we made a single-hop direct payment, there should have been no fee
-	// applied.
-	if p.Fee != 0 { // nolint:staticcheck
-		t.Fatalf("incorrect Fee, got %v, want %v", p.Fee, 0) // nolint:staticcheck
-	}
+	// As we made a single-hop direct payment, there should have been no
+	// fee applied.
+	require.Zero(ht, p.FeeSat, "fee should be 0")
+	require.Zero(ht, p.FeeMsat, "fee should be 0")
 
 	// Finally, verify that the payment request returned by the rpc matches
 	// the invoice that we paid.
-	if p.PaymentRequest != invoiceResp.PaymentRequest {
-		t.Fatalf("incorrect payreq, got: %v, want: %v",
-			p.PaymentRequest, invoiceResp.PaymentRequest)
-	}
+	require.Equal(ht, invoiceResp.PaymentRequest, p.PaymentRequest,
+		"incorrect payreq")
 
 	// Delete all payments from Alice. DB should have no payments.
-	delReq := &lnrpc.DeleteAllPaymentsRequest{}
-	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
-	_, err = net.Alice.DeleteAllPayments(ctxt, delReq)
-	if err != nil {
-		t.Fatalf("Can't delete payments at the end: %v", err)
-	}
+	alice.RPC.DeleteAllPayments()
 
 	// Check that there are no payments after test.
-	listReq := &lnrpc.ListPaymentsRequest{}
-	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
-	paymentsResp, err = net.Alice.ListPayments(ctxt, listReq)
-	if err != nil {
-		t.Fatalf("error when obtaining Alice payments: %v", err)
-	}
-	if len(paymentsResp.Payments) != 0 {
-		t.Fatalf("incorrect number of payments, got %v, want %v",
-			len(paymentsResp.Payments), 0)
-	}
+	ht.AssertNumPayments(alice, 0)
 
-	closeChannelAndAssert(t, net, net.Alice, chanPoint, false)
+	// TODO(yy): remove the sleep once the following bug is fixed.
+	// When the invoice is reported settled, the commitment dance is not
+	// yet finished, which can cause an error when closing the channel,
+	// saying there's active HTLCs. We need to investigate this issue and
+	// reverse the order to, first finish the commitment dance, then report
+	// the invoice as settled.
+	time.Sleep(2 * time.Second)
+
+	// Close the channel.
+	defer ht.CloseChannel(alice, chanPoint)
 }
 
 // testPaymentFollowingChannelOpen tests that the channel transition from

lntest/itest/lnd_test_list_on_test.go

@@ -24,10 +24,6 @@ var allTestCases = []*testCase{
 		name: "single hop invoice",
 		test: testSingleHopInvoice,
 	},
-	{
-		name: "list outgoing payments",
-		test: testListPayments,
-	},
 	{
 		name: "multi-hop payments",
 		test: testMultiHopPayments,