From 06998d5730d21f8d7dfb858d2370b404a2724aa3 Mon Sep 17 00:00:00 2001 From: "Sera (Bartok9)" Date: Thu, 23 Jul 2026 08:40:18 -0400 Subject: [PATCH 1/4] fix(peer_store): upsert peer address when re-adding known peer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #700. Co-authored-by prior attempts: - #735 @chahat-101 (abandoned; incorporated maintainer direction from review) - #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). --- src/peer_store.rs | 111 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 105 insertions(+), 6 deletions(-) diff --git a/src/peer_store.rs b/src/peer_store.rs index 8345bf711..b2650b31f 100644 --- a/src/peer_store.rs +++ b/src/peer_store.rs @@ -42,17 +42,27 @@ where Self { peers, mutation_lock, kv_store, logger } } + /// Inserts or updates a peer entry. + /// + /// If the peer is already known with the same address, this is a no-op. If the peer is new or + /// the stored address changed (e.g. an LSP migrated hosts), the entry is updated and persisted. + /// In-memory state is only mutated after a successful store write, matching [`Self::remove_peer`]. pub(crate) async fn add_peer(&self, peer_info: PeerInfo) -> Result<(), Error> { let _guard = self.mutation_lock.lock().await; 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(()) } pub(crate) async fn remove_peer(&self, node_id: &PublicKey) -> Result<(), Error> { @@ -277,4 +287,93 @@ mod tests { assert_eq!(Err(Error::PersistenceFailed), peer_store.remove_peer(&node_id).await); assert_eq!(Some(peer_info), peer_store.get_peer(&node_id)); } + + #[tokio::test] + async fn peer_address_updated_on_readd() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let logger = Arc::new(TestLogger::new()); + let peer_store = PeerStore::new(Arc::clone(&store), Arc::clone(&logger)); + + let node_id = PublicKey::from_str( + "0276607124ebe6a6c9338517b6f485825b27c2dcc0b9fc2aa6a4c0df91194e5993", + ) + .unwrap(); + let old_address = SocketAddress::from_str("127.0.0.1:9738").unwrap(); + let new_address = SocketAddress::from_str("127.0.0.1:9739").unwrap(); + + peer_store.add_peer(PeerInfo { node_id, address: old_address.clone() }).await.unwrap(); + assert_eq!(peer_store.get_peer(&node_id), Some(PeerInfo { node_id, address: old_address })); + + // Re-adding the same peer with a new socket address must refresh the stored entry + // (regression for https://github.com/lightningdevkit/ldk-node/issues/700). + let updated = PeerInfo { node_id, address: new_address.clone() }; + peer_store.add_peer(updated.clone()).await.unwrap(); + assert_eq!(peer_store.get_peer(&node_id), Some(updated.clone())); + + let persisted_bytes = KVStore::read( + &*store, + PEER_INFO_PERSISTENCE_PRIMARY_NAMESPACE, + PEER_INFO_PERSISTENCE_SECONDARY_NAMESPACE, + PEER_INFO_PERSISTENCE_KEY, + ) + .await + .unwrap(); + let deser_peer_store = + PeerStore::read(&mut &persisted_bytes[..], (Arc::clone(&store), logger)).unwrap(); + assert_eq!(deser_peer_store.get_peer(&node_id), Some(updated)); + } + + #[tokio::test] + async fn peer_same_address_skips_persist() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let logger = Arc::new(TestLogger::new()); + let peer_store = PeerStore::new(Arc::clone(&store), Arc::clone(&logger)); + + let node_id = PublicKey::from_str( + "0276607124ebe6a6c9338517b6f485825b27c2dcc0b9fc2aa6a4c0df91194e5993", + ) + .unwrap(); + let address = SocketAddress::from_str("127.0.0.1:9738").unwrap(); + let peer_info = PeerInfo { node_id, address }; + + peer_store.add_peer(peer_info.clone()).await.unwrap(); + let first_bytes = KVStore::read( + &*store, + PEER_INFO_PERSISTENCE_PRIMARY_NAMESPACE, + PEER_INFO_PERSISTENCE_SECONDARY_NAMESPACE, + PEER_INFO_PERSISTENCE_KEY, + ) + .await + .unwrap(); + + // Identical re-add is a no-op for the store payload. + peer_store.add_peer(peer_info.clone()).await.unwrap(); + let second_bytes = KVStore::read( + &*store, + PEER_INFO_PERSISTENCE_PRIMARY_NAMESPACE, + PEER_INFO_PERSISTENCE_SECONDARY_NAMESPACE, + PEER_INFO_PERSISTENCE_KEY, + ) + .await + .unwrap(); + assert_eq!(first_bytes, second_bytes); + assert_eq!(peer_store.get_peer(&node_id), Some(peer_info)); + } + + #[tokio::test] + async fn add_peer_does_not_mutate_memory_if_persist_fails() { + let store: Arc = Arc::new(DynStoreWrapper(FailingStore)); + let logger = Arc::new(TestLogger::new()); + let peer_store = PeerStore::new(store, logger); + + let node_id = PublicKey::from_str( + "0276607124ebe6a6c9338517b6f485825b27c2dcc0b9fc2aa6a4c0df91194e5993", + ) + .unwrap(); + let peer_info = + PeerInfo { node_id, address: SocketAddress::from_str("127.0.0.1:9738").unwrap() }; + + assert_eq!(Err(Error::PersistenceFailed), peer_store.add_peer(peer_info.clone()).await); + assert_eq!(None, peer_store.get_peer(&node_id)); + } } From aac8f6b4c3caf46cddb14db09c994d3c2cbecd9d Mon Sep 17 00:00:00 2001 From: Bartok9 Date: Fri, 24 Jul 2026 08:37:41 -0400 Subject: [PATCH 2/4] ci: retrigger checks after flaky channel_full_cycle on self-hosted channel_full_cycle / channel_full_cycle_0conf_0reserve failed with splice wait / unknown splice funding txid on build (self-hosted, 1.85.0). Same family of failures appears on current upstream main CI; this PR only touches src/peer_store.rs. No code change. AI: assisted with Hermes/Grok (Nous). Review by Sera (agent_id=sera). From e59d73c98556e65117862e710fab1921cd9a6e63 Mon Sep 17 00:00:00 2001 From: "Sera (Bartok9)" Date: Sun, 26 Jul 2026 08:37:24 -0400 Subject: [PATCH 3/4] ci: retrigger checks after postgres lld bus-error flake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PostgreSQL Integration Tests failed linking with rust-lld bus error (LLVM crash, signal 7) on aac8f6b — not peer_store code. Original 06998d5 had postgres green but self-hosted 1.85.0 channel_full_cycle* flake. Empty retrigger only. AI: assisted with Hermes/Grok (Nous). Review by Sera (agent_id=sera). From 12faffccad92b8e7210c9a40640a54c0cbd844f4 Mon Sep 17 00:00:00 2001 From: Bartok9 Date: Sun, 26 Jul 2026 19:35:47 -0400 Subject: [PATCH 4/4] test: allow unused re-exports in shared test harness The shared `tests/common` module re-exports macros and helpers that are consumed by some but not all test binaries (e.g. reorg_test, probing_tests). Under `-D warnings` this now fails the build with unused_imports / unused_macros across those binaries. The module already carries `#![allow(dead_code)]` for exactly this shared-fixture reason; extend it to unused imports/macros so every test binary compiles again. --- tests/common/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 50e2b993c..458ef2622 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -7,6 +7,8 @@ #![cfg(any(test, cln_test, lnd_test, eclair_test, vss_test))] #![allow(dead_code)] +#![allow(unused_imports)] +#![allow(unused_macros)] pub(crate) mod external_node; pub(crate) mod logging;