-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(prover): WitnessGenerator refactoring #2 #2899
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM. Left a bunch of nits. Here's 1 not very popular opinion:
I believe we can have a single binary for all witness generators (except basic -- and it may include proof compressor as well), because they mostly do the same thing, just load slightly different artifacts & operate on different tables.
Basic Witness Generator is different (because it's not really a witness generator anyways, it runs the VM and does a lot of other things alongside).
Whilst I don't think we should change the approach in this PR, I'd think about it. This setup is already better than what we had before, but I'm super positive we can get away with a minimal amount of custom code for leaf/node/rt/scheduler.
Actually I beliebe we really can do it the way that Leaf/node/rt/scheduler have the minimum amount of different code and I think much more changes can be done on this field, but I want to do these changes step by step(to not break anything). But yes, I think we can do something like that in the next PRs |
What ❔
Introduce WitnessGenerator trait
Rename some methods
Why ❔
Checklist
zk fmt
andzk lint
.