[v0.21.x-branch] Backport #10814: Remove deprecated Send* / TrackPayment RPCs and outgoing_chan_id field

docs/release-notes/release-notes-0.21.0.md

@@ -252,6 +252,32 @@
   `lncli getdebuginfo` and `lncli encryptdebugpackage` commands similarly
   require the `--include_log` flag to include logs in the output.
 
+* [Removed the deprecated payment RPCs and `outgoing_chan_id`
+  field](https://github.com/lightningnetwork/lnd/pull/10814) that were
+  [announced for removal in 0.21](https://github.com/lightningnetwork/lnd/blob/master/docs/release-notes/release-notes-0.20.0.md#deprecations)
+  via the 0.20 release notes. Callers must migrate to the V2 equivalents:
+
+  | Removed RPC | Replacement |
+  |-------------|-------------|
+  | `lnrpc.SendPayment` (streaming) | `routerrpc.SendPaymentV2` |
+  | `lnrpc.SendPaymentSync` | `routerrpc.SendPaymentV2` |
+  | `lnrpc.SendToRoute` (streaming) | `routerrpc.SendToRouteV2` |
+  | `lnrpc.SendToRouteSync` | `routerrpc.SendToRouteV2` |
+  | `routerrpc.SendPayment` (streaming) | `routerrpc.SendPaymentV2` |
+  | `routerrpc.SendToRoute` | `routerrpc.SendToRouteV2` |
+  | `routerrpc.TrackPayment` (streaming) | `routerrpc.TrackPaymentV2` |
+
+  This also removes the corresponding REST routes
+  `POST /v1/channels/transaction-stream`, `POST /v1/channels/transactions`,
+  and `POST /v1/channels/transactions/route`. The orphan
+  `routerrpc.SendToRouteResponse` message (only used by the removed
+  `routerrpc.SendToRoute` RPC) is also dropped.
+
+  In addition, the deprecated `outgoing_chan_id` field is removed from
+  `lnrpc.QueryRoutesRequest` and `routerrpc.SendPaymentRequest` (proto tags
+  14 and 8 respectively, now reserved). Callers must use the multi-channel
+  `outgoing_chan_ids` field introduced in 0.20.
+
 ## Performance Improvements
 
 * Let the [channel graph cache be populated
@@ -331,7 +357,7 @@
 
 ### ⚠️ **Warning:** Deprecated fields in `lnrpc.Hop` will be removed in release version **0.22**
 
-  The following deprecated fields in the [`lnrpc.Hop`](https://lightning.engineering/api-docs/api/lnd/lightning/send-to-route-sync/#lnrpchop)
+  The following deprecated fields in the [`lnrpc.Hop`](https://lightning.engineering/api-docs/api/lnd/lightning/query-routes/#lnrpchop)
   message will be removed:
 
   | Field | Deprecated Since | Replacement |

docs/rest/websockets.md

@@ -130,8 +130,8 @@ ws.on('open', function() {
     // This empty message will be ignored by the channel acceptor though, this
     // is just for telling the grpc-gateway library that it can forward the
     // request to the gRPC interface now. If this were an RPC where the client
-    // always sends the first message (for example the streaming payment RPC
-    // /v1/channels/transaction-stream), we'd simply send the first "real"
+    // always sends the first message (for example the HTLC interceptor RPC
+    // /v2/router/htlcinterceptor), we'd simply send the first "real"
     // message here when needed.
     ws.send('{}');
 });

itest/lnd_channel_policy_test.go

@@ -170,29 +170,28 @@ func testUpdateChannelPolicy(ht *lntest.HarnessTest) {
 	routes.Routes[0].Hops[1].AmtToForward = amtSat
 	routes.Routes[0].Hops[1].AmtToForwardMsat = amtMSat
 
-	// Send the payment with the modified value.
-	alicePayStream := alice.RPC.SendToRoute()
-
-	sendReq := &lnrpc.SendToRouteRequest{
+	// Send the payment with the modified value and expect a failure because
+	// the amount is below the minimum HTLC size.
+	sendReq := &routerrpc.SendToRouteRequest{
 		PaymentHash: resp.RHash,
 		Route:       routes.Routes[0],
 	}
-	err := alicePayStream.Send(sendReq)
-	require.NoError(ht, err, "unable to send payment")
-
-	// We expect this payment to fail, and that the min_htlc value is
-	// communicated back to us, since the attempted HTLC value was too low.
-	sendResp, err := ht.ReceiveSendToRouteUpdate(alicePayStream)
-	require.NoError(ht, err, "unable to receive payment stream")
-
-	// Expected as part of the error message.
-	substrs := []string{
-		"AmountBelowMinimum",
-		"HtlcMinimumMsat: (lnwire.MilliSatoshi) 5000 mSAT",
-	}
-	for _, s := range substrs {
-		require.Contains(ht, sendResp.PaymentError, s)
-	}
+	sendResp := alice.RPC.SendToRouteV2(sendReq)
+	require.NotNil(ht, sendResp.Failure, "expected payment failure")
+	require.Equal(
+		ht, lnrpc.Failure_AMOUNT_BELOW_MINIMUM, sendResp.Failure.Code,
+	)
+
+	// The failure should carry the advertised min HTLC value so that
+	// callers can react to the channel policy.
+	require.NotNil(
+		ht, sendResp.Failure.ChannelUpdate,
+		"expected channel update in failure",
+	)
+	require.Equal(
+		ht, uint64(customMinHtlc),
+		sendResp.Failure.ChannelUpdate.HtlcMinimumMsat,
+	)
 
 	// Make sure sending using the original value succeeds.
 	payAmt = btcutil.Amount(5)
@@ -213,17 +212,12 @@ func testUpdateChannelPolicy(ht *lntest.HarnessTest) {
 		TotalAmtMsat: amtMSat,
 	}
 
-	sendReq = &lnrpc.SendToRouteRequest{
+	sendReq = &routerrpc.SendToRouteRequest{
 		PaymentHash: resp.RHash,
 		Route:       route,
 	}
-
-	err = alicePayStream.Send(sendReq)
-	require.NoError(ht, err, "unable to send payment")
-
-	sendResp, err = ht.ReceiveSendToRouteUpdate(alicePayStream)
-	require.NoError(ht, err, "unable to receive payment stream")
-	require.Empty(ht, sendResp.PaymentError, "expected payment to succeed")
+	sendResp = alice.RPC.SendToRouteV2(sendReq)
+	require.Nil(ht, sendResp.Failure, "expected payment to succeed")
 
 	// With our little cluster set up, we'll update the outbound fees and
 	// the max htlc size for the Bob side of the Alice->Bob channel, and

itest/lnd_routing_test.go

@@ -22,41 +22,20 @@ import (
 
 var sendToRouteTestCases = []*lntest.TestCase{
 	{
-		Name: "single hop with sync",
-		TestFunc: func(ht *lntest.HarnessTest) {
-			// useStream: false, routerrpc: false.
-			testSingleHopSendToRouteCase(ht, false, false)
-		},
-	},
-	{
-		Name: "single hop with stream",
-		TestFunc: func(ht *lntest.HarnessTest) {
-			// useStream: true, routerrpc: false.
-			testSingleHopSendToRouteCase(ht, true, false)
-		},
-	},
-	{
-		Name: "single hop with v2",
-		TestFunc: func(ht *lntest.HarnessTest) {
-			// useStream: false, routerrpc: true.
-			testSingleHopSendToRouteCase(ht, false, true)
-		},
+		Name:     "single hop",
+		TestFunc: testSingleHopSendToRoute,
 	},
 }
 
-// testSingleHopSendToRouteCase tests that payments are properly processed
-// through a provided route with a single hop. We'll create the following
-// network topology:
+// testSingleHopSendToRoute tests that payments are properly processed through
+// a provided route with a single hop. We'll create the following network
+// topology:
 //
 //	Carol --100k--> Dave
 //
 // We'll query the daemon for routes from Carol to Dave and then send payments
-// by feeding the route back into the various SendToRoute RPC methods. Here we
-// test all three SendToRoute endpoints, forcing each to perform both a regular
-// payment and an MPP payment.
-func testSingleHopSendToRouteCase(ht *lntest.HarnessTest,
-	useStream, useRPC bool) {
-
+// by feeding the route back into SendToRouteV2.
+func testSingleHopSendToRoute(ht *lntest.HarnessTest) {
 	const chanAmt = btcutil.Amount(100000)
 	const paymentAmtSat = 1000
 	const numPayments = 5
@@ -97,8 +76,6 @@ func testSingleHopSendToRouteCase(ht *lntest.HarnessTest,
 	ht.WaitForNodeBlockHeight(carol, minerHeight)
 	ht.WaitForNodeBlockHeight(dave, minerHeight)
 
-	// Query for routes to pay from Carol to Dave using the default CLTV
-	// config.
 	routesReq := &lnrpc.QueryRoutesRequest{
 		PubKey: dave.PubKeyStr,
 		Amt:    paymentAmtSat,
@@ -108,82 +85,28 @@ func testSingleHopSendToRouteCase(ht *lntest.HarnessTest,
 	// There should only be one route to try, so take the first item.
 	r := routes.Routes[0]
 
-	// Construct a closure that will set MPP fields on the route, which
-	// allows us to test MPP payments.
-	setMPPFields := func(i int) {
+	for i, rHash := range rHashes {
+		// Set the MPP record on the last hop with the payment addr from
+		// the corresponding invoice so the receiver can accept the
+		// HTLC.
 		hop := r.Hops[len(r.Hops)-1]
 		hop.TlvPayload = true
 		hop.MppRecord = &lnrpc.MPPRecord{
 			PaymentAddr:  payAddrs[i],
 			TotalAmtMsat: paymentAmtSat * 1000,
 		}
-	}
 
-	// Construct closures for each of the payment types covered:
-	//  - main rpc server sync
-	//  - main rpc server streaming
-	//  - routerrpc server sync
-	sendToRouteSync := func() {
-		for i, rHash := range rHashes {
-			setMPPFields(i)
-
-			sendReq := &lnrpc.SendToRouteRequest{
-				PaymentHash: rHash,
-				Route:       r,
-			}
-			resp := carol.RPC.SendToRouteSync(sendReq)
-			require.Emptyf(ht, resp.PaymentError,
-				"received payment error from %s: %v",
-				carol.Name(), resp.PaymentError)
-		}
-	}
-	sendToRouteStream := func() {
-		alicePayStream := carol.RPC.SendToRoute()
-
-		for i, rHash := range rHashes {
-			setMPPFields(i)
-
-			sendReq := &lnrpc.SendToRouteRequest{
-				PaymentHash: rHash,
-				Route:       routes.Routes[0],
-			}
-			err := alicePayStream.Send(sendReq)
-			require.NoError(ht, err, "unable to send payment")
-
-			resp, err := ht.ReceiveSendToRouteUpdate(alicePayStream)
-			require.NoError(ht, err, "unable to receive stream")
-			require.Emptyf(ht, resp.PaymentError,
-				"received payment error from %s: %v",
-				carol.Name(), resp.PaymentError)
-		}
-	}
-	sendToRouteRouterRPC := func() {
-		for i, rHash := range rHashes {
-			setMPPFields(i)
-
-			sendReq := &routerrpc.SendToRouteRequest{
-				PaymentHash: rHash,
-				Route:       r,
-			}
-			resp := carol.RPC.SendToRouteV2(sendReq)
-			require.Nilf(ht, resp.Failure, "received payment "+
-				"error from %s", carol.Name())
+		// Dispatch the payment along the prepared route and assert that
+		// no failure was returned.
+		sendReq := &routerrpc.SendToRouteRequest{
+			PaymentHash: rHash,
+			Route:       r,
 		}
-	}
-
-	// Using Carol as the node as the source, send the payments
-	// synchronously via the routerrpc's SendToRoute, or via the main RPC
-	// server's SendToRoute streaming or sync calls.
-	switch {
-	case !useRPC && useStream:
-		sendToRouteStream()
-	case !useRPC && !useStream:
-		sendToRouteSync()
-	case useRPC && !useStream:
-		sendToRouteRouterRPC()
-	default:
-		require.Fail(ht, "routerrpc does not support "+
-			"streaming send_to_route")
+		resp := carol.RPC.SendToRouteV2(sendReq)
+		require.Nilf(
+			ht, resp.Failure, "received payment error from %s",
+			carol.Name(),
+		)
 	}
 
 	// Verify that the payment's from Carol's PoV have the correct payment
@@ -431,22 +354,15 @@ func testSendToRouteErrorPropagation(ht *lntest.HarnessTest) {
 	resp := bob.RPC.AddInvoice(invoice)
 	rHash := resp.RHash
 
-	// Using Alice as the source, pay to the invoice from Bob.
-	alicePayStream := alice.RPC.SendToRoute()
-
-	sendReq := &lnrpc.SendToRouteRequest{
+	// Using Alice as the source, send to the invoice from Bob via a fake
+	// route - we expect this to fail with UnknownNextPeer.
+	sendReq := &routerrpc.SendToRouteRequest{
 		PaymentHash: rHash,
 		Route:       fakeRoute.Routes[0],
 	}
-	err := alicePayStream.Send(sendReq)
-	require.NoError(ht, err, "unable to send payment")
-
-	// At this place we should get an rpc error with notification
-	// that edge is not found on hop(0)
-	event, err := ht.ReceiveSendToRouteUpdate(alicePayStream)
-	require.NoError(ht, err, "payment stream has been closed but fake "+
-		"route has consumed")
-	require.Contains(ht, event.PaymentError, "UnknownNextPeer")
+	event := alice.RPC.SendToRouteV2(sendReq)
+	require.NotNil(ht, event.Failure, "expected payment failure")
+	require.Equal(ht, lnrpc.Failure_UNKNOWN_NEXT_PEER, event.Failure.Code)
 }
 
 // testPrivateChannels tests that a private channel can be used for

lnrpc/README.md

@@ -69,15 +69,6 @@ description):
      * Attempts to close a target channel. A channel can either be closed
        cooperatively if the channel peer is online, or using a "force" close to
        broadcast the latest channel state.
-  * SendPayment
-     * Send a payment over Lightning to a target peer.
-  * SendPaymentSync
-     * SendPaymentSync is the synchronous non-streaming version of SendPayment.
-  * SendToRoute
-    * Send a payment over Lightning to a target peer through a route explicitly
-      defined by the user.
-  * SendToRouteSync
-    * SendToRouteSync is the synchronous non-streaming version of SendToRoute.
   * AddInvoice
      * Adds an invoice to the daemon. Invoices are automatically settled once
        seen as an incoming HTLC.