-
Notifications
You must be signed in to change notification settings - Fork 510
Deduplicate crypto libraries, saving ~49KB flash #1632
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: dev
Are you sure you want to change the base?
Deduplicate crypto libraries, saving ~49KB flash #1632
Conversation
Fixes ed25519_verify() stack overflow by making large locals static, removing the need for rweather's Ed25519::verify() workaround. Vendors only the AES128+SHA256 subset of rweather/Crypto instead of all 37 files.
weebl2000
left a comment
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.
Nice improvement — vendoring the minimal subset is clean and the stack savings are real (~1.8KB moved to BSS). One concern about the static locals in ge.c.
JosiahWI
left a comment
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.
The code looks great. I'm giving my capstone team an opportunity to build this PR and test it and then I'll approve. :)
25b64dd to
48019bf
Compare
…_verify to ensure thread safety during concurrent calls.
JosiahWI
left a comment
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.
The reduction in program size from this PR will be very useful for my school capstone. One of my teammates has built our branch with it and confirmed that the Flash saving makes it possible to fit the program on an nRF52840 Dongle alongside an InternalFS and DFU bootloader.
Fixes a stack overflow in ed25519_verify() by making large local variables static, removing the need for rweather's Ed25519::verify() workaround.
Only vendors the AES128+SHA256 subset of rweather/Crypto instead of all 37 files.
No issues observed so far. Additional testing and confirmation are welcome. This change frees up stack space for future improvements and improves support for resource-limited devices.