feat: delegated aggregation#63
Conversation
*Adds runtime flags to configure delegated ZK aggregation, including RPC access, credentials, worker limits, reward thresholds, claim strategy, and dry-run support. *Validates aggregation settings at startup and logs the active setup so operators can enable the worker more safely and predictably. *Covers the new arguments with parsing tests to reduce CLI regressions.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| zk_config.validate()?; | ||
| log::info!( | ||
| "ZK aggregation enabled: rpc={}, workers={}, max_active_jobs={}, strategy={:?}, dry_run={}", | ||
| zk_config.node_rpc, |
There was a problem hiding this comment.
🔒 Agentic Security Review
Severity: HIGH
The startup log emits the full node_rpc value when ZK aggregation is enabled. RPC URLs commonly include credentials or API tokens in userinfo/query components, so this can leak secrets to shared log sinks.
Impact: Leaked RPC credentials can be reused to access privileged provider endpoints and expose downstream chain/account operations.
| aggregation_account: Option<String>, | ||
|
|
||
| /// Aggregation signing key or keystore path | ||
| #[arg(long = "aggregation-key", env = "MINER_AGGREGATION_KEY")] |
There was a problem hiding this comment.
🔒 Agentic Security Review
Severity: HIGH
[Privacy Guard] The new --aggregation-key flag accepts raw signing key material directly via CLI/environment as a plain string, which is exposed through process args, shell history, and environment propagation.
Impact: Exposure of this key enables unauthorized delegated-aggregation signing and account takeover for aggregation actions.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c558fab. Configure here.
| serde_json = { version = "1.0.132", default-features = false } | ||
| thiserror = "1" | ||
| tokio = { version = "1.36", features = ["full"] } | ||
| tempfile = "3.8" |
There was a problem hiding this comment.
Workspace dependency ordering will fail CI taplo check
Medium Severity
The tempfile = "3.8" entry is placed after tokio (line 39) but alphabetically it belongs before thiserror (line 38). The project's taplo.toml enforces reorder_keys = true for workspace.dependencies, and CI runs taplo format --check --config taplo.toml, so this ordering violation will fail the "Fast Checks" CI job.
Reviewed by Cursor Bugbot for commit c558fab. Configure here.
| ); | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Validation missing for required non-dry-run fields
Low Severity
validate() doesn't verify that aggregation_account and aggregation_key are present when dry_run is false. A user enabling ZK aggregation in production mode without providing these fields passes validation, but actual aggregation (claiming and proving) would require them for on-chain transaction signing. The gap allows an invalid configuration to silently start the miner.
Reviewed by Cursor Bugbot for commit c558fab. Configure here.
*Enables end-to-end delegated aggregation so workers can validate pending proofs, choose profitable batches, claim bundles, generate L1 proofs, and submit results. *Improves safety by checking required proving artifacts, validating public inputs and duplicate nullifiers locally, and requiring a miner bond outside dry runs. *Updates the CLI, docs, and tests to cover the new worker configuration, selection strategies, and proving path.
*Requires a secure keystore path and account for live aggregation, while still allowing dry runs, to prevent weak signing setup and reduce secret exposure in logs. *Revalidates claimed proofs after claiming and verifies aggregate proofs locally before submission so tampered inputs and invalid aggregates are caught before they reach the network.
1 participant
Note
Medium Risk
Adds new runtime configuration and CLI/env surface area for delegated ZK aggregation; while currently mostly validation/logging, it can change startup behavior and introduces new failure modes (e.g., required filesystem paths).
Overview
Adds an optional delegated ZK L1 aggregation configuration path to the miner, gated behind
--enable-zk-aggregation(and related env vars) with new CLI flags for RPC endpoint, account/key, artifacts directory, worker/job limits, min reward threshold, claim strategy, and a dry-run mode.Introduces
miner-service::zk_aggregationwithZkAggregationConfigvalidation (e.g., requires an existing--zk-bins-dir) and wires it intoServiceConfig/run()for validation and startup logging; includes basic unit tests and addstempfilefor dev/test support (plus lockfile updates).Reviewed by Cursor Bugbot for commit c558fab. Bugbot is set up for automated code reviews on this repo. Configure here.