refactor: split the validator's R2 witness cap from the RPC one

[flyq]

Summary

--witness-max-concurrent-requests sized two unrelated things on the validator: the RPC witness path and, under --witness-source r2, the R2 GETs. The trace server has kept those apart since R2 landed there; this brings the validator to the same shape.

• New --r2-max-concurrent-requests (env STATELESS_VALIDATOR_R2_MAX_CONCURRENT_REQUESTS) caps R2 GETs.
• --witness-max-concurrent-requests now sizes the RPC witness path only.
• The --r2-connections ceiling check moves to the new flag, so the per-connection share is computed against the budget that actually feeds those connections.
• The three docstrings that still named the old flag as the R2 cap — the --r2-connections help, R2WitnessClient::new, and the r2_connections gauge doc — now name the new one (review finding, both bots).
• The configured cap is retained on R2WitnessClient (max_concurrent_requests()) and logged in the two R2 startup lines, and a new test asserts which cap actually reaches the client on both target arms (review finding, human round: the guard test alone survived a revert of the wiring).

Why one number could not serve both

They are budgets against different services with different tolerances. The RPC cap sizes what we ask of someone else's node and is shared with the data endpoints; R2 is a different service that tolerates far higher parallelism. And under --witness-source r2 the RPC witness path is not used at all, so the flag was sizing a path that does not run.

The split is refused rather than made silently

An operator running R2 mode today writes the cap as --witness-max-concurrent-requests. 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:

--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
(env STATELESS_VALIDATOR_R2_MAX_CONCURRENT_REQUESTS) instead

Same treatment a leftover S3 credential gets, and for the same reason: silently ignoring configuration hides what is actually in effect. Under rpc the rule stays inert — there the old spelling still means exactly what it says. The message names the env spelling because r2-mode deployments configure through env files, and the guard comment records why both spellings together stay legal: one env template can feed rpc-mode and r2-mode roles alike, each mode reading only its own cap.

The refusal sits at the top of build_r2_client, beside validate_r2_flags, so it is mode-gated by its call site like every other R2 rule and a future R2-flavored witness source cannot skip it; the unit tests drive build_r2_client itself, so the wiring is covered, not just the predicate — including which cap reaches the client: with both spellings set it must be the R2 one, not the RPC one, pinned on the custom-domain and S3 arms alike (reverting either arm's constructor argument to the old field fails the test by value).

Also in this PR

chore(debug-trace-server): drop the unused alloy-op-evm dependency — dead since the single-home block-execution refactor (#171) removed its last use (OpAlloyReceiptBuilder); stateless-core remains the workspace's one user.

Testing

cargo test --workspace: 477 passed, 0 failed (3 new). fmt / clippy --all-targets --all-features / cargo sort clean.

Verified on a release binary against mainnet:

Case	Result
r2 mode + old spelling only	refused, error names both flags
r2 mode + new spelling	starts
rpc mode + old spelling	rule inert
--r2-connections 8 with r2 cap 4	refused
--r2-connections 8 with r2 cap 64	starts

Behavioural equivalence: a 2000-block 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 over the same endpoint (2001 blocks, 0 mismatches).

🤖 Generated with Claude Code

[mega-maxwell]

Claude review status

Living comment — rewritten in place. The review workflow keeps this single comment up to date instead of posting a new one each round, so it always describes the latest reviewed commit and the earlier text is intentionally gone. No reply is needed here; reply to a finding in its own review thread, and answer an open question in a reply on this PR. The next review round reconciles your answer.

✅ Review clean

Last reviewed: 0472616e..38d5f239 · updated 2026-08-27T01:41:08+00:00

New this round: 0 finding(s), 0 question(s) · Resolved this round: 1 · Open questions: 0

[claude]

Label check: this PR currently has no labels applied.

Based on the diff (adds a new --r2-max-concurrent-requests CLI flag/env var, splits R2 GET concurrency from the RPC witness path, updates docs and tests), this looks like a fit for enhancement (new configurable behavior), since none of the other available labels (bug, documentation, duplicate, good first issue, help wanted, invalid, question, wontfix, compat:seamless, agent) match as well.

Suggest adding enhancement.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d820f197ad

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[mega-maxwell]

⚠️ Review needs attention — 1 finding(s)

0 blocking · 0 should-fix · 1 suggestion(s) · 0 open question(s)

Reviewed head d820f197.

Findings without inline anchors:

• bin/stateless-validator/src/app.rs:172 — [Minor] Stale docs still name --witness-max-concurrent-requests as the R2 cap An operator following --help under --witness-source r2 sets --witness-max-concurrent-requests believing it caps R2; check_r2_concurrency_migration then refuses startup and points at the right flag, so it fails loudly rather than silently, but the misdirection wastes a deploy cycle and makes the new rule feel arbitrary. The metrics.rs docstring also misleads whoever writes the R2 dashboard divisor. Suggested fix: In all three docstrings, replace --witness-max-concurrent-requests with --r2-max-concurrent-requests (bin/stateless-validator/src/app.rs:172, bin/stateless-validator/src/r2_witness.rs:139, bin/stateless-validator/src/metrics.rs:277). The trace-server r2_connections doc at bin/debug-trace-server/src/main.rs:509 already has the target phrasing.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ded81c1e8c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[vincent-k2026]

The split and the fail-closed migration are both right — refusing the pre-split spelling instead of silently dropping the cap is the correct call for a mode with no RPC fallback, and putting the rule at the top of build_r2_client keeps it gated by the same call site as every other R2 rule.

Should fix: the one behavioural change in this PR has no test that discriminates

What actually changes behaviour is bin/stateless-validator/src/app.rs:503 and :524 — args.witness_max_concurrent_requests → args.r2_max_concurrent_requests. Revert just those two lines and r2_mode_refuses_the_pre_split_concurrency_spelling still passes: it asserts the guard fires, that each spelling lands on its own field, and that build_r2_client returns Ok. None of that observes which value reaches the semaphore.

R2WitnessClient only exposes origin() / connections() / target_label(), so the assertion can't be written today. Adding max_concurrent_requests() -> Option<usize> next to connections() (same kind of read-only transport property) would let the test assert that --witness-max-concurrent-requests 16 --r2-max-concurrent-requests 48 builds a client capped at 48, not 16 — a positive control that also pins the "both set" semantics.

Second: parse_r2() only builds the custom-domain target, so the S3 arm at app.rs:524 — the same rewiring, second occurrence — is untested. A variant with --r2-endpoint + credentials is cheap.

Non-blocking

• The refusal message names only the CLI flags. Given the env-file deploy shape documented in AGENTS.md, an operator in r2 mode most likely set STATELESS_VALIDATOR_WITNESS_MAX_CONCURRENT_REQUESTS; naming STATELESS_VALIDATOR_R2_MAX_CONCURRENT_REQUESTS in the message saves a "that flag doesn't exist" round trip.
• Setting both is accepted, and in r2 mode the RPC one is then silently ignored — the exact thing the guard exists to prevent. I think the trade-off is right (setting both proves the operator knows about the split), but it's a deliberate asymmetry and deserves a why line beside the guard, or the next reader files it as an oversight.
• This breaks r2-mode startup for anyone who doesn't rewrite their config. enhancement undersells it — a compat: label plus a release note fits better.