Skip to content

Conversation

@tcoratger
Copy link
Collaborator

Let me know if this makes sense but I guess that:

  • ExtractBinomialW is not needed since we know at compile time if we have a base field or an extension field and what W is.
  • I know that having a function for base field and one for extension could be a bit of code duplication but this is something that we are used to doing both in whir and Plonky3 for very low level optimizations so I say to myself that it's not so bad to do it here maybe, let me know :)

Copy link
Contributor

@Nashtare Nashtare left a comment

Choose a reason for hiding this comment

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

Overall this looks good, even though I'm not strongly opinionated towards this approach or the existing one, so I'll leave it to the others to see and decide.

FYI your tests are failing because you forgot to update the mmcs_verify example, should be using the new prove_all_tables_extension now. I've pushed a commit to fix it.

@tcoratger
Copy link
Collaborator Author

Overall this looks good, even though I'm not strongly opinionated towards this approach or the existing one, so I'll leave it to the others to see and decide.

FYI your tests are failing because you forgot to update the mmcs_verify example, should be using the new prove_all_tables_extension now. I've pushed a commit to fix it.

No problem, it's just a proposal to avoid overengineering with some additional trait but this comes with the cost of having two base and extension functions separately so it's a choice to make between the two approaches.

If you don't like don't hesitate to close at any time :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants