feat(chain): govern shared RPC request capacity - #2553
Conversation
|
Additional integration hardening landed in 2d1c3aa. A real-daemon registration test exposed a cross-priority single-flight inversion: same-hash foreground registration could join a background authority lookup that was deliberately held by startup jitter, while the background lookup could also acquire the serialized slot-state lane before its first admission. Name-hash resolution now coalesces separately per workload class, and initial high-water admission occurs before the serialized state lane. New regressions cover both boundaries. Focused chain tests are 49/49 green; the previously failing real daemon suites are now fully green at 67/67 knowledge-asset route tests and 14/14 memory-event tests. Package builds, type checks, lint, and diff checks remain green. |
2d1c3aa to
0b85aae
Compare
commented
Sep 13, 2026
|
Addressed the current review round in
Local validation is green for lint, chain/agent/CLI builds and typechecks, affected RPC unit tests, RFC64 authority/native-wiring/receiver/rollout integration tests, and CLI config/init/runtime/status tests. |
left a comment
There was a problem hiding this comment.
Operational Notice: Review Agent could not complete this review.
Business logic reviewer failed: 2026-09-13T17:23:53.605083Z ERROR codex_core::tools::router: error=exec_command failed for `/bin/bash -lc "nl -ba packages/cli/src/daemon/lifecycle.ts | sed -n '1310,1385p'
| provider, | ||
| (sharedSignal) => withOwnedRpcRequestContext( | ||
| { | ||
| requestClass: 'foreground', |
There was a problem hiding this comment.
🟡 Issue: Background endpoint validation bypasses the configured foreground reserve
What's wrong
The unconditional foreground context lets background tasks evade the governor's background bucket and cold-start jitter. This contradicts the process-wide capacity policy and can consume capacity reserved for user-facing work.
Example
With the documented default 80% foreground reserve, a background RFC-64 lookup against an uncached or failing endpoint performs its eth_chainId validation using the foreground bucket. Multiple adapters or repeated failed validations can consume the total burst that was intended to remain available for publishing and control-plane requests.
Suggested direction
Partition the validation single-flight by request class, or use another scheme that keeps background-initiated physical RPCs in the background lane while allowing foreground callers to start or join foreground-classed validation.
For Agents
Update configured chain-ID validation in evm-adapter-base.ts so a background-only initiator cannot consume foreground admission. Preserve waiter-local cancellation and avoid priority inversion when a real foreground waiter arrives. Add a governor test proving a background validation is charged to the background bucket while a concurrent foreground validation can proceed without joining lower-priority admission.
There was a problem hiding this comment.
Fixed in follow-up PR #2607. Configured chain-ID validation now preserves the initiating request class and partitions the single-flight by foreground/background priority, so background validation cannot spend the foreground reserve and foreground callers do not wait behind lower-priority work. The full chain suite passes: 119 files, 1,822 passed, 1 skipped.
| * privacy boundary. HTTP routes insert these completed blocks without probing | ||
| * feature-specific agent methods or forwarding provider-owned objects. | ||
| */ | ||
| export async function buildRfc64StatusBlocksV1(input: Readonly<{ |
There was a problem hiding this comment.
🟡 Issue: The CLI status facade still owns RFC-64 domain orchestration
What's wrong
The extraction reduces status.ts itself, but mostly moves the same cross-layer knowledge into another CLI file. The CLI now understands agent responsibility selection, bootstrap targets, private policy manifests, receiver internals, rollout modes, and service telemetry. This couples HTTP code to the topology of several RFC-64 subsystems and requires synchronized changes across packages.
Example
If readRfc64CatalogResponsibilitiesV1 is absent or renamed, the status builder reports responsibilities: [] while the same response may still report the catalog as enabled and selected. The optional-method facade makes broken composition indistinguishable from valid empty runtime state.
Suggested direction
Have the agent expose one canonical privacy-safe RFC-64 status snapshot. Keep a narrow compatibility adapter at the package boundary if old agents must be supported, rather than rebuilding domain state inside the HTTP layer.
For Agents
Move RFC-64 status composition to the agent-owned RFC-64 layer. Preserve the current privacy filtering and legacy public status shape. Expose one versioned, privacy-safe snapshot capability, then let the CLI perform only allow-list validation and HTTP serialization. Tests should verify the complete DTO, version-skew rejection, and absence of private identifiers without mocking seven independent optional methods.
There was a problem hiding this comment.
Fixed in follow-up PR #2609. RFC-64 status composition now lives in the agent and crosses the package boundary through one versioned, privacy-safe snapshot capability. The CLI rejects missing or unsupported aggregate schemas, allow-lists the DTO, and preserves the legacy HTTP blocks without probing seven optional subsystem methods. Focused validation passed: 38 agent tests and 34 non-environment status-route tests, plus agent/CLI build and type checks.
| const refreshAuthority = () => this.reconcileRfc64CatalogResponsibilityV1( | ||
| normalizedRecord.contextGraphId, | ||
| ).then(() => undefined); | ||
| if (options?.strict === true) return write.then(refreshAuthority); |
There was a problem hiding this comment.
🟡 Issue: Strict membership reconciliation lacks a regression test
What's wrong
Strict membership updates now await responsibility reconciliation, which can delay or reject join and membership workflows. The current tests would remain green if this changed behavior were accidentally removed or if strict calls stopped propagating reconciliation failures.
Example
Stub the membership upsert to succeed and keep reconcileRfc64CatalogResponsibilityV1 pending. upsertContextGraphMember(record, { strict: true }) should remain pending; after reconciliation rejects, the returned promise should reject with that error. Repeat without a membership store.
Suggested direction
Add tests that independently control persistence and reconciliation so reverting to the previous fire-and-forget behavior would fail.
For Agents
Add focused tests around upsertContextGraphMember in the membership lifecycle tests. Cover both configured-store and storeless strict calls, proving reconciliation is awaited and its rejection is visible; retain the existing detached/log-and-continue behavior for non-strict calls.
| } | ||
| try { | ||
| const provider = createRouteEvmProvider(rpcUrl, chain?.rpcUrls); | ||
| const provider = createRouteEvmProvider(rpcUrl, chain?.rpcUrls, routeRpcTransport); |
There was a problem hiding this comment.
🟡 Issue: The governed wallet-balance route is not exercised
What's wrong
A user-facing route was moved onto a new governed provider and failover path, but no test executes that path. Wiring failures, missing request context, or incorrect admission/accounting for wallet balance RPCs could therefore ship while all added tests remain green.
Example
Start a loopback RPC with one configured operational wallet, call GET /api/wallets/balances, and assert the returned native/token balances plus governor and route-usage counts. A saturated-governor case should verify that no RPC reaches the loopback endpoint and the route returns its documented bounded error response.
Suggested direction
Cover the real server route with the daemon route transport rather than testing only the transport’s health-probe method or client URL construction.
For Agents
Add a CLI route test for /api/wallets/balances using createDaemonRpcRuntime and a loopback JSON-RPC server. Exercise routeTransport.createProvider, native and token reads, accounting/admission, and local-capacity failure behavior.
There was a problem hiding this comment.
Fixed in follow-up PR #2597 (commit e6076a0): /api/wallets/balances now has real route coverage through createDaemonRpcRuntime. The healthy case proves native/token balances, method accounting, and governor admissions; the saturated case proves the bounded error and zero upstream RPC hits. The focused CLI run passes (57 tests).
3 participants
Summary
Adds a process-wide RPC request governor that bounds outbound EVM RPC demand while keeping publishing and control-plane operations responsive.
The governor is shared by the agent and every publisher wallet adapter in one daemon. It sits immediately before the HTTP transport, so initial requests, ethers retries, and endpoint fallbacks all consume the same budget. RFC-64 authority, policy, and native reconciliation reads are classified as background work and cannot consume the foreground reservation.
This is enabled by default; operators can override the policy per field through
chain.rpcRequestBudget.User-visible effect
Default policy
maxRequestsPerSecond10foregroundReservePercent802 req/s; foreground can use the full10 req/sburstRequests20maxQueueSize256startupJitterMs30000Example operator override:
{ "chain": { "rpcRequestBudget": { "maxRequestsPerSecond": 8, "foregroundReservePercent": 75, "burstRequests": 16, "maxQueueSize": 128, "startupJitterMs": 45000 } } }Network defaults and operator config merge per field, with operator values taking precedence. Invalid limits fail during configuration resolution.
Request flow
sequenceDiagram autonumber participant Worker as RFC-64 catalog worker participant User as Publish / control request participant Provider as Shared EVM provider pool participant Governor as Process RPC governor participant RPC as RPC endpoint Worker->>Provider: Chain read in background context Provider->>Governor: acquire(background) alt Background share available Governor-->>Provider: permit (total + background token) Provider->>RPC: JSON-RPC request else Background share exhausted or startup jitter active Governor-->>Provider: bounded queue / defer end User->>Provider: Foreground chain operation Provider->>Governor: acquire(foreground) Governor-->>Provider: priority permit from total budget Provider->>RPC: JSON-RPC request alt Endpoint asks for retry RPC-->>Provider: 429 / transient 5xx Provider->>Governor: acquire(same request class again) Governor-->>Provider: permit or defer Provider->>RPC: retry else Endpoint fallback Provider->>Governor: acquire(same request class on fallback) Governor-->>Provider: permit or defer Provider->>RPC: request via next endpoint endObservability
Each RPC telemetry window now includes one
rpc_request_governorstructured log line with:GET /api/statusalso exposesrfc64Catalog.authorityRpcCircuit. Together with the existing per-context-graph operational status, this makes authority circuit state, cooldown, and freshness visible without exposing RPC URLs or private catalog identities.Safety and compatibility
Validation
Rollout check
After deployment, acceptance should confirm:
rpc_request_governor background_admittedremains bounded by the configured background rate over representative windows.maxQueueSizecontinuously.