-
Notifications
You must be signed in to change notification settings - Fork 23
Add mldsa-native.h API header file #537
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Jake Massimo <[email protected]>
); | ||
|
||
#define crypto_sign MLD_NAMESPACETOP | ||
#define crypto_sign MLD_NAMESPACE(sign) |
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.
We will merge #532 that removes namespacetop, so won't have this issue on rebase.
Signed-off-by: Jake Massimo <[email protected]>
Signed-off-by: Jake Massimo <[email protected]>
#include "../../mldsa/sign.h" | ||
#define MLD_CONFIG_API_PARAMETER_SET MLD_CONFIG_PARAMETER_SET | ||
#define MLD_CONFIG_API_NAMESPACE_PREFIX mldsa | ||
#include "../../mldsa/mldsa_native.h" |
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.
let's change this to #include "mldsa_native/mldsa/mldsa_native.h
and add the symlink mldsa_native
.
Like in mlkem-native
mldsa/ntt.c mldsa/ntt.h mldsa/packing.c mldsa/packing.h mldsa/params.h mldsa/poly.c | ||
mldsa/poly.h mldsa/polyvec.c mldsa/polyvec.h mldsa/randombytes.h mldsa/reduce.h | ||
mldsa/rounding.h mldsa/sign.c mldsa/sign.h mldsa/symmetric.h mldsa/sys.h mldsa/zetas.inc | ||
mldsa/mldsa_native.h mldsa/ntt.c mldsa/ntt.h mldsa/packing.c mldsa/packing.h mldsa/params.h |
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.
liboqs does not actually need that file. Let's exclude it.
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.
@jakemas IIRC the plan was to have the monobuild files in the monobuild example folder first, and eventually restructure everything to follow the directory pattern as in mlkem-native (with mldsa/src/*
for the bulk of the source, and mldsa/mldsa_native.*
for the toplevel). I don't think we want mldsa_native.c
in the toplevel directory.
Why is mldsa_native.c
needed for this change at all, actually?
When the file
mldsa-native.c
is within the directorymldsa/
we get issues with the Windows CI was failing with duplicate symbol errors because the NMAKE wildcard pattern{mldsa}.c
was including both individual source files AND the single compilation unitmldsa_native.c
, causing conflicts. I movedmldsa-native.c
up one directory to solve this, but we may instead want to switch over to the mlkem-nativesrc/
style.Implemented:
Merging #532 first will clean up NAMESPACETOP issues, so should be done before this PR.
Testing CI, once scope and review is happy, I will remove fix ups, create a nice commit message etc.