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

fix compile error when halo2-pse feature active #57

Conversation

zemse
Copy link

@zemse zemse commented Jan 18, 2024

Note: Base branch of this PR is set to release to show the changes. But it is intended to be merged on another branch.

how to reproduce issue?

Change default feature halo2-axiom to halo2-pse in both snark-verifier and snark-verifier-sdk 's Cargo.toml file. And cargo build.

It will give this error:

error[E0277]: the trait bound `Bn256: pairing::MultiMillerLoop` is not satisfied
   --> snark-verifier-sdk/src/evm.rs:119:54
    |
119 |     > + AccumulationDecider<G1Affine, Rc<EvmLoader>, DecidingKey = KzgDecidingKey<Bn256>>;
    |                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `pairing::MultiMillerLoop` is not implemented for `Bn256`
    |
    = note: required for `Bn256` to implement `snark_verifier::util::arithmetic::MultiMillerLoop`
note: required by a bound in `KzgDecidingKey`
   --> /Users/sohamzemse/Workspace/axiom/static-call-work/snark-verifier/snark-verifier/src/pcs/kzg/decider.rs:15:30
    |
15  | pub struct KzgDecidingKey<M: MultiMillerLoop> {
    |                              ^^^^^^^^^^^^^^^ required by this bound in `KzgDecidingKey`

Applying the patch in this PR fixes this issue.

@jonathanpwang
Copy link

Oh is that all that was necessary? nice

@jonathanpwang
Copy link

@zemse After this PR #59 and updating the halo2-pse halo2_proofs to v0.3.0, it seems the import error is gone. Please confirm, and then we can close this.

@zemse
Copy link
Author

zemse commented Jan 19, 2024

Yes, this is not needed.

@zemse zemse closed this Jan 19, 2024
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