Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

Optimize randomBytes #345

Merged
merged 6 commits into from Aug 17, 2021
Merged

Optimize randomBytes #345

merged 6 commits into from Aug 17, 2021

Conversation

ghost
Copy link

@ghost ghost commented Aug 17, 2021

Description

This PR refactors and optimizes randomBytes().

  • For browser or web worker environment, use window.crypto.getRandomValues()
  • For node environment, we use sodium-native to generate random bytes safely
  • by dropping sodium-universal, we can minimize dependency and the bundle size (32kb -> 1.3kb when Minified + Gzipped) and still support web worker environment.
  • handles the limit of getRandomValues() which is about the requested length.
  • supports IE 11

Also, this PR upgrades @types/node & prettier and formats codes in order to use optional chaining without errors.

References

Motivation and Context

How has this been tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • Add tests to cover changes as needed.
  • Update documentation as needed.

@ghost ghost requested review from AmritKumar, bb111189, renlulu and teye as code owners August 17, 2021 04:07
@codecov-commenter
Copy link

codecov-commenter commented Aug 17, 2021

Codecov Report

Merging #345 (8ea40ca) into dev (0260567) will decrease coverage by 0.13%.
The diff coverage is 76.19%.

❗ Current head 8ea40ca differs from pull request most recent head cc24749. Consider uploading reports for the commit cc24749 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #345      +/-   ##
==========================================
- Coverage   80.80%   80.67%   -0.14%     
==========================================
  Files          44       44              
  Lines        1782     1790       +8     
  Branches      294      298       +4     
==========================================
+ Hits         1440     1444       +4     
- Misses        341      345       +4     
  Partials        1        1              
Impacted Files Coverage Δ
packages/zilliqa-js-core/src/net.ts 98.59% <ø> (ø)
packages/zilliqa-js-crypto/src/random.ts 75.00% <64.28%> (-15.91%) ⬇️
packages/zilliqa-js-account/src/transaction.ts 90.20% <100.00%> (ø)
packages/zilliqa-js-account/src/wallet.ts 82.03% <100.00%> (-0.14%) ⬇️
packages/zilliqa-js-blockchain/src/chain.ts 62.92% <100.00%> (ø)
packages/zilliqa-js-crypto/src/util.ts 82.45% <100.00%> (ø)
packages/zilliqa-js-subscriptions/src/ws.ts 64.96% <100.00%> (ø)
packages/zilliqa-js-util/src/unit.ts 80.95% <100.00%> (ø)

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 0260567...cc24749. Read the comment docs.

@ghost ghost changed the title Refactor randomBytes Optimize randomBytes Aug 17, 2021
Copy link
Contributor

@teye teye left a comment

Choose a reason for hiding this comment

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

Good! Works with the examples! No issues on react.

Copy link
Contributor

@bb111189 bb111189 left a comment

Choose a reason for hiding this comment

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

LGTM

@bb111189 bb111189 merged commit 5a84b55 into dev Aug 17, 2021
@ghost ghost deleted the refactor/random-bytes branch August 17, 2021 09:49
@ghost ghost mentioned this pull request Nov 3, 2021
5 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants