Skip to content

Conversation

@dcousens
Copy link
Contributor

See https://stackoverflow.com/questions/10984974/why-do-people-say-there-is-modulo-bias-when-using-a-random-number-generator for a good explanation.

secp256k1.n is approximately 2^256 - 2^128, so the bias is negligible, but it does exist.
No security notice needed.

Will back port.

@dcousens dcousens added the bug label Aug 21, 2015
@dcousens
Copy link
Contributor Author

ping @jprichardson, @weilu, @rubensayshi

dcousens added a commit that referenced this pull request Aug 21, 2015
ECPair: fix modulo bias in makeRandom
@dcousens dcousens merged commit 8c717fa into master Aug 21, 2015
@dcousens
Copy link
Contributor Author

Merged as 252336a, changed (d.compareTo(secp256k1.n) > 0) to (d.compareTo(secp256k1.n) >= 0). This is in line with https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/ecpair.js#L25-L26.

@dcousens
Copy link
Contributor Author

Applied to 1.5.x as b89c82b

@dcousens dcousens mentioned this pull request Aug 21, 2015
Closed
@weilu
Copy link
Contributor

weilu commented Aug 23, 2015

@dcousens you still need to fix >= in master.

@dcousens
Copy link
Contributor Author

@weilu catch. We need to put that under test.
Applied fix in 96c04d0.

@dcousens dcousens mentioned this pull request Sep 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants