Skip to content
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 support for secp256k1 #548

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add support for secp256k1 #548

wants to merge 1 commit into from

Conversation

sosthene-nitrokey
Copy link
Collaborator

@sosthene-nitrokey sosthene-nitrokey commented Oct 22, 2024

@nitrokey-ci
Copy link
Collaborator

nitrokey-ci commented Oct 22, 2024

No significant changes.

Insignifcant changes
metric value change
binary-size-nk3am 1,371,692 🔴 +4,876 (+0.36%)
binary-size-nk3am-test 1,878,443 🔴 +1,035 (+0.06%)
binary-size-nk3xn 493,496 🔴 +1,632 (+0.33%)
binary-size-nk3xn-test 540,524 🔴 +1,396 (+0.26%)
binary-size-nkpk 710,095 🔴 +445 (+0.06%)

@robin-nitrokey
Copy link
Member

What’s going on with the binary size? NKPK is especially strange as it should not be affected by an opcard/SE050 change, but even for the others I would have expected less.

@sosthene-nitrokey
Copy link
Collaborator Author

I think it's because of the bump of toolchain version.

@sosthene-nitrokey
Copy link
Collaborator Author

Oh wait no this is wrong indeed. The metrics are the wrong way, and with the force push they are up-to-date with regards to main.

@sosthene-nitrokey
Copy link
Collaborator Author

It's because of the removal of default-mechanisms which is not merged in trussed upstream but is in our branch.

@robin-nitrokey
Copy link
Member

robin-nitrokey commented Oct 28, 2024

At least a part of the problem is that the trussed commit is based on upstream and not on the fork, so it misses some size optimizations. Not sure if this explains everything.

@sosthene-nitrokey
Copy link
Collaborator Author

Local testing suggest it is the main reason.

@sosthene-nitrokey
Copy link
Collaborator Author

Binary size changes are now insignificant.

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.

Support secp256k1/k256 Koblitz curve
4 participants