From 0c0cb6fccbb426924f21915053b0bc93ca287e74 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 14 Oct 2024 16:54:56 +0000 Subject: [PATCH] Don't bump the `next_node_counter` when using a removed counter If we manage to pull a `node_counter` from `removed_node_counters` for reuse, `add_channel_between_nodes` would `unwrap_or` with the `next_node_counter`-incremented value. This visually looks right, except `unwrap_or` is always called, causing us to always increment `next_node_counter` even if we don't use it. This will result in the `node_counter`s always growing any time we add a new node to our graph, leading to somewhat larger memory usage when routing and a debug assertion failure in `test_node_counter_consistency`. The fix is trivial, this is what `unwrap_or_else` is for. --- lightning/src/routing/gossip.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index 47d80d86c..9d85a6c58 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -2077,9 +2077,9 @@ where }, IndexedMapEntry::Vacant(node_entry) => { let mut removed_node_counters = self.removed_node_counters.lock().unwrap(); - **chan_info_node_counter = removed_node_counters - .pop() - .unwrap_or(self.next_node_counter.fetch_add(1, Ordering::Relaxed) as u32); + **chan_info_node_counter = removed_node_counters.pop().unwrap_or_else(|| { + self.next_node_counter.fetch_add(1, Ordering::Relaxed) as u32 + }); node_entry.insert(NodeInfo { channels: vec![short_channel_id], announcement_info: None, -- 2.39.5