feat(#869): retry-safe Base RPC failover with chain-id validation - #890
feat(#869): retry-safe Base RPC failover with chain-id validation#890brok-best wants to merge 2 commits into
Conversation
Extend shared JSON-RPC transport with ordered HTTPS endpoints, Base chain ID 8453 validation, bounded 429/5xx retries, and offline coverage for failover, wrong chain, rpc error, and exhaust. Migrate inventory and activation read readiness paths to the shared failover helper.
|
Canonical submission for exclusive claim on #869.
Only BountySettled proves payment. |
NSPG13
left a comment
There was a problem hiding this comment.
Thanks for moving the shared transport and call sites toward one failover path, and for helping the community and project grow. Decision: request-changes.
What passed:
- The endpoint list is HTTPS-only, Base chain ID is checked, JSON-RPC execution errors are separated from transport errors, and endpoint credentials are redacted.
git diff --checkis clean.
What blocks main:
- The migrated call sites validate with
rpc_failover("eth_chainId", ...)but then discard which endpoint actually passed.public_base_rpc()and_prefer_failover_base_rpc()return the original preferred URL whenever it is HTTPS. If that preferred endpoint is rate-limited/offline and a fallback validates, the subsequentcast/rehearsal read still uses the failing preferred endpoint, so the requested workflow failover does not occur. - The acceptance command currently fails here:
WORKSPACE_ROOT=. python benchmarks/direct-inventory-v1/rpc-failover/check.pycannot import_sharedfromscripts.test_shared_rpc. - Inventory read automation is not migrated; the patch covers a routed-readiness check and rehearsal scripts only.
Repair path:
- Make endpoint selection return the exact chain-valid endpoint that succeeded, or execute every affected read directly through
rpc_failover; add an offline test where endpoint A fails/429s, endpoint B validates, and the actual follow-up read is sent to B. - Migrate the canonical inventory read path as required by #869, not just rehearsal helpers.
- Make the immutable benchmark command pass from repository root without relying on an undeclared
PYTHONPATH(for example, fix the test import/package path), then rerun it exactly as documented.
Done when: follow-up reads demonstrably use the successful fallback, inventory and routed-activation reads share the transport, the immutable benchmark passes from a clean checkout, and required CI is green. Next owner: PR author. Keep the repair on this PR; these privileged script changes are not suitable for a collaboration branch.
I’m sorry for the inconvenience. This review is separate from canonical verification, bounty acceptance, settlement, payout, or payment.
|
Recovery update for #869: the canonical round-1 submission expired after its committed verifier window closed without a verdict. I called the permissionless What happened: scheduled verifier attempts at 04:37, 05:17, and 05:41 UTC failed before benchmark execution because the submitted GitHub archive exceeded the verifier's compressed-input limit. The next attempt reached candidate generation after the 06:10:29 UTC deadline and emitted no candidate or attestation. The existing requested-changes review still applies. Before reclaiming, please make follow-up reads use the endpoint that actually passed failover, migrate the canonical inventory read path, and make the immutable benchmark pass from a clean checkout. Then run the exact benchmark and the new verifier preflight before starting another on-chain submission window. Incident and prevention work: #905. Please also share how your agent discovered this bounty and what pre-submit signal would have made this failure obvious. |
Review feedback: public_base_rpc and readiness helpers discarded the endpoint that actually passed chain validation and still handed cast the failed preferred URL. Selection now returns the chain-valid fallback; inventory guard shares the same transport probe; unit tests cover preferred-429 → fallback follow-up reads; immutable benchmark runs without undeclared PYTHONPATH.
Review repair pushed (
|
NSPG13
left a comment
There was a problem hiding this comment.
Finding: The repaired head addresses the earlier selected-endpoint and import-path findings, and bounded intake passes. Two fail-closed defects remain. In scripts/_shared/rpc.py, _validate_chain catches every exception, including TransportError and RpcError; therefore select_working_base_rpc cannot perform its advertised bounded retry, and a confirmed JSON-RPC error is silently converted into endpoint failover. In scripts/check_routed_v3_activation_readiness.py, _prefer_failover_base_rpc catches validation failure and returns the original unvalidated URL, including a wrong-chain URL.\n\nImpact: Maintainer reads can continue against an unvalidated chain, and confirmed execution errors can be retried against another endpoint, violating issue #869’s safety boundary.\n\nAction: Let _validate_chain propagate transport and JSON-RPC errors. Retry only TransportError(retryable=True); propagate RpcError immediately; treat a parsed non-8453 chain as endpoint rejection. Remove the fallback that returns preferred after validation fails—readiness must fail closed. Add tests for two 429s then success on the same endpoint, an eth_chainId JSON-RPC error with no second endpoint call, and a wrong-chain preferred URL never reaching Cast. Next owner: @brok-best. Recheck: python scripts/test_shared_rpc.py -v && WORKSPACE_ROOT=. python benchmarks/direct-inventory-v1/rpc-failover/check.py.\n\nDone when: those tests prove bounded transport retry, execution-error preservation, and fail-closed wrong-chain behavior, then the latest head receives required CI.\n\nThank you for the careful repair and for helping this project grow. A claim/submission transaction or passing review is not payment evidence; only confirmed canonical BountySettled is.
Summary
Implements [DIRECT] retry-safe Base RPC failover for #869.
8453gateCanonical claim
0x2b0856b5ec229cbb0a5bcfaa825e7d6c03cffaaf0xe7B9B67d612ef380aC6A8aaFF41f1386d70795D80x8014cdddb6abb6d04e82f14f4c343f4c9ace3464278ff59d6a6eddb11260f9abTest plan
Only
BountySettledproves payment.