feat(dht)!: add trust quarantine thresholds

[mickvandijke]

Summary

• Add trust quarantine policy to DHT routing with three thresholds: 0.35 lazy swap eligibility, 0.20 quarantine/automatic-avoidance, and 0.45 new-peer admission/readmission.
• Immediate close-group eviction is currently disabled while trust scoring stabilizes. Peers below 0.20 remain in the routing table and are avoided by automatic lookup/dial policy instead.
• Keep lazy swap-out as the replacement path: low-trust peers remain eligible to be swapped out when better routing-table candidates arrive.
• Gate all new routing-table admissions at 0.45, including peers that would occupy non-close routing slots and peers attempting to re-enter after quarantine.
• Preserve existing routing-table peers with trust in [0.20, 0.45): they may remain in the table and can move into the close group, while peers below 0.20 are avoided by automatic lookup/dial paths.
• Filter quarantined or below-threshold peers from local lookup results, FIND_NODE responses, automatic lookup, bootstrap, bucket refresh, and self-lookup paths.
• Keep explicit sends unblocked; quarantine is routing-table and automatic lookup policy, not a blanket transport block.
• Update docs and tests for the new trust policy and threshold semantics.

Breaking Changes

• AdaptiveDhtConfig now includes quarantine_threshold and quarantine_readmit_threshold.
• New routing-table peers must meet quarantine_readmit_threshold before admission when trust quarantine is enabled.
• Peers below quarantine_threshold are no longer handed out through DHT lookup results or used by automatic lookup/dial maintenance paths.

Tests

• cargo fmt --all -- --check
• cargo check --all-features
• cargo clippy --all-features -- -D clippy::panic -D clippy::unwrap_used -D clippy::expect_used
• cargo test --all-features quarantine --lib
• cargo test --all-features --lib
• cargo test --all-features --test trust_flow --test sybil_protection
• cargo test --all-features - not rerun for this amendment; prior run on this PR failed in tests/dht_self_advertisement.rs wildcard-bind self-entry assertions after all library tests passed; appears unrelated to this change.

Greptile Summary

This PR adds a three-tiered trust quarantine policy to the DHT routing layer: a 0.20 automatic-avoidance threshold, a 0.45 new-peer admission threshold, and the existing 0.35 lazy-swap threshold. Immediate close-group eviction is disabled for now while trust scoring stabilizes. The implementation gates new routing-table admissions, close-group evictions, local lookup results, automatic lookup/dial paths, bootstrap, and bucket refresh.

• Adds quarantine_threshold and quarantine_readmit_threshold fields to AdaptiveDhtConfig, DhtNetworkConfig, and DhtCoreEngine, wired end-to-end from AdaptiveDHT through DhtNetworkManager to the core routing engine.
• Introduces enforce_close_group_trust_gate, check_new_peer_admission, should_avoid_for_lookup, and should_avoid_automatic_candidate as new trust-gating primitives guarded by quarantine_enabled().
• Adds broadcast_routing_events_with_quarantine to run deferred close-group evictions after any routing-table admission and emit a single merged KClosestPeersChanged event.

Confidence Score: 4/5

The new quarantine logic is well-tested, correctly gated behind quarantine_enabled(), and does not block explicit sends. All automatic lookup paths consistently apply the new filters.

The three-threshold eviction and lookup-filtering paths are covered by purpose-built unit tests exercising boundary conditions, deferred eviction, and readmission. The _previous_close_group dead parameter and quarantined_peers growth under adversarial churn are notable open questions but neither breaks current behaviour.

src/dht/core_engine.rs — the enforce_close_group_trust_gate dead parameter and quarantined_peers unbounded growth warrant a second look before long-running node deployments.

Important Files Changed

Filename	Overview
src/dht/core_engine.rs	Core trust-gating primitives added. _previous_close_group parameter in enforce_close_group_trust_gate is accepted at every call site but unused. quarantined_peers HashSet has no upper-bound or expiry mechanism.
src/dht_network_manager.rs	Quarantine enforcement threaded through all automatic lookup paths. broadcast_routing_events_with_quarantine correctly merges admission and quarantine-eviction events into a single KClosestPeersChanged.
src/adaptive/dht.rs	New thresholds added to AdaptiveDhtConfig with validation, defaults, and propagation. Tests cover defaults, validation, and threshold ordering.
src/network.rs	trust_enforcement(false) zeroes all three thresholds; trust_enforcement(true) uses full AdaptiveDhtConfig::default().
tests/sybil_protection.rs	Struct literals updated with ..Default::default() for the two new fields; test intent unchanged.
tests/trust_flow.rs	Same struct-literal fixups; existing trust-flow tests unaffected.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Trust event reported] --> B[Update TrustEngine score]
    B --> C[enforce_trust_quarantine]
    C --> D{trust_engine present?}
    D -- No --> Z[return false]
    D -- Yes --> E[enforce_close_group_trust_gate]
    E --> F{quarantine enabled?}
    F -- No --> G[return empty events]
    F -- Yes --> H{node_count > K AND K-closest peer below quarantine_threshold?}
    H -- No --> I[return empty events]
    H -- Yes --> J[evict peer, add to quarantined_peers]
    J --> H
    J --> K[emit PeerRemoved + KClosestPeersChanged]
    K --> L[broadcast_routing_events]
    M[New peer admission] --> N{peer already known?}
    N -- No --> O[check_new_peer_admission]
    O --> P{quarantine enabled?}
    P -- No --> Q[admit peer]
    P -- Yes --> R{trust >= readmit_threshold?}
    R -- No --> S[Reject]
    R -- Yes --> T[remove from quarantined_peers, admit]
    T --> U[broadcast_routing_events_with_quarantine]
    U --> V{PeerAdded or KClosestPeersChanged?}
    V -- No --> W[broadcast directly]
    V -- Yes --> X[enforce_close_group_trust_gate deferred pass]
    X --> Y[merge KClosestPeersChanged, broadcast]
    AA[Automatic lookup / bootstrap / bucket refresh] --> AB[should_avoid_automatic_peer]
    AB --> AC{quarantine enabled?}
    AC -- No --> AD[include peer]
    AC -- Yes --> AE{trust below quarantine_threshold OR quarantined and below readmit?}
    AE -- Yes --> AF[skip peer]
    AE -- No --> AG{unknown peer below readmit_threshold?}
    AG -- Yes --> AF
    AG -- No --> AD

Loading

Prompt To Fix All With AI

Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
src/dht/core_engine.rs:1607
**Dead parameter silently dropped**

`_previous_close_group` is accepted at every call site (including `broadcast_routing_events_with_quarantine` which extracts the original old close group expressly to pass here) but the implementation never reads it. The eviction loop operates solely on the live K-closest snapshot, so the "which peers were newly promoted vs. long-standing" distinction the parameter implies is never applied. If future logic needs it (e.g., giving just-promoted peers a grace period), the current signature already supports it; if it is not needed, removing the parameter avoids misleading callers about the function's contract.

### Issue 2 of 4
src/dht/core_engine.rs:1396
**`quarantined_peers` grows without a bound or expiry**

Entries are added in `enforce_close_group_quarantine` / `enforce_close_group_trust_gate` and removed only in `check_new_peer_admission` when a peer successfully re-enters the routing table at or above the readmit threshold. A peer that is quarantined and then permanently leaves the network (or is never rediscovered) keeps its entry in this `HashSet` forever. For a long-running node operating under adversarial churn this can accumulate a large number of stale entries. A periodic sweep removing entries whose last-seen timestamp exceeds the `live_threshold`, or capping the set at a generous but finite size, would bound the growth.

### Issue 3 of 4
src/dht/core_engine.rs:1443-1444
**Inconsistent initial `quarantine_readmit_threshold` in `DhtCoreEngine::new()`**

`quarantine_threshold` is initialised to `0.0` (quarantine disabled) but `quarantine_readmit_threshold` is initialised to `DEFAULT_QUARANTINE_READMIT_THRESHOLD` (0.45). When `DhtNetworkManager` constructs the engine it immediately calls `set_trust_quarantine_thresholds(config.quarantine_threshold, config.quarantine_readmit_threshold)` which overrides the 0.45, so production code is unaffected. However, a future caller that constructs `DhtCoreEngine` directly and introspects the threshold before calling `set_trust_quarantine_thresholds` would observe an asymmetric disabled state (threshold=0.0, readmit=0.45). Initialising `quarantine_readmit_threshold: 0.0` here would make the default state symmetrically disabled.

### Issue 4 of 4
src/dht/core_engine.rs:1618-1634
**`trust_score` closure called twice per candidate in the eviction loop**

Inside the `while` loop the closure is invoked once in the `find` iterator (with the `is_finite()` guard) and then a second time unconditionally for the `quarantined_peers.insert` decision. Because the `find` predicate already requires `is_finite() && score < quarantine_threshold`, the second call will always produce the same result and the `if` branch will always be taken. Capturing the score from the first call into a local variable would eliminate the duplicate lookup and make the invariant explicit.

_{Reviews (1): Last reviewed commit: "fix(docs): document adaptive trust enfor..." | Re-trigger Greptile}

Greptile also left 4 inline comments on this PR.

[Copilot]

Pull request overview

This PR introduces a trust-based quarantine policy for DHT routing, adding separate thresholds for (1) swap eligibility, (2) quarantine/automatic-avoidance, and (3) new-peer admission/readmission, and propagates that policy through routing-table admission, lookup candidate selection, and DHT maintenance flows.

Changes:

• Extend AdaptiveDhtConfig/DhtNetworkConfig to include quarantine thresholds and validate/enforce them throughout DHT operations.
• Add close-group quarantine enforcement (evict low-trust close peers when RT has surplus above K) and filter quarantined/below-threshold peers from local lookup results and automatic maintenance paths.
• Update integration tests and documentation to reflect the new trust threshold semantics and breaking config changes.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/trust_flow.rs	Updates tests to build AdaptiveDhtConfig with new fields via ..Default::default().
tests/sybil_protection.rs	Updates docs/comments and config construction to align with quarantine + swap semantics.
src/network.rs	Updates node config docs and builder toggle to enable/disable all adaptive thresholds coherently.
src/dht/core_engine.rs	Adds quarantine thresholds/state and enforcement logic + extensive unit tests for threshold behavior.
src/dht_network_manager.rs	Applies quarantine filtering to lookup/bootstrap/refresh/self-lookup paths and re-runs quarantine after admissions.
src/adaptive/dht.rs	Adds new config fields, validation, propagation into the manager config, and triggers quarantine enforcement after trust updates.
README.md	Updates high-level trust system description to reflect quarantine model.
docs/trust-signals-api.md	Re-documents API semantics around the new thresholds and behavior.
docs/SECURITY_MODEL.md	Updates security model docs to match quarantine-based routing enforcement.
docs/ROUTING_TABLE_DESIGN.md	Updates routing-table design invariants/threshold descriptions to reflect quarantine + admission gating.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

[dirvine]

Hermes review

Verdict: changes requested before merge — one correctness issue in the stale-revalidation retry path.

Blocking

• src/dht/core_engine.rs:3051 / src/dht_network_manager.rs:4995-5007 — re_evaluate_admission() applies the new-peer/readmit gate before checking whether the candidate was already admitted while stale-peer pings were in flight. add_node() correctly only calls check_new_peer_admission() when the peer is not already in the routing table; the retry path should preserve the same invariant.

  Why it matters: revalidate_and_retry_admission() removes stale non-responders first, collects those PeerRemoved/KClosestPeersChanged events, then calls re_evaluate_admission(...)?. If a concurrent task admitted the same candidate during the revalidation window and its current trust is below quarantine_readmit_threshold, the retry can return an error even though the candidate is now an existing peer. That drops the already-collected removal events without broadcasting them, so routing-event consumers can miss removals that already happened in the table.

  Suggested minimal fix:

  1. Make re_evaluate_admission() mirror add_node():

     let peer_already_known = self.has_node(&candidate.id).await;
     if !peer_already_known {
         self.check_new_peer_admission(&candidate.id, trust_score(&candidate.id))?;
     }

     then continue into add_with_diversity().
  2. Consider making revalidate_and_retry_admission() robust against future retry failures by ensuring removal events for already-evicted stale peers are still broadcast/logged even if candidate re-admission fails.

CI / local checks

Local checks on head 179e4f60aacf73f300a64e783c562cc06d3b4800:

• cargo fmt --all -- --check ✅
• cargo clippy --all-features -- -D clippy::panic -D clippy::unwrap_used -D clippy::expect_used ✅
• cargo test --all-features quarantine --lib --no-fail-fast ✅
• cargo test --all-features --lib --no-fail-fast ✅
• cargo test --all-features --test trust_flow --test sybil_protection --no-fail-fast ✅

GitHub checks: build/lint/unit/mutation are green. Security Audit is red due quinn-proto 0.11.14 / RUSTSEC-2026-0185 (upgrade to >=0.11.15), which appears dependency-wide rather than caused by this diff. claude-review failed because the Claude GitHub App is not installed.

The quarantine/admission design otherwise looks coherent: local FIND_NODE responses and automatic lookup/bootstrap/refresh paths are filtering through the quarantine policy, explicit sends remain unblocked, and the marker cap avoids unbounded growth. The stale-revalidation retry asymmetry above is the one I would fix before merging.

[dirvine]

Additional notes from the parallel review pass:

• Docs should be cleaned up before merge. docs/SECURITY_MODEL.md still describes an implemented EigenTrust++ / global power-iteration reputation system and says trust affects storage placement / witness selection. The current code path is local EMA trust used for DHT routing admission/quarantine. Since this PR is changing that same trust/security contract, leaving the old EigenTrust++ wording in the security model is misleading.

• Clarify the stale-revalidation exception. The main automatic lookup/bootstrap/refresh paths do filter quarantined/low-trust peers, but stale K-closest revalidation can still ping a retained low-trust close-group peer when the table is at <= K (stale_k_closest() -> ping_with_identity_confirmation). I think that is probably a reasonable eviction-probe exception; it should be documented explicitly so “automatic lookup/dial paths avoid low-trust peers” is not read as an absolute ban on maintenance probes.

• Public API break is real but appears intentional. Adding required public fields to AdaptiveDhtConfig breaks downstream struct literals that do not use ..Default::default(). The PR is already marked feat(dht)! and lists breaking changes, so I am not treating that as a separate blocker, but a short migration note would be useful for consumers.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

[grumbach]

Putting the July testnet results on record here, since so far they only live in Slack: as parameterized, this branch partitioned a healthy network in all three runs against it (07-10, 07-15, and 07-16 with 5d5f69f applied). Upload success fell 99.9% to 30.2% on 07-10; on 07-16 the fleet went from zero failures to every client failing within ~30 min of the first hourly audit burst, with routing-table admissions collapsing (59,274 to 633 on the 07-15 run).

Why: one EMA step (0.124) moves a neutral peer 0.5 to 0.438, below the 0.45 admission bar, so a single failed interaction of any kind locks a peer out. Failures report at weights up to 5 while almost nothing reports success (421k negative vs 35k positive deltas on 07-16), recovery is decay-only (4h to 31h per single event), and gated lookups prevent a sub-0.45 peer from earning its way back. So under load: timeouts become penalties, penalties block admissions, routing tables stop learning, uploads fail.

The dangers as designed:

• Converts overload into partition. Timeouts under load are load symptoms, not misbehavior, and our audit scheduler currently produces hourly timeout waves.
• Cheap targeted eviction: saturate a victim until it times out for its neighbors and the fleet quarantines it for ~31h. No forged evidence needed. Doing this to a target address's close group clears the neighborhood for sybil placement.
• Structurally punishes NAT'd peers: DHT ping is single-shot direct dial, and dial failures carry weight ~2.25.
• Invisible in ops: admission rejections are returned as errors and never logged; both incident investigations needed hours of ES forensics.

What I'd want before merge:

1. No trust penalty from load-induced timeouts; only proof-carrying failures (digest mismatch, signed contradiction) may cross the admission bar.
2. Success reporting wired for served requests/audits, so the score measures honesty rather than load.
3. Config invariant: neutral minus one max-weight step must not cross the readmit bar.
4. Probation for unknown peers instead of lockout on first failure.
5. A fleet-scale test injecting X% random request timeouts, asserting admissions and upload success do not collapse. It would have caught all three failures.
6. A counter plus rate-limited WARN on admission rejection.

Not repeating the earlier bot/Hermes findings (marker-set growth, re_evaluate_admission ordering, docs drift). Happy to pair on the penalty classification; the audit-side rate bounding is on me and I'll make sure the two land compatibly.