-
Notifications
You must be signed in to change notification settings - Fork 97
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
Adds tests against the wycheproof test vectors #630
Adds tests against the wycheproof test vectors #630
Conversation
442bb1f
to
a899629
Compare
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. The 3 test cases we skip sound reasonable to me.
Can you please put a comment here with:
- old binary size
- new binary size
- difference in kB
Thanks!
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. The 3 test cases we skip sound reasonable to me.
Can you please put a comment here with:
- old binary size
- new binary size
- difference in kB
Thanks!
It looks like the subzero core binary failed to start in the background in the SignTx regression test action. Hard to tell what's going on, we probably need to improve the logic in the action so it's easier to get logs output. Did you run it locally with ASAN + UBSAN without any issues? |
a899629
to
81a3d9b
Compare
Ran locally with Added sleeps in the CI script and the tests execute successfully which confirms the theory. |
Old binary size: 546916 bytes |
Approving with some nits, please fix them before merging. |
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.
Actually approving this time.
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
Along the same spirit as the secp256k1 PR 1245. This tests against all 463 vectors but includes exclusions for 3 of them. Comments include the details outlining why each was excluded. The json and c code was generated using the included python code (utils/tests_wycheproof_generate.py).
642c433
to
069a863
Compare
Along the same spirit as the secp256k1 PR 1245. This tests against all 463 vectors but includes exclusions for 3 of them. Comments include the details outlining why each was excluded.
The json and c code was generated using the included python code (utils/tests_wycheproof_generate.py).
We decided not to duplicate this generation code for now.