Remove deprecated Send* / TrackPayment RPCs and outgoing_chan_id field#10814
Conversation
Remove the compatibility fallback in QueryRoutes and ExtractPaymentIntent that accepted the deprecated single outgoing_chan_id field alongside the replacement outgoing_chan_ids. Callers must now use outgoing_chan_ids. Update TestQueryRoutes and TestExtractPaymentIntent accordingly.
Remove SendToRoute and SendToRouteSync helpers from the test harness and update integration tests to use routerrpc.SendToRouteV2: - lnd_routing_test.go: collapse three SendToRoute test cases (sync, stream, v2) into a single test using SendToRouteV2; update testSendToRouteErrorPropagation to assert on Failure.Code instead of PaymentError string - lnd_channel_policy_test.go: replace streaming SendToRoute with SendToRouteV2 and assert on HTLCAttempt.Failure instead of PaymentError string
Remove handler implementations and macaroon permission entries for the now-deleted lnrpc RPCs: SendPayment, SendPaymentSync, SendToRoute, and SendToRouteSync. Also remove the dead payment infrastructure that was exclusively used by these handlers: paymentStream, rpcPaymentRequest, rpcPaymentIntent, extractPaymentIntent, dispatchPaymentIntent, sendPayment, and sendPaymentSync.
…mpls Remove the SendPayment, SendToRoute, and TrackPayment shim methods from router_server_deprecated.go that delegated to their V2 counterparts. Remove their macaroon permission entries from router_server.go and the now-unused legacyTrackPaymentServer wrapper.
erickcestari
added
the
backport-v0.21.x-branch
erickcestari
force-pushed
the
deprecated-rpc-removal
branch
from
381b584 to
a618a93
Compare
github-actions
Bot
added
the
severity-critical
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request completes the removal of legacy payment RPCs and fields that were marked for deprecation in version 0.20. By removing these interfaces, the codebase is streamlined, and callers are now required to use the V2 variants (SendPaymentV2, SendToRouteV2, TrackPaymentV2) and the multi-channel outgoing_chan_ids field. This is a breaking change intended to simplify the API surface for the 0.21 release. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes several deprecated payment and tracking RPCs and fields across the lnrpc and routerrpc packages, updating documentation and tests to use their V2 equivalents. Feedback indicates that the body of the QueryProbability function was accidentally deleted, which would cause a compilation error; it is recommended to restore this function and move it to router_server.go to keep the codebase clean.
yyforyongyu
left a comment
yyforyongyu
left a comment
There was a problem hiding this comment.
Good move on splitting, LGTM🚢
Remove the following deprecated RPC definitions that were announced for removal in 0.21 via the 0.20 release notes: lnrpc: - SendPayment (bidirectional streaming) - SendPaymentSync - SendToRoute (bidirectional streaming) - SendToRouteSync routerrpc: - SendPayment (streaming) - SendToRoute - TrackPayment (streaming) Also remove the now-unused PaymentState enum and PaymentStatus message that were only used by the deprecated TrackPayment response stream, plus the corresponding REST annotations from the yaml files. Drop the now-orphan routerrpc.SendToRouteResponse message that was only referenced by the deleted routerrpc.SendToRoute RPC. Also remove the deprecated outgoing_chan_id field from lnrpc.QueryRoutesRequest (tag 14) and routerrpc.SendPaymentRequest (tag 8); their tag numbers are now reserved. Callers must use the multi-channel outgoing_chan_ids field introduced in 0.20. Drop the compat fallback in router_backend.go that previously consumed the field, and regenerate all protobuf, gRPC, REST gateway, JSON, and swagger files.
Add entries to the Breaking Changes section covering the payment and tracking RPCs and the `outgoing_chan_id` field removed in this branch, all of which were announced for removal in 0.21 via the 0.20 release notes.
erickcestari
force-pushed
the
deprecated-rpc-removal
branch
from
a618a93 to
05bdb66
Compare
github-actions
Bot
added
severity-critical
PR Severity: CRITICALauto-classified | 14 files | 1604 lines changed (excluding tests and auto-generated) Critical (1 file):
High (10 files):
Low/Excluded (tests, docs, auto-generated - not counted):
Analysis: CRITICAL is driven by rpcserver.go (core server coordination). PR removes deprecated RPC endpoints (~4800 line net deletion). The 1604 qualified lines would trigger a bump, but CRITICAL is already the maximum. To override, add a severity-override-{critical,high,medium,low} label. |
| // The deprecated single outgoing_chan_id field is no longer | ||
| // honored. Reject requests that still set it so that callers do | ||
| // not silently get an unrestricted route. | ||
| if in.OutgoingChanId != 0 { |
There was a problem hiding this comment.
if we do it in the first commit there is no need for this removal in the first place ?
|
Successfully created backport PR for |
| ], | ||
| "default": "APRIORI" | ||
| }, | ||
| "PaymentPaymentStatus": { |
There was a problem hiding this comment.
The OpenAPI/Swagger definition for the payment-status enum was renamed from lnrpcPaymentPaymentStatus to PaymentPaymentStatus (and the standalone routerrpcPaymentStatus was removed)
▎ as a side effect of the deprecated TrackPayment V1 removal. Downstream consumers regenerating clients from the swagger JSON will need to update identifier references.
Just noting it here, in case someone runs into it
…21.x-branch [v0.21.x-branch] Backport #10814: Remove deprecated Send* / TrackPayment RPCs and outgoing_chan_id field
4 participants
Context
This PR is carved out of #10795 ("multi: remove deprecated RPCs and
config flags scheduled for 0.21"), which was split into smaller,
independently-reviewable PRs by topic. This is the payment RPC piece:
removal of the legacy
Send*andTrackPaymentsurface that wasannounced for removal in 0.21 via the 0.20 release notes.
The replacements have shipped since 0.20: callers should be on the V2
variants (
SendPaymentV2,SendToRouteV2,TrackPaymentV2) and themulti-channel
outgoing_chan_idsfield. This PR drops the deprecatedsurface they replace.
What changes
RPCs removed
lnrpc
SendPayment(bidirectional streaming)SendPaymentSyncSendToRoute(bidirectional streaming)SendToRouteSyncrouterrpc
SendPayment(streaming)SendToRouteTrackPayment(streaming)The now-orphan
routerrpc.SendToRouteResponsemessage and thePaymentStateenum /PaymentStatusmessage (only referenced by thedeprecated
TrackPaymentresponse) are dropped as well.Fields removed
lnrpc.QueryRoutesRequest.outgoing_chan_id(tag 14, now reserved)routerrpc.SendPaymentRequest.outgoing_chan_id(tag 8, now reserved)The compatibility fallback in
QueryRoutesandExtractPaymentIntentthat accepted the single-channel field alongside
outgoing_chan_idsis removed; callers must now use
outgoing_chan_ids.Server-side cleanup
deleted lnrpc RPCs are removed from
rpcserver.go, along with thedead payment infrastructure that was exclusively used by them:
paymentStream,rpcPaymentRequest,rpcPaymentIntent,extractPaymentIntent,dispatchPaymentIntent,sendPayment,sendPaymentSync.routerrpcshim methods inrouter_server_deprecated.gothatdelegated to V2, the matching macaroon entries, and the
legacyTrackPaymentServerwrapper are removed.pb.go,pb.gw.go,pb.json.go,_grpc.pb.go, RESTyaml, and swagger artifacts are regenerated.
Itest migration
lnd_routing_test.go: the threeSendToRoutetest cases (sync,stream, v2) are collapsed into a single test using
SendToRouteV2;testSendToRouteErrorPropagationnow asserts onFailure.Codeinstead of the legacy
PaymentErrorstring.lnd_channel_policy_test.go: streamingSendToRoutereplaced withSendToRouteV2, asserting onHTLCAttempt.Failure.SendToRoute/SendToRouteSynchelpers are removed from the testharness.
Release notes
Breaking Changes entries added to
release-notes-0.21.0.mdcoveringthe RPC and field removals.
Breaking changes / caller-facing notes
This is a breaking change for any client still on the legacy surface.
Migration:
SendPayment/SendPaymentSync(lnrpc) andSendPayment(routerrpc) callers must switch to
routerrpc.SendPaymentV2.SendToRoute/SendToRouteSync(lnrpc) andSendToRoute(routerrpc) callers must switch to
routerrpc.SendToRouteV2. Errorreporting moves from the string
PaymentErrorfield to thestructured
Failure/HTLCAttempt.Failurefield.TrackPayment(routerrpc) callers must switch toTrackPaymentV2;the
PaymentState/PaymentStatustypes are gone.outgoing_chan_idmust populateoutgoing_chan_ids(a repeated field) instead. The old tag isreserved, so stale clients sending it will be rejected by proto
parsing.