multi: remove deprecated RPCs and config flags scheduled for 0.21#10795
multi: remove deprecated RPCs and config flags scheduled for 0.21#10795erickcestari wants to merge 9 commits into
Conversation
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 deprecation cycle for several RPCs, configuration flags, and fields that were announced for removal in the 0.20 release. By removing these legacy components, the codebase is simplified and technical debt is reduced, while ensuring that users migrate to the more robust V2 alternatives. 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
|
🔴 PR Severity: CRITICAL
🔴 Critical (2 files)
🟠 High (3 files, plus 7 auto-generated excluded from count)
🟡 Medium (9 files)
🟢 Low (7 files — excluded from severity calculation)
AnalysisThis PR touches Even though most changes are deletions (3,055 deletions vs. 423 additions, net reduction of ~2,632 lines), removing RPC handlers from the core server warrants careful expert review to ensure no active functionality is inadvertently dropped and that all call sites have been migrated. The >500 non-test/non-generated lines changed threshold for a severity bump is met (~1,473 lines), but the PR is already at the maximum CRITICAL level. To override, add a |
There was a problem hiding this comment.
Code Review
This pull request removes deprecated Tor v2 support and several legacy payment and tracking RPCs, including SendPayment, SendPaymentSync, SendToRoute, and TrackPayment, along with associated configuration flags and test cases. The changes simplify the codebase by removing obsolete logic and updating integration tests to use V2 RPC equivalents. Review feedback suggests moving the QueryProbability function to router_server.go and deleting the now-redundant router_server_deprecated.go file, while also improving the function's documentation to comply with the LND style guide for exported functions.
b6c6036 to
74f5520
Compare
74f5520 to
a99d010
Compare
|
@claude Check that all deprecated features have been removed correctly. |
|
I'll analyze this and get back to you. |
|
This bot does not work on forked repos. |
yyforyongyu
left a comment
There was a problem hiding this comment.
Looking good, left a few comments found by gpt
a99d010 to
95d7c04
Compare
|
Thanks for the review @yyforyongyu. Here is a summary of the changes since the last review:
|
|
I double-checked the remaining matches after the latest update. Two leftovers still look worth addressing/clarifying:
|
|
Open question: how do we want to handle the deprecated This PR currently keeps the deprecated fields in the public proto surface and rejects non-zero values at runtime:
I confirmed both are still present directly in the proto files:
Keeping them has the advantage that old clients get an explicit error instead of having the field silently ignored. But it also means the fields remain in generated clients, swagger, and API docs, so they are not fully removed from the proto surface. Should we instead remove these fields from the messages and reserve their tag numbers? For example: message QueryRoutesRequest {
reserved 14;
...
}and: message SendPaymentRequest {
reserved 8;
...
}There is precedent for reserved tags in the current codebase:
The tradeoff is that deleting the fields means old binary gRPC clients that still send those tags may have them decoded as unknown fields, so the server may no longer be able to return a targeted “use Which behavior do we want for the 0.21 removal? I am for removing it completely. |
|
Thanks for the review @ziggie1984 ! The summary of the changes after addressing your comments:
|
b512b99 to
830c817
Compare
|
It looks like my last push caused the CI to break. I'm working on the fix. |
830c817 to
8ba2e0d
Compare
8ba2e0d to
6706f45
Compare
|
One more Tor-related edge case worth clarifying/fixing: restored onion keys are not validated as v3. The new-key path now correctly creates v3 services because case nil:
keyParam = string(privateKey)That means a restored key such as This is about our own hidden service / watchtower hidden service restoration path, not about parsing other peers’ historical v2 onion addresses. The call path is roughly:
The default plaintext old-v2-key case is partially blocked now because Proposal: fail strictly when a restored onion private key is not v3. For example, after loading/decrypting a stored key, reject anything that does not start with I think strict failure is safer than silently regenerating because regenerating would change our advertised onion identity automatically. For watchtower in particular, address stability can matter, so changing it should be an explicit user action rather than a hidden migration side effect. A small unit test with a fake |
There was a problem hiding this comment.
Code Review
This pull request removes support for Tor v2 onion services and several deprecated payment-related RPCs, including their proto definitions, server implementations, and associated tests. The review feedback identifies multiple style guide violations regarding function call wrapping in integration tests and the router backend, specifically where calls that fit within the 80-character limit were unnecessarily wrapped or where long calls were wrapped incorrectly. Additionally, a redundant test case in lnrpc/routerrpc/router_backend_test.go was noted for removal to improve code cleanliness.
| payReqs, rHashes, _ := ht.CreatePayReqs(dave, paymentAmtSat, | ||
| numPayments) |
There was a problem hiding this comment.
This function call wrapping violates the style guide. If a function call fits within the 80-character limit, it should be on a single line. If it must be wrapped, the closing parenthesis should be on its own line and all arguments should start on a new line after the opening parenthesis.
payReqs, rHashes, _ := ht.CreatePayReqs(dave, paymentAmtSat, numPayments)References
- Wrapping long function calls: If a function call exceeds the column limit, place the closing parenthesis on its own line and start all arguments on a new line after the opening parenthesis. (link)
There was a problem hiding this comment.
There was a problem hiding this comment.
Got it! I'll update it.
6706f45 to
5fd6223
Compare
Good idea. I've pushed new changes addressing this. |
|
The CI with fuzz regression caught a failed roundtrip from https://github.com/lightningnetwork/lnd/actions/runs/25880327640/job/76058506859?pr=10795#step:9:41 |
ed42312 to
d38f1d4
Compare
| payReqs, rHashes, _ := ht.CreatePayReqs(dave, paymentAmtSat, | ||
| numPayments) |
There was a problem hiding this comment.
yyforyongyu
left a comment
There was a problem hiding this comment.
Happy cleanup! My main comment is the error case when serializing the legacy v2, sth like,
- A graph node containing an old v2 onion address can be read from DB.
- If that node is later updated/re-saved, the write path sees the v2
*tor.OnionAddr. graph/db/addr.goorgraph/db/sql_store.gorejects it.- The whole node update/write fails, not just the obsolete address.
I think we can just drop v2 addrs at serialization.
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 the --tor.v2 config flag and all associated handling. Tor v2 onion services have been obsolete since October 2021 when the Tor network dropped support for them. Update config.go, lnd.go, and server.go to assume v3 when a tor hidden service is configured.
d38f1d4 to
b331ce7
Compare
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.
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. Regenerate all protobuf, gRPC, REST gateway, JSON, and swagger files.
b331ce7 to
c48f890
Compare
|
Thanks for the review @yyforyongyu . Your comments should be addressed now! |
c48f890 to
9464850
Compare
…e notes Add entries to the Breaking Changes section of the 0.21 release notes covering the RPCs, fields, and config flags removed in this PR that were announced for removal in 0.21 via the 0.20 release notes.
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 the affected pb.go/swagger artifacts.
9464850 to
79ae4a4
Compare
Drop the v2 onion encoder branches from lnwire.WriteOnionAddr, graph/db.encodeOnionAddr, graph/db.collectAddressRecords, and the tor.OnionHostToFakeIP helper. lnd no longer produces v2 onion addresses on any code path. Decoders for v2 addresses are intentionally retained in lnwire, graph/db, and tor so that existing graph databases and peer-announced v2 addresses continue to deserialize. Update unit tests to use v3 onion fixtures.
79ae4a4 to
a7a69d6
Compare

This PR removes the deprecated RPCs, fields, and configuration options that
were announced for removal in 0.21 via the
0.20 release notes.
Changes
Removed deprecated RPCs
The following RPCs are removed. Callers must migrate to the V2 equivalents
listed in the 0.20 release notes:
lnrpc.SendPayment(streaming)routerrpc.SendPaymentV2lnrpc.SendPaymentSyncrouterrpc.SendPaymentV2lnrpc.SendToRoute(streaming)routerrpc.SendToRouteV2lnrpc.SendToRouteSyncrouterrpc.SendToRouteV2routerrpc.SendPayment(streaming)routerrpc.SendPaymentV2routerrpc.SendToRouterouterrpc.SendToRouteV2routerrpc.TrackPayment(streaming)routerrpc.TrackPaymentV2This includes removing the corresponding REST gateway routes:
POST /v1/channels/transaction-streamPOST /v1/channels/transactionsPOST /v1/channels/transactions/routeRemoved deprecated
OutgoingChanIdfieldThe single-channel
outgoing_chan_idfield onQueryRoutesRequestandSendPaymentRequestis no longer honored. Callers must use the multi-channeloutgoing_chan_idsfield introduced in 0.20.The compatibility fallback that accepted either field (and errored when both
were set) is removed from the routerrpc backend.
Removed deprecated tor v2 support
The
--tor.v2configuration flag is removed. Tor v2 onion services have beenobsolete since October 2021 when the Tor network dropped support for them.
The hidden service setup paths in
config.go,lnd.go, andserver.goaresimplified to assume v3 whenever a hidden service is configured.
Internal cleanups
rpcserver.gothat wasexclusively used by the deleted lnrpc handlers:
paymentStream,rpcPaymentRequest,rpcPaymentIntent,extractPaymentIntent,dispatchPaymentIntent,sendPayment, andsendPaymentSync(~640 lines).legacyTrackPaymentServerwrapper,PaymentStateenum, andPaymentStatusmessage that were only used by the deletedrouterrpc.TrackPaymentstream.SendToRouteandSendToRouteSynchelpers from the integrationtest harness.
Test changes
itest/lnd_routing_test.go: collapsed threeSendToRoutetest cases (sync,stream, v2) into a single test exercising
SendToRouteV2. Updated the errorpropagation test to assert on
HTLCAttempt.Failure.Codeinstead of thelegacy
PaymentErrorstring.itest/lnd_channel_policy_test.go: migrated from streamingSendToRoutetoSendToRouteV2with structured failure assertions.lnrpc/routerrpc/router_backend_test.go: removed thesingleChanIDandbothChanIdstest variants that exercised the deprecated single-field pathand its conflict detection.
Commits
The PR is split into six logical commits:
proto: remove deprecated SendPayment, SendToRoute, TrackPayment RPCs- allproto, yaml, and regenerated stub files
lnrpc: remove deprecated Send* RPC server implementations- server-sidehandlers and dead payment infrastructure in
rpcserver.gorouterrpc: remove deprecated SendPayment, SendToRoute, TrackPayment impls-server-side shims and macaroon entries
routerrpc: remove deprecated outgoing_chan_id field handling- backendcompatibility fallback removal
lncfg: remove deprecated tor v2 support- tor v2 config flag removalitest: migrate deprecated lnrpc Send* calls to routerrpc V2- test harnessand integration test updates
Breaking changes
This is a breaking change for any client that still uses the deprecated RPCs,
fields, or config flag listed above. All have had V2 / replacement equivalents
available for multiple releases.