From d820f197ada4f5748348fcd37ac2a140e1ffb932 Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Tue, 25 Aug 2026 23:42:12 +0800 Subject: [PATCH 1/5] refactor(validator): split the R2 witness cap from the RPC one `--witness-max-concurrent-requests` sized two unrelated things: the RPC witness path and, under `--witness-source r2`, the R2 GETs. They are budgets against different services with different tolerances -- the RPC gateway is shared with the data endpoints and sizes what we ask of someone else's node, while R2 tolerates far higher parallelism -- so one number could not be right for both. The trace server has kept them apart as `--witness-max-concurrent-requests` and `--r2-max-concurrent-requests` since R2 landed there; this brings the validator to the same shape. `--r2-max-concurrent-requests` (env `STATELESS_VALIDATOR_R2_MAX_CONCURRENT_REQUESTS`) now caps R2 GETs, and `--witness-max-concurrent-requests` sizes the RPC witness path only. The `--r2-connections` ceiling check moves with it, so the per-connection share is computed against the budget that actually feeds those connections. The split cannot be silent. An operator running R2 mode today writes the cap as `--witness-max-concurrent-requests`, and carrying that spelling forward would leave R2 uncapped -- in a mode with no RPC fallback, so the fetcher would aim its whole in-flight window at the bucket. Under `--witness-source r2`, the old spelling without the new one is therefore refused at startup by name, the way a leftover S3 credential is; the error says which flag to set instead. Under `rpc` the rule stays inert, where the old spelling still means exactly what it says. The check lives in `check_r2_concurrency_migration` rather than inline in `run` so a test can drive it: the existing tests note that `validate_r2_flags` is unreachable from the parse layer, and this rule would have inherited that blind spot. Verified on a real binary against mainnet: the guard fires in r2 mode and stays inert in rpc mode; `--r2-connections` is rejected above the new cap and accepted below it; and a 2000-block equivalence run over the S3 endpoint with the new flag produces a chain identical, block hash for block hash, to the same range validated over the RPC witness path and to the pre-split run. Co-Authored-By: Claude Opus 4.8 --- AGENTS.md | 2 +- bin/stateless-validator/src/app.rs | 57 ++++++++++++---- bin/stateless-validator/src/lib.rs | 3 +- bin/stateless-validator/tests/integration.rs | 68 ++++++++++++++++++++ 4 files changed, 117 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 306832ea..e9b176f9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -150,7 +150,7 @@ The pick is work-conserving: a connection with a free permit, searched from a ro `--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. +The validator splits the two the same way: `--r2-max-concurrent-requests` caps R2 GETs while `--witness-max-concurrent-requests` sizes only the RPC witness path, so a budget written for one service cannot silently become the other's. They were one flag until the split, and carrying the old spelling into `--witness-source r2` is refused at startup by name rather than left to drop the cap — that mode has no RPC fallback, so an uncapped fetcher aims its whole in-flight window at the bucket. `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. diff --git a/bin/stateless-validator/src/app.rs b/bin/stateless-validator/src/app.rs index db7fa462..6a55ead6 100644 --- a/bin/stateless-validator/src/app.rs +++ b/bin/stateless-validator/src/app.rs @@ -213,16 +213,25 @@ pub struct CommandLineArgs { #[clap(long, env = "STATELESS_VALIDATOR_DATA_MAX_CONCURRENT_REQUESTS")] pub data_max_concurrent_requests: Option, - /// 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. + /// Maximum concurrent in-flight RPC witness fetches, independent of the data cap. Omit + /// for unlimited. Applies to `--witness-source rpc` only; R2 GETs are capped by + /// `--r2-max-concurrent-requests`, which is a separate budget against a separate service. #[clap(long, env = "STATELESS_VALIDATOR_WITNESS_MAX_CONCURRENT_REQUESTS")] pub witness_max_concurrent_requests: Option, + /// Maximum concurrent in-flight R2 witness GETs. Omit for unlimited. Deliberately + /// separate from `--witness-max-concurrent-requests`: that one sizes what we ask of the + /// RPC gateway, while R2 is a different service that tolerates far higher parallelism, + /// and under `--witness-source r2` the RPC witness path is not used at all. + /// + /// Against `--r2-custom-domain` this is what bounds the GETs multiplexed onto each HTTP/2 + /// connection, so keep the per-connection share (this value divided by + /// `--r2-connections`) 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_R2_MAX_CONCURRENT_REQUESTS")] + pub r2_max_concurrent_requests: Option, + /// Fetcher caught-up poll interval (milliseconds). Also rate-limits `eth_blockNumber`. /// Lower values reduce tip-following lag at the cost of more RPC traffic when caught up. #[clap(long, env = "STATELESS_VALIDATOR_POLL_INTERVAL_MS")] @@ -341,6 +350,7 @@ pub async fn run() -> Result<()> { straight from the R2 bucket, and there is no RPC witness fallback" ); } + check_r2_concurrency_migration(&args)?; let timeouts = stateless_r2::fetch::FetchTimeouts { per_attempt: per_attempt_timeout, connect: args @@ -484,7 +494,7 @@ fn build_r2_client( access, timeouts, retry, - args.witness_max_concurrent_requests, + args.r2_max_concurrent_requests, connections, )?; metrics::record_r2_connections(client.connections()); @@ -505,7 +515,7 @@ fn build_r2_client( args.r2_secret_access_key.as_ref().expect("S3 target").as_ref().to_string(), timeouts, retry, - args.witness_max_concurrent_requests, + args.r2_max_concurrent_requests, )?; info!( endpoint = %client.origin(), @@ -519,6 +529,31 @@ fn build_r2_client( Ok(client) } +/// Rejects the pre-split spelling of the R2 concurrency cap. +/// +/// `--witness-max-concurrent-requests` used to cap R2 GETs as well as RPC witness calls. +/// Now that the two budgets are separate, carrying the old spelling forward would leave R2 +/// uncapped -- and `--witness-source r2` has no RPC fallback, so the fetcher would point its +/// whole in-flight window at the bucket. Silently dropping a cap an operator wrote down is +/// worse than refusing to start, so this refuses by name, the way a leftover S3 credential is. +/// +/// Inert outside `--witness-source r2`: under `rpc` the old spelling still means exactly what +/// it says, and every `--r2-*` flag is unread. +pub fn check_r2_concurrency_migration(args: &CommandLineArgs) -> Result<()> { + if args.witness_source != WitnessSource::R2 { + return Ok(()); + } + if args.witness_max_concurrent_requests.is_some() && args.r2_max_concurrent_requests.is_none() { + return Err(eyre::eyre!( + "--witness-max-concurrent-requests no longer caps R2 GETs under --witness-source \ + r2; it now sizes the RPC witness path only. Set --r2-max-concurrent-requests to \ + the value you want R2 capped at (and unset --witness-max-concurrent-requests, \ + which is unread in this mode)." + )); + } + Ok(()) +} + /// This binary's `--r2-*` flags, in the spellings its operators use. fn r2_flags(args: &CommandLineArgs) -> R2Flags<'_> { R2Flags { @@ -540,8 +575,8 @@ fn r2_flags(args: &CommandLineArgs) -> R2Flags<'_> { ), connections: R2Flag::new("--r2-connections", args.r2_connections.as_deref()), max_concurrent_requests: R2CountFlag::new( - "--witness-max-concurrent-requests", - args.witness_max_concurrent_requests, + "--r2-max-concurrent-requests", + args.r2_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 diff --git a/bin/stateless-validator/src/lib.rs b/bin/stateless-validator/src/lib.rs index a5a8ea2b..3bb7fe5a 100644 --- a/bin/stateless-validator/src/lib.rs +++ b/bin/stateless-validator/src/lib.rs @@ -11,7 +11,8 @@ pub(crate) mod validator_db; pub(crate) mod workers; pub use app::{ - CommandLineArgs, VALIDATOR_DB_FILENAME, WitnessSource, load_or_create_chain_spec, run, + CommandLineArgs, VALIDATOR_DB_FILENAME, WitnessSource, check_r2_concurrency_migration, + load_or_create_chain_spec, run, }; pub use chain_sync::{ValidationTask, ValidatorFetcher, ValidatorHooks, ValidatorProcessor}; pub use r2_witness::{R2WitnessClient, R2WitnessError}; diff --git a/bin/stateless-validator/tests/integration.rs b/bin/stateless-validator/tests/integration.rs index 94f1b12e..e78f2174 100644 --- a/bin/stateless-validator/tests/integration.rs +++ b/bin/stateless-validator/tests/integration.rs @@ -134,6 +134,74 @@ fn witness_max_concurrent_requests_flag_and_env() { ); } +#[test] +fn r2_max_concurrent_requests_flag_and_env() { + assert_optional_numeric_flag::( + "--r2-max-concurrent-requests", + "STATELESS_VALIDATOR_R2_MAX_CONCURRENT_REQUESTS", + |a| a.r2_max_concurrent_requests, + ); +} + +/// The R2 and RPC witness caps are separate budgets against separate services, so setting one +/// must not move the other. +#[test] +fn witness_and_r2_concurrency_caps_are_independent() { + let _guard = stateless_test_utils::env::env_lock(); + let parse = |extra: &[&str]| CommandLineArgs::try_parse_from(BASE_ARGS.iter().chain(extra)); + + let a = parse(&["--witness-max-concurrent-requests", "7"]).unwrap(); + assert_eq!(a.witness_max_concurrent_requests, Some(7)); + assert_eq!(a.r2_max_concurrent_requests, None); + + let b = parse(&["--r2-max-concurrent-requests", "9"]).unwrap(); + assert_eq!(b.r2_max_concurrent_requests, Some(9)); + assert_eq!(b.witness_max_concurrent_requests, None); + + let both = + parse(&["--witness-max-concurrent-requests", "7", "--r2-max-concurrent-requests", "9"]) + .unwrap(); + assert_eq!(both.witness_max_concurrent_requests, Some(7)); + assert_eq!(both.r2_max_concurrent_requests, Some(9)); +} + +/// Carrying the pre-split spelling into `--witness-source r2` must fail by name rather than +/// leave R2 uncapped: that mode has no RPC fallback, so an uncapped fetcher aims its whole +/// in-flight window at the bucket. +#[test] +fn r2_mode_refuses_the_pre_split_concurrency_spelling() { + let _guard = stateless_test_utils::env::env_lock(); + let parse = |extra: &[&str]| { + CommandLineArgs::try_parse_from(BASE_ARGS.iter().chain(extra)).expect("parses") + }; + + let stale = parse(&["--witness-source", "r2", "--witness-max-concurrent-requests", "48"]); + let err = stateless_validator::check_r2_concurrency_migration(&stale) + .expect_err("the old spelling must be refused in r2 mode"); + let msg = err.to_string(); + assert!(msg.contains("--witness-max-concurrent-requests"), "{msg}"); + assert!(msg.contains("--r2-max-concurrent-requests"), "{msg}"); + + // Migrated: the new spelling alone is accepted. + let migrated = parse(&["--witness-source", "r2", "--r2-max-concurrent-requests", "48"]); + assert!(stateless_validator::check_r2_concurrency_migration(&migrated).is_ok()); + + // Both set is accepted too -- the RPC cap is simply unread in this mode. + let both = parse(&[ + "--witness-source", + "r2", + "--witness-max-concurrent-requests", + "16", + "--r2-max-concurrent-requests", + "48", + ]); + assert!(stateless_validator::check_r2_concurrency_migration(&both).is_ok()); + + // Under `rpc` the old spelling still means what it says, so the rule stays inert. + let rpc = parse(&["--witness-source", "rpc", "--witness-max-concurrent-requests", "48"]); + assert!(stateless_validator::check_r2_concurrency_migration(&rpc).is_ok()); +} + #[test] fn tip_buffer_flag_and_env() { assert_optional_numeric_flag::("--tip-buffer", "STATELESS_VALIDATOR_TIP_BUFFER", |a| { From ded81c1e8ce5d5020984c9e98767fd7c37817d1a Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 26 Aug 2026 15:47:19 +0800 Subject: [PATCH 2/5] refactor(validator): fold the R2 cap migration check into build_r2_client The pre-split-spelling refusal now sits beside validate_r2_flags, where every other R2-mode rule lives, gated by call site instead of an internal mode guard that was dead at its only production call site. Drops the test-only pub export, moves the tests in-file to drive build_r2_client itself, deletes the independence test that clap guarantees by construction, and restyles the error to the shared flag-named convention. Co-Authored-By: Claude Fable 5 --- bin/stateless-validator/src/app.rs | 103 ++++++++++++++----- bin/stateless-validator/src/lib.rs | 3 +- bin/stateless-validator/tests/integration.rs | 59 ----------- 3 files changed, 77 insertions(+), 88 deletions(-) diff --git a/bin/stateless-validator/src/app.rs b/bin/stateless-validator/src/app.rs index 6a55ead6..927d3d2d 100644 --- a/bin/stateless-validator/src/app.rs +++ b/bin/stateless-validator/src/app.rs @@ -215,7 +215,7 @@ pub struct CommandLineArgs { /// Maximum concurrent in-flight RPC witness fetches, independent of the data cap. Omit /// for unlimited. Applies to `--witness-source rpc` only; R2 GETs are capped by - /// `--r2-max-concurrent-requests`, which is a separate budget against a separate service. + /// `--r2-max-concurrent-requests`. #[clap(long, env = "STATELESS_VALIDATOR_WITNESS_MAX_CONCURRENT_REQUESTS")] pub witness_max_concurrent_requests: Option, @@ -350,7 +350,6 @@ pub async fn run() -> Result<()> { straight from the R2 bucket, and there is no RPC witness fallback" ); } - check_r2_concurrency_migration(&args)?; let timeouts = stateless_r2::fetch::FetchTimeouts { per_attempt: per_attempt_timeout, connect: args @@ -469,6 +468,16 @@ fn build_r2_client( timeouts: stateless_r2::fetch::FetchTimeouts, retry: BackoffPolicy, ) -> Result { + // `--witness-max-concurrent-requests` capped R2 GETs too before the caps were split. + // Refuse the pre-split spelling by name rather than leave R2 uncapped: this mode has no + // RPC fallback, so an uncapped fetcher aims its whole in-flight window at the bucket. + if args.witness_max_concurrent_requests.is_some() && args.r2_max_concurrent_requests.is_none() { + return Err(eyre::eyre!( + "--witness-max-concurrent-requests no longer caps R2 GETs under --witness-source \ + r2 (it now sizes only the RPC witness path): set --r2-max-concurrent-requests \ + instead" + )); + } // 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. @@ -529,31 +538,6 @@ fn build_r2_client( Ok(client) } -/// Rejects the pre-split spelling of the R2 concurrency cap. -/// -/// `--witness-max-concurrent-requests` used to cap R2 GETs as well as RPC witness calls. -/// Now that the two budgets are separate, carrying the old spelling forward would leave R2 -/// uncapped -- and `--witness-source r2` has no RPC fallback, so the fetcher would point its -/// whole in-flight window at the bucket. Silently dropping a cap an operator wrote down is -/// worse than refusing to start, so this refuses by name, the way a leftover S3 credential is. -/// -/// Inert outside `--witness-source r2`: under `rpc` the old spelling still means exactly what -/// it says, and every `--r2-*` flag is unread. -pub fn check_r2_concurrency_migration(args: &CommandLineArgs) -> Result<()> { - if args.witness_source != WitnessSource::R2 { - return Ok(()); - } - if args.witness_max_concurrent_requests.is_some() && args.r2_max_concurrent_requests.is_none() { - return Err(eyre::eyre!( - "--witness-max-concurrent-requests no longer caps R2 GETs under --witness-source \ - r2; it now sizes the RPC witness path only. Set --r2-max-concurrent-requests to \ - the value you want R2 capped at (and unset --witness-max-concurrent-requests, \ - which is unread in this mode)." - )); - } - Ok(()) -} - /// This binary's `--r2-*` flags, in the spellings its operators use. fn r2_flags(args: &CommandLineArgs) -> R2Flags<'_> { R2Flags { @@ -585,3 +569,68 @@ fn r2_flags(args: &CommandLineArgs) -> R2Flags<'_> { tuning: &[], } } + +#[cfg(test)] +mod tests { + use super::*; + + /// Argv for `--witness-source r2` with a loopback custom-domain target, so + /// [`build_r2_client`] — the seam every R2-mode rule is gated behind — runs the rules + /// from the path production takes. + fn parse_r2(extra: &[&str]) -> CommandLineArgs { + let argv = [ + "stateless-validator", + "--data-dir", + "/tmp/x", + "--rpc-endpoint", + "http://rpc", + "--witness-source", + "r2", + "--r2-custom-domain", + "http://127.0.0.1:9000", + ]; + CommandLineArgs::try_parse_from(argv.iter().chain(extra)).expect("parses") + } + + fn build(args: &CommandLineArgs) -> Result { + let timeouts = stateless_r2::fetch::FetchTimeouts { + per_attempt: Duration::from_secs(1), + connect: Duration::from_secs(1), + }; + let retry = + BackoffPolicy { initial: Duration::from_millis(1), max: Duration::from_millis(1) }; + build_r2_client(args, timeouts, retry) + } + + /// Carrying the pre-split spelling of the R2 concurrency cap into `--witness-source r2` + /// must fail by name rather than leave R2 uncapped: that mode has no RPC fallback, so an + /// uncapped fetcher aims its whole in-flight window at the bucket. Outside r2 mode the + /// rule is unreachable by construction — `build_r2_client` is only called from the + /// `WitnessSource::R2` arm, the same call-site gating as every other R2 rule. + #[test] + fn r2_mode_refuses_the_pre_split_concurrency_spelling() { + let _guard = stateless_test_utils::env::env_lock(); + + let stale = parse_r2(&["--witness-max-concurrent-requests", "48"]); + let msg = + build(&stale).expect_err("the old spelling must be refused in r2 mode").to_string(); + assert!(msg.contains("--witness-max-concurrent-requests"), "{msg}"); + assert!(msg.contains("--r2-max-concurrent-requests"), "{msg}"); + + // Migrated: the new spelling alone is accepted. + build(&parse_r2(&["--r2-max-concurrent-requests", "48"])) + .expect("migrated spelling builds"); + + // Both set is accepted — the RPC cap is simply unread in this mode — and each + // spelling lands on its own field. + let both = parse_r2(&[ + "--witness-max-concurrent-requests", + "16", + "--r2-max-concurrent-requests", + "48", + ]); + assert_eq!(both.witness_max_concurrent_requests, Some(16)); + assert_eq!(both.r2_max_concurrent_requests, Some(48)); + build(&both).expect("both caps set builds"); + } +} diff --git a/bin/stateless-validator/src/lib.rs b/bin/stateless-validator/src/lib.rs index 3bb7fe5a..a5a8ea2b 100644 --- a/bin/stateless-validator/src/lib.rs +++ b/bin/stateless-validator/src/lib.rs @@ -11,8 +11,7 @@ pub(crate) mod validator_db; pub(crate) mod workers; pub use app::{ - CommandLineArgs, VALIDATOR_DB_FILENAME, WitnessSource, check_r2_concurrency_migration, - load_or_create_chain_spec, run, + CommandLineArgs, VALIDATOR_DB_FILENAME, WitnessSource, load_or_create_chain_spec, run, }; pub use chain_sync::{ValidationTask, ValidatorFetcher, ValidatorHooks, ValidatorProcessor}; pub use r2_witness::{R2WitnessClient, R2WitnessError}; diff --git a/bin/stateless-validator/tests/integration.rs b/bin/stateless-validator/tests/integration.rs index e78f2174..b0e839ec 100644 --- a/bin/stateless-validator/tests/integration.rs +++ b/bin/stateless-validator/tests/integration.rs @@ -143,65 +143,6 @@ fn r2_max_concurrent_requests_flag_and_env() { ); } -/// The R2 and RPC witness caps are separate budgets against separate services, so setting one -/// must not move the other. -#[test] -fn witness_and_r2_concurrency_caps_are_independent() { - let _guard = stateless_test_utils::env::env_lock(); - let parse = |extra: &[&str]| CommandLineArgs::try_parse_from(BASE_ARGS.iter().chain(extra)); - - let a = parse(&["--witness-max-concurrent-requests", "7"]).unwrap(); - assert_eq!(a.witness_max_concurrent_requests, Some(7)); - assert_eq!(a.r2_max_concurrent_requests, None); - - let b = parse(&["--r2-max-concurrent-requests", "9"]).unwrap(); - assert_eq!(b.r2_max_concurrent_requests, Some(9)); - assert_eq!(b.witness_max_concurrent_requests, None); - - let both = - parse(&["--witness-max-concurrent-requests", "7", "--r2-max-concurrent-requests", "9"]) - .unwrap(); - assert_eq!(both.witness_max_concurrent_requests, Some(7)); - assert_eq!(both.r2_max_concurrent_requests, Some(9)); -} - -/// Carrying the pre-split spelling into `--witness-source r2` must fail by name rather than -/// leave R2 uncapped: that mode has no RPC fallback, so an uncapped fetcher aims its whole -/// in-flight window at the bucket. -#[test] -fn r2_mode_refuses_the_pre_split_concurrency_spelling() { - let _guard = stateless_test_utils::env::env_lock(); - let parse = |extra: &[&str]| { - CommandLineArgs::try_parse_from(BASE_ARGS.iter().chain(extra)).expect("parses") - }; - - let stale = parse(&["--witness-source", "r2", "--witness-max-concurrent-requests", "48"]); - let err = stateless_validator::check_r2_concurrency_migration(&stale) - .expect_err("the old spelling must be refused in r2 mode"); - let msg = err.to_string(); - assert!(msg.contains("--witness-max-concurrent-requests"), "{msg}"); - assert!(msg.contains("--r2-max-concurrent-requests"), "{msg}"); - - // Migrated: the new spelling alone is accepted. - let migrated = parse(&["--witness-source", "r2", "--r2-max-concurrent-requests", "48"]); - assert!(stateless_validator::check_r2_concurrency_migration(&migrated).is_ok()); - - // Both set is accepted too -- the RPC cap is simply unread in this mode. - let both = parse(&[ - "--witness-source", - "r2", - "--witness-max-concurrent-requests", - "16", - "--r2-max-concurrent-requests", - "48", - ]); - assert!(stateless_validator::check_r2_concurrency_migration(&both).is_ok()); - - // Under `rpc` the old spelling still means what it says, so the rule stays inert. - let rpc = parse(&["--witness-source", "rpc", "--witness-max-concurrent-requests", "48"]); - assert!(stateless_validator::check_r2_concurrency_migration(&rpc).is_ok()); -} - #[test] fn tip_buffer_flag_and_env() { assert_optional_numeric_flag::("--tip-buffer", "STATELESS_VALIDATOR_TIP_BUFFER", |a| { From 0236343f9c69181f540851de7e4a128becee9f51 Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 26 Aug 2026 15:54:35 +0800 Subject: [PATCH 3/5] docs(validator): finish the cap rename in the three docstrings that still name the old flag The --r2-connections help, the R2WitnessClient::new doc, and the r2_connections gauge doc all still pointed at --witness-max-concurrent-requests as the R2 cap; following the help under --witness-source r2 would walk an operator straight into the migration refusal. Co-Authored-By: Claude Fable 5 --- bin/stateless-validator/src/app.rs | 2 +- bin/stateless-validator/src/metrics.rs | 2 +- bin/stateless-validator/src/r2_witness.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/stateless-validator/src/app.rs b/bin/stateless-validator/src/app.rs index 927d3d2d..db82f59f 100644 --- a/bin/stateless-validator/src/app.rs +++ b/bin/stateless-validator/src/app.rs @@ -169,7 +169,7 @@ pub struct CommandLineArgs { /// 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 + /// `--r2-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. diff --git a/bin/stateless-validator/src/metrics.rs b/bin/stateless-validator/src/metrics.rs index 1b344c90..6be652b4 100644 --- a/bin/stateless-validator/src/metrics.rs +++ b/bin/stateless-validator/src/metrics.rs @@ -274,7 +274,7 @@ pub fn record_r2_negotiated_version(version: &'static str) { /// 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 +/// budget, so a dashboard reads it against `--r2-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) { diff --git a/bin/stateless-validator/src/r2_witness.rs b/bin/stateless-validator/src/r2_witness.rs index f0aacb65..217041ff 100644 --- a/bin/stateless-validator/src/r2_witness.rs +++ b/bin/stateless-validator/src/r2_witness.rs @@ -136,7 +136,7 @@ impl R2WitnessClient { /// `--rpc-initial-backoff-ms` / `--rpc-max-backoff-ms`, so one pair of flags tunes both /// paths. `max_concurrent_requests` caps the number of GETs in flight at once (`None` = /// unlimited, `Some(0)` clamps to 1 — same semantics as the RPC witness semaphore; in R2 - /// mode this client is the only enforcement of `--witness-max-concurrent-requests`). Fails + /// mode this client is the only enforcement of `--r2-max-concurrent-requests`). Fails /// if the endpoint is not a bare `scheme://host[:port]` origin or the HTTP client cannot be /// built. pub fn new( From 0472616ec84d05b8589d7f16909a0eea1165e2da Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 26 Aug 2026 16:03:05 +0800 Subject: [PATCH 4/5] chore(debug-trace-server): drop the unused alloy-op-evm dependency Dead since the single-home block-execution refactor (#171) removed the last use, OpAlloyReceiptBuilder; stateless-core remains the workspace's one user. Co-Authored-By: Claude Fable 5 --- Cargo.lock | 1 - bin/debug-trace-server/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 74cdabbf..2ec67af9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1892,7 +1892,6 @@ dependencies = [ "alloy-evm", "alloy-genesis", "alloy-network", - "alloy-op-evm", "alloy-primitives", "alloy-rpc-types-eth", "alloy-rpc-types-trace", diff --git a/bin/debug-trace-server/Cargo.toml b/bin/debug-trace-server/Cargo.toml index c962a45d..aa3d704f 100644 --- a/bin/debug-trace-server/Cargo.toml +++ b/bin/debug-trace-server/Cargo.toml @@ -12,7 +12,6 @@ exclude.workspace = true alloy-consensus.workspace = true alloy-evm.workspace = true alloy-genesis.workspace = true -alloy-op-evm.workspace = true alloy-primitives.workspace = true alloy-rpc-types-eth.workspace = true alloy-rpc-types-trace.workspace = true From 38d5f239fe6e160985924a3af79765fe4ca98955 Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Thu, 27 Aug 2026 09:38:19 +0800 Subject: [PATCH 5/5] test(validator): pin which cap reaches the R2 client, on both target arms The migration guard fires on the flags alone, so its test passed even with the constructors still wired to the old field. Retain the configured cap on R2WitnessClient (the fetcher decomposes it into per-connection permits and cannot report it back), expose it, and assert that with both spellings set the client is capped by --r2-max-concurrent-requests, not the RPC cap - driven through build_r2_client on the custom-domain and S3 arms alike, with the uncapped default pinned too. Reverting either arm now fails by value. Also from the review round: the configured cap joins the two R2 startup log lines, the refusal names the env spelling (the deployments the guard exists for configure through env files), and the guard comment records why setting both caps stays legal (one env template can feed rpc- and r2-mode roles). Co-Authored-By: Claude Fable 5 --- bin/stateless-validator/src/app.rs | 64 ++++++++++++++++++++--- bin/stateless-validator/src/r2_witness.rs | 13 ++++- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/bin/stateless-validator/src/app.rs b/bin/stateless-validator/src/app.rs index db82f59f..29faec83 100644 --- a/bin/stateless-validator/src/app.rs +++ b/bin/stateless-validator/src/app.rs @@ -471,11 +471,15 @@ fn build_r2_client( // `--witness-max-concurrent-requests` capped R2 GETs too before the caps were split. // Refuse the pre-split spelling by name rather than leave R2 uncapped: this mode has no // RPC fallback, so an uncapped fetcher aims its whole in-flight window at the bucket. + // Both spellings together stay legal — one env template can feed rpc-mode and r2-mode + // roles alike, each mode reading only its own cap — so only old-spelling-alone is refused. + // The message names the env spelling too: the deployments this guard exists for configure + // through env files, where the flag spelling alone costs a name-translation round trip. if args.witness_max_concurrent_requests.is_some() && args.r2_max_concurrent_requests.is_none() { return Err(eyre::eyre!( "--witness-max-concurrent-requests no longer caps R2 GETs under --witness-source \ r2 (it now sizes only the RPC witness path): set --r2-max-concurrent-requests \ - instead" + (env STATELESS_VALIDATOR_R2_MAX_CONCURRENT_REQUESTS) instead" )); } // Every coherence rule lives in the shared validator, so the reads below rest on an @@ -511,6 +515,7 @@ fn build_r2_client( domain = %client.origin(), cf_access, connections = client.connections(), + max_concurrent_requests = ?client.max_concurrent_requests(), "Witness source: R2 (custom domain)" ); client @@ -529,6 +534,7 @@ fn build_r2_client( info!( endpoint = %client.origin(), bucket = args.r2_bucket.as_deref().unwrap_or_default(), + max_concurrent_requests = ?client.max_concurrent_requests(), "Witness source: R2 (direct S3)" ); client @@ -574,10 +580,25 @@ fn r2_flags(args: &CommandLineArgs) -> R2Flags<'_> { mod tests { use super::*; - /// Argv for `--witness-source r2` with a loopback custom-domain target, so - /// [`build_r2_client`] — the seam every R2-mode rule is gated behind — runs the rules - /// from the path production takes. - fn parse_r2(extra: &[&str]) -> CommandLineArgs { + /// A loopback custom-domain target, the default shape for rules that are target-agnostic. + const CUSTOM_DOMAIN_TARGET: &[&str] = &["--r2-custom-domain", "http://127.0.0.1:9000"]; + + /// The signed S3 target: the endpoint plus its credential quad. + const S3_TARGET: &[&str] = &[ + "--r2-endpoint", + "https://acc.r2.cloudflarestorage.com", + "--r2-bucket", + "witness", + "--r2-access-key-id", + "key-id", + "--r2-secret-access-key", + "secret", + ]; + + /// Argv for `--witness-source r2` with the given target flags, so [`build_r2_client`] — + /// the seam every R2-mode rule is gated behind — runs the rules from the path production + /// takes. + fn parse_r2_with_target(target: &[&str], extra: &[&str]) -> CommandLineArgs { let argv = [ "stateless-validator", "--data-dir", @@ -586,10 +607,12 @@ mod tests { "http://rpc", "--witness-source", "r2", - "--r2-custom-domain", - "http://127.0.0.1:9000", ]; - CommandLineArgs::try_parse_from(argv.iter().chain(extra)).expect("parses") + CommandLineArgs::try_parse_from(argv.iter().chain(target).chain(extra)).expect("parses") + } + + fn parse_r2(extra: &[&str]) -> CommandLineArgs { + parse_r2_with_target(CUSTOM_DOMAIN_TARGET, extra) } fn build(args: &CommandLineArgs) -> Result { @@ -616,6 +639,9 @@ mod tests { build(&stale).expect_err("the old spelling must be refused in r2 mode").to_string(); assert!(msg.contains("--witness-max-concurrent-requests"), "{msg}"); assert!(msg.contains("--r2-max-concurrent-requests"), "{msg}"); + // The env spelling too: the deployments this guard exists for configure through env + // files, and the flag spelling alone would cost a name-translation round trip. + assert!(msg.contains("STATELESS_VALIDATOR_R2_MAX_CONCURRENT_REQUESTS"), "{msg}"); // Migrated: the new spelling alone is accepted. build(&parse_r2(&["--r2-max-concurrent-requests", "48"])) @@ -633,4 +659,26 @@ mod tests { assert_eq!(both.r2_max_concurrent_requests, Some(48)); build(&both).expect("both caps set builds"); } + + /// The migration guard fires on the flags alone, so its test above would still pass with + /// the constructors wired to the old field. This is the assertion that observes which cap + /// actually reaches the client — with both spellings set it must be the R2 one, not the + /// RPC one — on both target arms, plus the uncapped default, so a revert of either arm's + /// wiring fails here by value. + #[test] + fn the_r2_cap_not_the_rpc_one_reaches_the_client() { + let _guard = stateless_test_utils::env::env_lock(); + + for target in [CUSTOM_DOMAIN_TARGET, S3_TARGET] { + let both = parse_r2_with_target( + target, + &["--witness-max-concurrent-requests", "16", "--r2-max-concurrent-requests", "48"], + ); + let capped = build(&both).expect("both caps set builds"); + assert_eq!(capped.max_concurrent_requests(), Some(48), "{target:?}"); + + let uncapped = build(&parse_r2_with_target(target, &[])).expect("no caps builds"); + assert_eq!(uncapped.max_concurrent_requests(), None, "{target:?}"); + } + } } diff --git a/bin/stateless-validator/src/r2_witness.rs b/bin/stateless-validator/src/r2_witness.rs index 217041ff..fdec1685 100644 --- a/bin/stateless-validator/src/r2_witness.rs +++ b/bin/stateless-validator/src/r2_witness.rs @@ -103,6 +103,9 @@ impl R2WitnessError { #[derive(Debug)] pub struct R2WitnessClient { fetcher: R2ObjectFetcher, + /// The configured in-flight GET cap, retained here because the fetcher decomposes it into + /// per-connection permits and cannot report the configured value back. + max_concurrent_requests: Option, } /// The fetcher's pacing view of a `BackoffPolicy` — the adapter-layer conversion that keeps @@ -128,6 +131,12 @@ impl R2WitnessClient { self.fetcher.connections() } + /// The configured cap on in-flight GETs (`None` = unlimited; see [`Self::new`] for the + /// exact semantics), for startup logging. + pub fn max_concurrent_requests(&self) -> Option { + self.max_concurrent_requests + } + /// 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 @@ -158,7 +167,7 @@ impl R2WitnessClient { max_concurrent_requests, ) .map_err(|e| eyre::eyre!(e))?; - Ok(Self { fetcher }) + Ok(Self { fetcher, max_concurrent_requests }) } /// Builds a client that fetches unsigned through a Cloudflare custom domain fronting the @@ -182,7 +191,7 @@ impl R2WitnessClient { ) .map(|fetcher| fetcher.on_version_observed(metrics::record_r2_negotiated_version)) .map_err(|e| eyre::eyre!(e))?; - Ok(Self { fetcher }) + Ok(Self { fetcher, max_concurrent_requests }) } /// Fetches and decodes the witness for `(number, hash)` from R2.