Webpack minification causing BigNumber arguments to be filtered

[generalpiston]

Issue

Running into Invalid number of arguments to Solidity function when calling zeroEx.token.setUnlimitedProxyAllowanceAsync(address, account).

Environment

• Using Webpack to transpile/aggregate/minify
• Browser
• Using web3 version 1.0.0-beta.21 and 0x.js is using version 0.20.4.

Notes

• It seems like isBigNumber (https://github.com/ethereum/web3.js/blob/develop/lib/utils/utils.js#L489) doesn't recognize the amount 0x.js sends as a BigNumber.
• Not minifying fixes this issue.

[generalpiston]

Here's a bash script to hack around this:

#!/bin/bash

utils=$( find ./node_modules -name utils.js | grep "web3/lib/utils" )
for f in ${utils}; do
  sed -i.bk -E "s|object.constructor.name === 'BigNumber'|object.s !== undefined \&\& object.e !== undefined \&\& object.c !== undefined|" ${f};
done;

[koirikivi]

I also ran into this issue.

I think it would be best to rewrite the way the isBigNumber check works. The constructor name check doesn't seem robust, and other libraries might use different versions of BigNumber.js which causes the instanceof check to fail as well.

The latest version of BigNumber.js sets the field _isBigNumber = true for all BigNumbers it creates, and offers a isBigNumber function that can be used to check if an object is a BigNumber. However, this doesn't seem to be implemented in the forked version used by web3 (https://github.com/frozeman/bignumber.js-nolookahead).

I think it would be good to update that package so that it also offers a isBigNumber function, and then rewrite the isBigNumber used by web3 in terms of that.

As a (temporary?) solution I stopped mangling of the BigNumber identifier in uglifyjs-webpack-plugin configuration like this:

        new UglifyJsPlugin({
            uglifyOptions: {
                mangle: {
                    reserved: ['BigNumber'],
                },
            },
        })

which seems to work.

[beether]

I thought I submitted a bug for this in version 0.2.x.

Easiest work-around is to always use web3's version of BigNumber by doing: const BigNumber = web3.toBigNumber(0). Any calls to web3 should use that BigNumber constructor. You can cast other-BigNumbers to web3 BigNumber: var web3bn = new BigNumber(otherBnInstance)

Some takeaways from this bug that has wasted so much of everyone's time:

• Having a function take a variable-length of arguments is god awful. On top of that, having it take variable-type arguments where different types have different meanings, is cancer. Finally, if that function is also intended to map to some other API (in this case, the ABI), then the whole thing contracts aids.
• This is vastly superior in every feasible universe: contractInstance.someMethod([params], {from, value, etc}.
• The error messages are awful. A better approach is to do the above, and complain about a particular input not being of an acceptable type. Or an input not being expected. Or an input being omitted. Wow! We just saved about ten million man-hours by not promoting disease.

[graemecode]

@beether The problem with the above solution, which is otherwise somewhat elegant, is that you can't configure the BigNumber class. For example:

// By default BigNumber's `toString` method converts to exponential notation if the value has
// more then 20 digits. We want to avoid this behavior, so we set EXPONENTIAL_AT to a high number
BigNumber.config({
    EXPONENTIAL_AT: 1000,
});

Moreover, my codebase might not have access to web3 everywhere I might want to use BigNumber, and solutions to that particular problem can also become messy.

[nivida]

This should be solved within the next release. (PR #2000)