From: Matt Corallo <git@bluematt.me>
Date: Mon, 14 Oct 2024 16:54:56 +0000 (+0000)
Subject: Don't bump the `next_node_counter` when using a removed counter
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=0c0cb6fccbb426924f21915053b0bc93ca287e74;p=rust-lightning

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.
---

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,