Repository navigation
fix(peer_store): upsert peer address when re-adding known peer - #1175
Open
vincenzopalazzo wants to merge 1 commit into
Open
vincenzopalazzo wants to merge 1 commit into
vincenzopalazzo wants to merge 1 commit into
Conversation
PeerStore::add_peer returned early when the pubkey was already stored, so Node::connect(..., persist=true) with a new address silently kept the stale address used for reconnection (see lightningdevkit#700). Split add_peer into two methods sharing a persist-then-apply helper, following the maintainer review on lightningdevkit#1002: - upsert_peer updates the stored address on re-add. Used where the address is explicitly supplied: Node::connect with persist set, open_channel, and the LSPS2 payment path. - add_peer_if_missing only stores unknown peers. Used for the gossip-derived address of inbound channel counterparties, which must not override an explicitly stored address. In-memory state is only mutated after a successful write, matching remove_peer. Following the review suggestion on lightningdevkit#1002, avoid cloning the entire peer map: apply the update temporarily under the write lock for serialization, restore it, persist, then apply it permanently. Readers cannot observe the temporary value as they need the same lock. Re-adding an unchanged peer is a no-op. remove_peer is switched to the same pattern for consistency. Addresses the peer-store side of lightningdevkit#700. The liquidity-source side (NodeBuilder/Liquidity add_liquidity_source) is deliberately left out here; it is tracked in lightningdevkit#1147. Supersedes lightningdevkit#1002 and lightningdevkit#1115. Co-authored-by: Bartok <259807879+Bartok9@users.noreply.github.com> Assisted-by: goose (AI)
|
I've assigned @tnull as a reviewer! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #1002 and #1115. Addresses the peer-store side of #700; the
liquidity-source side is tracked in #1147.
Problem
PeerStore::add_peerreturned early when the pubkey was alreadystored, so
Node::connect(node_id, new_address, persist = true)silentlykept the stale address for reconnection.