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 ECDSA public key format #556

Closed
tomaka opened this issue Oct 10, 2018 · 4 comments · Fixed by #2352
Closed

Add ECDSA public key format #556

tomaka opened this issue Oct 10, 2018 · 4 comments · Fixed by #2352
Labels
difficulty:moderate getting-started Issues that can be tackled if you don't know the internals of libp2p very well

Comments

@tomaka
Copy link
Member

tomaka commented Oct 10, 2018

libp2p/specs#100

@twittner
Copy link
Contributor

Is there a specific need for this? In the spec ECDSA is optional and does not seem particularly attractive on its own.

@tomaka
Copy link
Member Author

tomaka commented Oct 12, 2018

We don't have any need for this, but let's have an issue open.

@tomaka tomaka added the getting-started Issues that can be tackled if you don't know the internals of libp2p very well label Feb 21, 2019
@davxy
Copy link
Contributor

davxy commented Nov 16, 2021

Hi @tomaka , I've started working on this issue.

I already have a working implementation compatible with the go-libp2p one but, before PR submission, I would like to ask some opinions wrt multiple curves support.

Preliminary:

  • I've used RustCrypto p256 crate. I also made an attempt using ring but I would like to have support for wasm and I prefer a plain Rust crate.
  • The feature is optional (as rsa and secp256k1)
  • Currently the implementation (as the golang one) supports only p256 curve with sha256.

I was wondering if we would like to keep a door open for more curves that may be inserted in the future.

For example: my current implementation has introduced the following function in identity.rs:

+    #[cfg(feature = "ecdsa")]
+    pub fn generate_ecdsa() -> Keypair {
+        Keypair::Ecdsa(ecdsa::Keypair::generate())
+    }

Maybe could be good to define a CurveId enum in ecdsa module:

    pub enum CurveId {
        Secp256r1
    }

And then define ecdsa::Keypair::generate to take a CurveId parameter (maybe optional with default to P256?).

As I said, for the moment there is only one possibility (i.e. p256), but maybe is a good idea to keep a door open for future extensions.

Thank you

@mxinden
Copy link
Member

mxinden commented Nov 19, 2021

I've used RustCrypto p256 crate. I also made an attempt using ring but I would like to have support for wasm and I prefer a plain Rust crate.

@burdges and @dignifiedquire, as experts on rust crypto, do you have any opinions on this?

As I said, for the moment there is only one possibility (i.e. p256), but maybe is a good idea to keep a door open for future extensions.

I am not too worried making breaking changes at the in-process API level in the future in case we do decide to support other curves. (Quite the opposite for network level breaking changes.)

jxs pushed a commit to jxs/rust-libp2p that referenced this issue Dec 18, 2023
* Inform application layer of slow peer

* Implement 1.1-like scoring for slow peers

* Fix initialisation bug

* Add scoring for queue maxing out queue lengths

* Appease clippy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty:moderate getting-started Issues that can be tackled if you don't know the internals of libp2p very well
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants