Drop support for ERC20 tokens with decimals > 18

[smol-ninja]

A deposit of 10M ERC20 tokens with 32 decimals would fail because of uint128 type used for amounts. So I would like to discuss the idea of dropping support for ERC20 tokens with decimal values higher than 18.

In practice, there are no assets with decimals higher than 18. Even if they exist, providing support to these assets does not make sense if they cannot be deposited into the contract. So until we use uint256 for amounts, it only adds complexity. That's why either we should drop support for those assets or use uint256 for amounts.

The following table shows how much amount can be deposited with uint128 data type.

Decimals	Max deposit (in asset decimals)
25	34,028,236,692,094
26	3,402,823,669,209
27	340,282,366,921
28	34,028,236,692
29	3,402,823,669
30	340,282,367
31	34,028,237
32	3,402,824
33	340,282
34	34,028
35	3,403
36	340
37	34
38	3
39	0

WDYT @sablier-labs/everybody?

[PaulRBerg]

Agree to not allow more than 18 decimals.

There are some tokens with more than 18 decimals, but Flow is optimized for stablecoins, anyway.

[smol-ninja]

There are some tokens with more than 18 decimals

I wanted to say "Even if they exist, providing support to these assets does not make sense if they cannot be deposited into the contract". Updated my OP.

[andreivladbrg]

Thanks Shub for opening this.

I agree that there are almost no assets with more than 18 decimals, also mentioned here.

That's why either we should drop support for those assets or use uint256 for amounts.

Correct. If we want to support them, we should change the current implementation to uint256 for transfer amounts. Otherwise, it would lead to problems. The bigger issue isn't with deposit but with withdraw and refund. Imagine a scenario where the sender deposits multiple times an amount that fits in uint128. Since we round down to 18 decimals, the amount will fit in stream.balance even when summed up multiple times. However, if the recipient wants to withdraw, and enough time has passed to stream an amount that, when converted to the transfer amount, causes a revert, the funds would basically be stuck.

And as you said, adding this complexity is not worth it.

---

I agree with your conclusion, we should drop the support for assets with >18 decimals.

[smol-ninja]

Good point on withdraw and refund. That would be a bigger problem.

[andreivladbrg]

Created the issue: #156

Before closing, I would appreciate your input as well @razgraf.

---

Also, I want to mention that if, in the future, more assets with more than 18 decimals appear (tho I am skeptical), we can release a new version. For simplicity, let's not allow them in the first version.

[razgraf]

If most assets have 18 or less it makes sense to do this! One thing I'm interested in and it would be great if you could look into is if any memecoins that used Sablier had > 18.

Should be easy to see with a subgraph query but I can't do it right at this moment. Cc: @gavriliumircea