1268/4 decimals for amounts everywhere

[alfetopito]

Summary

Closes #1268

Makes all displayed amounts up to at most 4 decimals of precision.
Any amount bellow that will show < 0.0001.

USD estimations and percentages remain at 2 decimals.
Prices remain at 6 decimals.

To Test

1. Fill in all values on the input

• Observe all amounts have at most 4 decimals, including fee breakdowns

2. Click on swap

• Observe confirmation modal has 4 decimals in all currency amounts

3. Place order

• Observe toast notification has 4 decimals

4. While on expert mode, same should be true

[github-actions]

• 🔭 GP Swap: Gnosis Protocol v2 Swap UI

[MareenG]

Still testing but while testing suddenly the displayed Balance disappeared....
This happend right after the nose picking cow disappeared.
I tried it again and it disappeared again after several price updates. (Couldn't find a clear pattern yet like after 5x times it disappers)

[MareenG]

I know I am annoying with bringing always the whole cutting vs rounding issue :).
But tt would be good to have it consistent for the same value and either cut it or round it.

[MareenG]

The price impact is shown with 6 decimals is it supposed to be like this?

[MareenG]

[elena-zh]

The price impact is shown with 6 decimals is it supposed to be like this?

@MareenG , for percentages there is a separate issue exist #999 . I think, it is not fixed yet...

[MareenG]

I wasn't able to get a screenshot yet but occasionally in the TO token field the value was displayed with less than 4 digits but in the fee breakdown it was displayed with 4 digits.
It looked like this (edited image to display the issue):

[MareenG]

@elena-zh Ok I was unsure as this was in the description :)

USD estimations and percentages remain at 2 decimals.

[MareenG]

The decimals of the price is not consistent one is always displayed with 4 the other with 6

[MareenG]

I can't test no 3 when using Chrome as I currently can't place an order...If I change to production or Brave it works .

Bildschirmaufnahme.2021-08-19.um.13.23.21.mov

[elena-zh]

In addition to @MareenG comments, I'd like to add some calculation and rounding issues:

1. From (incl.fee) amount may not correspond to the sum of From amount and Fee (for a buy order)

2. Amount is not rounded to 4 decimals when 0 exists in a decimal part: 11.0528 +(11.0528*0.005) = 11.1081, but not 11.1
   

For sell order also rounding might be incorrect

3. it would be nice not to separate '<' sign with an amount on toast notifications/in transaction logs
   
   
   and everywhere else

[MareenG]

If you insert manually an amount into the FROM field with more than 4 decimals it is displayed with 3 decimals in the confirmation modal

Bildschirmaufnahme.2021-08-19.um.14.53.22.mov

Sometimes it is even displayed with less decimals e.g. for cases such as if you type in 1.0000007 it is displayed as 1

[MareenG]

The total digits seem to be limited to 9.
For bigger token volumes not all decimals are displayed.

[MareenG]

In some cases in the TO field also more than 4 digits are shown

[MareenG]

In the Price field it is not shown as < 0.000001 for really small values it is displayed as 0

[W3stside]

ooof @alfetopito I don't envy you in this PR with the amount of comments you got 😂

[alfetopito]

Lot's of great points!

ooof @alfetopito I don't envy you in this PR with the amount of comments you got joy

I though you had something to add instead of continuing with the bullying =P

[W3stside]

Lot's of great points!

ooof @alfetopito I don't envy you in this PR with the amount of comments you got joy

I though you had something to add instead of continuing with the bullying =P

nope just bullying 😅 😂

[alfetopito]

Still testing but while testing suddenly the displayed Balance disappeared....
This happend right after the nose picking cow disappeared.
I tried it again and it disappeared again after several price updates. (Couldn't find a clear pattern yet like after 5x times it disappers)

Couldn't reproduce either.
I have a guess this is related to the balances loaded from blockchain.
In any way, whatever issue this is should happen also in develop/prod as these changes do no change the balance displaying logic.

[alfetopito]

I know I am annoying with bringing always the whole cutting vs rounding issue :).
But tt would be good to have it consistent for the same value and either cut it or round it.

Don't worry, keep the feedback coming, eventually we'll be able to fix it 😅

Before	After

PS: The quotes are different because there are a few minutes between each screenshot and I stashed the changes to verify

[alfetopito]

The price impact is shown with 6 decimals is it supposed to be like this?

@MareenG , for percentages there is a separate issue exist #999 . I think, it is not fixed yet...

I could swear I fixed this, but you prove me wrong. I'll keep as is until #999 is addressed though, to not make this PR more complex.

[alfetopito]

I've just changed that to round the opposite* input to 4 decimals to fix this issue (#1281 (comment)), but the current input can have as many decimals as you need.
Actually not that much, it breaks after a long enough number...

Anyway, the point is: If you are typing, go wild. The input is not capped.
So this can happen and it's expected.

*By opposite I mean, if you type on to, the from will be capped at 4 decimals, and vice-versa.

[alfetopito]

I wasn't able to get a screenshot yet but occasionally in the TO token field the value was displayed with less than 4 digits but in the fee breakdown it was displayed with 4 digits.
It looked like this (edited image to display the issue):

Should be covered by the same fix as #1281 (comment)

[alfetopito]

The decimals of the price is not consistent one is always displayed with 4 the other with 6

This one took me a looong time to figure out. The explanation is simple:
1 price is < 0, so any precision we pass to it is applied.
1 price is > 0 and < 100k, so 4 decimals is the max precision.

The lib that does the "smart" formatting does this to better format big numbers and makes some assumptions.
Not a trivial fix (I mean, not a big deal but requires changes to another repository), so I won't fix it in this PR.

See https://github.com/gnosis/dex-js/blob/c8fa22f01ab1e04552fa02b55c89ccaa94f62a4f/src/utils/format.ts#L112

[alfetopito]

I've just changed that to round the opposite* input to 4 decimals to fix this issue (#1281 (comment)), but the current input can have as many decimals as you need.
Actually not that much, it breaks after a long enough number...
Anyway, the point is: If you are typing, go wild. The input is not capped.
So this can happen and it's expected.
*By opposite I mean, if you type on to, the from will be capped at 4 decimals, and vice-versa.

Makes sense that the user can go wild with the amount of decimals they want to insert.
However, I wondered how we want to handle pressing the max balance button? Currently it look like this is handled as manually inserting all decimals. So there is no cap.
Bildschirmaufnahme.2021-08-23.um.10.17.55.mov
Bildschirmaufnahme.2021-08-23.um.10.16.56.mov

That's the expected behaviour.
There's no cap on input, it'll fit as much precision as the selected token, which can be up to 18 decimals.

[alfetopito]

I have another thing to add. This is rather an aesthetic questions.
In some cases less decimals are shown as we don't want to show 0

e.g.
Input:

Confirmation modal:

OR

This approach makes sense.
However, I feel this looks a bit odd as we display 4 digits for all other cases and in this case we show less.

Additionally, I also noticed this has a minor effect on the USD estimation (see first example)

As I explained in #1281 (comment), this is happening because of rounding. 10.000008 with 4 decimals of precision rounds to 10.0000, removing the zeros => 10.

As for the USD estimation, hmm. That should not impact it because the estimations should be based on the full amount.
I guess that's because the price has been updated in the mean time.
I double checked the code and it's just like that. The exact value - not the displayed one - is used to calculate the USD amount.

[alfetopito]

This is quite an edge case bit in this example the rounding is not true.
It should state that you are receiving < 0.0001
Especially in the min received section it is wrong

Yes, I see how that could be a problem.
Not that it's not true, it's just rounding 🤷

What about rounding down, then?
That would be equivalent to truncating values, so this specific case would end up with < 0.0001

@gnosis/gp-frontend any opinion on this one?

[fairlighteth]

@alfetopito Just for reference, this is what the Safe has been using I think https://github.com/gnosis/safe/blob/a87d5d8af99a4d6fd35424a7e0743379a44bf9a8/archive/specs/common/format_amounts.rst

[mergify]

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

[elena-zh]

Hey @alfetopito , it might be another edge case, but it would be nice to remove a period in the end of a number when a decimal part is rounded to 0

However, based on the lib info, here we should show 1 decimal number

[mergify]

Sorry but I didn't understand the command. Please consult the commands documentation 📚.

Hey, I reacted but my real name is @Mergifyio

[alfetopito]

@biocom

@alfetopito Just for reference, this is what the Safe has been using I think https://github.com/gnosis/safe/blob/a87d5d8af99a4d6fd35424a7e0743379a44bf9a8/archive/specs/common/format_amounts.rst

Yes, David based formatSmart on that :)

@elena-zh

However, based on the lib info, here we should show 1 decimal number

Yes, that is an issue with the lib.

I'll address that in another PR as this requires changes to the format lib like on #999

[anxolin]

Other than the trailing dot, that I also manage to reproduce, but only briefly, probably was because by chance the price ended on zero or something.

Good to merge since that issue was moved to be solved in other PR.

[elena-zh]

I retested the rp and reported remaining low prio issues in separate tasks.