Skip to content

multi: auto-persist connected address for channel peers#10975

Open
ZZiigguurraatt wants to merge 2 commits into
lightningnetwork:masterfrom
ZZiigguurraatt:auto_remember_peer_addresses
Open

multi: auto-persist connected address for channel peers#10975
ZZiigguurraatt wants to merge 2 commits into
lightningnetwork:masterfrom
ZZiigguurraatt:auto_remember_peer_addresses

Conversation

@ZZiigguurraatt

@ZZiigguurraatt ZZiigguurraatt commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

When lnd completes an outbound connection to a peer that we have an
open channel with, record the address we dialed in that peer's LinkNode
entry (if it is not already listed). Peers without an open channel are
skipped — so casual connections do not grow the on-disk store, and a
lingering entry for a peer whose channels were closed but not yet
reaped by PruneLinkNodes will not be extended if we happen to
reconnect to them in the meantime.

Motivation

There are two sources of addresses used to reach a channel peer:

  1. LinkNode — the peer's address stored in the channel state DB.
  2. Gossip — addresses from the peer's most recent
    NodeAnnouncement, stored in the on-disk channel graph.

On startup, establishPersistentConnections dials the union of these
two sources. That behaviour is unchanged by this commit.

Before this commit:

LinkNode was written once at channel-open and not updated afterward.
Gossip carries subsequent listener changes when a peer re-signs and
re-broadcasts a NodeAnnouncement, but there are cases where the
address we can actually reach the peer at is not in their current
gossip entry:

* the peer has since removed the address from their
  `NodeAnnouncement` but is still listening on the same host and
  port,
* the peer moved to a new host and/or port and we learned the new
  address out-of-band faster than the peer's re-broadcast
  `NodeAnnouncement` could catch up, or
* the peer never advertised the address in gossip in the first
  place (private channel, LSP setup, LAN address).

In any of these cases, lnd could reach the peer at the address at
some point — via automatic reconnect (at startup or after a
mid-session disconnect) while the address was still in gossip, or
via lncli connect <pub>@<addr> for addresses that were never in
gossip or where gossip lagged the peer's move. But the address was
not persisted anywhere — so the next restart's startup dial failed
even though a live connection had just worked.

After this commit:

Every successful outbound connection to a channel peer appends the
dialed address to the peer's LinkNode entry (deduped by string). The
next restart's startup dial includes it, so an operator-supplied or
otherwise learned address survives across restarts for channel peers.

Non-channel peers, addresses already listed, and inbound connections
are skipped (see below for the reasoning behind the inbound skip).

Only outbound

The auto-persist path runs only for outbound connections. For inbound
TCP the peer's RemoteAddr() is the ephemeral source port their
kernel picked for the outbound half of their side, not a listener we
could dial back to. Persisting it would inflate the LinkNode address
list with non-dialable entries that fail on every subsequent startup
dial.

There is no equivalent for learning a peer's new listener from an
inbound connection today: BOLT #1 Init does not carry a listen
address, and NodeAnnouncement gossip is the intended source of
truth. lnd already forwards this: an lnd peer with a confirmed public
channel to us pushes its signed NodeAnnouncement on reconnect
(peer.maybeSendNodeAnn), and the gossiper writes it to the on-disk
channel graph, which establishPersistentConnections reads. That
covers the public-channel-with-lnd-peer case automatically. The gap
this commit narrows is what remains: operator-supplied addresses for
private channels, non-lnd peers, or gossip-lag windows.

Design

  • channeldb.LinkNodeDB.AddAddressIfPeerKnown(pub, addr) appends
    addr to the peer's LinkNode entry when one already exists and the
    address is not already listed. It is a no-op (nil error) when the
    peer has no LinkNode entry, which is the invariant we need to gate
    the store's growth.

  • server.maybePersistPeerAddress is called from peerConnected
    only when inbound is false and only after addPeer. It checks
    chanStateDB.FetchOpenChannels for the peer and only then calls
    the DB helper. Errors are logged and swallowed.

Since channel peers always have a LinkNode entry (created at channel
open, and RepairLinkNodes reinstates missing ones at startup), the
"no-op when no entry" branch of AddAddressIfPeerKnown will not fire
in practice for channel peers — the open-channel gate is what keeps the
store from growing.

The new address is surfaced through the existing remembered_addresses
field on Peer introduced in #10973; no new RPC surface is required.

itest

auto persist channel peer address opens a channel between Alice and
Bob at Bob's initial P2P port, disconnects them, restarts Bob on a new
port, then reconnects Alice to Bob at the new port. The test asserts
that Alice's remembered_addresses for Bob then contains both the
channel-open address and the newly-connected address.

Depends on #10973: the itest reads the remembered_addresses field
and uses the include_offline_persistent_peers flag added there.

Contributes to #10870.

ListPeers previously returned only currently-connected peers with a single
`address` field, leaving no way to see:

  1. Why lnd is maintaining a connection to a given peer.
  2. What addresses lnd has stored for reconnecting to the peer.
  3. What the peer currently advertises in its NodeAnnouncement.
  4. Which peers lnd is auto-reconnecting to but is not currently connected
     to.
  5. Whether a reconnect attempt is in flight.

This commit fills those gaps by adding to the `Peer` message:

  * `is_persistent` — true if lnd is auto-reconnecting to the peer.
  * `remembered_addresses` — addresses lnd has stored for the peer (from
    the LinkNode record captured at first channel open).
  * `gossip_addresses` — addresses from the peer's current
    NodeAnnouncement, populated inline instead of requiring a separate
    GetNodeInfo call per peer.
  * `reconnect_pending` — true if lnd currently has an in-flight reconnect
    attempt for the peer.

`ListPeersRequest` gains `include_offline_persistent_peers`. When set, the
response also includes peers that are in lnd's auto-reconnect set but not
currently connected. Per-connection fields (address, bytes_sent, ping_time,
etc.) are empty/zero for those entries.

`lncli listpeers` gains an `--include_offline_persistent_peers` flag.

itest: `listpeers address fields` opens a channel between two nodes,
suspends the peer, and asserts the new field state in both the connected
and offline-persistent cases.

Partial fix for lightningnetwork#10871
@github-actions github-actions Bot added the severity-critical Requires expert review - security/consensus critical label Jul 16, 2026
@github-actions

Copy link
Copy Markdown

🔴 PR Severity: CRITICAL

Classification | 11 files | 601 additions / 14 deletions (310 lines / 7 files after excluding tests & auto-generated code)

🔴 Critical (3 files)
  • channeldb/nodes.go - channeldb/* covers channel state persistence and DB migrations
  • rpcserver.go - explicitly listed as core server coordination
  • server.go - explicitly listed as core server coordination
🟠 High (2 files)
  • lnrpc/lightning.pb.go - generated code under lnrpc/* (RPC/API definitions); excluded from line/file bump counting as auto-generated
  • lnrpc/lightning.swagger.json - generated API doc mirroring the proto change under lnrpc/*
🟡 Medium (2 files)
  • cmd/commands/commands.go - cmd/* CLI client code, kept at its own tier regardless of the server-side changes elsewhere in the PR
  • lnrpc/lightning.proto - *.proto file, classified as an API change per the explicit proto rule
🟢 Low (4 files)
  • docs/release-notes/release-notes-0.22.0.md - release notes
  • itest/list_on_test.go - test-only change
  • itest/lnd_network_test.go - test-only change
  • lntest/rpc/lnd.go - test infrastructure helper

Analysis

The highest-severity files in this PR are server.go, rpcserver.go, and channeldb/nodes.go, all of which fall directly under the explicit CRITICAL list (core server coordination and channel-state persistence). That alone sets the PR severity to CRITICAL, independent of any size-based bump.

For transparency: after excluding test files (itest/*, lntest/*) and auto-generated code (lnrpc/lightning.pb.go), the remaining diff is 7 files and ~310 lines — below the >20 files / >500 lines bump thresholds. However, the PR does touch two distinct critical-tier areas (channeldb/* and the server.go/rpcserver.go pair), which would normally trigger a "touches multiple distinct critical packages" bump; since CRITICAL is already the top tier, this has no further effect on the label.

Given the changes span new node-related channeldb persistence, new RPC surface (proto/pb.go/swagger), and wiring into server.go/rpcserver.go, this PR warrants expert review of the wallet/server-state-management path in addition to standard RPC-surface review.


To override, add a severity-override-{critical,high,medium,low} label.

When lnd completes an outbound connection to a peer that we have an
open channel with, record the address we dialed in that peer's LinkNode
entry (if it is not already listed). Peers without an open channel are
skipped — so casual connections do not grow the on-disk store, and a
lingering entry for a peer whose channels were closed but not yet
reaped by `PruneLinkNodes` will not be extended if we happen to
reconnect to them in the meantime.

Motivation
----------

There are two sources of addresses used to reach a channel peer:

  1. LinkNode — the peer's address stored in the channel state DB.
  2. Gossip — addresses from the peer's most recent
     `NodeAnnouncement`, stored in the on-disk channel graph.

On startup, `establishPersistentConnections` dials the union of these
two sources. That behaviour is unchanged by this commit.

Before this commit:

  LinkNode was written once at channel-open and not updated afterward.
  Gossip carries subsequent listener changes when a peer re-signs and
  re-broadcasts a `NodeAnnouncement`, but there are cases where the
  address we can actually reach the peer at is not in their current
  gossip entry:

    * the peer has since removed the address from their
      `NodeAnnouncement` but is still listening on the same host and
      port,
    * the peer moved to a new host and/or port and we learned the new
      address out-of-band faster than the peer's re-broadcast
      `NodeAnnouncement` could catch up, or
    * the peer never advertised the address in gossip in the first
      place (private channel, LSP setup, LAN address).

  In any of these cases, lnd could reach the peer at the address at
  some point — via automatic reconnect (at startup or after a
  mid-session disconnect) while the address was still in gossip, or
  via `lncli connect <pub>@<addr>` for addresses that were never in
  gossip or where gossip lagged the peer's move. But the address was
  not persisted anywhere — so the next restart's startup dial failed
  even though a live connection had just worked.

After this commit:

  Every successful outbound connection to a channel peer appends the
  dialed address to the peer's LinkNode entry (deduped by string). The
  next restart's startup dial includes it, so an operator-supplied or
  otherwise learned address survives across restarts for channel peers.

  Non-channel peers, addresses already listed, and inbound connections
  are skipped (see below for the reasoning behind the inbound skip).

Only outbound
-------------

The auto-persist path runs only for outbound connections. For inbound
TCP the peer's `RemoteAddr()` is the ephemeral source port their
kernel picked for the outbound half of their side, not a listener we
could dial back to. Persisting it would inflate the LinkNode address
list with non-dialable entries that fail on every subsequent startup
dial.

There is no equivalent for learning a peer's new listener from an
inbound connection today: BOLT #1 `Init` does not carry a listen
address, and `NodeAnnouncement` gossip is the intended source of
truth. lnd already forwards this: an lnd peer with a confirmed public
channel to us pushes its signed `NodeAnnouncement` on reconnect
(`peer.maybeSendNodeAnn`), and the gossiper writes it to the on-disk
channel graph, which `establishPersistentConnections` reads. That
covers the public-channel-with-lnd-peer case automatically. The gap
this commit narrows is what remains: operator-supplied addresses for
private channels, non-lnd peers, or gossip-lag windows.

Design
------

  * `channeldb.LinkNodeDB.AddAddressIfPeerKnown(pub, addr)` appends
    `addr` to the peer's LinkNode entry when one already exists and the
    address is not already listed. It is a no-op (nil error) when the
    peer has no LinkNode entry, which is the invariant we need to gate
    the store's growth.

  * `server.maybePersistPeerAddress` is called from `peerConnected`
    only when `inbound` is false and only after `addPeer`. It checks
    `chanStateDB.FetchOpenChannels` for the peer and only then calls
    the DB helper. Errors are logged and swallowed.

Since channel peers always have a LinkNode entry (created at channel
open, and `RepairLinkNodes` reinstates missing ones at startup), the
"no-op when no entry" branch of `AddAddressIfPeerKnown` will not fire
in practice for channel peers — the open-channel gate is what keeps the
store from growing.

The new address is surfaced through the existing `remembered_addresses`
field on `Peer` introduced in lightningnetwork#10973; no new RPC surface is required.

itest
-----

`auto persist channel peer address` opens a channel between Alice and
Bob at Bob's initial P2P port, disconnects them, restarts Bob on a new
port, then reconnects Alice to Bob at the new port. The test asserts
that Alice's `remembered_addresses` for Bob then contains both the
channel-open address and the newly-connected address.

Depends on lightningnetwork#10973: the itest reads the `remembered_addresses` field
and uses the `include_offline_persistent_peers` flag added there.

Contributes to lightningnetwork#10870.
@ZZiigguurraatt
ZZiigguurraatt force-pushed the auto_remember_peer_addresses branch from 51a2f1f to fd3c6cb Compare July 16, 2026 00:54
ZZiigguurraatt added a commit to ZZiigguurraatt/lnd that referenced this pull request Jul 16, 2026
Extends `lncli disconnect` with two ways to prune a peer's stored
LinkNode state:

  * `--forget_node` for whole-peer removal.
  * `--forget_address` for surgical, per-address removal.

Both operations are gated by a shared `--force` flag when the requested
action would lose meaningful state:

  * `--forget_node` drops the live connection (base `disconnect` behavior)
    and additionally deletes the peer's LinkNode entry. The LinkNode
    delete is refused if open channels still exist with the peer unless
    `--force` is also set.
  * `--forget_address <host:port>` removes a single stored address from
    the peer's LinkNode entry. The live connection is left in place
    unless the removed address happens to be the currently-connected
    one, in which case the connection is dropped so lnd can re-dial via
    the remaining stored/gossip addresses. If the removed address would
    be the last one stored for the peer, behaviour depends on whether
    open channels exist: with no open channels the LinkNode entry is
    deleted automatically; with open channels the request is refused
    unless `--force` is also set. Note that after `--forget_address
    --force` removes the last stored address for a channel peer, the
    peer stays in the persistent-reconnect set — the peer-termination
    watcher falls back to the peer's NodeAnnouncement addresses for
    reconnect. `--forget_node` is the way to also stop reconnecting
    from our side (until lnd next restarts). Even with `--forget_address
    --force`, a channel peer is only truly orphaned when it also has
    no NodeAnnouncement.
  * `--force` on its own is rejected. `--forget_node` and `--forget_address`
    are mutually exclusive.

Response Status carries a warning when `--forget_address` triggers the
last-address LinkNode delete.

DB helper:

  * new `channeldb.LinkNodeDB.RemoveAddressForPeer(pub, addr, allowLast)`
    returning `(deletedEntry bool, err error)`.
  * new sentinel errors: `ErrNodeAddressNotFound` (address not stored
    for peer), `ErrLastPeerAddress` (refusing to remove last address
    without `allowLast`).

Server:

  * `DisconnectPeer` signature grows `forgetNode` and `force` parameters.
  * new `ForgetPeerAddress` method handles the per-address case,
    including the "disconnect if removed address is current" behaviour.
  * all internal callers of `DisconnectPeer` updated to pass
    `false, false`.

Proto: `DisconnectPeerRequest` gains `forget_node` (2), `force` (3), and
`forget_address` (4). `DisconnectPeerResponse.Status` reuses its
existing type.

itest: `disconnect forget` covers all branches — per-address removal,
last-address gating, `--forget_node` with and without `--force` against a
channel peer, and both error cases (`--force` alone; `--forget_node` +
`--forget_address`).

Depends on lightningnetwork#10975 (which depends on lightningnetwork#10973): the itest seeds a second
stored address by reconnecting Alice on Bob's alternate listener, which
`remembered_addresses` field added in lightningnetwork#10973.

Contributes to lightningnetwork#10870.
Contributes to lightningnetwork#10871.
ZZiigguurraatt added a commit to ZZiigguurraatt/lnd that referenced this pull request Jul 16, 2026
Extends `lncli disconnect` with two ways to prune a peer's stored
LinkNode state:

  * `--forget_node` for whole-peer removal.
  * `--forget_address` for surgical, per-address removal.

Both operations are gated by a shared `--force` flag when the requested
action would lose meaningful state:

  * `--forget_node` drops the live connection (base `disconnect` behavior)
    and additionally deletes the peer's LinkNode entry. The LinkNode
    delete is refused if open channels still exist with the peer unless
    `--force` is also set.
  * `--forget_address <host:port>` removes a single stored address from
    the peer's LinkNode entry. The live connection is left in place
    unless the removed address happens to be the currently-connected
    one, in which case the connection is dropped so lnd can re-dial via
    the remaining stored/gossip addresses. If the removed address would
    be the last one stored for the peer, behaviour depends on whether
    open channels exist: with no open channels the LinkNode entry is
    deleted automatically; with open channels the request is refused
    unless `--force` is also set. Note that after `--forget_address
    --force` removes the last stored address for a channel peer, the
    peer stays in the persistent-reconnect set — the peer-termination
    watcher falls back to the peer's NodeAnnouncement addresses for
    reconnect. `--forget_node` is the way to also stop reconnecting
    from our side (until lnd next restarts). Even with `--forget_address
    --force`, a channel peer is only truly orphaned when it also has
    no NodeAnnouncement.
  * `--force` on its own is rejected. `--forget_node` and `--forget_address`
    are mutually exclusive.

Response Status carries a warning when `--forget_address` triggers the
last-address LinkNode delete.

DB helper:

  * new `channeldb.LinkNodeDB.RemoveAddressForPeer(pub, addr, allowLast)`
    returning `(deletedEntry bool, err error)`.
  * new sentinel errors: `ErrNodeAddressNotFound` (address not stored
    for peer), `ErrLastPeerAddress` (refusing to remove last address
    without `allowLast`).

Server:

  * `DisconnectPeer` signature grows `forgetNode` and `force` parameters.
  * new `ForgetPeerAddress` method handles the per-address case,
    including the "disconnect if removed address is current" behaviour.
  * all internal callers of `DisconnectPeer` updated to pass
    `false, false`.

Proto: `DisconnectPeerRequest` gains `forget_node` (2), `force` (3), and
`forget_address` (4). `DisconnectPeerResponse.Status` reuses its
existing type.

itest: `disconnect forget` covers all branches — per-address removal,
last-address gating, `--forget_node` with and without `--force` against a
channel peer, and both error cases (`--force` alone; `--forget_node` +
`--forget_address`).

Depends on lightningnetwork#10975 (which depends on lightningnetwork#10973): the itest seeds a second
stored address by reconnecting Alice on Bob's alternate listener, which
lightningnetwork#10975's auto-persist captures into LinkNode; it also reads the
`remembered_addresses` field added in lightningnetwork#10973.

Contributes to lightningnetwork#10871.
ZZiigguurraatt added a commit to ZZiigguurraatt/lnd that referenced this pull request Jul 16, 2026
Extends `lncli disconnect` with two ways to prune a peer's stored
LinkNode state:

  * `--forget_node` for whole-peer removal.
  * `--forget_address` for surgical, per-address removal.

Both operations are gated by a shared `--force` flag when the requested
action would lose meaningful state:

  * `--forget_node` drops the live connection (base `disconnect` behavior)
    and additionally deletes the peer's LinkNode entry. The LinkNode
    delete is refused if open channels still exist with the peer unless
    `--force` is also set.
  * `--forget_address <host:port>` removes a single stored address from
    the peer's LinkNode entry. The live connection is left in place
    unless the removed address happens to be the currently-connected
    one, in which case the connection is dropped so lnd can re-dial via
    the remaining stored/gossip addresses. If the removed address would
    be the last one stored for the peer, behaviour depends on whether
    open channels exist: with no open channels the LinkNode entry is
    deleted automatically; with open channels the request is refused
    unless `--force` is also set. Note that after `--forget_address
    --force` removes the last stored address for a channel peer, the
    peer stays in the persistent-reconnect set — the peer-termination
    watcher falls back to the peer's NodeAnnouncement addresses for
    reconnect. `--forget_node` is the way to also stop reconnecting
    from our side (until lnd next restarts). Even with `--forget_address
    --force`, a channel peer is only truly orphaned when it also has
    no NodeAnnouncement.
  * `--force` on its own is rejected. `--forget_node` and `--forget_address`
    are mutually exclusive.

Response Status carries a warning when `--forget_address` triggers the
last-address LinkNode delete.

DB helper:

  * new `channeldb.LinkNodeDB.RemoveAddressForPeer(pub, addr, allowLast)`
    returning `(deletedEntry bool, err error)`.
  * new sentinel errors: `ErrNodeAddressNotFound` (address not stored
    for peer), `ErrLastPeerAddress` (refusing to remove last address
    without `allowLast`).

Server:

  * `DisconnectPeer` signature grows `forgetNode` and `force` parameters.
  * new `ForgetPeerAddress` method handles the per-address case,
    including the "disconnect if removed address is current" behaviour.
  * all internal callers of `DisconnectPeer` updated to pass
    `false, false`.

Proto: `DisconnectPeerRequest` gains `forget_node` (2), `force` (3), and
`forget_address` (4). `DisconnectPeerResponse.Status` reuses its
existing type.

itest: `disconnect forget` covers all branches — per-address removal,
last-address gating, `--forget_node` with and without `--force` against a
channel peer, and both error cases (`--force` alone; `--forget_node` +
`--forget_address`).

Depends on lightningnetwork#10975 (which depends on lightningnetwork#10973): the itest seeds a second
stored address by reconnecting Alice on Bob's alternate listener, which
`remembered_addresses` field added in lightningnetwork#10973.

Contributes to lightningnetwork#10871.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

severity-critical Requires expert review - security/consensus critical

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant