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

chore: update libp2p-crypto #113

Closed

Conversation

achingbrain
Copy link
Collaborator

@achingbrain achingbrain commented Nov 26, 2021

libp2p-crypto@0.20.0 uses noble-ed25519 and noble-secp256k1 which
is smaller and faster than the version in https://www.npmjs.com/package/secp256k1

This PR updates the dep so we don't pull in 0.19.x and 0.20.x in
consuming bundles.

Ref: libp2p/js-libp2p-crypto#202

libp2p-crypto@0.20.0 uses noble-secp256k1 and noble-ed25519 which
are smaller and faster than the version in https://www.npmjs.com/package/secp256k1

This PR updates the dep so we don't pull in 0.19.x and 0.20.x in
consuming bundles.
@codecov-commenter
Copy link

codecov-commenter commented Nov 26, 2021

Codecov Report

Merging #113 (50b9330) into master (735f287) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #113   +/-   ##
=======================================
  Coverage   89.13%   89.13%           
=======================================
  Files          16       16           
  Lines        1832     1832           
  Branches      243      243           
=======================================
  Hits         1633     1633           
  Misses        199      199           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 735f287...50b9330. Read the comment docs.

Copy link
Contributor

@dapplion dapplion left a comment

Choose a reason for hiding this comment

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

Would be nice to benchmark and consider using native bindings for Lodestar at least

@achingbrain
Copy link
Collaborator Author

achingbrain commented Nov 27, 2021

@dapplion native bindings are much, much faster 😁 , though require node 15+ - libp2p/js-libp2p-crypto#211

This is ok, I think, as officially libp2p supports LTS+Current which as of a couple of weeks ago is 16 and 17.

Actually, scratch that, I updated the libp2p-crypto PR to fall back to the pure-js implementation if Ed25519 isn't supported.

@dapplion
Copy link
Contributor

@dapplion native bindings are much, much faster grin , though require node 15+ - libp2p/js-libp2p-crypto#211

This is ok, I think, as officially libp2p supports LTS+Current which as of a couple of weeks ago is 16 and 17.

Actually, scratch that, I updated the libp2p-crypto PR to fall back to the pure-js implementation if Ed25519 isn't supported.

Oh! Very interested! gossip encryption shows up in our Lodestar benchmarks. Do you have benchmarks comparing native bindings, current lib and noble's? We just bumped Lodestar to use NodeJS v16 by default so it shouldn't be an issue 👍

@dapplion
Copy link
Contributor

@dapplion native bindings are much, much faster grin , though require node 15+ - libp2p/js-libp2p-crypto#211

This is ok, I think, as officially libp2p supports LTS+Current which as of a couple of weeks ago is 16 and 17.

Actually, scratch that, I updated the libp2p-crypto PR to fall back to the pure-js implementation if Ed25519 isn't supported.

Oh! Very interested! We just bumped Lodestar to use NodeJS v16 by default so it shouldn't be an issue 👍

Damn with those benchmarks, crazy differences

@achingbrain
Copy link
Collaborator Author

Superseded by #115

@achingbrain
Copy link
Collaborator Author

And/or #116

@achingbrain achingbrain closed this Dec 1, 2021
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.

4 participants