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

0s are dropped, producing hex strings with inconsistent length #1

Open
nnkken opened this issue May 3, 2018 · 1 comment
Open

0s are dropped, producing hex strings with inconsistent length #1

nnkken opened this issue May 3, 2018 · 1 comment

Comments

@nnkken
Copy link

nnkken commented May 3, 2018

In the package, the implementation for browser is:

var randomBytes = cryptoLib.getRandomValues(new Uint8Array(size));
var returnValue = '0x'+ Array.from(randomBytes).map(function(arr){ return arr.toString(16); }).join('');

And if the number (ranged 0 - 255) is less than 16, arr.toString(16) returns only one single hex digit.

Given that the number of bytes is large enough, the probability to have missing 0s trends to 100%.
(For size = 32, probability to actually have 64 characters = (15/16)^32 = 12.7%, which matches the statistics got in browser)

This is a serious problem, since it greatly decreases the probability of having digit 0.

I've tested calling randomHex(32) 1 million times, and see the statistics of digits appeared. The result is as follows:

[1997961, 3999880, 4004169, 4002023, 3998594, 3998180, 3999718, 4000678, 4002234, 4000798, 3998786, 3999998, 4001340, 4000397, 3997218, 3999074]

As you can see, the probability of having 0 digit is dropped to about half of normal.

(BTW, where is the source code? I got the implementation by looking into node_modules...)

Related issue:
web3/web3.js#1490

@ghost ghost mentioned this issue May 3, 2022
5 tasks
@ghost
Copy link

ghost commented May 3, 2022

@nnkken Fixed on #3

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 a pull request may close this issue.

1 participant