-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add functional correctness proofs for AArch64-optimized NTT and invNTT in HOL Light #662
Conversation
95845ba
to
e8d9c90
Compare
e8d9c90
to
251ca05
Compare
251ca05
to
3b1266c
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.
Something is still fishy in the Makefile target for object files -- see comments.
Also, the following fails:
make -j8 p256/bignum_montmul_p256.native
$S2N_BIGNUM_DIR/tools/run-proof.sh ./p256/bignum_montmul_p256.native ./p256_output
> Fatal error: exception Failure("dest_cons4: 4-byte inst code 2839547939 != first 4 bytes of []")
Could you also add a brief README
that explains what one should do to fully compile + run a proof, and refer to the nix environment for setup? Is it feasible to run at least one proof in CI to confirm that the setup is functional? (e.g. a "hol-light sanity check" job or similar)
218e722
to
0d4929c
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.
Thank you @potsrevennil for fixing the Makefile issue.
Something is still not right, though:
make -j8 p256/bignum_montmul_p256.correct
continues to fail with
Fatal error: exception Failure("dest_cons4: 4-byte inst code 2839547939 != first 4 bytes of []")
make: *** [Makefile:440: p256/bignum_montmul_p256.correct] Error 2
81db6f5
to
a670c3e
Compare
a670c3e
to
8cbad52
Compare
8cbad52
to
d026c4f
Compare
c486428
to
b8d3fdf
Compare
25776b6
to
051bc32
Compare
I have tried to reproduce proof results on macOS. In short:
|
I note that
which is obviously "not an ELF file". Is this the problem? |
Yes, s2n-bignum/HOL-Light's object file parser only supports ELF |
To confirm this PR is working OK, we need a testcase which is much smaller and faster than either NTT or INTT. What's the smallest, simplest, fastest proof that we can add? |
We've tested small examples before. Now the PR is targeting the actual ML-KEM proofs. |
I have reproduced the proofs successfully on an EC2 Graviton 3 (c7g) machine, running Ubuntu 24.04. |
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.
Thanks @potsrevennil for the work!
Signed-off-by: Thing-han, Lim <[email protected]>
The scripts in s2n_bignum assume execution within the $ROOT/<arch>/ directory and rely on relative paths. To integrate these scripts with Nix, it is necessary to adjust the paths used for loading scripts to ensure compatibility. Additionally, the Makefile located under $ROOT/<arch> has been copied to the mlkem-native directory and modified as needed to support this change. Signed-off-by: Thing-han, Lim <[email protected]>
Signed-off-by: Thing-han, Lim <[email protected]>
Signed-off-by: Thing-han, Lim <[email protected]>
Signed-off-by: Thing-han, Lim <[email protected]>
Signed-off-by: Hanno Becker <[email protected]>
051bc32
to
a1b8105
Compare
Summary:
Steps:
s2n-bignum/arm/
is copied toproofs/hol_light
and adjusted to be compatible with nix.Performed local tests:
lint
passingtests all
passingtests bench
passingtests cbmc
passingDo you expect this change to impact performance: Yes/No
If yes, please provide local benchmarking results.