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

Additional ecdsa tests prototype #2121

Merged
merged 10 commits into from
Apr 26, 2024
Merged

Additional ecdsa tests prototype #2121

merged 10 commits into from
Apr 26, 2024

Conversation

chipshort
Copy link
Collaborator

No description provided.

Copy link
Member

@aumetra aumetra left a comment

Choose a reason for hiding this comment

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

A few small nits

packages/crypto/tests/rootberg_secp256k1.rs Outdated Show resolved Hide resolved
packages/crypto/tests/rootberg_secp256k1.rs Outdated Show resolved Hide resolved
packages/crypto/tests/rootberg_secp256k1.rs Outdated Show resolved Hide resolved
@chipshort
Copy link
Collaborator Author

I have updated the tests to use the non-normalized test files, which allows us to remove the ugly comment check.
I also added the r1 test files and refactored the code a bit, so we can reuse the hashers and json types.

@webmaster128 webmaster128 added this to the 2.1.0 milestone Apr 25, 2024
@webmaster128
Copy link
Member

Is this ready for review? I added it to the 2.1 milestone as it gives us extra confidence in the newly added r1 APIs.

@chipshort
Copy link
Collaborator Author

Yes, the code itself is ready for review. Only thing I don't know is how stable the rootberg format is, but we can always adjust to that later if it comes up.

@webmaster128 webmaster128 marked this pull request as ready for review April 25, 2024 11:23
@chipshort chipshort merged commit 36635db into main Apr 26, 2024
26 of 28 checks passed
@chipshort chipshort deleted the chipshort/ecdsa-tests branch April 26, 2024 09:46
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