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

Error handling for InvalidEvalDomain improvement #86

Open
CPerezz opened this issue Jan 11, 2022 · 1 comment
Open

Error handling for InvalidEvalDomain improvement #86

CPerezz opened this issue Jan 11, 2022 · 1 comment
Labels
D-easy Difficulty: easy P-low Priority: low T-refactor Type: cleanup/refactor

Comments

@CPerezz
Copy link
Contributor

CPerezz commented Jan 11, 2022

We should do something like:

.ok_or(Error::InvalidEvalDomainSize {
                log_size_of_group: plonk_verifier_key.n.trailing_zeros(),
                adacity: <F as FftField>::FftParams::TWO_ADACITY,
            })?

instead of unwrap() but I'm having some issues implementing this.
I could just remove the parameters from InvalidEvalDomainSize or something quick, or leave it as unwrap and open another issue/PR for this case (since it's rather independent)

Originally posted by @joebebel in #61 (comment)

@CPerezz CPerezz added D-easy Difficulty: easy P-low Priority: low T-refactor Type: cleanup/refactor labels Jan 11, 2022
@ghost
Copy link

ghost commented Jan 11, 2022

I eventually added some workaround, but I think it could use some additional work. Maybe the GeneralEvaluationDomain can be constructed in one place instead of the several places it is now.

Also it might be better if GeneralEvaluationDomain::new returns a Result instead of an Option, not sure about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D-easy Difficulty: easy P-low Priority: low T-refactor Type: cleanup/refactor
Projects
None yet
Development

No branches or pull requests

1 participant