-
Notifications
You must be signed in to change notification settings - Fork 50
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/hypernova serialization #159
Conversation
…, proof and public inputs
584989e
to
d6e0c41
Compare
@@ -309,4 +311,201 @@ pub mod tests { | |||
assert!(verified); | |||
println!("Decider verify, {:?}", start.elapsed()); | |||
} | |||
|
|||
#[test] | |||
fn test_decider_serialization() { |
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.
Just not sure on why do we have timers in this test. We can just write formal benchmarks if we want any no?
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.
Yup, I just followed what was done with nova, I don't mind removing timers. I agree that we could initiate some benchmarks, but I'm not sure that's on the priority list right now? Wdyt? cc @arnaucube
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.
sure, I use to place dbg!
to know the timings while I test things, but agree with removing them.
Regarding benchmarks, I think that it's not a priority since we're still in the 'exploration phase' where things are not much optimized and we know for sure that are slower than they would be.
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.
LGTM!
Thanks for this @dmpierre !
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.
Great!! LGTM ^^
Implements serde for hypernova prover and verifier params, proof and public inputs. Serding the ccs could make serialization costs increase quickly, so I did not implement the
CanonicalDeserialize
trait forHypernova
,ProverParams
andVerifierParams
.For testing, I followed the same logic as what is being done in
test_decider_serialization
for nova. Testing is done here.