Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce AsyncProver #275

Closed
wants to merge 82 commits into from
Closed

Conversation

plafer
Copy link
Contributor

@plafer plafer commented May 2, 2024

Based on #271.

Introduced an asynchronous version of Prover (under a new "async" feature). A few notes:

  • Uses the new return-position impl Trait in traits of Rust 1.75. I chose not to use the async syntactic sugar, as it doesn't allow us to specify + Send + Sync on the return Futures (as explained in the linked blog post).
    • This is a pragmatic solution, since the compiler team currently recommends using the trait-variant crate, but the trait_variant macro didn't like our E: FieldElement<BaseField = Self::BaseField> bound in generate_proof() (with a difficult to decipher error message).
    • Also, I'm not sure that I like generating another trait just to get + Send bounds. So I decided to only support the + Send + Sync variant, so that it would work with tokio's spawn().
  • We should fix the tracing spans in a few spots if we decide to go down this route
  • After Generalize auxiliary trace building #271 is merged, I would split Prover::generate_proof() into many sub- functions, and call the appropriate ones in AsyncProver::generate_proof() so as to remove the amount of copy/paste code
  • Personal TODO: make sure the CI will properly build/check the new "async" feature

I'm happy to revisit any of these assumptions 🙂.

@plafer plafer changed the base branch from main to next May 2, 2024 17:40
@plafer
Copy link
Contributor Author

plafer commented May 3, 2024

Closing as we will go in a different direction

@plafer plafer closed this May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants