-
Notifications
You must be signed in to change notification settings - Fork 41
feat: add Android 16KB page size support #57
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
Conversation
|
Thanks. This is a pretty big PR - if we just wanted 16KiB support, we could have had that much more simply. On the other hand, updating the build system is certainly worthwhile, so I'm happy to do the review. It might just take a bit longer. |
|
@swansontec yes this is mainly due to the reboot of the FastCryptoTest project on the latest React Native stack (new arch & latest version to be ready for 16kb and to be able to test end2end on the 16kb simulator) |
swansontec
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.
I still need to review the FastCryptoTest project, but here are my initial thoughts about the changes to react-native-fast-crypto itself.
Since this PR includes changes to the binaries, we do risk supply-chain attacks. Before I merge it, I'll be sure to re-run the build so we have known-good .so files.
swansontec
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.
We don't actually need the scripts/utils/android-tools.js file.
|
I have been unable to build libsecp256k1 with these changes, sadly. Instead of trying to fix the the old deps system, I've started a branch that copies over the react-native-zano build system, which is written in TypeScript. So far I have the Android side building, and should have iOS done soon. I understand that the deadline for 16KiB support is the end of the month, so I do want to get this working & published. |
|
Here is the follow-on PR: #58 I kept your FastCryptoTest upgrade pretty much as-is, so thanks for that. |
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
Description
In order to be compatible with upcoming Android requirements, we must upgrade NDK and all build tools to be able to support 16 KB page size. Fixes #56
The FastCryptoTest app were upgraded to latest React Native and is confirmed to works on Android 16KB simulator and pass all the tests. I also confirm it works on iOS.