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

Polyfill nodejs crypto with noble #360

Merged
merged 9 commits into from
May 6, 2024

Conversation

Duddino
Copy link
Member

@Duddino Duddino commented Apr 29, 2024

Abstract

Quite a few of our libraries use the crypto module, which is only available on nodejs.
Prior to this PR we were using the crypto-browserify. However, in #47, we decided to use the noble library, which is considerably smaller.
This also paves the way to remove bn.js and our next biggest library elliptic, which is the only remaining library which depends on it in the main bundle (Ledger also depends on bn.js, but it's in a different bundle).
This cuts down the main bundle from 419KB to 386KB (Gzipped)

Testing

Test that the polyfill works as intended.
The affected libraries were: pivx-promos, hdkey, base32

@Duddino Duddino self-assigned this Apr 29, 2024
Copy link

netlify bot commented Apr 29, 2024

Deploy Preview for cheery-moxie-4f1121 ready!

Name Link
🔨 Latest commit 9f8419a
🔍 Latest deploy log https://app.netlify.com/sites/cheery-moxie-4f1121/deploys/663905712a335f0008be7a08
😎 Deploy Preview https://deploy-preview-360--cheery-moxie-4f1121.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@panleone panleone left a comment

Choose a reason for hiding this comment

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

importing seed phrase does not work

panleone
panleone previously approved these changes May 2, 2024
Copy link
Member

@panleone panleone left a comment

Choose a reason for hiding this comment

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

tACK

JSKitty
JSKitty previously approved these changes May 3, 2024
Copy link
Member

@JSKitty JSKitty left a comment

Choose a reason for hiding this comment

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

tACK d47cbcb

Tested as many wallet functions as I could: derivation of Legacy and HD, Shield and Transparent Txs, Promo Creation and Redemption, Staking and Unstaking - all is working well and the bundles reduced nicely!

Co-authored-by: JSKitty <mail@jskitty.cat>
@Duddino Duddino dismissed stale reviews from JSKitty and panleone via 03d0138 May 6, 2024 16:29
Copy link
Member

@panleone panleone left a comment

Choose a reason for hiding this comment

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

re-tACK 9f8419a

@panleone panleone merged commit 68f6f09 into PIVX-Labs:master May 6, 2024
5 checks passed
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.

3 participants