Skip to content

Add secp256k1 & brainpool EC curves (rebased) #78

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

Merged
merged 7 commits into from
Jun 18, 2025

Conversation

sake
Copy link
Contributor

@sake sake commented Jun 16, 2025

This MR is based on the work of #60.

It rebases the work against the current main branch, solving a few minor merge conflicts.
It furthermore addresses the review comments regarding the println statements in EcdsaTest.kt.

Copy link
Owner

@whyoleg whyoleg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
I've started the CI checks and will merge after they finished.

@sake
Copy link
Contributor Author

sake commented Jun 17, 2025

Regarding the OOM error in the Android tests, I looked at the code again and I can not see any place which could introduce a memory leak.
Did you have any problems running these tests in the CI?

@whyoleg
Copy link
Owner

whyoleg commented Jun 17, 2025

I beleive there are no memory leaks there, but probably the issue with low memory or slow GC on android emulator.
In this PR we increased the amount of curves under testing and so more objects were generated.

Later today, I will merge some of my other changes, which will drop running tests for BouncyCastle on the Android emulator— it's unnecessary, as there is no difference in behavior between Android and JVM for it, and so there is no reason to test it.
After that, there should be no OOM.

@whyoleg
Copy link
Owner

whyoleg commented Jun 17, 2025

@sake feel free to rebase, the relevant changes are merged to main

@sake sake force-pushed the add-ec-curves-rebased branch from ef51963 to c3881a4 Compare June 17, 2025 18:17
@whyoleg whyoleg merged commit 17f5a08 into whyoleg:main Jun 18, 2025
48 of 49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants