-
Notifications
You must be signed in to change notification settings - Fork 360
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
refactor: clean up witness package, introduces clean witness.Witness
interface
#450
Conversation
…uple schema.Schema
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.
Looks all good. A few minor comments but otherwise good to go. I think this change makes a lot of sense and makes the whole binary representation of witness so much more clear and practical to use.
I don't understand why the tests are failing though. May it due to double filling in FromJSON?
Motivation for this PR
#445 and #442 . + decouple *schema.Schema from witness data structure; we only need a
schema.Schema
when doingJSON
(un)marshaling, which happens for pretty printing or in the playground. This had a significant cost in cpu + in storage, and in 99% of cases we didn't use it.See
witness.ExampleWitness
for usage andwitness.Witness
interface for the gist of the PR.Additionally,
groth16
plonk
andconstraint
packages now directly reason with[]fr.Element
(as they should).Breaking changes;
[uint32(nbPublic)|uint32(nbSecret)|uint32(nbElements)|elements...]
instead of[uint32(nbElements)|elements...]
.fr.Vector
manage its serialization (in gnark-crypto), but a witness needs these 2 extra uint32 to ensure consistency when serializing / deserializing.