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

refactor: move prove_from_frames into Prover trait #1238

Merged
merged 1 commit into from
Apr 12, 2024

Conversation

arajasek
Copy link
Contributor

Closes #1217.

  • Adds lang() method to the Prover trait
  • Adds prove_from_frames() method to the Prover trait with the default implementation extracted from both Nova and SuperNova
  • Adds a new from_frames() method to the Prover trait to assist the prove_from_frames() method
    • Better name suggestions welcome here

@arajasek arajasek requested review from a team as code owners April 11, 2024 17:19
fn lang(&self) -> &Arc<Lang<F, C>>;

/// Converts input into Self::Frames according to the rules of the Prover
fn from_frames(
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 wasn't thrilled about having to add this method, but I didn't have a better way. Suggestions welcome!

Copy link
Member

Choose a reason for hiding this comment

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

Can you provide the default implementation in the trait itself so we don't need to replicate them on both provers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the problem is that Frame is an associated type of the Prover trait, so I can't make it specific to MultiFrame (or any other concrete type). I could make it generic over Frame, but that seems bad to me.

@arajasek arajasek requested a review from a team as a code owner April 11, 2024 23:01
@arajasek
Copy link
Contributor Author

Force-pushed to fix up linter, sorry about that.

Copy link
Member

@arthurpaulino arthurpaulino left a comment

Choose a reason for hiding this comment

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

Thanks!!

@arthurpaulino arthurpaulino added this pull request to the merge queue Apr 12, 2024
Merged via the queue into argumentcomputer:main with commit a351957 Apr 12, 2024
11 checks passed
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.

Move lang and prove_from_frames to the Prover trait
3 participants