refactor: use configurable prover url for batch operations#1831
refactor: use configurable prover url for batch operations#1831sergeytimoshin merged 2 commits intomainfrom
Conversation
WalkthroughThis update introduces configurable prover client parameters throughout the proof generation workflow. Functions responsible for batch proof generation now accept and propagate Changes
Sequence Diagram(s)sequenceDiagram
participant Config
participant EpochManager
participant BatchContext
participant Processor
participant ProofClient
Config->>EpochManager: Provide prover_url, polling_interval, max_wait_time
EpochManager->>BatchContext: Initialize with prover config fields
Processor->>BatchContext: Access prover config
Processor->>ProofClient: Instantiate with config (prover_url, etc.)
Processor->>ProofClient: Generate ZKP proofs (shared client)
ProofClient-->>Processor: Return proofs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (20)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
forester-utils/src/instructions/state_batch_append.rs (1)
92-96: Remove unnecessary clone ofprover_urlSince
prover_urlis already owned by this function (passed by value), the.clone()on line 93 is unnecessary.- let proof_client = Arc::new(ProofClient::with_config( - prover_url.clone(), - polling_interval, - max_wait_time, - )); + let proof_client = Arc::new(ProofClient::with_config( + prover_url, + polling_interval, + max_wait_time, + ));forester-utils/src/instructions/state_batch_nullify.rs (1)
132-136: Remove unnecessary clone ofprover_urlSame as in the previous file, since
prover_urlis already owned by this function, the.clone()is unnecessary.- let proof_client = Arc::new(ProofClient::with_config( - prover_url.clone(), - polling_interval, - max_wait_time, - )); + let proof_client = Arc::new(ProofClient::with_config( + prover_url, + polling_interval, + max_wait_time, + ));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
forester-utils/src/instructions/address_batch_update.rs(3 hunks)forester-utils/src/instructions/state_batch_append.rs(5 hunks)forester-utils/src/instructions/state_batch_nullify.rs(4 hunks)forester/src/epoch_manager.rs(1 hunks)forester/src/processor/v2/address.rs(1 hunks)forester/src/processor/v2/common.rs(2 hunks)forester/src/processor/v2/state.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test sdk-libs
- GitHub Check: Test program-libs-slow
- GitHub Check: Test program-libs-fast
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: lint
- GitHub Check: stateless-js-v1
- GitHub Check: test-2-foresters
- GitHub Check: test-double-registration
- GitHub Check: test-e2e
- GitHub Check: test-state-batched
- GitHub Check: test-state-photon-batched
- GitHub Check: test-address-batched
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v2
- GitHub Check: cli-v2
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
forester/src/processor/v2/common.rs (2)
1-1: LGTM - Duration import added for new fields.The Duration import is correctly added to support the new prover timing configuration fields.
29-31: LGTM - BatchContext properly extended with prover configuration.The three new fields are well-named and appropriately typed. The public visibility is correct for a context struct that needs to propagate configuration through the processing pipeline.
forester/src/processor/v2/address.rs (1)
26-28: LGTM - Prover configuration properly propagated.The function call correctly passes the new prover configuration parameters from the BatchContext. The
.clone()onprover_urlis appropriate for the String type.forester/src/processor/v2/state.rs (2)
39-41: LGTM - Append operation updated with prover configuration.The function call correctly passes the prover configuration parameters from the BatchContext for the append batch operation.
147-159: LGTM - Nullify operation updated with prover configuration.The function call correctly passes the prover configuration parameters from the BatchContext for the nullify batch operation. The parameter propagation is consistent with the append operation.
forester-utils/src/instructions/address_batch_update.rs (3)
1-1: LGTM - Duration import added for new parameters.The Duration import is correctly added to support the new prover timing configuration parameters.
32-34: LGTM - Function signature properly extended.The three new parameters are well-named and appropriately typed. The parameter order is logical and consistent with other similar functions in the codebase.
261-261: LGTM - ProofClient properly configured with custom parameters.The change from
ProofClient::local()toProofClient::with_config()correctly utilizes the configurable prover parameters, enabling external prover service configuration.forester-utils/src/instructions/state_batch_append.rs (1)
147-148: Good refactoring to share ProofClient across concurrent operationsThe change to pass a shared
ProofClientinstance instead of creating one per proof generation is a solid performance improvement. UsingArcfor thread-safe sharing is the correct approach.Also applies to: 174-177
forester-utils/src/instructions/state_batch_nullify.rs (1)
223-242: Good simplification of concurrent proof generationThe removal of
tokio::spawnwrappers is a good simplification.futures::future::join_allalready executes the futures concurrently, so the extra task spawning was unnecessary overhead. The error handling is also cleaner and more direct.
Co-authored-by: Swen Schäferjohann <42959314+SwenSchaeferjohann@users.noreply.github.com>
2 participants
Summary by CodeRabbit
New Features
Refactor