Wrong padding in toString() function

[macor161]

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.

[dcousens]

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 👍

[fanatid]

wow, currently this looks like not padding at all? 🤔

bn.js/lib/bn.js

Line 474 in e69c617

while (out.length % padding !== 0) {

[macor161]

I submited a pull request with the fix and a test.
Let me know if you need anything else!

[therightstuff]

@macor161 I think you're right to bring the #199 conversation back here until we reach some kind of consensus

@fanatid @dcousens

[macor161]

@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

[macor161]

Let me know when you reach a consensus, I will submit a pull request! :)

[macor161]

I will close this issue as there doesn't seem to be any activity.