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

[Bartok9]

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

• Salvage of design discussion on Update peer socket address when peer already existsRefresh the stored address if an existing peer is added again #735 and peer-store plot of fix: upsert peer address on connect #801
• Not a docs/link nit — behavior fix + regression tests after docs: point rust_crate README link at the ldk-node crate page #995 first-time trivial bar

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]

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.

[Bartok9]

The failing build jobs are the known-flaky channel_full_cycle_0conf integration test (tracked in #984 — panics with unknown splice funding txid), not related to this change. This PR only touches src/peer_store.rs; the other lint/build/bindings jobs are green. Happy to rebase or re-run once a maintainer kicks CI. Thanks!

[Bartok9]

Re-triggered CI after check-cln flake (test_disconnect_during_payment timed out waiting for ChannelReady at 60s). Unrelated to this PR (only touches src/peer_store.rs); sibling CLN jobs on recent main are green. Happy to re-run again if it flaps.

[Bartok9]

Doctor triage: build-and-test (0FC Integration Tests) fails with a reorg_test panic (reorg_depth = 1, force_close = true), but all other build-and-test matrices (HRN, PostgreSQL, VSS, VSS No-Auth) and all other CI jobs pass. This looks like a timing-sensitive flake in reorg_test. I don't have permissions to re-run CI on this repo. Worth a manual re-run if it reproduces.

[Bartok9]

Rebased onto current main (516f8cb) so this picks up #1040 (Keep Python README within package builds). The previous tip failed check-python on hatchling (readme = "../../README.md"); that was staleness, not the peer_store change. Diff is still only src/peer_store.rs.

[Jolah1]

ACK e50cf35. The upsert behavior and persistence ordering look correct. I ran the focused peer-store tests locally; all 5 passed.

[Bartok9]

Doctor triage (2026-08-23): check-python fails with a panic in src/logger.rs (Failed to open log file) during Python integration tests. This is unrelated to the peer_store.rs changes — main CI passes check-python and the Rust tests pass locally. Appears to be a CI infra/flake issue; re-run needed.

[Bartok9]

Triage: check-python panic in src/logger.rs (log file NotFound) is CI infra/flake, not caused by src/peer_store.rs changes. All other checks pass. Cannot self-rerun upstream workflow (READ-only). Leaving for maintainer re-run or next CI cycle.

[tnull]

Could we avoid cloning the entire peer map here? We can temporarily apply
the update while holding the write lock, encode it, and then restore the
original state. Readers cannot observe the temporary value because they
need the same lock.

After persistence succeeds, we apply the update permanently. This avoids
allocating and cloning an O(n) second map while retaining the existing
persistence-first semantics.

Suggested change

	let data = {
	let mut locked_peers = self.peers.write().expect("lock");
	if locked_peers.contains_key(&peer_info.node_id) {
	return Ok(());
	let locked_peers = self.peers.read().expect("lock");
	if let Some(existing) = locked_peers.get(&peer_info.node_id) {
	if existing.address == peer_info.address {
	return Ok(());
	}
	}
	locked_peers.insert(peer_info.node_id, peer_info);
	PeerStoreSerWrapper(&locked_peers).encode()
	let mut updated_peers = locked_peers.clone();
	updated_peers.insert(peer_info.node_id, peer_info.clone());
	PeerStoreSerWrapper(&updated_peers).encode()
	};
	self.persist_peers(data).await
	self.persist_peers(data).await?;
	self.peers.write().expect("lock").insert(peer_info.node_id, peer_info);
	Ok(())
	let data = {
	let mut locked_peers = self.peers.write().expect("lock");
	if let Some(existing) = locked_peers.get(&peer_info.node_id) {
	if existing.address == peer_info.address {
	return Ok(());
	}
	}
	// Temporarily apply the update so it is included in the
	// serialized representation.
	let previous = locked_peers.insert(peer_info.node_id, peer_info.clone());
	let data = PeerStoreSerWrapper(&locked_peers).encode();
	// Restore the original in-memory state before persistence.
	match previous {
	Some(previous) => {
	locked_peers.insert(peer_info.node_id, previous);
	},
	None => {
	locked_peers.remove(&peer_info.node_id);
	},
	}
	data
	};
	self.persist_peers(data).await?;
	// Persistence succeeded, so apply the update permanently.
	self.peers.write().expect("lock").insert(peer_info.node_id, peer_info);
	Ok(())

[Bartok9]

Thanks @tnull — applied your suggestion: temporary write-lock insert → encode → restore, then permanent insert only after successful persist. Pushed in the latest tip. Avoids the O(n) clone while keeping persistence-first semantics.