Why doesn't BigNumber.toString allow an arbitrary base?

[mirceanis]

Hello, I'm getting this error when using @ethersproject/bignumber

bigNumber.toString does not accept parameters (code=UNEXPECTED_ARGUMENT, version=bignumber/5.0.1)

It's coming from this toString() override where the comment indicates this is intentionally not supported:

ethers.js/packages/bignumber/src.ts/bignumber.ts

Line 186 in d817416

// Lots of people expect this, which we do not support, so check

I'm wondering why is this not supported.

On the surface it looks like ethers could pipe the base and padding parameters to the underlying BN implementation, like so:

    toString(base: any, padding: any): string {
        return toBN(this).toString(base || 10, padding);
    }

What am I missing?

[ricmoo]

This has come up before, which is why that error was added, people passed things in, expecting it to work and it doesn't. :)

There are a few reasons. I may forget some, but these are what come to mind. :)

Firstly, this is not supported is because then it becomes something that must remain supported. Currently I use the BN.js library, but it is completely isolated (along with the elliptic package) so that in the future I can remove it without breaking anything. This might happen in the non-distant future, which would dramatically shrink the library, or at allow a dramatically smaller ES2020 version (which would become its own dist file).

Also, the string values used for various bases (other than 10 and 16; 2 and 8 are as well, but are less interesting) are not well standardized or have multiple standards, and so become very hard to accurately present what everyone expects, and the need to possibly pad becomes complicated as well (as indicating padded bits becomes the next request) since it is also non-standardarized.

In general it isn't a wonderful idea to overload the toString signature, especially when you deal with JavaScript environments on embedded devices or older implementations. I cannot recall for certain, but I believe Otto also had this restriction, so this could once have caused a problem running inside the Geth console?

The BigNumber class only supports base-10 strings (.toString()) and base-16 strings (.toHexString()) as these are also used frequently internally to ethers, are otherwise well defined and frequently used for Ethereum purposes. Even the address package manually imports BN.js for its base36 maths, since I don't want to commit to support base36 (as defined by IBAN).

Any other base is generally not really Ethereum-related, so is better served by a more specialized big number library (which most will accept a base-10 string as input, so conversion is fairly simple). I try to keep ethers lean. :)

I hope that helps. Feel free to rebut though. :)

[mirceanis]

It's not about allowing an arbitrary base, this toString overload is causing crashes in other libraries that use the BigNumber class.
I'm getting that error in a project that also uses BN.js indirectly through a different library.
From the looks of it, that other library is affected by the fact that BigNumber.toString() is being overridden by @ethersproject/bignumber

I'll try to condense all of this in a tiny project to show how the error manifests.

[ricmoo]

There is no reason that anything expecting an ethers BigNumber should be passing in a parameter to .toString(). This behaviour has been around for quite some time (and even when it didn't throw, it would simply return wrong results, since the value was ignored). My guess is the function you are calling is expecting a BN.js instance, in which case an ethers BigNumber is not a valid substitute.

You can convert the ethers BigNumber into a BN.js using BN(bigNumber.toString()) and then pass that in? Let me know a minimal example though, and I'll try to provide some guidance...

[ricmoo]

Any luck with an example? I may close this soon otherwise... :s

[mirceanis]

Not yet, thanks for reminding me.
Please don't close this yet as it seems the border between ethers and the rest of the code interacting with BN.js is not very clearly defined.
I think it would be better if ethers BN tweaks don't "leak" into external code.

[ricmoo]

Cool cool. I’ll keep it open.

Keep in mind ethers exposes absolutely zero parts of BN.js. It is not meant to be compatible with BN.js and BigNumber should be thought of as its own implementation of a big number library. There is no way to use any part of BN.js from any export in ethers. :)

[ricmoo]

Any luck? Mind if I close this now? :)

[mirceanis]

I'm closing it since there was no time to dig deeper. I'll reopen once I run into this again.