diff --git a/CLAUDE.md b/CLAUDE.md index 2f7bfb2fa..efd1f7763 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -160,9 +160,10 @@ curl -X POST http://localhost:3069/v1 \ - Skip **all** reputation-derived filtering — both the score-threshold/cooldown filter and tiered (highest-tier-only) selection. A score-0, fully-cooled-down supplier is reachable. - Still apply RPC type filtering (only endpoints supporting the requested RPC type) - - Still apply the config `blocked_suppliers` list, the endpoint policy (`require_https` / - `require_domain`), and the supplier blacklist (signature/validation failures). None of these - are reputation, and the header does not override them. + - Still apply the config `blocked_suppliers` list, the `blocked_domains` list (the nuclear + domain ban), the endpoint policy (`require_https` / `require_domain`), and the supplier + blacklist (signature/validation failures). None of these are reputation, and the header + does not override them. - Log filtered supplier list and endpoint counts - If none of the specified suppliers are available in the current session, the request will fail - Header takes precedence over load testing configuration (if any) @@ -292,11 +293,216 @@ router_config: PATH_WEBSOCKET_IDLE_TIMEOUT=45m # pod restart instead of a config-map edit ``` +**Reputation Drain** (`POST /admin/reputation/drain/{serviceId}`) + +Temporarily benches every **scored** endpoint of one operator (eTLD+1) for a service by +writing a cooldown expiry onto its score. Selection already excludes endpoints in cooldown +regardless of score, so this reuses a filter every selection path is guaranteed to consult +rather than adding a second exclusion some path could miss. + +Answers "where would this traffic go if operator X were unavailable" without waiting for X +to fail. + +**Tumble is not a substitute.** A tumble re-dials but leaves every operator eligible, so the +connection can land straight back where it started. Measured on gnosis: **8 consecutive +tumbles failed to move a ~500 frames/s subscription** off the two operators already carrying +it, because each rebind could reselect them. Drain first, *then* tumble — the rebind then has +nowhere else to go. + +```bash +# what would be benched (always do this first — the response lists domains_seen, so a typo +# reads as "matched 0, and here is what actually exists" rather than a silent no-op) +curl -X POST "http://localhost:13069/admin/reputation/drain/gnosis?domain=op-beta.example&rpc_type=websocket&dry_run=true" + +# bench for 20m, websocket only — the operator's json_rpc / rest traffic is untouched +curl -X POST "http://localhost:13069/admin/reputation/drain/gnosis?domain=op-beta.example&rpc_type=websocket&duration=20m" + +# then move the live connections that are already bound +curl -X POST "http://localhost:13069/admin/websocket/tumble/gnosis" + +# release early +curl -X POST "http://localhost:13069/admin/reputation/drain/gnosis?domain=op-beta.example&rpc_type=websocket&duration=0" +``` + +Query parameters: `domain=` (**required**, `url=` is an alias — a drain +with no target would bench the whole service, which is never what anyone meant to type) · +`duration=` (default `15m`; `0` releases) · `rpc_type=` +(default all) · `dry_run=true`. + +**Target by URL/domain, never by node id.** The handler resolves the target against live +endpoint details into *every* identifier a reputation key could carry — full endpoint address, +supplier address, URL, hostname, eTLD+1 — because key granularity is per-service config. The +same operator is a hostname on one service and a `pokt1…` supplier address on another. An +eTLD+1-only filter returns `matched: 0` on a supplier-keyed service while looking like it +worked; check `identifiers_resolved` and `matched_endpoints` in the response to tell "target +names nothing" apart from "names endpoints that carry no score yet". + +**The bench is an overlay, not a score write.** It lives in `drainedKeys` and is applied when +scores are read. This is load-bearing: an earlier version wrote `CooldownUntil` onto the Score, +and `refreshFromStorage` — which overwrites the local cache from Redis unconditionally — +erased every drain within a refresh cycle while the endpoint still reported `drained=N`. +**Anything that must outlive a storage refresh cannot live on the score.** Same trap as the +circuit breaker's `refreshFromRedis`, approached from the other direction. + +**The gate is `GetScores` → `IsInCooldown()`** in `protocol/shannon/reputation.go`, not +`FilterByScore` — that one only compares `Value` against the threshold and ignores cooldown +entirely. A test asserting on `Score.CooldownUntil` proves nothing about whether selection +will honour a drain; assert through `GetScores`. + +**Not a penalty.** `Value`, `CriticalStrikes` and `RecentCriticalRate` are left untouched, so +the quality signal stays readable *while* the drain is in effect — which matters, because +reading it is usually the entire point of draining. A drain that rewrote the score would +destroy the measurement it exists to enable. + +**Release is deliberately narrow:** it lifts only cooldowns the drain itself wrote and that +nothing has overwritten since. A cooldown earned for real while the drain was up survives — +otherwise "undo my experiment" would silently un-bench a legitimately failing endpoint. + +**Unscored endpoints are benched too.** This was once a real gap — the bench lived on the +score, so an endpoint reputation had never observed read as "initial score, not in cooldown" +and stayed selectable. The predicate rewrite closed it: the drain filter matches the +endpoint's **live URL** and runs *before* `GetScores` is consulted, so an endpoint carrying no +score at all is still excluded. The response's `unscored_warning` is retained as a diagnostic +but no longer marks an incomplete drain. + +**A drain applies at the pace of rebinds, not instantly.** Endpoints leave the selectable set +immediately, but a *bound* WebSocket connection only moves at its own next rebind — rollover, +stall or `session_expired`. Measured 2026-08-07: gnosis 42 connections → 0 in ~19 min and bsc +26 → 0 in ~11 min, on rollover alone with no tumble. Budget for that mid-incident. + +**Fleet-wide: ONE call, not one per pod.** Unlike the other admin endpoints, drains are +written to shared storage (a dedicated `__drains__` hash, **never** the score) and every +replica adopts them on its next refresh. Storage is authoritative and the refresh *replaces* +the local set, so a release propagates too — merging would leave a released drain benched +forever on whichever pod did not issue it. + +If the storage write fails the drain still applies locally and the response carries +`propagation_error` plus a warning saying **THIS POD ONLY** — a partial drain nobody realises +is partial is the failure mode worth shouting about. Conversely a storage *outage* never +clears in-force drains; losing Redis mid-incident must not silently un-bench everyone. + +**A drain is never reported as a fault.** `path_endpoints_in_cooldown` counts only cooldowns +an endpoint **earned** — it reads the un-overlaid score. Drains get their own gauge, +`path_endpoints_drained{domain, rpc_type, service_id}`. Folding the two together would make +every drain read as a quality incident on the dashboards the drain exists to let you read, +and would eventually page someone over a bench we applied ourselves. Selection excludes both; +only reporting distinguishes them. + +**It cannot be forgotten.** `duration` is capped at **5h** (rejected, not clamped — silently +shortening a drain is worse than saying no), the shared key carries a TTL past its longest +drain, and expired entries are filtered on read and reaped. There is no way to bench an +operator indefinitely through this endpoint. Re-issue to extend. + +**Domain Blacklist (`blocked_domains`) — the nuclear ban** + +Permanently bans an operator domain from serving specific RPC types on **ALL services**, +where a drain is temporary (5h cap), per-service, and yields when it would empty the pool. + +```yaml +gateway_config: + blocked_domains: + - domain: example.xyz # eTLD+1 (matches every host under it) or exact hostname + rpc_types: [websocket] # omit = every RPC type +``` +```bash +PATH_BLOCKED_DOMAINS=example.xyz:websocket,other.example # pod restart, no config edit +# entry = domain[:type1|type2]; env UNIONS with config — it can widen a ban, never narrow one +``` + +Semantics, all deliberate: +- Covers **every** path that hands out endpoints: primary selection (HTTP + WebSocket, which + also covers retry/hedge/batch and WS rebind — they draw from the filtered pool), **fallback + endpoints** (which bypass every session-endpoint filter and needed explicit coverage), and + **health checks** (paid relays — a nuclear ban stops the probes too). +- **`Target-Suppliers` cannot override it** (unlike drains, which that header bypasses — the + documented gap). The filter runs before the allowlist, next to `blocked_suppliers`. +- **No preferred-endpoint exemption** (drain bug 4's shape) and matching is on the **live + URL**, so it survives session rollovers by construction (drain bug 2's shape). +- **It can empty the pool.** A drain yields as a preference; a ban that yields when the banned + operator is all that remains is not a ban. The request fails instead. +- A malformed entry (typo'd rpc_type, empty domain) **refuses to boot** rather than silently + narrowing the ban. +- Live WS connections: a ban change requires a restart (config or env), and the restart closes + every connection with a clean handshake; reconnects cannot re-select the banned domain. No + separate force-close step exists or is needed. +- Known limitation: health-check suppression is exact for all-type and websocket bans; a ban + covering only a subset of the HTTP-carried types (json_rpc/rest/comet_bft) leaves the + endpoint's other HTTP probes running (the executor keys HTTP checks on endpoint address, + not per-type URL). + +Metrics: `path_blocked_domains_configured{domain, rpc_type}` (1 per entry at startup — +"is the ban loaded on this pod", readable at any LOG_LEVEL) and +`path_endpoints_domain_blocked_total{domain, rpc_type, service_id}` (counted per selection +pass — the honest "is it actually engaging" signal). Configured-but-never-engaging is a red +flag: verify before trusting, four drain bugs reported benched while serving. + +Tests: `protocol/shannon/domain_blocklist_test.go` asserts through the production callers +(`getSessionsUniqueEndpoints`, `getUniqueEndpoints`, `GetEndpointsForHealthCheck`), and every +call site was revert-checked (filter removed → tests fail). + **Circuit Breaker — when to use:** - After deploying a fix for a bug that caused false positive circuit breaker lockouts - When a domain is stuck in circuit breaker state due to a transient issue that has resolved - Rolling restarts alone don't work because `refreshFromRedis` repopulates in-memory state from Redis +## WebSocket Frames Are Reward-Eligible Relays + +**Every endpoint→client WebSocket frame is signed by the relay miner and mined as a +reward-eligible relay**, paired with the *most recent* request. poktroll +`pkg/relayer/proxy/websockets/bridge.go`: + +> Each message (inbound or outbound) is treated as a reward-eligible relay. For example, with +> eth_subscribe, both the initial subscription request and each received event would be +> eligible for rewards. […] Currently, the RelayMiner is paid for each incoming and outgoing +> message transmitted. + +PATH validates that signature in `validateEndpointWebsocketMessage` → `ValidateRelayResponse` +(`protocol/shannon/websocket_context.go`). + +**The asymmetry:** HTTP is 1 client request = 1 relay, client-driven. A WS subscription is 1 +signed request = **unbounded** relays, and the push rate is chosen by **the supplier being +paid**. PATH signs the anchoring subscribe with its *own* application key, so the gateway's app +stake funds it. The only brake is `relayMeter.IsOverServicing(...)` — the application's +per-session allowance — and it engages on almost nothing. Measured 2026-08-07: +`path_supplier_exhausted_total` fires on **bsc alone** and nowhere else on the fleet, bursting +0 → ~11/s over ~4h with long zero troughs between. An earlier note recording it as 0 fleetwide +was a snapshot that landed in a trough — read this counter over a range, never with an instant +query. On every other service the brake is effectively dormant. + +Consequence: a per-domain frames/s number is closer to a **settlement-volume** meter than a +demand meter. Do not reason about it as load on the supplier. + +### `path_websocket_connection_frame_rate` — the distribution, not the sum + +Histogram `{service_id, domain}`, every live connection observed every 15s, buckets +`0.1 … 2500`. Emitted from the same `sampleRates` pass that feeds the tumble ranking, so the +two can never disagree. + +**Why it exists:** `path_websocket_messages_total` is a per-domain SUM, and a sum cannot tell +*one firehose plus a hundred idle sockets* apart from *a hundred ordinary subscribers*. Those +have opposite explanations and the difference is not academic — measured fleetwide, one +operator held **16.9% of WS connections and earned 66.1% of WS relays** (3.9× over-index), +while on gnosis **2 connections out of ~70 carried ~97% of frames**. A handful of connections +can produce that entire fleetwide number without the operator doing anything. + +```promql +histogram_quantile(0.5, sum by (le, domain) (rate(path_websocket_connection_frame_rate_bucket{service_id="gnosis"}[10m]))) +histogram_quantile(0.99, sum by (le, domain) (rate(path_websocket_connection_frame_rate_bucket{service_id="gnosis"}[10m]))) +``` + +**Read it as:** p50 ≈ 0 with p99 in the hundreds → a few firehoses landed there, a placement +artifact, nobody is doing anything. p50 materially above other operators → systematic across +that operator's whole connection population. + +**Idle connections are observed at 0 deliberately.** Dropping them would leave the quantiles +describing only the connections that carry traffic — exactly the population the metric exists +to be measured *against*. + +**Trap:** clients do not choose their operator; selection assigns it. So which operator a +high-volume subscriber lands on is effectively a random draw, and any per-operator *average* +conflates "inflates every stream" with "the big streams landed here". Only the distribution — +or a same-client-different-operator comparison via a drain — separates them. + ## Endpoint Selection — Registration-Weighted, Capped per Operator A provider's share of a service follows the **supplier registrations** it holds, not the machines it runs. Each registration carries its own per-session service allowance, so registrations are both what a provider can actually serve and what the chain settles on. How a provider spreads its registrations across its own infrastructure is not a routing input. @@ -374,6 +580,46 @@ Recorded on **every** band pick, so `outcome="reshaped"` over the total is the r **What to watch after enabling:** `path_supplier_exhausted_total` for the **thin** operators the excess lands on, not the capped one — a solo-registration backend gains share while still holding one supplier's per-session allowance. Same failure mode as the backend-URL dedup, and self-correcting. Retry success rate — `path_relays_total{request_type="retry"}` split by `status_code` — must not fall; roughly 60% of retries already fail, so that pool is marginal to begin with. +## Testing Changes That Affect Routing + +Three separate bugs shipped in the admin-drain feature, all with passing tests, all the same +mistake: **the test asserted on something the author wrote, not on the value the production +caller receives.** Each reported success in production while excluding nothing. + +1. The bench was written onto `Score.CooldownUntil` — `refreshFromStorage` overwrites the + cache from Redis unconditionally and erased it within a refresh cycle. Test asserted + "`CooldownUntil` is set", which was true, briefly. +2. The bench resolved to a fixed set of `EndpointKey`s. `EndpointAddr` embeds the supplier + address and sessions rotate their supplier set, so it went stale every rollover (~20 min). + Every test used a static cache, so nothing rotated. +3. The filter deleted from the `endpoints` map passed in, while the function builds its result + by walking `cached`. Nothing was ever excluded; the Warn log never fired. + +**Use the selection harness** (`protocol/shannon/selection_harness_test.go`). It answers the +only question that matters — *does selection still return this endpoint?* — in one call: + +```go +s := newSelectionScenario(t, "gnosis", opBetaA, opAlphaA, opGammaA) +s.Drain("op-beta.example", sharedtypes.RPCType_WEBSOCKET, time.Hour) +s.AssertExcluded(sharedtypes.RPCType_WEBSOCKET, opBetaA) +s.RotateSuppliers(1) // simulates a session rollover +s.AssertExcluded(sharedtypes.RPCType_WEBSOCKET, opBetaA) +``` + +Three rules, all cheap: + +- **Assert on the production caller's return value**, never on a helper, a cached field, or a + gauge. Scores, drain maps, `path_endpoints_drained` and `/ready` have each reported a bench + that selection did not honour. +- **Before committing a bug fix, revert the fix and confirm the test fails.** Done twice + during this work: caught nothing the first time, caught the real bug the third. +- **Two observables disagreeing about the same state is a bug** — stop and find it. When + `path_endpoints_drained` said 60 benched while `/ready` said 0 excluded, that contradiction + was the bug announcing itself and it was rationalised away twice. + +Anything keyed on `EndpointAddr` must be tested across `RotateSuppliers`. That key embeds a +supplier address, and supplier sets rotate every session. + ## Testing Strategy - **Unit Tests** - Standard Go tests with `-short` flag diff --git a/cmd/main.go b/cmd/main.go index e9b37e4d7..c7b68f2d6 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -38,6 +38,22 @@ var ( // the executable to get the full path to the config file. const defaultConfigPath = "config/.config.yaml" +// websocketShutdownTimeout bounds the close-handshake sweep over live websocket +// connections at termination. +// +// The sweep runs the closes concurrently, so this is a ceiling on the slowest peer rather +// than a per-connection cost — five seconds is far past the one-second write deadline on +// each close frame, and comfortably inside the pod's 30s termination grace period. Being +// polite must never be the reason a pod gets SIGKILLed, which would produce exactly the +// abrupt teardown the sweep exists to prevent. +const websocketShutdownTimeout = 5 * time.Second + +// websocketShutdowner is implemented by protocols that hold live websocket bridges and +// must close them explicitly at termination. +type websocketShutdowner interface { + ShutdownWebsockets(ctx context.Context, reason string) int +} + func main() { log.Printf(`{"level":"info","message":"PATH 🌿 gateway starting..."}`) @@ -347,6 +363,14 @@ func main() { // failing startup. websocketAdmin, _ := protocol.(router.WebsocketAdmin) + // Admin handler to temporarily bench one operator's endpoints for a service via + // POST /admin/reputation/drain/{serviceId}. nil when reputation is disabled, which + // leaves the endpoint reporting 503 rather than failing startup. + var reputationAdmin router.ReputationAdmin + if reputationSvc := protocol.GetReputationService(); reputationSvc != nil { + reputationAdmin = reputationSvc + } + // Initialize the API router to serve requests to the PATH API. apiRouter := router.NewRouter( logger, @@ -357,6 +381,7 @@ func main() { gtw.DomainCircuitBreaker, chainStateAdmin, websocketAdmin, + reputationAdmin, unifiedServicesConfig, ) @@ -433,6 +458,30 @@ func main() { logger.Info().Msg("Shutting down PATH...") + // Close live websocket connections FIRST, and with a close handshake. + // + // server.Shutdown below cannot do this: it explicitly does not close hijacked + // connections, and every websocket is hijacked. Without this the process just exits, + // every socket dies with its TCP connection, and both peers see an abnormal closure + // (1006) — indistinguishable from a crash for the client, and logged as their own + // fault by the endpoint. Fleetwide that made every rollout emit a burst of 1006s + // across all services within the same second. + // + // Before backgroundCancel() so the bridges are torn down deliberately rather than + // racing a context cancellation that would reach them as a generic failure. + // + // Its own short budget, well inside the pod's termination grace period: the close + // frames are best-effort and a peer that will not answer must not delay exiting. + // + // Behind an assertion rather than a method on gateway.Protocol: holding live + // websocket bridges is a property of the protocol implementation, not of the + // interface, and a protocol that holds none needs nothing here. + if wsShutdowner, ok := protocol.(websocketShutdowner); ok { + wsCtx, wsCancel := context.WithTimeout(context.Background(), websocketShutdownTimeout) + wsShutdowner.ShutdownWebsockets(wsCtx, "gateway shutting down") + wsCancel() + } + // Cancel background context to stop all background services (pprof, health checks) backgroundCancel() diff --git a/config/config.schema.yaml b/config/config.schema.yaml index 3724eff6d..91761f131 100644 --- a/config/config.schema.yaml +++ b/config/config.schema.yaml @@ -394,6 +394,19 @@ properties: description: "Enable/disable health checks." type: boolean default: false + sync_allowance: + description: "Default blocks behind the latest an endpoint may be before it counts as out of sync. Per-service entries in local[] override it. 0 disables the sync check." + type: integer + minimum: 0 + default: 0 + max_workers: + description: "Maximum concurrent health check workers. Auto-sized from service and check counts when unset." + type: integer + minimum: 1 + backend_dedup: + description: "Fire ONE relay per unique backend URL each cycle (a rotating representative supplier) and fan the result to the other suppliers sharing that URL. Cuts redundant health-check relay volume by the redundancy factor. WebSocket checks are never deduped. Default: true." + type: boolean + default: true coordination: description: "Leader election configuration for multi-instance deployments." type: object @@ -447,6 +460,10 @@ properties: description: "Enable/disable health checks for this service." type: boolean default: true + sync_allowance: + description: "Blocks behind the latest an endpoint may be before it counts as out of sync. Overrides the global default for this service. 0 disables the sync check." + type: integer + minimum: 0 checks: description: "List of health check configurations." type: array @@ -461,8 +478,17 @@ properties: description: "Name of the health check." type: string type: - description: "Type of health check (e.g., 'jsonrpc')." + description: "RPC protocol type (must match service's rpc_types): json_rpc, rest, comet_bft, websocket, grpc." type: string + enum: ["json_rpc", "rest", "comet_bft", "websocket", "grpc"] + enabled: + description: "Enable/disable this individual check." + type: boolean + headers: + description: "Request headers." + type: object + additionalProperties: + type: string method: description: "HTTP method (GET, POST)." type: string @@ -481,10 +507,17 @@ properties: timeout: description: "Request timeout." type: string + archival: + description: "Archival-specific check: only runs on endpoints marked archival." + type: boolean + sync_check: + description: "Enable sync check validation. When true, extracts block height from the response and validates against the service's sync_allowance. Requires sync_allowance > 0." + type: boolean + default: false reputation_signal: description: "Reputation signal to emit on failure." type: string - enum: ["minor_error", "major_error", "critical_error"] + enum: ["minor_error", "major_error", "critical_error", "fatal_error"] # ==================================== # UNIFIED SERVICE CONFIGURATION diff --git a/gateway/health_check_executor.go b/gateway/health_check_executor.go index 45af070c6..07fd2339a 100644 --- a/gateway/health_check_executor.go +++ b/gateway/health_check_executor.go @@ -902,7 +902,7 @@ func (e *HealthCheckExecutor) recordCheckResult( // Over-servicing rejections — same no-penalty rule as the request path. // Without this, every health-check probe to an exhausted supplier records a // MajorError signal and pins their reputation at 0 even after the request - // path has stopped penalizing them. Production canary observed easy2stake's + // path has stopped penalizing them. Production canary observed operator-zeta's // BSC supplier set stuck at score=0 with success-only request-path signals // because the health-check executor was draining them in parallel. if heuristic.IsOverServicedError(checkErr.Error()) { @@ -1245,8 +1245,21 @@ func (e *HealthCheckExecutor) ExecuteCheckViaProtocol( } // Heuristic analysis - detect bad gateway, empty responses, and other error patterns - // This runs BEFORE QoS validation to catch issues the basic validation might miss - heuristicResult := heuristic.Analyze(responseBody, httpStatusCode, servicePayload.EffectiveRPCType(), "") + // This runs BEFORE QoS validation to catch issues the basic validation might miss. + // + // The method must be threaded through here exactly as the other four call sites do it + // (protocol/shannon/context.go, gateway/hedge.go, http_request_context_handle_request.go). + // This call passed a hardcoded "" - so even with JSONRPCMethod populated on the payload, + // every method-aware rule was blind at this site, and this is the site that decides the + // health check's outcome. A CometBFT `health` response + // ({"jsonrpc":"2.0","id":1,"result":{}}) is flagged jsonrpc_empty_object_result at + // confidence 0.95 unless the method is known, which maps to SignalMajorError below. + jsonrpcMethod := servicePayload.JSONRPCMethod + if jsonrpcMethod == "" { + // REST checks carry no JSON-RPC method; path-aware rules key off Path instead. + jsonrpcMethod = servicePayload.Path + } + heuristicResult := heuristic.Analyze(responseBody, httpStatusCode, servicePayload.EffectiveRPCType(), jsonrpcMethod) if heuristicResult.ShouldRetry { heuristicErr := fmt.Errorf("heuristic detected error: %s - %s", heuristicResult.Reason, heuristicResult.Details) e.logger.Debug(). @@ -1628,12 +1641,25 @@ func (e *HealthCheckExecutor) buildServicePayload(check HealthCheckConfig) proto rpcType = sharedtypes.RPCType_UNKNOWN_RPC } + // JSONRPCMethod drives the method-aware heuristic checks. User traffic gets it + // from QoS parsing; a health check never goes through QoS, so without this it + // stayed empty and the heuristic fell back to Path — which for a JSON-RPC check + // is "/", matching no method at all. + // + // That broke the CometBFT carve-out in analyzeJSONRPC: `health` legitimately + // answers {"jsonrpc":"2.0","id":1,"result":{}}, and the empty-object rule is + // skipped only when rpcType is COMET_BFT or isCometBFTMethod(method) holds. + // Cosmos services declare the check as type json_rpc, so neither guard fired and + // every healthy node was scored jsonrpc_empty_object_result → retry + minor_error. + // Measured 2026-08-06: 16 cosmos services failing this check 100%, ~19% of all + // failing health-check signals fleet-wide. return protocol.Payload{ - Method: check.Method, - Path: check.Path, - Data: check.Body, - Headers: headers, - RPCType: rpcType, // Set from aligned health check type + Method: check.Method, + Path: check.Path, + Data: check.Body, + Headers: headers, + RPCType: rpcType, // Set from aligned health check type + JSONRPCMethod: extractJSONRPCMethod([]byte(check.Body)), } } diff --git a/gateway/health_check_executor_test.go b/gateway/health_check_executor_test.go index 9713cdf48..5d8f2f6ff 100644 --- a/gateway/health_check_executor_test.go +++ b/gateway/health_check_executor_test.go @@ -12,7 +12,9 @@ import ( sharedtypes "github.com/pokt-network/poktroll/x/shared/types" "github.com/stretchr/testify/require" + protocolobservations "github.com/pokt-network/path/observation/protocol" "github.com/pokt-network/path/protocol" + "github.com/pokt-network/path/qos/heuristic" "github.com/pokt-network/path/reputation" ) @@ -247,7 +249,7 @@ func TestConfigMergingSyncAllowance(t *testing.T) { // mockQoSServiceWithSyncAllowance is a minimal mock that tracks SetSyncAllowance calls. type mockQoSServiceWithSyncAllowance struct { - QoSService // embed interface — only SetSyncAllowance is used + QoSService // embed interface — only SetSyncAllowance is used syncAllowance uint64 called bool } @@ -579,7 +581,7 @@ func (m *recordedSignalReputationSvc) RecordedSignals() []reputation.Signal { // TestRecordCheckResult_OverServicedSkipsPenalty pins the invariant that the // health-check executor must not penalize a supplier when the relay-miner has // signaled the application's per-session stake budget is exhausted. Without -// this skip the executor was draining easy2stake's BSC supplier set to score=0 +// this skip the executor was draining operator-zeta's BSC supplier set to score=0 // in production despite the request-path no-penalty fix. func TestRecordCheckResult_OverServicedSkipsPenalty(t *testing.T) { cases := []struct { @@ -823,3 +825,213 @@ func TestFanOutcomeToSibling_OverServicedNotPenalized(t *testing.T) { require.Empty(t, rep.RecordedSignals(), "fanned over-serviced failures must not penalize the sibling") } + +// TestBuildServicePayload_JSONRPCMethod pins that the health check payload carries the +// JSON-RPC method from its body. +// +// Regression: the field was never set, so protocol/shannon/context.go fell back to +// payload.Path ("/") when calling the heuristic. That defeated the CometBFT carve-out +// in analyzeJSONRPC — `health` returns {"jsonrpc":"2.0","id":1,"result":{}}, which the +// empty-object rule flags unless rpcType is COMET_BFT or the method is recognized as +// CometBFT. Cosmos services declare the check as type json_rpc, so every healthy node +// was retried and penalized minor_error on every cycle. +func TestBuildServicePayload_JSONRPCMethod(t *testing.T) { + e := &HealthCheckExecutor{logger: polyzero.NewLogger()} + + tests := []struct { + name string + check HealthCheckConfig + wantMethod string + }{ + { + name: "cometbft health over json_rpc carries the method", + check: HealthCheckConfig{ + Name: "health", + Type: "json_rpc", + Method: "POST", + Path: "/", + Body: `{"jsonrpc":"2.0","id":1,"method":"health"}`, + }, + wantMethod: "health", + }, + { + name: "evm method carries through", + check: HealthCheckConfig{ + Name: "eth_blockNumber", + Type: "json_rpc", + Method: "POST", + Path: "/", + Body: `{"jsonrpc":"2.0","id":1,"method":"eth_blockNumber","params":[]}`, + }, + wantMethod: "eth_blockNumber", + }, + { + name: "rest check has no json-rpc method", + check: HealthCheckConfig{ + Name: "syncing", + Type: "rest", + Method: "GET", + Path: "/cosmos/base/tendermint/v1beta1/syncing", + }, + wantMethod: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + payload := e.buildServicePayload(tt.check) + require.Equal(t, tt.wantMethod, payload.JSONRPCMethod) + // Path must stay intact: REST path-aware validation still depends on it. + require.Equal(t, tt.check.Path, payload.Path) + }) + } +} + +// TestCometBFTHealthResponseNotFlaggedByHeuristic is the end-to-end assertion behind the +// fix: with the method populated, a healthy CometBFT `health` response must survive the +// heuristic that previously flagged it. +// +// BOTH heuristic sites are asserted. The response passes through them in order, and +// either one alone fails the check: +// +// site 1 - protocol/shannon/context.go, after the relay returns +// site 2 - health_check_executor.go, which decides the check's outcome +// +// Fixing only site 1 changes nothing observable: the response simply reaches site 2 and +// is rejected there on the same rule, moving the metric from status_code="error" to +// status_code="200" while cosmos `health` stays at 100% failure. +func TestCometBFTHealthResponseNotFlaggedByHeuristic(t *testing.T) { + e := &HealthCheckExecutor{logger: polyzero.NewLogger()} + payload := e.buildServicePayload(HealthCheckConfig{ + Name: "health", + Type: "json_rpc", + Method: "POST", + Path: "/", + Body: `{"jsonrpc":"2.0","id":1,"method":"health"}`, + }) + + // The correct CometBFT answer from a healthy node. + healthyResponse := []byte(`{"jsonrpc":"2.0","id":1,"result":{}}`) + + // Both sites resolve the method the same way: payload method, Path as fallback. + jsonrpcMethod := payload.JSONRPCMethod + if jsonrpcMethod == "" { + jsonrpcMethod = payload.Path + } + require.Equal(t, "health", jsonrpcMethod, "method must survive payload construction") + + t.Run("site 1: protocol layer", func(t *testing.T) { + result := heuristic.Analyze(healthyResponse, http.StatusOK, payload.RPCType, jsonrpcMethod) + require.False(t, result.ShouldRetry, + "healthy CometBFT health response must not be flagged (got %q: %s)", result.Reason, result.Details) + }) + + t.Run("site 2: health check executor", func(t *testing.T) { + // EffectiveRPCType, matching the executor's own call. + result := heuristic.Analyze(healthyResponse, http.StatusOK, payload.EffectiveRPCType(), jsonrpcMethod) + require.False(t, result.ShouldRetry, + "healthy CometBFT health response must not be flagged (got %q: %s)", result.Reason, result.Details) + }) +} + +// healthCheckProtocolCtx returns a canned backend response to the health check relay. +type healthCheckProtocolCtx struct { + body string + statusCode int +} + +func (m *healthCheckProtocolCtx) HandleServiceRequest([]protocol.Payload) ([]protocol.Response, error) { + return []protocol.Response{{ + Bytes: []byte(m.body), + HTTPStatusCode: m.statusCode, + EndpointAddr: "pokt1a-https://a.example.com", + }}, nil +} + +func (m *healthCheckProtocolCtx) SetParentContext(context.Context) {} +func (m *healthCheckProtocolCtx) MarkAsHedge() {} +func (m *healthCheckProtocolCtx) MarkAsRetry() {} +func (m *healthCheckProtocolCtx) MarkAsHealthCheck() {} +func (m *healthCheckProtocolCtx) GetObservations() protocolobservations.Observations { + return protocolobservations.Observations{} +} + +// healthCheckProtocol hands the executor the canned context above. +type healthCheckProtocol struct { + mockProtocolForRetry + protocolCtx *healthCheckProtocolCtx +} + +func (m *healthCheckProtocol) BuildHTTPRequestContextForEndpoint( + _ context.Context, + _ protocol.ServiceID, + _ protocol.EndpointAddr, + _ sharedtypes.RPCType, + _ *http.Request, + _ bool, +) (ProtocolRequestContext, protocolobservations.Observations, error) { + return m.protocolCtx, protocolobservations.Observations{}, nil +} + +// TestExecuteCheckViaProtocol_CometBFTHealthPasses drives the real executor path, which is +// what makes this a regression test rather than a restatement of the heuristic's rules. +// +// The executor runs its OWN heuristic.Analyze after the relay returns. That call passed a +// hardcoded "", so populating JSONRPCMethod on the payload alone left cosmos `health` at +// 100% failure - the response cleared the protocol-layer check and was then rejected here. +func TestExecuteCheckViaProtocol_CometBFTHealthPasses(t *testing.T) { + tests := []struct { + name string + check HealthCheckConfig + body string + wantError bool + }{ + { + name: "healthy cometbft health response passes", + check: HealthCheckConfig{ + Name: "health", Type: "json_rpc", Method: "POST", Path: "/", + Body: `{"jsonrpc":"2.0","id":1,"method":"health"}`, + ExpectedStatusCode: 200, + }, + body: `{"jsonrpc":"2.0","id":1,"result":{}}`, + wantError: false, + }, + { + // The rule must still bite where it was meant to: an EVM method has no + // business returning an empty object. + name: "evm empty object result is still rejected", + check: HealthCheckConfig{ + Name: "eth_blockNumber", Type: "json_rpc", Method: "POST", Path: "/", + Body: `{"jsonrpc":"2.0","id":1,"method":"eth_blockNumber","params":[]}`, + ExpectedStatusCode: 200, + }, + body: `{"jsonrpc":"2.0","id":1,"result":{}}`, + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := &HealthCheckExecutor{ + logger: polyzero.NewLogger(), + protocol: &healthCheckProtocol{protocolCtx: &healthCheckProtocolCtx{body: tt.body, statusCode: 200}}, + } + + _, err := e.ExecuteCheckViaProtocol( + context.Background(), + protocol.ServiceID("pocket"), + protocol.EndpointAddr("pokt1a-https://a.example.com"), + tt.check, + 0, // syncAllowance: sync check disabled + nil, // capture + ) + + if tt.wantError { + require.Error(t, err) + require.Contains(t, err.Error(), "jsonrpc_empty_object_result") + return + } + require.NoError(t, err, "healthy CometBFT health response must pass the executor's own heuristic") + }) + } +} diff --git a/gateway/unified_service_config.go b/gateway/unified_service_config.go index 5ff91edff..55691e503 100644 --- a/gateway/unified_service_config.go +++ b/gateway/unified_service_config.go @@ -114,6 +114,26 @@ type EndpointPolicyConfig struct { RequireDomain bool `yaml:"require_domain,omitempty"` } +// BlockedDomainConfig is one entry of the gateway-operator domain blocklist — the +// nuclear ban. Every endpoint whose URL is at Domain (exact hostname or eTLD+1 match) +// is permanently excluded from serving the listed RPC types on EVERY service: primary +// selection, retry/hedge, WebSocket (initial bind and rebind), fallback endpoints, and +// health checks. Nothing overrides it — not Target-Suppliers, not a preferred/bound +// endpoint — and unlike an admin drain it does not yield when it would empty the pool. +// +// Known limitation: health-check suppression is exact for all-type bans and for +// websocket bans; a ban covering only a subset of the HTTP-carried types (json_rpc, +// rest, comet_bft) still lets the endpoint's other HTTP health checks probe it. +type BlockedDomainConfig struct { + // Domain is an eTLD+1 ("op-alpha.example", matching every host under it) or an exact + // hostname ("s019.op-alpha.example", matching only that host). Case-insensitive. + Domain string `yaml:"domain"` + + // RPCTypes lists the banned RPC types ("websocket", "json_rpc", "rest", + // "comet_bft", "grpc"). Empty means every RPC type. + RPCTypes []string `yaml:"rpc_types,omitempty"` +} + // ServiceReputationConfig holds per-service reputation configuration. type ServiceReputationConfig struct { Enabled *bool `yaml:"enabled,omitempty"` diff --git a/metrics/leaderboard.go b/metrics/leaderboard.go index 643f6b69a..db2bed430 100644 --- a/metrics/leaderboard.go +++ b/metrics/leaderboard.go @@ -63,7 +63,14 @@ type LeaderboardDataProvider interface { // GetCooldownCountData returns per-(domain, service_id, rpc_type) counts of // endpoints currently in strike cooldown. Optional: implementations may return // nil if cooldown tracking is not supported. + // + // Must count only EARNED cooldowns. Endpoints benched by an admin drain belong in + // GetDrainedCountData, or a deliberate bench reads as a fault on every dashboard. GetCooldownCountData(ctx context.Context) ([]CooldownCountEntry, error) + + // GetDrainedCountData returns per-(domain, service_id, rpc_type) counts of endpoints + // benched by an admin drain. Optional: implementations may return nil. + GetDrainedCountData(ctx context.Context) ([]CooldownCountEntry, error) } // LeaderboardPublisher publishes endpoint leaderboard metrics every 10 seconds @@ -216,6 +223,23 @@ func (lp *LeaderboardPublisher) publishLeaderboard(ctx context.Context) { } lp.logger.Debug().Int("entries", len(cooldownCounts)).Msg("Published cooldown counts") } + + // Publish admin-drain counts on their own series, for the same reset reason: a drain + // that expires must read as zero rather than sticking at its last value. + drainedCounts, err := lp.provider.GetDrainedCountData(ctx) + if err != nil { + lp.logger.Warn().Err(err).Msg("Failed to get drained count data") + return + } + + EndpointsDrained.Reset() + + if len(drainedCounts) > 0 { + for _, entry := range drainedCounts { + EndpointsDrained.WithLabelValues(entry.Domain, entry.RPCType, entry.ServiceID).Set(float64(entry.Count)) + } + lp.logger.Warn().Int("entries", len(drainedCounts)).Msg("⚠️ endpoints are benched by an admin drain") + } } // PublishOnce can be called to manually trigger a leaderboard publish (for testing) diff --git a/metrics/metrics.go b/metrics/metrics.go index ae671f363..af596c7ee 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -475,11 +475,56 @@ func RecordCircuitBreakerEvent(serviceID, domain, reasonCategory, event string) var EndpointsInCooldown = promauto.NewGaugeVec( prometheus.GaugeOpts{ Name: MetricPrefix + "endpoints_in_cooldown", - Help: "Number of endpoints currently in strike cooldown (Score.CooldownUntil in the future). Published every 10s. Cooldown is independent from score-below-threshold — an endpoint can be cooldown'd even with a high score after critical strikes.", + Help: "Number of endpoints in a cooldown they EARNED (Score.CooldownUntil in the future). Excludes endpoints benched by an admin drain — see path_endpoints_drained. Published every 10s. Cooldown is independent from score-below-threshold — an endpoint can be cooldown'd even with a high score after critical strikes.", }, []string{LabelDomain, LabelRPCType, LabelServiceID}, ) +// EndpointsDrained counts endpoints benched by an admin drain rather than by anything they +// did. +// +// Deliberately a SEPARATE series from path_endpoints_in_cooldown. A drain removes an +// operator's traffic exactly the way a cooldown does, so folding the two together makes a +// deliberate bench indistinguishable from a quality incident on every dashboard — and a +// drain exists to OBSERVE an operator, so contaminating the signal defeats its purpose. +// Nobody should be paged over a drain we applied ourselves. +var EndpointsDrained = promauto.NewGaugeVec( + prometheus.GaugeOpts{ + Name: MetricPrefix + "endpoints_drained", + Help: "Number of endpoints currently benched by an admin drain (POST /admin/reputation/drain). Not a fault: these were removed from selection deliberately and the bench expires on its own. Published every 10s.", + }, + []string{LabelDomain, LabelRPCType, LabelServiceID}, +) + +// EndpointsDomainBlockedTotal counts endpoints removed from selection by the +// gateway-operator domain blocklist (blocked_domains config / PATH_BLOCKED_DOMAINS). +// +// Counted per selection pass, so the RATE tracks how often the ban is actually engaging +// — the honest signal, after four drain bugs whose gauges reported benched endpoints +// selection kept serving. Zero while path_blocked_domains_configured is nonzero means no +// banned endpoint appeared in any session, OR the ban is not engaging — investigate +// before trusting it. +var EndpointsDomainBlockedTotal = promauto.NewCounterVec( + prometheus.CounterOpts{ + Name: MetricPrefix + "endpoints_domain_blocked_total", + Help: "Endpoints removed from a selection pass by the gateway-operator domain blocklist (blocked_domains / PATH_BLOCKED_DOMAINS). Deliberate config-driven bans, not faults.", + }, + []string{LabelDomain, LabelRPCType, LabelServiceID}, +) + +// BlockedDomainsConfigured reports the (domain, rpc_type) entries the domain blocklist +// booted with — set once at startup, 1 per entry, rpc_type="all" for all-type bans. +// Exists because LOG_LEVEL=error hides the startup log, and "is the ban even loaded on +// this pod" must be answerable from Prometheus. Compare with +// path_endpoints_domain_blocked_total: configured but never engaging is a red flag. +var BlockedDomainsConfigured = promauto.NewGaugeVec( + prometheus.GaugeOpts{ + Name: MetricPrefix + "blocked_domains_configured", + Help: "1 for each (domain, rpc_type) entry in the gateway-operator domain blocklist on this pod (rpc_type=\"all\" = every type). Set at startup.", + }, + []string{LabelDomain, LabelRPCType}, +) + // ============================================================================= // Supplier Blacklist Events (Counter) // Labels: domain, supplier, service_id, reason @@ -1195,6 +1240,36 @@ var WebsocketMessagesTotal = promauto.NewCounterVec( []string{LabelDomain, LabelServiceID, "direction", LabelReputationSignal}, ) +// WebsocketConnectionFrameRate observes EVERY live connection's endpoint→client frames/sec +// on each sampler pass, bucketed per service and operator. +// +// Why a histogram when path_websocket_messages_total already exists: that counter is a per +// domain SUM, and a sum cannot distinguish "one firehose plus a hundred idle sockets" from +// "a hundred ordinary subscribers". Those two have completely different explanations — +// the first is where a high-volume client happened to land, the second is a property of +// the operator — and per-operator earnings differ by ~4x fleetwide on exactly that +// ambiguity. Only the distribution separates them. +// +// Both frame directions are reward-eligible under the relay miner (each endpoint→client +// push is signed and mined paired with the most recent request), so this is closer to a +// settlement-volume distribution than a traffic one. +// +// Idle connections are observed at 0 deliberately. Dropping them would leave the p50 +// describing only the connections that carry traffic, which is precisely the population +// the metric exists to size against everything else. +// +// Buckets span one frame per ten seconds (a newHeads subscription on a slow chain) to +// thousands per second (a logs/pendingTransactions firehose), with resolution concentrated +// in 1..500 where the interesting separation lives. +var WebsocketConnectionFrameRate = promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: MetricPrefix + "websocket_connection_frame_rate", + Help: "Per-connection endpoint→client frames/sec, sampled periodically, by domain and service_id. Reveals the DISTRIBUTION that path_websocket_messages_total sums away.", + Buckets: []float64{0.1, 0.5, 1, 2, 5, 10, 25, 50, 100, 250, 500, 1000, 2500}, + }, + []string{LabelDomain, LabelServiceID}, +) + // WebsocketRebindTotal counts websocket session-rebind episodes by outcome. // EXPERIMENTAL / canary observability for the session-rebind feature // (PATH_WEBSOCKET_SESSION_REBIND). result is one of the WSRebind* labels above: @@ -1637,6 +1712,13 @@ func RecordWebsocketMessage(domain, serviceID, direction, reputationSignal strin WebsocketMessagesTotal.WithLabelValues(domain, serviceID, direction, reputationSignal).Inc() } +// RecordWebsocketConnectionFrameRate observes one live connection's current frames/sec. +// Called once per connection per sampler pass, including for connections currently at +// zero — see WebsocketConnectionFrameRate for why the zeros are load-bearing. +func RecordWebsocketConnectionFrameRate(domain, serviceID string, framesPerSec float64) { + WebsocketConnectionFrameRate.WithLabelValues(domain, serviceID).Observe(framesPerSec) +} + // RecordWebsocketRebind records a websocket session-rebind episode outcome and, on // success, the number of subscriptions replayed. EXPERIMENTAL / canary observability // for the session-rebind feature. result should be one of the WSRebind* labels; trigger diff --git a/protocol/shannon/config.go b/protocol/shannon/config.go index 95fca1ee8..6946a59ea 100644 --- a/protocol/shannon/config.go +++ b/protocol/shannon/config.go @@ -108,6 +108,12 @@ type ( // Allows gateway operators to enforce HTTPS-only, domain-only, etc. EndpointPolicy gateway.EndpointPolicyConfig `yaml:"endpoint_policy,omitempty"` + // BlockedDomains permanently bans operator domains from serving specific RPC types + // on ALL services — the nuclear ban. See gateway.BlockedDomainConfig for semantics. + // The PATH_BLOCKED_DOMAINS env var appends entries at pod-restart speed + // (union — env can widen a ban, never narrow one). + BlockedDomains []gateway.BlockedDomainConfig `yaml:"blocked_domains,omitempty"` + // UnifiedServices is the unified YAML-driven service configuration. // This consolidates all per-service settings (type, rpc_types, fallback, health_checks) // into a single structure with defaults and per-service overrides. diff --git a/protocol/shannon/domain_blocklist.go b/protocol/shannon/domain_blocklist.go new file mode 100644 index 000000000..79df65cc6 --- /dev/null +++ b/protocol/shannon/domain_blocklist.go @@ -0,0 +1,216 @@ +package shannon + +import ( + "fmt" + "net/url" + "sort" + "strings" + "sync" + "sync/atomic" + + sharedtypes "github.com/pokt-network/poktroll/x/shared/types" + + "github.com/pokt-network/path/gateway" + shannonmetrics "github.com/pokt-network/path/metrics/protocol/shannon" +) + +// envBlockedDomains appends ban entries at pod-restart speed, without a config rollout. +// Entries are comma-separated: "domain" bans every RPC type, "domain:type1|type2" bans +// only those types. Example: +// +// PATH_BLOCKED_DOMAINS=op-alpha.example:websocket,op-beta.example:websocket,evil.example +// +// Env entries are UNIONED with the blocked_domains config list — the env var can widen +// a ban but never narrow one. +const envBlockedDomains = "PATH_BLOCKED_DOMAINS" + +// maxDomainDecisionCacheEntries bounds decisionCache as a safety net; endpoint URLs are +// a bounded set so this ceiling is never expected to be reached (same rationale as +// rawIPCache in endpoint_policy.go). +const maxDomainDecisionCacheEntries = 1 << 16 // 65536 + +// domainBlocklist is the compiled form of the gateway-operator (domain, rpc_type) +// blocklist — the nuclear ban. An endpoint whose URL matches a blocked domain is removed +// from EVERY path that hands out endpoints: primary selection (HTTP and WebSocket, +// which also covers retry/hedge/batch and the WebSocket rebind, since they all draw from +// the pool this filters), fallback endpoints, and health checks. +// +// Contrast with the two things it deliberately is not: +// - blocked_suppliers: keyed on supplier address, per-service. Supplier addresses +// rotate with sessions and one operator holds many; a domain names the operator's +// infrastructure directly. +// - admin drains: temporary (5h cap), per-service, reputation-overlay based, and they +// yield when the pool would empty. This blocklist is permanent (config-driven), +// fleet-wide across all services, and does NOT yield. +// +// A nil *domainBlocklist is valid and blocks nothing; all methods are nil-safe. +type domainBlocklist struct { + // blocked maps a lowercase domain — either an eTLD+1 ("op-alpha.example") or an exact + // hostname ("s019.op-alpha.example") — to the set of banned RPC types. + // A nil set bans every RPC type. + blocked map[string]map[sharedtypes.RPCType]struct{} + + // decisionCache memoizes rawURL -> matched blocklist key ("" = no match). The filter + // runs per endpoint per selection on the relay hot path, and the uncached path does a + // url.Parse. The blocklist is immutable after startup, so entries never invalidate. + decisionCache sync.Map // map[string]string + decisionCacheCount atomic.Int64 +} + +// newDomainBlocklist compiles config entries into a matcher. Returns (nil, nil) when the +// list is empty. Returns an error — refusing to boot — on an empty domain or an unknown +// rpc_type: silently narrowing or dropping a nuclear ban is worse than failing loudly. +func newDomainBlocklist(entries []gateway.BlockedDomainConfig) (*domainBlocklist, error) { + if len(entries) == 0 { + return nil, nil + } + + mapper := gateway.NewRPCTypeMapper() + blocked := make(map[string]map[sharedtypes.RPCType]struct{}, len(entries)) + + for _, e := range entries { + domain := strings.ToLower(strings.TrimSpace(e.Domain)) + if domain == "" { + return nil, fmt.Errorf("blocked_domains: entry with an empty domain") + } + + existing, seen := blocked[domain] + + // No rpc_types = ban everything, absorbing any narrower entry for the domain. + if len(e.RPCTypes) == 0 { + blocked[domain] = nil + continue + } + // Already banned for everything; a narrower entry cannot un-ban. + if seen && existing == nil { + continue + } + + set := existing + if set == nil { + set = make(map[sharedtypes.RPCType]struct{}, len(e.RPCTypes)) + } + for _, t := range e.RPCTypes { + rpcType, err := mapper.ParseRPCType(strings.TrimSpace(t)) + if err != nil { + return nil, fmt.Errorf("blocked_domains: domain %q: %w", domain, err) + } + set[rpcType] = struct{}{} + } + blocked[domain] = set + } + + return &domainBlocklist{blocked: blocked}, nil +} + +// parseBlockedDomainsEnv parses the PATH_BLOCKED_DOMAINS value into config entries. +// Malformed pieces are not silently dropped here — an empty domain surfaces as an error +// from newDomainBlocklist. +func parseBlockedDomainsEnv(raw string) []gateway.BlockedDomainConfig { + raw = strings.TrimSpace(raw) + if raw == "" { + return nil + } + + var entries []gateway.BlockedDomainConfig + for _, piece := range strings.Split(raw, ",") { + piece = strings.TrimSpace(piece) + if piece == "" { + continue + } + domain, typesStr, hasTypes := strings.Cut(piece, ":") + entry := gateway.BlockedDomainConfig{Domain: strings.TrimSpace(domain)} + if hasTypes { + for _, t := range strings.Split(typesStr, "|") { + if t = strings.TrimSpace(t); t != "" { + entry.RPCTypes = append(entry.RPCTypes, t) + } + } + } + entries = append(entries, entry) + } + return entries +} + +// IsBlocked reports whether an endpoint at rawURL is banned from serving rpcType. +// Matching is on the URL — never on EndpointAddr or supplier address — so a ban survives +// session rollovers by construction: an endpoint rotated into a session at a blocked +// domain is banned the moment it appears (the lesson of drain bug 2). +func (b *domainBlocklist) IsBlocked(rawURL string, rpcType sharedtypes.RPCType) bool { + if b == nil || rawURL == "" { + return false + } + + key := b.matchKey(rawURL) + if key == "" { + return false + } + + set := b.blocked[key] + if set == nil { + return true // banned for every RPC type + } + _, banned := set[rpcType] + return banned +} + +// matchKey resolves rawURL to the blocklist key it matches ("" = none), memoized. +func (b *domainBlocklist) matchKey(rawURL string) string { + if v, ok := b.decisionCache.Load(rawURL); ok { + return v.(string) + } + + key := b.computeMatchKey(rawURL) + + if b.decisionCacheCount.Load() < maxDomainDecisionCacheEntries { + if _, loaded := b.decisionCache.LoadOrStore(rawURL, key); !loaded { + b.decisionCacheCount.Add(1) + } + } + return key +} + +// computeMatchKey checks the exact hostname first (most specific), then the eTLD+1. +func (b *domainBlocklist) computeMatchKey(rawURL string) string { + if parsed, err := url.Parse(rawURL); err == nil { + if host := strings.ToLower(parsed.Hostname()); host != "" { + if _, ok := b.blocked[host]; ok { + return host + } + } + } + if domain, err := shannonmetrics.ExtractDomainOrHost(rawURL); err == nil { + domain = strings.ToLower(domain) + if _, ok := b.blocked[domain]; ok { + return domain + } + } + return "" +} + +// configuredEntries returns the compiled (domain, rpc_type) pairs for startup logging and +// the path_blocked_domains_configured gauge, sorted for stable output. An all-types ban +// is reported as rpc_type "all". +func (b *domainBlocklist) configuredEntries() [][2]string { + if b == nil { + return nil + } + mapper := gateway.NewRPCTypeMapper() + var out [][2]string + for domain, set := range b.blocked { + if set == nil { + out = append(out, [2]string{domain, "all"}) + continue + } + for rpcType := range set { + out = append(out, [2]string{domain, mapper.FormatRPCType(rpcType)}) + } + } + sort.Slice(out, func(i, j int) bool { + if out[i][0] != out[j][0] { + return out[i][0] < out[j][0] + } + return out[i][1] < out[j][1] + }) + return out +} diff --git a/protocol/shannon/domain_blocklist_test.go b/protocol/shannon/domain_blocklist_test.go new file mode 100644 index 000000000..69fea190d --- /dev/null +++ b/protocol/shannon/domain_blocklist_test.go @@ -0,0 +1,537 @@ +package shannon + +import ( + "context" + "errors" + "sort" + "testing" + + "github.com/pokt-network/poktroll/pkg/polylog/polyzero" + apptypes "github.com/pokt-network/poktroll/x/application/types" + sessiontypes "github.com/pokt-network/poktroll/x/session/types" + sharedtypes "github.com/pokt-network/poktroll/x/shared/types" + sdk "github.com/pokt-network/shannon-sdk" + "github.com/stretchr/testify/require" + + "github.com/pokt-network/path/gateway" + "github.com/pokt-network/path/protocol" +) + +// ============================================================================= +// Domain blocklist — the nuclear ban +// ============================================================================= +// +// Everything below asserts through the PRODUCTION callers — getSessionsUniqueEndpoints, +// getUniqueEndpoints, GetEndpointsForHealthCheck — not through the blocklist's own maps. +// Four drain bugs shipped with passing tests because the tests asserted on state the +// author wrote instead of the value the caller receives; see selection_harness_test.go. + +func compileBlocklist(t *testing.T, entries ...gateway.BlockedDomainConfig) *domainBlocklist { + t.Helper() + bl, err := newDomainBlocklist(entries) + require.NoError(t, err) + return bl +} + +// --- matcher units -------------------------------------------------------------------- + +func TestDomainBlocklist_MatchesETLDPlusOneAndExactHost(t *testing.T) { + bl := compileBlocklist(t, + gateway.BlockedDomainConfig{Domain: "op-alpha.example"}, + gateway.BlockedDomainConfig{Domain: "n1.op-gamma.example"}, + ) + + // eTLD+1 entry matches every host under it. + require.True(t, bl.IsBlocked("https://s019.op-alpha.example", sharedtypes.RPCType_JSON_RPC)) + require.True(t, bl.IsBlocked("wss://other.op-alpha.example/ws", sharedtypes.RPCType_WEBSOCKET)) + + // Exact-hostname entry matches only that host. + require.True(t, bl.IsBlocked("https://n1.op-gamma.example", sharedtypes.RPCType_JSON_RPC)) + require.False(t, bl.IsBlocked("https://n2.op-gamma.example", sharedtypes.RPCType_JSON_RPC), + "an exact-hostname entry must not ban the operator's other hosts") + + require.False(t, bl.IsBlocked("https://f019.op-beta.example", sharedtypes.RPCType_JSON_RPC)) +} + +func TestDomainBlocklist_RPCTypeScoping(t *testing.T) { + bl := compileBlocklist(t, + gateway.BlockedDomainConfig{Domain: "op-beta.example", RPCTypes: []string{"websocket"}}, + gateway.BlockedDomainConfig{Domain: "op-alpha.example"}, // all types + ) + + require.True(t, bl.IsBlocked("wss://f019.op-beta.example", sharedtypes.RPCType_WEBSOCKET)) + require.False(t, bl.IsBlocked("https://f019.op-beta.example", sharedtypes.RPCType_JSON_RPC), + "a websocket-only ban must not take the operator's HTTP traffic with it") + + for _, rpcType := range []sharedtypes.RPCType{ + sharedtypes.RPCType_JSON_RPC, sharedtypes.RPCType_WEBSOCKET, + sharedtypes.RPCType_REST, sharedtypes.RPCType_COMET_BFT, + } { + require.True(t, bl.IsBlocked("https://s019.op-alpha.example", rpcType), + "an entry with no rpc_types must ban every type (got through on %s)", rpcType) + } +} + +func TestDomainBlocklist_AllTypesEntryAbsorbsNarrowerOne(t *testing.T) { + // Order must not matter: (ws-only, all) and (all, ws-only) both mean "everything". + for _, entries := range [][]gateway.BlockedDomainConfig{ + {{Domain: "op-alpha.example", RPCTypes: []string{"websocket"}}, {Domain: "op-alpha.example"}}, + {{Domain: "op-alpha.example"}, {Domain: "op-alpha.example", RPCTypes: []string{"websocket"}}}, + } { + bl := compileBlocklist(t, entries...) + require.True(t, bl.IsBlocked("https://s019.op-alpha.example", sharedtypes.RPCType_JSON_RPC), + "a narrower entry must never un-ban a type the all-types entry covers") + } +} + +func TestDomainBlocklist_EnvParsing(t *testing.T) { + entries := parseBlockedDomainsEnv(" op-alpha.example:websocket , op-beta.example:websocket|json_rpc ,evil.example,, ") + require.Equal(t, []gateway.BlockedDomainConfig{ + {Domain: "op-alpha.example", RPCTypes: []string{"websocket"}}, + {Domain: "op-beta.example", RPCTypes: []string{"websocket", "json_rpc"}}, + {Domain: "evil.example"}, + }, entries) + + require.Nil(t, parseBlockedDomainsEnv("")) + require.Nil(t, parseBlockedDomainsEnv(" ")) +} + +func TestDomainBlocklist_RejectsBadConfig(t *testing.T) { + _, err := newDomainBlocklist([]gateway.BlockedDomainConfig{ + {Domain: "op-alpha.example", RPCTypes: []string{"websockets"}}, // typo + }) + require.Error(t, err, "an unknown rpc_type must refuse to boot, not silently narrow the ban") + + _, err = newDomainBlocklist([]gateway.BlockedDomainConfig{{Domain: " "}}) + require.Error(t, err, "an empty domain must refuse to boot") +} + +func TestDomainBlocklist_NilBlocksNothing(t *testing.T) { + var bl *domainBlocklist + require.False(t, bl.IsBlocked("https://s019.op-alpha.example", sharedtypes.RPCType_JSON_RPC)) + + empty, err := newDomainBlocklist(nil) + require.NoError(t, err) + require.Nil(t, empty) +} + +// --- production caller: getSessionsUniqueEndpoints ------------------------------------ + +// blocklistScenario drives the REAL selection entry point (getSessionsUniqueEndpoints) +// with a session whose endpoint map is pre-seeded into the session cache — the same +// harnessEndpoint fixtures the selection harness uses, with the supplier address and +// backend URL kept separate so a session rollover can be simulated. +type blocklistScenario struct { + t *testing.T + p *Protocol + serviceID protocol.ServiceID + sessionID string + urls []string + // supplierOf maps a URL to the supplier fronting it; RotateSuppliers swaps these. + supplierOf map[string]string +} + +func newBlocklistScenario( + t *testing.T, + serviceID string, + entries []gateway.BlockedDomainConfig, + urls ...string, +) *blocklistScenario { + t.Helper() + s := &blocklistScenario{ + t: t, + p: &Protocol{logger: polyzero.NewLogger(), blockedDomains: compileBlocklist(t, entries...)}, + serviceID: protocol.ServiceID(serviceID), + sessionID: "session-gen0", + urls: append([]string(nil), urls...), + supplierOf: make(map[string]string, len(urls)), + } + for i, u := range urls { + s.supplierOf[u] = supplierAddrForIndex(i, 0) + } + s.seedSession() + return s +} + +func (s *blocklistScenario) seedSession() { + eps := make(map[protocol.EndpointAddr]endpoint, len(s.urls)) + for _, u := range s.urls { + ep := &harnessEndpoint{supplier: s.supplierOf[u], url: u} + eps[ep.Addr()] = ep + } + s.p.sessionEndpointsCache.Store(s.sessionID, eps) +} + +// RotateSuppliers simulates a session rollover: same backend URLs, fresh supplier +// addresses, new session ID. +func (s *blocklistScenario) RotateSuppliers(generation int) { + s.t.Helper() + s.sessionID = "session-gen" + string(rune('0'+generation)) + for i, u := range s.urls { + s.supplierOf[u] = supplierAddrForIndex(i, generation) + } + s.seedSession() +} + +func (s *blocklistScenario) session() sessiontypes.Session { + return sessiontypes.Session{ + SessionId: s.sessionID, + Header: &sessiontypes.SessionHeader{SessionId: s.sessionID, ServiceId: string(s.serviceID)}, + Application: &apptypes.Application{Address: "pokt1app"}, + } +} + +// survivors returns the backend URLs the production selection entry point still hands +// out, under the given call shape (allowlist / preferred endpoint / reputation flag). +func (s *blocklistScenario) survivors( + rpcType sharedtypes.RPCType, + filterByReputation bool, + allowedSuppliers []string, + preferredURL string, +) []string { + s.t.Helper() + var preferred protocol.EndpointAddr + if preferredURL != "" { + preferred = protocol.EndpointAddr(s.supplierOf[preferredURL] + "-" + preferredURL) + } + got, _, err := s.p.getSessionsUniqueEndpoints( + context.Background(), s.serviceID, []sessiontypes.Session{s.session()}, + filterByReputation, rpcType, allowedSuppliers, preferred, + ) + if err != nil { + // An emptied pool is a legitimate outcome of a nuclear ban — the request fails + // rather than being served by a banned operator. Anything else is a real error. + require.ErrorIs(s.t, err, errProtocolContextSetupNoEndpoints) + return nil + } + out := make([]string, 0, len(got)) + for _, ep := range got { + out = append(out, ep.GetURL(rpcType)) + } + sort.Strings(out) + return out +} + +func TestSelection_DomainBlocklistRemovesBannedOperator(t *testing.T) { + s := newBlocklistScenario(t, "gnosis", + []gateway.BlockedDomainConfig{{Domain: "op-beta.example", RPCTypes: []string{"websocket"}}}, + opBetaA, opBetaB, opAlphaA, opGammaA) + + got := s.survivors(sharedtypes.RPCType_WEBSOCKET, true, nil, "") + require.NotContains(t, got, opBetaA) + require.NotContains(t, got, opBetaB) + require.Contains(t, got, opAlphaA) + require.Contains(t, got, opGammaA) +} + +func TestSelection_DomainBlocklistScopedToRPCType(t *testing.T) { + s := newBlocklistScenario(t, "gnosis", + []gateway.BlockedDomainConfig{{Domain: "op-beta.example", RPCTypes: []string{"websocket"}}}, + opBetaA, opGammaA) + + require.NotContains(t, s.survivors(sharedtypes.RPCType_WEBSOCKET, true, nil, ""), opBetaA) + require.Contains(t, s.survivors(sharedtypes.RPCType_JSON_RPC, true, nil, ""), opBetaA, + "a websocket ban must not take the operator's HTTP traffic with it") +} + +// Target-Suppliers bypasses reputation (including drains — a documented, open gap). +// It must NOT bypass this blocklist: the filter runs before the allowlist, same as +// blocked_suppliers, so a banned operator is unreachable even when explicitly pinned. +func TestSelection_DomainBlocklistIgnoresTargetSuppliers(t *testing.T) { + s := newBlocklistScenario(t, "gnosis", + []gateway.BlockedDomainConfig{{Domain: "op-beta.example", RPCTypes: []string{"websocket"}}}, + opBetaA, opGammaA) + + got := s.survivors(sharedtypes.RPCType_WEBSOCKET, true, []string{s.supplierOf[opBetaA]}, "") + require.NotContains(t, got, opBetaA, + "Target-Suppliers must not resurrect a banned operator — this ban is nuclear") +} + +// Drain bug 4's shape: every websocket rebind passes the endpoint it is already bound to +// as the preferred endpoint, and an exemption there means the ban never applies to the +// connections that matter. removeBlockedDomains takes no preferred endpoint at all; this +// pins that the full production path cannot re-pick a banned endpoint either. +func TestSelection_DomainBlocklistAppliesEvenToPreferredEndpoint(t *testing.T) { + s := newBlocklistScenario(t, "gnosis", + []gateway.BlockedDomainConfig{{Domain: "op-beta.example", RPCTypes: []string{"websocket"}}}, + opBetaA, opBetaB, opAlphaA, opGammaA) + + got := s.survivors(sharedtypes.RPCType_WEBSOCKET, true, nil, opBetaA) + require.NotContains(t, got, opBetaA) + require.NotContains(t, got, opBetaB) + require.Contains(t, got, opAlphaA) +} + +// Drain bug 2's shape: state keyed on EndpointAddr silently lifts at the next rollover, +// because the supplier set rotates while the backend URLs stay. The ban matches on the +// live URL, so it must survive any number of rotations. +func TestSelection_DomainBlocklistSurvivesSupplierRotation(t *testing.T) { + s := newBlocklistScenario(t, "gnosis", + []gateway.BlockedDomainConfig{{Domain: "op-beta.example", RPCTypes: []string{"websocket"}}}, + opBetaA, opAlphaA, opGammaA) + + require.NotContains(t, s.survivors(sharedtypes.RPCType_WEBSOCKET, true, nil, ""), opBetaA) + + s.RotateSuppliers(1) + require.NotContains(t, s.survivors(sharedtypes.RPCType_WEBSOCKET, true, nil, ""), opBetaA) + + s.RotateSuppliers(2) + require.NotContains(t, s.survivors(sharedtypes.RPCType_WEBSOCKET, true, nil, ""), opBetaA) +} + +// Health checks and leaderboard gathering call with filterByReputation=false, which +// skips reputation, drains, the supplier blacklist and session-exhaustion. The domain +// blocklist must NOT be among the things that flag turns off. +func TestSelection_DomainBlocklistAppliesToReputationFreeCalls(t *testing.T) { + s := newBlocklistScenario(t, "gnosis", + []gateway.BlockedDomainConfig{{Domain: "op-beta.example", RPCTypes: []string{"websocket"}}}, + opBetaA, opGammaA) + + require.NotContains(t, s.survivors(sharedtypes.RPCType_WEBSOCKET, false, nil, ""), opBetaA, + "filterByReputation=false must not bypass the domain blocklist") +} + +// Unlike a drain — which yields rather than empty the pool, because a drain is a +// preference — the nuclear ban empties it. Serving a request from an operator the config +// says must never serve it is worse than failing the request. +func TestSelection_DomainBlocklistCanEmptyThePool(t *testing.T) { + s := newBlocklistScenario(t, "gnosis", + []gateway.BlockedDomainConfig{{Domain: "op-beta.example"}}, + opBetaA, opBetaB) + + require.Empty(t, s.survivors(sharedtypes.RPCType_JSON_RPC, true, nil, ""), + "a nuclear ban must hold even when the banned operator is all that remains") +} + +// --- production caller: getUniqueEndpoints (fallback endpoints) ----------------------- + +// Fallback endpoints are handed out raw by getUniqueEndpoints, bypassing every +// session-endpoint filter — the ban must cover them explicitly or it has a +// fallback-shaped hole. +func TestGetUniqueEndpoints_DomainBlocklistCoversFallbackEndpoints(t *testing.T) { + p := &Protocol{ + logger: polyzero.NewLogger(), + blockedDomains: compileBlocklist(t, + gateway.BlockedDomainConfig{Domain: "op-beta.example"}), + serviceFallbackMap: map[protocol.ServiceID]serviceFallback{ + "gnosis": { + Endpoints: map[protocol.EndpointAddr]endpoint{ + "fb-operator-beta": &harnessEndpoint{supplier: "fallback", url: opBetaA}, + "fb-operator-gamma": &harnessEndpoint{supplier: "fallback", url: opGammaA}, + }, + SendAllTraffic: true, + }, + }, + } + + got, _, err := p.getUniqueEndpoints( + context.Background(), "gnosis", nil, true, sharedtypes.RPCType_JSON_RPC, nil, "") + require.NoError(t, err) + + urls := make([]string, 0, len(got)) + for _, ep := range got { + urls = append(urls, ep.GetURL(sharedtypes.RPCType_JSON_RPC)) + } + require.NotContains(t, urls, opBetaA, "send-all-traffic fallback must still honor the ban") + require.Contains(t, urls, opGammaA) + + // The shared config map must not have been mutated by the filter. + require.Len(t, p.serviceFallbackMap["gnosis"].Endpoints, 2, + "the filter must clone the shared fallback map, never mutate it") +} + +func TestGetUniqueEndpoints_BannedFallbackCannotRescueEmptyPool(t *testing.T) { + p := &Protocol{ + logger: polyzero.NewLogger(), + blockedDomains: compileBlocklist(t, + gateway.BlockedDomainConfig{Domain: "op-beta.example"}), + serviceFallbackMap: map[protocol.ServiceID]serviceFallback{ + "gnosis": { + Endpoints: map[protocol.EndpointAddr]endpoint{ + "fb-operator-beta": &harnessEndpoint{supplier: "fallback", url: opBetaA}, + }, + }, + }, + } + + // No sessions, and the only fallback is banned: the request must fail rather than + // be served by the banned operator. + _, _, err := p.getUniqueEndpoints( + context.Background(), "gnosis", nil, true, sharedtypes.RPCType_JSON_RPC, nil, "") + require.Error(t, err) +} + +// --- production caller: NewProtocol (the wiring itself) ------------------------------- + +// Every scenario above hand-builds Protocol{blockedDomains: ...}, which leaves the one +// step none of them can see: does NewProtocol actually compile the config + env into the +// field selection reads? Deleting the constructor wiring would keep every other test +// green while shipping an inert ban — the exact shape of all four drain bugs. + +type constructorFullNode struct{ FullNode } + +func (f *constructorFullNode) GetAccountClient() *sdk.AccountClient { return &sdk.AccountClient{} } + +// Any valid secp256k1 scalar works; this key exists only so newSigner constructs. +const constructorTestKeyHex = "0000000000000000000000000000000000000000000000000000000000000001" + +func newProtocolViaConstructor(t *testing.T) (*Protocol, error) { + t.Helper() + // Keep the constructor goroutine-free: no throughput sampler, no reputation service. + t.Setenv("PATH_WEBSOCKET_SESSION_REBIND", "false") + return NewProtocol(context.Background(), polyzero.NewLogger(), GatewayConfig{ + GatewayAddress: "pokt1gateway", + GatewayPrivateKeyHex: constructorTestKeyHex, + BlockedDomains: []gateway.BlockedDomainConfig{ + {Domain: "cfgonly.example", RPCTypes: []string{"json_rpc"}}, + }, + }, &constructorFullNode{}) +} + +func TestNewProtocol_WiresDomainBlocklistFromConfigAndEnv(t *testing.T) { + t.Setenv(envBlockedDomains, "envonly.example:websocket") + + p, err := newProtocolViaConstructor(t) + require.NoError(t, err) + + // Both sources must reach the compiled blocklist (env is a union with config). + require.True(t, p.blockedDomains.IsBlocked("https://a.cfgonly.example", sharedtypes.RPCType_JSON_RPC)) + require.True(t, p.blockedDomains.IsBlocked("wss://a.envonly.example", sharedtypes.RPCType_WEBSOCKET)) + + // And the constructed instance must actually EXCLUDE through real selection — the + // field being set is necessary but not sufficient. + eps := make(map[protocol.EndpointAddr]endpoint) + for i, u := range []string{"wss://a.envonly.example", opGammaA} { + ep := &harnessEndpoint{supplier: supplierAddrForIndex(i, 0), url: u} + eps[ep.Addr()] = ep + } + p.sessionEndpointsCache.Store("session-ctor", eps) + session := sessiontypes.Session{ + SessionId: "session-ctor", + Header: &sessiontypes.SessionHeader{SessionId: "session-ctor", ServiceId: "gnosis"}, + Application: &apptypes.Application{Address: "pokt1app"}, + } + got, _, err := p.getSessionsUniqueEndpoints( + context.Background(), "gnosis", []sessiontypes.Session{session}, + true, sharedtypes.RPCType_WEBSOCKET, nil, "") + require.NoError(t, err) + urls := make([]string, 0, len(got)) + for _, ep := range got { + urls = append(urls, ep.GetURL(sharedtypes.RPCType_WEBSOCKET)) + } + require.NotContains(t, urls, "wss://a.envonly.example", + "a ban present only in the env var must exclude through the constructed Protocol") + require.Contains(t, urls, opGammaA) +} + +func TestNewProtocol_RefusesToBootOnMalformedEnvBan(t *testing.T) { + t.Setenv(envBlockedDomains, "envonly.example:websockets") // typo'd rpc type + + _, err := newProtocolViaConstructor(t) + require.Error(t, err, + "a malformed nuclear ban must refuse to boot, not silently drop the entry") +} + +// --- production caller: GetEndpointsForHealthCheck ------------------------------------ + +// healthCheckFullNode serves a fixed session; block-height and shared-params queries +// fail, which GetEndpointsForHealthCheck treats leniently (no session-expiry filter). +type healthCheckFullNode struct { + FullNode + session sessiontypes.Session +} + +func (f *healthCheckFullNode) IsInSessionRollover() bool { return false } +func (f *healthCheckFullNode) GetSession(_ context.Context, _ protocol.ServiceID, _ string) (sessiontypes.Session, error) { + return f.session, nil +} + +// Failing the height lookup exercises the lenient path: no session-expiry filtering. +func (f *healthCheckFullNode) GetCurrentBlockHeight(_ context.Context) (int64, error) { + return 0, errors.New("no chain in this test") +} +func (f *healthCheckFullNode) GetSessionWithExtendedValidity(ctx context.Context, serviceID protocol.ServiceID, appAddr string) (sessiontypes.Session, error) { + return f.GetSession(ctx, serviceID, appAddr) +} + +func newHealthCheckBlocklistProtocol(t *testing.T, entries ...gateway.BlockedDomainConfig) *Protocol { + t.Helper() + session := sessiontypes.Session{ + SessionId: "session-hc", + Header: &sessiontypes.SessionHeader{SessionId: "session-hc", ServiceId: "gnosis", ApplicationAddress: "pokt1app"}, + // The delegation check runs against the session's embedded Application. + Application: &apptypes.Application{ + Address: "pokt1app", + DelegateeGatewayAddresses: []string{"pokt1gateway"}, + }, + } + p := &Protocol{ + logger: polyzero.NewLogger(), + FullNode: &healthCheckFullNode{session: session}, + gatewayMode: protocol.GatewayModeCentralized, + gatewayAddr: "pokt1gateway", + ownedApps: map[protocol.ServiceID][]string{"gnosis": {"pokt1app"}}, + blockedDomains: compileBlocklist(t, entries...), + unifiedServicesConfig: &gateway.UnifiedServicesConfig{ + Services: []gateway.ServiceConfig{{ + ID: "gnosis", + HealthChecks: &gateway.ServiceHealthCheckOverride{ + Local: []gateway.HealthCheckConfig{ + {Name: "block", Type: gateway.HealthCheckTypeJSONRPC}, + {Name: "ws", Type: gateway.HealthCheckTypeWebSocket}, + }, + }, + }}, + }, + } + + eps := make(map[protocol.EndpointAddr]endpoint) + for i, u := range []string{opBetaA, opGammaA} { + ep := &harnessEndpoint{supplier: supplierAddrForIndex(i, 0), url: u} + eps[ep.Addr()] = ep + } + p.sessionEndpointsCache.Store("session-hc", eps) + return p +} + +// Health checks are paid relays; an all-type ban must stop the probes entirely, not +// just the user traffic. +func TestGetEndpointsForHealthCheck_AllTypeBanExcludesEndpoint(t *testing.T) { + p := newHealthCheckBlocklistProtocol(t, + gateway.BlockedDomainConfig{Domain: "op-beta.example"}) + + infos, err := p.GetEndpointsForHealthCheck()("gnosis") + require.NoError(t, err) + require.NotEmpty(t, infos, "the unbanned endpoint must still be probed") + + for _, info := range infos { + require.NotContains(t, string(info.Addr), "op-beta.example", + "a fully banned endpoint must receive no health-check probes at all") + } +} + +// A websocket-only ban is enforced by omitting WebSocketURL — the executor gates its +// websocket probe on WebSocketURL != "" — while HTTP probes continue. +func TestGetEndpointsForHealthCheck_WebsocketBanBlanksWebSocketURL(t *testing.T) { + p := newHealthCheckBlocklistProtocol(t, + gateway.BlockedDomainConfig{Domain: "op-beta.example", RPCTypes: []string{"websocket"}}) + + infos, err := p.GetEndpointsForHealthCheck()("gnosis") + require.NoError(t, err) + + var sawOpBeta, sawOpGamma bool + for _, info := range infos { + switch info.HTTPURL { + case opBetaA: + sawOpBeta = true + require.Empty(t, info.WebSocketURL, + "a websocket ban must strip the WebSocket probe URL") + case opGammaA: + sawOpGamma = true + require.NotEmpty(t, info.WebSocketURL, + "the unbanned endpoint must keep its WebSocket probe") + } + } + require.True(t, sawOpBeta, "a websocket-only ban must not remove the endpoint's HTTP probes") + require.True(t, sawOpGamma) +} diff --git a/protocol/shannon/endpoint_filters.go b/protocol/shannon/endpoint_filters.go index 44a54b8a4..5eacef487 100644 --- a/protocol/shannon/endpoint_filters.go +++ b/protocol/shannon/endpoint_filters.go @@ -4,7 +4,10 @@ import ( "strings" "github.com/pokt-network/poktroll/pkg/polylog" + sharedtypes "github.com/pokt-network/poktroll/x/shared/types" + "github.com/pokt-network/path/metrics" + shannonmetrics "github.com/pokt-network/path/metrics/protocol/shannon" "github.com/pokt-network/path/protocol" ) @@ -49,6 +52,58 @@ func removeBlockedSuppliers( return removed } +// removeBlockedDomains deletes endpoints whose URL is at a domain the gateway operator +// has banned for this RPC type (blocked_domains config / PATH_BLOCKED_DOMAINS) and counts +// each removal on path_endpoints_domain_blocked_total. Returns the number removed. +// +// Deliberately NO exemption for a requested/preferred endpoint and NO pool-empty safety +// net. The drain grants neither either (the preferred-endpoint exemption is what defeated +// it — every WebSocket rebind prefers the endpoint it is already bound to), and this +// blocklist is stricter than a drain: a ban that yields when the banned operator is all +// that remains is not a ban. If the pool empties, the request fails rather than being +// served by an operator the config says must never serve it. +// +// Matching is on the endpoint's live URL, never on EndpointAddr — supplier sets rotate +// every session, so an addr-keyed ban silently lifts at the next rollover (drain bug 2). +func removeBlockedDomains( + endpoints map[protocol.EndpointAddr]endpoint, + blocklist *domainBlocklist, + rpcType sharedtypes.RPCType, + serviceID protocol.ServiceID, + logger polylog.Logger, +) int { + if blocklist == nil || len(endpoints) == 0 { + return 0 + } + + rpcTypeLabel := strings.ToLower(rpcType.String()) + removed := 0 + for addr, ep := range endpoints { + epURL := ep.GetURL(rpcType) + if epURL == "" { + epURL = ep.PublicURL() + } + if !blocklist.IsBlocked(epURL, rpcType) { + continue + } + + delete(endpoints, addr) + removed++ + + domain := "unknown" + if d, err := shannonmetrics.ExtractDomainOrHost(epURL); err == nil { + domain = d + } + metrics.EndpointsDomainBlockedTotal.WithLabelValues(domain, rpcTypeLabel, string(serviceID)).Inc() + + logger.Debug(). + Str("domain", domain). + Str("endpoint", string(addr)). + Msg("Skipping endpoint at operator-blocked domain") + } + return removed +} + // retainAllowedSuppliers deletes endpoints whose supplier is not in // allowedSuppliers, always keeping requestedEndpointAddr. Returns the number of // endpoints removed. The supplier address is taken from the "supplier-url" diff --git a/protocol/shannon/leaderboard.go b/protocol/shannon/leaderboard.go index 7500c06e6..999f02e92 100644 --- a/protocol/shannon/leaderboard.go +++ b/protocol/shannon/leaderboard.go @@ -2,6 +2,7 @@ package shannon import ( "context" + "strings" sharedtypes "github.com/pokt-network/poktroll/x/shared/types" @@ -415,14 +416,44 @@ func (p *Protocol) GetSupplierScoreData(ctx context.Context) ([]metrics.Supplier // This metric exposes that state so dashboards can answer "how many endpoints // does this domain currently have locked out?" without needing /ready introspection. func (p *Protocol) GetCooldownCountData(ctx context.Context) ([]metrics.CooldownCountEntry, error) { - logger := p.logger.With("method", "GetCooldownCountData") + // Counts only cooldowns the endpoint EARNED. Drains are reported separately by + // GetDrainedCountData so a bench we applied on purpose never reads as a fault. + return p.countBenchedEndpoints(ctx, "GetCooldownCountData", + func(_ reputation.EndpointKey, score reputation.Score, _ string, _ sharedtypes.RPCType) bool { + return score.IsInCooldown() + }) +} + +// GetDrainedCountData implements the metrics.LeaderboardDataProvider interface, reporting +// endpoints removed from selection by POST /admin/reputation/drain. +// +// Separate from the cooldown count because the two mean opposite things operationally: a +// cooldown is the endpoint's fault and warrants attention, a drain is ours and warrants +// none. A drain is also usually applied in order to OBSERVE an operator, so publishing it +// as a cooldown would corrupt the very signal it exists to produce. +func (p *Protocol) GetDrainedCountData(ctx context.Context) ([]metrics.CooldownCountEntry, error) { + return p.countBenchedEndpoints(ctx, "GetDrainedCountData", + func(key reputation.EndpointKey, _ reputation.Score, domain string, rpcType sharedtypes.RPCType) bool { + return p.reputationService.IsDomainDrained(key.ServiceID, domain, strings.ToLower(rpcType.String())) + }) +} + +// countBenchedEndpoints walks the active sessions and counts, per (domain, service, +// rpc_type), the endpoints matching include. Shared by the cooldown and drain counters so +// the two can never diverge in how they enumerate endpoints — only in what they select. +func (p *Protocol) countBenchedEndpoints( + ctx context.Context, + method string, + include func(reputation.EndpointKey, reputation.Score, string, sharedtypes.RPCType) bool, +) ([]metrics.CooldownCountEntry, error) { + logger := p.logger.With("method", method) if p.unifiedServicesConfig == nil { - logger.Debug().Msg("No unified services config available, returning empty cooldown counts") + logger.Debug().Msg("No unified services config available, returning empty counts") return nil, nil } if p.reputationService == nil { - logger.Debug().Msg("Reputation service not enabled, returning empty cooldown counts") + logger.Debug().Msg("Reputation service not enabled, returning empty counts") return nil, nil } @@ -450,22 +481,26 @@ func (p *Protocol) GetCooldownCountData(ctx context.Context) ([]metrics.Cooldown } for endpointAddr, ep := range endpoints { - keyBuilder := p.reputationService.KeyBuilderForService(serviceID) - key := keyBuilder.BuildKey(serviceID, endpointAddr, actualRPCType) - score, scoreErr := p.reputationService.GetScore(ctx, key) - if scoreErr != nil { - // No score recorded → can't be in cooldown. Skip silently. - continue - } - if !score.IsInCooldown() { - continue - } - endpointURL := ep.GetURL(actualRPCType) domain, domainErr := shannonmetrics.ExtractDomainOrHost(endpointURL) if domainErr != nil { domain = shannonmetrics.ErrDomain } + + // Resolved before the include check because a drain is matched on DOMAIN, + // not on a reputation key — an unscored endpoint at a drained operator is + // still benched, so it must still be counted. + keyBuilder := p.reputationService.KeyBuilderForService(serviceID) + key := keyBuilder.BuildKey(serviceID, endpointAddr, actualRPCType) + // A drain never writes to the Score, so this reports only cooldowns the + // endpoint EARNED. Drains are counted separately by GetDrainedCountData, + // keeping a deliberate bench from reading as a quality incident. + score, _ := p.reputationService.GetScore(ctx, key) + + if !include(key, score, domain, actualRPCType) { + continue + } + k := groupKey{ Domain: domain, ServiceID: string(serviceID), @@ -486,7 +521,7 @@ func (p *Protocol) GetCooldownCountData(ctx context.Context) ([]metrics.Cooldown }) } - logger.Debug().Int("total_entries", len(entries)).Msg("Built cooldown count data") + logger.Debug().Int("total_entries", len(entries)).Msg("Built benched-endpoint count data") return entries, nil } diff --git a/protocol/shannon/protocol.go b/protocol/shannon/protocol.go index 5f8745b1f..b90126dd9 100644 --- a/protocol/shannon/protocol.go +++ b/protocol/shannon/protocol.go @@ -142,6 +142,11 @@ type Protocol struct { // endpointPolicy holds operator-level security policies for endpoint selection. endpointPolicy gateway.EndpointPolicyConfig + // blockedDomains is the gateway-operator (domain, rpc_type) blocklist — the nuclear + // ban, applied to every path that hands out endpoints (selection, fallback, health + // checks) across ALL services. nil when nothing is banned. See domain_blocklist.go. + blockedDomains *domainBlocklist + // supplierBlacklist tracks suppliers with validation/signature errors. // These suppliers are temporarily excluded from selection to prevent // penalizing domain reputation for individual supplier issues. @@ -212,6 +217,27 @@ func (p *Protocol) TumbleWebsockets(req protocol.WebsocketTumbleRequest) protoco return result } +// ShutdownWebsockets closes every live websocket bridge on this pod with a proper close +// handshake, so a rollout reaches clients as 1012 ("please reconnect") and endpoints as an +// orderly close, instead of both peers seeing the socket vanish. +// +// Must be called on SIGTERM before the HTTP server is shut down: http.Server.Shutdown does +// not close hijacked connections, and every websocket is hijacked, so nothing else in the +// termination path touches these. +// +// Bounded by ctx — a pod that cannot finish being polite inside its grace period must still +// exit. Returns the number of connections actually closed. +func (p *Protocol) ShutdownWebsockets(ctx context.Context, reason string) int { + closed := p.wsConnRegistry.shutdownAll(ctx, reason) + if closed > 0 { + p.logger.With("method", "ShutdownWebsockets").Info(). + Int("closed", closed). + Str("reason", reason). + Msg("🔌 closed live websocket connections with a close handshake before exiting") + } + return closed +} + // serviceFallback holds the fallback information for a service, // including the endpoints and whether to send all traffic to fallback. type serviceFallback struct { @@ -289,6 +315,26 @@ func NewProtocol( "Recommended: Start with max_parallel_endpoints=1 and test thoroughly before increasing.") } + // Compile the gateway-operator domain blocklist: config entries plus any + // PATH_BLOCKED_DOMAINS env additions (union — env widens, never narrows). + // A malformed entry refuses to boot: silently dropping a nuclear ban is worse. + blockedDomainEntries := append( + append([]gateway.BlockedDomainConfig(nil), config.BlockedDomains...), + parseBlockedDomainsEnv(os.Getenv(envBlockedDomains))..., + ) + blockedDomains, err := newDomainBlocklist(blockedDomainEntries) + if err != nil { + return nil, fmt.Errorf("invalid domain blocklist (blocked_domains config / %s env): %w", envBlockedDomains, err) + } + for _, entry := range blockedDomains.configuredEntries() { + // Warn so the ban is visible at LOG_LEVEL=warn; the gauge covers LOG_LEVEL=error. + shannonLogger.Warn(). + Str("domain", entry[0]). + Str("rpc_type", entry[1]). + Msg("🚫 domain blocklist entry active — endpoints at this domain are banned on ALL services") + metrics.BlockedDomainsConfigured.WithLabelValues(entry[0], entry[1]).Set(1) + } + protocolInstance := &Protocol{ logger: shannonLogger, @@ -330,6 +376,9 @@ func NewProtocol( // endpointPolicy holds operator-level endpoint security policies endpointPolicy: config.EndpointPolicy, + // blockedDomains is the compiled (domain, rpc_type) nuclear ban list + blockedDomains: blockedDomains, + // supplierBlacklist tracks suppliers with validation/signature errors supplierBlacklist: newSupplierBlacklist(), @@ -986,6 +1035,20 @@ func (p *Protocol) getUniqueEndpoints( // Get fallback configuration for the service ID. fallbackEndpoints, shouldSendAllTrafficToFallback := p.getServiceFallbackEndpoints(serviceID) + // The domain blocklist covers fallback endpoints too — both return paths below hand + // them out raw, bypassing every session-endpoint filter, and a nuclear ban with a + // fallback-shaped hole is not nuclear. Clone first: getServiceFallbackEndpoints + // returns the shared config-owned map, which must not be mutated. + if p.blockedDomains != nil && len(fallbackEndpoints) > 0 { + fallbackEndpoints = maps.Clone(fallbackEndpoints) + if blockedCount := removeBlockedDomains(fallbackEndpoints, p.blockedDomains, rpcType, serviceID, logger); blockedCount > 0 { + logger.Warn(). + Int("blocked", blockedCount). + Int("remaining", len(fallbackEndpoints)). + Msg("🚫 Filtered out fallback endpoints at operator-blocked domains") + } + } + // If the service is configured to send all traffic to fallback endpoints, // return only the fallback endpoints and skip session endpoint logic. if shouldSendAllTrafficToFallback && len(fallbackEndpoints) > 0 { @@ -1200,6 +1263,20 @@ func (p *Protocol) getSessionsUniqueEndpoints( Msg("Filtered out config-blocked suppliers") } + // GATEWAY-OPERATOR DOMAIN BLOCKLIST (the nuclear ban) + // Permanently exclude endpoints at domains banned for this RPC type, on every + // service. Applied before the allowlist so Target-Suppliers cannot override it, + // and with no preferred-endpoint exemption so a WebSocket rebind cannot re-pick + // a banned endpoint it is already bound to. Uses actualRPCType: after an RPC-type + // fallback the surviving endpoints serve the fallback type, and their URL for the + // originally requested type may not exist. + if blockedCount := removeBlockedDomains(qualifiedEndpoints, p.blockedDomains, actualRPCType, serviceID, logger); blockedCount > 0 { + logger.Warn(). + Int("blocked", blockedCount). + Int("remaining", len(qualifiedEndpoints)). + Msg("🚫 Filtered out endpoints at operator-blocked domains") + } + // ENDPOINT POLICY FILTERING // Apply operator-level security policies (require_https, require_domain). // This is a gateway operator decision, not a per-service override. @@ -1653,23 +1730,17 @@ func (p *Protocol) GetEndpointsForHealthCheck() func(protocol.ServiceID) ([]gate continue } - // Filter endpoints by RPC type support + // Filter endpoints by RPC type support, honoring the domain blocklist: + // a banned (domain, rpc_type) must receive no probes — health checks are + // paid relays, and "block HC too" is the point of a nuclear ban. for addr, ep := range sessionEndpoints { - supportsAnyType := false - for rpcType := range healthCheckRPCTypes { - url := ep.GetURL(rpcType) - if url != "" { - supportsAnyType = true - break - } - } - - if supportsAnyType { + supportsHTTP, supportsWS := p.healthCheckTypeSupport(ep, healthCheckRPCTypes) + if supportsHTTP || supportsWS { allEndpoints[addr] = ep } else { logger.Debug(). Str("endpoint", string(addr)). - Msg("Skipping endpoint - does not support any health check RPC types") + Msg("Skipping endpoint - does not support any health check RPC types (or all are operator-blocked)") } } } @@ -1677,16 +1748,8 @@ func (p *Protocol) GetEndpointsForHealthCheck() func(protocol.ServiceID) ([]gate // Also include fallback endpoints if configured, filtered by RPC type fallbackEndpoints, _ := p.getServiceFallbackEndpoints(serviceID) for addr, ep := range fallbackEndpoints { - supportsAnyType := false - for rpcType := range healthCheckRPCTypes { - url := ep.GetURL(rpcType) - if url != "" { - supportsAnyType = true - break - } - } - - if supportsAnyType { + supportsHTTP, supportsWS := p.healthCheckTypeSupport(ep, healthCheckRPCTypes) + if supportsHTTP || supportsWS { allEndpoints[addr] = ep } } @@ -1704,8 +1767,11 @@ func (p *Protocol) GetEndpointsForHealthCheck() func(protocol.ServiceID) ([]gate HTTPURL: ep.PublicURL(), } - // Get WebSocket URL if available - if wsURL, err := ep.WebsocketURL(); err == nil { + // Get WebSocket URL if available. A blank WebSocketURL is how the health + // check executor decides an endpoint gets no WebSocket probe (it gates on + // WebSocketURL != ""), so a websocket domain ban is enforced by omission here. + if wsURL, err := ep.WebsocketURL(); err == nil && + !p.blockedDomains.IsBlocked(wsURL, sharedtypes.RPCType_WEBSOCKET) { info.WebSocketURL = wsURL } @@ -1726,6 +1792,33 @@ func (p *Protocol) GetEndpointsForHealthCheck() func(protocol.ServiceID) ([]gate } } +// healthCheckTypeSupport reports which transport a health-check probe may use against +// this endpoint: supportsHTTP when at least one HTTP-carried health-check RPC type +// (json_rpc, rest, comet_bft) has a URL and is not operator-blocked for its domain, +// supportsWS likewise for websocket. Neither true = the endpoint gets no probes at all. +// +// Known limitation (documented on gateway.BlockedDomainConfig): the executor runs all +// HTTP-carried checks against one endpoint address, so a ban covering only SOME of an +// endpoint's HTTP types cannot suppress just those checks — the endpoint keeps its HTTP +// probes as long as any HTTP type survives. All-type and websocket bans are exact. +func (p *Protocol) healthCheckTypeSupport( + ep endpoint, + healthCheckRPCTypes map[sharedtypes.RPCType]struct{}, +) (supportsHTTP, supportsWS bool) { + for rpcType := range healthCheckRPCTypes { + url := ep.GetURL(rpcType) + if url == "" || p.blockedDomains.IsBlocked(url, rpcType) { + continue + } + if rpcType == sharedtypes.RPCType_WEBSOCKET { + supportsWS = true + } else { + supportsHTTP = true + } + } + return supportsHTTP, supportsWS +} + // getHealthCheckRPCTypes extracts the RPC types used in health checks for a service. // Returns a map of RPC types (as keys) that are configured in health checks. // If no local health checks are configured, returns default RPC types (JSON_RPC) diff --git a/protocol/shannon/reputation.go b/protocol/shannon/reputation.go index 077b29268..6dda4ad32 100644 --- a/protocol/shannon/reputation.go +++ b/protocol/shannon/reputation.go @@ -2,12 +2,14 @@ package shannon import ( "context" + "strings" "github.com/pokt-network/poktroll/pkg/polylog" sharedtypes "github.com/pokt-network/poktroll/x/shared/types" "github.com/pokt-network/path/gateway" "github.com/pokt-network/path/metrics" + shannonmetrics "github.com/pokt-network/path/metrics/protocol/shannon" "github.com/pokt-network/path/protocol" "github.com/pokt-network/path/reputation" ) @@ -92,6 +94,67 @@ func (p *Protocol) filterByReputation( } } + // Admin drains are evaluated against the endpoint's LIVE URL, not against a reputation + // key, and that is deliberate. EndpointAddr is `supplierAddr-url` and the supplier set + // rotates every session, so a bench resolved to concrete keys goes stale at the next + // rollover — which is exactly how the first version of this silently stopped working + // while still reporting endpoints benched. Matching on the URL means an endpoint rotated + // into the session at a drained operator is benched the moment it appears. + // + // Applied BEFORE the reputation checks below so a drain does not depend on scores + // existing: an unscored endpoint at a drained operator must still be excluded. + // Filter `cached` itself, NOT the incoming `endpoints` map: everything below builds the + // returned set by walking `cached`, so deleting from `endpoints` has no effect on the + // result. An earlier version did exactly that and the ban was completely inert in + // production while reporting success. + if p.reputationService != nil { + rpcTypeStr := strings.ToLower(rpcType.String()) + kept := cached[:0:0] + drained := 0 + // NO exemption for requestedEndpointAddr here, deliberately — every other filter in + // this function grants one, and granting it to drains defeated them entirely. + // + // A websocket rebind passes the endpoint it is ALREADY bound to as preferredAddr + // (ReconnectEndpoint in websocket_context.go), and a connection re-selects at every + // session rollover. So the exemption fired on precisely the connections a drain + // exists to move: each rollover re-picked the drained endpoint because it was + // "preferred". Measured in production 2026-08-07 — drain applied fleet-wide, every + // connection tumbled, path_endpoints_drained reporting 170 benched, and four + // minutes later the drained operator still served 73% of the service's frames. + // + // Sticky placement is what a drain overrides; it cannot also be what protects an + // endpoint from one. The pool-empty branch below is the real safety net, and it is + // sufficient: an endpoint is only kept when dropping it would leave nothing. + for _, ak := range cached { + if domain, domainErr := shannonmetrics.ExtractDomainOrHost(ak.ep.GetURL(rpcType)); domainErr == nil && + p.reputationService.IsDomainDrained(serviceID, domain, rpcTypeStr) { + drained++ + continue + } + kept = append(kept, ak) + } + + switch { + case drained == 0: + // nothing benched for this service/rpc_type + + case len(kept) > 0: + logger.Warn(). + Int("drained_endpoints", drained). + Int("remaining", len(kept)). + Msg("⚠️ excluded endpoints benched by an admin drain") + cached = kept + + default: + // A drain must never empty the pool — that would be an outage rather than a + // redistribution. A ban is an operator preference, not a correctness + // constraint, so it yields rather than severing the service. + logger.Warn(). + Int("drained_endpoints", drained). + Msg("⚠️ admin drain would empty the endpoint pool — keeping drained endpoints as last resort") + } + } + // Hoist out of the loop; neither the threshold nor the rpc_type label // changes per endpoint. minThreshold := p.getMinThresholdForService(serviceID) diff --git a/protocol/shannon/reputation_test.go b/protocol/shannon/reputation_test.go index 10ad42699..c3513e488 100644 --- a/protocol/shannon/reputation_test.go +++ b/protocol/shannon/reputation_test.go @@ -1135,9 +1135,9 @@ func TestReputation_KeyGranularityPerDomain(t *testing.T) { serviceID := protocol.ServiceID("eth") - // Multiple endpoints from DIFFERENT suppliers but SAME hosting domain (nodefleet.net) - endpoint1Addr := protocol.EndpointAddr("pokt1supplier1-https://rm-01.eu.nodefleet.net") - endpoint2Addr := protocol.EndpointAddr("pokt1supplier2-https://rm-02.us.nodefleet.net") + // Multiple endpoints from DIFFERENT suppliers but SAME hosting domain (op-delta.example) + endpoint1Addr := protocol.EndpointAddr("pokt1supplier1-https://rm-01.eu.op-delta.example") + endpoint2Addr := protocol.EndpointAddr("pokt1supplier2-https://rm-02.us.op-delta.example") // One endpoint from a DIFFERENT domain endpoint3Addr := protocol.EndpointAddr("pokt1supplier3-https://relay.pokt.network") @@ -1150,7 +1150,7 @@ func TestReputation_KeyGranularityPerDomain(t *testing.T) { key2 := keyBuilder.BuildKey(serviceID, endpoint2Addr, sharedtypes.RPCType_JSON_RPC) key3 := keyBuilder.BuildKey(serviceID, endpoint3Addr, sharedtypes.RPCType_JSON_RPC) - // Verify keys 1 and 2 are the same (same domain: nodefleet.net) + // Verify keys 1 and 2 are the same (same domain: op-delta.example) require.Equal(t, key1, key2, "Endpoints from same domain should have same key") require.NotEqual(t, key1, key3, "Endpoints from different domains should have different keys") @@ -1468,3 +1468,79 @@ func TestReputationWebSocketRecording_RPCType(t *testing.T) { t.Log("Verified: WebSocket observations use WEBSOCKET RPC type in reputation keys") } + +// THE REGRESSION TEST for the admin drain. +// +// The drain block originally deleted from the `endpoints` map passed in, but this function +// builds its result by walking `cached` — so the deletions had no effect on what it returned +// and the ban was completely inert in production while the API reported success and the +// gauge reported endpoints benched. Asserting on the RETURNED map is the only thing that +// catches that; every unit test I had written up to then asserted on helpers instead. +func Test_filterByReputation_ExcludesDrainedOperator(t *testing.T) { + ctx := context.Background() + logger := polyzero.NewLogger() + + config := reputation.Config{Enabled: true, InitialScore: 80, MinThreshold: 30, StorageType: "memory"} + config.HydrateDefaults() + store := reputationstorage.NewMemoryStorage(config.RecoveryTimeout) + svc := reputation.NewService(config, store) + require.NoError(t, svc.Start(ctx)) + defer func() { _ = svc.Stop() }() + + p := &Protocol{logger: logger, reputationService: svc} + serviceID := protocol.ServiceID("gnosis") + + endpoints := map[protocol.EndpointAddr]endpoint{ + "https://f019.op-beta.example": &mockEndpoint{addr: "https://f019.op-beta.example"}, + "https://r001.op-alpha.example": &mockEndpoint{addr: "https://r001.op-alpha.example"}, + "https://n1.op-gamma.example": &mockEndpoint{addr: "https://n1.op-gamma.example"}, + } + + // Nothing benched yet: every endpoint survives. + require.Len(t, p.filterByReputation(ctx, serviceID, endpoints, sharedtypes.RPCType_WEBSOCKET, logger, ""), 3) + + svc.DrainDomain(ctx, reputation.DrainRequest{ + ServiceID: serviceID, Domain: "op-beta.example", RPCType: "websocket", Duration: time.Hour, + }) + + filtered := p.filterByReputation(ctx, serviceID, endpoints, sharedtypes.RPCType_WEBSOCKET, logger, "") + require.NotContains(t, filtered, protocol.EndpointAddr("https://f019.op-beta.example"), + "a drained operator must be absent from the RETURNED set, not merely from a map nobody reads") + require.Contains(t, filtered, protocol.EndpointAddr("https://r001.op-alpha.example")) + require.Contains(t, filtered, protocol.EndpointAddr("https://n1.op-gamma.example")) + + // A drain is scoped to its rpc_type: the same operator still serves HTTP. + httpFiltered := p.filterByReputation(ctx, serviceID, endpoints, sharedtypes.RPCType_JSON_RPC, logger, "") + require.Contains(t, httpFiltered, protocol.EndpointAddr("https://f019.op-beta.example")) +} + +// Banning every operator must not sever the service. A drain is an operator preference, not +// a correctness constraint, so it yields rather than returning an empty pool. +func Test_filterByReputation_DrainNeverEmptiesPool(t *testing.T) { + ctx := context.Background() + logger := polyzero.NewLogger() + + config := reputation.Config{Enabled: true, InitialScore: 80, MinThreshold: 30, StorageType: "memory"} + config.HydrateDefaults() + store := reputationstorage.NewMemoryStorage(config.RecoveryTimeout) + svc := reputation.NewService(config, store) + require.NoError(t, svc.Start(ctx)) + defer func() { _ = svc.Stop() }() + + p := &Protocol{logger: logger, reputationService: svc} + serviceID := protocol.ServiceID("gnosis") + + endpoints := map[protocol.EndpointAddr]endpoint{ + "https://f019.op-beta.example": &mockEndpoint{addr: "https://f019.op-beta.example"}, + "https://r001.op-alpha.example": &mockEndpoint{addr: "https://r001.op-alpha.example"}, + } + + for _, d := range []string{"op-beta.example", "op-alpha.example"} { + svc.DrainDomain(ctx, reputation.DrainRequest{ + ServiceID: serviceID, Domain: d, RPCType: "websocket", Duration: time.Hour, + }) + } + + filtered := p.filterByReputation(ctx, serviceID, endpoints, sharedtypes.RPCType_WEBSOCKET, logger, "") + require.Len(t, filtered, 2, "draining every operator must yield rather than empty the pool") +} diff --git a/protocol/shannon/selection_harness_test.go b/protocol/shannon/selection_harness_test.go new file mode 100644 index 000000000..b124c54fd --- /dev/null +++ b/protocol/shannon/selection_harness_test.go @@ -0,0 +1,374 @@ +package shannon + +import ( + "context" + "sort" + "strings" + "testing" + "time" + + "github.com/pokt-network/poktroll/pkg/polylog/polyzero" + apptypes "github.com/pokt-network/poktroll/x/application/types" + sessiontypes "github.com/pokt-network/poktroll/x/session/types" + sharedtypes "github.com/pokt-network/poktroll/x/shared/types" + "github.com/stretchr/testify/require" + + "github.com/pokt-network/path/protocol" + "github.com/pokt-network/path/reputation" + reputationstorage "github.com/pokt-network/path/reputation/storage" +) + +// ============================================================================= +// Selection-path test harness +// ============================================================================= +// +// WHY THIS EXISTS. +// +// Three separate bugs shipped in the admin-drain feature, all with passing tests, all the +// same mistake: the tests asserted on something the author wrote rather than on the value +// the production caller receives. +// +// 1. The bench was written onto Score.CooldownUntil. refreshFromStorage overwrites the +// cache from storage unconditionally, so it was erased within a refresh cycle. The test +// asserted "CooldownUntil is set" — which was true, briefly. +// 2. The bench resolved the target to a fixed set of EndpointKeys. EndpointAddr embeds the +// supplier address and sessions rotate their supplier set, so the keys went stale every +// rollover. Every test used a static cache, so nothing rotated. +// 3. The filter deleted from the `endpoints` map passed in, while the function builds its +// result by walking `cached`. Nothing was ever excluded. The tests asserted on helpers. +// +// In all three the real question was the same one nobody asked: DOES SELECTION STILL RETURN +// THIS ENDPOINT? There was no cheap way to ask it, so each fix grew unit tests around the +// edges instead, and each shipped broken while reporting success in production. +// +// This harness makes that question a single call. Anything that claims to change which +// endpoints are reachable — drains, cooldowns, thresholds, the operator-alias work — should +// be asserted through Survivors(), not through the state it happens to write. +type selectionScenario struct { + t *testing.T + ctx context.Context + svc reputation.ReputationService + p *Protocol + serviceID protocol.ServiceID + + // urls is the backend URL of every endpoint in the scenario, in insertion order. + urls []string + // supplierOf maps a URL to the supplier address currently fronting it. RotateSuppliers + // changes these without touching the URLs — which is exactly what a session rollover + // does, and what defeated the key-snapshot implementation. + supplierOf map[string]string +} + +// harnessEndpoint keeps the supplier address and the backend URL separate, so a session +// rotation can be simulated by changing one without the other. mockEndpoint collapses them +// into a single field, which made rotation impossible to express — which is part of why the +// rotation bug was never caught. +type harnessEndpoint struct { + supplier string + url string +} + +var _ endpoint = (*harnessEndpoint)(nil) + +func (e *harnessEndpoint) Addr() protocol.EndpointAddr { + return protocol.EndpointAddr(e.supplier + "-" + e.url) +} +func (e *harnessEndpoint) PublicURL() string { return e.url } +func (e *harnessEndpoint) GetURL(_ sharedtypes.RPCType) string { return e.url } +func (e *harnessEndpoint) WebsocketURL() (string, error) { return e.url, nil } +func (e *harnessEndpoint) Supplier() string { return e.supplier } +func (e *harnessEndpoint) IsFallback() bool { return false } +func (e *harnessEndpoint) Session() *sessiontypes.Session { + return &sessiontypes.Session{Header: &sessiontypes.SessionHeader{}, Application: &apptypes.Application{}} +} + +// newSelectionScenario builds a scenario over the given backend URLs. Each URL gets a +// distinct supplier address, mirroring how a real session fronts one backend per supplier. +func newSelectionScenario(t *testing.T, serviceID string, urls ...string) *selectionScenario { + t.Helper() + + ctx := context.Background() + cfg := reputation.Config{Enabled: true, InitialScore: 80, MinThreshold: 30, StorageType: "memory"} + cfg.HydrateDefaults() + + store := reputationstorage.NewMemoryStorage(cfg.RecoveryTimeout) + svc := reputation.NewService(cfg, store) + require.NoError(t, svc.Start(ctx)) + t.Cleanup(func() { _ = svc.Stop() }) + + s := &selectionScenario{ + t: t, ctx: ctx, svc: svc, + p: &Protocol{logger: polyzero.NewLogger(), reputationService: svc}, + serviceID: protocol.ServiceID(serviceID), + urls: append([]string(nil), urls...), + supplierOf: make(map[string]string, len(urls)), + } + for i, u := range urls { + s.supplierOf[u] = supplierAddrForIndex(i, 0) + } + return s +} + +func supplierAddrForIndex(i, generation int) string { + return "pokt1gen" + string(rune('a'+generation)) + "supplier" + string(rune('0'+i%10)) +} + +// endpointMap builds the map that selection is handed. +func (s *selectionScenario) endpointMap() map[protocol.EndpointAddr]endpoint { + out := make(map[protocol.EndpointAddr]endpoint, len(s.urls)) + for _, u := range s.urls { + ep := &harnessEndpoint{supplier: s.supplierOf[u], url: u} + out[ep.Addr()] = ep + } + return out +} + +// Survivors returns the backend URLs selection would still consider, sorted. +// +// THIS is the production-visible outcome. Assert on it rather than on scores, cooldowns, +// drain maps or gauges — all four have reported a bench that selection did not honour. +func (s *selectionScenario) Survivors(rpcType sharedtypes.RPCType) []string { + s.t.Helper() + filtered := s.p.filterByReputation( + s.ctx, s.serviceID, s.endpointMap(), rpcType, polyzero.NewLogger(), "", + ) + out := make([]string, 0, len(filtered)) + for _, ep := range filtered { + out = append(out, ep.GetURL(rpcType)) + } + sort.Strings(out) + return out +} + +// SurvivorsPreferring is Survivors for the callers that pass a preferred endpoint — +// which is every websocket path. Bridge setup passes selectedEndpointAddr and, critically, +// a rebind passes preferredAddr: the endpoint the connection is ALREADY bound to. +// +// Survivors' hardcoded "" is why three drain tests passed while production kept serving a +// drained operator. A websocket connection re-selects on every session rollover, so the +// preferred-endpoint path is the one that decides whether a drain ever takes effect on the +// traffic that matters — and it was the one path no test called. +func (s *selectionScenario) SurvivorsPreferring(rpcType sharedtypes.RPCType, preferredURL string) []string { + s.t.Helper() + preferred := protocol.EndpointAddr(s.supplierOf[preferredURL] + "-" + preferredURL) + filtered := s.p.filterByReputation( + s.ctx, s.serviceID, s.endpointMap(), rpcType, polyzero.NewLogger(), preferred, + ) + out := make([]string, 0, len(filtered)) + for _, ep := range filtered { + out = append(out, ep.GetURL(rpcType)) + } + sort.Strings(out) + return out +} + +// AssertExcludedPreferring fails unless the drained URL is absent even when it is the +// endpoint the caller would prefer to keep. +func (s *selectionScenario) AssertExcludedPreferring(rpcType sharedtypes.RPCType, preferredURL string, unwanted ...string) { + s.t.Helper() + got := s.SurvivorsPreferring(rpcType, preferredURL) + for _, u := range unwanted { + require.NotContains(s.t, got, u, + "%s must not be selectable for %s even when preferred — a websocket rebind always "+ + "prefers where it already is, so an exemption there means the drain never applies", + u, rpcTypeName(rpcType)) + } +} + +// AssertServes fails unless selection returns exactly the given URLs. +func (s *selectionScenario) AssertServes(rpcType sharedtypes.RPCType, want ...string) { + s.t.Helper() + sort.Strings(want) + require.Equal(s.t, want, s.Survivors(rpcType), + "selection returned the wrong endpoint set for %s", rpcTypeName(rpcType)) +} + +// AssertExcluded fails unless every given URL is absent from selection. +func (s *selectionScenario) AssertExcluded(rpcType sharedtypes.RPCType, unwanted ...string) { + s.t.Helper() + got := s.Survivors(rpcType) + for _, u := range unwanted { + require.NotContains(s.t, got, u, + "%s must not be selectable for %s — a bench that selection ignores is not a bench", + u, rpcTypeName(rpcType)) + } +} + +// AssertSelectable fails unless every given URL is present in selection. +func (s *selectionScenario) AssertSelectable(rpcType sharedtypes.RPCType, wanted ...string) { + s.t.Helper() + got := s.Survivors(rpcType) + for _, u := range wanted { + require.Contains(s.t, got, u, "%s must remain selectable for %s", u, rpcTypeName(rpcType)) + } +} + +// Drain bans an operator, exactly as the admin endpoint does. +func (s *selectionScenario) Drain(domain string, rpcType sharedtypes.RPCType, d time.Duration) { + s.t.Helper() + s.svc.DrainDomain(s.ctx, reputation.DrainRequest{ + ServiceID: s.serviceID, Domain: domain, RPCType: rpcTypeName(rpcType), Duration: d, + }) +} + +// Release lifts a ban. +func (s *selectionScenario) Release(domain string, rpcType sharedtypes.RPCType) { + s.t.Helper() + s.Drain(domain, rpcType, 0) +} + +// DriveIntoCooldown makes an endpoint earn a real cooldown, so tests can tell an earned +// cooldown apart from an admin bench — they must never be conflated. +func (s *selectionScenario) DriveIntoCooldown(url string, rpcType sharedtypes.RPCType) { + s.t.Helper() + kb := s.svc.KeyBuilderForService(s.serviceID) + key := kb.BuildKey(s.serviceID, protocol.EndpointAddr(s.supplierOf[url]+"-"+url), rpcType) + for i := 0; i < 8; i++ { + require.NoError(s.t, s.svc.RecordSignal(s.ctx, key, + reputation.NewCriticalErrorSignal("service_error", 200*time.Millisecond))) + } +} + +// RotateSuppliers re-fronts every backend URL with fresh supplier addresses, leaving the +// URLs untouched. +// +// This is what a session rollover does, and it is the single most important thing this +// harness can express: any state keyed on EndpointAddr silently goes stale here. The +// key-snapshot drain passed every test until this was possible to write. +func (s *selectionScenario) RotateSuppliers(generation int) { + s.t.Helper() + for i, u := range s.urls { + s.supplierOf[u] = supplierAddrForIndex(i, generation) + } +} + +func rpcTypeName(rpcType sharedtypes.RPCType) string { + return strings.ToLower(rpcType.String()) +} + +// ============================================================================= +// The three shipped bugs, as assertions +// ============================================================================= + +const ( + opBetaA = "https://f019.op-beta.example" + opBetaB = "https://f026.op-beta.example" + opAlphaA = "https://r001.op-alpha.example" + opGammaA = "https://n1.op-gamma.example" +) + +// BUG 3 — the filter mutated a map its result was not built from, so nothing was excluded +// while the API and the gauge both reported the operator benched. +func TestSelection_DrainRemovesOperatorFromTheReturnedSet(t *testing.T) { + s := newSelectionScenario(t, "gnosis", opBetaA, opBetaB, opAlphaA, opGammaA) + + s.AssertServes(sharedtypes.RPCType_WEBSOCKET, opBetaA, opBetaB, opAlphaA, opGammaA) + + s.Drain("op-beta.example", sharedtypes.RPCType_WEBSOCKET, time.Hour) + s.AssertExcluded(sharedtypes.RPCType_WEBSOCKET, opBetaA, opBetaB) + s.AssertSelectable(sharedtypes.RPCType_WEBSOCKET, opAlphaA, opGammaA) +} + +// BUG 4 — the drain exempted requestedEndpointAddr, and every websocket path supplies one. +// +// A websocket rebind passes the endpoint it is ALREADY bound to as preferredAddr +// (websocket_context.go, ReconnectEndpoint). The exemption therefore fired on exactly the +// connections a drain is meant to move: each rollover re-selected the drained endpoint +// because it was "preferred", and a connection re-selects every session. +// +// Measured in production 2026-08-07: drain applied fleet-wide with path_endpoints_drained +// reporting 170 endpoints benched, all connections tumbled, and four minutes later the +// drained operator still served 73% of the service's websocket frames. +// +// The three earlier drain tests all passed because Survivors() hardcodes "" — the one call +// shape the websocket path never uses. +func TestSelection_DrainAppliesEvenToThePreferredEndpoint(t *testing.T) { + s := newSelectionScenario(t, "gnosis", opBetaA, opBetaB, opAlphaA, opGammaA) + + s.Drain("op-beta.example", sharedtypes.RPCType_WEBSOCKET, time.Hour) + + // A rebind on a connection currently bound to opBetaA prefers opBetaA. + s.AssertExcludedPreferring(sharedtypes.RPCType_WEBSOCKET, opBetaA, opBetaA, opBetaB) + + // ...and still has somewhere to go. + got := s.SurvivorsPreferring(sharedtypes.RPCType_WEBSOCKET, opBetaA) + require.Contains(t, got, opAlphaA) + require.Contains(t, got, opGammaA) +} + +// The pool-empty guard is what keeps the above from being an outage: when the preferred +// endpoint is the ONLY thing left, yielding is correct — a bench is an operator preference, +// not a correctness constraint. +func TestSelection_PreferredEndpointSurvivesWhenItIsAllThatRemains(t *testing.T) { + s := newSelectionScenario(t, "gnosis", opBetaA) + + s.Drain("op-beta.example", sharedtypes.RPCType_WEBSOCKET, time.Hour) + + require.Equal(t, []string{opBetaA}, + s.SurvivorsPreferring(sharedtypes.RPCType_WEBSOCKET, opBetaA), + "a drain must never empty the pool, even against the preferred endpoint") +} + +// BUG 2 — the bench was a snapshot of EndpointKeys, so a session rollover silently lifted it. +func TestSelection_DrainSurvivesSupplierRotation(t *testing.T) { + s := newSelectionScenario(t, "gnosis", opBetaA, opAlphaA, opGammaA) + + s.Drain("op-beta.example", sharedtypes.RPCType_WEBSOCKET, time.Hour) + s.AssertExcluded(sharedtypes.RPCType_WEBSOCKET, opBetaA) + + // Session N+1: same backends, entirely new supplier addresses. + s.RotateSuppliers(1) + s.AssertExcluded(sharedtypes.RPCType_WEBSOCKET, opBetaA) + + s.RotateSuppliers(2) + s.AssertExcluded(sharedtypes.RPCType_WEBSOCKET, opBetaA) +} + +// A ban is scoped to one protocol: banning WebSocket must not take the operator's HTTP +// traffic with it. +func TestSelection_DrainIsScopedToRPCType(t *testing.T) { + s := newSelectionScenario(t, "gnosis", opBetaA, opGammaA) + + s.Drain("op-beta.example", sharedtypes.RPCType_WEBSOCKET, time.Hour) + s.AssertExcluded(sharedtypes.RPCType_WEBSOCKET, opBetaA) + s.AssertSelectable(sharedtypes.RPCType_JSON_RPC, opBetaA) +} + +func TestSelection_ReleaseRestoresSelectability(t *testing.T) { + s := newSelectionScenario(t, "gnosis", opBetaA, opGammaA) + + s.Drain("op-beta.example", sharedtypes.RPCType_WEBSOCKET, time.Hour) + s.AssertExcluded(sharedtypes.RPCType_WEBSOCKET, opBetaA) + + s.Release("op-beta.example", sharedtypes.RPCType_WEBSOCKET) + s.AssertSelectable(sharedtypes.RPCType_WEBSOCKET, opBetaA) +} + +// Banning every operator must yield rather than sever the service: a ban is an operator +// preference, not a correctness constraint. +func TestSelection_DrainNeverEmptiesThePool(t *testing.T) { + s := newSelectionScenario(t, "gnosis", opBetaA, opAlphaA) + + s.Drain("op-beta.example", sharedtypes.RPCType_WEBSOCKET, time.Hour) + s.Drain("op-alpha.example", sharedtypes.RPCType_WEBSOCKET, time.Hour) + + s.AssertServes(sharedtypes.RPCType_WEBSOCKET, opBetaA, opAlphaA) +} + +// An earned cooldown and an admin bench must both remove an endpoint from selection, and a +// release must lift only the bench — the two are independent by construction, and operators +// rely on that when reading dashboards during an experiment. +func TestSelection_EarnedCooldownAndAdminBenchAreIndependent(t *testing.T) { + s := newSelectionScenario(t, "gnosis", opBetaA, opAlphaA, opGammaA) + + s.DriveIntoCooldown(opAlphaA, sharedtypes.RPCType_JSON_RPC) + s.Drain("op-beta.example", sharedtypes.RPCType_JSON_RPC, time.Hour) + + s.AssertExcluded(sharedtypes.RPCType_JSON_RPC, opBetaA, opAlphaA) + s.AssertSelectable(sharedtypes.RPCType_JSON_RPC, opGammaA) + + // Lifting the bench must not rescue the endpoint that earned its cooldown. + s.Release("op-beta.example", sharedtypes.RPCType_JSON_RPC) + s.AssertSelectable(sharedtypes.RPCType_JSON_RPC, opBetaA) + s.AssertExcluded(sharedtypes.RPCType_JSON_RPC, opAlphaA) +} diff --git a/protocol/shannon/websocket_context.go b/protocol/shannon/websocket_context.go index 6b9cd5a8a..22a515c57 100644 --- a/protocol/shannon/websocket_context.go +++ b/protocol/shannon/websocket_context.go @@ -358,7 +358,11 @@ func (p *Protocol) CheckWebsocketConnection( logger.Debug().Err(err).Msg("❌ Failed to connect to websocket endpoint") return nil, getWebsocketConnectionErrorObservation(logger, serviceID, selectedEndpoint, err) } - defer conn.Close() + // Close with a handshake, not a bare socket drop. This probe runs ~89 times a second + // fleetwide, and a bare Close() makes every one of them surface in the endpoint's log + // as an abnormal closure (1006 / "Aborted state") attributed to the endpoint. 1000 + // Normal Closure is accurate: the probe got what it came for and is leaving. + defer websockets.CloseEndpointConn(conn, gorillaws.CloseNormalClosure, "health check complete") // Handshake-only probe: connecting was the whole test. // diff --git a/protocol/shannon/websocket_registry.go b/protocol/shannon/websocket_registry.go index 845894f5c..717d6af8f 100644 --- a/protocol/shannon/websocket_registry.go +++ b/protocol/shannon/websocket_registry.go @@ -4,8 +4,10 @@ import ( "context" "sort" "sync" + "sync/atomic" "time" + "github.com/pokt-network/path/metrics" "github.com/pokt-network/path/protocol" "github.com/pokt-network/path/websockets" ) @@ -166,18 +168,46 @@ func (r *websocketConnRegistry) startRateSampler(ctx context.Context) { }() } -// sampleRates refreshes every live connection's frames/sec. +// sampleRates refreshes every live connection's frames/sec, and publishes each connection's +// rate as a histogram observation. +// +// The per-connection observation is the point: path_websocket_messages_total is a per-domain +// SUM, and a sum cannot tell "one firehose among a hundred idle sockets" apart from "a +// hundred ordinary subscribers". Emitting here rather than from a separate ticker reuses the +// pass that already computes the rate, so the distribution can never disagree with the +// ranking a tumble is spent on. +// +// Observations are collected under the lock but recorded after releasing it: a Prometheus +// histogram takes its own internal lock, and nesting that inside the registry mutex would put +// an unrelated subsystem on the critical path of every admin tumble and rebind. func (r *websocketConnRegistry) sampleRates(now time.Time) { if r == nil { return } + + type rateSample struct { + domain string + serviceID string + rate float64 + } + var samples []rateSample + r.mu.Lock() - defer r.mu.Unlock() - for _, svc := range r.conns { + for serviceID, svc := range r.conns { for _, entry := range svc { entry.sample(now) + samples = append(samples, rateSample{ + domain: entry.domain, + serviceID: string(serviceID), + rate: entry.rate, + }) } } + r.mu.Unlock() + + for _, s := range samples { + metrics.RecordWebsocketConnectionFrameRate(s.domain, s.serviceID, s.rate) + } } // register adds a live connection. Called when the bridge hands over its controller. @@ -230,6 +260,67 @@ func (r *websocketConnRegistry) deregister(serviceID protocol.ServiceID, wrc *we } } +// shutdownAll closes every live bridge on this pod with a proper close handshake and +// returns how many finished before ctx expired. +// +// Exists because http.Server.Shutdown explicitly does NOT close hijacked connections, and +// every websocket is hijacked — so without this the process exits, every socket dies with +// the TCP connection, and both peers report an abnormal closure. Fleetwide that made every +// rollout emit a burst of 1006s across all services in the same second: the client cannot +// tell a deploy from a crash, and the endpoint operator sees a fault they did not cause. +func (r *websocketConnRegistry) shutdownAll(ctx context.Context, reason string) int { + if r == nil { + return 0 + } + + // Snapshot under the lock and release it BEFORE closing anything. bridge.Close runs + // shutdown(), which calls AttachBridge(nil) → deregister → r.mu.Lock(). Closing while + // holding even the read lock self-deadlocks. tumble() gets away with holding it only + // because Tumble() is a non-blocking channel send; Close() is synchronous. + r.mu.RLock() + controllers := make([]websockets.BridgeController, 0, len(r.conns)) + for _, svc := range r.conns { + for _, entry := range svc { + controllers = append(controllers, entry.controller) + } + } + r.mu.RUnlock() + + if len(controllers) == 0 { + return 0 + } + + // Concurrently, because each close writes a frame to both peers under a one-second + // deadline apiece. Serially that is seconds per connection, which on a busy replica + // overruns the pod's termination grace period and gets the process SIGKILLed — the + // exact abrupt teardown this is here to avoid. + var closed atomic.Int64 + var wg sync.WaitGroup + for _, c := range controllers { + wg.Add(1) + go func(c websockets.BridgeController) { + defer wg.Done() + c.Close(reason) + closed.Add(1) + }(c) + } + + done := make(chan struct{}) + go func() { + wg.Wait() + close(done) + }() + + select { + case <-done: + case <-ctx.Done(): + // Report what actually completed rather than what was attempted: a shutdown that + // timed out having closed half the connections must not read as a clean sweep. + } + + return int(closed.Load()) +} + // updateBinding re-points an entry at the endpoint the connection just rebound onto, so // a subsequent domain-filtered tumble matches on where the connection IS, not where it // started. A no-op for connections that were never registered. diff --git a/protocol/shannon/websocket_registry_framerate_test.go b/protocol/shannon/websocket_registry_framerate_test.go new file mode 100644 index 000000000..ebe722db5 --- /dev/null +++ b/protocol/shannon/websocket_registry_framerate_test.go @@ -0,0 +1,111 @@ +package shannon + +import ( + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + "github.com/stretchr/testify/require" + + pathmetrics "github.com/pokt-network/path/metrics" +) + +// observedCount returns how many per-connection rate observations have been recorded for a +// domain/service pair. +func observedCount(t *testing.T, domain, serviceID string) uint64 { + t.Helper() + obs, err := pathmetrics.WebsocketConnectionFrameRate.GetMetricWithLabelValues(domain, serviceID) + require.NoError(t, err) + m, ok := obs.(prometheus.Metric) + require.True(t, ok, "histogram child must expose the Metric interface") + var pb dto.Metric + require.NoError(t, m.Write(&pb)) + return pb.GetHistogram().GetSampleCount() +} + +// A per-domain SUM cannot tell one firehose apart from many ordinary subscribers, which is +// the entire reason this histogram exists. That only works if EVERY live connection is +// observed on every pass — including the silent ones. Dropping the zeros would leave the +// quantiles describing only the connections that carry traffic, i.e. exactly the population +// the metric is supposed to be measured against. +func Test_sampleRates_ObservesEveryConnectionIncludingIdleOnes(t *testing.T) { + c := require.New(t) + pathmetrics.WebsocketConnectionFrameRate.Reset() + + r := newWebsocketConnRegistry() + + // One firehose and two silent connections on the same operator — the shape measured on + // live gnosis, where 2 of ~70 connections carried ~97% of the frames. + var busyFrames uint64 + r.register("gnosis", &websocketRequestContext{}, &fakeController{}, + "loud.example", "supplier-loud", func() uint64 { return busyFrames }) + r.register("gnosis", &websocketRequestContext{}, &fakeController{}, + "loud.example", "supplier-loud", func() uint64 { return 0 }) + r.register("gnosis", &websocketRequestContext{}, &fakeController{}, + "quiet.example", "supplier-quiet", func() uint64 { return 0 }) + + busyFrames = 7500 // 500 frames/s across one 15s interval + + base := time.Now() + r.sampleRates(base.Add(websocketRateSampleInterval)) + + c.Equal(uint64(2), observedCount(t, "loud.example", "gnosis"), + "both connections on the operator must be observed, not just the busy one") + c.Equal(uint64(1), observedCount(t, "quiet.example", "gnosis"), + "a silent connection still contributes an observation at zero") + + // A second pass must add one observation per live connection, so the histogram tracks + // the population over time rather than only the moment a connection appeared. + r.sampleRates(base.Add(2 * websocketRateSampleInterval)) + c.Equal(uint64(4), observedCount(t, "loud.example", "gnosis")) + c.Equal(uint64(2), observedCount(t, "quiet.example", "gnosis")) +} + +// The histogram must agree with the ranking a capped tumble is spent on: both read the same +// entry.rate from the same pass. If they could diverge, a dashboard would justify a tumble +// that the tumble itself would then decline to make. +func Test_sampleRates_HistogramAgreesWithTumbleRanking(t *testing.T) { + c := require.New(t) + pathmetrics.WebsocketConnectionFrameRate.Reset() + + r := newWebsocketConnRegistry() + var frames uint64 + r.register("gnosis", &websocketRequestContext{}, &fakeController{}, + "loud.example", "supplier-loud", func() uint64 { return frames }) + + frames = 7500 + now := time.Now().Add(websocketRateSampleInterval) + r.sampleRates(now) + + r.mu.Lock() + var sampled float64 + for _, entry := range r.conns["gnosis"] { + sampled = entry.rate + } + r.mu.Unlock() + + c.Greater(sampled, 0.0, "the sampled rate feeds both the metric and the tumble ranking") + c.Equal(uint64(1), observedCount(t, "loud.example", "gnosis")) +} + +// Deregistered connections must stop contributing: a histogram that kept observing closed +// sockets at zero would drag the distribution down over time and make a busy operator look +// progressively idler the longer the pod ran. +func Test_sampleRates_StopsObservingAfterDeregister(t *testing.T) { + c := require.New(t) + pathmetrics.WebsocketConnectionFrameRate.Reset() + + r := newWebsocketConnRegistry() + wrc := &websocketRequestContext{} + r.register("gnosis", wrc, &fakeController{}, "loud.example", "supplier-loud", func() uint64 { return 0 }) + + base := time.Now() + r.sampleRates(base.Add(websocketRateSampleInterval)) + c.Equal(uint64(1), observedCount(t, "loud.example", "gnosis")) + + r.deregister("gnosis", wrc) + r.sampleRates(base.Add(2 * websocketRateSampleInterval)) + c.Equal(uint64(1), observedCount(t, "loud.example", "gnosis"), + "a closed connection must not keep contributing observations") +} diff --git a/protocol/shannon/websocket_registry_shutdown_test.go b/protocol/shannon/websocket_registry_shutdown_test.go new file mode 100644 index 000000000..95969a522 --- /dev/null +++ b/protocol/shannon/websocket_registry_shutdown_test.go @@ -0,0 +1,118 @@ +package shannon + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/pokt-network/path/protocol" +) + +func Test_websocketShutdownAll_ClosesEveryConnectionAcrossServices(t *testing.T) { + c := require.New(t) + r := newWebsocketConnRegistry() + + eth := registerN(r, protocol.ServiceID("eth"), "one.example", 3) + gnosis := registerN(r, protocol.ServiceID("gnosis"), "two.example", 2) + + closed := r.shutdownAll(context.Background(), "gateway shutting down") + + c.Equal(5, closed) + for _, ctrl := range append(append([]*fakeController{}, eth...), gnosis...) { + c.Equal(1, ctrl.closeCount(), "every live connection must be closed, not just one service's") + } +} + +func Test_websocketShutdownAll_EmptyRegistryIsANoOp(t *testing.T) { + c := require.New(t) + c.Equal(0, newWebsocketConnRegistry().shutdownAll(context.Background(), "gateway shutting down")) +} + +// The real bridge's Close() runs shutdown(), which calls AttachBridge(nil) -> deregister, +// taking the registry's write lock. Sweeping while holding even the read lock therefore +// self-deadlocks against the very thing Close is supposed to do. +// +// This is the failure the snapshot-then-release in shutdownAll exists to prevent, and it +// is invisible to a fake that only records the call — so this fake deregisters itself, +// exactly like production. +func Test_websocketShutdownAll_DoesNotDeadlockWhenCloseDeregisters(t *testing.T) { + c := require.New(t) + r := newWebsocketConnRegistry() + svc := protocol.ServiceID("eth") + + const n = 8 + for i := 0; i < n; i++ { + wrc := &websocketRequestContext{} + ctrl := &deregisteringController{registry: r, serviceID: svc, wrc: wrc} + r.register(svc, wrc, ctrl, "one.example", "supplier-one", func() uint64 { return 0 }) + } + + done := make(chan int, 1) + go func() { + done <- r.shutdownAll(context.Background(), "gateway shutting down") + }() + + select { + case closed := <-done: + c.Equal(n, closed) + case <-time.After(10 * time.Second): + t.Fatal("shutdownAll deadlocked against deregister — the snapshot must be taken before the lock is released") + } + + // And the registry is genuinely drained, not merely unblocked. + r.mu.RLock() + remaining := len(r.conns) + r.mu.RUnlock() + c.Zero(remaining) +} + +// A pod that cannot finish being polite inside its grace period must still exit. Equally, +// a sweep that timed out having closed half the connections must not report a clean sweep +// — the count is what completed, not what was attempted. +func Test_websocketShutdownAll_HonoursTheDeadlineAndReportsOnlyWhatClosed(t *testing.T) { + c := require.New(t) + r := newWebsocketConnRegistry() + svc := protocol.ServiceID("eth") + + // One connection whose peer never answers the close handshake, and one that does. + stuck := &fakeController{closeAt: make(chan struct{})} + r.register(svc, &websocketRequestContext{}, stuck, "slow.example", "supplier-slow", func() uint64 { return 0 }) + quick := registerN(r, svc, "fast.example", 1)[0] + + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + + start := time.Now() + closed := r.shutdownAll(ctx, "gateway shutting down") + elapsed := time.Since(start) + + c.Less(elapsed, 5*time.Second, "a hung peer must not hold up process exit") + c.Equal(1, closed, "only the connection that actually closed may be counted") + c.Equal(1, quick.closeCount()) + + // Release the stuck close so the goroutine does not outlive the test. + close(stuck.closeAt) +} + +// deregisteringController mimics a real bridge: closing it removes it from the registry, +// which needs the registry's write lock. +type deregisteringController struct { + registry *websocketConnRegistry + serviceID protocol.ServiceID + wrc *websocketRequestContext + + mu sync.Mutex + closed bool +} + +func (d *deregisteringController) Tumble() bool { return false } + +func (d *deregisteringController) Close(string) { + d.registry.deregister(d.serviceID, d.wrc) + d.mu.Lock() + d.closed = true + d.mu.Unlock() +} diff --git a/protocol/shannon/websocket_registry_test.go b/protocol/shannon/websocket_registry_test.go index 7a21c15cc..301dcf476 100644 --- a/protocol/shannon/websocket_registry_test.go +++ b/protocol/shannon/websocket_registry_test.go @@ -11,11 +11,15 @@ import ( ) // fakeController is a websockets.BridgeController that records Tumble calls and can -// refuse them, standing in for a bridge with a tumble already queued. +// refuse them, standing in for a bridge with a tumble already queued. It also records +// Close calls, and can block in Close to stand in for a bridge whose peer never answers +// the close handshake. type fakeController struct { - mu sync.Mutex - calls int - refuse bool + mu sync.Mutex + calls int + refuse bool + closes int + closeAt chan struct{} // when non-nil, Close blocks until it is closed } func (f *fakeController) Tumble() bool { @@ -25,6 +29,26 @@ func (f *fakeController) Tumble() bool { return !f.refuse } +func (f *fakeController) Close(string) { + f.mu.Lock() + block := f.closeAt + f.mu.Unlock() + + if block != nil { + <-block + } + + f.mu.Lock() + f.closes++ + f.mu.Unlock() +} + +func (f *fakeController) closeCount() int { + f.mu.Lock() + defer f.mu.Unlock() + return f.closes +} + func (f *fakeController) callCount() int { f.mu.Lock() defer f.mu.Unlock() diff --git a/qos/evm/endpoint_selection_test.go b/qos/evm/endpoint_selection_test.go index 4113c1c14..9ceeebbf9 100644 --- a/qos/evm/endpoint_selection_test.go +++ b/qos/evm/endpoint_selection_test.go @@ -43,6 +43,16 @@ func (m *mockReputationService) ResetScore(ctx context.Context, key reputation.E return nil } +// Admin drains are exercised through protocol/shannon's selection harness, which asserts +// on what selection actually returns. These stubs only keep the interface satisfied. +func (m *mockReputationService) IsDomainDrained(serviceID protocol.ServiceID, domain, rpcType string) bool { + return false +} + +func (m *mockReputationService) DrainDomain(ctx context.Context, req reputation.DrainRequest) reputation.DrainResult { + return reputation.DrainResult{} +} + func (m *mockReputationService) KeyBuilderForService(serviceID protocol.ServiceID) reputation.KeyBuilder { return reputation.NewKeyBuilder("domain") } diff --git a/reputation/admin_drain.go b/reputation/admin_drain.go new file mode 100644 index 000000000..dacf4d9ac --- /dev/null +++ b/reputation/admin_drain.go @@ -0,0 +1,227 @@ +package reputation + +import ( + "context" + "strings" + "time" + + "github.com/pokt-network/path/protocol" +) + +// DrainKey identifies a benched operator for one service and RPC type. +// +// The drain is stored as this PREDICATE rather than as a resolved set of EndpointKeys, and +// that is the whole design. An earlier version resolved the target to concrete keys once and +// benched those: EndpointAddr is `supplierAddr-url` and the supplier set in a session rotates +// every rollover (~20 min), so within one session the benched keys went stale, the live +// endpoints at that operator had never been benched, and selection picked them freely — while +// the metric kept reporting the stale count as if the bench held. A drain that expires +// silently at the next rollover is worse than no drain. +// +// Matching happens against the endpoint's live URL at selection time, so an endpoint rotated +// into the session at a drained operator is benched the moment it appears. +type DrainKey struct { + ServiceID protocol.ServiceID + // Domain is the registrable domain (eTLD+1), lowercased. + Domain string + // RPCType is the lowercase wire form ("websocket", "json_rpc", …). Empty means every + // RPC type for the service. + RPCType string +} + +// DrainRequest benches one operator for a service. +type DrainRequest struct { + ServiceID protocol.ServiceID + + // Domain is the operator's registrable domain (eTLD+1). Required — an empty domain + // would bench the whole service, which is never what anyone meant to type. + Domain string + + // Duration is how long to bench for. Zero RELEASES an existing drain. + Duration time.Duration + + // RPCType optionally narrows to one protocol. Empty means all. + RPCType string + + // DryRun reports what would happen without changing anything. + DryRun bool +} + +// DrainResult reports the outcome. +type DrainResult struct { + ServiceID string `json:"service_id"` + Domain string `json:"domain"` + RPCType string `json:"rpc_type,omitempty"` + + Applied bool `json:"applied"` + Released bool `json:"released"` + + // MatchedEndpoints is how many live endpoints the target names, filled in by the + // caller. Zero means the domain matches nothing currently in session — the drain still + // applies (endpoints rotate in and out), but it is worth seeing. + MatchedEndpoints int `json:"matched_endpoints"` + + // DrainedUntil is when the bench expires. Absent on a release or dry run. + DrainedUntil string `json:"drained_until,omitempty"` + + // ActiveDrains lists every drain in force for this service after the call, so the + // caller can see the whole picture rather than just the change they made. + ActiveDrains []string `json:"active_drains"` + + // PropagationError is set when the drain could not be written to shared storage, + // meaning it applied to THIS POD ONLY. + PropagationError string `json:"propagation_error,omitempty"` + + DryRun bool `json:"dry_run"` +} + +// DrainDomain benches (or releases) one operator for a service. +// +// NOT a penalty: no Score field is touched, so the quality signal stays readable while the +// drain is in effect — reading it is usually the point of draining. Selection consults the +// drain separately, via IsDomainDrained. +// +// Per-service, expires on its own, and propagated to every replica through shared storage. +func (s *service) DrainDomain(ctx context.Context, req DrainRequest) DrainResult { + domain := strings.ToLower(strings.TrimSpace(req.Domain)) + rpcType := strings.ToLower(strings.TrimSpace(req.RPCType)) + + result := DrainResult{ + ServiceID: string(req.ServiceID), + Domain: domain, + RPCType: rpcType, + DryRun: req.DryRun, + } + + if !s.config.Enabled || domain == "" { + result.ActiveDrains = s.activeDrainsFor(req.ServiceID) + return result + } + + key := DrainKey{ServiceID: req.ServiceID, Domain: domain, RPCType: rpcType} + releasing := req.Duration <= 0 + until := time.Now().Add(req.Duration) + + if !req.DryRun { + s.mu.Lock() + if s.drainedDomains == nil { + s.drainedDomains = make(map[DrainKey]time.Time) + } + now := time.Now() + for k, exp := range s.drainedDomains { + if !now.Before(exp) { + delete(s.drainedDomains, k) + } + } + if releasing { + delete(s.drainedDomains, key) + } else { + s.drainedDomains[key] = until + } + s.mu.Unlock() + + // Propagate so ONE call benches the fleet. Applied after the local mutation so this + // pod is correct even if storage is down, with the result saying so. + var err error + if releasing { + err = s.storage.DeleteDrain(ctx, key) + } else { + err = s.storage.SetDrain(ctx, key, until) + } + if err != nil { + result.PropagationError = err.Error() + } + } + + result.Applied = !releasing + result.Released = releasing + if !releasing && !req.DryRun { + result.DrainedUntil = until.UTC().Format(time.RFC3339) + } + result.ActiveDrains = s.activeDrainsFor(req.ServiceID) + + if s.logger != nil { + s.logger.Warn(). + Str("service_id", string(req.ServiceID)). + Str("domain", domain). + Str("rpc_type", rpcType). + Dur("duration", req.Duration). + Bool("dry_run", req.DryRun). + Bool("released", releasing). + Msg("⚠️ admin reputation drain changed") + } + + return result +} + +// IsDomainDrained reports whether an operator is benched for this service and RPC type. +// +// Called from selection with the domain derived from the endpoint's LIVE URL, which is what +// makes the bench survive session rotation: it never depends on which supplier addresses +// happen to be in the current session. +// +// A drain with an empty RPCType covers every RPC type for the service. +func (s *service) IsDomainDrained(serviceID protocol.ServiceID, domain, rpcType string) bool { + if domain == "" { + return false + } + domain = strings.ToLower(domain) + rpcType = strings.ToLower(rpcType) + + s.mu.RLock() + defer s.mu.RUnlock() + if len(s.drainedDomains) == 0 { + return false + } + + now := time.Now() + for _, k := range [2]DrainKey{ + {ServiceID: serviceID, Domain: domain, RPCType: rpcType}, + {ServiceID: serviceID, Domain: domain, RPCType: ""}, + } { + if until, ok := s.drainedDomains[k]; ok && now.Before(until) { + return true + } + } + return false +} + +// activeDrainsFor lists the drains in force for a service, newest expiry last. +func (s *service) activeDrainsFor(serviceID protocol.ServiceID) []string { + s.mu.RLock() + defer s.mu.RUnlock() + + now := time.Now() + out := make([]string, 0, len(s.drainedDomains)) + for k, until := range s.drainedDomains { + if k.ServiceID != serviceID || !now.Before(until) { + continue + } + rpc := k.RPCType + if rpc == "" { + rpc = "all" + } + out = append(out, k.Domain+" ("+rpc+") until "+until.UTC().Format(time.RFC3339)) + } + return out +} + +// refreshDrains replaces the local drain set with the one in shared storage. +// +// REPLACE, not merge: a release issued on another replica shows up as the drain being absent +// from storage, and merging would keep benching it here forever. A storage failure leaves the +// local set untouched rather than clearing it — losing Redis mid-incident must not silently +// un-bench everything. +func (s *service) refreshDrains(ctx context.Context) { + drains, err := s.storage.ListDrains(ctx) + if err != nil { + if s.logger != nil { + s.logger.Warn().Err(err).Msg("failed to refresh admin drains; keeping the local set") + } + return + } + + s.mu.Lock() + s.drainedDomains = drains + s.mu.Unlock() +} diff --git a/reputation/admin_drain_test.go b/reputation/admin_drain_test.go new file mode 100644 index 000000000..17462fae4 --- /dev/null +++ b/reputation/admin_drain_test.go @@ -0,0 +1,229 @@ +package reputation + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func drainTestService(t *testing.T) (*service, Storage, context.Context) { + t.Helper() + + ctx := context.Background() + store := newMockStorage() + t.Cleanup(func() { _ = store.Close() }) + + config := Config{Enabled: true, InitialScore: 100, MinThreshold: 30} + config.HydrateDefaults() + + svc := NewService(config, store) + require.NoError(t, svc.Start(ctx)) + t.Cleanup(func() { _ = svc.Stop() }) + + return svc.(*service), store, ctx +} + +// THE REGRESSION TEST. +// +// The first implementation resolved the target to a fixed set of EndpointKeys and benched +// those. EndpointAddr is `supplierAddr-url` and a session rotates its supplier set every +// rollover, so within ~20 minutes the benched keys were stale, the live endpoints at that +// operator had never been benched, and selection picked them freely — while the metric kept +// reporting the stale count as though the bench held. In production every "drained" +// connection rebound straight back onto the drained operator. +// +// A drain is a property of the OPERATOR, so nothing about which supplier addresses happen to +// be in the current session may affect it. +func TestDrain_SurvivesSessionRotation(t *testing.T) { + svc, _, ctx := drainTestService(t) + + svc.DrainDomain(ctx, DrainRequest{ + ServiceID: "gnosis", Domain: "op-beta.example", RPCType: "websocket", Duration: 45 * time.Minute, + }) + + // Session N: these supplier addresses are in session now. + require.True(t, svc.IsDomainDrained("gnosis", "op-beta.example", "websocket")) + + // Session N+1: entirely different supplier addresses front the same operator. The drain + // is keyed on the operator, so it must still bite — this is what broke in production. + require.True(t, svc.IsDomainDrained("gnosis", "op-beta.example", "websocket"), + "a drain must not depend on which supplier addresses are in session") + + // A hostname at the same operator resolves to the same registrable domain upstream, so + // a machine rotated in mid-drain is covered too. + require.True(t, svc.IsDomainDrained("gnosis", "OP-BETA.EXAMPLE", "websocket"), + "matching must be case-insensitive") +} + +func TestDrain_ScopedToServiceAndRPCType(t *testing.T) { + svc, _, ctx := drainTestService(t) + + svc.DrainDomain(ctx, DrainRequest{ + ServiceID: "gnosis", Domain: "op-beta.example", RPCType: "websocket", Duration: 45 * time.Minute, + }) + + require.True(t, svc.IsDomainDrained("gnosis", "op-beta.example", "websocket")) + require.False(t, svc.IsDomainDrained("gnosis", "op-beta.example", "json_rpc"), + "draining websocket must leave the operator's HTTP traffic alone") + require.False(t, svc.IsDomainDrained("bsc", "op-beta.example", "websocket"), + "a drain must not leak across services") + require.False(t, svc.IsDomainDrained("gnosis", "op-gamma.example", "websocket"), + "a drain must not leak across operators") +} + +// A drain with no RPC type covers every protocol for the service. +func TestDrain_EmptyRPCTypeCoversAll(t *testing.T) { + svc, _, ctx := drainTestService(t) + + svc.DrainDomain(ctx, DrainRequest{ + ServiceID: "gnosis", Domain: "op-beta.example", Duration: 45 * time.Minute, + }) + + require.True(t, svc.IsDomainDrained("gnosis", "op-beta.example", "websocket")) + require.True(t, svc.IsDomainDrained("gnosis", "op-beta.example", "json_rpc")) + require.True(t, svc.IsDomainDrained("gnosis", "op-beta.example", "rest")) +} + +func TestDrain_ReleaseAndExpiry(t *testing.T) { + svc, _, ctx := drainTestService(t) + + svc.DrainDomain(ctx, DrainRequest{ + ServiceID: "gnosis", Domain: "op-beta.example", RPCType: "websocket", Duration: 45 * time.Minute, + }) + require.True(t, svc.IsDomainDrained("gnosis", "op-beta.example", "websocket")) + + res := svc.DrainDomain(ctx, DrainRequest{ + ServiceID: "gnosis", Domain: "op-beta.example", RPCType: "websocket", Duration: 0, + }) + require.True(t, res.Released) + require.False(t, svc.IsDomainDrained("gnosis", "op-beta.example", "websocket")) + + // A forgotten drain must lift itself — nobody should have to remember to unban anyone. + svc.mu.Lock() + svc.drainedDomains = map[DrainKey]time.Time{ + {ServiceID: "gnosis", Domain: "op-beta.example", RPCType: "websocket"}: time.Now().Add(-time.Second), + } + svc.mu.Unlock() + require.False(t, svc.IsDomainDrained("gnosis", "op-beta.example", "websocket"), + "an expired drain must not still bench") +} + +// A drain must never touch reputation: the quality signal has to stay readable while an +// operator is benched, since reading it is usually why the drain exists. +func TestDrain_DoesNotTouchScores(t *testing.T) { + svc, _, ctx := drainTestService(t) + + key := NewEndpointKey("gnosis", "pokt1abc-https://rm-01.op-beta.example", 0) + require.NoError(t, svc.RecordSignal(ctx, key, NewSuccessSignal(10*time.Millisecond))) + before, err := svc.GetScore(ctx, key) + require.NoError(t, err) + + svc.DrainDomain(ctx, DrainRequest{ + ServiceID: "gnosis", Domain: "op-beta.example", RPCType: "websocket", Duration: 45 * time.Minute, + }) + + after, err := svc.GetScore(ctx, key) + require.NoError(t, err) + require.Equal(t, before.Value, after.Value) + require.True(t, after.CooldownUntil.IsZero(), "a drain must never write a cooldown onto the score") + require.Equal(t, before.CriticalStrikes, after.CriticalStrikes) + require.Equal(t, before.RateCooldownCount, after.RateCooldownCount, + "a drain must be invisible to the rate-cooldown escalation ladder") +} + +// ---------- Fleet-wide propagation ---------- + +func TestDrain_PropagatesToOtherReplicas(t *testing.T) { + ctx := context.Background() + shared := newMockStorage() + t.Cleanup(func() { _ = shared.Close() }) + + cfg := Config{Enabled: true, InitialScore: 100, MinThreshold: 30} + cfg.HydrateDefaults() + + podA := NewService(cfg, shared).(*service) + require.NoError(t, podA.Start(ctx)) + t.Cleanup(func() { _ = podA.Stop() }) + podB := NewService(cfg, shared).(*service) + require.NoError(t, podB.Start(ctx)) + t.Cleanup(func() { _ = podB.Stop() }) + + podA.DrainDomain(ctx, DrainRequest{ + ServiceID: "gnosis", Domain: "op-beta.example", RPCType: "websocket", Duration: 45 * time.Minute, + }) + require.True(t, podA.IsDomainDrained("gnosis", "op-beta.example", "websocket")) + require.False(t, podB.IsDomainDrained("gnosis", "op-beta.example", "websocket"), "pod B has not refreshed yet") + + podB.refreshDrains(ctx) + require.True(t, podB.IsDomainDrained("gnosis", "op-beta.example", "websocket"), + "one admin call must bench every replica") + + // And a release must lift it everywhere, or un-banning would need N calls again. + podA.DrainDomain(ctx, DrainRequest{ + ServiceID: "gnosis", Domain: "op-beta.example", RPCType: "websocket", Duration: 0, + }) + podB.refreshDrains(ctx) + require.False(t, podB.IsDomainDrained("gnosis", "op-beta.example", "websocket")) +} + +// Losing storage mid-incident must not silently un-ban everyone. +func TestDrain_StorageFailureKeepsLocalDrains(t *testing.T) { + ctx := context.Background() + shared := newMockStorage() + + cfg := Config{Enabled: true, InitialScore: 100, MinThreshold: 30} + cfg.HydrateDefaults() + pod := NewService(cfg, shared).(*service) + require.NoError(t, pod.Start(ctx)) + t.Cleanup(func() { _ = pod.Stop() }) + + pod.DrainDomain(ctx, DrainRequest{ + ServiceID: "gnosis", Domain: "op-beta.example", RPCType: "websocket", Duration: 45 * time.Minute, + }) + _ = shared.Close() + + pod.refreshDrains(ctx) + require.True(t, pod.IsDomainDrained("gnosis", "op-beta.example", "websocket"), + "a storage outage must not clear in-force drains") +} + +func TestDrain_ReportsPropagationFailure(t *testing.T) { + ctx := context.Background() + shared := newMockStorage() + + cfg := Config{Enabled: true, InitialScore: 100, MinThreshold: 30} + cfg.HydrateDefaults() + pod := NewService(cfg, shared).(*service) + require.NoError(t, pod.Start(ctx)) + t.Cleanup(func() { _ = pod.Stop() }) + + _ = shared.Close() + res := pod.DrainDomain(ctx, DrainRequest{ + ServiceID: "gnosis", Domain: "op-beta.example", RPCType: "websocket", Duration: 45 * time.Minute, + }) + require.NotEmpty(t, res.PropagationError) + require.True(t, pod.IsDomainDrained("gnosis", "op-beta.example", "websocket"), "the local bench still applies") +} + +func TestDrain_DryRunChangesNothing(t *testing.T) { + svc, _, ctx := drainTestService(t) + + res := svc.DrainDomain(ctx, DrainRequest{ + ServiceID: "gnosis", Domain: "op-beta.example", RPCType: "websocket", + Duration: 45 * time.Minute, DryRun: true, + }) + require.True(t, res.DryRun) + require.False(t, svc.IsDomainDrained("gnosis", "op-beta.example", "websocket"), + "a dry run must bench nothing") +} + +// An empty domain must bench nothing rather than the whole service. +func TestDrain_EmptyDomainBenchesNothing(t *testing.T) { + svc, _, ctx := drainTestService(t) + + svc.DrainDomain(ctx, DrainRequest{ServiceID: "gnosis", Duration: 45 * time.Minute}) + require.False(t, svc.IsDomainDrained("gnosis", "op-beta.example", "websocket")) + require.False(t, svc.IsDomainDrained("gnosis", "", "websocket")) +} diff --git a/reputation/reputation.go b/reputation/reputation.go index 5ef402486..a10cf6ae9 100644 --- a/reputation/reputation.go +++ b/reputation/reputation.go @@ -43,6 +43,37 @@ func NewEndpointKey(serviceID protocol.ServiceID, endpointAddr protocol.Endpoint } } +// ParseEndpointKeyString is the inverse of EndpointKey.String(). +// +// EndpointAddr may itself contain colons (URLs carry "https://" and often a port), so the +// serviceID is taken up to the FIRST colon and the rpcType from after the LAST — anything +// between is the address, colons and all. Splitting naively on ":" corrupts every URL-keyed +// endpoint. +func ParseEndpointKeyString(s string) (EndpointKey, bool) { + lastColon := strings.LastIndex(s, ":") + if lastColon <= 0 { + return EndpointKey{}, false + } + rpcTypeStr := s[lastColon+1:] + + rest := s[:lastColon] + firstColon := strings.Index(rest, ":") + if firstColon <= 0 { + return EndpointKey{}, false + } + serviceID := rest[:firstColon] + endpointAddr := rest[firstColon+1:] + if endpointAddr == "" { + return EndpointKey{}, false + } + + return NewEndpointKey( + protocol.ServiceID(serviceID), + protocol.EndpointAddr(endpointAddr), + sharedtypes.RPCType(sharedtypes.RPCType_value[strings.ToUpper(rpcTypeStr)]), + ), true +} + // String returns a string representation of the endpoint key. // Format: "serviceID:endpointAddr:rpcType" // Example: "eth:pokt1abc-https://node.example.com:json_rpc" @@ -356,6 +387,18 @@ type ReputationService interface { // Used for administrative purposes or testing. ResetScore(ctx context.Context, key EndpointKey) error + // IsDomainDrained reports whether an operator (eTLD+1) is benched by an admin drain for + // this service and RPC type. Consulted by selection against the endpoint's LIVE URL, so + // the bench survives session rotation. Scores are never modified by a drain, so the + // quality signal stays readable while one is in force. + IsDomainDrained(serviceID protocol.ServiceID, domain, rpcType string) bool + + // DrainDomain temporarily benches every scored endpoint of one operator (eTLD+1) + // for a service by writing a cooldown expiry, without altering reputation itself. + // Administrative / experimental: it makes "what happens when this operator is not + // available" answerable without waiting for the operator to actually fail. + DrainDomain(ctx context.Context, req DrainRequest) DrainResult + // KeyBuilderForService returns the KeyBuilder for the given service. // Uses service-specific config if available, otherwise falls back to global default. KeyBuilderForService(serviceID protocol.ServiceID) KeyBuilder diff --git a/reputation/service.go b/reputation/service.go index f455c7b56..295a87f77 100644 --- a/reputation/service.go +++ b/reputation/service.go @@ -51,6 +51,12 @@ type service struct { // (ArchivalExpiresAt) is still checked at read time in GetArchivalEndpoints. archivalIndex map[protocol.ServiceID]map[EndpointKey]struct{} + // drainedDomains holds admin drains as PREDICATES — (service, domain, rpc_type) → + // expiry — rather than as resolved endpoint keys. Sessions rotate their supplier set + // every rollover, so a bench resolved to concrete keys goes stale within ~20 minutes + // while still appearing active. Guarded by mu. See DrainDomain. + drainedDomains map[DrainKey]time.Time + // Async write handling writeCh chan writeRequest stopCh chan struct{} @@ -817,9 +823,13 @@ func (s *service) refreshFromStorage(ctx context.Context) error { } s.mu.Unlock() + s.refreshDrains(ctx) + return nil } +// refreshDrains replaces the local admin-drain set with the one in shared storage. +// // SetArchivalStatus marks an endpoint as archival-capable with an expiry time. // This is called by health checks when an endpoint passes archival validation. // The status is shared across all replicas via Redis storage. diff --git a/reputation/service_test.go b/reputation/service_test.go index 79517484f..0b7432981 100644 --- a/reputation/service_test.go +++ b/reputation/service_test.go @@ -19,6 +19,7 @@ type mockStorage struct { mu sync.RWMutex scores map[string]Score perceivedBlocks map[string]uint64 + drains map[DrainKey]time.Time closed bool } @@ -26,9 +27,48 @@ func newMockStorage() *mockStorage { return &mockStorage{ scores: make(map[string]Score), perceivedBlocks: make(map[string]uint64), + drains: make(map[DrainKey]time.Time), } } +func (m *mockStorage) SetDrain(_ context.Context, key DrainKey, until time.Time) error { + m.mu.Lock() + defer m.mu.Unlock() + if m.closed { + return ErrStorageClosed + } + m.drains[key] = until + return nil +} + +func (m *mockStorage) DeleteDrain(_ context.Context, key DrainKey) error { + m.mu.Lock() + defer m.mu.Unlock() + if m.closed { + return ErrStorageClosed + } + delete(m.drains, key) + return nil +} + +func (m *mockStorage) ListDrains(_ context.Context) (map[DrainKey]time.Time, error) { + m.mu.Lock() + defer m.mu.Unlock() + if m.closed { + return nil, ErrStorageClosed + } + now := time.Now() + out := make(map[DrainKey]time.Time, len(m.drains)) + for key, until := range m.drains { + if !now.Before(until) { + delete(m.drains, key) + continue + } + out[key] = until + } + return out, nil +} + func (m *mockStorage) Get(_ context.Context, key EndpointKey) (Score, error) { m.mu.RLock() defer m.mu.RUnlock() diff --git a/reputation/storage.go b/reputation/storage.go index ac36e158b..b12af8907 100644 --- a/reputation/storage.go +++ b/reputation/storage.go @@ -3,6 +3,7 @@ package reputation import ( "context" "errors" + "time" "github.com/pokt-network/path/protocol" ) @@ -70,6 +71,26 @@ type Storage interface { RemoveEndpointBlockHeights(ctx context.Context, serviceID protocol.ServiceID, addrs []protocol.EndpointAddr) error // Close releases any resources held by the storage. + // SetDrain records an admin drain: this endpoint is benched until the given time. + // + // Drains are stored SEPARATELY from scores, and that separation is the whole point. + // An earlier version carried the bench on Score.CooldownUntil, where refreshFromStorage + // — which overwrites the local cache from storage unconditionally — erased it within a + // refresh cycle. Anything that must outlive a storage refresh cannot live on the score. + // + // Storing them here (rather than only in pod memory) is what makes one admin call apply + // fleet-wide: every replica picks the drain up on its next refresh, instead of the + // operator having to hit all N pods. + SetDrain(ctx context.Context, key DrainKey, until time.Time) error + + // DeleteDrain lifts an admin drain. Removing it from shared storage is what propagates + // a release to the other replicas. + DeleteDrain(ctx context.Context, key DrainKey) error + + // ListDrains returns every live admin drain. Expired entries are filtered out by the + // implementation, so callers can treat the result as currently-in-force. + ListDrains(ctx context.Context) (map[DrainKey]time.Time, error) + Close() error } diff --git a/reputation/storage/memory.go b/reputation/storage/memory.go index 859199bcc..35d71b39b 100644 --- a/reputation/storage/memory.go +++ b/reputation/storage/memory.go @@ -27,6 +27,10 @@ type MemoryStorage struct { endpointBlocks map[string]map[protocol.EndpointAddr]uint64 // serviceID -> endpointAddr -> block height ttl time.Duration closed bool + + // drains holds admin drains keyed by operator predicate. Deliberately separate from + // scores — see Storage.SetDrain for why a bench cannot live on the score. + drains map[reputation.DrainKey]time.Time } // scoreEntry holds a score with its expiration time. @@ -337,3 +341,52 @@ func (m *MemoryStorage) GetEndpointBlockHeights(ctx context.Context, serviceID p } return result, nil } + +// ---------- Admin drains ---------- + +// SetDrain records an admin drain. Kept separate from scores for the same reason as the +// Redis implementation: a bench carried on Score.CooldownUntil is erased by the next +// storage refresh. +func (m *MemoryStorage) SetDrain(_ context.Context, key reputation.DrainKey, until time.Time) error { + m.mu.Lock() + defer m.mu.Unlock() + if m.closed { + return reputation.ErrStorageClosed + } + if m.drains == nil { + m.drains = make(map[reputation.DrainKey]time.Time) + } + m.drains[key] = until + return nil +} + +// DeleteDrain lifts an admin drain. +func (m *MemoryStorage) DeleteDrain(_ context.Context, key reputation.DrainKey) error { + m.mu.Lock() + defer m.mu.Unlock() + if m.closed { + return reputation.ErrStorageClosed + } + delete(m.drains, key) + return nil +} + +// ListDrains returns live drains, dropping (and reaping) expired entries. +func (m *MemoryStorage) ListDrains(_ context.Context) (map[reputation.DrainKey]time.Time, error) { + m.mu.Lock() + defer m.mu.Unlock() + if m.closed { + return nil, reputation.ErrStorageClosed + } + + now := time.Now() + out := make(map[reputation.DrainKey]time.Time, len(m.drains)) + for key, until := range m.drains { + if !now.Before(until) { + delete(m.drains, key) + continue + } + out[key] = until + } + return out, nil +} diff --git a/reputation/storage/redis.go b/reputation/storage/redis.go index 6200a2769..f3668fadd 100644 --- a/reputation/storage/redis.go +++ b/reputation/storage/redis.go @@ -540,3 +540,120 @@ func (s *RedisStorage) GetEndpointBlockHeights(ctx context.Context, serviceID pr return heights, nil } + +// ---------- Admin drains ---------- + +// drainsHashKey is the single Redis hash holding every live admin drain, mapping an +// EndpointKey string to an RFC3339 expiry. +// +// A hash rather than one Redis key per drain: the refresh loop reads the whole set on +// every tick, so this is one HGETALL instead of a SCAN, and it cannot be picked up by the +// score-keyspace SCAN in List() — that pattern's parseKey rejects this name (no colons), +// so drains and scores stay strictly separate. +func (r *RedisStorage) drainsHashKey() string { + return r.keyPrefix + "__drains__" +} + +// drainField encodes a DrainKey as a hash field: "serviceID|domain|rpcType". +// +// Pipe-delimited because none of the three components can contain one — a service ID and an +// eTLD+1 are both restricted character sets, and rpcType is a fixed vocabulary. Using ":" +// (as endpoint keys do) would be ambiguous against URLs, which is what forced the endpoint +// key parser into its first-colon/last-colon dance. +func drainField(key reputation.DrainKey) string { + return string(key.ServiceID) + "|" + key.Domain + "|" + key.RPCType +} + +// parseDrainField is the inverse of drainField. An empty rpcType (drain covering every RPC +// type) round-trips as a trailing empty segment. +func parseDrainField(field string) (reputation.DrainKey, bool) { + parts := strings.Split(field, "|") + if len(parts) != 3 || parts[0] == "" || parts[1] == "" { + return reputation.DrainKey{}, false + } + return reputation.DrainKey{ + ServiceID: protocol.ServiceID(parts[0]), + Domain: parts[1], + RPCType: parts[2], + }, true +} + +// drainsHashTTLMargin is how long the drains hash outlives its longest drain. The margin +// exists so the key is never reaped while a drain is still meant to be in force, with +// room for clock skew between replicas. +const drainsHashTTLMargin = 10 * time.Minute + +// SetDrain records an admin drain in shared storage so every replica applies it. +// +// A TTL is set on the hash itself as a backstop: per-field expiry in ListDrains is what +// actually lifts a drain, but if that logic ever failed, an operator benched by a forgotten +// call would stay benched indefinitely. With the TTL the worst case is self-healing — +// nobody has to remember to unlock anyone. +// +// GT semantics: the TTL is only ever EXTENDED, never shortened, so writing a short drain +// cannot cut short a longer one already recorded in the same hash. +func (r *RedisStorage) SetDrain(ctx context.Context, key reputation.DrainKey, until time.Time) error { + if err := r.client.HSet(ctx, r.drainsHashKey(), drainField(key), until.UTC().Format(time.RFC3339)).Err(); err != nil { + return fmt.Errorf("failed to set drain in Redis: %w", err) + } + + ttl := time.Until(until) + drainsHashTTLMargin + if ttl > 0 { + // Best effort: a missing TTL only costs the backstop, and ListDrains still filters + // expired fields, so this must not fail the write. + _ = r.client.ExpireGT(ctx, r.drainsHashKey(), ttl).Err() + } + return nil +} + +// DeleteDrain lifts an admin drain across the fleet. +func (r *RedisStorage) DeleteDrain(ctx context.Context, key reputation.DrainKey) error { + if err := r.client.HDel(ctx, r.drainsHashKey(), drainField(key)).Err(); err != nil { + return fmt.Errorf("failed to delete drain from Redis: %w", err) + } + return nil +} + +// ListDrains returns the live drains, dropping expired entries. +// +// Expired fields are also deleted opportunistically. Redis cannot TTL individual hash +// fields, so without this the hash would accumulate every drain ever applied — and a +// forgotten drain must not become a permanent bench if a replica ever misreads the clock. +func (r *RedisStorage) ListDrains(ctx context.Context) (map[reputation.DrainKey]time.Time, error) { + raw, err := r.client.HGetAll(ctx, r.drainsHashKey()).Result() + if err != nil { + return nil, fmt.Errorf("failed to read drains from Redis: %w", err) + } + + now := time.Now() + drains := make(map[reputation.DrainKey]time.Time, len(raw)) + var expired []string + + for field, val := range raw { + until, parseErr := time.Parse(time.RFC3339, val) + if parseErr != nil { + // An unparseable entry can never expire on its own; drop it rather than let it + // sit in the hash forever. + expired = append(expired, field) + continue + } + if !now.Before(until) { + expired = append(expired, field) + continue + } + key, ok := parseDrainField(field) + if !ok { + expired = append(expired, field) + continue + } + drains[key] = until + } + + if len(expired) > 0 { + // Best effort: a failed cleanup only leaves dead fields that ListDrains already + // filters, so it must not fail the read. + _ = r.client.HDel(ctx, r.drainsHashKey(), expired...).Err() + } + + return drains, nil +} diff --git a/router/drain_resolve_test.go b/router/drain_resolve_test.go new file mode 100644 index 000000000..24a070093 --- /dev/null +++ b/router/drain_resolve_test.go @@ -0,0 +1,66 @@ +package router + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/pokt-network/path/protocol" +) + +func drainTestEndpoints() []protocol.EndpointDetails { + return []protocol.EndpointDetails{ + {Address: "pokt1aaa-https://f019.op-beta.example", SupplierAddress: "pokt1aaa", URL: "https://f019.op-beta.example"}, + {Address: "pokt1bbb-https://f026.op-beta.example", SupplierAddress: "pokt1bbb", URL: "https://f026.op-beta.example"}, + {Address: "pokt1ccc-https://r001.op-alpha.example", SupplierAddress: "pokt1ccc", URL: "https://r001.op-alpha.example"}, + {Address: "pokt1ddd-https://node.op-gamma.example", SupplierAddress: "pokt1ddd", URL: "https://node.op-gamma.example"}, + } +} + +// An operator is named by its eTLD+1 on a dashboard, so that must be the accepted input. +func TestResolveDrainDomain_ByOperatorDomain(t *testing.T) { + domain, matched := resolveDrainDomain(drainTestEndpoints(), "op-beta.example") + + require.Equal(t, "op-beta.example", domain) + require.Equal(t, 2, matched, "both operator-beta backends must resolve") +} + +// A hostname must still bench the whole OPERATOR, not just that machine. Benching one +// hostname would be defeated the moment a session rotated in a sibling machine — the same +// class of bug as keying the drain on endpoint addresses. +func TestResolveDrainDomain_HostnameBenchesTheOperator(t *testing.T) { + domain, matched := resolveDrainDomain(drainTestEndpoints(), "f019.op-beta.example") + + require.Equal(t, "op-beta.example", domain, + "a hostname must widen to the operator, or a rotation defeats the drain") + require.Equal(t, 1, matched) +} + +func TestResolveDrainDomain_ByFullURL(t *testing.T) { + domain, matched := resolveDrainDomain(drainTestEndpoints(), "https://r001.op-alpha.example/some/path") + + require.Equal(t, "op-alpha.example", domain, "a full URL must reduce to its operator domain") + require.Equal(t, 1, matched) +} + +// A target naming nothing resolves to no domain here; the handler then falls back to the +// literal registrable domain so the drain still applies to endpoints that rotate in later. +func TestResolveDrainDomain_UnknownTarget(t *testing.T) { + domain, matched := resolveDrainDomain(drainTestEndpoints(), "typo.example") + + require.Zero(t, matched) + require.Empty(t, domain) +} + +func TestResolveDrainDomain_CaseInsensitive(t *testing.T) { + domain, matched := resolveDrainDomain(drainTestEndpoints(), "Op-Beta.Example") + require.Equal(t, "op-beta.example", domain) + require.Equal(t, 2, matched) +} + +func TestRegistrableDomain(t *testing.T) { + require.Equal(t, "op-beta.example", registrableDomain("f019.op-beta.example")) + require.Equal(t, "example.com", registrableDomain("rm-01.eu.example.com")) + require.Equal(t, "example.com", registrableDomain("example.com")) + require.Equal(t, "localhost", registrableDomain("localhost")) +} diff --git a/router/operational_endpoints.go b/router/operational_endpoints.go index 8b88e84f3..9d8ab8ae7 100644 --- a/router/operational_endpoints.go +++ b/router/operational_endpoints.go @@ -4,10 +4,13 @@ import ( "encoding/json" "fmt" "net/http" + "net/url" "strconv" "strings" + "time" "github.com/pokt-network/path/protocol" + "github.com/pokt-network/path/reputation" ) // ServiceReadinessReporter provides readiness information for services. @@ -317,6 +320,184 @@ func (r *router) handleChainStateClear(w http.ResponseWriter, req *http.Request) }) } +const ( + // defaultDrainDuration is long enough to collect a clean rate window and short enough + // that a forgotten drain lifts itself well inside a shift. + defaultDrainDuration = 15 * time.Minute + + // maxDrainDuration is the hard ceiling on a single drain. + // + // A drain removes an operator's traffic, so an unbounded one is an outage nobody is + // tracking. Bounding it here means the worst case of "set a drain, got distracted" is + // self-healing: paired with the TTL on the shared drains key, there is no way to bench + // an operator permanently through this endpoint. Re-issue to extend. + // + // 5h covers a working session without re-issuing. It is deliberately still bounded: a + // ban meant to outlive a shift belongs in config, where it is reviewable and survives a + // restart, rather than in an admin call nobody can see after the fact. + maxDrainDuration = 5 * time.Hour +) + +// handleReputationDrain handles POST /admin/reputation/drain/{serviceId} +// +// Temporarily benches every scored endpoint belonging to one operator (eTLD+1) for the +// service, by writing a cooldown expiry onto its score. Selection already excludes +// endpoints in cooldown regardless of score, so this reuses a filter every selection path +// is guaranteed to consult. +// +// Why this exists: questions of the form "where would this traffic go if operator X were +// not available" are otherwise only answerable by waiting for X to fail. Tumbling a +// websocket connection is not a substitute — a tumble re-dials but leaves every operator +// eligible, so the connection can and does land straight back where it started. +// +// This is NOT a penalty. Value, CriticalStrikes and RecentCriticalRate are left alone, so +// the quality signal stays readable while the drain is in effect — which matters, because +// reading it is usually the entire point of draining. +// +// Per-pod in-memory state like the other admin endpoints; issue it to each pod. +// +// Query parameters: +// +// domain= operator to bench (REQUIRED) +// duration= how long, Go duration (default 15m). 0 releases this pod's drain. +// rpc_type= narrow to one protocol (websocket, json_rpc, …); default all +// dry_run=true report what would be benched without writing +// +// A drain expires on its own. It does not survive a pod restart. +func (r *router) handleReputationDrain(w http.ResponseWriter, req *http.Request) { + if r.reputationAdmin == nil { + http.Error(w, `{"error":"reputation admin not configured"}`, http.StatusServiceUnavailable) + return + } + + serviceID := strings.TrimPrefix(req.URL.Path, "/admin/reputation/drain/") + if serviceID == "" { + http.Error(w, `{"error":"service ID required: POST /admin/reputation/drain/{serviceId}"}`, http.StatusBadRequest) + return + } + + query := req.URL.Query() + + // Required rather than defaulted: a drain with no target would bench the whole + // service, which is never what anyone meant to type. + target := query.Get("domain") + if target == "" { + target = query.Get("url") + } + if target == "" { + http.Error(w, `{"error":"target required: ?domain="}`, http.StatusBadRequest) + return + } + + // Default 15m — long enough to collect a clean rate window, short enough that a + // forgotten drain heals itself well inside a shift. + duration := defaultDrainDuration + if raw := query.Get("duration"); raw != "" { + parsed, err := time.ParseDuration(raw) + if err != nil || parsed < 0 { + http.Error(w, `{"error":"duration must be a non-negative Go duration (e.g. 15m); 0 releases"}`, http.StatusBadRequest) + return + } + // Rejected rather than clamped: silently shortening a drain an operator believed + // they had set for days is worse than telling them the ceiling. Nobody should be + // able to bench an operator indefinitely by mistyping a duration. + if parsed > maxDrainDuration { + http.Error(w, fmt.Sprintf(`{"error":"duration exceeds the %s maximum; a drain must expire without anyone remembering to lift it"}`, maxDrainDuration), http.StatusBadRequest) + return + } + duration = parsed + } + + // Resolve the human-facing target (an operator domain, hostname, or URL) into the + // concrete identifiers a reputation key can carry. This MUST happen here rather than + // inside the reputation service: key granularity is per-service config — endpoint + // address, URL, domain, or supplier address — so the same operator is a hostname on + // one service and a pokt1… supplier address on another, and only the protocol layer + // holds the supplier→URL mapping that bridges them. + reporter, ok := r.readinessReporter() + if !ok { + http.Error(w, `{"error":"endpoint details unavailable; cannot resolve target"}`, http.StatusServiceUnavailable) + return + } + details, err := reporter.GetServiceEndpointDetails(protocol.ServiceID(serviceID)) + if err != nil { + http.Error(w, `{"error":"failed to list endpoints for service"}`, http.StatusInternalServerError) + return + } + + // Normalize whatever was typed — eTLD+1, hostname, or full URL — to the registrable + // domain the drain is keyed on, and report how many live endpoints it names so a typo + // is visible rather than silently benching nothing. + domain, matchedEndpoints := resolveDrainDomain(details, target) + if domain == "" { + domain = registrableDomain(strings.ToLower(strings.TrimSpace(target))) + } + + result := r.reputationAdmin.DrainDomain(req.Context(), reputation.DrainRequest{ + ServiceID: protocol.ServiceID(serviceID), + Domain: domain, + Duration: duration, + RPCType: query.Get("rpc_type"), + DryRun: query.Get("dry_run") == "true", + }) + result.MatchedEndpoints = matchedEndpoints + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(result) +} + +// resolveDrainIdentifiers maps a human-facing target — an eTLD+1, a hostname, or a full +// URL — onto every reputation-key identifier the matching endpoints could be keyed under, +// and reports how many endpoints matched. +// +// Every granularity is emitted for each matching endpoint (full address, supplier address, +// URL, hostname, eTLD+1) rather than trying to detect which one the service uses. Emitting +// a superset is safe because matching is exact string equality against keys that already +// belong to the requested service, and it means the drain does not silently bench nothing +// when a service's granularity is not what the caller assumed — which is exactly how the +// supplier-address-keyed services defeated an eTLD+1-only filter. +func resolveDrainDomain(details []protocol.EndpointDetails, target string) (string, int) { + target = strings.ToLower(strings.TrimSpace(target)) + // Accept a full URL as the target by reducing it to its host. + if u, err := url.Parse(target); err == nil && u.Host != "" { + target = strings.ToLower(u.Hostname()) + } + + domain := "" + matched := 0 + for _, d := range details { + host := "" + if u, err := url.Parse(d.URL); err == nil { + host = strings.ToLower(u.Hostname()) + } + if host == "" { + continue + } + reg := registrableDomain(host) + // Accept either an exact hostname or the operator domain, but always bench the + // OPERATOR: a drain that covered one hostname would be defeated the moment the + // session rotated in a sibling machine at the same operator. + if host != target && reg != target { + continue + } + domain = reg + matched++ + } + return domain, matched +} + +// registrableDomain returns the last two labels of a host ("rm-01.eu.example.com" → +// "example.com"). Deliberately the same naive rule the metrics layer uses for the `domain` +// label, so an operator identified from a dashboard resolves to the same thing here. +func registrableDomain(host string) string { + parts := strings.Split(host, ".") + if len(parts) < 2 { + return host + } + return strings.Join(parts[len(parts)-2:], ".") +} + // handleWebsocketTumble handles POST /admin/websocket/tumble/{serviceId} // // Forces live websocket connections for the service to rebind onto DIFFERENT suppliers. diff --git a/router/router.go b/router/router.go index 56d9242e4..f38446337 100644 --- a/router/router.go +++ b/router/router.go @@ -15,6 +15,7 @@ import ( "github.com/pokt-network/path/health" "github.com/pokt-network/path/metrics/devtools" "github.com/pokt-network/path/protocol" + "github.com/pokt-network/path/reputation" "github.com/pokt-network/path/request" ) @@ -38,6 +39,7 @@ type ( circuitBreakerAdmin CircuitBreakerAdmin chainStateAdmin ChainStateAdmin websocketAdmin WebsocketAdmin + reputationAdmin ReputationAdmin staticResponses StaticResponseResolver } gatewayHandler interface { @@ -71,6 +73,12 @@ type ( WebsocketAdmin interface { TumbleWebsockets(req protocol.WebsocketTumbleRequest) protocol.WebsocketTumbleResult } + // ReputationAdmin allows temporarily benching one operator's endpoints for a service + // via admin endpoints, so "what does this service look like without operator X" is + // answerable on demand instead of only when X happens to fail. Expires on its own. + ReputationAdmin interface { + DrainDomain(ctx context.Context, req reputation.DrainRequest) reputation.DrainResult + } ) /* --------------------------------- Init -------------------------------- */ @@ -85,6 +93,7 @@ func NewRouter( circuitBreakerAdmin CircuitBreakerAdmin, chainStateAdmin ChainStateAdmin, websocketAdmin WebsocketAdmin, + reputationAdmin ReputationAdmin, staticResponses StaticResponseResolver, ) *router { r := &router{ @@ -99,6 +108,7 @@ func NewRouter( circuitBreakerAdmin: circuitBreakerAdmin, chainStateAdmin: chainStateAdmin, websocketAdmin: websocketAdmin, + reputationAdmin: reputationAdmin, staticResponses: staticResponses, } r.handleRoutes() @@ -141,6 +151,10 @@ func (r *router) handleRoutes() { // rebind onto different suppliers (clients stay connected, subscriptions replayed) r.mux.HandleFunc("POST /admin/websocket/tumble/", r.handleWebsocketTumble) + // POST /admin/reputation/drain/{serviceId} - temporarily benches one operator's + // endpoints for a service (cooldown only; reputation itself is left untouched) + r.mux.HandleFunc("POST /admin/reputation/drain/", r.handleReputationDrain) + // requestHandlerFn defines the middleware chain for all service requests. // staticResponseMiddleware runs after the prefix strip (so it sees the cleaned path) // and before the relay handler, short-circuiting any configured static route. diff --git a/router/router_test.go b/router/router_test.go index 63320363b..0e2647814 100644 --- a/router/router_test.go +++ b/router/router_test.go @@ -32,6 +32,7 @@ func newTestRouter(t *testing.T) (*router, *MockgatewayHandler, *httptest.Server nil, // no circuit breaker in tests nil, // no chain state admin in tests nil, // no websocket admin in tests + nil, // no reputation admin in tests nil, // no static responses in tests ) ts := httptest.NewServer(r.mux) diff --git a/router/static_response_test.go b/router/static_response_test.go index 40c417a38..8f7284a46 100644 --- a/router/static_response_test.go +++ b/router/static_response_test.go @@ -39,6 +39,7 @@ func newStaticTestRouter(t *testing.T, resolver StaticResponseResolver) (*Mockga nil, nil, nil, + nil, resolver, ) ts := httptest.NewServer(r.mux) diff --git a/websockets/bridge.go b/websockets/bridge.go index ddb1dbcbc..a4e3433d9 100644 --- a/websockets/bridge.go +++ b/websockets/bridge.go @@ -148,6 +148,12 @@ type BridgeController interface { // goroutine. Returns false when the bridge cannot tumble (rebind disabled) or a // tumble is already queued for it. Tumble() bool + + // Close tears the bridge down with a proper close handshake on both sides and + // blocks until the frames have been written (or their short deadlines expire). + // Safe to call from any goroutine and idempotent — the underlying shutdown is + // sync.Once guarded, so racing with a self-initiated shutdown is harmless. + Close(reason string) } // BridgeAttacher is an optional interface an EndpointReconnector may implement to @@ -173,6 +179,15 @@ func (b *bridge) Tumble() bool { } } +// Close implements BridgeController. +// +// Routed through the same shutdown() every other teardown uses, so both peers get the +// close-frame write, the registry deregistration and the observation-channel close in the +// established order. reason is carried into the close frame's text. +func (b *bridge) Close(reason string) { + b.shutdown(fmt.Errorf("%w: %s", ErrBridgeGatewayShuttingDown, reason)) +} + // handleAdminTumble performs an operator-requested rebind onto a different supplier. // // It reuses the staleness watchdog's reconnect path — ErrEndpointTumbled makes @@ -664,19 +679,31 @@ func (b *bridge) shutdown(err error) { // see a malformed frame. closeCode, errMsg := b.determineCloseCodeAndMessage(err) closeCode = sanitizeCloseCode(closeCode) - closeMsg := websocket.FormatCloseMessage(closeCode, errMsg) + clientCloseMsg := websocket.FormatCloseMessage(closeCode, errMsg) + + // The two peers do NOT get the same code. PATH sits in the middle — + // + // external client <--(PATH is the server)-- PATH --(PATH is the client)--> relay miner + // + // — so a code that is correct facing one direction can be nonsense facing the + // other. See endpointCloseCode. + endpointCode := endpointCloseCode(closeCode) + endpointCloseMsg := clientCloseMsg + if endpointCode != closeCode { + endpointCloseMsg = websocket.FormatCloseMessage(endpointCode, errMsg) + } // Write close messages with timeout to prevent hanging on broken connections closeTimeout := time.Now().Add(1 * time.Second) if b.clientConn != nil { - if err := b.clientConn.WriteControl(websocket.CloseMessage, closeMsg, closeTimeout); err != nil { + if err := b.clientConn.WriteControl(websocket.CloseMessage, clientCloseMsg, closeTimeout); err != nil { b.logger.Warn().Err(err).Msg("⚠️ could not write close message to client connection") } b.clientConn.Close() } if b.endpointConn != nil { - if err := b.endpointConn.WriteControl(websocket.CloseMessage, closeMsg, closeTimeout); err != nil { + if err := b.endpointConn.WriteControl(websocket.CloseMessage, endpointCloseMsg, closeTimeout); err != nil { b.logger.Warn().Err(err).Msg("⚠️ could not write close message to endpoint connection") } b.endpointConn.Close() @@ -695,6 +722,33 @@ func (b *bridge) shutdown(err error) { }) } +// endpointCloseCode adapts a client-facing close code for the UPSTREAM direction. +// +// PATH is the server to the external client but the CLIENT to the relay miner, and RFC +// 6455 §7.4.1 defines 1011/1012/1013 as things a SERVER tells a client: "internal server +// error", "service restarting, reconnect", "try again later". Sent upstream they invert +// the roles — "service restarting, please reconnect" addressed to the relay miner asks it +// to reconnect to us, which is not something it does, and "internal server error" reports +// our fault as though the endpoint had one. Neither is what happened. +// +// 1001 Going Away is defined for both directions ("a server going down OR a browser +// having navigated away") and describes it exactly: the peer that dialed you is leaving. +// +// Everything else passes through unchanged — 1000 means the same thing in both +// directions, and application codes (3000-4999, e.g. the relay miner's own 4000 at +// session expiry) are propagated deliberately. +// +// gorilla accepts 1012 on read, so this is not about a protocol error; it is about the +// operator on the other end being told something true. +func endpointCloseCode(clientCode int) int { + switch clientCode { + case websocket.CloseInternalServerErr, websocket.CloseServiceRestart, websocket.CloseTryAgainLater: + return websocket.CloseGoingAway + default: + return clientCode + } +} + // sanitizeCloseCode maps RFC 6455 §7.4.1 reserved status codes — which are for // internal endpoint use and MUST NOT appear in a close frame on the wire — to a // valid code (1011 Internal Error). gorilla/websocket synthesizes a @@ -756,6 +810,13 @@ func (b *bridge) determineCloseCodeAndMessage(err error) (int, string) { // Check for specific error types using errors.Is for proper error chain handling switch { + case errors.Is(err, ErrBridgeGatewayShuttingDown): + // A deploy, not a fault. 1012 is the code that exists for exactly this and tells + // the client to come back — which it should, onto a replica that is not + // terminating. Distinguishing it from a crash is the entire point: a rollout + // previously reached both peers as 1006. + return websocket.CloseServiceRestart, "gateway shutting down, please reconnect" + case errors.Is(err, ErrBridgeContextCanceled): // Expected shutdown - encourage reconnection return websocket.CloseServiceRestart, "service restarting, please reconnect" diff --git a/websockets/bridge_close_direction_test.go b/websockets/bridge_close_direction_test.go new file mode 100644 index 000000000..f6fbfa4f5 --- /dev/null +++ b/websockets/bridge_close_direction_test.go @@ -0,0 +1,136 @@ +package websockets + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gorilla/websocket" + "github.com/pokt-network/poktroll/pkg/polylog/polyzero" + "github.com/stretchr/testify/require" + + "github.com/pokt-network/path/observation" +) + +// PATH sits between two peers and is a different role to each: +// +// external client <--(PATH is the server)-- PATH --(PATH is the client)--> relay miner +// +// so one close code cannot be right for both. shutdown() previously wrote the SAME frame +// to each, which sent the relay miner 1012 "service restarting, please reconnect" — +// a server→client code telling a server to reconnect to us. +// +// This reads the close frame off BOTH ends of one real bridge in a single test, because +// the defect is precisely that the two directions were not distinguished; a test that +// looked at either side alone would have passed before the fix. +func Test_Bridge_ShutdownSendsDirectionAppropriateCloseCodes(t *testing.T) { + c := require.New(t) + + endpointClosed := make(chan *websocket.CloseError, 1) + + // Endpoint side: upgrade, then read until PATH closes and report what it saw. + endpoint := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + conn, err := (&websocket.Upgrader{}).Upgrade(w, r, nil) + if err != nil { + return + } + defer conn.Close() + for { + if _, _, err := conn.ReadMessage(); err != nil { + closeErr, ok := err.(*websocket.CloseError) + if !ok { + closeErr = &websocket.CloseError{Code: websocket.CloseAbnormalClosure, Text: err.Error()} + } + endpointClosed <- closeErr + return + } + } + })) + defer endpoint.Close() + + // tumbleAwareReconnector implements BridgeAttacher, which is how StartBridge hands out + // the controller in production — no reaching into bridge internals. + reconnector := &tumbleAwareReconnector{mockReconnector: &mockReconnector{url: wsURL(endpoint)}} + processor := &mockWebsocketMessageProcessor{} + obsChan := make(chan *observation.RequestResponseObservations, 100) + + // Waited on at the end: the bridge goroutine reads package-level vars that other tests + // in this package mutate and restore, so letting it outlive the test races them. + bridgeDone := make(chan (<-chan struct{}), 1) + clientServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + completion, err := StartBridge( + context.Background(), polyzero.NewLogger(), r, w, + wsURL(endpoint), http.Header{}, processor, obsChan, reconnector, + ) + c.NoError(err) + bridgeDone <- completion + })) + defer clientServer.Close() + + clientConn, _, err := websocket.DefaultDialer.Dial(wsURL(clientServer), nil) + c.NoError(err) + defer clientConn.Close() + + var controller BridgeController + require.Eventually(t, func() bool { + controller = reconnector.liveController() + return controller != nil + }, 5*time.Second, 10*time.Millisecond, "bridge never attached") + + // The gateway is terminating — the exact path a rollout takes. + go controller.Close("gateway shutting down") + + // Client side: PATH is the server here, so 1012 "come back" is correct and useful. + _ = clientConn.SetReadDeadline(time.Now().Add(5 * time.Second)) + _, _, clientErr := clientConn.ReadMessage() + c.Error(clientErr) + c.True( + websocket.IsCloseError(clientErr, websocket.CloseServiceRestart), + "client should be told the service is restarting so it reconnects, got: %v", clientErr, + ) + + // Endpoint side: PATH is the CLIENT here. 1012 would tell a server to reconnect to us. + select { + case closeErr := <-endpointClosed: + c.Equal(websocket.CloseGoingAway, closeErr.Code, + "endpoint must be told the peer that dialed it is going away (1001), not handed a server→client code: %v", closeErr) + c.NotEqual(websocket.CloseServiceRestart, closeErr.Code) + c.NotEqual(websocket.CloseAbnormalClosure, closeErr.Code, + "endpoint must not see the socket simply vanish") + case <-time.After(5 * time.Second): + t.Fatal("endpoint never observed a close") + } + + select { + case completion := <-bridgeDone: + select { + case <-completion: + case <-time.After(5 * time.Second): + t.Error("bridge goroutine did not exit") + } + case <-time.After(5 * time.Second): + t.Error("bridge never started") + } +} + +// endpointCloseCode is the whole rule; pin it directly so the intent survives independent +// of the bridge wiring above. +func Test_endpointCloseCode_RemapsServerOnlyCodes(t *testing.T) { + t.Parallel() + c := require.New(t) + + // Server→client codes are meaningless addressed to a server we dialed. + c.Equal(websocket.CloseGoingAway, endpointCloseCode(websocket.CloseServiceRestart)) + c.Equal(websocket.CloseGoingAway, endpointCloseCode(websocket.CloseTryAgainLater)) + c.Equal(websocket.CloseGoingAway, endpointCloseCode(websocket.CloseInternalServerErr)) + + // Codes that mean the same thing in both directions pass through untouched. + c.Equal(websocket.CloseNormalClosure, endpointCloseCode(websocket.CloseNormalClosure)) + c.Equal(websocket.CloseGoingAway, endpointCloseCode(websocket.CloseGoingAway)) + + // Application codes are propagated deliberately — 4000 is the relay miner's own + // session-expiry close coming back to it. + c.Equal(4000, endpointCloseCode(4000)) +} diff --git a/websockets/bridge_idle_test.go b/websockets/bridge_idle_test.go index 969a9efe2..ef7f361bd 100644 --- a/websockets/bridge_idle_test.go +++ b/websockets/bridge_idle_test.go @@ -48,14 +48,34 @@ func startIdleTestBridge(t *testing.T, reconnector *mockReconnector) *websocket. processor := &mockWebsocketMessageProcessor{} obsChan := make(chan *observation.RequestResponseObservations, 100) + // Published out of the upgrade handler so cleanup can wait for the bridge goroutine to + // actually exit. Without the wait it outlives the test and keeps reading the package + // vars that shrinkIdleBounds restores on cleanup — a data race that fails -race for + // the whole package. Cleanups run LIFO and shrinkIdleBounds is always called first, so + // registering the wait here guarantees it runs before the restore. + bridgeDone := make(chan (<-chan struct{}), 1) + clientServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - _, err := StartBridge( + completion, err := StartBridge( context.Background(), polyzero.NewLogger(), r, w, wsURL(endpoint), http.Header{}, processor, obsChan, reconnector, ) c.NoError(err) + bridgeDone <- completion })) t.Cleanup(clientServer.Close) + t.Cleanup(func() { + select { + case completion := <-bridgeDone: + select { + case <-completion: + case <-time.After(5 * time.Second): + t.Error("bridge goroutine did not exit") + } + case <-time.After(5 * time.Second): + t.Error("bridge never started") + } + }) clientConn, _, err := websocket.DefaultDialer.Dial(wsURL(clientServer), nil) c.NoError(err) diff --git a/websockets/bridge_reconnect.go b/websockets/bridge_reconnect.go index ec68c3c03..4c70bc306 100644 --- a/websockets/bridge_reconnect.go +++ b/websockets/bridge_reconnect.go @@ -307,7 +307,20 @@ func (b *bridge) handleEndpointDown(down endpointDisconnect) { b.endpointCancel() } if b.endpointConn != nil { - b.endpointConn.Close() + // Close the WebSocket properly rather than dropping the TCP socket. A bare Close() + // looks to the endpoint like the peer vanished mid-read, and rebinds fire on EVERY + // session rollover — so this filled operator logs continuously across the fleet: + // geth reported `websocket: bad close code 1006`, Nethermind `An exception caused + // the WebSocket to enter the Aborted state (Failed: 0)`. + // + // Normal shutdown already does this (see bridge.go); only the rebind path skipped + // it, which is why the noise looked constant rather than occasional. 1000 Normal + // Closure is correct: a rebind is an orderly move, not a fault on the endpoint's + // part, and telling them otherwise misattributes our routing decision as their error. + // Best effort with a short deadline: the endpoint may already be gone (a rollover + // often starts BECAUSE it closed on us), and a rebind must not stall waiting to be + // polite to a socket that is not there. + CloseEndpointConn(b.endpointConn.Conn, websocket.CloseNormalClosure, "rebinding to a new session") b.endpointConn = nil } @@ -325,14 +338,16 @@ func (b *bridge) handleEndpointDown(down endpointDisconnect) { if replayErr != nil { // Replay-frame construction failed (e.g. re-signing error). The new connection // is unusable without restored subscriptions; close the client to reconnect. - newConn.Close() + // The endpoint did nothing wrong — the failure is ours — so it gets a clean 1000 + // rather than a dropped socket it would log as an abnormal closure. + CloseEndpointConn(newConn, websocket.CloseNormalClosure, "subscription replay failed") b.logger.Error().Err(replayErr).Msg("❌ [WS-REBIND] failed to build subscription replay frames — closing client") b.reconnector.OnReconnectOutcome(false, 0, ReconnectStageReplay) b.shutdown(fmt.Errorf("%w: subscription replay failed: %w", ErrBridgeEndpointUnavailable, replayErr)) return } if err := writeReplayFrames(newConn, replayFrames); err != nil { - newConn.Close() + CloseEndpointConn(newConn, websocket.CloseNormalClosure, "subscription replay failed") b.logger.Error().Err(err).Msg("❌ [WS-REBIND] failed to replay subscriptions onto new endpoint — closing client") b.reconnector.OnReconnectOutcome(false, 0, ReconnectStageReplay) b.shutdown(fmt.Errorf("%w: subscription replay write failed: %w", ErrBridgeEndpointUnavailable, err)) diff --git a/websockets/close_handshake_test.go b/websockets/close_handshake_test.go new file mode 100644 index 000000000..37e3aff3f --- /dev/null +++ b/websockets/close_handshake_test.go @@ -0,0 +1,113 @@ +package websockets + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gorilla/websocket" + "github.com/stretchr/testify/require" +) + +// TestCloseEndpointConn_SendsCloseFrameInsteadOfDroppingTheSocket asserts on what the +// ENDPOINT observes, which is the only thing that matters here: the whole defect was that +// PATH's teardowns looked like abnormal disconnects to the node operator. +// +// Asserting that we called a close helper would prove nothing — the bug was that a bare +// Close() produces a read error on the peer rather than a close frame, so the test has to +// stand up a real websocket server and read what arrives. +func TestCloseEndpointConn_SendsCloseFrameInsteadOfDroppingTheSocket(t *testing.T) { + t.Parallel() + + closeErr := make(chan error, 1) + upgrader := websocket.Upgrader{} + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + conn, err := upgrader.Upgrade(w, r, nil) + if err != nil { + closeErr <- err + return + } + defer conn.Close() + // Read until the peer goes away; the error carries how it went away. + for { + if _, _, err := conn.ReadMessage(); err != nil { + closeErr <- err + return + } + } + })) + defer srv.Close() + + wsURL := "ws" + strings.TrimPrefix(srv.URL, "http") + conn, _, err := websocket.DefaultDialer.Dial(wsURL, nil) + require.NoError(t, err) + + CloseEndpointConn(conn, websocket.CloseNormalClosure, "health check complete") + + select { + case err := <-closeErr: + require.Error(t, err, "endpoint should observe the connection ending") + + // A bare Close() surfaces as 1006 abnormal closure / unexpected EOF, which is + // exactly what operators reported. gorilla synthesizes 1006 for "peer vanished", + // so its ABSENCE is the assertion. + require.False(t, + websocket.IsCloseError(err, websocket.CloseAbnormalClosure), + "endpoint saw an abnormal closure (1006) — the close handshake did not happen: %v", err, + ) + require.NotContains(t, err.Error(), "unexpected EOF", + "endpoint saw a truncated read rather than a close frame: %v", err) + + // And the code it did see is the one we chose. + require.True(t, + websocket.IsCloseError(err, websocket.CloseNormalClosure), + "expected 1000 normal closure, got: %v", err, + ) + case <-time.After(5 * time.Second): + t.Fatal("endpoint never observed the connection closing") + } +} + +// TestCloseEndpointConn_NilIsANoOp keeps the helper safe on teardown paths that may run +// before a connection exists. +func TestCloseEndpointConn_NilIsANoOp(t *testing.T) { + t.Parallel() + require.NotPanics(t, func() { CloseEndpointConn(nil, websocket.CloseNormalClosure, "") }) +} + +// TestCloseEndpointConn_DoesNotBlockOnADeadPeer pins the best-effort contract: teardown +// frequently begins BECAUSE the peer already went away, and a rebind must not stall being +// polite to a socket that is not there. +func TestCloseEndpointConn_DoesNotBlockOnADeadPeer(t *testing.T) { + t.Parallel() + + upgrader := websocket.Upgrader{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + conn, err := upgrader.Upgrade(w, r, nil) + if err != nil { + return + } + // Hang up immediately, without a close handshake. + _ = conn.UnderlyingConn().Close() + })) + defer srv.Close() + + wsURL := "ws" + strings.TrimPrefix(srv.URL, "http") + conn, _, err := websocket.DefaultDialer.Dial(wsURL, nil) + require.NoError(t, err) + + done := make(chan struct{}) + go func() { + CloseEndpointConn(conn, websocket.CloseNormalClosure, "peer already gone") + close(done) + }() + + select { + case <-done: + case <-time.After(closeHandshakeTimeout + 3*time.Second): + t.Fatal("CloseEndpointConn blocked on a dead peer") + } +} diff --git a/websockets/connection.go b/websockets/connection.go index ea52c2138..cce10ee5d 100644 --- a/websockets/connection.go +++ b/websockets/connection.go @@ -159,6 +159,35 @@ func ConnectWebsocketEndpoint( return conn, nil } +// closeHandshakeTimeout bounds how long a close frame may take to write. Every caller of +// CloseEndpointConn is on a teardown path where the peer may already be gone, so being +// polite must never block; one second is far past a healthy write and far short of +// anything a caller would notice. +const closeHandshakeTimeout = time.Second + +// CloseEndpointConn closes an endpoint websocket connection with a proper close handshake +// rather than dropping the TCP socket underneath it. +// +// A bare conn.Close() sends no close frame, so the endpoint's read fails mid-frame and it +// reports the peer as having vanished: geth logs `websocket: close 1006 (abnormal +// closure): unexpected EOF`, Nethermind `An exception caused the WebSocket to enter the +// Aborted state (Failed: 0)`. Those are OUR teardowns being reported as the endpoint's +// fault, and the volume is not incidental — the websocket health-check probe dials, +// measures and closes ~89 times a second fleetwide, so every operator running websocket +// endpoints sees a continuous stream of abnormal closures caused entirely by PATH. +// +// Best effort by construction: the write is unchecked and deadline-bounded because a +// teardown often begins BECAUSE the peer already went away, and there is nothing useful +// to do about a close frame that cannot be delivered. +func CloseEndpointConn(conn *websocket.Conn, code int, reason string) { + if conn == nil { + return + } + closeMsg := websocket.FormatCloseMessage(code, reason) + _ = conn.WriteControl(websocket.CloseMessage, closeMsg, time.Now().Add(closeHandshakeTimeout)) + _ = conn.Close() +} + // newConnection creates a new websocket connection wrapper. // // ctx stops the connection's read/ping loops (its Done channel). onDisconnect is diff --git a/websockets/errors.go b/websockets/errors.go index d7b8a4c53..fa576d476 100644 --- a/websockets/errors.go +++ b/websockets/errors.go @@ -54,6 +54,16 @@ var ( // ErrEndpointStalled/ErrEndpointTumbled which must land elsewhere. ErrEndpointSessionExpired = errors.New("endpoint session expired: bound session ended without the supplier disconnecting") + // ErrBridgeGatewayShuttingDown indicates the gateway process is terminating and is + // closing this bridge deliberately, rather than the connection failing. + // + // Without it the process simply exits and every live websocket dies with the TCP + // socket and no close handshake, which both peers report as an abnormal closure + // (1006) — the client cannot tell a deploy from a crash, and the endpoint logs a + // fault it did not cause. http.Server.Shutdown does not help: it explicitly does not + // close hijacked connections, and every websocket is hijacked. + ErrBridgeGatewayShuttingDown = errors.New("gateway shutting down") + // ErrBridgeIdleTimeout indicates the client has held the connection open without ever // establishing a subscription and without sending a frame for idleConnectionThreshold. //