-
Notifications
You must be signed in to change notification settings - Fork 23
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
Differential test to verify all implementations #24
Comments
So, to implement this we need to add CI which would raise a warning when some consistency checks are failed? Or is there a better approach? If CI it is, is there any preference towards a particular system? |
Yup, CI is great! All the tests should read from the same configuration file or something that has the inputs and outputs, so the different languages are all verified to match! |
So, we need following?
|
Yup |
I see no good way to actually provide a single source for automated reading test vectors while autotest. It will degrade development experience significantly: introduction of file reading during running tests, decoding of the values into native format, using a file outside of working dir (referencing top level). There's few strategies to tackle this one; though they're not simple.
|
So, I added basic CI in 9a068af and ccd26a8 for all four entities the repo provides as you suggested here and at #30 (comment). Let me unassign myself while some direction regarding the analysis in the previous comment would be chosen. |
These are great points and ideas. I was thinking more a test.env in each of the repos and a CI that verifies it is the same, rather han a config file per se, but your point eegarding parsing etc still stands. An 80/20 for now is independent CIs for now with comments that they should be kept consistent with the other files. I agree with you that the easiest thing is to add logging/reporting to a file and ensure the outputs are the same in all cases. |
I didn't really looked below Jest in So I personally would tackle this one further in the following two streams.
So I'd propose to convert this issue into a test vectors document with known/potential inconsistencies section. Then based on this doc start to introduce sync automation. Which could eventually be absorbed by |
Sweet, that sounds great. A circom test can use circom-tester library to do witness generation at least within tests, which is fast; proofgen will work 99% of the time if witnessgen works so I think it's pretty good substitute. |
It's important to have consistency across these impls. A few examples:
c
is returned as a plain number in js, circom vs a field element in rust.You should run a differential test on random values for message and private key (maybe pre-generate and freeze them).
The text was updated successfully, but these errors were encountered: