-
Notifications
You must be signed in to change notification settings - Fork 33
feat: Make crypto implementation pluggable. #133
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tnx for your contribution. Not a bad idea, will review in more detail next week
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, you should probably put latest master as we are converting to esm
Codecov Report
@@ Coverage Diff @@
## master #133 +/- ##
==========================================
- Coverage 88.59% 85.92% -2.68%
==========================================
Files 16 18 +2
Lines 1841 1918 +77
Branches 248 242 -6
==========================================
+ Hits 1631 1648 +17
- Misses 210 270 +60
Continue to review full report at Codecov.
|
@mpetrunic Rebased and updated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, nice abstraction! Are there any performance tests to ensure equal throughput? Results should be the same, but just to be sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM from an organizational perspective
master PR#133 It varies on each run but it's identical |
@dapplion This potentially allows you in lodestar to use some native library that might be faster than stablelib (purejs). |
yeah looking forward to testing multiple impl |
If you want, I have sample implementation with sodium-native in https://github.com/web3-storage/ipfs-elastic-provider-bitswap-peer/blob/main/src/noise-crypto.js |
Let's goo! 🔥 If you wanna contribute bringing this live in Lodestar I may airdrop you some $LODE tokens in the future |
I've implemented this interface with WASM using Rust and the Here's some benchmarks: Throughput:
Handshakes per second:
|
Hello.
This PR extracts all low level crypto interface in a specific file and makes the crypto interface pluggable in
Noise
.Hope this helps!