Summary of Changes

Hello, I'm Gemini Code Assist¹! 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

• Removal of Deprecated RPCs: Removed several deprecated RPCs, including lnrpc.SendPayment, lnrpc.SendToRoute, and routerrpc.TrackPayment, along with their REST gateway equivalents. Callers are directed to use the V2 equivalents.
• Removal of Deprecated Fields: Removed the outgoing_chan_id field from QueryRoutesRequest and SendPaymentRequest, requiring callers to use the multi-channel outgoing_chan_ids field.
• Tor v2 Support Removal: Removed the --tor.v2 configuration flag and associated infrastructure, as Tor v2 onion services are obsolete.
• Internal Cleanup and Testing: Cleaned up dead payment infrastructure in rpcserver.go and migrated integration tests to use the V2 RPC equivalents.

🔴 PR Severity: CRITICAL

Highest-severity file wins | 15 non-test/non-generated files | ~1,473 lines changed (excl. tests & generated)

Analysis

This PR touches rpcserver.go (788 lines removed) and server.go, both of which are explicitly classified as CRITICAL core server coordination files. The changes appear to be a significant cleanup/removal of deprecated RPC endpoints — particularly in rpcserver.go (~788 lines removed) and lnrpc/routerrpc/router_server_deprecated.go (~114 lines removed), along with corresponding proto/swagger changes.

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 severity-override-{critical,high,medium,low} label.}
<!-- pr-severity-bot -->

@claude Check that all deprecated features have been removed correctly.

Claude Code is working…

I'll analyze this and get back to you.

View job run

This bot does not work on forked repos.

Thanks for the review @yyforyongyu.

Here is a summary of the changes since the last review:

• proto removal (4728515): dropped the now-orphan SendRequest/SendResponse/SendToRouteRequest messages from lightning.proto, regenerated pb.go. Removed /v1/channels/transaction-stream from LndClientStreamingURIs, updated docs/rest/websockets.md example, and removed stale SendPayment*/SendToRoute* doc bullets from lnrpc/README.md.
• outgoing_chan_id (1d37383): non-zero deprecated outgoing_chan_id on QueryRoutesRequest/SendPaymentRequest is now explicitly rejected with an error instead of silently dropped.
• tor v2 (72b4388): removed --tor.v2 from docs/configuring_tor.md and sample-lnd.conf; cleaned up tor/cmd_onion.go (dropped V2 OnionType, V2KeyParam, V2 branches in prepareKeyparam/AddOnion, updated RSA1024 → ED25519-V3 protocol example); updated tor/cmd_onion_test.go expectations.
• itest (d0225e7): re-added the HtlcMinimumMsat == customMinHtlc assertion on the SendToRouteV2 failure path in lnd_channel_policy_test.go.
• release notes (95d7c04): reworded the outgoing_chan_id bullet to reflect that the proto field is kept but non-zero values are rejected.

I double-checked the remaining matches after the latest update. Two leftovers still look worth addressing/clarifying:

1. routerrpc.SendToRouteResponse still exists even though the deprecated routerrpc.SendToRoute RPC has been removed.

   • lnrpc/routerrpc/router.proto: message SendToRouteResponse
   • generated references remain in lnrpc/routerrpc/router.pb.go
   • I did not find a remaining RPC method that returns this message; SendToRouteV2 returns lnrpc.HTLCAttempt.

2. Tor v2 onion-service creation/config is removed, but Tor v2 address parsing/storage compatibility remains.

   • tor/onionaddr.go: V2DecodedLen, V2Len
   • tor/tor.go: v2 onion parsing paths
   • additional compatibility paths remain in lnwire and graph DB address handling, for example graph/db/sql_store.go still stores/decodes addressTypeTorV2.

Open question: how do we want to handle the deprecated outgoing_chan_id proto fields?

This PR currently keeps the deprecated fields in the public proto surface and rejects non-zero values at runtime:

• lnrpc.QueryRoutesRequest.outgoing_chan_id = 14
• routerrpc.SendPaymentRequest.outgoing_chan_id = 8

I confirmed both are still present directly in the proto files:

• lnrpc/lightning.proto: uint64 outgoing_chan_id = 14 [jstype = JS_STRING, deprecated = true];
• lnrpc/routerrpc/router.proto: uint64 outgoing_chan_id = 8 [jstype = JS_STRING, deprecated = true];

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:

• lnrpc/lightning.proto: reserved 11 in GetInfoResponse
• lnrpc/lightning.proto: reserved 2 with the comment “Previously used for confirmation_height. Do not reuse.”
• lnrpc/lightning.proto: reserved 3 in QueryRoutesRequest
• lnrpc/routerrpc/router.proto: reserved 1 in QueryMissionControlResponse
• lnrpc/routerrpc/router.proto: reserved 3, 4, 5, 6 in PairHistory
• lnrpc/routerrpc/router.proto: reserved 3 in PairData
• lnrpc/invoicesrpc/invoices.proto: reserved 1 in SubscribeSingleInvoiceRequest

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 outgoing_chan_ids” error. Keeping the fields allows that explicit rejection, but leaves deprecated fields in the API surface.

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:

• ec24f0a proto: removed orphan routerrpc.SendToRouteResponse; removed deprecated outgoing_chan_id field from lnrpc.QueryRoutesRequest (tag 14) and routerrpc.SendPaymentRequest (tag 8), reserving both tags; dropped the runtime fallback in router_backend.go; regenerated pb.go/swagger.
• ffcc993 tor v2: dropped V2 onion encoder paths in lnwire.WriteOnionAddr, graph/db.encodeOnionAddr, graph/db.collectAddressRecords, and tor.OnionHostToFakeIP. Decoders kept for backwards-compat with existing DBs and peer gossip. Tests updated to V3 fixtures.
• b512b99 docs: updated 0.21 release notes to reflect the proto field removal (vs runtime rejection), the SendToRouteResponse drop, and the asymmetric V2 encoder removal.

It looks like my last push caused the CI to break. I'm working on the fix.

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 prepareKeyparam defaults to NEW:ED25519-V3. But if an OnionStore returns an existing private key, prepareKeyparam currently uses it directly:

case nil:
    keyParam = string(privateKey)

That means a restored key such as RSA1024:... can still be passed to Tor via ADD_ONION, even though --tor.v2, tor.V2, and V2KeyParam were removed.

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:

• server.createNewHiddenService / watchtower.createNewHiddenService
• tor.NewOnionFile(...) as the store
• TorController.AddOnion
• prepareKeyparam
• stored key replaces NEW:ED25519-V3 if PrivateKey() succeeds

The default plaintext old-v2-key case is partially blocked now because OnionFile.PrivateKey() no longer treats RSA1024 as a valid plaintext key prefix. But encrypted old v2 keys, custom stores, or future store implementations can still return RSA1024:... after retrieval/decryption and bypass that check.

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 ED25519-V3 and return a clear error telling the user to remove the old onion key file if they want a new v3 service generated.

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 OnionStore returning RSA1024:... would lock this in.

Thanks for the review @yyforyongyu . Your comments should be addressed now!

Summary

Two commits on remove-deprecated-rpcs-20, both addressing the
signature-verification failure introduced when multi: remove tor v2 onion address encoder paths (30ed7a6) made the wire codec asymmetric
for v2 onion addresses.

17e65bb22 -- multi: keep wire codec faithful for legacy v2 onion addresses

Transient design. Restored byte-level fidelity in the wire codec but
still filtered v2 at storage entry.

• lnwire/node_announcement.go -- removed the post-Decode v2 strip so
  DataToSign sees the original signed bytes.
• lnwire/writer.go -- restored the v2 branch in WriteOnionAddr and
  deleted ErrV2OnionAddrNotSupported. Encode/DataToSign now round-trip
  v2.
• lnwire/writer_test.go -- onion address v2 rejected -> onion address v2 with concrete expected bytes.
• graph/db/models/node.go -- NodeFromWireAnnouncement strips v2 when
  copying msg.Addresses into the Node (filtering moved out of the codec
  but still pre-storage).
• netann/node_announcement_test.go -- new regression test: signs a
  [v3, v2, ipv4] announcement, round-trips, verifies signature passes,
  asserts v2 dropped from Node.Addresses.

f8a4e4c4d -- multi: persist v2 onion addresses and filter at consumption sites

Pivot. Stops filtering at storage entry so we can reproduce signed
bytes after a restart, and moves the v2 drop to the dial site.

• graph/db/models/node.go -- NodeFromWireAnnouncement no longer strips;
  Node.Addresses carries v2 into storage.
• graph/db/addr.go -- restored the v2 branch in encodeOnionAddr so the
  DB persists v2. Deleted FilterSerializableAddrs. Added
  FilterReachableAddrs (the inverse intent: addresses we can dial
  today).
• channeldb/codec.go, channeldb/nodes.go, graph/db/kv_store.go --
  dropped the FilterSerializableAddrs calls on every write path; the
  scalar net.Addr case in codec.go simplified back to plain
  SerializeAddr.
• server.go::fetchNodeAdvertisedAddrs -- applies FilterReachableAddrs
  so the dialer skips v2.
• channeldb/codec_test.go -- rewritten for the "preserve, don't filter"
  contract.
• netann/node_announcement_test.go -- regression test updated: v2 must
  survive into Node.Addresses so re-broadcast can reproduce the signed
  bytes.

Net effect

Invariant: what the peer signed = what we store = what we re-emit. lnd
still never produces v2 itself (lncfg refuses it as input). v2 is
dropped only at the dial site, where Tor stopped serving v2 in October
2021 anyway.

If this is the desired behavior we want, I'll squash and rebase the commits.

I'm working on separating this PR into 2 different ones. One for the tor changes and another for the removal of the deprecated RPC. It's probably make it easier to review both.