bn.js
bn.js copied to clipboard
indutny
Wrong padding in toString() function
When the length of a number is larger than the requested padding, toString will add an unnecessary padding.
For example:
new BN('123').toString(10, 2)
Should return 123 but returns 0123.
Or
new BN('12345678').toString(10, 6)
Should return 12345678 but returns 000012345678.
A simple fix would be to first check if the length is actually smaller than the requested padding before applying it.
I can submit a pull request to fix this issue.
I can submit a pull request to fix this issue.
@macor161 please do!
But if you don't have time, please please submit a failing test :+1:
wow, currently this looks like not padding at all? :thinking: https://github.com/indutny/bn.js/blob/e69c617b3297b99aca429f30842e27979ef9beb5/lib/bn.js#L474
I submited a pull request with the fix and a test. Let me know if you need anything else!
@macor161 I think you're right to bring the https://github.com/indutny/bn.js/pull/199 conversation back here until we reach some kind of consensus
@fanatid @dcousens
@therightstuff I like the idea of changing the signature, I think it's better than introducing a new function.
What do you think of those 2 signatures?:
toString(base: number, bytePadding: number);
toString(options: Object);
Internally, it would look something like this:
toString(base: number | options: Object, [bytePadding: number])
And options would be:
base: number = 10
bytePadding: number = 0
padding: number = 0
This keeps the function backward compatible and is easily extendible