Fix edge case calculation errors in remove liquidity screen

[DimitarSD]

Fixes #584 .

Changes proposed in this PR:

• making sure any number has 18 decimals at all times, since toPrecision() alone led to errors in certain edge cases

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/oceanprotocol/market/6i5WZM6VZbpMJBUfmH5b8Sru5ZJN
✅ Preview: https://market-git-fix-584-oceanprotocol.vercel.app

[kremalicious]

In my tests it looked like using toPrecision in combination with rounding down is the solution #584 (comment)

Or any reason why toFixed would be better here?

[mihaisc]

Based on documentation we should use toPrecision with rounding. toFixed will fail. Check documentation #584 (comment)

[DimitarSD]

Using toPrecision() keeps breaking the code when having a low amount of shares. I'm using the exact same code from @kremalicious and I keep getting the same error. So far, I never got an error using .toFixed - it's because it counts the 0s

toPrecision digit range is.sd: number: integer, 1 to 1e+9 inclusive - it doesn't count 0s

Also, it seems that when adding a low amount of liquidity and when trying to remove it we get another conversion error with toPrecision - check issue #589

[mihaisc]

toPrecision converts in scientific notation, use http://mikemcl.github.io/decimal.js/#toExpPos

[mihaisc]

Actually i think we need http://mikemcl.github.io/decimal.js/#toExpNeg

[DimitarSD]

Neither of the two works and I keep getting the same error over and over again. I tested it with toExpPos and toExpNeg, in the cases with

• .toString()
• .toFixed(18, Decimal.ROUND_DOWN) / .toFixed(18)
• .toPrecision(18, Decimal.ROUND_DOWN) / .toPrecision(18)

Best way to test:

1. Switch the network to Polygon (or Rinkeby)
2. Choose a dataset for which you don't have an added liquidity (or if you have, remove it first). I've been using Maritime Word List on Polygon
3. Add the minimum amount of liquidity to a pool (0.0001 - or a close amount)
4. Try to remove the liquidity from the pool

Using .toFixed(18, Decimal.ROUND_DOWN) / .toFixed(18) is the only time when the code doesn't fail with one of the two errors (when working with min amount or converting to wei)

[mihaisc]

ok, i'll take a look at it

[mihaisc]

@DimitarSD please test this, it works for me

[DimitarSD]

@mihaisc I'm getting only converting to wei error. Btw I'm testing by adding the min allowed amount for liquidity - 0.00001

[mihaisc]

When are you substracting? Can you show me the code?

[DimitarSD]

I didn't change anything in the code. I made a pull and ran the code with your latest changes. The only thing I did was to add 0.00001 liquidity to Maritime Word List on Polygon and then tried to remove it (hence the error)

[mihaisc]

Ok, i seem to replicate this as well. It looks like sometimes precision is ignored

[DimitarSD]

It's not ignored, .toPrecision doesn't count 0s in the beginning after the decimal point.

If we have 0.004673258821970082 (total 18 digits after the decimal point) when using .toPrecision(18) it will count only 4673258821970082 (16 digits) - it will skip the first two 0s. And because we use .toPrecision(18) it will add two 0s at the end to make it 18. However, we then get 0.00467325882197008200 (20 digits), and when converted to a string and passed to wei conversion it breaks.

Every time we have 0s after the decimal point, and the total number of digits is close to 18 it will break (most of the time in edge cases and very small amounts)

[mihaisc]

Not the best solution, but at the end of the day it's a form of rounding down.

[DimitarSD]

When using the slider or MAX button now it works, however (again an edge case) when trying to remove 1% of 0.00001 or even 100% of 0.00001, it fails after the APPROVE & REMOVE is clicked. I'm not sure if this is actually coming from ocean.js

[mihaisc]

Ok, good catch. It was because ocean and dt values. Now it should be ok.

[DimitarSD]

@mihaisc I get the same first error again if I try to remove 1% of 0.00001 (I don't have problems if I try to remove 100%). Also I noticed that certain percentages (like 9%) doesn't calculate properly and it basically add nothing:

[qlty-cloud-legacy]

Code Climate has analyzed commit 294c570 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Duplication	2

View more on Code Climate.

[DimitarSD]

@mihaisc after the last updates I didn't manage to reproduce any of the bugs so far.

[kremalicious]

could not break it anymore in my limited manual test cases so looks like we made it 🥳