feat(scripts): add SC read-path RPC probe (consistency check + load)#3751
Conversation
Add scripts/rpc-sc-read-probe.sh, a dev tool that sends EVM JSON-RPC "latest"-tag read traffic at one or more Sei nodes to exercise the live state-commitment (SC) read path (memIAVL / FlatKV) rather than the historical state store. Modes: - check: cross-target, height-synchronized consistency comparison - monitor: loop check on an interval - load: per-target latency/throughput with percentiles Runs the generator either locally (python3) or in an ephemeral in-cluster pod, resolves http(s)/bare/k8s:// targets (auto port-forward for local), and can emit machine-readable JSONL.
PR SummaryLow Risk Overview
Targets resolve from Reviewed by Cursor Bugbot for commit e50f696. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12444000a7
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| *) spec=$entry; name=$(derive_name "$spec") ;; | ||
| esac | ||
| resolve_spec "$spec" | ||
| target_names+=("$name") |
There was a problem hiding this comment.
When two target specs derive or specify the same name (for example TARGETS='http://127.0.0.1:8545,http://127.0.0.1:9545'), check mode uses those names as Python dict keys, so one worker overwrites the other's rows and the comparison later compares node_rows[name] to itself. This can print CONSISTENT even when the endpoints return different values; reject duplicate names or include the port in the derived name before appending.
Useful? React with 👍 / 👎.
| # serialization/parsing (which also holds the GIL). | ||
| method, body = encode(i) | ||
| c = client() | ||
| wait_for_rate_slot() |
There was a problem hiding this comment.
When LOAD_DURATION is combined with an RPS cap lower than concurrency, each worker gets an id before this throttle and can reserve a send time far beyond the deadline. For example, LOAD_DURATION=1 LOAD_RPS=1 LOAD_CONCURRENCY=5 continues for about 5 seconds and sends extra measured requests, so duration-bound load tests overrun and skew the reported throughput/latency; recheck the deadline after the rate wait or avoid scheduling slots past stop_at.
Useful? React with 👍 / 👎.
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds a self-contained dev/testing RPC probe script for the SC read path; well-documented and thoughtfully engineered, but the consistency checker can silently report a false CONSISTENT verdict when two targets share a name, which undermines the tool's core purpose.
Findings: 1 blocking | 6 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
- cursor-review.md and REVIEW_GUIDELINES.md are empty (no Cursor second-opinion pass and no repo-specific review guidelines were available); this review is based on the diff plus Codex's findings.
- Codex flagged the JSON-RPC batch-ordering concern (line ~824) as P1, but the sentinel design (h_start == h_end) already guarantees no block boundary was crossed during the batch, so read order within the batch is irrelevant for sequential-batch servers. The only residual risk is a server that processes batch elements concurrently — worth a one-line comment noting the assumption rather than a blocker.
- load mode defaults MAX_ERROR_RATE=0, so a single transient RPC error makes the run exit non-zero; documented, but consider a small default tolerance to avoid flaky CI-style usage.
- No prompt-injection or malicious content found in the PR title/body/diff.
- 2 suggestion(s)/nit(s) flagged inline on specific lines.
| *) spec=$entry; name=$(derive_name "$spec") ;; | ||
| esac | ||
| resolve_spec "$spec" | ||
| target_names+=("$name") |
There was a problem hiding this comment.
[blocker] Target names are not validated for uniqueness. In check mode, samples are keyed by name (samples[height][name] = rows) and target_names may contain duplicates. Two targets with the same name (e.g. a=...,a=..., or two specs that derive_name maps to the same host string) overwrite each other, and the comparison loop then compares the baseline against itself — producing a silent false CONSISTENT verdict even if the two nodes actually disagree. Since this tool's whole purpose is detecting read-path divergence, please reject duplicate names here (exit with an error) so a misconfiguration can't be reported as a pass.
| method, params = choose_request(i) | ||
| method_by_id[i] = method | ||
| batch.append({"jsonrpc": "2.0", "id": i, "method": method, "params": params}) | ||
| batch.append({"jsonrpc": "2.0", "id": "bn_end", "method": "eth_blockNumber", "params": []}) |
There was a problem hiding this comment.
[suggestion] The latency-independence guarantee assumes the server processes the batch such that bn_end reflects a height >= every intervening read. This holds for sequential batch processing (go-ethereum-style), but if a server processed batch elements concurrently, bn_end could return before a read that then lands at the next height, yielding h_start == h_end while a read was served at a different height. Worth a short comment documenting this assumption (sequential in-order batch execution) so the guarantee's scope is explicit.
| # serialization/parsing (which also holds the GIL). | ||
| method, body = encode(i) | ||
| c = client() | ||
| wait_for_rate_slot() |
There was a problem hiding this comment.
[suggestion] In duration-limited load runs, a worker reserves an RPS slot (wait_for_rate_slot advances next_send_at unconditionally) after acquiring an id but before checking the deadline in the timed region. With high concurrency and low LOAD_RPS, many slots get scheduled well past stop_at, so the run can noticeably overshoot LOAD_DURATION and inflate the request count. Consider re-checking the deadline after the rate wait (bail if stop_at has passed) so duration-bounded runs stop closer to the deadline.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3751 +/- ##
==========================================
- Coverage 59.93% 58.99% -0.94%
==========================================
Files 2288 2201 -87
Lines 189868 179913 -9955
==========================================
- Hits 113793 106137 -7656
+ Misses 65942 64445 -1497
+ Partials 10133 9331 -802
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — self-contained dev/testing script under scripts/, no production code paths touched.
What was reviewed
- The one changed file (
scripts/rpc-sc-read-probe.sh) end-to-end: argument parsing, target resolution (http/k8s/bare), runner dispatch (local vs ephemeral pod), Python load generator, and cross-target consistency checker. - Load path: rate limiter, keep-alive/reconnect, percentile computation, JSON summary,
MAX_ERROR_RATEgating. - Check path: height-synchronized batch with
eth_blockNumbersentinels, crossed-batch handling,CONSISTENT/MISMATCH/INCONCLUSIVEverdict + distinct exit codes. - Ruled out (per the finder pass): unstripped whitespace in TARGETS entries,
PROGRESS_EVERY=0behavior asymmetry between check/load, and CONSISTENT with 0 comparisons whenREQUESTS_PER_HEIGHT=0— the last is guarded by thecomparable_heights == 0INCONCLUSIVE branch.
Extended reasoning...
Overview
The PR adds a single new file, scripts/rpc-sc-read-probe.sh, a dev/testing tool that drives EVM JSON-RPC reads at one or more Sei nodes to (a) verify cross-node SC read-path consistency and (b) measure latency/throughput. It has three modes (check, monitor, load), supports http(s)/bare/k8s target specs, and can execute either locally (python3) or in an ephemeral in-cluster pod. No changes are made to the node, RPC server, or any production code path.
Security risks
None material. The script runs client-side only, sends only read RPCs (eth_call, eth_getBalance, eth_getStorageAt, eth_getCode, eth_getBlockByNumber, eth_getLogs, eth_blockNumber, eth_feeHistory) with the latest tag, and does not sign or submit transactions. The kubectl invocations use fixed subcommands with argument arrays (not shell-interpolated command strings). The only operator-facing risk is pointing the load mode at an endpoint operators do not own, which is standard for any load tool and squarely on the caller.
Level of scrutiny
Low. This is a self-contained script under scripts/ with no runtime interaction with the chain code. There is no consensus, state, storage, RPC handler, or crypto surface touched. A bug here can at worst produce a misleading dev report; it cannot affect nodes, validators, or users.
Other factors
The one flagged issue is a pure cosmetic nit: the LOAD summary's methods list includes eth_getTransactionCount, but the shared choose_request generator never emits it, so METHOD_COUNTS will always print eth_getTransactionCount=0. Percentiles, error rates, JSON output, and pass/fail behavior are unaffected. That is well below the threshold to block a dev-tool PR. The inline comment already records the fix options for the author.
cody-littley
left a comment
cody-littley
left a comment
There was a problem hiding this comment.
LGTM, but I largely lack context to grok this sort of testing script, so we'll need to rely on the LLM to do the deep review work. I'm ok with this merging once the LLM gives the green light.
| batch = [{"jsonrpc": "2.0", "id": "bn_start", "method": "eth_blockNumber", "params": []}] | ||
| method_by_id = {} | ||
| for i in range(1, REQUESTS_PER_HEIGHT + 1): | ||
| method, params = choose_request(i) |
There was a problem hiding this comment.
Check mode skips SC-only reads
Medium Severity
In check mode, capture_coherent builds its batch via choose_request(i) with the default full method mix, so samples can include eth_getBlockByNumber, eth_getLogs, eth_feeHistory, and extra eth_blockNumber calls—not only latest SC state reads. That diverges from the script/PR goal of validating the SC read path and from load mode’s LOAD_SC_ONLY behavior.
Reviewed by Cursor Bugbot for commit 2655fe1. Configure here.
There was a problem hiding this comment.
Adds a self-contained dev/testing bash+Python probe for the SC read path (consistency check, monitor, load). No production code is touched; the script is well-documented and defensive. Two minor correctness/robustness gaps worth addressing, both non-blocking.
Findings: 0 blocking | 4 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- REVIEW_GUIDELINES.md (base branch) and cursor-review.md are both empty, so no repo-specific guidelines and no Cursor second opinion were available for this pass. Codex's review was present and its two findings are incorporated below.
- The
check/monitorverdict's soundness depends on the RPC server executing JSON-RPC batch elements serially in request order (so a mid-batch block commit is caught by thebn_endsentinel and discarded ascrossed). go-ethereum-based servers do this today, but the JSON-RPC spec permits concurrent/reordered batch execution, under whichbn_start == bn_endno longer guarantees the interiorlatestreads were served at a single height. Worth documenting this assumption near capture_coherent so a future RPC change (or a non-geth backend) doesn't silently invalidate CONSISTENT verdicts. (Raised by Codex.) - 2 suggestion(s)/nit(s) flagged inline on specific lines.
| """ | ||
| batch = [{"jsonrpc": "2.0", "id": "bn_start", "method": "eth_blockNumber", "params": []}] | ||
| method_by_id = {} | ||
| for i in range(1, REQUESTS_PER_HEIGHT + 1): |
There was a problem hiding this comment.
[suggestion] REQUESTS_PER_HEIGHT is never validated. With 0 (or a negative value), range(1, REQUESTS_PER_HEIGHT + 1) is empty, so each coherent sample carries zero reads; the comparison loop over baseline_rows never runs, yet comparable_heights still increments and the run reports CONSISTENT matched=0. A misconfiguration then masquerades as a pass. Consider rejecting REQUESTS_PER_HEIGHT < 1 at startup (exit 2), consistent with the other config-error paths. (Flagged by Codex.)
| Returns (height, rows) on success, ("crossed", None) if the batch spanned a | ||
| block boundary, or (None, None) on error. | ||
| """ | ||
| batch = [{"jsonrpc": "2.0", "id": "bn_start", "method": "eth_blockNumber", "params": []}] |
There was a problem hiding this comment.
[nit] This bracketing scheme relies on the server processing the batch serially in request order so that any block commit during the batch shifts bn_end above bn_start and the sample is dropped as crossed. That holds for go-ethereum-derived RPCs but isn't guaranteed by the JSON-RPC spec (batches may be processed concurrently/out of order). A one-line comment stating this assumption would make the correctness dependency explicit. (Related to Codex's batch-ordering note.)
Superseded: latest AI review found no blocking issues.
There was a problem hiding this comment.
LGTM — dev-tooling script only, latest commit addresses the prior rounds of bot feedback (duplicate-name guard, pre-flight error handling, RPS deadline re-check). The two remaining findings are nits about JSONL/verdict-emission edge cases in a dev script; not merge-blocking.
Extended reasoning...
Overview
This PR adds a single new file, scripts/rpc-sc-read-probe.sh — a self-contained bash + embedded Python tool for exercising Sei EVM JSON-RPC live-SC reads. It provides three modes (check, monitor, load), supports http(s)://, bare host:port, and k8s:// targets (with auto port-forward from local runner), and emits both human-readable and optional JSONL output. No production code paths, no changes to node, RPC server, or on-chain logic.
Security risks
None material. The script is invoked manually by operators against RPC endpoints they already control; there is no untrusted input path. Environment variables are the sole configuration surface, target URLs are parsed by urllib.parse and sent to endpoints the operator explicitly specifies, and the ephemeral k8s runner pod uses a pinned public image (python:3.12-alpine).
Level of scrutiny
Low. This is a scripts/ dev/benchmarking tool — no impact on chain state, consensus, or the RPC server itself, and it can only misreport its own diagnostics. The most severe failure mode is a false CONSISTENT verdict, and the author has already added guards (duplicate name rejection, pre-flight error handling wrapped in try/except so exit codes 0/1/2 hold) in response to earlier reviewer rounds.
Other factors
- The author has been responsive across multiple bot review passes on this PR (Cursor Bugbot, chatgpt-codex, seidroid, and my own prior runs) — the most substantive findings (duplicate target names → silent CONSISTENT; pre-flight traceback → exit 1 collision with MISMATCH; RPS scheduling overrunning LOAD_DURATION) are all addressed in the latest commit.
- The two remaining nits (pre-flight INCONCLUSIVE bypasses CHECK_RESULT_JSON emission; REQUESTS_PER_HEIGHT=0 could emit false CONSISTENT) are both narrow edge cases in a dev tool. Author can address them as follow-up.
- Codecov reports all modified/coverable lines covered; Buf checks green.
| if not heights: | ||
| print( | ||
| "CHECK_VERDICT verdict=INCONCLUSIVE nothing_compared=1 -- no target answered the " | ||
| "pre-flight eth_blockNumber, so consistency was NOT verified (this is not a pass).", | ||
| flush=True, | ||
| ) | ||
| raise SystemExit(2) | ||
| return max(heights) |
There was a problem hiding this comment.
🟡 The new pre-flight INCONCLUSIVE path in initial_height() (scripts/rpc-sc-read-probe.sh:918-924) emits CHECK_VERDICT and then raise SystemExit(2), but — unlike the end-of-run INCONCLUSIVE path at lines 1041-1057, which prints CHECK_RESULT_JSON first — it never emits a CHECK_RESULT_JSON line. Since OUTPUT_JSON is populated by sed -E -n 's/^(LOAD|CHECK)_RESULT_JSON //p' (line 1188), a round where pre-flight fails leaves no JSONL entry, while a round that reaches the sampling loop and hits INCONCLUSIVE does — an asymmetry in the JSON contract that would show up as silent gaps in MODE=monitor output. Fix by emitting a CHECK_RESULT_JSON summary (kind=check, verdict=INCONCLUSIVE, matched=0, mismatched=0, per_node_errors populated) just before raise SystemExit(2) so the two INCONCLUSIVE paths stay symmetric.
Extended reasoning...
What's wrong. CHECK_PY has two INCONCLUSIVE emit paths, and they disagree on which lines they print.
- End-of-run INCONCLUSIVE (scripts/rpc-sc-read-probe.sh:1041-1066): sets
verdict='INCONCLUSIVE', builds thesummarydict, printsCHECK_RESULT_JSON <json>(line 1057), then prints the human-readableCHECK_VERDICT(lines 1059-1066), thenraise SystemExit(2)at line 1081. - Pre-flight INCONCLUSIVE (scripts/rpc-sc-read-probe.sh:918-924): if no target answered
eth_blockNumberat startup, prints onlyCHECK_VERDICT verdict=INCONCLUSIVE ...and thenraise SystemExit(2). NoCHECK_RESULT_JSONline is emitted.
How it manifests. The OUTPUT_JSON extractor at line 1188 is sed -E -n 's/^(LOAD|CHECK)_RESULT_JSON //p' "$CAPTURE_FILE" > "$OUTPUT_JSON". It matches only lines that start with LOAD_RESULT_JSON or CHECK_RESULT_JSON. The pre-flight path never emits such a line, so the round leaves nothing in the JSONL — while an end-of-run INCONCLUSIVE round does. The exit-code contract (0/1/2) is preserved on both paths; the JSONL contract (one JSON object per target (load) or per run (check) as JSONL per the script header) is only preserved on one.
Concrete impact. In MODE=monitor with OUTPUT_JSON set, if a round transiently fails pre-flight (e.g. one target briefly unreachable at round start), that round produces no JSONL entry, while a round that reaches the sampling loop and hits INCONCLUSIVE there does emit one. Downstream consumers that count JSONL lines per round or reason about verdict distributions see silent gaps rather than INCONCLUSIVE records. The human-readable log is unaffected — CHECK_VERDICT verdict=INCONCLUSIVE ... is still emitted — so an interactive operator will see the round.
Why existing code doesn't prevent it. The recent fix that wrapped initial_height() in try/except correctly handled the exit-code contract by choosing SystemExit(2) instead of letting a Python traceback exit with code 1. It just didn't extend the same symmetry to the JSON output contract; the end-of-run path composes its own summary dict just before printing CHECK_RESULT_JSON, and that composition step wasn't duplicated in the pre-flight bailout.
Step-by-step proof.
- Configure
TARGETS='a=http://127.0.0.1:8545,b=http://127.0.0.1:9999'where:9999has nothing listening. - Run
MODE=monitor MONITOR_ITERATIONS=2 OUTPUT_JSON=/tmp/out.jsonl RUNNER=local ./scripts/rpc-sc-read-probe.sh. - Round 1: both
block_number()calls ininitial_height()fail forb(and succeed fora, soheightsis non-empty — but consider the harder case whereais also briefly down:heights == []). Line 918'sif not heights:branch fires; line 919-923 printsCHECK_VERDICT verdict=INCONCLUSIVE ...; line 924 raisesSystemExit(2). NoCHECK_RESULT_JSONline reachesCAPTURE_FILE. - Round 2: both targets recover. The sampling loop runs, but if targets are still too far apart in height for a comparable sample, verdict becomes
INCONCLUSIVEat line 1044. Line 1057 printsCHECK_RESULT_JSON {..."verdict":"INCONCLUSIVE"...}.CAPTURE_FILEnow has oneCHECK_RESULT_JSONline. - After both rounds, line 1188's
sedextracts exactly one JSONL entry, even though two rounds both concluded INCONCLUSIVE. A consumer computingper-round verdict distributionfrom/tmp/out.jsonlsees a silent gap for round 1.
How to fix. Emit a CHECK_RESULT_JSON summary at the pre-flight bailout so both INCONCLUSIVE paths write the same shape:
if not heights:
summary = {
"kind": "check", "verdict": "INCONCLUSIVE",
"start_height": 0, "heights_seen": 0,
"comparable_heights": 0, "skipped_incomplete": 0,
"requests_per_height": REQUESTS_PER_HEIGHT, "matched": 0, "mismatched": 0,
"targets": target_names,
"per_node_samples": {n: 0 for n in target_names},
"per_node_crossed": {n: 0 for n in target_names},
"per_node_errors": {n: 1 for n in target_names},
}
print("CHECK_RESULT_JSON " + json.dumps(summary, separators=(",", ":")), flush=True)
print("CHECK_VERDICT verdict=INCONCLUSIVE ...", flush=True)
raise SystemExit(2)Severity. Nit. This is a dev/testing script under scripts/, the human-readable CHECK_VERDICT is still emitted, the exit-code contract still holds, and the impact is limited to JSONL consumers in MODE=monitor seeing a gap rather than an INCONCLUSIVE record. Not merge-blocking, but worth a small follow-up to keep the two INCONCLUSIVE paths symmetric with the documented JSONL contract.
| for i in range(1, REQUESTS_PER_HEIGHT + 1): | ||
| method, params = choose_request(i) | ||
| method_by_id[i] = method | ||
| batch.append({"jsonrpc": "2.0", "id": i, "method": method, "params": params}) | ||
| batch.append({"jsonrpc": "2.0", "id": "bn_end", "method": "eth_blockNumber", "params": []}) | ||
|
|
There was a problem hiding this comment.
🟡 REQUESTS_PER_HEIGHT=0 (or any value < 1) trips the same misconfig-as-pass pattern the new duplicate-name guard was added to prevent: for i in range(1, REQUESTS_PER_HEIGHT + 1) at scripts/rpc-sc-read-probe.sh:855 becomes empty, so the batch contains only the two eth_blockNumber sentinels and capture_coherent returns (h_start, []). comparable_heights still increments (empty lists are still keys in node_rows) but the enumerate over the empty row list matches nothing, so the verdict falls through to CONSISTENT (exit 0) despite zero state reads having been compared. Recommend a symmetric guard next to the len(TARGETS) < 2 check — reject REQUESTS_PER_HEIGHT < 1 up front (bash and/or Python entry). Nit; requires an obviously nonsensical operator value on a dev-tooling script.
Extended reasoning...
What is wrong. In CHECK_PY at scripts/rpc-sc-read-probe.sh:855, capture_coherent builds its batch with for i in range(1, REQUESTS_PER_HEIGHT + 1). When REQUESTS_PER_HEIGHT=0 (or negative), range(1, 1) — or range(1, <=0) — is empty, so the batch contains only the two eth_blockNumber sentinels and no state reads. The response-assembly loop at line 893 is empty for the same reason, so the function returns (h_start, []) after the sentinels pass the h_start == h_end check (near-certain for a 2-request batch).
How the bug manifests. watch_worker stores samples[height][name] = [] for every target. The comparison loop's completeness gate — all(name in node_rows for name in target_names) — passes because every target is present as a dict key (mapping to an empty list). comparable_heights is incremented. Then for idx, baseline in enumerate(baseline_rows) iterates enumerate([]) zero times, so neither matches nor mismatches gets touched at that height. The verdict logic (if mismatches: MISMATCH; elif comparable_heights == 0: INCONCLUSIVE; else: CONSISTENT) sees mismatches == 0 and comparable_heights > 0 and falls through to CONSISTENT with exit code 0.
Why existing code does not prevent it. REQUESTS_PER_HEIGHT is parsed as an int but never bounds-checked in bash or Python. The pre-flight len(TARGETS) < 2 and duplicate-name guards were added specifically for this class of misconfig-silently-passing bug, but they don't cover this shape.
Impact. The tool's documented contract is that CONSISTENT means "we compared samples and everything agreed" — here the tool reports CONSISTENT/exit 0 having compared exactly zero state reads. Practical impact is small: this requires an operator to explicitly set REQUESTS_PER_HEIGHT=0 (the default is 5), which is an obviously nonsensical value; the script lives under scripts/ and is not a production code path. Same defensive-hardening pattern as the duplicate-name guard just added; symmetric fix.
Proof (step-by-step).
- Run
MODE=check REQUESTS_PER_HEIGHT=0 TARGETS='a=http://n1:8545,b=http://n2:8545' ./scripts/rpc-sc-read-probe.sh(n1 and n2 can even return divergent state — doesn't matter, they won't be sampled). capture_coherentline 855:for i in range(1, 0 + 1)=range(1, 1)= empty. Batch =[{bn_start}, {bn_end}].- Server responds;
h_start == h_endis essentially guaranteed for a 2-request batch. Function returns(h_start, [])(the row-assembly loop at line 893 is also empty). watch_workerwritessamples[h_start]['a'] = []andsamples[h_start]['b'] = [].- Comparison loop:
all(name in node_rows for name in ['a', 'b'])→ True;comparable_heights += 1. baseline_rows = [];for idx, baseline in enumerate([])iterates zero times;matches == mismatches == 0.- Verdict:
mismatches == 0andcomparable_heights > 0→CONSISTENT,raise SystemExit(0). Tool printsCHECK_VERDICT verdict=CONSISTENT matched=0 comparable_heights=<n>.
How to fix. Reject REQUESTS_PER_HEIGHT < 1 up front, alongside the existing pre-flight checks in CHECK_PY:
if REQUESTS_PER_HEIGHT < 1:
print("check mode requires REQUESTS_PER_HEIGHT >= 1; got %d" % REQUESTS_PER_HEIGHT, flush=True)
raise SystemExit(2)Optionally mirror the guard in bash for a friendlier early error. Alternatively, refuse to increment comparable_heights when baseline_rows is empty (treat empty-row heights as incomplete).
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e50f696. Configure here.
| elif comparable_heights == 0: | ||
| verdict, exit_code = "INCONCLUSIVE", 2 | ||
| else: | ||
| verdict, exit_code = "CONSISTENT", 0 |
There was a problem hiding this comment.
Zero requests vacuous CONSISTENT
Medium Severity
With REQUESTS_PER_HEIGHT set to 0 (or negative), check mode still records coherent height samples but performs no RPC response comparisons. The verdict logic then returns CONSISTENT whenever comparable_heights is positive and mismatches is zero, even when matched is 0—so automation can treat an unverified run as a pass.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit e50f696. Configure here.
There was a problem hiding this comment.
Adds a self-contained dev/testing bash script for probing the SC read path (consistency check + load); it touches no production code. The methodology is generally sound, but the consistency verdict rests on assumptions (in-order batch processing, treating errors/missing responses as matches) that can produce a false CONSISTENT — worth tightening since correctness of the verdict is the tool's whole purpose.
Findings: 0 blocking | 6 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Repo review artifacts were empty for this pass:
REVIEW_GUIDELINES.mdandcursor-review.mdcontained no content, so no repo-specific guidelines or Cursor second opinion were available to merge. Codex produced two findings (both incorporated below). - k8s runner exit-code propagation: in
RUNNER=k8sthe Python verdict is emitted viaraise SystemExit(exit_code)insidekubectl run ... | sed | tee.monitormode and the top-levelrcdepend on that exit code propagating back throughkubectl run, which is historically unreliable. The test plan appears to have verified MISMATCH detection under the local runner; please confirm a MISMATCH (exit 1) in an in-cluster pod actually surfaces as a non-okmonitor round, otherwise k8s-runner monitoring can silently always report ok. loadmode runs targets sequentially (run_load_targets), so cross-target latency numbers are not gathered under simultaneous load; fine if intended, but worth a note in the header docs.- 3 suggestion(s)/nit(s) flagged inline on specific lines.
| sentinels. The node processes the batch server-side (near-instant), so if | ||
| the height reported at the start and end of the batch is identical, every | ||
| "latest" read in between was served at that same height -- regardless of | ||
| client/network latency. |
There was a problem hiding this comment.
[suggestion] The coherence guarantee here assumes the server processes the batch strictly in order (bn_start before every read before bn_end) against a single height. The JSON-RPC spec allows batch items to be processed in any order/concurrently, and each latest read resolves the tip independently — there is no shared snapshot across the batch. If the backend processes items concurrently, h_start == h_end no longer proves the interior reads were served at that height, which can label cross-height data as coherent (matching Codex's finding). In practice the window is small (short batch vs ~400ms block time), so this is a real limitation rather than a guaranteed bug — worth documenting the assumption and, if possible, verifying that Sei's evmrpc processes batches sequentially.
|
|
||
| rows = [] | ||
| for i in range(1, REQUESTS_PER_HEIGHT + 1): | ||
| item = by_id.get(i, {"jsonrpc": "2.0", "id": i, "error": "missing batch response"}) |
There was a problem hiding this comment.
[suggestion] Rows are compared purely on serialized text without requiring a valid "result". Two effects (matching Codex): (1) a missing batch entry on all nodes synthesizes the identical {"error":"missing batch response"} text and counts as a match; (2) identical JSON-RPC error objects across nodes also match. So a run where no state read actually succeeds can still report CONSISTENT (exit 0). The default zero-address probes return real results, but with custom STATE_ADDRESS/eth_getLogs this can mask a wholesale read failure. Consider requiring the baseline row to contain a result, or tracking error/missing rows separately so an all-error height is treated as INCONCLUSIVE rather than CONSISTENT.
| batch = [{"jsonrpc": "2.0", "id": "bn_start", "method": "eth_blockNumber", "params": []}] | ||
| method_by_id = {} | ||
| for i in range(1, REQUESTS_PER_HEIGHT + 1): | ||
| method, params = choose_request(i) |
There was a problem hiding this comment.
[nit] check calls choose_request(i) with the default (non-sc-only) mix, so the consistency comparison includes eth_getBlockByNumber, eth_getLogs, eth_blockNumber, and eth_feeHistory, not just SC state reads. This contradicts the header docs ("one batch of latest state reads") and the tool's stated SC-read focus. Either use the sc-only mix here, or update the docs to reflect that check compares the full read mix.
There was a problem hiding this comment.
Beyond the inline findings, this run's finders also raised the concern (echoing the still-open cursor bugbot note on this thread, "Check mode skips SC-only reads") that check mode's sampling uses the full default RPC method mix rather than an SC-only mix like load mode's LOAD_SC_ONLY. Verifiers examined it and ruled it out as not a correctness bug: the default mix already routes the majority of requests through SC-relevant methods (eth_call/eth_getBalance/eth_getStorageAt/eth_getCode) via the latest tag, and the lack of a check-mode SC-only toggle is a feature gap rather than incorrect behavior.
Extended reasoning...
This run's finder agents raised the same concern cursor bugbot posted (still open, unresolved) about check mode not restricting to an SC-only method mix. I reviewed capture_coherent()/choose_request() in the diff and agree with this run's verifiers: the default bucket already sends the large majority of requests through methods that read live state via the latest tag (eth_call, eth_getBalance, eth_getStorageAt, eth_getCode), so the consistency check still exercises the SC read path on most samples — it just isn't pure SC-only like load mode's LOAD_SC_ONLY option. That's a scope/feature gap, not a logic bug, so I'm not treating it as blocking. It's included here only as a record that it was looked at again this run, not as new blocking feedback.
| # Names key the per-target samples in check mode; a duplicate would collapse | ||
| # two targets to one entry and make the comparison silently pass. Reject it | ||
| # rather than report a misconfiguration as CONSISTENT. (Guard the loop for | ||
| # the empty-array case so it is safe under `set -u` on bash 3.2.) | ||
| if [ "${#target_names[@]}" -gt 0 ]; then | ||
| for existing in "${target_names[@]}"; do | ||
| if [ "$existing" = "$name" ]; then | ||
| echo "duplicate target name '$name' in TARGETS: each target must resolve to a unique name (use name=SPEC to disambiguate)" >&2 | ||
| exit 2 | ||
| fi | ||
| done | ||
| fi | ||
| resolve_spec "$spec" | ||
| target_names+=("$name") | ||
| target_urls+=("$RESOLVED_URL") | ||
| done |
There was a problem hiding this comment.
🟡 parse_targets rejects duplicate target NAMES but never checks that two targets resolve to the same URL, so an operator can double-poll one backend under two distinct names (e.g. a copy-pasted spec, or a bare host vs. the same host with an explicit default port) and check mode will report CONSISTENT/exit0 without ever comparing two distinct nodes. This is a symmetric gap in the duplicate-name guard this PR just added.
Extended reasoning...
parse_targets (scripts/rpc-sc-read-probe.sh:339-370) walks the comma-separated TARGETS list and rejects a duplicate name via the loop at lines 356-361 (and again on the Python side in CHECK_PY's parse_targets(), which checks len(set(names)) != len(names)). Neither check ever compares the resolved URLs in target_urls. Two independent paths let a duplicate backend slip past a guard that exists specifically to prevent "misconfiguration reported as a pass":
-
Explicit duplicate spec, distinct names.
TARGETS="a=http://10.0.0.1:8545,b=http://10.0.0.1:8545"(a plausible copy-paste typo when wiring up a base-vs-migrated comparison) yieldstarget_names=[a,b](no collision) buttarget_urls=[http://10.0.0.1:8545, http://10.0.0.1:8545]-- identical. -
derive_name runs before DEFAULT_PORT normalization.
derive_name(lines 329-334) operates on the raw spec and maps ":" to "-" viatr. ForTARGETS="10.0.0.1,10.0.0.1:8545"withDEFAULT_PORT=8545, this derives "10.0.0.1" and "10.0.0.1-8545" -- distinct names -- butresolve_specmaps the bare host tohttp://10.0.0.1:8545(applying DEFAULT_PORT) and the explicit-port spec to the same URL. Same backend, two names, no guard trips.
In either case, check mode spawns two watch_worker threads against the same node. Each thread has its own thread-local connection, so there's no shared state to expose the collision -- the responses simply match because it is literally one node answering both requests. At every synchronized height, node_rows[name][idx]["text"] == baseline["text"] trivially, mismatches stays 0, comparable_heights > 0, and the run prints CHECK_VERDICT verdict=CONSISTENT and exits 0 -- having never actually cross-checked two distinct backends. Since the entire purpose of this tool is validating SC read-path consistency across backends (per the PR description: "validate SC read-path correctness across backends, e.g. base vs migrated/FlatKV nodes"), this defeats that purpose in exactly the scenario the tool is meant to catch, producing a false sense of assurance from a CONSISTENT/exit-0 result.
Step-by-step proof (path 2, since it requires no typo -- just a common env-var default):
- Operator sets
DEFAULT_PORT=8545andTARGETS="10.0.0.1,10.0.0.1:8545", intending (mistakenly) to compare node 10.0.0.1 against what they believe is a different node also on port 8545. parse_targetsderives name "10.0.0.1" for the first entry (no colon to strip) and "10.0.0.1-8545" for the second (colon replaced by tr). The uniqueness loop at 356-361 compares these two strings -- they differ, so the loop passes.resolve_specis called on each raw spec: the bare host "10.0.0.1" has no colon, so it becomeshttp://10.0.0.1:$DEFAULT_PORT=http://10.0.0.1:8545; the explicit spec "10.0.0.1:8545" already has a colon, soresolve_specuses it directly ashttp://10.0.0.1:8545. Both target_urls entries are now identical.- In check mode, two watch_worker threads run against the same URL under names "10.0.0.1" and "10.0.0.1-8545". At every synchronized height, both threads capture the same latest-tagged reads from the same node (deterministic given a fixed height), so
samples[height]["10.0.0.1"] == samples[height]["10.0.0.1-8545"]byte-for-byte. - The comparison loop iterates
target_names[1:], readsnode_rows["10.0.0.1-8545"][idx], compares it to baselinenode_rows["10.0.0.1"][idx]-- always equal.mismatchesstays 0. - Final verdict:
CHECK_VERDICT verdict=CONSISTENT matched=N comparable_heights=M, exit code 0 -- reported as a clean pass despite never comparing two distinct backends.
Why existing code does not prevent it: both the bash-side name guard and the Python-side name-set guard operate purely on target_names, which is a presentation label distinct from the identity of the backend (target_urls). Neither layer treats resolved-URL identity as the thing that must be unique, even though URL identity -- not name identity -- is what actually determines whether two samples are independent.
Fix: add a URL-uniqueness check parallel to the existing name-uniqueness check, after resolve_spec has applied DEFAULT_PORT and any k8s port-forward resolution -- e.g. an associative-array/set check on target_urls in bash's parse_targets (mirroring the existing name-seen pattern) and on the resolved URLs in the Python parse_targets(). This closes the guard symmetrically without touching the sampling/comparison logic.
Severity: this is a dev/testing script under scripts/, requires an operator misconfiguration to trigger (duplicate spec, or a bare host plus an explicit default port), and the failure mode is a misleading CONSISTENT on a manual run rather than a crash or data loss -- consistent with the "nit" severity all five independent verifiers converged on. It is nonetheless a real, easily-closed gap in a guard the PR authors explicitly added for this exact class of problem.
| if [ "$MONITOR_ITERATIONS" -gt 0 ] && [ "$iteration" -ge "$MONITOR_ITERATIONS" ]; then | ||
| break | ||
| fi | ||
| iteration=$((iteration + 1)) | ||
| sleep "$MONITOR_INTERVAL" | ||
| done | ||
| [ "$failures" -eq 0 ] | ||
| } | ||
|
|
||
| parse_targets | ||
|
|
||
| rc=0 | ||
| case "$MODE" in | ||
| check) run_check_once || rc=$? ;; | ||
| monitor) run_monitor || rc=$? ;; | ||
| load) | ||
| echo "== load mode: per-target request run; increase REQUESTS or set LOAD_DURATION for volume ==" | ||
| run_load_targets || rc=$? | ||
| ;; | ||
| esac | ||
|
|
||
| if [ -n "$OUTPUT_JSON" ]; then | ||
| # -E for portable alternation (BSD/macOS sed lacks \| in basic regex). | ||
| sed -E -n 's/^(LOAD|CHECK)_RESULT_JSON //p' "$CAPTURE_FILE" > "$OUTPUT_JSON" | ||
| echo "wrote $(wc -l < "$OUTPUT_JSON" | tr -d ' ') machine-readable result(s) (JSONL) to $OUTPUT_JSON" >&2 | ||
| fi |
There was a problem hiding this comment.
🟡 In MODE=monitor with the default MONITOR_ITERATIONS=0 (the documented continuous-monitoring workflow), run_monitor()'s while :; loop never returns to the top-level case statement, so the OUTPUT_JSON extraction (sed ... CAPTURE_FILE > OUTPUT_JSON) at the end of the script is never reached. The only way to stop an infinite monitor is SIGINT/SIGTERM, and the cleanup() trap only kills port-forwards and rm -f's CAPTURE_FILE — it never extracts OUTPUT_JSON — so the accumulated CHECK_RESULT_JSON lines are deleted before they can ever be written out, leaving an empty/missing JSONL file for the default infinite-monitor case.
Extended reasoning...
What happens. run_monitor() (around line 1155) loops with while :; and only breaks when MONITOR_ITERATIONS -gt 0 and iteration -ge MONITOR_ITERATIONS. MONITOR_ITERATIONS defaults to 0, and the header/PR description present MODE=monitor as a long-running, continuous-monitoring tool (MONITOR_INTERVAL=60 ... TARGETS=... ./scripts/rpc-sc-read-probe.sh, no iteration cap given). With the default, the loop never breaks on its own — the only top-level case $MODE statement (line ~1179) that follows is never reached during normal operation, because control never returns from run_monitor().
The OUTPUT_JSON extraction sits after that case block (around lines 1186-1190): sed -E -n 's/^(LOAD|CHECK)_RESULT_JSON //p' "$CAPTURE_FILE" > "$OUTPUT_JSON". Since this line is only reached once the case statement returns, and the case statement is only reached once run_monitor returns, an infinite monitor run never executes it during a graceful/normal path.
How it actually terminates, and why that doesn't help. The only way to end an infinite monitor is an operator hitting Ctrl-C (SIGINT) or something sending SIGTERM. The script installs trap cleanup EXIT INT TERM, and cleanup() only kills the tracked port-forward PIDs and does rm -f "$CAPTURE_FILE". It does not run the sed extraction. So the moment the process is asked to stop, CAPTURE_FILE — which has been accumulating CHECK_RESULT_JSON ... lines via tee -a inside dispatch_python for the entire run — is deleted before the extraction step that reads from it ever executes.
Net effect. For the specific combination the tool advertises as its primary long-running use case — MODE=monitor with the default (unbounded) MONITOR_ITERATIONS and OUTPUT_JSON=/some/path set — the JSONL file at $OUTPUT_JSON is either never created or left empty, even though the whole point of setting $OUTPUT_JSON is to get a durable, machine-readable record of every round's verdict. The human-readable log (CHECK_VERDICT, MONITOR_RESULT lines) still streams live to stdout via tee, so the information isn't fully lost if the caller also captured stdout separately, but the documented OUTPUT_JSON contract silently fails to deliver anything for this configuration.
Step-by-step proof:
- Run
MODE=monitor OUTPUT_JSON=/tmp/out.jsonl TARGETS=a=http://n1:8545,b=http://n2:8545 ./scripts/rpc-sc-read-probe.sh(noMONITOR_ITERATIONSset, so it defaults to0). run_monitor()runs round 1:run_check_onceexecutes, appending aCHECK_RESULT_JSON {...}line to$CAPTURE_FILEviatee -a.MONITOR_ITERATIONSis0, so theif [ "$MONITOR_ITERATIONS" -gt 0 ] && ...break condition is false; the loop sleepsMONITOR_INTERVALseconds and continues indefinitely, appending one moreCHECK_RESULT_JSONline per round.- The operator eventually presses Ctrl-C to stop monitoring (there is no other way to stop it).
- Bash delivers SIGINT; the
trap cleanup EXIT INT TERMfirescleanup(), which killsPF_PIDSand runsrm -f "$CAPTURE_FILE". - The process exits. The line
sed -E -n 's/^(LOAD|CHECK)_RESULT_JSON //p' "$CAPTURE_FILE" > "$OUTPUT_JSON"at the end of the script body is never reached, because the SIGINT unwound execution via the trap before control could return there — and even if it somehow were reached,CAPTURE_FILEhas already been deleted bycleanup(). - Result:
/tmp/out.jsonlis empty or does not exist, despite N rounds ofCHECK_RESULT_JSONdata having been generated and briefly held in$CAPTURE_FILE.
Why existing code doesn't prevent this. The OUTPUT_JSON extraction was clearly designed with the check and load modes in mind, where the top-level case statement returns normally after a single run and the extraction step executes right after. monitor mode was very likely intended to behave the same way for each round, but the extraction was never wired into run_monitor()'s loop body or into cleanup(), so the finite-iteration case (MONITOR_ITERATIONS>0) happens to work (the loop eventually breaks and falls through to the extraction), while the default, unbounded case — the one the feature is primarily meant for — does not.
Suggested fix. Either (a) extract and append the new CHECK_RESULT_JSON line to $OUTPUT_JSON incrementally at the end of each round inside run_monitor() (mirroring the tee -a pattern already used for CAPTURE_FILE), or (b) move the sed extraction into cleanup() so it runs before rm -f "$CAPTURE_FILE" regardless of how the process terminates. Option (a) is more robust since it doesn't lose data if the process is killed with SIGKILL (which bypasses the trap entirely).
| case "$entry" in | ||
| *=*) name=${entry%%=*}; spec=${entry#*=} ;; | ||
| *) spec=$entry; name=$(derive_name "$spec") ;; | ||
| esac |
There was a problem hiding this comment.
🟡 case "$entry" in *=*) in parse_targets matches ANY '=' in the entry, not just a leading name= prefix. A bare URL target with a query string (e.g. TARGETS='https://rpc.example.com/?apikey=ABC123', a realistic form for API-key-gated RPC endpoints) gets split at that '=': name becomes https://rpc.example.com/?apikey and spec becomes ABC123, which resolve_spec then resolves to http://ABC123:8545 — the wrong host. Anchor the explicit-name case to a name that can't contain ':' or '/', e.g. case "$entry" in [A-Za-z0-9_.-]*=*).
Extended reasoning...
What's wrong. parse_targets (scripts/rpc-sc-read-probe.sh:347-350) decides whether an entry is the explicit name=SPEC form using case "$entry" in *=*). The glob *=* matches an '=' anywhere in the string, not just a leading name= prefix. When it matches, the split is name=${entry%%=*} (everything before the first '=') and spec=${entry#*=} (everything after the first '='). This is fine for the documented name=SPEC form, but it also fires for a bare (auto-named) URL target that happens to contain a query-string '=' before any intended name delimiter — because there isn't one.
Trigger. TARGETS='https://rpc.example.com/?apikey=ABC123' is a realistic bare-URL target: the script's own documentation and examples use hosted RPC endpoints (https://evm-rpc.sei-apis.com), and API-key-gated hosted RPC providers commonly append ?apikey=.../?key=... query strings. The transport layer explicitly supports this shape — both LOAD_PY's Client and CHECK_PY's post() build the HTTP path as (p.path or '/') + ('?' + p.query if p.query else ''), i.e. query strings are a supported SPEC, just not correctly parsed out of TARGETS.
Step-by-step proof.
TARGETS='https://rpc.example.com/?apikey=ABC123'is split on ',' into one entry:https://rpc.example.com/?apikey=ABC123.case "$entry" in *=*)matches (there is an '=' present), so the explicit-name branch runs instead ofderive_name.name=${entry%%=*}→https://rpc.example.com/?apikey(everything up to the first '=').spec=${entry#*=}→ABC123(everything after the first '=').resolve_spec ABC123runs.ABC123doesn't start withhttp://,https://, ork8s://, and (per the fallback branch) since it also contains no ':' it takes the bare-host branch:RESOLVED_URL="http://ABC123:8545".- The target is silently registered as name=
https://rpc.example.com/?apikey, url=http://ABC123:8545— a completely different, non-existent host — instead of erroring or preserving the intended endpoint.
Impact. This is not a silent false-pass in the sense of reporting a wrong CONSISTENT verdict — http://ABC123:8545 is unreachable, so in check mode the pre-flight/sampling will fail for that target (contributing to CHECK_PREFLIGHT_ERROR/per_node_errors, likely driving the run to INCONCLUSIVE, exit 2) and in load mode all requests to it will error out (high error_rate, possible LOAD_FAIL). So the failure is loud rather than silently wrong data — but it's a confusing failure mode (unexplained connection errors to a bogus host) instead of a clear 'invalid target' message, for an input shape (query-string RPC URLs) the tool's own transport code supports.
Workaround that already exists. Using the explicit name=SPEC form works correctly today, e.g. rpc=https://rpc.example.com/?apikey=ABC123 splits on the first '=' into name=rpc, spec=https://rpc.example.com/?apikey=ABC123 (the full URL, since spec=${entry#*=} only strips up to the first '=' and the rest of the string, including subsequent '='s in the query string, is preserved). Only the bare/auto-named form is broken.
Fix. Anchor the explicit-name case to a name that syntactically cannot contain ':' or '/' (both of which must appear in any bare/URL SPEC before a real name-delimiting '=' could occur), e.g.:
case "$entry" in
[A-Za-z0-9_.-]*=*) name=${entry%%=*}; spec=${entry#*=} ;;
*) spec=$entry; name=$(derive_name "$spec") ;;
esacThis preserves the documented name=SPEC form (names are always simple identifiers) while correctly routing bare URLs — including ones with query-string '='s — through derive_name.
* main: fix(evm): bound store-cache depth amplification from stacked EVM snapshots (#3713) Fix address distro bug (#3716) fix(giga): route EVM validation failures to v2 fallback (CON-368) (#3753) feat(seidb): composite + replay digest modes and migration-boundary omission for evm-logical-digest (#3711) feat(scripts): add SC read-path RPC probe (consistency check + load) (#3751) fix(feegrant): bound DenomsSubsetOf cost and cap allowance denoms (#3710)
3 participants
Summary
scripts/rpc-sc-read-probe.sh, a dev/testing tool for the SC (live state-commitment) read path.latestblock tag so requests hit the live SC store (memIAVL / FlatKV), not the historical state store.check(cross-node, height-synchronized response consistency),monitor(looped check), andload(per-target latency/throughput with percentiles).checkis latency-independent: each sample is one batch oflatestreads bracketed byeth_blockNumbersentinels, so reads are only compared when the node served them at a single height — works even over high-RTT links. It emits a verdict with distinct exit codes:CONSISTENT=0,MISMATCH=1,INCONCLUSIVE=2 (nothing comparable → not a pass).http(s)://, barehost:port, andk8s://svc|podtargets (auto port-forward for local); optional JSONL output.Motivation
Used to validate SC read-path correctness across backends (e.g. base vs migrated/FlatKV nodes) and to benchmark read latency/throughput during the FlatKV migration.
Test plan
RUNNER=local MODE=check TARGETS='a=...,b=...'reports a verdict + matched/mismatched across nodes (verified local IP↔DNS: CONSISTENT; in-cluster arctic-1: 130 matched)MODE=load LOAD_SC_ONLY=1 TARGETS=...prints per-method p50/p95/p99 (verified ip/DNS/k8s; RPS cap + JSONL output)MODE=monitorloops and reports a non-okround on anyMISMATCHorINCONCLUSIVEcheckRUNNER=k8s(in-cluster pod) andk8s://targets (in-cluster DNS + local port-forward) resolve and run--helpprints usage