Fix: Support converting small number to AmountTokenV2

[lmuntaner]

Motivation

TokenAmountV2 can't convert small numbers such as 0.00000002 which is 2 e8s of ICP.

The problem was that fromNumber was using toString and toString was converting 0.00000002 to the scientific notation. Which we specifically don't want to support when converting from a string.

Solution, use toFixed(8) instead of toString. We already support only a precision of 8 decimals.

Changes

• Use toFixed(8) instead of toString in fromNumber of TokenAmountV2.

Tests

• Change test "does not support scientific notation" to do it fromString.
• Add tests for 8 decimals.
• Add a test for fromNumber with very small numbers.
• Add test for fromNumber to check that precision is 8 decimals.

Todos

• Add entry to changelog (if necessary).

[lmuntaner]

@dskloetd I added a test that it's broken with the current implementation, but I feel it should work.

I propose to support scientific notation like in TokenAmount.

[github-actions]

size-limit report 📦

Path	Size
@dfinity/ckbtc	6.88 KB (0%)
@dfinity/cmc	1.01 KB (0%)
@dfinity/ledger-icrc	2.92 KB (0%)
@dfinity/ledger-icp	14.77 KB (0%)
@dfinity/nns	33.85 KB (0%)
@dfinity/nns-proto	76.3 KB (0%)
@dfinity/sns	15.04 KB (0%)
@dfinity/utils	4.34 KB (+0.28% 🔺)
@dfinity/ic-management	1.94 KB (0%)

[dskloetd]

I think the problem is the .toString() here:

ic-js/packages/utils/src/parser/token.ts

Line 203 in ac1bbf0

amount: amount.toString(),

That should be .toFixed(token.decimals).

[dskloetd]

Hm, now I'm not sure.

(0.123).toFixed(18)
'0.122999999999999998'

On the other hand, if we really want to support 18 decimals, we simply can't use the number for any intermediate value.
Supporting scientific notation for this just doesn't seem right either.

I think I prefer hard-coding .toFixed(8) until we stop using number as the type for the number input.

[lmuntaner]

I think I prefer hard-coding .toFixed(8) until we stop using number as the type for the number input.

That's a good idea, we anyway agreed to set the precision to 8 decimals.

I used .toFixed(8) and that also means supporting exponential notation, so I changed a test.

[dskloetd]

Can you fix the PR title and description as well?

[dskloetd]

Asking again: Can you fix the PR title and description as well?

[lmuntaner]

@dskloetd ready for another review

[dskloetd]

Are these not coming through?

Can you fix the PR title and description as well?

Asking again: Can you fix the PR title and description as well?

[lmuntaner]

Are these not coming through?

Can you fix the PR title and description as well?

Asking again: Can you fix the PR title and description as well?

Oops, sorry. I wasn't seeing them because they were not in any file and I looked only to the comments in the files.

[lmuntaner]

@dskloetd ready for another review

[dskloetd]

When I look, the PR title is still "Fix: Add test that is broken".
You're not actually adding a test that is broken, right? Then it wouldn't pass CI.
Am I reading this wrong, or what do you mean by that?

[lmuntaner]

When I look, the PR title is still "Fix: Add test that is broken". You're not actually adding a test that is broken, right? Then it wouldn't pass CI. Am I reading this wrong, or what do you mean by that?

Sorry, again, my bad. I'm a little bit all over the place this week.