Float-to-Integer conversion producing too many digits in Liquidity Pool exchange

[christophersanborn]

Describe the bug
Some decimal values with imprecise floating point representation are not converting correctly to integer values. Extra digits are produced, with result that a user might sell MORE of their asset than they intend to.

To Reproduce
Steps to reproduce the behavior:

1. Go to https://develop.bitshares.org/#/pools — "Liquidity Pools" tab
2. Search "ARTCASA" in Asset B
3. Find pool 1.19.145 "BTWTY.ARTCASAMM"
4. Click the exchange icon
5. For amount to sell, enter 0.00013 BTWTY
6. Click Submit
7. Do not confirm transaction, but instead click the JSON fields to reveal amount to sell.
8. Observe incorrect amount

Expected behavior

BTWTY has a precision of 5, so the user would expect to be selling 13 units.

Encountered behavior

User is in fact selling 129999999998 units.

Screenshots

Desktop (please complete the following information):

• OS: MacOS
• Browser Firefox

Additional context

Problem may stem from not rounding the result from parseFloat in PoolExchangeModal.jsx, e.g. here:

bitshares-ui/app/components/Modal/PoolExchangeModal.jsx

Line 81 in 24c9e71

parseFloat(amountToSell) * amountToSellPrecision,

        ApplicationApi.liquidityPoolExchange(
            account,
            pool.get("id"),
            pool.getIn([`asset_${amountToSellTag}`, "symbol"]),
            parseFloat(amountToSell) * amountToSellPrecision,
            pool.getIn([`asset_${minToReceiveTag}`, "symbol"]),
            parseFloat(minToReceive) * minToReceivePrecision

        )

Case in point, in the Node.js REPL, the following can be observed:

> parseFloat("0.00013")*100000
12.999999999999998
> 

Better may be to simply round the result:

> Math.round(parseFloat("0.00013")*100000);
13
> 

An additional concern:

Although the result of parseFloat in the above example gives digits following the decimal point, it DOES place the decimal point in the correct location. The value passed in to ApplicationApi.liquidityPoolExchange() as the amountToSell parameter, gets passed along from there to TransactionBuilder as, presumably, a floating point value. Somehow or other TransactionBuilder is not detecting this and throwing an error as it should, nor truncating at the decimal point. Worse, it seems to be silently removing the decimal point and using all digits, including the sub-integral digits, to make the integer amount_to_sell value. This suggests a deeper problem in the TransactionBuilder functionality.

[ioBanker]

Yeah; doing so Math.round(parseFloat(minToReceive) * minToReceivePrecision) would solve this issue but not sure about other scenarios, will be implementing it now at dex.iobanker.com and please check the results.

[abitmore]

... it seems to be silently removing the decimal point and using all digits, including the sub-integral digits, to make the integer amount_to_sell value. This suggests a deeper problem in the TransactionBuilder functionality.

I don't exactly know where it went wrong, but it seems the input (E.G. the float 12.999999999999998) is converted to Long here:

        if (typeof value === "number") {
            value = "" + value;
        }
        return Long.fromString(value, unsigned);

The result is clearly unexpected.

> parseFloat("0.00013")*100000
12.999999999999998
> Long.fromString(""+parseFloat("0.00013")*100000)
Long { low: 1150981118, high: 30, unsigned: false }
> Long.fromString(""+parseFloat("0.00013")*100000).toString()
129999999998

Created issue bitshares/bitsharesjs#99 and pull request bitshares/bitsharesjs#100. Wish it helps.

[abitmore]

FWIW rounding the input should work.

> parseFloat("0.00013")*100000
12.999999999999998
> Long.fromString(""+parseFloat("0.00013")*100000)
Long { low: 1150981118, high: 30, unsigned: false }
> Long.fromString(""+parseFloat("0.00013")*100000).toString()
129999999998
> Long.fromString(""+Math.round(parseFloat("0.00013")*100000),true)
Long { low: 13, high: 0, unsigned: true }
> Long.fromString(""+Math.round(parseFloat("0.00013")*100000),true).toString()
13