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

cost-model.rs: Correct lookup required degree calculation. #549

Merged
merged 1 commit into from
Apr 18, 2022

Conversation

therealyingtong
Copy link
Collaborator

Closes #539

Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

ACK

@@ -125,7 +125,7 @@ impl FromStr for Lookup {

impl Lookup {
fn required_degree(&self) -> usize {
1 + cmp::max(1, self.input_deg) + cmp::max(1, self.table_deg)
2 + cmp::max(1, self.input_deg) + cmp::max(1, self.table_deg)
Copy link
Contributor

Choose a reason for hiding this comment

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

I confirmed that the reason for this bug was that this cost model example was implemented before the lookup argument was made perfectly zero-knowledge: 1f1b705

I also agree that this happens to ensure the lowest returned degree is 4, matching the real code.

@str4d str4d merged commit dc2e454 into main Apr 18, 2022
@str4d str4d deleted the patch-cost-model-lookup branch April 18, 2022 09:52
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.

Cost model miscalculates the degree of lookup arguments
3 participants