Skip to content

Commit 6996ce2

Browse files
committed
fix(model): count only verified-healthy providers per location
provider_count counted every connected provider with a Public provide key, with no health or location check, so a location that survived the gate still over-reported. Beta advertised 305 for the US against 151 health-passing providers fleet-wide, 3 of which claimed a country they did not egress from. Apply the shared providerCountFilter at the increment: a provider counts only where a probe measured it healthy and observed it egressing from the country it claims. NULL claimed country fails closed.
1 parent 77f4828 commit 6996ce2

2 files changed

Lines changed: 181 additions & 1 deletion

File tree

model/network_client_location_model.go

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1861,6 +1861,10 @@ func UpdateClientLocations(ctx context.Context, ttl time.Duration) (returnErr er
18611861

18621862
initialClientLocations := &InitialClientLocations{}
18631863

1864+
// one bulk load per pass, outside the tx: this loop runs over the whole
1865+
// provider population
1866+
countFilter := newProviderCountFilter(ctx)
1867+
18641868
server.Tx(ctx, func(tx server.PgTx) {
18651869

18661870
locationClientCounts := map[server.Id]int{}
@@ -1869,9 +1873,13 @@ func UpdateClientLocations(ctx context.Context, ttl time.Duration) (returnErr er
18691873
ctx,
18701874
`
18711875
SELECT
1876+
network_client_location_reliability.client_id,
18721877
network_client_location_reliability.city_location_id,
18731878
network_client_location_reliability.region_location_id,
1874-
network_client_location_reliability.country_location_id
1879+
network_client_location_reliability.country_location_id,
1880+
-- the country the provider CLAIMS, to check against the country a
1881+
-- probe observed it egressing from
1882+
country_location.country_code
18751883
18761884
FROM network_client_location_reliability
18771885
@@ -1892,6 +1900,9 @@ func UpdateClientLocations(ctx context.Context, ttl time.Duration) (returnErr er
18921900
client_connection_reliability_score.client_id = network_client_location_reliability.client_id AND
18931901
client_connection_reliability_score.lookback_index = 0
18941902
1903+
LEFT JOIN location AS country_location ON
1904+
country_location.location_id = network_client_location_reliability.country_location_id
1905+
18951906
WHERE
18961907
network_client_location_reliability.connected = true AND
18971908
network_client_location_reliability.valid = true AND
@@ -1928,15 +1939,35 @@ func UpdateClientLocations(ctx context.Context, ttl time.Duration) (returnErr er
19281939
)
19291940
server.WithPgResult(result, err, func() {
19301941
for result.Next() {
1942+
var clientId server.Id
19311943
var cityLocationId server.Id
19321944
var regionLocationId server.Id
19331945
var countryLocationId server.Id
1946+
var countryCode *string
19341947
server.Raise(result.Scan(
1948+
&clientId,
19351949
&cityLocationId,
19361950
&regionLocationId,
19371951
&countryLocationId,
1952+
&countryCode,
19381953
))
19391954

1955+
// This is the number every app shows when a user picks a
1956+
// location, so count only providers a probe has MEASURED
1957+
// healthy and OBSERVED egressing from the country they claim.
1958+
// Counting on the claim alone advertised providers that were
1959+
// either unreachable or in a different country entirely.
1960+
//
1961+
// countryCode is NULL when the claimed country has no location
1962+
// row, which cannot be verified against anything -- fail closed,
1963+
// same as an unobserved provider.
1964+
if countryCode == nil {
1965+
continue
1966+
}
1967+
if !countFilter.countsTowardCountry(clientId, *countryCode) {
1968+
continue
1969+
}
1970+
19401971
// count each client at most once per distinct location id. A
19411972
// client whose geo lookup resolved neither a city nor a region
19421973
// is stored with city = region = country (see

model/network_client_location_model_test.go

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"slices"
7+
"strings"
78
"testing"
89
"time"
910
"unicode/utf8"
@@ -3236,3 +3237,151 @@ func TestProviderCountFilter(t *testing.T) {
32363237
// comparison is case insensitive on the caller's side
32373238
assert.Equal(t, f.countsTowardCountry(healthy, "US"), true)
32383239
}
3240+
3241+
// Testing_CreateProviderAtLocation inserts exactly the rows a provider needs
3242+
// to clear the pre-existing UpdateClientLocations gate: a network_client row,
3243+
// a Public provide_key, and a network_client_location_reliability row that is
3244+
// connected, valid, and pinned to countryId at all three granularities (city =
3245+
// region = country), mirroring the country-only fallback in
3246+
// SetConnectionLocation (a geo lookup with no city/region resolves to the
3247+
// country id at every column -- see the fix(beta) comment there).
3248+
//
3249+
// It also creates the `location` row for countryId itself: the gated query
3250+
// under test resolves the provider's CLAIMED country by joining
3251+
// network_client_location_reliability.country_location_id back to `location`,
3252+
// and loadClientLocations only surfaces a location that has a `location` row.
3253+
// Both go through raw SQL rather than CreateLocation/SetConnectionLocation
3254+
// because the caller picks countryId up front (so a later lookup can key on
3255+
// it), and CreateLocation always mints its own id.
3256+
//
3257+
// health and observed egress location are deliberately NOT set here -- every
3258+
// caller states its own via SetProviderEgressHealth/SetProviderEgressLocation,
3259+
// exactly as the pre-gate minimums (connected/valid/Public key) are set here
3260+
// while health is layered on top by each test.
3261+
func Testing_CreateProviderAtLocation(
3262+
ctx context.Context,
3263+
networkId server.Id,
3264+
clientId server.Id,
3265+
countryId server.Id,
3266+
countryCode string,
3267+
) {
3268+
countryCode = strings.ToLower(countryCode)
3269+
3270+
server.Tx(ctx, func(tx server.PgTx) {
3271+
server.RaisePgResult(tx.Exec(
3272+
ctx,
3273+
`
3274+
INSERT INTO location (
3275+
location_id,
3276+
location_type,
3277+
location_name,
3278+
country_location_id,
3279+
country_code,
3280+
location_full_name
3281+
)
3282+
VALUES ($1, $2, $3, $1, $3, $3)
3283+
ON CONFLICT (location_id) DO NOTHING
3284+
`,
3285+
countryId,
3286+
LocationTypeCountry,
3287+
countryCode,
3288+
))
3289+
3290+
server.RaisePgResult(tx.Exec(
3291+
ctx,
3292+
`
3293+
INSERT INTO network_client (
3294+
client_id,
3295+
network_id
3296+
)
3297+
VALUES ($1, $2)
3298+
ON CONFLICT (client_id) DO NOTHING
3299+
`,
3300+
clientId,
3301+
networkId,
3302+
))
3303+
3304+
// client_address_hash_count = 1 AND location_count = 1 AND
3305+
// country_location_id IS NOT NULL is exactly the GENERATED `valid`
3306+
// expression on this table (see the CREATE TABLE in db_migrations.go);
3307+
// `valid` itself cannot be assigned directly.
3308+
server.RaisePgResult(tx.Exec(
3309+
ctx,
3310+
`
3311+
INSERT INTO network_client_location_reliability (
3312+
client_id,
3313+
network_id,
3314+
update_block_number,
3315+
city_location_id,
3316+
region_location_id,
3317+
country_location_id,
3318+
client_address_hash_count,
3319+
location_count,
3320+
connected
3321+
)
3322+
VALUES ($1, $2, 0, $3, $3, $3, 1, 1, true)
3323+
ON CONFLICT (client_id) DO UPDATE SET
3324+
network_id = $2,
3325+
city_location_id = $3,
3326+
region_location_id = $3,
3327+
country_location_id = $3,
3328+
client_address_hash_count = 1,
3329+
location_count = 1,
3330+
connected = true
3331+
`,
3332+
clientId,
3333+
networkId,
3334+
countryId,
3335+
))
3336+
})
3337+
3338+
SetProvide(ctx, clientId, map[ProvideMode][]byte{
3339+
ProvideModePublic: []byte("testing-public-provide-secret"),
3340+
})
3341+
}
3342+
3343+
// TestUpdateClientLocationsCountIsGated is the core assertion for this task:
3344+
// UpdateClientLocations must count a provider toward provider_count only where
3345+
// a probe measured it healthy AND observed it egressing from the country it
3346+
// claims. Before this change, connected + valid + a Public provide key was
3347+
// enough on its own -- an unreachable or misrepresenting provider still
3348+
// inflated the count.
3349+
func TestUpdateClientLocationsCountIsGated(t *testing.T) {
3350+
(&server.TestEnv{ApplyDbMigrations: true}).Run(t, func(t testing.TB) {
3351+
ctx := context.Background()
3352+
3353+
networkId := server.NewId()
3354+
countryId := server.NewId()
3355+
3356+
healthy := server.NewId()
3357+
unhealthy := server.NewId()
3358+
unprobed := server.NewId()
3359+
3360+
// three providers in the same country, all connected with a Public
3361+
// provide key; only `healthy` is measured healthy and observed in US
3362+
for _, clientId := range []server.Id{healthy, unhealthy, unprobed} {
3363+
Testing_CreateProviderAtLocation(ctx, networkId, clientId, countryId, "US")
3364+
}
3365+
SetProviderEgressHealth(ctx, &ProviderEgressHealth{
3366+
ClientId: healthy, OKCount: 131, Total: 131, MeasuredAt: server.NowUtc(),
3367+
})
3368+
SetProviderEgressHealth(ctx, &ProviderEgressHealth{
3369+
ClientId: unhealthy, OKCount: 0, Total: 131, MeasuredAt: server.NowUtc(),
3370+
})
3371+
for _, clientId := range []server.Id{healthy, unhealthy} {
3372+
SetProviderEgressLocation(ctx, &ProviderEgressLocation{
3373+
ClientId: clientId, CountryCode: "US",
3374+
Verdict: "verified", ObservedAt: server.NowUtc(),
3375+
})
3376+
}
3377+
3378+
UpdateClientLocations(ctx, 1*time.Hour)
3379+
3380+
clientLocations, err := loadClientLocations(ctx, map[server.Id]bool{countryId: true})
3381+
assert.Equal(t, err, nil)
3382+
3383+
// only the measured-healthy, observed-in-US provider is counted.
3384+
// Before this change all three counted.
3385+
assert.Equal(t, clientLocations[countryId].ClientCount, 1)
3386+
})
3387+
}

0 commit comments

Comments
 (0)