mirror of
https://github.com/jmcorgan/fips.git
synced 2026-08-11 17:17:54 +00:00
Merge branch 'master' into next: fipstop TUI overhaul and disconnect notify-peer fix
This commit is contained in:
+39
-17
@@ -1616,9 +1616,6 @@ impl Node {
|
||||
/// Best-effort: send failures are logged and ignored since the transport
|
||||
/// may already be degraded. This runs before transports are shut down.
|
||||
async fn send_disconnect_to_all_peers(&mut self, reason: DisconnectReason) {
|
||||
let disconnect = Disconnect::new(reason);
|
||||
let plaintext = disconnect.encode();
|
||||
|
||||
// Collect node_addrs to avoid borrow conflict with send helper
|
||||
let peer_addrs: Vec<NodeAddr> = self
|
||||
.peers
|
||||
@@ -1637,24 +1634,41 @@ impl Node {
|
||||
|
||||
let mut sent = 0usize;
|
||||
for node_addr in &peer_addrs {
|
||||
match self
|
||||
.send_encrypted_link_message(node_addr, &plaintext)
|
||||
.await
|
||||
{
|
||||
Ok(()) => sent += 1,
|
||||
Err(e) => {
|
||||
debug!(
|
||||
peer = %self.peer_display_name(node_addr),
|
||||
error = %e,
|
||||
"Failed to send disconnect (transport may be down)"
|
||||
);
|
||||
}
|
||||
if self.send_disconnect_to_peer(node_addr, reason).await {
|
||||
sent += 1;
|
||||
}
|
||||
}
|
||||
|
||||
info!(sent, total = peer_addrs.len(), reason = %reason, "Sent disconnect notifications");
|
||||
}
|
||||
|
||||
/// Send a Disconnect notification to a single peer.
|
||||
///
|
||||
/// Best-effort: a send failure (peer already gone, transport down) is
|
||||
/// logged and swallowed so callers can proceed with teardown regardless.
|
||||
/// Returns `true` if the message was sent successfully.
|
||||
async fn send_disconnect_to_peer(
|
||||
&mut self,
|
||||
node_addr: &NodeAddr,
|
||||
reason: DisconnectReason,
|
||||
) -> bool {
|
||||
let plaintext = Disconnect::new(reason).encode();
|
||||
match self
|
||||
.send_encrypted_link_message(node_addr, &plaintext)
|
||||
.await
|
||||
{
|
||||
Ok(()) => true,
|
||||
Err(e) => {
|
||||
debug!(
|
||||
peer = %self.peer_display_name(node_addr),
|
||||
error = %e,
|
||||
"Failed to send disconnect (transport may be down)"
|
||||
);
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn static_peer_addresses(&self, peer_config: &PeerConfig) -> Vec<PeerAddress> {
|
||||
peer_config
|
||||
.addresses_by_priority()
|
||||
@@ -2644,8 +2658,8 @@ impl Node {
|
||||
|
||||
/// Disconnect a peer via the control API.
|
||||
///
|
||||
/// Removes the peer and suppresses auto-reconnect.
|
||||
pub(crate) fn api_disconnect(&mut self, npub: &str) -> Result<serde_json::Value, String> {
|
||||
/// Notifies the peer, removes it locally, and suppresses auto-reconnect.
|
||||
pub(crate) async fn api_disconnect(&mut self, npub: &str) -> Result<serde_json::Value, String> {
|
||||
let peer_identity =
|
||||
PeerIdentity::from_npub(npub).map_err(|e| format!("invalid npub '{npub}': {e}"))?;
|
||||
let node_addr = *peer_identity.node_addr();
|
||||
@@ -2654,6 +2668,14 @@ impl Node {
|
||||
return Err(format!("peer not found: {npub}"));
|
||||
}
|
||||
|
||||
// Notify the peer before we tear down the link, so it drops its own
|
||||
// session and re-handshakes symmetrically rather than holding a stale
|
||||
// session that never re-emits its tree/filter announcements. The link
|
||||
// must still exist for the send, so this runs before removal.
|
||||
// Best-effort: a send failure must not block the local teardown.
|
||||
self.send_disconnect_to_peer(&node_addr, DisconnectReason::ConfigurationChange)
|
||||
.await;
|
||||
|
||||
// Remove the peer (full cleanup: sessions, indices, links, tree, bloom)
|
||||
self.remove_active_peer(&node_addr);
|
||||
|
||||
|
||||
@@ -1561,6 +1561,29 @@ impl Node {
|
||||
})
|
||||
.collect();
|
||||
|
||||
// Per-configured-transport-type peer counts (`show_status`). Seed every
|
||||
// configured transport type at 0 so an idle-but-configured type stays
|
||||
// visible, then tally the peers whose active link rides that type.
|
||||
let mut transport_peer_counts: std::collections::BTreeMap<String, usize> =
|
||||
std::collections::BTreeMap::new();
|
||||
for id in self.transport_ids() {
|
||||
if let Some(handle) = self.get_transport(id) {
|
||||
transport_peer_counts
|
||||
.entry(handle.transport_type().name.to_string())
|
||||
.or_insert(0);
|
||||
}
|
||||
}
|
||||
for peer in self.peers() {
|
||||
if let Some(link) = self.get_link(&peer.link_id())
|
||||
&& let Some(handle) = self.get_transport(&link.transport_id())
|
||||
{
|
||||
*transport_peer_counts
|
||||
.entry(handle.transport_type().name.to_string())
|
||||
.or_insert(0) += 1;
|
||||
}
|
||||
}
|
||||
|
||||
let tree = self.tree_state();
|
||||
let snapshot = crate::control::snapshot::StatsSnapshot {
|
||||
history: std::sync::Arc::new(self.stats_history.clone()),
|
||||
estimated_mesh_size: self.estimated_mesh_size,
|
||||
@@ -1573,6 +1596,9 @@ impl Node {
|
||||
link_count: self.links.len(),
|
||||
transport_count: self.transports.len(),
|
||||
session_count: self.sessions.len(),
|
||||
root: *tree.root(),
|
||||
is_root: tree.is_root(),
|
||||
transport_peer_counts,
|
||||
peer_aliases: std::sync::Arc::new(self.peer_aliases.clone()),
|
||||
acl_status: self.peer_acl_status(),
|
||||
peer_meta: std::sync::Arc::new(peer_meta),
|
||||
@@ -1588,6 +1614,28 @@ impl Node {
|
||||
self.publish_entities_snapshot();
|
||||
}
|
||||
|
||||
/// Resolve the npub of the spanning-tree root for `show_tree`'s `root_npub`.
|
||||
///
|
||||
/// Resolution order: this node when it is root, then the root as a live
|
||||
/// authenticated peer (cryptographically attested npub), then the
|
||||
/// identity-cache, else `None`.
|
||||
pub(crate) fn resolve_root_npub(&self, tree: &crate::tree::TreeState) -> Option<String> {
|
||||
if tree.is_root() {
|
||||
return Some(self.npub());
|
||||
}
|
||||
let root_addr = tree.root();
|
||||
if let Some(peer) = self.get_peer(root_addr) {
|
||||
return Some(peer.npub());
|
||||
}
|
||||
for (addr, pubkey, _last_seen) in self.identity_cache_iter() {
|
||||
if addr == root_addr {
|
||||
let (xonly, _parity) = pubkey.x_only_public_key();
|
||||
return Some(crate::identity::encode_npub(&xonly));
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Project the Category-D derived/routing/cache state into a
|
||||
/// [`RoutingSnapshot`](crate::control::snapshot::RoutingSnapshot) and
|
||||
/// publish it via `ArcSwap`, so `show_tree` / `show_bloom` / `show_cache`
|
||||
@@ -1637,9 +1685,11 @@ impl Node {
|
||||
})
|
||||
.collect();
|
||||
let parent_addr = my_coords.parent_id();
|
||||
let root_npub = self.resolve_root_npub(tree);
|
||||
let tree_view = snap::TreeView {
|
||||
my_node_addr: *tree.my_node_addr(),
|
||||
root: *tree.root(),
|
||||
root_npub,
|
||||
is_root: tree.is_root(),
|
||||
depth: my_coords.depth(),
|
||||
my_coords: my_coords.entries().iter().map(|e| e.node_addr).collect(),
|
||||
@@ -1672,12 +1722,30 @@ impl Node {
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
// Uptree filter metrics: the last filter actually sent to the tree
|
||||
// parent (`record_sent_filter`), which is what the parent currently
|
||||
// holds for us. `None` for a root node (nothing sent uptree) or before
|
||||
// the first announce. The estimate is this node's whole subtree
|
||||
// (split-horizon), not the mesh.
|
||||
let (uptree_fill_ratio, uptree_estimated_count) = if tree.is_root() {
|
||||
(None, None)
|
||||
} else {
|
||||
match bloom.last_sent_filter(parent_addr) {
|
||||
Some(filter) => (
|
||||
Some(filter.fill_ratio()),
|
||||
filter.estimated_count(max_inbound_fpr),
|
||||
),
|
||||
None => (None, None),
|
||||
}
|
||||
};
|
||||
let bloom_view = snap::BloomView {
|
||||
own_node_addr: *self.node_addr(),
|
||||
is_leaf_only: self.is_leaf_only(),
|
||||
sequence: bloom.sequence(),
|
||||
leaf_dependents: bloom.leaf_dependents().iter().copied().collect(),
|
||||
peer_filters: bloom_peers,
|
||||
uptree_fill_ratio,
|
||||
uptree_estimated_count,
|
||||
};
|
||||
|
||||
// --- coord cache (show_cache, show_routing) ---
|
||||
@@ -1819,6 +1887,12 @@ impl Node {
|
||||
})
|
||||
.unwrap_or_default();
|
||||
|
||||
// Cold-start gate for effective_depth, mirroring `evaluate_parent`:
|
||||
// if any peer has an SRTT measurement, unmeasured peers are excluded
|
||||
// (their effective_depth is `None`); during cold start (no peer has
|
||||
// SRTT) every peer falls back to the default link cost of 1.0.
|
||||
let any_peer_has_srtt = self.peers().any(|p| p.has_srtt());
|
||||
|
||||
let peer_rows: Vec<snap::PeerRow> = self
|
||||
.peers()
|
||||
.map(|peer| {
|
||||
@@ -1855,6 +1929,17 @@ impl Node {
|
||||
.mmp()
|
||||
.map(|mmp| project_entity_mmp(&mmp.metrics, format!("{}", mmp.mode()), None));
|
||||
|
||||
// effective_depth = tree_depth + link_cost, the value
|
||||
// `evaluate_parent` ranks on. Computed only when the peer has
|
||||
// coords and passes the cold-start measurement gate.
|
||||
let effective_depth = peer.coords().and_then(|coords| {
|
||||
if any_peer_has_srtt && !peer.has_srtt() {
|
||||
None
|
||||
} else {
|
||||
Some(coords.depth() as f64 + peer.link_cost())
|
||||
}
|
||||
});
|
||||
|
||||
snap::PeerRow {
|
||||
node_addr,
|
||||
npub: peer.npub(),
|
||||
@@ -1872,6 +1957,7 @@ impl Node {
|
||||
transport_addr: peer.current_addr().map(|a| format!("{}", a)),
|
||||
link_info,
|
||||
tree_depth: peer.coords().map(|c| c.depth()),
|
||||
effective_depth,
|
||||
stats: snap::PeerLinkStats {
|
||||
packets_sent: stats.packets_sent,
|
||||
packets_recv: stats.packets_recv,
|
||||
@@ -2054,6 +2140,10 @@ impl Node {
|
||||
(Some(srtt), Some(setx)) => Some(setx * (1.0 + srtt / 100.0)),
|
||||
_ => None,
|
||||
};
|
||||
let trend = |dual: &crate::mmp::algorithms::DualEwma| {
|
||||
dual.initialized()
|
||||
.then(|| crate::control::queries::trend_label(dual.short(), dual.long()))
|
||||
};
|
||||
Some(snap::MmpSessionRow {
|
||||
remote: *addr,
|
||||
display_name: self.peer_display_name(addr),
|
||||
@@ -2065,6 +2155,11 @@ impl Node {
|
||||
smoothed_etx,
|
||||
srtt_ms,
|
||||
sqi,
|
||||
trends: snap::MmpSessionTrends {
|
||||
rtt_trend: trend(&metrics.rtt_trend),
|
||||
loss_trend: trend(&metrics.loss_trend),
|
||||
etx_trend: trend(&metrics.etx_trend),
|
||||
},
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
|
||||
@@ -325,6 +325,64 @@ async fn test_disconnect_clears_session() {
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
/// A manual (control-API) disconnect must notify the peer, not just tear
|
||||
/// down the local side.
|
||||
///
|
||||
/// Regression test: `api_disconnect` previously removed the peer locally but
|
||||
/// sent it no Disconnect message. The peer kept its session and never
|
||||
/// re-emitted its tree/filter announcements, so on reconnect it was never
|
||||
/// re-adopted as a child and its bloom filter was never recorded. The fix
|
||||
/// sends the disconnected peer a scoped Disconnect so both sides tear down
|
||||
/// symmetrically. This test drives `api_disconnect` on node 0 and verifies
|
||||
/// node 1 receives the notification and removes node 0.
|
||||
#[tokio::test]
|
||||
async fn test_api_disconnect_notifies_peer() {
|
||||
// Two-node topology: 0 -- 1.
|
||||
let edges = vec![(0, 1)];
|
||||
let mut nodes = run_tree_test(2, &edges, false).await;
|
||||
verify_tree_convergence(&nodes);
|
||||
|
||||
let node0_addr = *nodes[0].node.node_addr();
|
||||
let node1_addr = *nodes[1].node.node_addr();
|
||||
let node1_npub = nodes[1].node.npub();
|
||||
|
||||
// Both sides start with each other as a peer.
|
||||
assert!(
|
||||
nodes[0].node.get_peer(&node1_addr).is_some(),
|
||||
"Node 0 should have node 1 before disconnect"
|
||||
);
|
||||
assert!(
|
||||
nodes[1].node.get_peer(&node0_addr).is_some(),
|
||||
"Node 1 should have node 0 before disconnect"
|
||||
);
|
||||
|
||||
// Operator disconnects node 1 via the control API on node 0.
|
||||
nodes[0]
|
||||
.node
|
||||
.api_disconnect(&node1_npub)
|
||||
.await
|
||||
.expect("api_disconnect should succeed");
|
||||
|
||||
// Node 0 tore down its side immediately.
|
||||
assert!(
|
||||
nodes[0].node.get_peer(&node1_addr).is_none(),
|
||||
"Node 0 should have removed node 1 after api_disconnect"
|
||||
);
|
||||
|
||||
// The Disconnect notification must reach node 1.
|
||||
tokio::time::sleep(Duration::from_millis(50)).await;
|
||||
process_available_packets(&mut nodes).await;
|
||||
|
||||
// Node 1 must have torn down its side in response — proving the
|
||||
// notification was actually emitted and received.
|
||||
assert!(
|
||||
nodes[1].node.get_peer(&node0_addr).is_none(),
|
||||
"Node 1 should have removed node 0 after receiving the disconnect notification"
|
||||
);
|
||||
|
||||
cleanup_nodes(&mut nodes).await;
|
||||
}
|
||||
|
||||
/// Verify that different disconnect reasons are handled correctly.
|
||||
///
|
||||
/// Sends each reason code and verifies the peer is removed regardless.
|
||||
|
||||
Reference in New Issue
Block a user