bitshares / bitshares-ui

Fully featured Graphical User Interface / Reference Wallet for the BitShares Blockchain
https://wallet.bitshares.org
MIT License
518 stars 570 forks source link

Inaccurate floating point numbers used in credit offers #3487

Closed abitmore closed 2 years ago

abitmore commented 2 years ago

Describe the bug

Certain credit offers cannot be accepted due to the use of inaccurate floating point numbers to calculate collateral amounts.

To Reproduce Steps to reproduce the behavior:

  1. Go to https://develop.bitshares.org/#/lending/p2p , page 2
  2. Click on '$' icon of the 1.21.79 offer
  3. Input amount 100 (XBTSX.HIVE), see the collateral amount is 5,999.99881 BTS, which is insufficient because the required collateral ratio is 1 HIVE / 60 BTS
  4. Submit, see error

Expected behavior No error

Screenshots image

Desktop (please complete the following information):

Additional context

xiangxn commented 2 years ago

I have dealt with this issue, if you have time you can review and merge. @sschiessl-bcp @abitmore

sschiessl-bcp commented 2 years ago

Please check again

abitmore commented 2 years ago

image

abitmore commented 2 years ago

I think it's not fixed.

xiangxn commented 2 years ago

I replaced the division with multiplication, the precision should no longer affect @abitmore

sschiessl-bcp commented 2 years ago

@xiangxn please check the calculation and if accurate now we can close

abitmore commented 2 years ago

Offer 1.21.67, this is fine: image

but too much collateral here: image

BTW for the offer 1.21.69, the percentage is strange: image

xiangxn commented 2 years ago

Sorry, I forgot to clean up the original ceil function

xiangxn commented 2 years ago

I've fixed this and tested it. you can merge.

@sschiessl-bcp @abitmore

abitmore commented 2 years ago

I think ceil is still needed somewhere.

For example, for offer 1.21.3, if to borrow 1 BTS, the collateral 0.2381 CNY is sufficient, image

But if to borrow 0.1 BTS, the collateral 0.0238 CNY is insufficient: image

By the way, the amount of fee (0.86869 BTS) is wrong. It should be 1 BTS.

xiangxn commented 2 years ago

I have reprocessed. @abitmore

The Fee issue, I checked it, this is because the partial op number returned by the api did not cause the front-end read to fail, which requires opening a new issue #3505.

For example: /* 40 */ blind_transfer_operation This is not in the parameter list.

Because of this problem, UI's fee calculation after OP number 40 is all wrong.

sschiessl-bcp commented 2 years ago

Checked the cases above, looks good.

I guess the one below is not an issue, still there is a rounding error with big numbers. Funny enough, adding another 0 rounds properly again image