feat(zk-host): enable all configured proving backends by mw2000 · Pull Request #3920 · base/base

mw2000 · 2026-07-10T19:29:03Z

Summary

Enables one ZK host to initialize every configured proving backend and route each request to its selected backend. Backend availability is inferred from configuration presence, with shared witness-provider setup centralized in base-proof-zk-backend and the legacy ZK_BACKEND selector removed.

cb-heimdall · 2026-07-14T15:52:06Z

✅ Heimdall Review Status

Requirement Status More Info

Reviews

✅ 1/1

Denominator calculation

Show calculation

1 if user is bot 0

1 if user is external 0

2 if repo is sensitive 0

From .codeflow.yml 1

Additional review requirements

Show calculation

Max	0

0

From CODEOWNERS 0

Global minimum 0

Max 1

1

1 if commit is unverified 1

Sum 2

Discover backend availability from its existing settings so one host can serve mock, dry-run, cluster, and network requests without a selector environment variable. Co-authored-by: Cursor <cursoragent@cursor.com>

Preserve mock-only compatibility for stale partial RPC settings, reuse parsed RPC configuration, and clarify presence-based backend enablement. Co-authored-by: Cursor <cursoragent@cursor.com>

Exercise mock, dry-run, cluster, and network enablement together while making optional string parsing accept borrowed string options directly. Co-authored-by: Cursor <cursoragent@cursor.com>

Keep binary wiring minimal by moving backend validation and shared prover setup into the backend crate, while removing stale selector configuration. Co-authored-by: Cursor <cursoragent@cursor.com>

Treat either S3 setting as cluster intent while keeping the default dry-run deployment free of implicit cluster configuration. Co-authored-by: Cursor <cursoragent@cursor.com>

Keep backend-specific invariants in focused resolvers and derive shared witness RPC settings from the resolved backend configs to prevent drift. Co-authored-by: Cursor <cursoragent@cursor.com>

Removed outdated information about RPC configuration and backend settings.

Report backend-specific validation failures before tolerated mock-only RPC errors, and warn when incomplete RPC settings are intentionally ignored. Co-authored-by: Cursor <cursoragent@cursor.com>

Build independent configured backends in parallel so expensive key setup does not serialize multi-backend startup. Co-authored-by: Cursor <cursoragent@cursor.com>

Drain parallel initialization tasks on failure or cancellation and reject mismatched RPC settings before sharing a witness provider. Co-authored-by: Cursor <cursoragent@cursor.com>

Keep canonical RPC resolution alongside backend configs and consolidate parallel task error handling without changing startup behavior. Co-authored-by: Cursor <cursoragent@cursor.com>

Validate that at least one backend is configured at the config boundary, ignore orphan S3 settings without a cluster endpoint, and preserve the Docker region default. Co-authored-by: Cursor <cursoragent@cursor.com>

github-actions · 2026-07-14T17:08:00Z

Review Summary

The PR successfully moves from a single ZK_BACKEND enum selector to presence-based multi-backend configuration. The refactoring is structurally sound — validation logic is centralized in SuccinctZkProversConfig, witness provider construction is shared, and backends are initialized concurrently via JoinSet. The CLI simplification is a clear improvement.

Key findings (see inline comments for details)

Correctness / Behavior change risks:

Dry-run is implicitly enabled whenever RPC URLs are present. Operators configuring only cluster or network backends will get an unexpected dry-run prover consuming CPU/memory on every job. This is a silent behavior change from the old explicit ZK_BACKEND selector.
Partial RPC errors are silently swallowed in mock-only mode (Err(_) arm in backend_configs). Operators with stale partial RPC env vars get mock-only behavior with no indication their RPC config was ignored (addressed with a warn! log, but this is still a subtle behavior change).
USE_KMS_REQUESTER=true without NETWORK_PRIVATE_KEY is now an error even with mock enabled, but stale CLI_S3_BUCKET/CLI_S3_REGION without SP1_CLUSTER_API_ENDPOINT is silently ignored — inconsistent validation semantics.

API design:

The BackendConfigs tuple returns rpc separately AND embeds it in each SuccinctZkBackendConfig variant, creating dual ownership with an implicit invariant that they must always match.
SuccinctZkProversConfig has 17 public fields with no Default impl, making test construction verbose.

Testing:

Tests cover single-backend and edge cases well, but don't exercise the core multi-backend scenario (all backends enabled simultaneously with full config validation).

Overall: the approach is reasonable, but the implicit dry-run activation and the dual-RPC-ownership pattern warrant attention before merge.

github-actions · 2026-07-14T17:09:14Z