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

Replace k256 library with rust-secp256k1 #35

Closed
lescuer97 opened this issue Oct 27, 2023 · 7 comments
Closed

Replace k256 library with rust-secp256k1 #35

lescuer97 opened this issue Oct 27, 2023 · 7 comments

Comments

@lescuer97
Copy link

Hi! I was having a look around the code and notice the library is using k256 instead of the rust-secp256k1 library. k256 is not cryptographic audited compared to rust-secp256k1 which is probably the most audited crypto library out there.

I was looking to work on a cashu library and found this package. do you think this would be a good change?
I would probably be interested in doing the change.

@thesimplekid
Copy link
Collaborator

thesimplekid commented Oct 27, 2023 via email

@thesimplekid
Copy link
Collaborator

thesimplekid commented Oct 27, 2023 via email

@thesimplekid
Copy link
Collaborator

thesimplekid commented Apr 9, 2024

I think its worth looking into this again.

It still seems to be an open issue but other crates in the ecosystem are using the secp256k1 re-exported by the bitcoin crate which is already a dependency, Using this instead of k256 would reduce a dependency here and on other projects that use CDK.

@yukibtc Do you have any advice here?

@thesimplekid thesimplekid reopened this Apr 9, 2024
@yukibtc
Copy link
Contributor

yukibtc commented Apr 9, 2024

I agree, I think we should use secp256k1 instead of k256 for the same reason you said. That's one of the things I started doing in my fork :)

The same for the bip32 crate: https://github.com/rust-bitcoin/rust-bitcoin/blob/master/bitcoin/src/bip32.rs


rust-secp256k1 can be compiled to WASM. If you are on macOS you need to do this:

brew install llvm
LLVM_PATH=$(brew --prefix llvm)
AR="${LLVM_PATH}/bin/llvm-ar" CC="${LLVM_PATH}/bin/clang" cargo build --target wasm32-unknown-unknown

On Linux it works without having to do anything (at least on debian).

@yukibtc
Copy link
Contributor

yukibtc commented Apr 9, 2024

I can take this issue if you're not already working on it, so I can get more familiar with the library :)

@thesimplekid
Copy link
Collaborator

I can take this issue if you're not already working on it, so I can get more familiar with the library

I haven't started so that's great, thanks

@thesimplekid
Copy link
Collaborator

That's one of the things I started doing in my fork

Definitely open an issue or shoot me a message with any other suggestions

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

No branches or pull requests

3 participants