From ddffca9c4b70e38d8d98b5ace0b29977507fb69b Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Tue, 22 Sep 2026 22:58:48 +0000 Subject: [PATCH 1/2] Stop the flooded dedup-cache tests depending on host speed The two tests that fill the lookup dedup cache with 4096 requests ran with the default 10 s expiry, and the handler stamps each entry with the wall clock. On a loaded host the flood can take longer than that, so the earliest entries were purged before the test read the cache: the eviction test failed its own precondition, and the availability test passed without ever facing a full cache. Both now build the node with a one-day expiry, which leaves expiry out of what they measure, and the availability test checks that the cache is full before relying on it. --- src/node/tests/discovery.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/node/tests/discovery.rs b/src/node/tests/discovery.rs index d55f4f45..2bf4b64c 100644 --- a/src/node/tests/discovery.rs +++ b/src/node/tests/discovery.rs @@ -670,12 +670,25 @@ fn register_peers(node: &mut Node, count: usize) -> Vec { .collect() } +/// A node whose dedup entries cannot age out while a flood test runs. +/// +/// The handler stamps each entry with the wall clock and purges entries older +/// than `recent_expiry_secs` (10 s by default) on every arrival. The flood +/// tests below measure capacity, not expiry, and a 4096-request flood on a +/// loaded host can take longer than the default, so the earliest entries +/// would be purged before the test reads the cache. +fn unexpiring_node() -> Node { + let mut config = Config::new(); + config.node.lookup.recent_expiry_secs = 86_400; + make_node_with(config) +} + #[tokio::test] async fn test_a_full_dedup_cache_admits_the_new_request_by_evicting_the_oldest() { // A full cache used to drop the arriving request, which let one peer // spend 4096 fresh request_ids and stop the node forwarding anyone // else's lookups until the entries aged out. - let mut node = make_node(); + let mut node = unexpiring_node(); let from = make_node_addr(0xAA); flood_requests(&mut node, &from, 1, MAX_RECENT_LOOKUP_REQUESTS as u64).await; @@ -746,11 +759,16 @@ async fn test_a_node_whose_dedup_cache_is_flooded_still_answers_a_lookup_for_its // The availability claim. Filling the cache used to make the node // unresolvable, because the cache-full drop sat ahead of the check for // whether the request names us. - let mut node = make_node(); + let mut node = unexpiring_node(); let flooder = make_node_addr(0xAA); let other = make_node_addr(0xAB); flood_requests(&mut node, &flooder, 1, MAX_RECENT_LOOKUP_REQUESTS as u64).await; + assert_eq!( + node.lookup.recent_requests.len(), + MAX_RECENT_LOOKUP_REQUESTS, + "precondition: the cache is full, or the lookup below is not tested against a flood" + ); let my_addr = *node.node_addr(); let payload = lookup_request_payload(u64::MAX, &my_addr); From c5e070f98357abb877ce923ec3357e0963711990 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Tue, 22 Sep 2026 22:59:09 +0000 Subject: [PATCH 2/2] Correct the rekey tick comments to match the retirement phase The poll_rekey doc still said it reproduced the earlier priority and phase grouping exactly, and the comment at its call site listed three phases and described the snapshots as unchanged. The responder hold added a retirement phase between drains and initiations, and two snapshot fields, so both comments now describe the four phases as they are. --- src/node/handlers/rekey.rs | 15 ++++++++------- src/proto/fmp/core.rs | 5 ++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/node/handlers/rekey.rs b/src/node/handlers/rekey.rs index b9725494..f9afc00e 100644 --- a/src/node/handlers/rekey.rs +++ b/src/node/handlers/rekey.rs @@ -126,13 +126,14 @@ impl Node { }; // The shell snapshots each healthy peer's rekey ages/flags (every clock - // read resolved here); the core decides cutover/drain/trigger with no - // clock, phase-grouped to preserve the pre-refactor execution order. - // The batch `poll_rekey` + snapshots STAY SHELL-SIDE and BYTE-UNCHANGED: - // the cross-peer phase-grouping (all Cutover → all Drain → - // all InitiateRekey) governs the shared `index_allocator` free-then-alloc - // SEQUENCE that appears on the wire. The machine must NOT re-poll; it - // CONSUMES each decided `ConnAction` in the same order the batch returned. + // read resolved here); the core decides cutover, drain, retirement and + // trigger with no clock and returns the actions phase-grouped: all + // Cutover, then all Drain, then all RetirePending, then all + // InitiateRekey. That grouping fixes the shared `index_allocator` + // free-then-allocate sequence that appears on the wire, so the batch + // `poll_rekey` call stays here in the shell. The machine must NOT + // re-poll; it CONSUMES each decided `ConnAction` in the order the batch + // returned. let snapshots = self.rekey_peers(); for action in self.fmp.poll_rekey(snapshots, &cfg) { match action { diff --git a/src/proto/fmp/core.rs b/src/proto/fmp/core.rs index dbbc6d4e..f1069528 100644 --- a/src/proto/fmp/core.rs +++ b/src/proto/fmp/core.rs @@ -525,8 +525,7 @@ impl Fmp { } /// Decide the per-tick rekey choreography for the healthy peers the shell - /// snapshotted. Reproduces the pre-refactor priority and phase grouping - /// exactly: + /// snapshotted, in this priority: /// /// - **Cutover** takes precedence: a peer with a pending session this node /// initiated and no in-flight rekey cuts over and is considered for @@ -538,7 +537,7 @@ impl Fmp { /// trigger fires when the peer is neither mid-rekey, dampened, nor /// holding a pending session, and its jittered time threshold or send /// counter is reached. A draining peer can thus both drain and - /// re-trigger in the same tick, as before. + /// re-trigger in the same tick. /// /// Actions are returned phase-grouped (all cutovers, then all drains, then /// all retirements, then all rekey initiations) to preserve the global