diff --git a/src/node/tests/spanning_tree.rs b/src/node/tests/spanning_tree.rs index 70f31aa..39bd743 100644 --- a/src/node/tests/spanning_tree.rs +++ b/src/node/tests/spanning_tree.rs @@ -852,3 +852,103 @@ async fn test_rejects_tree_announce_with_inconsistent_root() { cleanup_nodes(&mut nodes).await; } + +/// A peer whose TreeAnnounce advertises a root worse (higher NodeAddr) than +/// ours must be answered with our own current TreeAnnounce, so a node that has +/// fallen back to self-root (e.g. because its parent's attaching announce was +/// lost) can re-attach without waiting for the parent's slow periodic +/// re-broadcast. +/// +/// This reproduces, at the unit level, the single-uplink-leaf convergence +/// wedge: a node with exactly one peer has its periodic parent re-evaluation +/// disabled, so if it misses its parent's attaching announce it is stranded as +/// a self-root until the parent's next periodic re-broadcast. The receive-path +/// re-push makes recovery event-driven: the stranded node's self-root announce +/// provokes its better-rooted parent to echo its real position straight back. +#[tokio::test] +async fn test_tree_announce_repushed_on_root_disagreement() { + // Converge a 2-node tree: one node is root, the other its child. + let mut nodes = run_tree_test(2, &[(0, 1)], false).await; + + // Identify which index is the root (smallest NodeAddr) and which is the + // attached child — the topology is address-order dependent. + let root_idx = if nodes[0].node.tree_state().is_root() { + 0 + } else { + 1 + }; + let child_idx = 1 - root_idx; + + let root_addr = *nodes[root_idx].node.node_addr(); + let child_addr = *nodes[child_idx].node.node_addr(); + + // Sanity: before the disturbance the child is attached to the root. + assert_eq!( + nodes[child_idx].node.tree_state().root(), + &root_addr, + "child should start attached to the root" + ); + assert!( + !nodes[child_idx].node.tree_state().is_root(), + "child should not start as its own root" + ); + + // Simulate the lost attaching announce: force the child back to self-root + // AND drop its cached view of the root's tree position, as it would be if + // it had never processed the root's attaching announce. The child's + // advertised root is now itself, disagreeing with the root's view, and its + // own periodic re-evaluation cannot recover it (single peer). + { + // Field-split borrow (tree_state mut + identity immut) mirrors the + // production re-sign sites; Identity is not Clone here. + let n = &mut nodes[child_idx].node; + n.tree_state.become_root(); + n.tree_state.remove_peer(&root_addr); + n.tree_state.sign_declaration(&n.identity).unwrap(); + } + assert!(nodes[child_idx].node.tree_state().is_root()); + + // Drain any packets the disturbance may have queued so the counters below + // isolate the re-push. + let _ = drain_all_packets(&mut nodes, false).await; + + // The stranded child announces its (self-root) position to its only peer, + // the root. The root's advertised root differs from the child's, so the + // root must re-push its current announce back. + let root_sent_before = nodes[root_idx].node.stats().tree.sent; + nodes[child_idx] + .node + .send_tree_announce_to_peer(&root_addr) + .await + .unwrap(); + + // Deliver the child's announce to the root and let the resulting re-push + // (and the child's processing of it) settle. + let _ = drain_all_packets(&mut nodes, false).await; + + // The root emitted at least one TreeAnnounce in response to the child's + // root-disagreeing announce (the receive-path re-push). + let root_sent_after = nodes[root_idx].node.stats().tree.sent; + assert!( + root_sent_after > root_sent_before, + "root should re-push its TreeAnnounce when a peer advertises a worse root \ + (sent before={}, after={})", + root_sent_before, + root_sent_after + ); + + // End state: the child has re-attached to the root rather than remaining a + // stranded self-root. + assert!( + !nodes[child_idx].node.tree_state().is_root(), + "child should have re-attached, not remained a self-root" + ); + assert_eq!( + nodes[child_idx].node.tree_state().root(), + &root_addr, + "child should have re-converged to the root" + ); + let _ = child_addr; + + cleanup_nodes(&mut nodes).await; +} diff --git a/src/node/tree.rs b/src/node/tree.rs index c5f42e1..3e6a64d 100644 --- a/src/node/tree.rs +++ b/src/node/tree.rs @@ -216,6 +216,41 @@ impl Node { "Processed TreeAnnounce" ); + // Re-push our current position when the announcing peer advertises a + // strictly worse (higher NodeAddr) root than ours. Root election is + // smallest-NodeAddr-wins, so a peer on a higher root has a stale or + // pre-attachment view and can attach to (or re-attach through) us; + // reply with our current declaration so it does so without waiting + // for the next periodic re-broadcast cadence. + // + // Only the better-rooted side echoes. If the peer's root is lower + // (better) than ours, WE are the stale side: the peer would ignore + // our worse root anyway, and we converge via the parent re-evaluation + // below, so echoing back is pure waste — and during a root change or + // partition merge it would double announce traffic in the learning + // direction. Equal roots are already converged. Restricting to `>` + // keeps the echo to the one direction that helps. + // + // This closes a convergence wedge on a single-uplink node: its only + // peer pushes the attaching announce once at promotion time, and if + // that datagram is lost the single-uplink node cannot self-correct + // (its own periodic parent re-evaluation is disabled below two peers) + // and is stranded as a self-root until the parent's next periodic + // re-broadcast (~reeval_interval_secs later). Echoing on root + // disagreement makes tree-position exchange self-healing on the + // receive path and is naturally bounded by the per-peer 500 ms + // tree-announce rate limiter, so it does not storm during normal + // convergence (it stops as soon as the peer adopts our root). + if *announce.ancestry.root_id() > *self.tree_state.root() + && let Err(e) = self.send_tree_announce_to_peer(from).await + { + debug!( + peer = %self.peer_display_name(from), + error = %e, + "Failed to re-push TreeAnnounce on root disagreement" + ); + } + // Bloom filter exchange initiation is handled at handshake completion // ([handshake.rs] mark_update_needed on the new peer) and on actual // content changes via [bloom.rs::handle_filter_announce]'s