fix(model): count only providers holding a Public provide key#5
Open
Ryanmello07 wants to merge 6 commits into
Open
fix(model): count only providers holding a Public provide key#5Ryanmello07 wants to merge 6 commits into
Ryanmello07 wants to merge 6 commits into
Conversation
A provider without a provide_mode=3 key cannot accept a contract from a client outside its own network, so counting it advertises supply nobody can reach. On beta this reported 39 US providers when 2 were reachable. Filters both UpdateClientLocations and UpdateClientScores: GetProviderLocations gates on loadLocationStables as well, so fixing only the first leaves the symptom in place. Co-Authored-By: Claude <[email protected]> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
…e score filter
Review follow-ups to "count only providers holding a Public provide key".
1. The location *group* source query in UpdateClientScores was still bare
`connected = true AND valid = true`. It fills locationGroupClientScores ->
the clientScoreLocationGroup* redis keys -> loadClientScores ->
FindProviders2 whenever a spec carries a LocationGroupId, so a user
selecting a promoted group (e.g. "Strong Privacy Laws") still received
providers that cannot accept their contract, and CreateContract rejected
them with NoPermission. Same EXISTS predicate as the other two queries.
2. The UpdateClientScores filter had no test of its own -- every existing
test that reaches it gives all its clients a Public key, so deleting the
clause broke nothing. Two new tests set up connected+valid providers that
are identical except for the Public provide key:
- TestUpdateClientScoresCountsOnlyPublicProviders covers the per-location
query through loadLocationStables (the gate GetProviderLocations uses)
and loadClientScores.
- TestUpdateClientScoresGroupCountsOnlyPublicProviders covers the group
query through the group cache.
Both were verified to fail with their respective EXISTS clause removed.
3. The SQL comments claimed the destination "must hold a key for exactly
that mode". That is not an invariant the code holds:
resolveNonCompanionProvideMode lets a Stream-only destination settle a
cross-network contract as a companion stream. Reworded to state the
intent -- a companion-only destination is a return path, not general
provider supply, so excluding it from provider counts is deliberate.
No SQL change.
Co-Authored-By: Claude <[email protected]>
Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
Beta runs Docker Compose; the mainstream deployment does not use Docker at all. A jail built on Docker networking, or on the process creating its own namespace (needing CAP_NET_ADMIN on a component that is unprivileged today), fits one and breaks the other. The prober now requires no privileges and assumes nothing about how it is confined. Each deployment supplies confinement natively -- a restricted network under compose, systemd IPAddressDeny/IPAddressAllow otherwise -- and the prober verifies it at startup by attempting a direct connection to a geolocation address and refusing to run if it succeeds. That turns correct operator configuration from an assumption the design rests on into a runtime precondition, identically in both environments. Co-Authored-By: Claude <[email protected]> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
It still justified the separate process by namespace confinement after the jail section moved to deployment-supplied restriction. The process boundary is still right, for two reasons that survive: it is the smallest unit either Docker or systemd can restrict, and it keeps geolocation code out of the component holding database credentials. Co-Authored-By: Claude <[email protected]> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
Four tasks: honour forceMinimum on the location read path (the Important finding from the P0 final review), a server-side due-provider endpoint so selection is durable across restarts, a prober startup self-check, and deployment confinement for both environments. The confinement design is portable because it has to be -- beta runs Docker Compose and mainstream runs no Docker at all. The prober takes no capabilities and inspects no mechanism; it tests the property directly by attempting a direct connection to a geolocation address and refusing to start if one succeeds. Co-Authored-By: Claude <[email protected]> Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
This was referenced Jul 26, 2026
…ders discoverable to their own network
Two production risks in the provider counting/scoring change, found by a
backward-compatibility audit of this PR.
1. Country-only clients were counted three times.
A client whose geo lookup resolved neither a city nor a region is stored
with city_location_id = region_location_id = country_location_id --
SetConnectionLocation writes the coarsest available id into the NOT NULL
city/region columns. Both fan-out loops then walked city, region and
country unconditionally, so one such client added 3 to its own country's
provider count and was inserted three times into its country's scoring
map. The inflation is worst exactly where geo resolution is coarsest:
datacenter, mobile and VPN egress.
Both loops now go through the distinct set of location ids, so a client
counts once per location it is actually in. A genuinely city-granular
client still rolls up into its region and its country unchanged -- that
half is asserted explicitly, because a dedupe keyed on the client rather
than on (client, location) would silently stop city clients counting
toward their country, a worse regression than the one being fixed.
(The per-location and per-group scoring maps are keyed by client id, so
they already absorbed the repeat; the count was the live defect. The
loops are made explicit anyway so the two stay in step.)
2. Network-only providers vanished from their own network's discovery.
The Public-only filter is right for the public provider count -- a
stranger genuinely cannot use a ProvideModeNetwork provider -- but wrong
for the candidate pool. Such a provider serves same-network sources today
via the working CreateContractNoEscrow path and is discoverable to them
now; filtering the pool to Public made it undiscoverable to exactly the
users it exists for. That is a live regression for anyone running
providers for their own organisation.
The pool must also not hand a cross-network caller a provider whose
contract will be refused, so this is not a revert:
- UpdateClientLocations keeps the Public-only filter. That number is
shown to everyone and should reflect what a stranger can reach.
- The UpdateClientScores source queries now admit Public or Network --
exactly the two modes GetProvideRelationship can return, hence
exactly what CreateContract can accept -- and record NetworkOnly on
each ClientScore.
- FindProviders2 filters at request time: a candidate is eligible if it
is publicly usable, or if its NetworkId equals the requesting
session's network. This cannot be baked into the cache: the client
score redis entries are keyed by (forceMinimum, rankMode, locationId,
callerLocationId) with no network component, so one cached set is
shared by callers from every network.
NetworkOnly is stored negated deliberately. The score cache is gob
encoded with a 5h ttl, so entries written before the field existed decode
with the zero value; the zero value therefore has to mean "publicly
usable", or every provider would be treated as network-only until the
cache turned over.
The ClientFilter exported alongside the samples still counts only
publicly usable providers. It is read solely by loadLocationStables,
which decides the public Stable flag GetProviderLocations publishes -- a
public surface, so it follows the same rule as the provider count. A
location whose only supply is network-only is not stable and reports no
providers.
No Stream carve-out: resolveNonCompanionProvideMode does return
companion=true for a Stream-only destination, but that dead-ends at
CreateCompanionTransferEscrow, which requires a pre-existing
reverse-direction origin contract, so a Stream-only destination can never
bootstrap a cross-network session. The inline comments that implied
Stream was the only excluded case are corrected.
Tests: a country-only provider counted exactly once and a city provider
still counted at all three granularities, for both UpdateClientLocations
and UpdateClientScores; and all four visibility cases in FindProviders2
(Public and Network-only providers x same-network and other-network
callers). The two existing pool assertions that encoded the Public-only
pool are updated to the new two-tier contract; their location-stability
assertions are unchanged and still pass.
Co-Authored-By: Claude <[email protected]>
Claude-Session: https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg
Ryanmello07
force-pushed
the
feat/provider-count-provide-mode
branch
from
July 26, 2026 15:14
7537390 to
7717b76
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Filters provider counting to clients holding
provide_mode=3. Observed on beta: 39 US providers advertised, 2 reachable.Both
UpdateClientLocationsandUpdateClientScoresare filtered, sinceGetProviderLocationsalso gates onloadLocationStables(populated byUpdateClientScores) — filtering only the first leaves the symptom in place.Two existing beta tests (
TestUpdateClientLocationsCountsClientsWithoutReliabilityScores,TestUpdateClientScoresCountsClientsWithoutReliabilityScores) connected a client with no provide key at all; they now set a Public key explicitly, since the reliability-score join is what they are testing.🤖 Generated with Claude Code
https://claude.ai/code/session_01QtgqtCmKJRXdsQ5ktiqwkg