Merge pull request #8177 from yyforyongyu/payment-status

routerrpc: optionally return the new payment status

docs/release-notes/release-notes-0.18.0.md

@@ -63,6 +63,11 @@
 * A new config value,
   [http-header-timeout](https://github.com/lightningnetwork/lnd/pull/7715), is added so users can specify the amount of time the http server will wait for a request to complete before closing the connection. The default value is 5 seconds.
 
+* [`routerrpc.usestatusinitiated` is
+  introduced](https://github.com/lightningnetwork/lnd/pull/8177) to signal that
+  the new payment status `Payment_INITIATED` should be used for payment-related
+  RPCs. It's recommended to use it to provide granular controls over payments.
+
 ## RPC Additions
 
 * [Deprecated](https://github.com/lightningnetwork/lnd/pull/7175)

itest/list_on_test.go

@@ -490,6 +490,10 @@ var allTestCases = []*lntest.TestCase{
 		Name:     "trackpayments",
 		TestFunc: testTrackPayments,
 	},
+	{
+		Name:     "trackpayments compatible",
+		TestFunc: testTrackPaymentsCompatible,
+	},
 	{
 		Name:     "open channel fee policy",
 		TestFunc: testOpenChannelUpdateFeePolicy,

itest/lnd_max_htlcs_test.go

@@ -20,6 +20,17 @@ func testMaxHtlcPathfind(ht *lntest.HarnessTest) {
 	maxHtlcs := 5
 
 	alice, bob := ht.Alice, ht.Bob
+
+	// Restart nodes with the new flag so they understand the new payment
+	// status.
+	ht.RestartNodeWithExtraArgs(alice, []string{
+		"--routerrpc.usestatusinitiated",
+	})
+	ht.RestartNodeWithExtraArgs(bob, []string{
+		"--routerrpc.usestatusinitiated",
+	})
+
+	ht.EnsureConnected(alice, bob)
 	chanPoint := ht.OpenChannel(
 		alice, bob, lntest.OpenChannelParams{
 			Amt:            1000000,

itest/lnd_trackpayments_test.go

@@ -14,8 +14,16 @@ import (
 // testTrackPayments tests whether a client that calls the TrackPayments api
 // receives payment updates.
 func testTrackPayments(ht *lntest.HarnessTest) {
-	// Open a channel between alice and bob.
 	alice, bob := ht.Alice, ht.Bob
+
+	// Restart Alice with the new flag so she understands the new payment
+	// status.
+	ht.RestartNodeWithExtraArgs(alice, []string{
+		"--routerrpc.usestatusinitiated",
+	})
+
+	// Open a channel between alice and bob.
+	ht.EnsureConnected(alice, bob)
 	channel := ht.OpenChannel(
 		alice, bob, lntest.OpenChannelParams{
 			Amt: btcutil.Amount(300000),
@@ -47,36 +55,115 @@ func testTrackPayments(ht *lntest.HarnessTest) {
 
 	// Make sure the payment doesn't error due to invalid parameters or so.
 	_, err := paymentClient.Recv()
-	require.NoError(ht, err, "unable to get payment update.")
+	require.NoError(ht, err, "unable to get payment update")
 
-	// Assert the first payment update is an inflight update.
+	// Assert the first payment update is an initiated update.
 	update1, err := tracker.Recv()
-	require.NoError(ht, err, "unable to receive payment update 1.")
+	require.NoError(ht, err, "unable to receive payment update 1")
 
-	require.Equal(
-		ht, lnrpc.PaymentFailureReason_FAILURE_REASON_NONE,
-		update1.FailureReason,
-	)
-	require.Equal(ht, lnrpc.Payment_IN_FLIGHT, update1.Status)
+	require.Equal(ht, lnrpc.Payment_INITIATED, update1.Status)
+	require.Equal(ht, lnrpc.PaymentFailureReason_FAILURE_REASON_NONE,
+		update1.FailureReason)
 	require.Equal(ht, invoice.PaymentRequest, update1.PaymentRequest)
 	require.Equal(ht, amountMsat, update1.ValueMsat)
 
-	// Assert the second payment update is a payment success update.
+	// Assert the first payment update is an inflight update.
 	update2, err := tracker.Recv()
-	require.NoError(ht, err, "unable to receive payment update 2.")
+	require.NoError(ht, err, "unable to receive payment update 2")
 
-	require.Equal(
-		ht, lnrpc.PaymentFailureReason_FAILURE_REASON_NONE,
-		update2.FailureReason,
-	)
-	require.Equal(ht, lnrpc.Payment_SUCCEEDED, update2.Status)
+	require.Equal(ht, lnrpc.Payment_IN_FLIGHT, update2.Status)
+	require.Equal(ht, lnrpc.PaymentFailureReason_FAILURE_REASON_NONE,
+		update2.FailureReason)
 	require.Equal(ht, invoice.PaymentRequest, update2.PaymentRequest)
 	require.Equal(ht, amountMsat, update2.ValueMsat)
-	require.Equal(
-		ht, hex.EncodeToString(invoice.RPreimage),
-		update2.PaymentPreimage,
+
+	// Assert the third payment update is a payment success update.
+	update3, err := tracker.Recv()
+	require.NoError(ht, err, "unable to receive payment update 3")
+
+	require.Equal(ht, lnrpc.Payment_SUCCEEDED, update3.Status)
+	require.Equal(ht, lnrpc.PaymentFailureReason_FAILURE_REASON_NONE,
+		update3.FailureReason)
+	require.Equal(ht, invoice.PaymentRequest, update3.PaymentRequest)
+	require.Equal(ht, amountMsat, update3.ValueMsat)
+	require.Equal(ht, hex.EncodeToString(invoice.RPreimage),
+		update3.PaymentPreimage)
+
+	// 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)
+
+	ht.CloseChannel(alice, channel)
+}
+
+// testTrackPaymentsCompatible checks that when `routerrpc.usestatusinitiated`
+// is not set, the new Payment_INITIATED is replaced with Payment_IN_FLIGHT.
+func testTrackPaymentsCompatible(ht *lntest.HarnessTest) {
+	// Open a channel between alice and bob.
+	alice, bob := ht.Alice, ht.Bob
+	channel := ht.OpenChannel(
+		alice, bob, lntest.OpenChannelParams{
+			Amt: btcutil.Amount(300000),
+		},
 	)
 
+	// Call the TrackPayments api to listen for payment updates.
+	req := &routerrpc.TrackPaymentsRequest{
+		NoInflightUpdates: false,
+	}
+	tracker := alice.RPC.TrackPayments(req)
+
+	// Create an invoice from bob.
+	var amountMsat int64 = 1000
+	invoiceResp := bob.RPC.AddInvoice(
+		&lnrpc.Invoice{
+			ValueMsat: amountMsat,
+		},
+	)
+	invoice := bob.RPC.LookupInvoice(invoiceResp.RHash)
+
+	// Send payment from alice to bob.
+	paymentClient := alice.RPC.SendPayment(
+		&routerrpc.SendPaymentRequest{
+			PaymentRequest: invoice.PaymentRequest,
+			TimeoutSeconds: 60,
+		},
+	)
+
+	// Assert the first track payment update is an inflight update.
+	update1, err := tracker.Recv()
+	require.NoError(ht, err, "unable to receive payment update 1")
+	require.Equal(ht, lnrpc.Payment_IN_FLIGHT, update1.Status)
+
+	// Assert the first track payment update is an inflight update.
+	update2, err := tracker.Recv()
+	require.NoError(ht, err, "unable to receive payment update 2")
+	require.Equal(ht, lnrpc.Payment_IN_FLIGHT, update2.Status)
+
+	// Assert the third track payment update is a payment success update.
+	update3, err := tracker.Recv()
+	require.NoError(ht, err, "unable to receive payment update 3")
+	require.Equal(ht, lnrpc.Payment_SUCCEEDED, update3.Status)
+
+	// Assert the first payment client update is an inflight update.
+	payment1, err := paymentClient.Recv()
+	require.NoError(ht, err, "unable to get payment update")
+	require.Equal(ht, lnrpc.Payment_IN_FLIGHT, payment1.Status)
+
+	// Assert the second payment client update is an inflight update.
+	payment2, err := paymentClient.Recv()
+	require.NoError(ht, err, "unable to get payment update")
+	require.Equal(ht, lnrpc.Payment_IN_FLIGHT, payment2.Status)
+
+	// Assert the third payment client update is a success update.
+	payment3, err := paymentClient.Recv()
+	require.NoError(ht, err, "unable to get payment update")
+	require.Equal(ht, lnrpc.Payment_SUCCEEDED, payment3.Status)
+
 	// 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,

lnrpc/routerrpc/config.go

@@ -10,9 +10,18 @@ import (
 // The fields with struct tags are meant to be parsed as normal configuration
 // options, while if able to be populated, the latter fields MUST also be
 // specified.
+//
+//nolint:lll
 type Config struct {
 	RoutingConfig
 
+	// UseStatusInitiated is a boolean that indicates whether the router
+	// should use the new status code `Payment_INITIATED`.
+	//
+	// TODO(yy): remove this config after the new status code is fully
+	// deployed to the network(v0.20.0).
+	UseStatusInitiated bool `long:"usestatusinitiated" description:"If true, the router will send Payment_INITIATED for new payments, otherwise Payment_In_FLIGHT will be sent for compatibility concerns."`
+
 	// RouterMacPath is the path for the router macaroon. If unspecified
 	// then we assume that the macaroon will be found under the network
 	// directory, named DefaultRouterMacFilename.