diff --git a/AGENTS.md b/AGENTS.md index a16d906b..b1e0d63e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,7 +40,7 @@ The project uses nightly `2026-02-03` toolchain (edition 2024, rust-version 1.95 | `stateless-db` | `crates/stateless-db` | redb-backed persistence: table definitions, read/write helpers, `ContractCache` | | `stateless-common` | `crates/stateless-common` | RPC client, metrics/logging utilities, witness size estimation | | `stateless-test-utils` | `crates/stateless-test-utils` | Test fixtures (blocks, witnesses, contracts) and env-var lock for integration tests | -| `stateless-r2` | `crates/stateless-r2` | Shared R2 (S3) witness primitives: SigV4 signer, object-key layout, endpoint parsing, signed PUT, and the retrying witness-object GET fetcher; consumed by mega-reth's uploaders (write) and both binaries' R2 witness sources (read) | +| `stateless-r2` | `crates/stateless-r2` | Shared R2 witness primitives: SigV4 signer, object-key layout, endpoint parsing, signed PUT, and the retrying witness-object GET fetcher over either the signed S3 API or an unsigned Cloudflare custom domain; consumed by mega-reth's uploaders (write) and both binaries' R2 witness sources (read) | | `stateless-validator` | `bin/stateless-validator` | Main binary: chain sync, parallel validation workers (`app.rs` / `workers.rs` / `main.rs`) | | `debug-trace-server` | `bin/debug-trace-server` | Standalone RPC server for debug/trace methods | @@ -125,6 +125,19 @@ Below the response cache, a bounded in-memory `BlockData` cache keyed by block h The cache pins 4 shards (largest cacheable entry = `max_bytes / 4` on any host), counts non-retained inserts, and drops an entry when a trace fails for a data-attributable reason (`TraceError::Data` — bad witness) while request-attributable failures (invalid tracer configs) never evict. In local cache mode with a `--witness-generator-endpoint` plus at least one fallback `--witness-endpoint`, request-serving witness fetches route by block age: blocks at least `--witness-local-window` blocks below the local tip skip the generator (which prunes beyond its `BACKUP` window) and fetch from the fallbacks; without the generator flag, witness endpoints are plain failover and routing is disabled. With the `--r2-*` flag group (endpoint, bucket, access key id, secret), every request-serving witness fetch tries a direct SigV4-signed R2 GET (light decode, capped at half the remaining witness budget) before the RPC chain, falling back on any failure; `--r2-max-concurrent-requests` caps R2 GETs separately from the RPC witness semaphore. +`--r2-custom-domain` is the alternative R2 target (mutually exclusive with `--r2-endpoint`, rejected at startup by name): unsigned GETs of `/{key}` through a Cloudflare custom domain fronting the bucket, which negotiates HTTP/2 (many in-flight GETs multiplex over a few connections instead of holding one each against the h1.1-only S3 endpoint) and can serve the immutable witness objects from edge cache; optional `--r2-access-client-id`/`--r2-access-client-secret` attach Cloudflare Access service-token headers (rejected at startup on a non-loopback `http://` domain, and validated as header values there so a stray newline fails by name instead of becoming a per-GET retryable transport error). +Leftover S3 flags alongside the domain are rejected by name rather than silently ignored, the client sends a `User-Agent` (Cloudflare's Browser Integrity Check 403s requests without one), and the configured target is published as `debug_trace_r2_target_info{target}` / `r2_target_info{target}` so the target-less R2 series can be attributed during a rollout. +Every `--r2-*` coherence rule — empty values, target exclusion, leftovers, an incomplete S3 quad, the Access pair, the connection count, and tuning flags with no target — lives in `stateless_common::validate_r2_flags`, so both binaries give the same verdict in the same words; each error names the offending flag, which clap cannot do without its `error-context` feature. +Version selection on the custom domain is pure ALPN (no `http2_prior_knowledge`, so the plaintext loopback path keeps working), which means a grey-clouded record, a non-Cloudflare origin, or a zone with HTTP/2 off degrades to HTTP/1.1 while the h2 tuning goes inert: the fetcher warns once with the protocol it actually got and publishes `..._r2_negotiated_http_version_info{version}`, and `pool_max_idle_per_host` is bounded so the h1.1 fallback cannot accumulate idle sockets that `pool_idle_timeout(None)` would never reap. +One `reqwest::Client` holds exactly one HTTP/2 connection and hyper never opens a second to relieve a saturated one (a pooled h2 connection reports liveness rather than stream capacity, and its dispatch channel is unbounded), so the edge's per-connection stream limit — Cloudflare advertises 100 in the `SETTINGS_MAX_CONCURRENT_STREAMS` it sends on every connection (`CLOUDFLARE_MAX_CONCURRENT_STREAMS`; `nghttp -nv https:///` reads what a given zone offers) — is a per-process ceiling rather than a per-request one. +`--r2-connections` (default 1) is what lifts it: it holds that many clients and picks one *per attempt*, so a retry leaves the connection that just failed, and one dropped connection no longer takes every in-flight GET down with it — which is the availability argument, and the one that matters in the validator's fallback-less R2 mode. +The pick is work-conserving: a connection with a free permit, searched from a rotating cursor, so a GET is never queued behind a connection whose permits are held by a slow transfer while another sits idle, and one budget of `max` is not silently partitioned into `N` budgets of `max/N` (which queues distinctly worse at the same offered load). Only when every connection is full does a fetch wait, and it waits on the cursor's own pick rather than on whichever has the most room — under saturation that one is the connection that just dropped every GET riding it. +`--r2-max-concurrent-requests` stays the cap across all of them, split evenly and rounded up (rounding down would leave some connection at zero permits and wedge every GET routed to it), so raising the connection count alone spreads the same concurrency thinner instead of raising the ceiling; the per-connection share is what must stay at or below the stream limit, and the fetcher warns at startup when it exceeds it. +The count is published as `debug_trace_r2_connections` / `r2_connections`, and is rejected by name at zero, on a non-numeric or blank value, on the S3 target (HTTP/1.1 already opens a socket per in-flight GET there), and above the cap it divides — more connections than permits would leave some of them permanently idle. +It travels as text and is parsed after clap, so a blank env line — what a templated env file renders for a variable a role does not set — is named rather than aborting startup through clap's unnamed value error, and stays inert on the validator under `--witness-source rpc`, where every `--r2-*` flag is deliberately unread. +On the validator the shared semaphore is `--witness-max-concurrent-requests`, which sizes the RPC gateway too, so the two consumers trade off against each other. +`stateless-common`'s shared JSON-RPC client pins `http1_only`: `stateless-r2` enables reqwest's `http2` feature and Cargo unifies it workspace-wide, which would otherwise move the multi-MB witness RPC payloads onto one non-adaptive h2 connection per host. +Client-side routing, budgets, and fallback match the S3 target, but edge behavior is zone configuration: **a cache rule making these objects cacheable must set 404s to bypass cache**, or a pre-upload frontier miss gets pinned for the negative-cache TTL (stalling the validator's tip-following in its fallback-less R2 mode) and a cached 404 can false-fire the below-band `kind="missing"` bucket-integrity alarm. The bucket is the same store the public gateway reads and can lead the generator at the frontier (uploader and generator RPC server publish from different files), so frontier hits are real; the frontier band is a small near-tip window (`R2_FRONTIER_WINDOW`, 32 blocks of uploader-lag grace on either side of the local tip — deliberately far narrower than the 4096-block routing window, so a stale catching-up tip cannot silence holes above it), hits there are labeled `witness_r2_frontier` (vs `witness_r2` past the band), the speculative frontier probe runs on an eighth of the remaining stage (vs half for blocks R2 must hold, so degraded R2 cannot burn half of every near-tip request's budget), and a `missing` classifies by band: in-band is the expected probe-ahead outcome (excluded from the alarm), below-band feeds `debug_trace_r2_witness_errors_total{kind="missing"}` (the bucket-integrity alarm, still covering recent-but-below-tip holes), and above-band — only reachable behind a stale catching-up tip — lands on its own `kind="missing_above_tip"` series, visible without flooding the alarm on every catch-up. Any witness-chain RPC attempt under a deadline is capped at the tightest of three bounds — half the full witness stage (`RpcClientConfig::witness_per_attempt_timeout`, derived from `--witness-timeout`), the global `--rpc-per-attempt-timeout-ms` (an explicitly stricter operator setting is honored, never loosened), and — only while the round still has an untried provider to rotate to — half of what the call still has as the attempt starts (recomputed after any concurrency-permit wait, so neither an old-block-clamped stage, a post-R2 remainder, nor a long permit queue defeats the reserve). The round's last hop, and every hop of a single-provider chain, takes the remainder whole under the ceiling instead: rotation stays protected without structurally condemning a slow-but-honest transfer, and the witness decode runs outside the attempt window (bounded by the deadline alone), so CPU-bound decode neither burns the reserve nor reads as a provider stall while a corrupt payload still rotates as the provider's error; deadline-less chain-sync fetches keep the general 20s cap so a slower-than-cap transfer still completes. diff --git a/Cargo.lock b/Cargo.lock index e4ffc487..74cdabbf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4480,6 +4480,7 @@ dependencies = [ "futures-channel", "futures-core", "futures-util", + "h2", "http", "http-body", "http-body-util", diff --git a/README.md b/README.md index 0f659009..095d62e8 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ The workspace contains two binaries and five library crates: | `stateless-db` | `crates/stateless-db` | redb-backed persistence: table definitions, read/write helpers, bounded `ContractCache` | | `stateless-common` | `crates/stateless-common` | Shared utilities: RPC client, logging, metrics | | `stateless-test-utils` | `crates/stateless-test-utils` | Test fixtures (blocks, witnesses, contracts) and env-var lock for integration tests | -| `stateless-r2` | `crates/stateless-r2` | Shared R2 (S3) witness primitives: SigV4 signer, object-key layout, endpoint parsing, signed PUT/GET with retry; consumed by mega-reth's witness uploaders (write), this repo's validator, and the trace server's witness source (read) | +| `stateless-r2` | `crates/stateless-r2` | Shared R2 witness primitives: SigV4 signer, object-key layout, endpoint parsing, signed PUT, and the retrying witness-object GET fetcher over either the signed S3 API or an unsigned Cloudflare custom domain; consumed by mega-reth's witness uploaders (write), this repo's validator, and the trace server's witness source (read) | | `stateless-validator` | `bin/stateless-validator` | Main binary: chain sync, parallel validation workers | | `debug-trace-server` | `bin/debug-trace-server` | Standalone RPC server for debug/trace methods | @@ -74,8 +74,9 @@ cargo run --release --bin stateless-validator -- \ - `--genesis-file`: Path to genesis JSON file containing hardfork activation configuration (required on first run, stored in database for subsequent runs) - `--start-block`: Trusted block hash to initialize validation from (required for first-time setup) - `--end-block`: Inclusive end block; validate up to this height, then stop cleanly (useful to slice a fixed range across multiple servers) -- `--witness-source`: Where to fetch witnesses from: `rpc` (default) or `r2` (straight from the R2 bucket over the S3 API) -- `--r2-endpoint`, `--r2-bucket`, `--r2-access-key-id`, `--r2-secret-access-key`: R2 connection settings, all required with `--witness-source r2` (prefer the env var for the secret) +- `--witness-source`: Where to fetch witnesses from: `rpc` (default) or `r2` (straight from the R2 bucket, over either the signed S3 API or a Cloudflare custom domain) +- `--r2-endpoint`, `--r2-bucket`, `--r2-access-key-id`, `--r2-secret-access-key`: R2 connection settings for the S3-endpoint target of `--witness-source r2`, all four required together (prefer the env var for the secret) +- `--r2-custom-domain`: alternative R2 target for `--witness-source r2` that replaces the four flags above — unsigned HTTP/2 GETs through a Cloudflare custom domain fronting the bucket (mutually exclusive with `--r2-endpoint`, and any of the four left set is rejected at startup by name rather than silently ignored; optional `--r2-access-client-id`/`--r2-access-client-secret` attach Cloudflare Access service-token headers, which require an `https://` domain unless it is loopback; the domain's cache rule must set 404s to bypass cache, since R2 mode has no RPC fallback and a cached pre-upload 404 would stall tip-following) - `--report-validation-endpoint`: RPC endpoint URL for reporting validated blocks via `mega_setValidatedBlocks` (disabled if not provided) - `--metrics-enabled`: Enable Prometheus metrics endpoint (disabled by default) - `--metrics-port`: Port for Prometheus metrics HTTP endpoint (default: 9090) @@ -157,6 +158,16 @@ Object storage tolerates far higher parallelism than a shared RPC gateway and th Frontier probes usually miss — the uploader typically lags the generator — and cost one fast 404; the frontier band is a small near-tip window (32 blocks of uploader-lag grace on either side of the local tip — far narrower than the 4096-block routing window, and a stale, catching-up tip cannot silence holes above it), hits there are labeled `witness_r2_frontier` (vs `witness_r2` past the band) so their hit rate stays separable, and the speculative probe runs on an eighth of the remaining stage (vs half for blocks R2 must hold), so degraded R2 cannot burn half of every near-tip request's budget before the RPC chain runs. A `missing` classifies by band: in-band is expected probe-ahead (excluded from `debug_trace_r2_witness_errors_total{kind="missing"}`), below-band feeds that bucket-integrity alarm (the object must exist there), and above-band — only reachable behind a stale catching-up tip — lands on its own `kind="missing_above_tip"` series so catch-up windows stay visible without flooding the alarm. The bucket is the same store the public gateway serves witnesses from, so at the frontier it can lead the generator (whose RPC server publishes from a different file than the uploader reads); the route needs a local DB (`--data-dir`) to anchor block age. +`--r2-custom-domain` selects the alternative R2 target (mutually exclusive with `--r2-endpoint`, rejected at startup with an error naming both): unsigned GETs of `/{key}` through a Cloudflare custom domain fronting the bucket, which negotiates HTTP/2 — many in-flight GETs multiplex over a few connections instead of holding one connection each against the HTTP/1.1-only S3 endpoint — and can serve the immutable witness objects from edge cache; optional `--r2-access-client-id`/`--r2-access-client-secret` attach Cloudflare Access service-token headers. +Client-side routing, budgets, and fallback match the S3 target; edge behavior is zone configuration, and **any cache rule making these objects cacheable must set 404s to bypass cache** — an edge-cached 404 would otherwise pin a pre-upload frontier miss for the negative-cache TTL and can false-fire the below-band `kind="missing"` bucket-integrity alarm. +The client sends a `User-Agent`, because Cloudflare's Browser Integrity Check — on by default on many zones — challenges requests without one, and that arrives here as a non-retryable 403 on every GET. +The edge's per-connection stream limit bounds the process, not the request: one `reqwest::Client` holds exactly one HTTP/2 connection and hyper does not open a second when the first saturates, and Cloudflare advertises 100 in the `SETTINGS_MAX_CONCURRENT_STREAMS` it sends on every connection (`nghttp -nv https:///` reads what a given zone offers). +`--r2-connections` (default 1) raises that ceiling by holding several clients and picking one per attempt — which also means a retry leaves the connection that just failed, and a dropped connection no longer fails every GET riding it at once. +The pick is work-conserving: a connection that has a free permit, searched from a rotating cursor, so the shared cap is not partitioned into per-connection budgets that queue independently. A fetch waits only when every connection is full. +`--r2-max-concurrent-requests` remains the cap across all connections, split evenly and rounded up, so the value to keep at or below the stream limit is the per-connection share rather than the total; the fetcher warns at startup when that share exceeds it, and `debug_trace_r2_connections` publishes the count. +A count of zero, a non-numeric or blank one, one set alongside the S3 endpoint (whose HTTP/1.1 pool already opens a socket per in-flight GET), and one larger than the cap it divides are each rejected at startup by name — the last because more connections than permits leaves some of them permanently idle. +The configured target is published as the constant-1 gauge `debug_trace_r2_target_info{target}` (`r2_target_info` on the validator), so the target-less R2 series can be attributed to one target or the other during a rollout. +Whether the domain actually delivered HTTP/2 is a separate question — selection is pure ALPN, so a misconfigured zone degrades to HTTP/1.1 with the h2 tuning inert — and is answered by `..._r2_negotiated_http_version_info{version}` plus a one-time warning naming the protocol that was negotiated. Any single witness-chain RPC attempt under a deadline is additionally capped at the tightest of: half the witness stage budget, the global per-attempt timeout, and — only while the round still has an untried provider to rotate to — half of what the call still has as the attempt starts (recomputed after any permit wait). The round's last hop, and every hop of a single-provider chain, takes the remainder whole under the ceiling instead, so a stalled endpoint (or a saturated concurrency permit — waits are deadline-bounded too) can never consume the stage while a rotation is still worth reserving for, and a slow-but-honest transfer is never structurally condemned; the witness decode runs outside the attempt window, bounded by the request deadline alone. `--r2-max-concurrent-requests` caps in-flight GETs separately from `--witness-max-concurrent-requests` — the RPC cap sizes a shared gateway, R2 tolerates far more. @@ -177,7 +188,8 @@ Each command-line flag has an equivalent environment variable: - `STATELESS_VALIDATOR_START_BLOCK` → `--start-block` - `STATELESS_VALIDATOR_END_BLOCK` → `--end-block` - `STATELESS_VALIDATOR_WITNESS_SOURCE` → `--witness-source` -- `STATELESS_VALIDATOR_R2_ENDPOINT` / `_R2_BUCKET` / `_R2_ACCESS_KEY_ID` / `_R2_SECRET_ACCESS_KEY` → `--r2-*` +- `STATELESS_VALIDATOR_R2_ENDPOINT` / `_R2_BUCKET` / `_R2_ACCESS_KEY_ID` / `_R2_SECRET_ACCESS_KEY` → `--r2-*` (the signed S3 target) +- `STATELESS_VALIDATOR_R2_CUSTOM_DOMAIN` / `_R2_ACCESS_CLIENT_ID` / `_R2_ACCESS_CLIENT_SECRET` → `--r2-custom-domain` and its Cloudflare Access pair - `STATELESS_VALIDATOR_REPORT_VALIDATION_ENDPOINT` → `--report-validation-endpoint` - `STATELESS_VALIDATOR_METRICS_ENABLED` → `--metrics-enabled` (set to `true` to enable) - `STATELESS_VALIDATOR_METRICS_PORT` → `--metrics-port` diff --git a/bin/debug-trace-server/src/main.rs b/bin/debug-trace-server/src/main.rs index 48d825f9..e3dbf9c8 100644 --- a/bin/debug-trace-server/src/main.rs +++ b/bin/debug-trace-server/src/main.rs @@ -55,7 +55,10 @@ use alloy_rpc_types_eth::BlockId; use clap::Parser; use eyre::Result; use jsonrpsee::server::{Server, ServerConfig, middleware::rpc::RpcServiceBuilder}; -use stateless_common::{RedactedSecret, RpcClient, RpcClientConfig, logging::LogArgs}; +use stateless_common::{ + R2CountFlag, R2Flag, R2Flags, R2Target, R2TuningFlag, RedactedSecret, RpcClient, + RpcClientConfig, logging::LogArgs, validate_r2_flags, +}; use stateless_core::{ BisectResolver, ChainStore, ContractStore, DivergenceLookups, PipelineConfig, chain_spec::ChainSpec, db::BlockMeta, pipeline::run_pipeline, @@ -96,6 +99,12 @@ use crate::chain_sync::{TraceFetcher, TraceHooks, TraceProcessor}; /// Command line arguments for the debug-trace-server. #[derive(Parser, Debug)] #[clap(name = "debug-trace-server", about = "Debug/Trace RPC Server")] +// Every `--r2-*` coherence rule is enforced after parsing, by +// `stateless_common::validate_r2_flags`, rather than through clap attributes: this workspace +// builds clap without its `error-context` feature (root `Cargo.toml`), so every clap rejection +// is generic and names no argument — useless to an operator debugging an env file. A blank env +// line also reads as *presence* to clap, so leaving exclusion to it would report a phantom +// conflict where the real fault is an empty value. struct Args { /// RPC server listen address. #[clap(long, env = "DEBUG_TRACE_SERVER_ADDR", default_value = "0.0.0.0:8545")] @@ -316,43 +325,97 @@ struct Args { /// path). With `--r2-bucket` and the credential flags, every witness fetch tries the /// bucket first, with the RPC witness chain as fallback — frontier probes usually miss /// (one fast 404) while historical fetches are served here. Requires a local DB - /// (`--data-dir`) to anchor block age. - #[clap(long, env = "DEBUG_TRACE_SERVER_R2_ENDPOINT", requires_all = ["r2_bucket", "r2_access_key_id", "r2_secret_access_key"])] + /// (`--data-dir`) to anchor block age. Alternative target: `--r2-custom-domain`. + #[clap(long, env = "DEBUG_TRACE_SERVER_R2_ENDPOINT")] r2_endpoint: Option, + /// Cloudflare custom domain fronting the witness bucket, e.g. + /// `https://witness.example.com` (bare origin — objects are fetched as `/{key}`). + /// Alternative to the `--r2-endpoint` credential quad: GETs go unsigned through the CDN + /// edge, which multiplexes them over HTTP/2 and can serve the immutable witness objects + /// from edge cache. Client-side routing, budgets, and fallback match the S3 target + /// (including the `--data-dir` requirement); edge behavior is zone configuration — + /// **any cache rule making these objects cacheable must set 404s to bypass cache**, + /// or a pre-upload frontier miss gets pinned for the negative-cache TTL and a cached + /// 404 can false-fire the below-band `missing` bucket-integrity alarm. + #[clap(long, env = "DEBUG_TRACE_SERVER_R2_CUSTOM_DOMAIN")] + r2_custom_domain: Option, + + /// Cloudflare Access service-token client id, sent as `CF-Access-Client-Id` on every + /// custom-domain GET. Omit when the domain is locked by an IP allowlist instead. + /// Redacted like the secret: the id alone is enough to look up the token. + #[clap(long, env = "DEBUG_TRACE_SERVER_R2_ACCESS_CLIENT_ID")] + r2_access_client_id: Option, + + /// Cloudflare Access service-token client secret, sent as `CF-Access-Client-Secret`. + /// Prefer the env var over the flag so the secret stays out of shell history and + /// process listings. + #[clap(long, env = "DEBUG_TRACE_SERVER_R2_ACCESS_CLIENT_SECRET")] + r2_access_client_secret: Option, + /// R2 bucket holding the archived witness objects. Requires `--r2-endpoint`. - #[clap(long, env = "DEBUG_TRACE_SERVER_R2_BUCKET", requires = "r2_endpoint")] + #[clap(long, env = "DEBUG_TRACE_SERVER_R2_BUCKET")] r2_bucket: Option, /// R2 access key id (Object Read). Requires `--r2-endpoint`. - #[clap(long, env = "DEBUG_TRACE_SERVER_R2_ACCESS_KEY_ID", requires = "r2_endpoint")] + #[clap(long, env = "DEBUG_TRACE_SERVER_R2_ACCESS_KEY_ID")] r2_access_key_id: Option, /// R2 secret access key. Requires `--r2-endpoint`. Prefer the env var over the flag so /// the secret stays out of shell history and process listings. - #[clap(long, env = "DEBUG_TRACE_SERVER_R2_SECRET_ACCESS_KEY", requires = "r2_endpoint")] + #[clap(long, env = "DEBUG_TRACE_SERVER_R2_SECRET_ACCESS_KEY")] r2_secret_access_key: Option, /// R2 connection-establishment timeout (milliseconds). A healthy handshake to the local - /// anycast edge is tens of ms; hangs past this are the per-IP connection-budget - /// mitigation's signature and surface as retryable `connect`-kind errors, falling back - /// to the RPC chain fast. + /// anycast edge is tens of ms. On the S3 endpoint, hangs past this are the per-IP + /// connection-budget mitigation's signature and keep landing in the connect phase, since + /// every in-flight GET holds its own connection; they surface as retryable `connect`-kind + /// errors, falling back to the RPC chain fast. The custom domain pools a single h2 + /// connection, so this bounds its first handshake and any reconnect — a path that breaks + /// after that surfaces as `transport` against the per-attempt budget instead, until the + /// keep-alive ping reaps the connection. + /// + /// An `Option` rather than a clap default so "explicitly set" stays distinguishable: + /// setting it with no R2 target configured is rejected by name instead of being silently + /// ignored. [`DEFAULT_CONNECT_TIMEOUT`] applies when it is absent. + /// + /// [`DEFAULT_CONNECT_TIMEOUT`]: stateless_r2::fetch::DEFAULT_CONNECT_TIMEOUT #[clap( long, env = "DEBUG_TRACE_SERVER_R2_CONNECT_TIMEOUT_MS", - default_value_t = stateless_r2::fetch::DEFAULT_CONNECT_TIMEOUT.as_millis() as u64, value_parser = clap::value_parser!(u64).range(100..), - requires = "r2_endpoint", )] - r2_connect_timeout_ms: u64, + r2_connect_timeout_ms: Option, /// Maximum concurrent in-flight R2 witness GETs. Omit for unlimited. Deliberately /// separate from /// `--witness-max-concurrent-requests`: that cap sizes the shared RPC gateway, while R2 /// tolerates far higher parallelism. - #[clap(long, env = "DEBUG_TRACE_SERVER_R2_MAX_CONCURRENT_REQUESTS", requires = "r2_endpoint")] + /// + /// On the custom-domain target keep this at or below the edge's per-connection stream + /// limit (Cloudflare's is 100, advertised in its `SETTINGS_MAX_CONCURRENT_STREAMS`): + /// anything above it queues inside the HTTP/2 connection rather than on this semaphore, + /// where the wait counts against the per-attempt timeout and never reaches + /// `debug_trace_r2_witness_queue_wait_seconds`. The fetcher warns at startup when the + /// configured value exceeds the limit. + #[clap(long, env = "DEBUG_TRACE_SERVER_R2_MAX_CONCURRENT_REQUESTS")] r2_max_concurrent_requests: Option, + /// HTTP/2 connections the custom-domain target spreads its GETs over (default: 1). + /// + /// One `reqwest::Client` holds exactly one HTTP/2 connection and hyper opens no second one + /// when the first saturates, so this is the only way past the edge's per-connection stream + /// limit — and the only way a dropped connection stops taking every GET riding it down + /// with it. `--r2-max-concurrent-requests` remains the cap across all of them, split evenly + /// and rounded up, so raising this alone spreads the same concurrency thinner rather than + /// lifting the ceiling; a count larger than that cap is rejected, since the surplus + /// connections could never be filled. + /// + /// Taken as text and parsed after clap so a blank env line is rejected by name rather than + /// through clap's unnamed value error. + #[clap(long, env = "DEBUG_TRACE_SERVER_R2_CONNECTIONS")] + r2_connections: Option, + /// Chain-sync pipeline tip buffer: stay this many blocks behind the upstream head so the /// fetcher does not race the witness generator — a fetch issued the moment a block appears /// typically arrives before its witness is written and burns a failed round plus a backoff @@ -435,8 +498,37 @@ fn witness_endpoint_chain(args: &Args) -> Vec<&str> { .collect() } -/// Validates cross-flag invariants that clap cannot express per-field. -fn validate_args(args: &Args) -> Result<()> { +/// This binary's `--r2-*` flags, in the spellings its operators use. +fn r2_flags<'a>(args: &'a Args, tuning: &'a [R2TuningFlag<'a>]) -> R2Flags<'a> { + R2Flags { + endpoint: R2Flag::new("--r2-endpoint", args.r2_endpoint.as_deref()), + bucket: R2Flag::new("--r2-bucket", args.r2_bucket.as_deref()), + access_key_id: R2Flag::new("--r2-access-key-id", args.r2_access_key_id.as_deref()), + secret_access_key: R2Flag::new( + "--r2-secret-access-key", + args.r2_secret_access_key.as_ref().map(AsRef::as_ref), + ), + custom_domain: R2Flag::new("--r2-custom-domain", args.r2_custom_domain.as_deref()), + access_client_id: R2Flag::new( + "--r2-access-client-id", + args.r2_access_client_id.as_ref().map(AsRef::as_ref), + ), + access_client_secret: R2Flag::new( + "--r2-access-client-secret", + args.r2_access_client_secret.as_ref().map(AsRef::as_ref), + ), + connections: R2Flag::new("--r2-connections", args.r2_connections.as_deref()), + max_concurrent_requests: R2CountFlag::new( + "--r2-max-concurrent-requests", + args.r2_max_concurrent_requests, + ), + tuning, + } +} + +/// Validates cross-flag invariants that clap cannot express per-field, and reports which R2 +/// target the flags select so the construction below does not have to decide it a second time. +fn validate_args(args: &Args) -> Result { // Early, flag-named mirror of `PipelineConfig::validate` (see its doc for the rationale); // only meaningful with chain sync, where `blocks_to_keep` becomes the stale-reset // threshold. @@ -459,41 +551,37 @@ fn validate_args(args: &Args) -> Result<()> { --witness-endpoint: list the generator once, via the dedicated flag" ); } - // Clap's `requires` wiring makes the `--r2-*` flags all-or-nothing, but it checks - // presence, not content: an env var injected as the empty string (a secret that failed - // to materialize is the common shape) would otherwise build a live source whose every - // GET reports `kind="missing"` — the exact counter operators watch for - // bucket-completeness gaps. Reject emptiness before a bad deploy can read as data loss. - let r2_values = [ - ("--r2-endpoint", args.r2_endpoint.as_deref()), - ("--r2-bucket", args.r2_bucket.as_deref()), - ("--r2-access-key-id", args.r2_access_key_id.as_deref()), - ("--r2-secret-access-key", args.r2_secret_access_key.as_ref().map(|s| s.as_ref())), + // Every R2 coherence rule — empty values, target exclusion, leftovers from the other + // target, an incomplete credential quad, the Access pair, and tuning flags with nothing to + // tune — comes from the shared validator, so the two binaries cannot drift apart on them + // again. Unlike the validator, this one checks on every startup: the check predates the + // custom-domain work here and operators already rely on a bad `--r2-*` value failing fast + // rather than surfacing later as `kind="missing"`, the counter watched for bucket gaps. + let tuning = [ + R2TuningFlag::new("--r2-connect-timeout-ms", args.r2_connect_timeout_ms.is_some()), + R2TuningFlag::new( + "--r2-max-concurrent-requests", + args.r2_max_concurrent_requests.is_some(), + ), ]; - for (flag, value) in r2_values { - if value.is_some_and(str::is_empty) { - eyre::bail!( - "{flag} is set but empty (empty env var injection?): unset it or give it a value" - ); - } - } + let target = validate_r2_flags(&r2_flags(args, &tuning))?; // The R2 route anchors block age (frontier vs historical) to the local DB tip; without // --data-dir every block would classify as frontier and a genuine bucket hole would // never reach the `kind="missing"` alarm. An operator who configured R2 asked for the // real route — fail closed instead of running a blind approximation. - if args.r2_endpoint.is_some() && args.data_dir.is_none() { + if target != R2Target::None && args.data_dir.is_none() { eyre::bail!( - "--r2-endpoint requires --data-dir: the R2 witness route anchors block age \ + "the R2 witness route requires --data-dir: it anchors block age \ (frontier vs historical) to the local DB tip" ); } - Ok(()) + Ok(target) } #[tokio::main] async fn main() -> Result<()> { let args = Args::parse(); - validate_args(&args)?; + let r2_target = validate_args(&args)?; let _log_guard = args.log.init_tracing()?; info!( @@ -518,7 +606,7 @@ async fn main() -> Result<()> { witness_timeout_secs = args.witness_timeout, witness_old_block_timeout_secs = old_block_witness_timeout_secs(&args), witness_local_window = args.witness_local_window, - r2_witness_configured = args.r2_endpoint.is_some(), + r2_witness_configured = r2_target != R2Target::None, tip_buffer = args.tip_buffer, response_cache_disabled = args.response_cache_disabled, response_cache_max_size = args.response_cache_max_size, @@ -588,40 +676,73 @@ async fn main() -> Result<()> { ), } - // Direct-from-R2 witness source. Clap's `requires` wiring makes the four - // `--r2-*` flags all-or-nothing and `validate_args` rejects empty values and the - // data-dir-less combination, so matching on the quad only splits "configured" from - // "absent". Shares the RPC path's per-attempt timeout and retry pacing. - let r2_witness_source = match ( - &args.r2_endpoint, - &args.r2_bucket, - &args.r2_access_key_id, - &args.r2_secret_access_key, - ) { - (Some(endpoint), Some(bucket), Some(access_key_id), Some(secret)) => { + // Direct-from-R2 witness source — unsigned through a Cloudflare custom domain when + // configured (h2-multiplexed, edge-cacheable), otherwise SigV4-signed against the bare + // S3 endpoint. Which one is settled by `validate_args`, whose verdict is matched on below: + // clap carries no constraint at all here, so parsing accepts both targets and the shared + // validator rejects by name — along with empty values, S3 flags left over from the other + // target, an incomplete S3 quad, and the data-dir-less combination. Shares the RPC path's + // per-attempt timeout and retry pacing. + let r2_timeouts = stateless_r2::fetch::FetchTimeouts { + per_attempt: per_attempt_timeout, + connect: args + .r2_connect_timeout_ms + .map_or(stateless_r2::fetch::DEFAULT_CONNECT_TIMEOUT, std::time::Duration::from_millis), + }; + // Dispatch on the target the shared validator already selected. Re-deriving it from the + // flags here would be a second copy of the precedence rule, which is the drift this PR + // exists to end; the reads inside each arm rest on what that validator proved. + let r2_source = match r2_target { + R2Target::None => None, + R2Target::CustomDomain { connections } => { + let domain = args.r2_custom_domain.as_deref().expect("custom-domain target"); + let access = + args.r2_access_client_id.as_ref().zip(args.r2_access_client_secret.as_ref()).map( + |(client_id, secret)| stateless_r2::fetch::CfAccessCredentials { + client_id: client_id.as_ref().to_string(), + client_secret: secret.as_ref().to_string(), + }, + ); + let cf_access = access.is_some(); + let source = R2WitnessSource::new_custom_domain( + domain, + access, + r2_timeouts, + rpc_retry, + args.r2_max_concurrent_requests, + connections, + )?; + metrics::record_r2_target(source.target_label()); + metrics::record_r2_connections(source.connections()); + info!( + domain = %source.origin(), + cf_access, + connections = source.connections(), + "Historical witness source: R2 (custom domain), RPC chain as fallback" + ); + Some(source) + } + R2Target::S3 => { + let take = |v: &Option| v.clone().expect("S3 target"); let source = R2WitnessSource::new( - endpoint, - bucket.clone(), - access_key_id.clone(), - secret.as_ref().to_string(), - stateless_r2::fetch::FetchTimeouts { - per_attempt: per_attempt_timeout, - connect: std::time::Duration::from_millis(args.r2_connect_timeout_ms), - }, + args.r2_endpoint.as_deref().expect("S3 target"), + take(&args.r2_bucket), + take(&args.r2_access_key_id), + args.r2_secret_access_key.as_ref().expect("S3 target").as_ref().to_string(), + r2_timeouts, rpc_retry, args.r2_max_concurrent_requests, )?; - // Log the parsed origin, not the raw flag value — the raw string is - // operator input and this line is info-level. - let (origin, _) = stateless_r2::endpoint::parse_endpoint(endpoint); + metrics::record_r2_target(source.target_label()); info!( - endpoint = %origin, - bucket, "Historical witness source: R2 (direct S3), RPC chain as fallback" + endpoint = %source.origin(), + bucket = args.r2_bucket.as_deref().unwrap_or_default(), + "Historical witness source: R2 (direct S3), RPC chain as fallback" ); - Some(Arc::new(source)) + Some(source) } - _ => None, }; + let r2_witness_source = r2_source.map(Arc::new); let validator_db = init_validator_db(&args, &rpc_client).await?; @@ -1433,10 +1554,11 @@ mod tests { ); } - /// The `--r2-*` group is all-or-nothing: any subset missing a member must fail parsing, - /// the full quad must parse, and none-of-them stays valid. + /// The S3 `--r2-*` set is all-or-nothing: any subset missing a member is rejected by + /// `validate_args` naming what is absent, the full quad passes, and none-of-them stays + /// valid. Rejection is post-parse throughout — clap carries no constraint here. #[test] - fn r2_flag_group_is_all_or_nothing() { + fn r2_s3_flag_set_is_all_or_nothing() { // Parsing reads every `#[clap(env = ...)]` variable, so serialize with the // env-mutating tests. let _guard = stateless_test_utils::env::env_lock(); @@ -1452,41 +1574,214 @@ mod tests { ]; assert_eq!(parse_args(&full).r2_bucket.as_deref(), Some("witness-mainnet")); - assert_eq!(parse_args(&full).r2_connect_timeout_ms, 1000, "default connect timeout"); + assert_eq!( + parse_args(&full).r2_connect_timeout_ms, + None, + "unset; the fetcher default applies" + ); let with_timeout: Vec<&str> = full.iter().copied().chain(["--r2-connect-timeout-ms", "2000"]).collect(); - assert_eq!(parse_args(&with_timeout).r2_connect_timeout_ms, 2000); - // Tuning flags are part of the fail-loud group: explicitly set without the - // endpoint they must be rejected, not silently ignored (defaults stay exempt). - let base = - ["debug-trace-server", "--rpc-endpoint", "http://r", "--witness-endpoint", "http://w"]; - assert!( - Args::try_parse_from(base.iter().copied().chain(["--r2-connect-timeout-ms", "2000"])) - .is_err() - ); + assert_eq!(parse_args(&with_timeout).r2_connect_timeout_ms, Some(2000)); + // A tuning flag explicitly set with no target must be rejected rather than silently + // ignored — by name, from `validate_args`, with defaults staying exempt. + let err = validate_args(&parse_args(&["--r2-connect-timeout-ms", "2000"])) + .unwrap_err() + .to_string(); + assert!(err.contains("--r2-connect-timeout-ms"), "{err}"); let _ = parse_args(&[]); // no R2 flags stays valid - // Dropping any one flag=value pair of the quad breaks the group. + // Dropping any one flag=value pair of the quad is rejected by `validate_args`, which + // names the missing flag; clap's `requires` used to do it but could not (see `Args`). for skip in 0..4 { let partial: Vec<&str> = full .chunks(2) .enumerate() .filter(|(i, _)| *i != skip) .flat_map(|(_, pair)| pair.iter().copied()) + .chain(["--data-dir", "/tmp/dts-test"]) .collect(); - let base = [ - "debug-trace-server", - "--rpc-endpoint", - "http://r", - "--witness-endpoint", - "http://w", - ]; + let dropped = full[skip * 2]; + let err = validate_args(&parse_args(&partial)).unwrap_err().to_string(); + assert!(err.contains(dropped), "missing {dropped} must be named: {err}"); + } + } + + /// The custom-domain R2 target: stands alone (no credential quad), unlocks the shared + /// tuning flags, is mutually exclusive with the S3 endpoint, and carries the Access + /// token pair as an all-or-nothing add-on. + #[test] + fn r2_custom_domain_target_wiring() { + let _guard = stateless_test_utils::env::env_lock(); + let base = + ["debug-trace-server", "--rpc-endpoint", "http://r", "--witness-endpoint", "http://w"]; + let domain = ["--r2-custom-domain", "https://witness.example.com"]; + + assert_eq!( + parse_args(&domain).r2_custom_domain.as_deref(), + Some("https://witness.example.com") + ); + // The shared tuning flags belong to whichever target is configured, so each must be + // accepted with the custom domain and still rejected by name with no target at all — + // the direction a revert would break silently for every custom-domain deployment. + let with_tuning: Vec<&str> = domain + .iter() + .copied() + .chain([ + "--r2-connect-timeout-ms", + "2000", + "--r2-max-concurrent-requests", + "48", + "--r2-connections", + "8", + ]) + .collect(); + assert_eq!(parse_args(&with_tuning).r2_connect_timeout_ms, Some(2000)); + assert_eq!(parse_args(&with_tuning).r2_max_concurrent_requests, Some(48)); + assert_eq!(parse_args(&with_tuning).r2_connections.as_deref(), Some("8")); + for orphan in [ + ["--r2-connect-timeout-ms", "2000"], + ["--r2-max-concurrent-requests", "48"], + ["--r2-connections", "8"], + ] { + let err = validate_args(&parse_args(&orphan)).unwrap_err().to_string(); assert!( - Args::try_parse_from(base.iter().copied().chain(partial)).is_err(), - "missing {} must fail parsing", - full[skip * 2], + err.contains(orphan[0]), + "{orphan:?} without either R2 target must be rejected by name: {err}", ); } + + // The two read targets are mutually exclusive — rejected by `validate_args` with an + // error naming both flags (clap's own rejection would name neither — see `Args`). + let both = Args::try_parse_from( + base.iter() + .copied() + .chain(domain) + .chain([ + "--r2-endpoint", + "https://acc.r2.cloudflarestorage.com", + "--r2-bucket", + "witness-mainnet", + "--r2-access-key-id", + "ak", + "--r2-secret-access-key", + "sk", + "--data-dir", + "/tmp/dts-test", + ]) + .collect::>(), + ) + .expect("both targets must parse; rejection happens post-parse"); + let err = validate_args(&both).unwrap_err().to_string(); + assert!(err.contains("--r2-endpoint") && err.contains("--r2-custom-domain"), "{err}"); + + // Access pair: each half requires the other, and both require the domain — rejected by + // name, since a half-set pair would otherwise `zip` to `None` and build a working but + // silently unauthenticated client. + let half: Vec<&str> = + domain.iter().copied().chain(["--r2-access-client-id", "tok"]).collect(); + let err = validate_args(&parse_args(&half)).unwrap_err().to_string(); + assert!(err.contains("--r2-access-client-secret"), "client id without secret: {err}"); + + // The connection count is a multiplexing concept: zero of them builds a transport that + // can carry nothing, and the HTTP/1.1 S3 target already opens a socket per in-flight + // GET, so honouring a count there would promise a spread that never happens. + let zero: Vec<&str> = domain.iter().copied().chain(["--r2-connections", "0"]).collect(); + let err = validate_args(&parse_args(&zero)).unwrap_err().to_string(); + assert!(err.contains("--r2-connections") && err.contains("at least 1"), "{err}"); + + let on_s3 = Args::try_parse_from( + base.iter() + .copied() + .chain([ + "--r2-endpoint", + "https://acc.r2.cloudflarestorage.com", + "--r2-bucket", + "witness-mainnet", + "--r2-access-key-id", + "ak", + "--r2-secret-access-key", + "sk", + "--r2-connections", + "8", + "--data-dir", + "/tmp/dts-test", + ]) + .collect::>(), + ) + .expect("rejection happens post-parse"); + let err = validate_args(&on_s3).unwrap_err().to_string(); + assert!( + err.contains("--r2-connections") && err.contains("--r2-custom-domain"), + "a connection count on the S3 target must be rejected by name: {err}" + ); + + let orphan_pair = ["--r2-access-client-id", "tok", "--r2-access-client-secret", "sk"]; + let err = validate_args(&parse_args(&orphan_pair)).unwrap_err().to_string(); + assert!(err.contains("--r2-custom-domain"), "token pair without the domain: {err}"); + let full: Vec<&str> = domain + .iter() + .copied() + .chain(["--r2-access-client-id", "tok", "--r2-access-client-secret", "sk"]) + .collect(); + assert_eq!(parse_args(&full).r2_access_client_id.as_ref().map(|s| s.as_ref()), Some("tok")); + } + + /// Custom-domain misconfigurations fail the same startup validation as the S3 flags: + /// empty values and the data-dir-less combination. + #[test] + fn validate_args_rejects_custom_domain_misconfigurations() { + let _guard = stateless_test_utils::env::env_lock(); + let dir = ["--data-dir", "/tmp/dts-test"]; + + assert!( + validate_args(&parse_args(&[ + "--r2-custom-domain", + "https://w.example.com", + dir[0], + dir[1] + ])) + .is_ok() + ); + assert!( + validate_args(&parse_args(&["--r2-custom-domain", "https://w.example.com"])).is_err(), + "custom domain without --data-dir must fail" + ); + assert!( + validate_args(&parse_args(&["--r2-custom-domain", "", dir[0], dir[1]])).is_err(), + "empty custom domain must fail" + ); + assert!( + validate_args(&parse_args(&[ + "--r2-custom-domain", + "https://w.example.com", + "--r2-access-client-id", + "tok", + "--r2-access-client-secret", + "", + dir[0], + dir[1], + ])) + .is_err(), + "empty access secret must fail" + ); + // A blank custom-domain env line over a working S3 config must get the named + // empty-value error, not a phantom target conflict. + let blank_over_s3 = parse_args(&[ + "--r2-endpoint", + "https://acc.r2.cloudflarestorage.com", + "--r2-bucket", + "witness-mainnet", + "--r2-access-key-id", + "ak", + "--r2-secret-access-key", + "sk", + "--r2-custom-domain", + "", + dir[0], + dir[1], + ]); + let err = validate_args(&blank_over_s3).unwrap_err().to_string(); + assert!(err.contains("--r2-custom-domain") && err.contains("empty"), "{err}"); } /// R2 misconfigurations fail startup validation: an empty value on any `--r2-*` flag diff --git a/bin/debug-trace-server/src/metrics.rs b/bin/debug-trace-server/src/metrics.rs index 96b92235..565757bd 100644 --- a/bin/debug-trace-server/src/metrics.rs +++ b/bin/debug-trace-server/src/metrics.rs @@ -8,7 +8,7 @@ use std::net::SocketAddr; use eyre::Result; -use metrics::{Counter, Gauge, Histogram, counter, histogram}; +use metrics::{Counter, Gauge, Histogram, counter, gauge, histogram}; use metrics_derive::Metrics; use metrics_exporter_prometheus::{Matcher, PrometheusBuilder}; pub use stateless_common::{ @@ -519,6 +519,45 @@ pub fn record_r2_witness_queue_wait(seconds: f64) { histogram!(R2_WITNESS_QUEUE_WAIT_SECONDS).record(seconds); } +/// Which R2 target this process was configured with, as a constant-1 info gauge labeled +/// `target`. The R2 series above carry no target dimension, so during a fleet rollout — some +/// hosts on the custom domain, some still on the S3 endpoint — a spike in +/// `debug_trace_r2_witness_errors_total` cannot be attributed to one or the other. Joining on +/// this gauge supplies that dimension without changing the established metric contract. +const R2_TARGET_INFO: &str = "debug_trace_r2_target_info"; + +/// Publishes the configured R2 target once at startup. +pub fn record_r2_target(target: &'static str) { + gauge!(R2_TARGET_INFO, "target" => target).set(1.0); +} + +/// How many HTTP/2 connections the custom-domain target spreads its GETs over. +/// +/// A plain value rather than an info label: it is the divisor for the per-connection stream +/// budget, so a dashboard wants to read it against `--r2-max-concurrent-requests` and against +/// the edge's limit, not group by it. Published only for the custom-domain target, where one +/// client is one connection and the count is therefore a real property of the transport. +const R2_CONNECTIONS: &str = "debug_trace_r2_connections"; + +/// Publishes the custom-domain connection count once at startup. +pub fn record_r2_connections(connections: usize) { + gauge!(R2_CONNECTIONS).set(connections as f64); +} + +/// The protocol the R2 custom-domain target actually negotiated, as a constant-1 info gauge +/// labeled `version`, published once the first response has been seen. +/// +/// Separate from the target gauge on purpose: that one answers "what was configured" and can be +/// published at startup, while this one is only knowable after a request. Folding both into one +/// gauge would mean publishing it twice with different label sets, leaving the startup series +/// stuck at 1 forever alongside the corrected one. +const R2_NEGOTIATED_VERSION_INFO: &str = "debug_trace_r2_negotiated_http_version_info"; + +/// Publishes the protocol the custom-domain target negotiated. +pub fn record_r2_negotiated_version(version: &'static str) { + gauge!(R2_NEGOTIATED_VERSION_INFO, "version" => version).set(1.0); +} + /// Canonical number → hash resolution counter, labeled `(source, outcome)` — how often /// by-number requests resolve their canonical hash from the local DB index vs upstream, /// and how often resolution misses or fails. diff --git a/bin/debug-trace-server/src/r2_witness.rs b/bin/debug-trace-server/src/r2_witness.rs index c2744a97..5060dfec 100644 --- a/bin/debug-trace-server/src/r2_witness.rs +++ b/bin/debug-trace-server/src/r2_witness.rs @@ -1,6 +1,7 @@ //! Direct-from-R2 witness source. //! -//! Fetches the primary witness object straight from the R2 bucket over the S3 API and decodes +//! Fetches the primary witness object straight from the R2 bucket — via SigV4-signed S3 +//! GETs or unsigned GETs through a Cloudflare custom domain, per construction — and decodes //! it with the **light** decoder — the trace server never verifies the witness proof, so the //! full decode's per-point elliptic-curve work would buy nothing (see //! `stateless_core::light_witness`). The transport core is `stateless-r2`'s @@ -18,7 +19,7 @@ use alloy_primitives::B256; use stateless_common::{BackoffPolicy, WitnessDecodingError, decode_witness_payload_light}; use stateless_core::{LightWitness, withdrawals::MptWitness}; use stateless_r2::{ - fetch::{FetchTimeouts, R2GetError, R2ObjectFetcher, RetryPacing}, + fetch::{CfAccessCredentials, FetchTimeouts, R2GetError, R2ObjectFetcher, RetryPacing}, keys, }; use tokio::task::JoinError; @@ -40,8 +41,8 @@ pub(crate) const KIND_MISSING_ABOVE_TIP: &str = "missing_above_tip"; /// Failure outcome of an R2 witness fetch. #[derive(Debug, thiserror::Error)] pub enum R2WitnessError { - /// The signed GET failed (absent object, transport, throttle, unexpected status, or - /// out of deadline while queued). + /// The GET failed (absent object, transport, throttle, unexpected status, or out of + /// deadline while queued). #[error(transparent)] Get(#[from] R2GetError), /// The object was fetched but its bytes did not decode to a witness tuple — a corrupt @@ -98,7 +99,29 @@ pub struct R2WitnessSource { fetcher: R2ObjectFetcher, } +/// The fetcher's pacing view of a `BackoffPolicy` — the adapter-layer conversion that keeps +/// `stateless-r2` free of a dependency on this workspace's backoff type. +fn pacing(backoff: &BackoffPolicy) -> RetryPacing { + RetryPacing { initial: backoff.initial, max: backoff.max } +} + impl R2WitnessSource { + /// The configured target's origin, for startup logging (see [`R2ObjectFetcher::origin`]). + pub fn origin(&self) -> &str { + self.fetcher.origin() + } + + /// The configured target's metric label (see [`R2ObjectFetcher::target_label`]). + pub const fn target_label(&self) -> &'static str { + self.fetcher.target_label() + } + + /// How many HTTP/2 connections the transport spreads its GETs over, for startup logging + /// (see [`R2ObjectFetcher::connections`]). + pub fn connections(&self) -> usize { + self.fetcher.connections() + } + /// Builds a source from an R2 endpoint origin, bucket, and bucket-scoped S3 credentials. /// /// `timeouts` bounds each individual GET end-to-end and in its connect phase (further @@ -120,9 +143,33 @@ impl R2WitnessSource { access_key_id, secret_access_key, timeouts, - RetryPacing { initial: retry_backoff.initial, max: retry_backoff.max }, + pacing(&retry_backoff), + max_concurrent_requests, + ) + .map_err(|e| eyre::eyre!(e))?; + Ok(Self { fetcher }) + } + + /// Builds a source that fetches unsigned through a Cloudflare custom domain fronting + /// the bucket (h2-multiplexed, edge-cacheable), with optional Cloudflare Access + /// service-token headers. The remaining parameters mean what they mean on [`Self::new`]. + pub fn new_custom_domain( + domain: &str, + access: Option, + timeouts: FetchTimeouts, + retry_backoff: BackoffPolicy, + max_concurrent_requests: Option, + connections: usize, + ) -> eyre::Result { + let fetcher = R2ObjectFetcher::new_custom_domain( + domain, + access, + timeouts, + pacing(&retry_backoff), max_concurrent_requests, + connections, ) + .map(|fetcher| fetcher.on_version_observed(metrics::record_r2_negotiated_version)) .map_err(|e| eyre::eyre!(e))?; Ok(Self { fetcher }) } @@ -218,6 +265,38 @@ mod tests { Instant::now() + Duration::from_secs(5) } + /// The custom-domain source serves the same decode path end-to-end, requesting the bare + /// `/{key}` layout (no bucket segment, no SigV4 authorization). + #[tokio::test] + async fn custom_domain_source_decodes_and_requests_bare_key() { + let (salt_witness, mpt_witness): (_, MptWitness) = + TestFixtures::mainnet_shared().first_paired_witness(); + let (_, payload) = stateless_common::encode_witness_payload(&salt_witness, &mpt_witness) + .expect("fixture witness must encode"); + + let (domain, _, heads) = + stateless_test_utils::mock_r2::mock_r2_capturing(vec![(200, payload)]).await; + let source = R2WitnessSource::new_custom_domain( + &domain, + None, + FetchTimeouts { + per_attempt: Duration::from_secs(5), + connect: stateless_r2::fetch::DEFAULT_CONNECT_TIMEOUT, + }, + BackoffPolicy::new(Duration::from_millis(5), Duration::from_millis(20)), + None, + 1, + ) + .unwrap(); + source + .get_witness_light(1, B256::ZERO, deadline()) + .await + .expect("valid object must fetch and decode"); + let head = heads.lock().unwrap()[0].to_lowercase(); + assert!(head.starts_with("get /block/0_999/1."), "bucketless key layout: {head}"); + assert!(!head.contains("authorization:"), "custom-domain GET must be unsigned: {head}"); + } + /// A fixture witness encoded with the uploader's `encode_witness_payload` must /// light-decode to the same kvs the RPC light path yields. #[tokio::test] diff --git a/bin/stateless-validator/src/app.rs b/bin/stateless-validator/src/app.rs index 2f744807..db7fa462 100644 --- a/bin/stateless-validator/src/app.rs +++ b/bin/stateless-validator/src/app.rs @@ -8,7 +8,8 @@ use alloy_rpc_types_eth::BlockId; use clap::{Parser, ValueEnum}; use eyre::Result; use stateless_common::{ - BackoffPolicy, RedactedSecret, RpcClient, RpcClientConfig, logging::LogArgs, + BackoffPolicy, R2CountFlag, R2Flag, R2Flags, R2Target, RedactedSecret, RpcClient, + RpcClientConfig, logging::LogArgs, validate_r2_flags, }; use stateless_core::{ChainStore, ContractStore, chain_spec::ChainSpec, db::BlockMeta}; use stateless_db::ContractCache; @@ -23,7 +24,8 @@ pub enum WitnessSource { /// `mega_getBlockWitness` RPC. #[default] Rpc, - /// Straight from the R2 bucket over the S3 API. Requires the `--r2-*` flags. + /// Straight from the R2 bucket: either the signed S3 API (`--r2-endpoint` and its + /// credential quad) or an unsigned Cloudflare custom domain (`--r2-custom-domain`). R2, } @@ -96,34 +98,86 @@ pub struct CommandLineArgs { pub witness_source: WitnessSource, /// R2 S3 endpoint origin, e.g. `https://.r2.cloudflarestorage.com` (no bucket path). - /// Required when `--witness-source r2`. + /// Required when `--witness-source r2`, unless `--r2-custom-domain` is used instead + /// (mutually exclusive — rejected at startup with an error naming both). #[clap(long, env = "STATELESS_VALIDATOR_R2_ENDPOINT")] pub r2_endpoint: Option, - /// R2 bucket holding the witnesses (e.g. `witness-mainnet`). Required when `--witness-source - /// r2`. + /// Cloudflare custom domain fronting the witness bucket, e.g. `https://witness.example.com` + /// (bare origin — objects are fetched as `/{key}`). Alternative to the `--r2-endpoint` + /// credential quad with `--witness-source r2`: GETs go unsigned through the CDN edge, which + /// multiplexes them over HTTP/2 and can serve the immutable witness objects from edge cache. + /// ⚠ R2 mode has no RPC fallback and retries a missing witness until the uploader wins the + /// race, so **any edge cache rule making these objects cacheable must set 404s to bypass + /// cache** — an edge-cached 404 would otherwise pin every pre-upload frontier miss for the + /// negative-cache TTL and stall tip-following for minutes at a time. + #[clap(long, env = "STATELESS_VALIDATOR_R2_CUSTOM_DOMAIN")] + pub r2_custom_domain: Option, + + /// Cloudflare Access service-token client id, sent as `CF-Access-Client-Id` on every + /// custom-domain GET. Omit when the domain is locked by an IP allowlist instead. + /// Redacted like the secret: the id alone is enough to look up the token. + #[clap(long, env = "STATELESS_VALIDATOR_R2_ACCESS_CLIENT_ID")] + pub r2_access_client_id: Option, + + /// Cloudflare Access service-token client secret, sent as `CF-Access-Client-Secret`. Prefer + /// the env var over the flag. + #[clap(long, env = "STATELESS_VALIDATOR_R2_ACCESS_CLIENT_SECRET")] + pub r2_access_client_secret: Option, + + /// R2 bucket holding the witnesses (e.g. `witness-mainnet`). Required for the S3-endpoint + /// target of `--witness-source r2` (not used with `--r2-custom-domain`). #[clap(long, env = "STATELESS_VALIDATOR_R2_BUCKET")] pub r2_bucket: Option, - /// R2 access key id (Object Read). Required when `--witness-source r2`. + /// R2 access key id (Object Read). Required for the S3-endpoint target of + /// `--witness-source r2` (not used with `--r2-custom-domain`). #[clap(long, env = "STATELESS_VALIDATOR_R2_ACCESS_KEY_ID")] pub r2_access_key_id: Option, - /// R2 secret access key. Required when `--witness-source r2`. Prefer the env var over the - /// flag. + /// R2 secret access key. Required for the S3-endpoint target of `--witness-source r2` + /// (not used with `--r2-custom-domain`). Prefer the env var over the flag. #[clap(long, env = "STATELESS_VALIDATOR_R2_SECRET_ACCESS_KEY")] pub r2_secret_access_key: Option, /// R2 connection-establishment timeout (milliseconds). A healthy handshake to the local - /// anycast edge is tens of ms; hangs past this are the per-IP connection-budget - /// mitigation's signature and surface as retryable `connect`-kind errors. + /// anycast edge is tens of ms. On the S3 endpoint, hangs past this are the per-IP + /// connection-budget mitigation's signature and keep landing in the connect phase, since + /// every in-flight GET holds its own connection; they surface as retryable `connect`-kind + /// errors. The custom domain pools a single h2 connection, so this bounds its first + /// handshake and any reconnect — a path that breaks after that surfaces as `transport` + /// against the per-attempt budget until the keep-alive ping reaps the connection, and in + /// R2 mode there is no RPC chain to fall back to. + /// + /// Left as an `Option` rather than defaulted by clap so that "explicitly set" stays + /// distinguishable; [`DEFAULT_CONNECT_TIMEOUT`] applies when it is absent. Unlike the trace + /// server, this binary does not reject it for having no R2 target: under + /// `--witness-source rpc` every `--r2-*` flag is inert by design, and under + /// `--witness-source r2` a target is mandatory, so the rule could never fire. + /// + /// [`DEFAULT_CONNECT_TIMEOUT`]: stateless_r2::fetch::DEFAULT_CONNECT_TIMEOUT #[clap( long, env = "STATELESS_VALIDATOR_R2_CONNECT_TIMEOUT_MS", - default_value_t = stateless_r2::fetch::DEFAULT_CONNECT_TIMEOUT.as_millis() as u64, value_parser = clap::value_parser!(u64).range(100..), )] - pub r2_connect_timeout_ms: u64, + pub r2_connect_timeout_ms: Option, + + /// HTTP/2 connections the custom-domain target spreads its GETs over (default: 1). + /// + /// One `reqwest::Client` holds exactly one HTTP/2 connection and hyper opens no second one + /// when the first saturates, so this is the only way past the edge's per-connection stream + /// limit — and the only way one dropped connection stops taking every in-flight GET with + /// it, which matters here because R2 mode has no RPC fallback. + /// `--witness-max-concurrent-requests` is still the cap across all of them, split evenly + /// and rounded up, so raising this alone spreads the same concurrency thinner rather than + /// raising the ceiling; a count larger than that cap is rejected, since the surplus + /// connections could never be filled. + /// + /// Taken as text and parsed after clap so a blank env line stays inert under + /// `--witness-source rpc` instead of aborting startup with clap's unnamed value error. + #[clap(long, env = "STATELESS_VALIDATOR_R2_CONNECTIONS")] + pub r2_connections: Option, /// Optional inclusive end block: validate up to this height, then stop cleanly. Used to slice /// a fixed block range across multiple servers. Omit to follow the chain tip indefinitely. @@ -161,6 +215,11 @@ pub struct CommandLineArgs { /// Maximum concurrent in-flight witness fetches, independent of the data cap. Omit for /// unlimited. Applies to both RPC witness calls and, with `--witness-source r2`, R2 GETs. + /// + /// Against `--r2-custom-domain` this is also what bounds the GETs multiplexed onto the + /// HTTP/2 connection, so keep it at or below the edge's per-connection stream limit + /// (Cloudflare's is 100): above it the surplus queues inside the connection instead, where + /// the wait is unobservable and still counts against the per-attempt timeout. #[clap(long, env = "STATELESS_VALIDATOR_WITNESS_MAX_CONCURRENT_REQUESTS")] pub witness_max_concurrent_requests: Option, @@ -244,7 +303,7 @@ pub async fn run() -> Result<()> { info!("Metrics disabled"); } - let work_dir = PathBuf::from(args.data_dir); + let work_dir = PathBuf::from(&args.data_dir); std::fs::create_dir_all(&work_dir) .map_err(|e| eyre::eyre!("Failed to create data dir {}: {e}", work_dir.display()))?; @@ -282,27 +341,14 @@ pub async fn run() -> Result<()> { straight from the R2 bucket, and there is no RPC witness fallback" ); } - let endpoint = require_r2(&args.r2_endpoint, "--r2-endpoint")?; - let bucket = require_r2(&args.r2_bucket, "--r2-bucket")?; - let access_key_id = require_r2(&args.r2_access_key_id, "--r2-access-key-id")?; - let secret_access_key = - require_r2(&args.r2_secret_access_key, "--r2-secret-access-key")?; - // Log the parsed origin, not the raw flag value — the raw string is operator - // input and this line is info-level. - let (origin, _) = stateless_r2::endpoint::parse_endpoint(endpoint); - info!(endpoint = %origin, bucket, "Witness source: R2 (direct S3)"); - Some(Arc::new(R2WitnessClient::new( - endpoint, - bucket.to_string(), - access_key_id.to_string(), - secret_access_key.to_string(), - stateless_r2::fetch::FetchTimeouts { - per_attempt: per_attempt_timeout, - connect: Duration::from_millis(args.r2_connect_timeout_ms), - }, - rpc_config.rpc_retry.clone(), - args.witness_max_concurrent_requests, - )?)) + let timeouts = stateless_r2::fetch::FetchTimeouts { + per_attempt: per_attempt_timeout, + connect: args + .r2_connect_timeout_ms + .map_or(stateless_r2::fetch::DEFAULT_CONNECT_TIMEOUT, Duration::from_millis), + }; + let client = build_r2_client(&args, timeouts, rpc_config.rpc_retry.clone())?; + Some(Arc::new(client)) } }; @@ -402,27 +448,105 @@ fn override_ms(ms: Option, default: Duration) -> Duration { ms.map(Duration::from_millis).unwrap_or(default) } -/// Unwraps a required `--r2-*` argument, erroring with the flag name when it is absent. -fn require_r2<'a, T: AsRef>(value: &'a Option, flag: &str) -> Result<&'a str> { - value - .as_ref() - .map(AsRef::as_ref) - .filter(|v| !v.is_empty()) - .ok_or_else(|| eyre::eyre!("{flag} is required with --witness-source r2")) +/// Builds the R2 witness client for `--witness-source r2`: the custom-domain target when +/// `--r2-custom-domain` is set, the SigV4-signed S3 target otherwise. +/// +/// Which target wins is already settled by the [`validate_r2_flags`] call below, so the arms +/// read the one that was chosen — a set-but-empty flag belonging to the *other* target is +/// rejected there rather than reaching a constructor. +fn build_r2_client( + args: &CommandLineArgs, + timeouts: stateless_r2::fetch::FetchTimeouts, + retry: BackoffPolicy, +) -> Result { + // Every coherence rule lives in the shared validator, so the reads below rest on an + // invariant that was actually checked: no empty values, exactly one target, and an Access + // pair that is either whole or absent. + let client = match validate_r2_flags(&r2_flags(args))? { + R2Target::None => { + return Err(eyre::eyre!( + "--witness-source r2 needs an R2 target: configure --r2-custom-domain, or \ + --r2-endpoint with its credential quad" + )); + } + R2Target::CustomDomain { connections } => { + let domain = args.r2_custom_domain.as_deref().expect("custom-domain target"); + let access = + args.r2_access_client_id.as_ref().zip(args.r2_access_client_secret.as_ref()).map( + |(client_id, client_secret)| stateless_r2::fetch::CfAccessCredentials { + client_id: client_id.as_ref().to_string(), + client_secret: client_secret.as_ref().to_string(), + }, + ); + let cf_access = access.is_some(); + let client = R2WitnessClient::new_custom_domain( + domain, + access, + timeouts, + retry, + args.witness_max_concurrent_requests, + connections, + )?; + metrics::record_r2_connections(client.connections()); + info!( + domain = %client.origin(), + cf_access, + connections = client.connections(), + "Witness source: R2 (custom domain)" + ); + client + } + R2Target::S3 => { + let take = |v: &Option| v.clone().expect("S3 target"); + let client = R2WitnessClient::new( + args.r2_endpoint.as_deref().expect("S3 target"), + take(&args.r2_bucket), + take(&args.r2_access_key_id), + args.r2_secret_access_key.as_ref().expect("S3 target").as_ref().to_string(), + timeouts, + retry, + args.witness_max_concurrent_requests, + )?; + info!( + endpoint = %client.origin(), + bucket = args.r2_bucket.as_deref().unwrap_or_default(), + "Witness source: R2 (direct S3)" + ); + client + } + }; + metrics::record_r2_target(client.target_label()); + Ok(client) } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn require_r2_rejects_absent_and_empty_values() { - assert!(require_r2(&None::, "--r2-endpoint").is_err()); - // An env var set to the empty string must not pass as configured. - assert!(require_r2(&Some(String::new()), "--r2-endpoint").is_err()); - assert_eq!( - require_r2(&Some("https://x".to_string()), "--r2-endpoint").unwrap(), - "https://x" - ); +/// This binary's `--r2-*` flags, in the spellings its operators use. +fn r2_flags(args: &CommandLineArgs) -> R2Flags<'_> { + R2Flags { + endpoint: R2Flag::new("--r2-endpoint", args.r2_endpoint.as_deref()), + bucket: R2Flag::new("--r2-bucket", args.r2_bucket.as_deref()), + access_key_id: R2Flag::new("--r2-access-key-id", args.r2_access_key_id.as_deref()), + secret_access_key: R2Flag::new( + "--r2-secret-access-key", + args.r2_secret_access_key.as_ref().map(AsRef::as_ref), + ), + custom_domain: R2Flag::new("--r2-custom-domain", args.r2_custom_domain.as_deref()), + access_client_id: R2Flag::new( + "--r2-access-client-id", + args.r2_access_client_id.as_ref().map(AsRef::as_ref), + ), + access_client_secret: R2Flag::new( + "--r2-access-client-secret", + args.r2_access_client_secret.as_ref().map(AsRef::as_ref), + ), + connections: R2Flag::new("--r2-connections", args.r2_connections.as_deref()), + max_concurrent_requests: R2CountFlag::new( + "--witness-max-concurrent-requests", + args.witness_max_concurrent_requests, + ), + // Empty on purpose. The orphan-tuning rule exists for a binary that validates R2 flags + // on every startup; here they are only read under `--witness-source r2`, where a target + // is mandatory, so the rule could never fire. Under `--witness-source rpc` every + // `--r2-*` flag is inert by design — see the call site in `run`. + tuning: &[], } } diff --git a/bin/stateless-validator/src/metrics.rs b/bin/stateless-validator/src/metrics.rs index d3b141bb..1b344c90 100644 --- a/bin/stateless-validator/src/metrics.rs +++ b/bin/stateless-validator/src/metrics.rs @@ -92,6 +92,9 @@ pub mod names { metric!(WITNESS_FETCH_R2_TIME, "witness_fetch_r2_time_seconds"); metric!(R2_WITNESS_RETRY_ATTEMPTS_TOTAL, "r2_witness_retry_attempts_total"); metric!(R2_WITNESS_ERRORS_TOTAL, "r2_witness_errors_total"); + metric!(R2_TARGET_INFO, "r2_target_info"); + metric!(R2_NEGOTIATED_VERSION_INFO, "r2_negotiated_http_version_info"); + metric!(R2_CONNECTIONS, "r2_connections"); // Contract cache metric!(CONTRACT_CACHE_HITS, "contract_cache_hits_total"); @@ -195,6 +198,16 @@ fn register_metric_descriptions() { names::R2_WITNESS_ERRORS_TOTAL, "R2 witness fetches that surfaced an error to the pipeline, by kind" ); + describe_gauge!( + names::R2_NEGOTIATED_VERSION_INFO, + "Protocol the R2 custom-domain target negotiated, as a constant-1 gauge labeled \ + `version` (h2 is the point of that target; http/1.1 means it silently degraded)" + ); + describe_gauge!( + names::R2_TARGET_INFO, + "Configured R2 target, as a constant-1 gauge labeled `target` (join to give the \ + target-less R2 series a target dimension during a rollout)" + ); // Contract cache describe_counter!(names::CONTRACT_CACHE_HITS, "Contract cache hits"); @@ -237,6 +250,37 @@ fn init_r2_witness_counters() { } } +/// Publishes the configured R2 target once at startup. +/// +/// The R2 series carry no target dimension, so during a fleet rollout — some hosts on the +/// custom domain, some still on the S3 endpoint — a spike in `r2_witness_errors_total` cannot +/// be attributed to either. Joining on this gauge supplies that dimension without changing the +/// established metric contract. +pub fn record_r2_target(target: &'static str) { + gauge!(names::R2_TARGET_INFO, "target" => target).set(1.0); +} + +/// The protocol the R2 custom-domain target actually negotiated, as a constant-1 info gauge +/// labeled `version`, published once the first response has been seen. +/// +/// Separate from the target gauge on purpose: that one answers "what was configured" and can be +/// published at startup, while this one is only knowable after a request. Folding both into one +/// gauge would mean publishing it twice with different label sets, leaving the startup series +/// stuck at 1 forever alongside the corrected one. +pub fn record_r2_negotiated_version(version: &'static str) { + gauge!(names::R2_NEGOTIATED_VERSION_INFO, "version" => version).set(1.0); +} + +/// How many HTTP/2 connections the custom-domain target spreads its GETs over. +/// +/// A plain value rather than an info label: it is the divisor for the per-connection stream +/// budget, so a dashboard reads it against `--witness-max-concurrent-requests` and against the +/// edge's limit rather than grouping by it. Published only for the custom-domain target, where +/// one client is one connection and the count is a real property of the transport. +pub fn record_r2_connections(connections: usize) { + gauge!(names::R2_CONNECTIONS).set(connections as f64); +} + /// Record validation timing and block statistics after successful validation. #[allow(clippy::too_many_arguments)] pub fn on_validation_success( diff --git a/bin/stateless-validator/src/r2_witness.rs b/bin/stateless-validator/src/r2_witness.rs index e7fb18a4..f0aacb65 100644 --- a/bin/stateless-validator/src/r2_witness.rs +++ b/bin/stateless-validator/src/r2_witness.rs @@ -1,6 +1,7 @@ //! Direct-from-R2 witness source. //! -//! Fetches the primary witness object straight from the R2 bucket over the S3 API and returns +//! Fetches the primary witness object straight from the R2 bucket — via SigV4-signed S3 GETs +//! or unsigned GETs through a Cloudflare custom domain, per construction — and returns //! the same `(SaltWitness, MptWitness)` tuple the RPC path yields. The transport core is //! [`R2ObjectFetcher`] from `stateless-r2`, shared with the debug-trace-server's historical //! witness source; this adapter owns what is validator-specific: the **full** payload decode @@ -15,7 +16,8 @@ //! `--end-block` slice over history, a permanently absent object means the run never //! completes and never fails: alert on `r2_witness_errors_total{kind="missing"}` staying hot //! for the same block, and use the object key from the error's log line to check/backfill -//! the bucket. +//! the bucket. On the custom-domain target, "appears once the uploader wins" additionally +//! assumes the edge does not cache 404s — see the `--r2-custom-domain` flag docs. use std::time::{Duration, Instant}; @@ -26,7 +28,7 @@ use stateless_common::{ }; use stateless_core::withdrawals::MptWitness; use stateless_r2::{ - fetch::{FetchTimeouts, R2GetError, R2ObjectFetcher, RetryPacing}, + fetch::{CfAccessCredentials, FetchTimeouts, R2GetError, R2ObjectFetcher, RetryPacing}, keys, }; use tokio::task::JoinError; @@ -36,7 +38,7 @@ use crate::metrics; /// Throttle applied before surfacing any deterministic (non-retryable) failure: the pipeline /// fetcher (`stateless-core/src/pipeline/fetcher.rs`) re-enqueues failed fetches with no delay, -/// so returning instantly would hot-loop signed GETs against R2. Delete this once the fetcher +/// so returning instantly would hot-loop GETs against R2. Delete this once the fetcher /// grows per-block re-enqueue backoff. Test builds shrink it so the failure-path tests run in /// milliseconds. const DETERMINISTIC_FAILURE_THROTTLE: Duration = @@ -50,7 +52,7 @@ const MAX_ATTEMPTS: usize = 9; /// Failure outcome of an R2 witness fetch. #[derive(Debug, thiserror::Error)] pub enum R2WitnessError { - /// The signed GET failed (absent object, transport, throttle, or unexpected status — + /// The GET failed (absent object, transport, throttle, or unexpected status — /// see [`R2GetError`], and the module docs for the `Missing` operator note). #[error(transparent)] Get(#[from] R2GetError), @@ -95,14 +97,37 @@ impl R2WitnessError { } } -/// Fetches witness objects straight from an R2 bucket over the S3 API with SigV4-signed GETs. +/// Fetches witness objects straight from an R2 bucket — SigV4-signed over the S3 API, or +/// unsigned through a Cloudflare custom domain, per construction. /// The fetcher's `Debug` redacts the credentials. #[derive(Debug)] pub struct R2WitnessClient { fetcher: R2ObjectFetcher, } +/// The fetcher's pacing view of a `BackoffPolicy` — the adapter-layer conversion that keeps +/// `stateless-r2` free of a dependency on this workspace's backoff type. +fn pacing(backoff: &BackoffPolicy) -> RetryPacing { + RetryPacing { initial: backoff.initial, max: backoff.max } +} + impl R2WitnessClient { + /// The configured target's origin, for startup logging (see [`R2ObjectFetcher::origin`]). + pub fn origin(&self) -> &str { + self.fetcher.origin() + } + + /// The configured target's metric label (see [`R2ObjectFetcher::target_label`]). + pub const fn target_label(&self) -> &'static str { + self.fetcher.target_label() + } + + /// How many HTTP/2 connections the transport spreads its GETs over, for startup logging + /// (see [`R2ObjectFetcher::connections`]). + pub fn connections(&self) -> usize { + self.fetcher.connections() + } + /// Builds a client from an R2 endpoint origin, bucket, and bucket-scoped S3 credentials. /// /// `timeouts` bounds each individual GET (end-to-end and connect). `retry_backoff` paces the @@ -129,13 +154,37 @@ impl R2WitnessClient { access_key_id, secret_access_key, timeouts, - RetryPacing { initial: retry_backoff.initial, max: retry_backoff.max }, + pacing(&retry_backoff), max_concurrent_requests, ) .map_err(|e| eyre::eyre!(e))?; Ok(Self { fetcher }) } + /// Builds a client that fetches unsigned through a Cloudflare custom domain fronting the + /// bucket (h2-multiplexed, edge-cacheable), with optional Cloudflare Access service-token + /// headers. The remaining parameters mean what they mean on [`Self::new`]. + pub fn new_custom_domain( + domain: &str, + access: Option, + timeouts: FetchTimeouts, + retry_backoff: BackoffPolicy, + max_concurrent_requests: Option, + connections: usize, + ) -> eyre::Result { + let fetcher = R2ObjectFetcher::new_custom_domain( + domain, + access, + timeouts, + pacing(&retry_backoff), + max_concurrent_requests, + connections, + ) + .map(|fetcher| fetcher.on_version_observed(metrics::record_r2_negotiated_version)) + .map_err(|e| eyre::eyre!(e))?; + Ok(Self { fetcher }) + } + /// Fetches and decodes the witness for `(number, hash)` from R2. /// /// Transport/429/5xx failures are retried internally, paced by the `retry_backoff` policy @@ -177,7 +226,7 @@ impl R2WitnessClient { .fetcher .get_block_object(number, hash, MAX_ATTEMPTS, None, metrics::on_r2_witness_retry) .await?; - let bytes = fetched.bytes; + let (bytes, queue_wait) = (fetched.bytes, fetched.queue_wait); // zstd + bincode over a multi-MB witness is CPU-bound; keep it off the runtime. let key = || keys::block_object_key(number, hash); @@ -187,7 +236,7 @@ impl R2WitnessClient { // Queue wait on the self-imposed concurrency cap is subtracted: folded in, it // would masquerade as R2 slowness. metrics::on_r2_witness_fetch_success( - started.elapsed().saturating_sub(fetched.queue_wait).as_secs_f64(), + started.elapsed().saturating_sub(queue_wait).as_secs_f64(), WitnessSizeBreakdown::new(&witness.0, &witness.1), ); Ok(witness) @@ -291,6 +340,50 @@ mod tests { assert_eq!(hits.load(Ordering::SeqCst), 1, "a successful fetch must take exactly one GET"); } + /// The custom-domain client serves the same full-decode path end-to-end, requesting the + /// bare `/{key}` layout (no bucket segment, no SigV4 authorization). + #[tokio::test] + async fn custom_domain_client_decodes_and_requests_bare_key() { + let (salt_witness, mpt_witness): (_, MptWitness) = + TestFixtures::mainnet_shared().first_paired_witness(); + let (_, payload) = stateless_common::encode_witness_payload(&salt_witness, &mpt_witness) + .expect("fixture witness must encode"); + + let (domain, _, heads) = + stateless_test_utils::mock_r2::mock_r2_capturing(vec![(200, payload)]).await; + let client = R2WitnessClient::new_custom_domain( + &domain, + None, + test_timeouts(), + test_backoff(), + None, + 1, + ) + .unwrap(); + let (decoded_salt, _) = + client.get_witness(1, B256::ZERO).await.expect("valid object must fetch and decode"); + assert_eq!(decoded_salt, salt_witness); + let head = heads.lock().unwrap()[0].to_lowercase(); + assert!(head.starts_with("get /block/0_999/1."), "bucketless key layout: {head}"); + assert!(!head.contains("authorization:"), "custom-domain GET must be unsigned: {head}"); + } + + /// Construction errors from the shared fetcher's custom-domain arm surface through the + /// same eyre conversion as the S3 arm. + #[test] + fn custom_domain_rejects_origin_with_path() { + let err = R2WitnessClient::new_custom_domain( + "https://witness.example.com/witness-mainnet", + None, + test_timeouts(), + test_backoff(), + None, + 1, + ) + .unwrap_err(); + assert!(err.to_string().contains("Invalid R2 custom domain")); + } + #[tokio::test] async fn undecodable_body_surfaces_decode_without_retry() { let (endpoint, hits) = mock_r2(vec![(200, "not a zstd witness")]).await; diff --git a/bin/stateless-validator/tests/integration.rs b/bin/stateless-validator/tests/integration.rs index 2d0a9b86..94f1b12e 100644 --- a/bin/stateless-validator/tests/integration.rs +++ b/bin/stateless-validator/tests/integration.rs @@ -182,6 +182,92 @@ fn witness_endpoint_is_optional_at_parse_time() { assert!(parse(&["--witness-source", "r2"]).unwrap().witness_endpoint.is_empty()); } +/// The custom-domain R2 target is mutually exclusive with the S3 endpoint, and the Access +/// token pair is all-or-nothing on top of it. +#[test] +fn r2_custom_domain_target_wiring() { + let _guard = stateless_test_utils::env::env_lock(); + let parse = |extra: &[&str]| CommandLineArgs::try_parse_from(BASE_ARGS.iter().chain(extra)); + + assert_eq!( + parse(&["--r2-custom-domain", "https://witness.example.com"]) + .unwrap() + .r2_custom_domain + .as_deref(), + Some("https://witness.example.com") + ); + // Every R2 coherence rule is enforced after parsing, by `stateless_common::validate_r2_flags`, + // so that each error can name the flag — clap's own rejections cannot, this workspace having + // built it without `error-context`. Parsing therefore accepts all of these shapes; the rules + // and their messages are covered by that function's own tests. + const DOMAIN: &str = "https://witness.example.com"; + for shape in [ + &["--r2-custom-domain", DOMAIN, "--r2-endpoint", "https://acc.r2.cloudflarestorage.com"][..], + &["--r2-custom-domain", DOMAIN, "--r2-access-client-id", "tok"], + &["--r2-access-client-id", "tok", "--r2-access-client-secret", "sk"], + &[ + "--r2-custom-domain", + DOMAIN, + "--r2-access-client-id", + "tok", + "--r2-access-client-secret", + "sk", + ], + &["--r2-custom-domain", DOMAIN, "--r2-connections", "0"], + ] { + assert!(parse(shape).is_ok(), "{shape:?} must parse; rejection happens post-parse"); + } + assert_eq!( + parse(&["--r2-custom-domain", DOMAIN, "--r2-connections", "8"]) + .unwrap() + .r2_connections + .as_deref(), + Some("8") + ); +} + +/// Under the default `--witness-source rpc` the `--r2-*` flags are inert, and a blank value — +/// what a templated env file renders for a variable a given role does not set — must stay +/// inert too. +/// +/// This pins the parse layer specifically. `--r2-connections` is text rather than a number for +/// exactly this reason: parsed by clap, a blank line aborts startup before `run` can decide the +/// flags are irrelevant, and it aborts with clap's unnamed value error because this workspace +/// builds clap without `error-context`. The gating of the rules themselves lives in `run` — +/// `validate_r2_flags` is reached only through `build_r2_client`, from the `WitnessSource::R2` +/// arm — which this test cannot observe. +#[test] +fn rpc_mode_tolerates_blank_and_conflicting_r2_values() { + let _guard = stateless_test_utils::env::env_lock(); + let parse = |extra: &[&str]| CommandLineArgs::try_parse_from(BASE_ARGS.iter().chain(extra)); + + assert!(parse(&["--witness-source", "rpc"]).is_ok()); + for blank in [ + ["--r2-bucket", ""], + ["--r2-custom-domain", ""], + ["--r2-connections", ""], + ["--r2-access-client-id", ""], + ] { + assert!( + parse(&blank).is_ok(), + "a blank {} must parse so it can stay inert in rpc mode", + blank[0] + ); + } + assert!( + parse(&[ + "--witness-source", + "rpc", + "--r2-endpoint", + "https://acc.r2.cloudflarestorage.com", + "--r2-custom-domain", + "https://witness.example.com", + ]) + .is_ok(), + "conflicting targets must parse in rpc mode; they are never read there" + ); +} + /// `canonical_chain_max_length` must reject 0 at parse time. A value of 0 would make /// `advance_chain` prune the entire canonical chain on every successful advance, /// rolling the pipeline back to the anchor each round and looping forever. diff --git a/crates/stateless-common/src/lib.rs b/crates/stateless-common/src/lib.rs index ea7a3827..5df7ef92 100644 --- a/crates/stateless-common/src/lib.rs +++ b/crates/stateless-common/src/lib.rs @@ -13,6 +13,8 @@ pub use witness_encoding::{ decode_witness_response, decode_witness_response_light, encode_witness_payload, encode_witness_response, }; +pub mod r2_args; +pub use r2_args::{R2CountFlag, R2Flag, R2Flags, R2Target, R2TuningFlag, validate_r2_flags}; pub mod secret; pub use secret::RedactedSecret; pub mod witness_size; diff --git a/crates/stateless-common/src/r2_args.rs b/crates/stateless-common/src/r2_args.rs new file mode 100644 index 00000000..bfa35f0c --- /dev/null +++ b/crates/stateless-common/src/r2_args.rs @@ -0,0 +1,502 @@ +//! Validation of the `--r2-*` argument set, shared by the binaries' argument structs. +//! +//! These rules decide which R2 target the witness fetcher is built with, and both binaries +//! enforce the same ones. They live here rather than in each binary because, written twice, +//! they diverged three ways inside a single change: one side named every missing member of an +//! incomplete credential quad while the other reported them one at a time, one side swept +//! empty values twice, and the shared tuning flags were gated on one binary and silently +//! ignored on the other. +//! +//! Enforced after parsing rather than through clap attributes because this workspace builds +//! clap without its `error-context` feature, so a clap rejection names no argument — useless +//! to an operator whose configuration is an env file. Every error raised here names the flag, +//! in the spelling the calling binary uses for it. + +use eyre::{Result, bail}; + +/// One `--r2-*` flag carrying a value: the spelling this binary gives it, and what it parsed. +#[derive(Clone, Copy)] +pub struct R2Flag<'a> { + /// The flag as an operator writes it, e.g. `--r2-endpoint`. + pub name: &'a str, + /// The parsed value. `Some("")` is the failed-env-injection shape, not a value. + pub value: Option<&'a str>, +} + +impl<'a> R2Flag<'a> { + /// Names a flag and its parsed value. + pub const fn new(name: &'a str, value: Option<&'a str>) -> Self { + Self { name, value } + } + + const fn is_set(&self) -> bool { + self.value.is_some() + } +} + +/// A flag whose value type belongs to the binary, so only its presence reaches these rules. +#[derive(Clone, Copy)] +pub struct R2TuningFlag<'a> { + /// The flag as an operator writes it, e.g. `--r2-connect-timeout-ms`. + pub name: &'a str, + /// Whether the operator set it explicitly (a defaulted value is not "set"). + pub set: bool, +} + +impl<'a> R2TuningFlag<'a> { + /// Names a tuning flag and whether it was explicitly set. + pub const fn new(name: &'a str, set: bool) -> Self { + Self { name, set } + } +} + +/// A flag whose numeric *value* the rules need, not just its presence. +#[derive(Clone, Copy)] +pub struct R2CountFlag<'a> { + /// The flag as an operator writes it, e.g. `--r2-max-concurrent-requests`. + pub name: &'a str, + /// The parsed count, `None` when the operator left it alone. + pub value: Option, +} + +impl<'a> R2CountFlag<'a> { + /// Names a count flag and its parsed value. + pub const fn new(name: &'a str, value: Option) -> Self { + Self { name, value } + } +} + +/// One binary's `--r2-*` flags, as parsed. +pub struct R2Flags<'a> { + /// Bare S3 endpoint origin; selects the signed target. + pub endpoint: R2Flag<'a>, + /// Bucket for the signed target. + pub bucket: R2Flag<'a>, + /// Access key id for the signed target. + pub access_key_id: R2Flag<'a>, + /// Secret access key for the signed target. + pub secret_access_key: R2Flag<'a>, + /// Cloudflare custom domain; selects the unsigned target. + pub custom_domain: R2Flag<'a>, + /// Cloudflare Access service-token client id (custom domain only). + pub access_client_id: R2Flag<'a>, + /// Cloudflare Access service-token client secret (custom domain only). + pub access_client_secret: R2Flag<'a>, + /// How many HTTP/2 connections the custom-domain target spreads its GETs over, unparsed + /// (see [`parse_r2_connections`] for why it arrives as a string). + pub connections: R2Flag<'a>, + /// The in-flight GET cap these connections divide, whatever the binary calls it. + pub max_concurrent_requests: R2CountFlag<'a>, + /// Flags that only mean something once a target is configured. + pub tuning: &'a [R2TuningFlag<'a>], +} + +/// Which target a validated flag set selects. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum R2Target { + /// No R2 flags configured; the caller decides whether that is fatal for its mode. + None, + /// SigV4-signed GETs against the bare S3 endpoint. + S3, + /// Unsigned GETs through a Cloudflare custom domain, spread over this many HTTP/2 + /// connections. Carried on the verdict rather than left for the caller to parse again: + /// the count is validated here, and a caller that re-derived it would be a second place + /// the same rule lives. + CustomDomain { connections: usize }, +} + +/// Validates one binary's `--r2-*` flags and reports which target they select. +/// +/// Rejects, each by flag name: a present-but-empty value, both targets at once, S3 flags left +/// behind by the documented migration to a custom domain, an incomplete S3 credential quad, a +/// half-configured Cloudflare Access pair or one attached to no custom domain, a connection +/// count that is zero, unparsable, attached to a target that cannot spread over connections, +/// or larger than the cap it divides, and tuning flags set with no target to tune. +/// +/// Emptiness is swept first so a blank env line is diagnosed as itself, rather than read as a +/// configured target or as a leftover from one — without that ordering, a blank +/// `..._R2_CUSTOM_DOMAIN=` beside a working S3 configuration gets told to unset the S3 +/// configuration. +pub fn validate_r2_flags(flags: &R2Flags<'_>) -> Result { + let s3_credentials = [flags.bucket, flags.access_key_id, flags.secret_access_key]; + let all_values = [ + flags.endpoint, + flags.bucket, + flags.access_key_id, + flags.secret_access_key, + flags.custom_domain, + flags.access_client_id, + flags.access_client_secret, + flags.connections, + ]; + + for flag in all_values { + if flag.value.is_some_and(str::is_empty) { + bail!( + "{} is set but empty (empty env var injection?): unset it or give it a value", + flag.name + ); + } + } + + // Past the sweep, presence is the only predicate: anything still set was meant. + let target = match (flags.custom_domain.is_set(), flags.endpoint.is_set()) { + (true, true) => bail!( + "{} and {} are mutually exclusive R2 targets: configure exactly one", + flags.endpoint.name, + flags.custom_domain.name + ), + (true, false) => { + // The domain replaces the S3 target, so anything left of it is dead configuration; + // reading past it in silence would hide which credentials are actually in use. + let leftovers = names_of(&s3_credentials, R2Flag::is_set); + if !leftovers.is_empty() { + bail!( + "{} replaces the S3 target: unset the leftover {} (ignoring them silently \ + would hide which credentials are actually in use)", + flags.custom_domain.name, + leftovers.join(", ") + ); + } + R2Target::CustomDomain { connections: 1 } + } + (false, true) => { + let missing = names_of(&s3_credentials, |f| !f.is_set()); + if !missing.is_empty() { + bail!( + "{} needs the whole S3 credential set; missing: {}", + flags.endpoint.name, + missing.join(", ") + ); + } + R2Target::S3 + } + (false, false) => { + // A partial quad with no endpoint builds nothing, so say what is missing rather + // than starting with the R2 route quietly disabled. + let present = names_of(&s3_credentials, R2Flag::is_set); + if !present.is_empty() { + bail!("{} is required alongside {}", flags.endpoint.name, present.join(", ")); + } + R2Target::None + } + }; + + validate_access_pair(flags, target)?; + let connections = validate_connections(flags, target)?; + + // The connection count joins the tuning flags here rather than being reported on its own, + // so an operator who orphaned several of them is told about all of them at once. + let orphan_tuning: Vec<&str> = flags + .tuning + .iter() + .filter(|flag| flag.set) + .map(|flag| flag.name) + .chain(flags.connections.value.map(|_| flags.connections.name)) + .collect(); + if target == R2Target::None && !orphan_tuning.is_empty() { + bail!( + "{} only applies once an R2 target is configured: set one, or unset the flag", + orphan_tuning.join(", ") + ); + } + + Ok(match target { + R2Target::CustomDomain { .. } => R2Target::CustomDomain { connections }, + settled => settled, + }) +} + +/// Parses the connection count, defaulting to a single connection when it is not set. +/// +/// It travels as a string rather than as a `usize` in the argument struct so that a blank line +/// — what a templated env file renders for an unset variable — is diagnosed here, by name, at +/// the point the R2 flags are actually read. Parsed by clap it would abort startup with clap's +/// unnamed "invalid value for one of the arguments" (this workspace builds clap without +/// `error-context`), and it would abort it even on a binary that never reads the R2 flags in +/// the mode it was started in. +fn parse_r2_connections(flag: R2Flag<'_>) -> Result { + let Some(raw) = flag.value else { return Ok(1) }; + let Ok(count) = raw.parse::() else { + bail!("{} must be a whole number of connections, got {raw:?}", flag.name); + }; + if count == 0 { + bail!("{} must be at least 1", flag.name); + } + Ok(count) +} + +/// The connection count is a custom-domain concept, must name at least one connection, and +/// cannot name more connections than the in-flight cap can fill. +/// +/// Rejected rather than clamped on the S3 target: there, one client already opens a socket per +/// concurrent request, so a count set there is a belief about the deployment that is not true, +/// and honouring it silently would leave the operator expecting a spread they did not get. +/// Rejected rather than clamped against the cap for the same reason — and because clamping +/// would quietly hand back fewer connections than the published gauge reports. +fn validate_connections(flags: &R2Flags<'_>, target: R2Target) -> Result { + let count = parse_r2_connections(flags.connections)?; + if flags.connections.value.is_none() { + return Ok(count); + } + if target == R2Target::S3 { + bail!( + "{} applies only to {}: the S3 target already opens a connection per in-flight GET", + flags.connections.name, + flags.custom_domain.name + ); + } + // The cap is split across the connections, so more connections than permits leaves some of + // them permanently idle — and the split rounds up, which past this point would be the one + // way the fetcher-wide total could exceed the cap by more than a rounding residue. + if let Some(max) = flags.max_concurrent_requests.value && + count > max + { + bail!( + "{} ({count}) exceeds {} ({max}): the cap is split across the connections, so more \ + connections than permits leaves some of them idle", + flags.connections.name, + flags.max_concurrent_requests.name + ); + } + Ok(count) +} + +/// The Cloudflare Access pair is all-or-nothing and belongs to the custom-domain target only. +/// +/// Checked here rather than by clap's `requires_all` so the error names the missing half. It +/// also protects the callers' `Option::zip`: one half alone would zip to `None`, which is a +/// legitimate configuration (an IP-allowlisted domain), so a half-set pair would otherwise +/// build a working but silently *unauthenticated* client — and on the custom-domain target the +/// edge answers unauthenticated GETs with a non-retryable 403. +fn validate_access_pair(flags: &R2Flags<'_>, target: R2Target) -> Result<()> { + let (id, secret) = (flags.access_client_id, flags.access_client_secret); + match (id.is_set(), secret.is_set()) { + (false, false) => return Ok(()), + (true, false) => { + bail!("{} is set without {}: configure both or neither", id.name, secret.name) + } + (false, true) => { + bail!("{} is set without {}: configure both or neither", secret.name, id.name) + } + (true, true) => {} + } + if !matches!(target, R2Target::CustomDomain { .. }) { + bail!( + "{} and {} apply only to {}: configure that target, or unset the pair", + id.name, + secret.name, + flags.custom_domain.name + ); + } + Ok(()) +} + +fn names_of<'a>(flags: &[R2Flag<'a>], keep: impl Fn(&R2Flag<'a>) -> bool) -> Vec<&'a str> { + flags.iter().filter(|f| keep(f)).map(|f| f.name).collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The flags one test cares about; everything it does not name is absent. + #[derive(Default)] + struct Cfg<'a> { + endpoint: Option<&'a str>, + bucket: Option<&'a str>, + key_id: Option<&'a str>, + secret: Option<&'a str>, + domain: Option<&'a str>, + access_id: Option<&'a str>, + access_secret: Option<&'a str>, + connections: Option<&'a str>, + max_concurrent: Option, + tuning: &'a [R2TuningFlag<'a>], + } + + impl<'a> Cfg<'a> { + /// The validator's flag spellings, so the messages read as an operator sees them. + fn flags(&'a self) -> R2Flags<'a> { + R2Flags { + endpoint: R2Flag::new("--r2-endpoint", self.endpoint), + bucket: R2Flag::new("--r2-bucket", self.bucket), + access_key_id: R2Flag::new("--r2-access-key-id", self.key_id), + secret_access_key: R2Flag::new("--r2-secret-access-key", self.secret), + custom_domain: R2Flag::new("--r2-custom-domain", self.domain), + access_client_id: R2Flag::new("--r2-access-client-id", self.access_id), + access_client_secret: R2Flag::new("--r2-access-client-secret", self.access_secret), + connections: R2Flag::new("--r2-connections", self.connections), + max_concurrent_requests: R2CountFlag::new( + "--r2-max-concurrent-requests", + self.max_concurrent, + ), + tuning: self.tuning, + } + } + + fn validate(&'a self) -> Result { + validate_r2_flags(&self.flags()) + } + + fn err(&'a self) -> String { + self.validate().unwrap_err().to_string() + } + } + + /// Every way a connection count can be wrong is named. Zero builds a fetcher that can + /// carry nothing; the S3 target cannot spread over connections at all; more connections + /// than permits leaves some of them idle. Named rather than clamped or ignored, because + /// either silence leaves the operator believing in a spread they did not get. + #[test] + fn connections_must_be_positive_belong_to_the_custom_domain_and_fit_the_cap() { + let zero = Cfg { domain: DOMAIN, connections: Some("0"), ..Cfg::default() }.err(); + assert!(zero.contains("--r2-connections") && zero.contains("at least 1"), "{zero}"); + + let junk = Cfg { domain: DOMAIN, connections: Some("eight"), ..Cfg::default() }.err(); + assert!(junk.contains("--r2-connections") && junk.contains("whole number"), "{junk}"); + + let on_s3 = Cfg { connections: Some("4"), ..s3() }.err(); + assert!( + on_s3.contains("--r2-connections") && on_s3.contains("--r2-custom-domain"), + "{on_s3}" + ); + + let orphan = Cfg { connections: Some("4"), ..Cfg::default() }.err(); + assert!(orphan.contains("--r2-connections"), "{orphan}"); + + let over_cap = Cfg { + domain: DOMAIN, + connections: Some("8"), + max_concurrent: Some(4), + ..Cfg::default() + } + .err(); + assert!( + over_cap.contains("--r2-connections") && + over_cap.contains("--r2-max-concurrent-requests"), + "more connections than permits must name both flags: {over_cap}" + ); + + assert!( + Cfg { + domain: DOMAIN, + connections: Some("8"), + max_concurrent: Some(48), + ..Cfg::default() + } + .validate() + .is_ok() + ); + } + + /// A blank line is what a templated env file renders for an unset variable, so it must be + /// diagnosed as itself rather than as a bad number — and, on a binary that reads the R2 + /// flags in only one mode, must not reach clap at all. + #[test] + fn a_blank_connection_count_is_named_as_an_empty_value() { + let blank = Cfg { domain: DOMAIN, connections: Some(""), ..Cfg::default() }.err(); + assert!(blank.contains("--r2-connections") && blank.contains("empty"), "{blank}"); + assert_eq!(parse_r2_connections(R2Flag::new("--r2-connections", None)).unwrap(), 1); + assert_eq!(parse_r2_connections(R2Flag::new("--r2-connections", Some("8"))).unwrap(), 8); + } + + /// A complete, valid S3 configuration. + fn s3<'a>() -> Cfg<'a> { + Cfg { + endpoint: Some("https://acc.r2.cloudflarestorage.com"), + bucket: Some("b"), + key_id: Some("k"), + secret: Some("s"), + ..Cfg::default() + } + } + + const DOMAIN: Option<&str> = Some("https://w.example.com"); + + #[test] + fn selects_the_configured_target() { + assert_eq!(s3().validate().unwrap(), R2Target::S3); + assert_eq!( + Cfg { domain: DOMAIN, ..Cfg::default() }.validate().unwrap(), + R2Target::CustomDomain { connections: 1 } + ); + assert_eq!(Cfg::default().validate().unwrap(), R2Target::None); + } + + /// Emptiness is diagnosed before target selection can read a blank line as a configured + /// target or as a leftover from one. + #[test] + fn empty_values_are_named_before_target_selection() { + let err = Cfg { domain: Some(""), ..s3() }.err(); + assert!(err.contains("--r2-custom-domain") && err.contains("set but empty"), "{err}"); + // Regression guard: this once reached the leftover branch and told the operator to + // unset their entire working S3 configuration. + assert!(!err.contains("--r2-endpoint"), "must not blame the working S3 config: {err}"); + } + + #[test] + fn both_targets_are_rejected_naming_both() { + let err = Cfg { domain: DOMAIN, ..s3() }.err(); + assert!(err.contains("--r2-endpoint") && err.contains("--r2-custom-domain"), "{err}"); + } + + #[test] + fn s3_flags_left_beside_a_custom_domain_are_named() { + let err = Cfg { domain: DOMAIN, bucket: Some("b"), ..Cfg::default() }.err(); + assert!(err.contains("--r2-bucket"), "{err}"); + assert!(!err.contains("--r2-access-key-id"), "only the flags actually set: {err}"); + } + + /// The S3 target is all-or-nothing, and every missing member is named in one message — the + /// trace server did this while the validator reported them one flag at a time. + #[test] + fn an_incomplete_s3_quad_names_every_missing_member() { + let err = Cfg { endpoint: s3().endpoint, ..Cfg::default() }.err(); + for missing in ["--r2-bucket", "--r2-access-key-id", "--r2-secret-access-key"] { + assert!(err.contains(missing), "{missing} must be named: {err}"); + } + + // Credentials with no endpoint select nothing, so say so rather than ignoring them. + let err = Cfg { bucket: Some("b"), ..Cfg::default() }.err(); + assert!(err.contains("--r2-endpoint") && err.contains("--r2-bucket"), "{err}"); + } + + /// A half-set Access pair is the shape that would otherwise `zip` to `None` and build a + /// working but unauthenticated client. + #[test] + fn a_half_set_access_pair_is_named() { + let err = Cfg { domain: DOMAIN, access_id: Some("tok"), ..Cfg::default() }.err(); + assert!(err.contains("--r2-access-client-id"), "{err}"); + assert!(err.contains("--r2-access-client-secret"), "{err}"); + + let err = Cfg { domain: DOMAIN, access_secret: Some("sec"), ..Cfg::default() }.err(); + assert!(err.contains("--r2-access-client-id"), "{err}"); + + // Both halves, but pointed at no custom domain. + let err = Cfg { access_id: Some("tok"), access_secret: Some("sec"), ..s3() }.err(); + assert!(err.contains("--r2-custom-domain"), "{err}"); + + let whole = Cfg { + domain: DOMAIN, + access_id: Some("tok"), + access_secret: Some("sec"), + ..Cfg::default() + }; + assert_eq!(whole.validate().unwrap(), R2Target::CustomDomain { connections: 1 }); + } + + /// A tuning flag with no target to tune is named rather than silently ignored — it was + /// gated by clap on the trace server and accepted-and-dropped on the validator. + #[test] + fn tuning_flags_need_a_target() { + let set: &[R2TuningFlag<'_>] = &[R2TuningFlag::new("--r2-connect-timeout-ms", true)]; + let err = Cfg { tuning: set, ..Cfg::default() }.err(); + assert!(err.contains("--r2-connect-timeout-ms"), "{err}"); + + // Fine with a target, and fine when left at its default. + assert!(Cfg { domain: DOMAIN, tuning: set, ..Cfg::default() }.validate().is_ok()); + let unset: &[R2TuningFlag<'_>] = &[R2TuningFlag::new("--r2-connect-timeout-ms", false)]; + assert!(Cfg { tuning: unset, ..Cfg::default() }.validate().is_ok()); + } +} diff --git a/crates/stateless-common/src/rpc_client.rs b/crates/stateless-common/src/rpc_client.rs index 3e7aa323..dadf0fbf 100644 --- a/crates/stateless-common/src/rpc_client.rs +++ b/crates/stateless-common/src/rpc_client.rs @@ -330,6 +330,16 @@ impl RpcClient { // the connect-phase bound applies uniformly to data, witness, and report endpoints. let http_client = reqwest::Client::builder() .connect_timeout(config.connect_timeout) + // Pinned to HTTP/1.1 on purpose. `stateless-r2` turns on reqwest's `http2` feature + // for the R2 custom-domain target and Cargo unifies features workspace-wide, so + // without this pin every JSON-RPC provider here would start offering h2 — and the + // endpoints accept it. That would put the multi-MB witness payloads on one shared + // connection per host under hyper's fixed 5MB connection / 2MB stream windows with + // adaptive sizing off, capping throughput at roughly window/RTT over the + // intercontinental links this client runs across. Moving the RPC path to h2 is a + // change worth measuring on its own, not a side effect of a feature added for a + // different client. + .http1_only() .build() .context("Failed to build HTTP client")?; diff --git a/crates/stateless-r2/Cargo.toml b/crates/stateless-r2/Cargo.toml index 8c457f81..efbb9823 100644 --- a/crates/stateless-r2/Cargo.toml +++ b/crates/stateless-r2/Cargo.toml @@ -19,10 +19,16 @@ percent-encoding.workspace = true # Pinned explicitly (not workspace-inherited): `put_object` exposes `&reqwest::Client`, so the # reqwest major is part of this crate's API and must match mega-reth's workspace (0.12). Bumping # it is a coordinated two-repo change that a workspace-wide bump must not ride over. -# No `http2` feature: R2's bare S3 endpoint negotiates HTTP/1.1 only (ALPN-verified live), -# so h2 buys these paths nothing today. Revisit alongside the R2 custom-domain work, which -# does ride an h2-capable stack. -reqwest = { version = "0.12", default-features = false, features = ["rustls-tls"] } +# `http2` enables ALPN h2 for endpoints that offer it. R2's bare S3 endpoint negotiates +# HTTP/1.1 only (by design — ALPN-verified live; the S3 fetcher additionally pins +# `http1_only`), but R2 **custom domains** ride the regular CDN stack and do speak h2, +# where many in-flight GETs multiplex over a few connections instead of holding one each. +# Note: Cargo feature unification propagates `http2` to every reqwest client in this workspace +# and in consuming ones (including mega-reth's on the coordinated bump) — those clients start +# *offering* h2 via ALPN, and an endpoint that accepts it gets one shared, non-adaptive h2 +# connection per host. Clients that should not move must say so: this crate's S3 fetcher and +# `stateless-common`'s shared JSON-RPC client both pin `http1_only`. +reqwest = { version = "0.12", default-features = false, features = ["http2", "rustls-tls"] } sha2.workspace = true tokio = { workspace = true, features = ["sync", "time"] } tracing = { workspace = true, features = ["std"] } diff --git a/crates/stateless-r2/src/fetch.rs b/crates/stateless-r2/src/fetch.rs index 30553c04..55494887 100644 --- a/crates/stateless-r2/src/fetch.rs +++ b/crates/stateless-r2/src/fetch.rs @@ -1,43 +1,61 @@ -//! Signed `GET` of witness objects with retry, backoff, and concurrency capping. +//! `GET` of witness objects with retry, backoff, and concurrency capping. //! //! [`R2ObjectFetcher`] is the transport core shared by every R2 witness *reader* — the //! validator's pipeline source and the debug-trace-server's historical witness source. -//! It owns exactly the parts whose behavior must not drift between readers: the SigV4-signed -//! `GET`, the response classification ([`R2GetError`]), the retry loop with jittered +//! It owns exactly the parts whose behavior must not drift between readers: the `GET` +//! itself, the response classification ([`R2GetError`]), the retry loop with jittered //! exponential backoff, and the in-flight concurrency cap. Everything reader-specific stays //! with the caller: payload decoding (full vs light), metrics, and failure pacing policies. //! +//! The fetcher reaches the bucket through one of two targets: +//! - the bare **S3 API endpoint** ([`R2ObjectFetcher::new`]) — SigV4-signed GETs of +//! `/{bucket}/{key}`; the endpoint negotiates HTTP/1.1 only, so every in-flight GET holds its own +//! connection; +//! - a **Cloudflare custom domain** fronting the bucket ([`R2ObjectFetcher::new_custom_domain`]) — +//! unsigned GETs of `/{key}` through the CDN edge, which negotiates h2 (many in-flight GETs +//! multiplex over a few connections) and can serve the immutable witness objects from edge cache. +//! Access control is the domain's business: an IP allowlist needs nothing from this client, and +//! Cloudflare Access service tokens ride along as headers via [`CfAccessCredentials`]. +//! //! The loop is optionally deadline-aware (see [`R2ObjectFetcher::get_block_object`]). use std::{ fmt::Display, - sync::Arc, + sync::{ + Arc, OnceLock, + atomic::{AtomicUsize, Ordering}, + }, time::{Duration, Instant}, }; use bytes::Bytes; use chrono::Utc; -use reqwest::Client; -use tokio::sync::Semaphore; +use reqwest::{ + Client, + header::{HeaderMap, HeaderName, HeaderValue}, +}; +use tokio::sync::{Semaphore, SemaphorePermit}; use tracing::warn; use crate::{ client::is_throttle_status, endpoint::parse_endpoint, keys, - sigv4::{SigV4Signer, encode_uri_path}, + sigv4::{SigV4Signer, encode_key_path, encode_uri_path}, }; /// Cap on the response body carried inside `Throttled`/`Status` errors. const MAX_ERROR_BODY_BYTES: usize = 1024; /// Default bound on connection establishment (DNS + TCP + TLS). A healthy handshake to the -/// local anycast edge is ~10-50ms; Cloudflare's per-IP connection mitigation manifests as -/// handshakes that hang without erroring, so anything past this is that signature (the one -/// legitimate slow case — a lost SYN retried at the kernel's 1s RTO — is cheaper to abort -/// and retry on a fresh attempt than to wait out). Keeps a mitigated endpoint from eating -/// the caller's whole budget before its fallback gets a turn. Operators tune it via the -/// binaries' `--r2-connect-timeout-ms`. +/// local anycast edge is ~10-50ms. On the S3 target — where HTTP/1.1 means one connection +/// per in-flight GET — Cloudflare's per-IP connection mitigation manifests as handshakes +/// that hang without erroring, so anything past this is that signature (the one legitimate +/// slow case — a lost SYN retried at the kernel's 1s RTO — is cheaper to abort and retry on +/// a fresh attempt than to wait out); on the custom-domain target, which holds only a few +/// multiplexed connections, a slow handshake is ordinary DNS/TCP/TLS trouble. Either way +/// the bound keeps a wedged endpoint from eating the caller's whole budget before its +/// fallback gets a turn. Operators tune it via the binaries' `--r2-connect-timeout-ms`. pub const DEFAULT_CONNECT_TIMEOUT: Duration = Duration::from_secs(1); /// The two HTTP-level bounds a fetcher applies to every GET attempt, threaded together so @@ -51,7 +69,8 @@ pub struct FetchTimeouts { pub connect: Duration, } -/// Failure outcome of a signed witness-object `GET`, after the fetcher's own retries. +/// Failure outcome of a witness-object `GET`, after the fetcher's own retries. Shared by both +/// targets: the classification happens on the response, below the signed/unsigned split. /// /// Decode failures are deliberately absent: the fetcher stops at bytes, and each reader /// classifies its own decode errors. @@ -72,9 +91,10 @@ pub enum R2GetError { /// [`R2ObjectFetcher::new`]). Bodies are best-effort and capped. Status { number: u64, key: String, status: u16, body: String }, /// Connection establishment failed or exceeded the connect timeout — distinct from - /// [`Self::Transport`] because a hung handshake to the local anycast edge is the - /// signature of the per-IP connection-budget mitigation, and operators alert on this - /// kind to detect it. + /// [`Self::Transport`] because on the S3 target a hung handshake to the local anycast + /// edge is the signature of the per-IP connection-budget mitigation, and operators + /// alert on this kind to detect it. On the custom-domain target (a few multiplexed h2 + /// connections) this kind is ordinary DNS/TCP/TLS/edge trouble. Connect { number: u64, key: String, source: reqwest::Error }, /// The caller's deadline expired while the fetch was still queued for a concurrency /// permit — under saturation the queue wait must not eat the budget the caller reserved @@ -184,25 +204,288 @@ pub struct FetchedObject { pub queue_wait: Duration, } -/// Fetches witness objects from an R2 bucket over the S3 API with SigV4-signed GETs. +/// Cloudflare Access service-token credentials, sent as the `CF-Access-Client-Id` / +/// `CF-Access-Client-Secret` headers on every custom-domain GET. /// -/// Cloning is cheap — the `reqwest::Client` and signer are internally reference-counted / -/// small. `Debug` is safe to derive: [`SigV4Signer`]'s own `Debug` redacts the credentials. +/// `Debug` redacts both halves: the id alone is enough to look up the token, so it gets the +/// same treatment as the secret. +#[derive(Clone)] +pub struct CfAccessCredentials { + /// The service token's client id. + pub client_id: String, + /// The service token's client secret. + pub client_secret: String, +} + +impl CfAccessCredentials { + /// Builds the two Access headers, marked sensitive so they are never HPACK-indexed and + /// stay redacted in header debug output. + /// + /// Fails when a value cannot be an HTTP header value — a trailing newline picked up from a + /// secret file or an unquoted env line is the usual cause. The offending value is never + /// echoed: it is the credential. Validating here turns what would otherwise be a builder + /// error raised per GET — classified retryable and so retried forever, with no startup + /// failure — into one named error at construction. + fn into_header_map(self) -> Result { + let mut headers = HeaderMap::new(); + for (name, value, flag) in [ + ("cf-access-client-id", self.client_id, "--r2-access-client-id"), + ("cf-access-client-secret", self.client_secret, "--r2-access-client-secret"), + ] { + let mut value = HeaderValue::from_str(&value).map_err(|_| { + format!( + "{flag} is not a valid HTTP header value (control characters — a trailing \ + newline from the secret store is the usual cause)" + ) + })?; + value.set_sensitive(true); + headers.insert(HeaderName::from_static(name), value); + } + Ok(headers) + } +} + +impl std::fmt::Debug for CfAccessCredentials { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("CfAccessCredentials") + .field("client_id", &"[redacted]") + .field("client_secret", &"[redacted]") + .finish() + } +} + +/// How a [`R2ObjectFetcher`] reaches the bucket. `Debug` is safe to derive: the only credential +/// holder left ([`SigV4Signer`]) redacts itself, and Access tokens live on the client's default +/// headers rather than here. #[derive(Clone, Debug)] -pub struct R2ObjectFetcher { +enum Target { + /// SigV4-signed GETs of `/{bucket}/{key}` against the bare S3 API endpoint. + S3 { + signer: SigV4Signer, + /// Endpoint origin (`scheme://host`, no trailing slash). + endpoint: String, + /// SigV4 canonical host (`host[:port]`). + host: String, + bucket: String, + }, + /// Unsigned GETs of `/{key}` against a Cloudflare custom domain fronting the bucket. + /// + /// Any Cloudflare Access service token is attached by the client's default headers, so it + /// cannot be bypassed by a request path that does not go through `request_parts`. + CustomDomain { + /// Domain origin (`scheme://host`, no trailing slash). + origin: String, + }, +} + +/// Parses a target's bare origin, rejecting anything that is not `scheme://host[:port]`. +/// +/// `label` and `example` name the flag the caller owns, so each target keeps its own error. +/// The raw input is never echoed back: a rejected shape may carry inline credentials +/// (userinfo), and this message ends up in startup logs. +fn parse_target_origin( + input: &str, + label: &str, + example: &str, +) -> Result<(String, String), String> { + let (origin, host) = parse_endpoint(input); + if host.is_empty() { + return Err(format!( + "Invalid R2 {label}: expected a bare scheme://host origin (no path/query/userinfo), \ + e.g. {example}" + )); + } + // A scheme reqwest cannot drive would pass startup and then fail inside `send()` as a + // builder error — classified retryable, so a permanently broken URL would be retried + // forever instead of failing once here. + if !origin.starts_with("https://") && !origin.starts_with("http://") { + return Err(format!( + "Invalid R2 {label}: only http and https are supported, e.g. {example}" + )); + } + Ok((origin, host)) +} + +/// Idle connections the pool may keep per host when `--r2-max-concurrent-requests` is unset. +/// +/// Only binds on the degraded HTTP/1.1 path: an h2 pool holds a single connection whatever this +/// says. There it is what stops `pool_idle_timeout(None)` — set so the multiplexed connection +/// survives the gaps between request waves — from letting idle sockets accumulate without +/// bound, which reqwest's `pool_max_idle_per_host` default of `usize::MAX` otherwise allows. +const MAX_IDLE_CONNECTIONS_PER_HOST: usize = 64; + +/// Streams a Cloudflare edge accepts on one HTTP/2 connection: its advertised +/// `SETTINGS_MAX_CONCURRENT_STREAMS`, which is Cloudflare's default since HTTP/2 Rapid Reset +/// and was read off the wire on the zone this target was built for. +pub const CLOUDFLARE_MAX_CONCURRENT_STREAMS: usize = 100; + +fn version_label(version: reqwest::Version) -> &'static str { + match version { + v if v == reqwest::Version::HTTP_09 => "http/0.9", + v if v == reqwest::Version::HTTP_10 => "http/1.0", + v if v == reqwest::Version::HTTP_11 => "http/1.1", + v if v == reqwest::Version::HTTP_2 => "h2", + v if v == reqwest::Version::HTTP_3 => "h3", + _ => "other", + } +} + +/// Whether `host[:port]` names the loopback interface, where plaintext `http` is the mock and +/// port-forward shape rather than a credential exposure. +fn is_loopback_host(host: &str) -> bool { + let bare = match host.strip_prefix('[') { + // IPv6 literal, `[::1]` or `[::1]:8080`. + Some(rest) => rest.split(']').next().unwrap_or(rest), + None => host.split(':').next().unwrap_or(host), + }; + bare.eq_ignore_ascii_case("localhost") || + bare.parse::().is_ok_and(|ip| ip.is_loopback()) +} + +/// The HTTP client settings both targets share; each constructor adds its own wire posture. +/// +/// Neither target may follow a redirect. A SigV4-signed GET cannot survive one — reqwest strips +/// `authorization` on cross-host hops and a same-host hop invalidates the signed URI, so +/// following just turns the real cause into a baffling 403. A 3xx from a custom domain is +/// itself the signal (Cloudflare Access bouncing an unauthenticated client to its login page), +/// which following would bury under an HTML body. Both surface the 3xx as a `Status` error. +fn base_client(timeouts: FetchTimeouts) -> reqwest::ClientBuilder { + Client::builder() + .timeout(timeouts.per_attempt) + .connect_timeout(timeouts.connect) + .redirect(reqwest::redirect::Policy::none()) +} + +/// One HTTP client and the permits bounding what that client may carry. +/// +/// Paired rather than kept as two parallel lists because a permit is only meaningful against +/// the connection it was taken for: the cap that binds is per connection — the edge limits +/// concurrent streams per connection, and one client holds exactly one connection. +#[derive(Debug)] +struct Connection { http: Client, - signer: SigV4Signer, - /// Endpoint origin (`scheme://host`, no trailing slash). - endpoint: String, - /// SigV4 canonical host (`host[:port]`). - host: String, - bucket: String, + permits: Semaphore, +} + +/// One fetcher-wide budget divided across `connections`, rounded up. +/// +/// The single home for that rounding, because three things derive from it — each connection's +/// permits, its idle-socket bound, and the share the startup advisory reports — and an +/// advisory that named a share the semaphores did not hand out would be worse than none. +/// Rounding *up* is deliberate: rounding down would round some connection to zero, and a +/// connection that can carry nothing is worse than a fetcher-wide total that overshoots by +/// less than the connection count. Callers normalise `connections` to at least 1 first. +fn per_connection(budget: usize, connections: usize) -> usize { + budget.div_ceil(connections) +} + +/// Each connection's share of the in-flight GET cap: `None` = unlimited, `Some(0)` clamps to 1 +/// — a zero-permit semaphore would wedge every fetch on `acquire()` forever. +fn per_connection_permits(max_concurrent_requests: Option, connections: usize) -> usize { + max_concurrent_requests + .map_or(Semaphore::MAX_PERMITS, |max| per_connection(max, connections.max(1))) + .max(1) +} + +/// A version callback that prints as its presence, so the fetcher keeps deriving `Debug`. +#[derive(Clone)] +struct VersionObserver(Arc); + +impl std::fmt::Debug for VersionObserver { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("") + } +} + +/// The connections a fetcher spreads its GETs over, and the cursor that rotates between them. +/// +/// One object rather than a slice beside a loose counter: the modulo that wraps the cursor is +/// only meaningful against this slice's length, so the two cannot be allowed to drift apart or +/// be cloned separately. +#[derive(Debug)] +struct ConnectionPool { + connections: Box<[Connection]>, + next: AtomicUsize, +} + +impl ConnectionPool { + /// Builds `connections` clients, each with its share of the in-flight budget. + /// + /// `connections` is the number of HTTP/2 connections the GETs are spread over, because one + /// `reqwest::Client` is one connection to a host and hyper will not open a second when the + /// first saturates. Anything past one connection therefore has to be asked for explicitly; + /// `0` is read as `1` rather than rejected, matching how the cap treats `Some(0)`. + fn build( + build: impl Fn() -> reqwest::Result, + connections: usize, + max_concurrent_requests: Option, + ) -> Result { + let permits = per_connection_permits(max_concurrent_requests, connections); + let connections = (0..connections.max(1)) + .map(|_| { + build() + .map(|http| Connection { http, permits: Semaphore::new(permits) }) + .map_err(|e| format!("Failed to build R2 HTTP client: {e}")) + }) + .collect::, _>>()?; + Ok(Self { connections, next: AtomicUsize::new(0) }) + } + + fn len(&self) -> usize { + self.connections.len() + } + + /// A connection with a free permit and that permit, or — when every connection is full — + /// the one to wait on. + /// + /// Work-conserving on purpose. Committing to the cursor's connection and then waiting on + /// *its* semaphore would partition one budget of `max` into `N` budgets of `max/N`, which + /// queues distinctly worse at the same offered load, and would leave a GET waiting behind + /// a connection whose permits are held by a slow transfer while another sits idle. + /// + /// The connection to wait on is the cursor's own pick, not the one with the most room: + /// under saturation that one is whichever just dropped every GET riding it, so choosing by + /// capacity would steer the wait straight into the fault. + /// + /// Exactly one cursor step per attempt, both outcomes included. Stepping again on the + /// full path would advance the cursor by two under saturation, and any stride sharing a + /// factor with the connection count then reaches only some of the connections — at 16 + /// connections, a stride of two leaves half of them never waited on. + fn acquire_or_wait_on(&self) -> Result<(&Connection, SemaphorePermit<'_>), &Connection> { + let start = self.next.fetch_add(1, Ordering::Relaxed); + for offset in 0..self.len() { + let connection = &self.connections[(start + offset) % self.len()]; + if let Ok(permit) = connection.permits.try_acquire() { + return Ok((connection, permit)); + } + } + Err(&self.connections[start % self.len()]) + } +} + +/// Fetches witness objects from an R2 bucket — SigV4-signed over the S3 API, or unsigned +/// through a Cloudflare custom domain (see the module docs for the two targets). +/// +/// Cloning is cheap — the `reqwest::Client` and the target's credential holders are +/// internally reference-counted / small. `Debug` is safe to derive: the target redacts. +#[derive(Clone, Debug)] +pub struct R2ObjectFetcher { + /// The connections this fetcher spreads its GETs over, each with its own permits. One + /// entry unless the caller asked for more. Shared across clones, so clones keep rotating + /// over the same connections instead of each starting at zero. + pool: Arc, + target: Target, /// Hard cap on a single GET attempt; with a deadline, each attempt uses /// `min(timeouts.per_attempt, remaining)`. per_attempt_timeout: Duration, pacing: RetryPacing, - /// Caps concurrent GETs across all fetches sharing this fetcher. - concurrency: Arc, + /// Protocol the first response actually used, empty until one arrives. Shared across + /// clones so the degradation warning fires once per fetcher, not per clone. + negotiated_version: Arc>, + /// Notified with that protocol the once it is learned, for callers that publish it. + /// Pushed rather than polled: the fact is knowable exactly once, and a caller that had to + /// poll would need its own dedup and would have to remember to poll on failure paths too. + on_version_observed: Option, } impl R2ObjectFetcher { @@ -212,6 +495,121 @@ impl R2ObjectFetcher { self.pacing } + /// This fetcher's target origin (`scheme://host[:port]`). + /// + /// Callers log this instead of the flag they were given: the raw operator string can carry + /// inline credentials (userinfo), and these lines are info-level. + pub fn origin(&self) -> &str { + match &self.target { + Target::S3 { endpoint, .. } => endpoint, + Target::CustomDomain { origin, .. } => origin, + } + } + + /// Stable lowercase label for the configured target, for callers' metric labels. + /// + /// Lives here for the same reason [`R2GetError::kind`] does: the vocabulary belongs to the + /// enum it names, so a renamed or added target cannot leave a binary publishing a stale + /// string that nothing would flag. + pub const fn target_label(&self) -> &'static str { + match &self.target { + Target::S3 { .. } => "s3", + Target::CustomDomain { .. } => "custom_domain", + } + } + + /// The protocol the first response on this fetcher actually used, once one has arrived. + /// + /// Answers "did the custom domain really give us h2", which the configured target alone + /// cannot: version selection is pure ALPN, so a degraded origin looks identical from the + /// configuration side. + pub fn negotiated_http_version(&self) -> Option<&'static str> { + self.negotiated_version.get().copied() + } + + /// Registers a callback invoked once, with the protocol of the first response this fetcher + /// receives. Chained after construction so the targets that cannot degrade need not pass it. + #[must_use] + pub fn on_version_observed( + mut self, + observe: impl Fn(&'static str) + Send + Sync + 'static, + ) -> Self { + self.on_version_observed = Some(VersionObserver(Arc::new(observe))); + self + } + + /// The protocol this target is built to speak, so a response that used another one is a + /// degradation rather than a surprise. Keeps the check off the response path's control + /// flow: without it the observation has to be gated on the target, purely because the + /// warning text names one. + const fn expected_version(&self) -> reqwest::Version { + match self.target { + // Pinned `http1_only` at construction, so ALPN cannot move it. + Target::S3 { .. } => reqwest::Version::HTTP_11, + Target::CustomDomain { .. } => reqwest::Version::HTTP_2, + } + } + + /// Records the protocol of the first response, warning once if a custom domain did not + /// negotiate HTTP/2. + /// + /// There is deliberately no `http2_prior_knowledge` — it would break the plaintext loopback + /// path the mocks and port-forwards rely on — so an h1-only peer simply answers normally and + /// nothing else notices. That is the trap worth naming: the three h2 knobs go inert, while + /// `pool_idle_timeout(None)` stays in force over an HTTP/1.1 pool, and the target gauge goes + /// on asserting the target that was *configured*. Warned once rather than per GET, which at + /// this call rate would be thousands of lines a minute. + fn observe_version(&self, version: reqwest::Version) { + let label = version_label(version); + if self.negotiated_version.set(label).is_err() { + return; // another response was first; it already warned and notified + } + if let Some(observe) = &self.on_version_observed { + (observe.0)(label); + } + if version != self.expected_version() { + warn!( + negotiated = label, + origin = self.origin(), + "R2 custom domain did not negotiate HTTP/2: the multiplexing this target exists \ + for is unavailable, its h2 tuning is inert, and connection reuse now follows \ + HTTP/1.1 pooling. Check that the domain is proxied by Cloudflare and that the \ + zone has HTTP/2 enabled." + ); + } + } + + /// Each connection's share of the configured concurrency, when that share over-subscribes + /// the edge's per-connection stream limit — that is, when the limit rather than this + /// fetcher's semaphores is what bounds the GETs actually in flight. + /// + /// One client holds exactly one HTTP/2 connection, and hyper never opens a second one to + /// relieve a saturated one: `is_open` on a pooled h2 connection reports liveness, not + /// stream capacity, and the dispatch channel behind it is unbounded. So a request past the + /// limit does not fail and does not get a connection of its own — it waits inside the + /// connection, where the wait is invisible to the caller's queue-wait metric and still + /// counts against the per-attempt timeout. Spreading the same concurrency over more + /// connections is what actually raises the ceiling, which is why this is measured per + /// connection rather than against the fetcher-wide total. + /// + /// Exactly *at* the limit is the intended sizing, not a misconfiguration: every permit maps + /// to a stream slot and nothing queues. Unlimited is not flagged either — it is the + /// unconfigured default, and a warning that fires on a default is one operators learn to + /// skip. + fn concurrency_over_edge_stream_limit( + max_concurrent_requests: Option, + connections: usize, + ) -> Option { + max_concurrent_requests + .map(|max| per_connection(max, connections.max(1))) + .filter(|share| *share > CLOUDFLARE_MAX_CONCURRENT_STREAMS) + } + + /// How many HTTP/2 connections this fetcher spreads its GETs over. + pub fn connections(&self) -> usize { + self.pool.len() + } + /// Builds a fetcher from an R2 endpoint origin, bucket, and bucket-scoped S3 credentials. /// /// `timeouts` bounds each individual GET (end-to-end and connect phase). `pacing` @@ -229,34 +627,147 @@ impl R2ObjectFetcher { pacing: RetryPacing, max_concurrent_requests: Option, ) -> Result { - let (origin, host) = parse_endpoint(endpoint); - if host.is_empty() { - // The raw input is not echoed: a rejected shape may carry inline credentials - // (userinfo), and this message ends up in startup logs. - return Err("Invalid R2 endpoint: expected a bare scheme://host origin (no \ - path/query/userinfo), e.g. https://.r2.cloudflarestorage.com" + let (origin, host) = parse_target_origin( + endpoint, + "endpoint", + "https://.r2.cloudflarestorage.com", + )?; + // A single connection's worth of clients: the signed endpoint speaks HTTP/1.1, whose + // pool already opens a socket per concurrent request, so spreading over more clients + // would buy nothing here. Spreading is a custom-domain concern, where multiplexing + // means one client is one connection. + let pool = ConnectionPool::build( + || { + base_client(timeouts) + // Pin the S3 target to HTTP/1.1 in the client rather than relying on the + // server's ALPN choice: the endpoint only speaks h1.1 today, and this keeps + // the signed path's wire behavior fixed even if that ever changes upstream. + .http1_only() + .build() + }, + 1, + max_concurrent_requests, + )?; + Ok(Self { + pool: Arc::new(pool), + target: Target::S3 { + signer: SigV4Signer::new(access_key_id, secret_access_key), + endpoint: origin, + host, + bucket, + }, + per_attempt_timeout: timeouts.per_attempt, + pacing, + negotiated_version: Arc::new(OnceLock::new()), + on_version_observed: None, + }) + } + + /// Builds a fetcher that GETs objects unsigned through a Cloudflare custom domain + /// fronting the bucket (`https:///{key}` — no bucket path segment, the domain is + /// bucket-scoped). + /// + /// `access` attaches Cloudflare Access service-token headers to every GET; leave it + /// `None` when the domain is locked by an IP allowlist instead. The remaining parameters + /// mean exactly what they mean on [`Self::new`]. Fails if the domain is not a bare + /// `scheme://host[:port]` origin — the object path is appended by this fetcher, and a + /// path-bearing domain would silently double it. + /// + /// `connections` is how many HTTP/2 connections the GETs are spread over, and + /// `max_concurrent_requests` is the in-flight cap across all of them. One is the multiplexed + /// shape this target exists for; more than one is how a caller gets past the edge's + /// per-connection stream limit, since one client is one connection and hyper opens no + /// second one when the first saturates. It is also how a caller stops one connection from + /// being a single point of failure: when a connection drops, every GET riding it fails + /// together, which matters where R2 has no fallback. + /// + /// The domain rides Cloudflare's h2-capable edge, so this client differs from the S3 one + /// in its HTTP/2 posture (all three knobs are inert on an endpoint that only offers + /// http/1.1 via ALPN, which also keeps this constructor honest against a non-h2 origin): + /// - adaptive flow-control windows — the defaults are sized well under the bandwidth-delay + /// product of an intercontinental path, and would cap a multi-MB tail object far below the + /// link's actual capacity; + /// - keep-alive pings, including while idle — the single multiplexed connection must survive + /// the gaps between request waves, or every wave re-pays the TLS handshake and + /// congestion-window ramp that connection reuse exists to avoid. + pub fn new_custom_domain( + domain: &str, + access: Option, + timeouts: FetchTimeouts, + pacing: RetryPacing, + max_concurrent_requests: Option, + connections: usize, + ) -> Result { + let (origin, host) = + parse_target_origin(domain, "custom domain", "https://witness.example.com")?; + // An Access service token is a bearer credential; plaintext would put it on the wire in + // the clear on every GET. Loopback stays allowed — that is the mock and port-forward + // shape, not an exposure. + if access.is_some() && origin.starts_with("http://") && !is_loopback_host(&host) { + return Err("Refusing to send Cloudflare Access credentials over plaintext http:// \ + to a non-loopback host: give --r2-custom-domain an https:// origin" .to_string()); } - let http = Client::builder() - .timeout(timeouts.per_attempt) - .connect_timeout(timeouts.connect) - // A SigV4-signed GET can never survive a redirect (reqwest strips `authorization` on - // cross-host hops, and a same-host hop invalidates the signed URI), so following one - // just turns the real cause into a baffling 403. Surface the 3xx as a `Status` error. - .redirect(reqwest::redirect::Policy::none()) - .build() - .map_err(|e| format!("Failed to build R2 HTTP client: {e}"))?; + // Built once and cloned per client: `into_header_map` validates the credential, and + // failing that validation is a property of the credential, not of a given connection. + let headers = access.map(CfAccessCredentials::into_header_map).transpose()?; + let connections = connections.max(1); + // Each client's own idle bound: the fetcher-wide bound divided the same way the permits + // are, so holding more clients cannot multiply the sockets the h1.1 fallback may keep. + let idle_per_client = per_connection( + max_concurrent_requests.unwrap_or(MAX_IDLE_CONNECTIONS_PER_HOST), + connections, + ); + let build = || { + let mut builder = base_client(timeouts) + // Cloudflare's Browser Integrity Check, on by default on many zones, challenges + // requests that carry no user agent — which would arrive here as a non-retryable + // 403 on every GET. It also makes this traffic attributable in zone analytics. + .user_agent(concat!("stateless-r2/", env!("CARGO_PKG_VERSION"))) + .http2_adaptive_window(true) + .http2_keep_alive_interval(Duration::from_secs(30)) + .http2_keep_alive_while_idle(true) + // Keep-alive pings hold the connection open at the protocol level, but the pool + // reaps idle connections on a 90s default of its own — shorter than the gaps + // between request waves that these multiplexed connections exist to survive. + .pool_idle_timeout(None) + // Paired with the line above: without an idle timeout, an unbounded idle pool + // would never release a socket. Inert on h2 (one connection per client); the + // bound that matters is on the h1.1 path this client silently falls back to + // against a non-h2 origin. + .pool_max_idle_per_host(idle_per_client) + // Stated rather than inherited: with keep-alive pings on, this is what bounds + // how long a blackholed connection keeps accepting doomed streams. + .http2_keep_alive_timeout(Duration::from_secs(20)); + if let Some(headers) = &headers { + // On the client, not per attempt: authentication is then a property of every + // request this fetcher makes, and an unusable credential fails at construction + // instead of once per GET as a retryable transport error. + builder = builder.default_headers(headers.clone()); + } + builder.build() + }; + if let Some(per_connection) = + Self::concurrency_over_edge_stream_limit(max_concurrent_requests, connections) + { + warn!( + per_connection, + connections = connections.max(1), + edge_stream_limit = CLOUDFLARE_MAX_CONCURRENT_STREAMS, + "R2 custom-domain concurrency over-subscribes the edge's per-connection HTTP/2 \ + stream limit: the surplus queues inside the connection rather than on the \ + semaphore, where the wait escapes the queue-wait metric and still counts \ + against the per-attempt timeout. Lower the concurrency or raise the \ + connection count." + ); + } Ok(Self { - http, - signer: SigV4Signer::new(access_key_id, secret_access_key), - endpoint: origin, - host, - bucket, + pool: Arc::new(ConnectionPool::build(build, connections, max_concurrent_requests)?), + target: Target::CustomDomain { origin }, per_attempt_timeout: timeouts.per_attempt, pacing, - concurrency: Arc::new(Semaphore::new( - max_concurrent_requests.unwrap_or(Semaphore::MAX_PERMITS).max(1), - )), + negotiated_version: Arc::new(OnceLock::new()), + on_version_observed: None, }) } @@ -291,21 +802,35 @@ impl R2ObjectFetcher { // waste capacity other fetches could use. let outcome = { let queued = Instant::now(); - // A deadline bounds the queue wait too: under saturation the caller's budget - // must stay available for its fallback, not drain waiting for a permit. - let permit = match deadline { - None => self.concurrency.acquire().await, - Some(d) => { - match tokio::time::timeout_at(d.into(), self.concurrency.acquire()).await { - Ok(acquired) => acquired, - Err(_) => return Err(R2GetError::Deadline { number, key }), - } + // The connection is chosen per attempt, not per fetch, so a retry lands on a + // different one than the attempt that just failed: one client is one + // connection, so retrying onto the same one retries into the same fault. + let (connection, permit) = match self.pool.acquire_or_wait_on() { + Ok(taken) => taken, + // Every connection full. A deadline bounds this wait too, since under + // saturation the caller's budget must stay available for its fallback + // rather than drain here. + Err(connection) => { + let waited = match deadline { + None => connection.permits.acquire().await, + Some(d) => { + match tokio::time::timeout_at( + d.into(), + connection.permits.acquire(), + ) + .await + { + Ok(acquired) => acquired, + Err(_) => return Err(R2GetError::Deadline { number, key }), + } + } + }; + (connection, waited.expect("semaphore is never closed")) } - } - .expect("semaphore is never closed"); + }; queue_wait += queued.elapsed(); let _permit = permit; - self.get_object(number, &key, deadline).await + self.get_object(connection, number, &key, deadline).await }; match outcome { Ok(bytes) => return Ok(FetchedObject { bytes, queue_wait }), @@ -334,19 +859,38 @@ impl R2ObjectFetcher { } } - /// Performs one SigV4-signed GET and classifies the response. No retry. + /// Builds one attempt's request for this fetcher's target: the signed S3 layout + /// (`/{bucket}/{key}` plus freshly-signed SigV4 headers) or the unsigned custom-domain + /// layout (`/{key}`, whose only credentials are the client's default headers). + /// + /// Called per attempt because a SigV4 signature is timestamped and cannot be reused. + fn request(&self, connection: &Connection, key: &str) -> reqwest::RequestBuilder { + let http = &connection.http; + match &self.target { + Target::S3 { signer, endpoint, host, bucket } => { + let canonical_uri = encode_uri_path(bucket, key); + // Signed-payload mode with an empty body: x-amz-content-sha256 = sha256(""). + let signed = signer.sign("GET", host, &canonical_uri, "", &[], b"", Utc::now()); + signed.into_iter().fold( + http.get(format!("{endpoint}{canonical_uri}")), + |request, (name, value)| request.header(name, value), + ) + } + Target::CustomDomain { origin } => { + http.get(format!("{origin}{}", encode_key_path(key))) + } + } + } + + /// Performs one GET against this fetcher's target and classifies the response. No retry. async fn get_object( &self, + connection: &Connection, number: u64, key: &str, deadline: Option, ) -> Result { - let canonical_uri = encode_uri_path(&self.bucket, key); - let url = format!("{}{}", self.endpoint, canonical_uri); - // Signed-payload mode with an empty body: x-amz-content-sha256 = sha256(""). - let signed = self.signer.sign("GET", &self.host, &canonical_uri, "", &[], b"", Utc::now()); - - let mut request = self.http.get(&url); + let mut request = self.request(connection, key); // Clamp the attempt to the remaining budget; an already-expired deadline degrades to // a floor timeout whose transport error the retry loop then surfaces as out-of-time. if let Some(deadline) = deadline { @@ -354,9 +898,6 @@ impl R2ObjectFetcher { deadline.saturating_duration_since(Instant::now()).max(Duration::from_millis(1)); request = request.timeout(self.per_attempt_timeout.min(remaining)); } - for (name, value) in signed { - request = request.header(name, value); - } let transport = |source: reqwest::Error| { let key = key.to_string(); if source.is_connect() { @@ -366,6 +907,7 @@ impl R2ObjectFetcher { } }; let response = request.send().await.map_err(transport)?; + self.observe_version(response.version()); let status = response.status(); if status.is_success() { @@ -388,7 +930,9 @@ impl R2ObjectFetcher { // parsing the S3 XML error code. A 404 with no parseable code (a proxy's bare 404, a // truncated body) still counts as Missing: for a correctly configured endpoint that is // by far the likeliest cause, and misreading a config error as Missing only changes - // the caller's metric kind, not the retry behavior. + // the caller's metric kind, not the retry behavior. Custom-domain 404s carry no S3 + // XML at all, so they classify as Missing through that same arm — also the right + // default there, where an absent object is the only routine 404. if code == 404 && s3_error_code(&body).is_none_or(|c| c == "NoSuchKey") { return Err(R2GetError::Missing { number, key: key.to_string() }); } @@ -414,7 +958,7 @@ fn s3_error_code(body: &str) -> Option<&str> { mod tests { use std::sync::atomic::{AtomicUsize, Ordering}; - use stateless_test_utils::mock_r2::{mock_r2, mock_r2_held}; + use stateless_test_utils::mock_r2::{mock_r2, mock_r2_capturing, mock_r2_held}; use super::*; @@ -675,6 +1219,412 @@ mod tests { let _ = holder.await; } + fn try_custom_fetcher( + domain: &str, + access: Option, + ) -> Result { + R2ObjectFetcher::new_custom_domain(domain, access, test_timeouts(), test_pacing(), None, 1) + } + + fn custom_fetcher(domain: &str, access: Option) -> R2ObjectFetcher { + try_custom_fetcher(domain, access).unwrap() + } + + fn custom_fetcher_with( + domain: &str, + max: Option, + connections: usize, + ) -> R2ObjectFetcher { + R2ObjectFetcher::new_custom_domain( + domain, + None, + test_timeouts(), + test_pacing(), + max, + connections, + ) + .unwrap() + } + + /// A credential that cannot be a header value fails at construction, by flag name and + /// without echoing the value. Left to `send()` it becomes a reqwest builder error, which + /// classifies as retryable `Transport` — so every GET would burn the full retry ramp + /// forever and no startup failure would ever name the real cause. + #[test] + fn access_credential_with_a_control_byte_fails_construction_without_echoing_it() { + let err = try_custom_fetcher( + "https://witness.example.com", + Some(CfAccessCredentials { + client_id: "tok-3f1.access".to_string(), + // The shape of `$(cat secret.txt)` or an unquoted env-file line. + client_secret: "sec-9a2\n".to_string(), + }), + ) + .expect_err("a trailing newline cannot be a header value"); + assert!(err.contains("--r2-access-client-secret"), "{err}"); + assert!(!err.contains("sec-9a2"), "the secret must never reach a log line: {err}"); + } + + /// An IPv6 loopback origin is still loopback. `Url::host_str` keeps the brackets on an + /// IPv6 host, so the origin `parse_endpoint` rebuilds stays a parseable authority and the + /// loopback check still recognises it — a plaintext port-forward to `[::1]` is the same + /// mock shape as `127.0.0.1` and must not be refused as an exposure. + #[test] + fn ipv6_loopback_origins_survive_the_round_trip() { + let creds = || { + Some(CfAccessCredentials { + client_id: "tok-3f1.access".to_string(), + client_secret: "sec-9a2".to_string(), + }) + }; + for origin in ["http://[::1]:8080", "http://[::1]"] { + let f = try_custom_fetcher(origin, creds()) + .unwrap_or_else(|e| panic!("{origin} must be accepted as loopback: {e}")); + assert_eq!(f.origin(), origin, "brackets must survive the origin rebuild"); + } + // The non-loopback IPv6 case still refuses plaintext credentials. + let err = try_custom_fetcher("http://[2001:db8::1]:8080", creds()) + .expect_err("a public IPv6 host over plaintext must be refused"); + assert!(err.contains("plaintext"), "{err}"); + } + + /// Access tokens are bearer credentials, so a plaintext non-loopback origin is refused + /// rather than putting them on the wire in the clear on every GET. + #[test] + fn access_credentials_are_refused_over_plaintext_to_a_remote_host() { + let access = || { + Some(CfAccessCredentials { + client_id: "tok-3f1.access".to_string(), + client_secret: "sec-9a2".to_string(), + }) + }; + let err = try_custom_fetcher("http://witness.example.com", access()) + .expect_err("plaintext + credentials to a remote host must be refused"); + assert!(err.contains("plaintext"), "{err}"); + + // Loopback stays usable: that is the mock and port-forward shape, and every + // custom-domain test here depends on it. + for loopback in ["http://127.0.0.1:8080", "http://localhost:8080", "http://[::1]:8080"] { + try_custom_fetcher(loopback, access()) + .unwrap_or_else(|e| panic!("{loopback} must be allowed: {e}")); + } + // Plaintext without credentials has nothing to leak. + custom_fetcher("http://witness.example.com", None); + } + + /// A scheme reqwest cannot drive is rejected at construction on both targets; left alone it + /// surfaces per GET as a retryable transport error, retrying a permanently broken URL. + #[test] + fn non_http_schemes_are_rejected_on_both_targets() { + let custom = + try_custom_fetcher("ftp://witness.example.com", None).expect_err("ftp is not drivable"); + assert!(custom.contains("only http and https"), "{custom}"); + + let s3 = R2ObjectFetcher::new( + "ftp://acc.r2.cloudflarestorage.com", + "witness-mainnet".to_string(), + "ak".to_string(), + "sk".to_string(), + test_timeouts(), + test_pacing(), + None, + ) + .expect_err("ftp is not drivable"); + assert!(s3.contains("only http and https"), "{s3}"); + } + + /// The custom-domain target degrades to HTTP/1.1 in silence against any origin that does + /// not offer h2 over ALPN — and every mock in this suite is exactly such an origin, so the + /// green tests here *are* the degraded path. Pinning that keeps it visible: the protocol + /// actually negotiated is observable, rather than inferred from the configured target. + #[tokio::test] + async fn custom_domain_reports_the_protocol_it_actually_negotiated() { + let (domain, _hits) = mock_r2(vec![(200, "witness bytes")]).await; + let fetcher = custom_fetcher(&domain, None); + assert_eq!(fetcher.negotiated_http_version(), None, "nothing observed before a request"); + + fetcher + .get_block_object(2500, "0xblock", MAX_ATTEMPTS, None, || ()) + .await + .expect("200 must succeed"); + assert_eq!( + fetcher.negotiated_http_version(), + Some("http/1.1"), + "the plaintext mock cannot offer h2, and that must be visible rather than assumed" + ); + } + + /// The advisory covers over-subscription only, and measures it per connection — spreading + /// the same concurrency over more connections is precisely the fix, so it must clear the + /// warning. Exactly at the limit every permit maps to a stream slot, which is the sizing + /// the flag documentation asks for; warning there would fire on a correct configuration, + /// and warning on unlimited would fire on the default. + #[test] + fn edge_stream_limit_advisory_covers_per_connection_over_subscription() { + let over = R2ObjectFetcher::concurrency_over_edge_stream_limit; + const LIMIT: usize = CLOUDFLARE_MAX_CONCURRENT_STREAMS; + assert_eq!(over(None, 1), None, "unlimited is the default, not a misconfiguration"); + assert_eq!(over(Some(LIMIT), 1), None, "at the limit is the intended sizing"); + assert_eq!( + over(Some(LIMIT + 1), 1), + Some(LIMIT + 1), + "one past the limit is one GET queued where the queue cannot be seen" + ); + assert_eq!(over(Some(4 * LIMIT), 4), None, "spreading it over four connections fits"); + assert_eq!( + over(Some(4 * LIMIT + 4), 4), + Some(LIMIT + 1), + "the warning reports the per-connection share, not the configured total" + ); + assert_eq!(over(Some(LIMIT + 1), 0), Some(LIMIT + 1), "zero connections is read as one"); + } + + /// The cap is fetcher-wide and the semaphores are per connection, so the split has to round + /// up: rounding down would give some connection zero permits and wedge every GET routed to + /// it, which is a worse failure than a total that overshoots by less than the connection + /// count. + #[test] + fn concurrency_splits_across_connections_rounding_up() { + assert_eq!(per_connection_permits(Some(48), 8), 6, "an even split is exact"); + assert_eq!(per_connection_permits(Some(3), 4), 1, "never zero, so no connection wedges"); + assert_eq!(per_connection_permits(Some(0), 1), 1, "a zero cap clamps rather than wedges"); + assert_eq!(per_connection_permits(Some(10), 0), 10, "zero connections is read as one"); + assert_eq!( + per_connection_permits(None, 8), + Semaphore::MAX_PERMITS, + "unlimited stays unlimited per connection rather than being divided into a cap" + ); + } + + /// Round-robin, and per attempt rather than per fetch: a retry has to be able to leave a + /// connection that just failed, since one connection is one client and a dropped connection + /// takes every GET riding it down together. + #[tokio::test] + async fn connections_are_handed_out_round_robin() { + let fetcher = custom_fetcher_with("https://witness.example.com", Some(12), 3); + assert_eq!(fetcher.connections(), 3); + let taken: Vec<_> = + (0..6).map(|_| fetcher.pool.acquire_or_wait_on().expect("free")).collect(); + let cycle: Vec<*const Connection> = + taken.iter().map(|(connection, _)| std::ptr::from_ref(*connection)).collect(); + assert_eq!(cycle[..3], cycle[3..], "the cursor wraps rather than drifting"); + assert_eq!( + cycle[..3].iter().collect::>().len(), + 3, + "consecutive attempts land on distinct connections" + ); + } + + /// The deployment shape this flag exists for — 16 connections under a cap of 1024 — must + /// spread evenly, both while filling and once saturated. + /// + /// The saturated half is the one worth pinning. Handing out a connection and choosing the + /// one to wait on used to be two calls, so a saturated attempt stepped the cursor twice, + /// and a stride sharing a factor with the connection count reaches only some of them: + /// at 16 connections a stride of two left half of them never waited on, while the other + /// half took every waiter. + #[tokio::test] + async fn a_saturated_pool_spreads_evenly_over_every_connection() { + const CONNECTIONS: usize = 16; + const CAP: usize = 1024; + let share = CAP / CONNECTIONS; + + let fetcher = custom_fetcher_with("https://witness.example.com", Some(CAP), CONNECTIONS); + let base = std::ptr::from_ref(&fetcher.pool.connections[0]) as usize; + let index = |c: &Connection| { + (std::ptr::from_ref(c) as usize - base) / std::mem::size_of::() + }; + + // Filling to the cap: the permits divide exactly, so every connection must end at its + // share — anything else means the cap leaked or a connection was skipped. + let mut held = Vec::new(); + let mut taken = [0usize; CONNECTIONS]; + for _ in 0..CAP { + let (connection, permit) = + fetcher.pool.acquire_or_wait_on().expect("room below the cap"); + taken[index(connection)] += 1; + held.push(permit); + } + assert_eq!(taken, [share; CONNECTIONS], "the cap must divide evenly across connections"); + + // Saturated: every further arrival is told which connection to wait on, and those must + // rotate over all of them rather than a subset. + let mut waiting = [0usize; CONNECTIONS]; + for _ in 0..CONNECTIONS * 4 { + let connection = + fetcher.pool.acquire_or_wait_on().expect_err("the cap is fully subscribed"); + waiting[index(connection)] += 1; + } + assert_eq!(waiting, [4; CONNECTIONS], "waiters must rotate over every connection"); + } + + /// A full connection is skipped rather than queued behind. + /// + /// Committing to the cursor's pick and then waiting on its semaphore would partition one + /// budget of `max` into `N` budgets of `max/N` — distinctly worse queueing at the same + /// offered load — and would strand a GET behind a connection whose permits are held by a + /// slow transfer while another connection sits idle. + #[tokio::test] + async fn a_saturated_connection_is_skipped_while_another_has_room() { + // Two connections, one permit each. + let fetcher = custom_fetcher_with("https://witness.example.com", Some(2), 2); + let (first, _first_permit) = fetcher.pool.acquire_or_wait_on().expect("both free"); + + let (second, _second_permit) = fetcher.pool.acquire_or_wait_on().expect("one still free"); + assert!(!std::ptr::eq(first, second), "the free connection is the one handed out"); + + assert!( + fetcher.pool.acquire_or_wait_on().is_err(), + "with both full, nothing is handed out" + ); + // And the cursor still names one to wait on rather than deadlocking the caller. + assert!(fetcher.pool.acquire_or_wait_on().unwrap_err().permits.try_acquire().is_err()); + } + + /// Cloudflare's Browser Integrity Check challenges user-agent-less requests, which would + /// arrive as a non-retryable 403 on every GET — and the validator's R2 mode has no fallback. + #[tokio::test] + async fn custom_domain_sends_a_user_agent() { + let (domain, _hits, heads) = mock_r2_capturing(vec![(200, "witness bytes")]).await; + custom_fetcher(&domain, None) + .get_block_object(2500, "0xblock", MAX_ATTEMPTS, None, || ()) + .await + .expect("200 must succeed"); + let head = heads.lock().unwrap()[0].to_lowercase(); + assert!(head.contains("user-agent: stateless-r2/"), "{head}"); + } + + #[test] + fn custom_domain_rejects_origin_with_path() { + // The fetcher appends the object path itself; a path-bearing domain would silently + // double it, so construction must fail fast (same policy as the S3 endpoint). + let err = + try_custom_fetcher("https://witness.example.com/witness-mainnet", None).unwrap_err(); + assert!(err.contains("Invalid R2 custom domain"), "{err}"); + } + + /// The custom-domain wire shape: `GET /{key}` with no bucket segment, no SigV4 + /// `authorization`, and no Access headers unless configured. + #[tokio::test] + async fn custom_domain_gets_bare_key_path_unsigned() { + let (domain, hits, heads) = mock_r2_capturing(vec![(200, "witness bytes")]).await; + let fetched = custom_fetcher(&domain, None) + .get_block_object(2500, "0xblock", MAX_ATTEMPTS, None, || ()) + .await + .expect("200 must succeed"); + assert_eq!(fetched.bytes.as_ref(), b"witness bytes"); + assert_eq!(hits.load(Ordering::SeqCst), 1); + let head = heads.lock().unwrap()[0].to_lowercase(); + assert!(head.starts_with("get /block/2000_2999/2500.0xblock http/1.1\r\n"), "{head}"); + assert!(!head.contains("authorization:"), "custom-domain GET must be unsigned: {head}"); + assert!(!head.contains("cf-access-client-id:"), "no Access headers configured: {head}"); + } + + /// The S3 wire shape stays what it was: `GET /{bucket}/{key}` carrying a SigV4 + /// `authorization` header — pinned so the custom-domain arm can never bleed into it. + #[tokio::test] + async fn s3_get_prefixes_the_bucket_and_signs() { + let (endpoint, _, heads) = mock_r2_capturing(vec![(200, "witness bytes")]).await; + fetcher(&endpoint) + .get_block_object(2500, "0xblock", MAX_ATTEMPTS, None, || ()) + .await + .expect("200 must succeed"); + let head = heads.lock().unwrap()[0].to_lowercase(); + assert!( + head.starts_with("get /witness-test/block/2000_2999/2500.0xblock http/1.1\r\n"), + "{head}" + ); + assert!(head.contains("authorization: aws4-hmac-sha256"), "{head}"); + } + + #[tokio::test] + async fn custom_domain_sends_access_headers_when_configured() { + let (domain, _, heads) = mock_r2_capturing(vec![(200, "ok")]).await; + let access = CfAccessCredentials { + client_id: "tok-3f1.access".to_string(), + client_secret: "sec-9a2".to_string(), + }; + custom_fetcher(&domain, Some(access)) + .get_block_object(1, "0xhash", 1, None, || ()) + .await + .expect("200 must succeed"); + let head = heads.lock().unwrap()[0].to_lowercase(); + assert!(head.contains("cf-access-client-id: tok-3f1.access"), "{head}"); + assert!(head.contains("cf-access-client-secret: sec-9a2"), "{head}"); + assert!(!head.contains("authorization:"), "{head}"); + } + + /// The custom-domain client shares the no-redirect policy: a 3xx (the shape of a + /// Cloudflare Access login bounce) must surface as `Status`, not be followed into an + /// HTML page. + #[tokio::test] + async fn custom_domain_redirects_are_not_followed() { + let (domain, hits) = mock_r2(vec![(302, "login bounce")]).await; + let err = custom_fetcher(&domain, None) + .get_block_object(1, "0xhash", MAX_ATTEMPTS, None, || ()) + .await + .unwrap_err(); + assert!(matches!(err, R2GetError::Status { status: 302, .. }), "{err}"); + assert_eq!(hits.load(Ordering::SeqCst), 1); + } + + /// Credentials are rebuilt per attempt: the retry after a throttle must still carry the + /// Access pair (custom domain) / a fresh SigV4 authorization (S3) — pinned so a future + /// hoist of header construction out of the attempt loop cannot silently strip retries. + #[tokio::test] + async fn retries_resend_credentials() { + let (domain, hits, heads) = mock_r2_capturing(vec![(503, "slow"), (200, "ok")]).await; + let access = CfAccessCredentials { + client_id: "tok-3f1.access".to_string(), + client_secret: "sec-9a2".to_string(), + }; + custom_fetcher(&domain, Some(access)) + .get_block_object(1, "0xhash", MAX_ATTEMPTS, None, || ()) + .await + .expect("retry must succeed"); + assert_eq!(hits.load(Ordering::SeqCst), 2); + let retry_head = heads.lock().unwrap()[1].to_lowercase(); + assert!(retry_head.contains("cf-access-client-id: tok-3f1.access"), "{retry_head}"); + assert!(retry_head.contains("cf-access-client-secret: sec-9a2"), "{retry_head}"); + + let (endpoint, hits, heads) = mock_r2_capturing(vec![(503, "slow"), (200, "ok")]).await; + fetcher(&endpoint) + .get_block_object(1, "0xhash", MAX_ATTEMPTS, None, || ()) + .await + .expect("retry must succeed"); + assert_eq!(hits.load(Ordering::SeqCst), 2); + let retry_head = heads.lock().unwrap()[1].to_lowercase(); + assert!(retry_head.contains("authorization: aws4-hmac-sha256"), "{retry_head}"); + } + + /// A custom-domain 404 carries no S3 XML; the bare body must classify as `Missing`. + #[tokio::test] + async fn custom_domain_bare_404_is_missing() { + let (domain, hits) = mock_r2(vec![(404, "not found")]).await; + let err = custom_fetcher(&domain, None) + .get_block_object(1, "0xhash", MAX_ATTEMPTS, None, || ()) + .await + .unwrap_err(); + assert!(matches!(err, R2GetError::Missing { .. }), "{err}"); + assert_eq!(hits.load(Ordering::SeqCst), 1, "404 must not be retried"); + } + + /// Neither Access half may leak through `Debug` — the fetcher (and its target) end up in + /// startup logs via `Debug` formatting. + #[test] + fn access_credentials_never_leak_through_debug() { + let access = CfAccessCredentials { + client_id: "tok-3f1.access".to_string(), + client_secret: "sec-9a2".to_string(), + }; + let direct = format!("{access:?}"); + let via_fetcher = format!("{:?}", custom_fetcher("https://w.example.com", Some(access))); + for rendered in [direct, via_fetcher] { + assert!(!rendered.contains("tok-3f1"), "{rendered}"); + assert!(!rendered.contains("sec-9a2"), "{rendered}"); + } + } + /// Six concurrent fetches against a limit of 2 must never exceed two in-flight GETs. #[tokio::test] async fn concurrency_limit_bounds_in_flight_gets() { diff --git a/crates/stateless-r2/src/lib.rs b/crates/stateless-r2/src/lib.rs index bfbdbc49..64a92e12 100644 --- a/crates/stateless-r2/src/lib.rs +++ b/crates/stateless-r2/src/lib.rs @@ -15,8 +15,8 @@ //! - [`endpoint`] — parsing an R2 endpoint into the origin and `SigV4` canonical host; //! - [`client`] — a signed `PUT` helper that classifies the response into a small retry-friendly //! error set ([`client::R2Error`]); -//! - [`fetch`] — the retrying signed-`GET` witness-object fetcher shared by the readers -//! ([`fetch::R2ObjectFetcher`]). +//! - [`fetch`] — the retrying witness-object `GET` fetcher shared by the readers, over either the +//! signed S3 API or an unsigned Cloudflare custom domain ([`fetch::R2ObjectFetcher`]). //! //! ## Object retention //! diff --git a/crates/stateless-r2/src/sigv4.rs b/crates/stateless-r2/src/sigv4.rs index 2f0a4508..451dcbfc 100644 --- a/crates/stateless-r2/src/sigv4.rs +++ b/crates/stateless-r2/src/sigv4.rs @@ -172,6 +172,18 @@ fn hmac(key: &[u8], data: &[u8]) -> Vec { pub fn encode_uri_path(bucket: &str, key: &str) -> String { let mut path = String::from("/"); path.push_str(&encode_segment(bucket)); + path.push_str(&encode_key_path(key)); + path +} + +/// Percent-encodes an object key into an absolute URL path with **no bucket segment** — the +/// custom-domain layout, where the domain itself is bucket-scoped and objects live at +/// `https:///{key}`. +/// +/// Uses the same segment encoding as [`encode_uri_path`], so the two layouts can never +/// disagree on how a key's bytes appear on the wire. +pub fn encode_key_path(key: &str) -> String { + let mut path = String::new(); for segment in key.split('/') { path.push('/'); path.push_str(&encode_segment(segment)); @@ -222,6 +234,19 @@ mod tests { assert_eq!(path, "/b/a%20b/c%3Ad"); } + /// The custom-domain layout is the S3 layout minus the bucket segment — same segment + /// encoding, so the two can never disagree on how a key's bytes appear on the wire. + #[test] + fn encode_key_path_is_the_bucketless_uri_path() { + assert_eq!( + encode_key_path("block/2000_2999/2045.0x23758c4d28eed6"), + "/block/2000_2999/2045.0x23758c4d28eed6" + ); + for key in ["block/2000_2999/2045.0xabc", "a b/c:d"] { + assert_eq!(encode_uri_path("bucket", key), format!("/bucket{}", encode_key_path(key))); + } + } + #[test] fn sign_produces_authorization_and_amz_headers() { let signer = SigV4Signer::new("access".to_string(), "secret".to_string()); diff --git a/crates/stateless-test-utils/src/mock_r2.rs b/crates/stateless-test-utils/src/mock_r2.rs index 4351dbba..d04dc8b0 100644 --- a/crates/stateless-test-utils/src/mock_r2.rs +++ b/crates/stateless-test-utils/src/mock_r2.rs @@ -2,7 +2,8 @@ //! //! [`mock_r2`] is shared by the R2 reader tests across `stateless-r2` and both binaries' //! adapters, so the response scripting stays identical across them; [`mock_r2_held`] -//! serves the fetcher's concurrency/deadline tests. +//! serves the fetcher's concurrency/deadline tests; [`mock_r2_capturing`] adds request-head +//! capture for the tests that assert which headers went on the wire. use std::sync::{ Arc, @@ -50,20 +51,48 @@ pub async fn mock_r2_held(status: u16, hold: std::time::Duration) -> (String, Ar /// /// Returns the endpoint origin (`http://127.0.0.1:`) and the request counter. pub async fn mock_r2(responses: Vec<(u16, impl Into>)>) -> (String, Arc) { + let (endpoint, hits, _) = mock_r2_capturing(responses).await; + (endpoint, hits) +} + +/// Cap on a captured request head, so a client that never terminates its headers cannot grow +/// the buffer without bound. +const MAX_CAPTURED_HEAD_BYTES: usize = 64 * 1024; + +/// [`mock_r2`] that additionally captures each request's head (request line + headers, read +/// up to the blank line that terminates it), for tests asserting the wire shape — the request +/// path and which auth headers were (or were not) sent. +pub async fn mock_r2_capturing( + responses: Vec<(u16, impl Into>)>, +) -> (String, Arc, Arc>>) { let responses: Vec<(u16, Vec)> = responses.into_iter().map(|(status, body)| (status, body.into())).collect(); let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let endpoint = format!("http://{}", listener.local_addr().unwrap()); let hits = Arc::new(AtomicUsize::new(0)); + let heads = Arc::new(std::sync::Mutex::new(Vec::new())); let counter = hits.clone(); + let captured = heads.clone(); tokio::spawn(async move { loop { let Ok((mut sock, _)) = listener.accept().await else { return }; let n = counter.fetch_add(1, Ordering::SeqCst); let (status, body) = &responses[n.min(responses.len() - 1)]; - // Drain the request head before replying. + // Read to the header terminator rather than taking whatever one read returned: + // callers assert on the *absence* of headers, and a partial capture cannot tell + // "never sent" from "not in that segment". Bounded so a client that never sends + // the blank line cannot wedge the accept loop. + let mut head_bytes = Vec::new(); let mut buf = [0u8; 4096]; - let _ = sock.read(&mut buf).await; + while !head_bytes.windows(4).any(|w| w == b"\r\n\r\n") && + head_bytes.len() < MAX_CAPTURED_HEAD_BYTES + { + match sock.read(&mut buf).await { + Ok(0) | Err(_) => break, + Ok(read) => head_bytes.extend_from_slice(&buf[..read]), + } + } + captured.lock().unwrap().push(String::from_utf8_lossy(&head_bytes).into_owned()); // The reason phrase is never interpreted; `location` matters only to // redirects-not-followed tests. let head = format!( @@ -76,5 +105,5 @@ pub async fn mock_r2(responses: Vec<(u16, impl Into>)>) -> (String, Arc< let _ = sock.write_all(body).await; } }); - (endpoint, hits) + (endpoint, hits, heads) }