-
Notifications
You must be signed in to change notification settings - Fork 572
Float-to-Integer conversion producing too many digits in Liquidity Pool exchange #3545
Description
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:
- Go to https://develop.bitshares.org/#/pools — "Liquidity Pools" tab
- Search "ARTCASA" in Asset B
- Find pool 1.19.145 "BTWTY.ARTCASAMM"
- Click the exchange icon
- For amount to sell, enter 0.00013 BTWTY
- Click Submit
- Do not confirm transaction, but instead click the JSON fields to reveal amount to sell.
- 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:
| 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.

