Skip to content

Commit

Permalink
Don't bump the next_node_counter when using a removed counter
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
TheBlueMatt committed Oct 16, 2024
1 parent 46d8a0d commit 0c0cb6f
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions lightning/src/routing/gossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 0c0cb6f

Please sign in to comment.