Skip to content

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

@christophersanborn

Description

@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

Screen Shot 2022-09-03 at 11 58 46 AM

Screen Shot 2022-09-03 at 11 58 02 AM

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions