feat(discovery): Phase 3 multi-hop role separation + opt-in /16 diversity

[metaphorics]

Phase 3 of the discovery rationalization

Builds on PR #237 (Phase 2 weighted composite + Selector seam). Adds multi-hop role separation (default-on) and opt-in /16 + family diversity via a wrap-around diversity.Diversity selector. Stacked PR: base = discovery-phase-2-weighted-selector.

Plan-deferred caveat (read this)

Phase 3's plan explicitly stated detailed design was deferred until Phase 2 telemetry validates the gap each phase is meant to close. This PR ships Phase 3 ahead of that data per user direction ("all four phases now"). The two open questions Phase 2 telemetry was meant to answer remain open:

1. How often does the inner selector emit duplicate relays in multi-hop output? (If never, role separation is no-op.)
2. How often would /16 + family constraint force pool relaxation given typical pool sizes? (If always, the opt-in flag is misleading.)

Phase 3 ships the mechanism; the necessity is to be validated by Phase 1+2 telemetry once production data accumulates.

What landed (5 atomic commits)

1. 35b4b8ce feat(types): add Family + Subnet16 fields to RelayDescriptor — advisory unsigned fields. Family string (operator-family bucket key), Subnet16 string (/16 prefix bucket key). Empty = no constraint contribution. CanonicalBytes deliberately unchanged so existing signed descriptors verify byte-identically.
2. a6b63f5d feat(discovery): add ClientState.DisableDiversityRoles + AnonymityGrade fields — inverted-default field name (zero ClientState gets role separation on). Documented why the inversion exists: most call sites use zero-value ClientState today, and the safe default for routing is "no duplicate hops."
3. 04e6b68d feat(discovery/diversity): selector wrapper + portal_discovery_diversity_relaxed_total metric — new package portal/discovery/selectors/diversity/. Diversity.SelectPriority is passthrough (no diversity work for single-hop); Diversity.SelectMultiHop walks inner-ranked URLs, applies role+anonymity dedup, relaxes on shortfall. New counter portal_discovery_diversity_relaxed_total{reason} with cardinality bounded to two reason values: anonymity_grade, role_separation.
4. 47d24227 test(discovery/diversity): unit tests + contract wiring + load-test extension — 7 unit/contract tests + selectortest.Contract wiring against diversity.New(mols.New()) + new -anonymity and -anonymity-collide flags on cmd/portal-loadtest/main.go.
5. efb56646 fix(discovery/diversity): use trace.Ranked for extras, not raw pool — eligibility-leak fix: buildCandidates originally used the raw pool (which includes banned/expired/suppressed relays) as the source of "extras" when the inner output ran short. Switched to trace.Ranked (eligible-only). Added TestDiversityBannedRelayExcludedFromExtras as a discriminating test (would fail on the original code).

Verification (local)

• go build ./... exit 0
• go vet ./... exit 0
• make lint exit 0 (0 issues)
• go test -count=1 ./... exit 0 (243 tests pass across 24 packages, up from Phase 2's 222 in 23)
• go mod verify clean

Phase 3 acceptance — runtime evidence

go run ./cmd/portal-loadtest -clients 100 -relays 5 -multi-hop 3 -anonymity (each relay assigned a unique synthetic Subnet16):

selector: mols+diversity
clients: 100  relays: 5  mode: multihop
anonymity-grade: enabled
…
zero duplicate-relay paths: PASS
zero /16 collisions: PASS
relaxation event metric: anonymity_grade=0  role_separation=0

go run ./cmd/portal-loadtest -clients 100 -relays 5 -multi-hop 3 -anonymity -anonymity-collide (all 5 relays forced into the same Subnet16 — should trigger 100% relaxation):

zero duplicate-relay paths: PASS              ← role separation still satisfied
zero /16 collisions: FAIL (100/100 clients had /16 collisions)
relaxation event metric: anonymity_grade=100  role_separation=0

Both runs match plan acceptance:

• Distinct Subnet16 → zero collisions, zero relaxation events.
• Forced-collision pool → relaxation triggers per client; metric increments accurately; role-separation invariant still holds.

Public API

• No public API breakage. RelaySet.PriorityRelays, RelaySet.PriorityMultiHop, and the Phase 1 *WithTrace siblings preserved byte-equal.
• New public types: diversity.Diversity (Selector wrapper), diversity.New, diversity.Option.
• New public fields: RelayDescriptor.Family, RelayDescriptor.Subnet16, ClientState.DisableDiversityRoles, ClientState.AnonymityGrade.
• New public metric: discovery.DiversityRelaxedTotal (counter-vec; label reason ∈ {anonymity_grade, role_separation}).
• RelayDescriptor.CanonicalBytes unchanged — Family + Subnet16 are advisory fields; legacy signed descriptors continue to verify.

Algorithm sketch (the actual implemented one)

SelectMultiHop(ctx, pool, client):
    inner_urls, inner_trace := inner.SelectMultiHop(ctx, pool, client)
    if !DiversityRoles && !AnonymityGrade: return inner_urls

    candidates := inner_urls + (trace.Ranked extras, eligible-only)
    out := []
    usedSubnets, usedFamilies := {}, {}
    for url in candidates:
        if url in out: continue                             # role separation (DiversityRoles)
        if AnonymityGrade:
            sub := state.Descriptor.Subnet16
            fam := state.Descriptor.Family
            if (sub != "" && sub in usedSubnets) ||
               (fam != "" && fam in usedFamilies): continue
            if sub != "": usedSubnets.add(sub)
            if fam != "": usedFamilies.add(fam)
        out.append(url)
        if len(out) >= MultiHopDepth: break

    if len(out) < MultiHopDepth:
        if AnonymityGrade:
            relax_anonymity(); metric.Inc("anonymity_grade")
            re-walk without subnet/family check
        if len(out) < MultiHopDepth and DiversityRoles:
            metric.Inc("role_separation")
            fall back to inner_urls (already had distinct URLs from MOLS)
    return out, trace_with_OutputURLs_replaced

Out of scope (deferred to Phase 4)

• Reservation voucher (libp2p Circuit Relay v2 surface).
• Production deploy + telemetry data to validate Phase 3 mechanisms.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: efb5664683

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve inner ranking when choosing diversity extras

After a diversity rejection, replacement hops are taken from extras, but this code sorts those URLs lexicographically instead of keeping the inner selector’s priority order. In multi-hop mode with weighted.New(...) wrapped by diversity, this can select a higher-load relay purely because its URL sorts earlier, which undermines the weighted policy exactly when replacements are needed (e.g., subnet/family collision filtering).

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Bypass diversity wrapper when all constraints are disabled

The wrapper still rewrites SelectMultiHop output even when DisableDiversityRoles=true and AnonymityGrade=false, because it always runs walk(...) over innerURLs + extras. If an inner selector intentionally returns a trimmed list shorter than MultiHopDepth, this path backfills from trace.Ranked and changes routing despite both diversity controls being off, violating the expected no-op semantics of disabling all diversity constraints.

Useful? React with 👍 / 👎.