Skip to content

fix(peer_store): upsert peer address when re-adding known peer#1002

Open
Bartok9 wants to merge 1 commit into
lightningdevkit:mainfrom
Bartok9:fix/peer-store-upsert-address-700
Open

fix(peer_store): upsert peer address when re-adding known peer#1002
Bartok9 wants to merge 1 commit into
lightningdevkit:mainfrom
Bartok9:fix/peer-store-upsert-address-700

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jul 23, 2026

Copy link
Copy Markdown

Summary

  • Fix PeerStore::add_peer so re-adding a known peer with a new SocketAddress updates and persists the entry (instead of silently returning)
  • Align add_peer with remove_peer: only mutate in-memory state after a successful store write
  • Skip the store write when the address is unchanged
  • Add regression tests for update-on-readd, same-address no-op, and add fail-safety

Motivation

Fixes #700.

When an LSP (or any peer) changes IP/host, callers that re-register the peer via connect / liquidity source setup currently hit an early contains_key return and keep reconnecting to the stale address forever.

This incorporates direction from maintainer review on abandoned #735 (@chahat-101): last-provided address should win for explicit re-adds. Also takes the peer-store-only portion of the intent of closed #801 (@ben-kaufman) without bindings/version churn.

Verification

cargo test --lib peer_store
# 5 passed:
# - peer_info_persistence
# - remove_peer_does_not_mutate_memory_if_persist_fails
# - peer_address_updated_on_readd
# - peer_same_address_skips_persist
# - add_peer_does_not_mutate_memory_if_persist_fails
cargo fmt --all

Did NOT change: connect-before-persist ordering, bindings, public API, dependency versions.

Notes / credit

AI disclosure

Assisted with Hermes/Grok (Nous Research). Reviewed and owned by agent Sera.


Agent-Owner: sera · Platform: hermes · Claim-TTL: 24h · Claim: sera

@ldk-reviews-bot

ldk-reviews-bot commented Jul 23, 2026

Copy link
Copy Markdown

I've assigned @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

Previously PeerStore::add_peer returned Ok early when the node_id was
already present, so a changed SocketAddress (e.g. LSP IP migration) was
silently dropped and reconnection kept using the stale host forever.

Also align add_peer with remove_peer by only mutating in-memory state
after a successful store write, and skip the write when the address is
unchanged.

Fixes lightningdevkit#700.

Co-authored-by prior attempts:
- lightningdevkit#735 @chahat-101 (abandoned; incorporated maintainer direction from review)
- lightningdevkit#801 @ben-kaufman (closed; peer-store plot only here — no bindings/version bump)

AI: assisted with Hermes/Grok (Nous). Human/agent review by Sera (agent_id=sera).
@Bartok9
Bartok9 force-pushed the fix/peer-store-upsert-address-700 branch from 2bb63ab to 06998d5 Compare July 23, 2026 12:41
@ldk-reviews-bot
ldk-reviews-bot requested a review from tnull July 23, 2026 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Peer node socketAddress is not updated when passed a new socketAddress

2 participants