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 specifying keys #85

Closed
wants to merge 3 commits into from
Closed

Add support for specifying keys #85

wants to merge 3 commits into from

Conversation

jacobheun
Copy link
Contributor

libp2p-crypto already supports multiple key formats, this PR exposes that by adding the type option to .create. This will allow users to create the other key types more easily.

@ghost ghost assigned jacobheun Oct 12, 2018
@ghost ghost added the status/in-progress In progress label Oct 12, 2018
@jacobheun
Copy link
Contributor Author

I am going to do some integration tests against libp2p and js-ipfs with the ed25519 key type and will report back once done.

@jacobheun
Copy link
Contributor Author

I ran this through the js-libp2p and js-ipfs test suites. I also had the tests alternate the key type (rsa and ed25519) used so that nodes connecting to one another would have different key types.

Aside from a minor issue with the libp2p-websocket-star-rendezvous code not respecting the crypto verify api, everything passed successfully. The issue with the rendezvous code has been fixed.

This should be good to merge.

@jacobheun jacobheun requested a review from pgte October 16, 2018 22:26
Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@jacobheun
Copy link
Contributor Author

@pgte I rebased this off the latest release, let me know if there are any updates you need from me for this to be released.

@blakebyrnes
Copy link

@jacobheun there's a ticket in https://github.com/libp2p/js-libp2p-crypto-secp256k1 to support secp keys in the latest libp2p. You can't unmarshal private keys since the underlying signature changed in peerId (https://github.com/libp2p/js-libp2p-crypto-secp256k1/pulls). Is there any reason to keep secp256 in it's own package at this point? Isn't it directly included in libp2p crypto now? Apologies if this is the wrong place to discuss.

@jacobheun jacobheun added the status/blocked Unable to be worked further until needs are met label Dec 19, 2018
@jacobheun
Copy link
Contributor Author

@blakebyrnes I created an issue at libp2p-crypto to discuss this libp2p/js-libp2p-crypto#135, since it should probably happen there.

@vasco-santos
Copy link
Member

Closing, as this was added on #95

@jacobheun jacobheun deleted the feat/key-types branch April 22, 2020 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/blocked Unable to be worked further until needs are met status/in-progress In progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants