Skip to content

Conversation

@Artemka374
Copy link
Contributor

@Artemka374 Artemka374 commented Jan 23, 2025

What ❔

Before Prover Gateway was polling ProofDataHandler for new proof generation data. This PR refactors interfaces of ProofDataHandler and implements the interaction backwards - ProofDataHandler periodically submits proof generation data to the gateway.

Why ❔

These changes were done in advance to make Prover Cluster possible

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zkstack dev fmt and zkstack dev lint.

@Artemka374 Artemka374 requested a review from EmilLuta January 30, 2025 09:18
Copy link
Contributor

@EmilLuta EmilLuta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a bunch of comments. I'd love to hear more about your opinion on locking batches and maybe add a few more tests?

All in all looks good, let me know what you think!

main_pool,
commitment_mode: self.commitment_mode,
l2_chain_id: self.l2_chain_id,
let processor = RequestProcessor::new(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this implementation relies on the HTTP communication without authentication. Do we consider moving this to RPC still communication? Asking as we have 2 resources (2 DB connections, one on API, one on requester) and more exposed interfaces (gateway needs to know about URL), whilst RPC would make this communication much simpler (data handler knows about gateway and that's it -- single resource utilization as well).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but that will be implemented separetely

general_config.insert("prover_gateway.api_url", format!("http://127.0.0.1:{port}"))?;
}

let prover_gateway_port = general_config
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this is needed (alone). Wouldn't the same be needed for URL (imagine when starting a new chain) and poll timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that I understand the question. This is needed, since zkstack reallocates default ports, and there might be port conflicts.

@@ -0,0 +1,24 @@
use axum::http::StatusCode;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this a duplicate of middleware in proof_data_handler? Couldn't we simply unify these 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, the same we have for proof_integration_api. I think we probably can do some macro-based general implementation for this kind of middleware.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(But I would prefer to do this separately)

@Artemka374 Artemka374 closed this Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants