From 2787062436ce8a34c06bcc09892ad592ffc1ecc9 Mon Sep 17 00:00:00 2001 From: Arjen <18398758+Origami74@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:13:39 +0100 Subject: [PATCH 1/2] fix(lookup): accept our own lookup response instead of relaying it away MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An originator could misfile the answer to its own lookup as transit, so discovery reported "requests went unanswered" while the replies were in fact arriving and being reverse-path forwarded to a peer. A request is flooded to every tree peer whose bloom filter claims the target. At a high fill ratio a false positive sends a copy out into the wider network, which can circulate it back to the originator. On arrival `classify_request` applied its only identity test, `target == my_addr`, which a lookup we originated never satisfies, so the copy was recorded in `recent_requests` as an ordinary transit entry keyed on our own `request_id`. When the target answered, `classify_response` consulted `recent_requests` first, matched that entry, and relayed our own answer to the peer that looped the request. The pending lookup was never satisfied, the ladder retried with a fresh id, and the same race repeated. The `None` arm's reasoning — "not a request we transited, so it claims to answer one of ours" — held only while our own ids stayed out of the dedup cache, and nothing enforced that. `resp_unsolicited` pinned at zero while `resp_received == resp_forwarded` was the tell: control never reached the originator arm. Affected nodes showed `resp_accepted` at 1 of 291. It is a race, not a hard failure: when the genuine reply beats the looped copy the cache is still clean and the lookup succeeds. It bites hardest at the junction between a quiet subtree and a dense public network, where a lookup for a descendant goes both correctly downward and outward through a false positive, and the wide network returns it first. Two changes close it, on both sides of the invariant: - `classify_response` tests the pending lookups before `recent_requests`. A response naming a target with a lookup outstanding, carrying an id that lookup issued, is ours whatever the dedup cache holds. The id is fresh 64-bit randomness per attempt and the target signs over it, so an id we never issued still cannot match and the replay properties the `Unsolicited` arm protects are unchanged. - `classify_request` drops a request whose id a pending lookup for that target issued, rather than recording it. Our own id never enters the transit cache, so a duplicate reply cannot be misrouted after we accept the first, and the returning copy is not forwarded a second time. The guard keys on the id, so another node's lookup for the same target still transits normally. Verified against the reported deployment: discovery now completes where it previously exhausted the four-attempt ladder. --- CHANGELOG.md | 22 +++++++ docs/design/fips-mesh-operation.md | 11 ++++ src/proto/lookup/core.rs | 74 +++++++++++++++++------- src/proto/lookup/tests/core.rs | 93 +++++++++++++++++++++++++++++- 4 files changed, 176 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37a63705..cb89e2e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +#### Discovery + +- A node no longer relays away the answer to its own lookup. A request is + flooded to every tree peer whose bloom filter claims the target, so a false + positive can send a copy out into the wider network and circulate it back to + the node that originated it. The only identity test on arrival was whether + the request named this node as the target, which a lookup this node + originated never satisfies, so the copy was filed in the request dedup cache + as ordinary transit under this node's own `request_id`. When the target + answered, the reply was reverse-path forwarded to the peer that looped the + request, the pending lookup was never satisfied, and discovery reported that + its requests went unanswered while the answers were in fact arriving. An + inbound response is now matched against this node's outstanding lookups + before the transit dedup record, and a returning copy of this node's own + request is dropped as the duplicate it is rather than recorded, so that id + never enters the transit cache at all. This was a race rather than a hard + failure: a reply that beat the looped copy found a clean cache and + succeeded, and the failure grew likelier as the bloom fill ratio rose. + Contributed by Arjen. + ## [0.5.0] - 2026-08-30 ### Added diff --git a/docs/design/fips-mesh-operation.md b/docs/design/fips-mesh-operation.md index dd8d4da7..45671341 100644 --- a/docs/design/fips-mesh-operation.md +++ b/docs/design/fips-mesh-operation.md @@ -272,6 +272,17 @@ follows the same path as the request. Greedy tree routing toward the `origin_coords` is used only as a fallback if the reverse-path entry has expired. +**Originator check first**: a node tests its own outstanding lookups before +consulting `recent_requests`. A request is flooded to every bloom-matching +tree peer, and a bloom false positive can send a copy out into the wider +network and back to the originator, which would otherwise file its own +`request_id` as an ordinary transit entry and relay its own answer away. The +originator arm therefore wins: a response naming a target with a lookup +outstanding, carrying a `request_id` that lookup issued, is accepted here +whatever the dedup cache holds. A returning copy of the request is likewise +dropped as a duplicate rather than recorded, so the originator's id never +enters the transit cache. + **Response-forwarded flag**: Each `recent_requests` entry tracks whether a response has already been forwarded for that `request_id`. If a second response arrives (e.g., from convergent request paths that reached the diff --git a/src/proto/lookup/core.rs b/src/proto/lookup/core.rs index 6aeedf84..c3eb2315 100644 --- a/src/proto/lookup/core.rs +++ b/src/proto/lookup/core.rs @@ -258,6 +258,26 @@ pub(crate) fn classify_request( }; } + // A request this node originated, flooded to every bloom-matching tree + // peer and circulated back to us by one of them. The only identity test + // below is `request.target == *my_addr`, and for a lookup we originated + // the target is someone else, so without this the copy is filed as an + // ordinary transit entry keyed on our own `request_id` — and the answer, + // when it comes, is reverse-path forwarded to the peer that looped the + // request instead of being accepted here. Dropping it as the duplicate it + // is also stops the copy being forwarded a second time: our flood already + // reached the peers that could carry it. + if lookup + .pending_lookups + .get(&request.target) + .is_some_and(|pending| pending.matches(request.request_id)) + { + return Classification { + outcome: RequestOutcome::Duplicate, + evicted: None, + }; + } + let evicted = make_room(lookup, from, max_recent, peer_count); lookup.record_recent(request.request_id, *from, now_ms); @@ -278,8 +298,9 @@ pub(crate) fn classify_request( Classification { outcome, evicted } } -/// How an inbound LookupResponse should be routed, decided from the -/// recent-request dedup state. +/// How an inbound LookupResponse should be routed, decided from the pending +/// lookups this node has outstanding and, failing that, the recent-request +/// dedup state. pub(crate) enum ResponseRoute { /// A response for a request we forwarded, but we already reverse-forwarded /// one for this request_id — drop to prevent response routing loops. @@ -294,7 +315,8 @@ pub(crate) enum ResponseRoute { Unsolicited, } -/// Classify an inbound LookupResponse against the recent-request dedup cache. +/// Classify an inbound LookupResponse against the pending lookups and the +/// recent-request dedup cache, in that order. /// /// Pure decision over `Lookup` state: sets `response_forwarded` when this is /// the first response we transit for the request. No I/O, no view, no metrics. @@ -303,6 +325,30 @@ pub(crate) fn classify_response( request_id: u64, target: &NodeAddr, ) -> ResponseRoute { + // Originator-ness is decided first, ahead of the transit dedup record. + // The response names a target this node has a lookup outstanding for and + // carries an id that lookup issued, so it answers us whatever else the + // dedup cache happens to hold for that id. Testing `recent_requests` + // first instead made this arm unreachable whenever a copy of our own + // flooded request found its way back to us and was filed as transit: the + // reply was relayed away rather than accepted, the pending lookup was + // never satisfied, and discovery failed while the answers were arriving. + // + // The id is fresh 64-bit randomness drawn per attempt and the target + // signs over it, so a harvested response is bound to the request it + // answered and cannot be redirected or replayed: an id we never issued + // still cannot match, and preferring this arm takes nothing away from + // what the `Unsolicited` arm protects. Replies to earlier attempts of a + // still-outstanding lookup match too, which is the common case on a link + // whose round trip exceeds the first rung of the retry ladder. + if lookup + .pending_lookups + .get(target) + .is_some_and(|pending| pending.matches(request_id)) + { + return ResponseRoute::Originator; + } + match lookup.recent_requests.get_mut(&request_id) { Some(recent) => { if recent.response_forwarded { @@ -314,25 +360,9 @@ pub(crate) fn classify_response( } } } - // Not a request we transited, so it claims to answer one of ours. - // Require that it names a target with a lookup outstanding and carries - // an id issued for it. The id is fresh 64-bit randomness drawn per - // attempt and the target signs over it, so a harvested response is - // bound to the request it answered and cannot be redirected or - // replayed. Replies to earlier attempts of a still-outstanding lookup - // still match, which is the common case on a link whose round trip - // exceeds the first rung of the retry ladder. - None => { - let solicited = lookup - .pending_lookups - .get(target) - .is_some_and(|pending| pending.matches(request_id)); - if solicited { - ResponseRoute::Originator - } else { - ResponseRoute::Unsolicited - } - } + // Neither a request we transited nor an answer to a lookup we have + // outstanding for its target. Nobody asked for it. + None => ResponseRoute::Unsolicited, } } diff --git a/src/proto/lookup/tests/core.rs b/src/proto/lookup/tests/core.rs index 858f5953..43dd9c13 100644 --- a/src/proto/lookup/tests/core.rs +++ b/src/proto/lookup/tests/core.rs @@ -152,8 +152,8 @@ fn classify_response_transit_on_fresh_forwarded_request() { .recent_requests .insert(42, RecentRequest::new(from_peer, 1000)); - // Transit is decided by the dedup record alone, so the target the response - // names plays no part here — pass one no pending lookup mentions. + // With no lookup of ours outstanding for the target, transit is decided by + // the dedup record — pass a target no pending lookup mentions. match classify_response(&mut lookup, 42, &make_node_addr(0xF1)) { ResponseRoute::Transit { from_peer: peer } => assert_eq!(peer, from_peer), _ => panic!("expected Transit"), @@ -181,6 +181,38 @@ fn classify_response_already_forwarded_on_second_call() { )); } +#[test] +fn classify_response_originator_wins_over_a_transit_dedup_record() { + // Regression. A copy of a request we originated can be flooded back to us + // and filed in `recent_requests` as ordinary transit, keyed on our own + // request_id. Consulting the dedup record first then classified the answer + // as Transit and relayed it to the peer that looped the request, so the + // pending lookup was never satisfied and discovery failed with "no reply" + // while the responses were in fact arriving. Originator-ness wins. + let target = make_node_addr(0x61); + let looping_peer = make_node_addr(0x62); + let mut lookup = empty_lookup(); + lookup + .recent_requests + .insert(4242, RecentRequest::new(looping_peer, 1000)); + let mut pending = PendingLookup::new(1000); + pending.record(4242); + lookup.pending_lookups.insert(target, pending); + + assert!(matches!( + classify_response(&mut lookup, 4242, &target), + ResponseRoute::Originator + )); + // And nothing was reverse-path forwarded on our behalf. + assert!( + !lookup + .recent_requests + .get(&4242) + .unwrap() + .response_forwarded + ); +} + #[test] fn classify_response_originator_when_a_pending_lookup_issued_the_id() { // No dedup record, so this is not a response we transit. It counts as ours @@ -442,6 +474,63 @@ fn classify_request_duplicate_on_second_call() { )); } +#[test] +fn classify_request_drops_our_own_request_looped_back_to_us() { + // Regression, request side of the same defect. Our flood reaches a peer + // that circulates it back; the only identity test is `target == my_addr`, + // which a lookup we originated never satisfies. Recording it would file + // our own request_id as a transit entry and hand the eventual answer to + // the looping peer, so the copy is dropped as the duplicate it is. + let mut lookup = empty_lookup(); + let looping_peer = make_node_addr(0x01); + let my_addr = make_node_addr(0x99); + let target = make_node_addr(0xAA); + let mut pending = PendingLookup::new(1000); + pending.record(77); + lookup.pending_lookups.insert(target, pending); + + let request = make_request_id(77, target, 3); + let classification = classify_request( + &mut lookup, + &request, + &looping_peer, + &my_addr, + 1000, + 5000, + 4096, + 1, + ); + assert!(matches!(classification.outcome, RequestOutcome::Duplicate)); + assert!(classification.evicted.is_none()); + // Crucially, our own id must not enter the transit dedup cache. + assert!(!lookup.recent_requests.contains_key(&77)); + assert!(lookup.recent_by_peer.is_empty()); + // A response for it is then ours to accept. + assert!(matches!( + classify_response(&mut lookup, 77, &target), + ResponseRoute::Originator + )); +} + +#[test] +fn classify_request_still_transits_a_foreign_id_for_a_target_we_are_looking_up() { + // The guard keys on the id, not the target: another node's lookup for the + // same target must still be transited normally while ours is outstanding. + let mut lookup = empty_lookup(); + let from = make_node_addr(0x01); + let my_addr = make_node_addr(0x99); + let target = make_node_addr(0xAA); + let mut pending = PendingLookup::new(1000); + pending.record(77); + lookup.pending_lookups.insert(target, pending); + + let request = make_request_id(88, target, 3); + let outcome = + classify_request(&mut lookup, &request, &from, &my_addr, 1000, 5000, 4096, 1).outcome; + assert!(matches!(outcome, RequestOutcome::Forward)); + assert_eq!(lookup.recent_requests.get(&88).unwrap().from_peer, from); +} + #[test] fn classify_request_evicts_rather_than_refusing_a_full_dedup_cache() { // Regression. The cache-full path used to drop the arriving request, so From e5586cb33315f9519123c5a6a5436a3aa3b1bf6b Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Mon, 31 Aug 2026 20:26:46 +0000 Subject: [PATCH 2/2] fix(lookup): count a request of our own that came back apart from a duplicate Dropping our own flooded request when a bloom false positive circulates it back to us recorded the drop under `req_duplicate`, whose documented meaning is that a peer resent a request. The two events are not the same, and only one of them says anything about the peer that delivered the frame. A returning copy has a nonzero floor in healthy operation and rises with the bloom fill ratio, so folding it into `req_duplicate` puts a permanent number on a counter an operator reads as neighbour misbehaviour, and leaves no way to tell a resending peer from this node's own fan-out coming home. It now carries its own rejection reason and counter, `req_own_loopback`, shown in fipstop as "Own Loopback", and its own log line naming the target. The control socket's show routing fixture gains the field. Two comments record what the guard rests on. `request.origin` is the obvious cheaper identity test and is unusable: it is unsigned and set by whoever sends the frame, so a peer could put this node's address on any request and make it refuse to transit that request. And the test reaches only as far as the last MAX_RECORDED_IDS a target's ladder issued, so a ladder configured with more rungs than that loses its earliest ids. The request-side regression test now builds its request with this node's own address as the origin, which is the shape the defect actually has. It was using a helper whose origin is a third party, so it exercised the guard without reproducing the case. --- CHANGELOG.md | 11 +++++++++++ src/bin/fipstop/ui/routing.rs | 1 + src/control/snapshots/show_routing.json | 2 ++ src/node/handlers/lookup.rs | 17 +++++++++++++++++ src/node/metrics.rs | 3 +++ src/node/reject.rs | 11 +++++++++++ src/node/stats.rs | 1 + src/proto/lookup/core.rs | 22 +++++++++++++++++++++- src/proto/lookup/tests/core.rs | 12 ++++++++++-- 9 files changed, 77 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb89e2e0..c026b52f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 succeeded, and the failure grew likelier as the bloom fill ratio rose. Contributed by Arjen. +- A lookup request of this node's own, returning to it, is no longer counted as + a duplicate from the peer that delivered it. The fix above drops that copy, + and it recorded the drop under the existing `req_duplicate` rejection, whose + documented meaning is that a peer resent a request. A returning copy has a + nonzero floor in healthy operation and rises with the bloom fill ratio, so + folding the two together put a permanent number on a counter an operator + reads as neighbour misbehaviour, and made the two events indistinguishable. + It now has its own rejection reason and counter, `req_own_loopback`, shown in + `fipstop` as "Own Loopback". `req_duplicate` returns to meaning only what it + says. + ## [0.5.0] - 2026-08-30 ### Added diff --git a/src/bin/fipstop/ui/routing.rs b/src/bin/fipstop/ui/routing.rs index 1961c8c5..1cae2ca5 100644 --- a/src/bin/fipstop/ui/routing.rs +++ b/src/bin/fipstop/ui/routing.rs @@ -189,6 +189,7 @@ fn draw_routing_stats( ("Deduplicated", lookup("req_deduplicated")), ("Target Is Us", lookup("req_target_is_us")), ("Duplicate", lookup("req_duplicate")), + ("Own Loopback", lookup("req_own_loopback")), ("Bloom Miss", lookup("req_bloom_miss")), ("Backoff Suppressed", lookup("req_backoff_suppressed")), ("Fwd Rate Limited", lookup("req_forward_rate_limited")), diff --git a/src/control/snapshots/show_routing.json b/src/control/snapshots/show_routing.json index 99e8ddd2..0e0685da 100644 --- a/src/control/snapshots/show_routing.json +++ b/src/control/snapshots/show_routing.json @@ -20,6 +20,7 @@ "req_forwarded": 0, "req_initiated": 0, "req_no_tree_peer": 0, + "req_own_loopback": 0, "req_received": 0, "req_sign_rate_limited": 0, "req_target_is_us": 0, @@ -96,6 +97,7 @@ "req_forwarded": 0, "req_initiated": 0, "req_no_tree_peer": 0, + "req_own_loopback": 0, "req_received": 0, "req_sign_rate_limited": 0, "req_target_is_us": 0, diff --git a/src/node/handlers/lookup.rs b/src/node/handlers/lookup.rs index 76ec77d1..90a45208 100644 --- a/src/node/handlers/lookup.rs +++ b/src/node/handlers/lookup.rs @@ -139,6 +139,23 @@ impl Node { "Duplicate LookupRequest, dropping" ); } + RequestOutcome::OwnRequestLooped => { + // Our own flooded request, come back to us through a bloom + // false positive. Counted apart from ReqDuplicate: that one + // describes a peer resending, and this one describes our own + // fan-out returning, so folding them together would put a + // permanent healthy floor on a counter an operator reads as + // neighbour misbehaviour. + self.metrics() + .lookup + .record_reject(DiscoveryReject::ReqOwnLoopback); + debug!( + request_id = request.request_id, + from = %self.peer_display_name(from), + target = %self.peer_display_name(&request.target), + "LookupRequest we originated came back to us, dropping" + ); + } RequestOutcome::RespondAsTarget => { // Answering costs a fresh Schnorr signature every time: the // proof is bound to the requester's request_id, so it cannot diff --git a/src/node/metrics.rs b/src/node/metrics.rs index dc40d1ba..b5727d04 100644 --- a/src/node/metrics.rs +++ b/src/node/metrics.rs @@ -277,6 +277,7 @@ pub struct LookupMetrics { pub req_received: Padded, pub req_decode_error: Counter, pub req_duplicate: Counter, + pub req_own_loopback: Counter, pub req_dedup_cache_full: Counter, pub req_dedup_evicted: Counter, pub req_sign_rate_limited: Counter, @@ -309,6 +310,7 @@ impl LookupMetrics { match reason { DiscoveryReject::ReqDecodeError => self.req_decode_error.inc(), DiscoveryReject::ReqDuplicate => self.req_duplicate.inc(), + DiscoveryReject::ReqOwnLoopback => self.req_own_loopback.inc(), DiscoveryReject::ReqDedupCacheFull => self.req_dedup_cache_full.inc(), DiscoveryReject::ReqSignRateLimited => self.req_sign_rate_limited.inc(), DiscoveryReject::ReqTtlExhausted => self.req_ttl_exhausted.inc(), @@ -326,6 +328,7 @@ impl LookupMetrics { req_received: self.req_received.get(), req_decode_error: self.req_decode_error.get(), req_duplicate: self.req_duplicate.get(), + req_own_loopback: self.req_own_loopback.get(), req_dedup_cache_full: self.req_dedup_cache_full.get(), req_dedup_evicted: self.req_dedup_evicted.get(), req_sign_rate_limited: self.req_sign_rate_limited.get(), diff --git a/src/node/reject.rs b/src/node/reject.rs index fbaaeab2..40bfbd9d 100644 --- a/src/node/reject.rs +++ b/src/node/reject.rs @@ -117,6 +117,16 @@ pub enum DiscoveryReject { /// Tracked via /// [`DiscoveryStats::req_duplicate`](crate::node::stats::DiscoveryStats). ReqDuplicate, + /// A request this node originated, flooded to its bloom-matching tree + /// peers and circulated back to it by one of them. Dropped without being + /// recorded, so the answer is accepted here rather than reverse-path + /// forwarded to the peer that looped it. Tracked via + /// [`DiscoveryStats::req_own_loopback`](crate::node::stats::DiscoveryStats). + /// + /// **This has a nonzero floor in healthy operation** and rises with the + /// bloom fill ratio. It says nothing about the peer that delivered the + /// copy, which is why it is not counted as [`Self::ReqDuplicate`]. + ReqOwnLoopback, /// Request dedup cache (`recent_requests`) is at capacity, so the /// `LookupRequest` is dropped without being forwarded. Tracked via /// [`DiscoveryStats::req_dedup_cache_full`](crate::node::stats::DiscoveryStats). @@ -398,6 +408,7 @@ mod tests { let variants = [ DiscoveryReject::ReqDecodeError, DiscoveryReject::ReqDuplicate, + DiscoveryReject::ReqOwnLoopback, DiscoveryReject::ReqDedupCacheFull, DiscoveryReject::ReqTtlExhausted, DiscoveryReject::ReqSignRateLimited, diff --git a/src/node/stats.rs b/src/node/stats.rs index 8367d857..0d82f62f 100644 --- a/src/node/stats.rs +++ b/src/node/stats.rs @@ -334,6 +334,7 @@ pub struct LookupStatsSnapshot { pub req_received: u64, pub req_decode_error: u64, pub req_duplicate: u64, + pub req_own_loopback: u64, pub req_dedup_cache_full: u64, pub req_dedup_evicted: u64, pub req_sign_rate_limited: u64, diff --git a/src/proto/lookup/core.rs b/src/proto/lookup/core.rs index c3eb2315..2aa57ac2 100644 --- a/src/proto/lookup/core.rs +++ b/src/proto/lookup/core.rs @@ -148,6 +148,12 @@ pub(crate) fn plan_initiate(request: &LookupRequest, rv: &impl RoutingView) -> V pub(crate) enum RequestOutcome { /// request_id already in the dedup cache — drop. Duplicate, + /// A request this node originated, looped back to it — drop without + /// recording. Kept separate from `Duplicate`, which means another node + /// resent a request, so the two do not share a rejection counter: this + /// one has a nonzero floor in healthy operation and says nothing about + /// the peer that delivered it. + OwnRequestLooped, /// We are the lookup target — the shell generates + sends the response. RespondAsTarget, /// Forward the request onward (the shell calls the forward planner). @@ -267,13 +273,27 @@ pub(crate) fn classify_request( // request instead of being accepted here. Dropping it as the duplicate it // is also stops the copy being forwarded a second time: our flood already // reached the peers that could carry it. + // + // `request.origin` looks like the cheaper identity test and cannot be + // used. It is unsigned and set by whoever sends the frame, so a peer + // could put this node's address on any request and make it refuse to + // transit that request. An id has to have been issued here to match, + // which is what makes this test safe to drop on. + // + // The test reaches only as far as `PendingLookup::ids`, which keeps the + // last `MAX_RECORDED_IDS` that a target's ladder issued. A ladder + // configured with more rungs than that loses its earliest ids, and a + // returning copy of one of those attempts is recorded as transit again. + // The bound is inherited from the originator test in `classify_response` + // rather than introduced here, and a reply to such an attempt was already + // being dropped as unsolicited. if lookup .pending_lookups .get(&request.target) .is_some_and(|pending| pending.matches(request.request_id)) { return Classification { - outcome: RequestOutcome::Duplicate, + outcome: RequestOutcome::OwnRequestLooped, evicted: None, }; } diff --git a/src/proto/lookup/tests/core.rs b/src/proto/lookup/tests/core.rs index 43dd9c13..66f4e21c 100644 --- a/src/proto/lookup/tests/core.rs +++ b/src/proto/lookup/tests/core.rs @@ -489,7 +489,12 @@ fn classify_request_drops_our_own_request_looped_back_to_us() { pending.record(77); lookup.pending_lookups.insert(target, pending); - let request = make_request_id(77, target, 3); + // Built here rather than through `make_request_id`, whose origin is a + // third party: the defect is our *own* request returning, so the request + // under test has to carry our address as its origin. The guard keys on + // the id and ignores the origin, so this asserts the same behaviour the + // helper would; it asserts it on the shape the defect actually has. + let request = LookupRequest::new(77, target, my_addr, TreeCoordinate::root(my_addr), 3, 0); let classification = classify_request( &mut lookup, &request, @@ -500,7 +505,10 @@ fn classify_request_drops_our_own_request_looped_back_to_us() { 4096, 1, ); - assert!(matches!(classification.outcome, RequestOutcome::Duplicate)); + assert!(matches!( + classification.outcome, + RequestOutcome::OwnRequestLooped + )); assert!(classification.evicted.is_none()); // Crucially, our own id must not enter the transit dedup cache. assert!(!lookup.recent_requests.contains_key(&77));