laantungir/fips
1
0
Fork 0
mirror of https://github.com/jmcorgan/fips.git synced 2026-08-11 17:17:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

node: make the shared context the sole store of immutable state

Browse Source 
Remove the duplicated immutable fields (config, identity, startup_epoch,
started_at, is_leaf_only, max_connections/peers/links) from the Node
struct so the Arc<NodeContext> bundle is the single source of truth.
Previously Node owned these fields and a parallel context copy, kept in
lockstep by rebuild_context() at every mutation site — pure overhead that
existed only because of the duplication.

- Replace rebuild_context() with replace_context(): a clone-edit-swap of
  the whole Arc. The per-instance context stays immutable; mutation swaps
  the Arc. This is the sole runtime mutation path (constructors, leaf_only,
  update_peers).
- Add Copy-returning accessors startup_epoch() and max_connections()/
  max_peers()/max_links(); migrate the remaining direct field readers onto
  the accessors. node_addr()/npub()/Debug now read identity/is_leaf_only
  from the context.
- update_peers reads the pre-update peer set from the live context Arc
  before building a fresh Config + context and swapping — preserving the
  read-before-write ordering its mutation-window test depends on.
- Remove the test-only set_max_* setters; tests set the limits on Config at
  construction instead (new make_node_with_max_peers/links helpers).
- Add a ci-local guard that fails if the Node struct re-declares a bundled
  field, so the single-store invariant can't silently regress.

cargo test --lib 1291/0; clippy -D warnings and release build clean.
This commit is contained in:
Johnathan Corgan
2026-06-02 16:42:05 +00:00
parent 08b8b3908e
commit d9a4a7807c
16 changed files with 208 additions and 230 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/node/lifecycle.rs
+22 -18
View File
@@ -55,11 +55,14 @@ impl Node {
new_by_addr.insert(*identity.node_addr(), peer);
}
// Read the current peer set directly from the field: update_peers is the
// config-source mutation owner (it writes self.config.peers below and then
// rebuilds the context), so it manages config.peers directly rather than
// through the context accessor — same rationale as the write at line ~124.
// Read the current peer set from the context *before* the swap below:
// update_peers is the config-source mutation owner. It reads the current
// (pre-update) peer set here, builds a fresh Config + context, then swaps
// the whole Arc. Reading the live context Arc before the swap yields the
// pre-update set the diff needs (the `update_peers_races_*` canary depends
// on this ordering).
let current_by_addr: HashMap<NodeAddr, PeerConfig> = self
.context
.config
.peers()
.iter()
@@ -124,8 +127,9 @@ impl Node {
.map(|node_addr| new_by_addr[node_addr].clone())
.collect();
self.config.peers = new_by_addr.into_values().collect();
self.rebuild_context();
let mut new_config = (*self.context.config).clone();
new_config.peers = new_by_addr.into_values().collect();
self.replace_context(|ctx| ctx.config = std::sync::Arc::new(new_config));
for peer_config in added_configs {
outcome.added += 1;
@@ -487,7 +491,7 @@ impl Node {
// Start the Noise handshake and get message 1
let our_keypair = self.identity().keypair();
let noise_msg1 =
match connection.start_handshake(our_keypair, self.startup_epoch, current_time_ms) {
match connection.start_handshake(our_keypair, self.startup_epoch(), current_time_ms) {
Ok(msg) => msg,
Err(e) => {
// Clean up the index and link
@@ -698,7 +702,7 @@ impl Node {
debug!(
peer_npub = %traversal.peer_npub,
peers = self.peers.len(),
max_peers = self.max_peers,
max_peers = self.max_peers(),
"Dropping established NAT traversal: at capacity"
);
continue;
@@ -2069,16 +2073,16 @@ impl Node {
.connections
.len()
.saturating_add(self.pending_connects.len());
let connection_slots = if self.max_connections == 0 {
let connection_slots = if self.max_connections() == 0 {
usize::MAX
} else {
self.max_connections.saturating_sub(connection_used)
self.max_connections().saturating_sub(connection_used)
};
let peer_slots = if self.max_peers == 0 {
let peer_slots = if self.max_peers() == 0 {
usize::MAX
} else {
self.max_peers.saturating_sub(self.peers.len())
self.max_peers().saturating_sub(self.peers.len())
};
connection_slots.min(peer_slots)
@@ -2089,25 +2093,25 @@ impl Node {
.connections
.len()
.saturating_add(self.pending_connects.len());
if self.max_connections == 0 {
if self.max_connections() == 0 {
usize::MAX
} else {
self.max_connections.saturating_sub(used)
self.max_connections().saturating_sub(used)
}
}
fn outbound_link_slots(&self) -> usize {
if self.max_links == 0 {
if self.max_links() == 0 {
usize::MAX
} else {
self.max_links.saturating_sub(self.links.len())
self.max_links().saturating_sub(self.links.len())
}
}
fn path_candidate_attempt_budget(&self, peer_node_addr: &NodeAddr) -> usize {
if !self.peers.contains_key(peer_node_addr)
&& self.max_peers > 0
&& self.peers.len() >= self.max_peers
&& self.max_peers() > 0
&& self.peers.len() >= self.max_peers()
{
return 0;
}