search

sablier-labs / flow

🍃 Smart contracts of the Sablier Flow protocol.
Other
10 stars 2 forks source link

Add assumptions about `decimals` implementation #337

Open smol-ninja opened 2 days ago

smol-ninja commented 2 days ago

In Security.md, there should be an assumption about the decimals implementation of ERC20 token since its optional as per the EIP.

- The token contract must implement `decimals()` in order to be compatible with Flow.
PaulRBerg commented 2 days ago

Yeah we could do this but lacking a decimals value is so bananas that I feel like this assumption is part of 99% of DeFi protocols.

smol-ninja commented 2 days ago

this assumption is part of 99% of DeFi protocols

Not true. Most protocols do not rely on decimals at the protocol level. As examples (shared by an auditor), the following two token can be traded on Uniswap / Sushiswap but does not have decimals implemented:

  1. https://etherscan.io/token/0xe0b7927c4af23765cb51314a0e0521a9645f0e2a#tokenTrade
  2. https://etherscan.io/token/0x1da4858ad385cc377165a298cc2ce3fce0c5fd31#tokenTrade

On the contrary, the percentage of protocols that rely on decimals being implemented may be smaller than we are anticipating. DEXes definitely don't require to have decimals implemented since their price formula simply takes the ratio of the two in a pool, lending protocols don't seem to require decimals as well since they rely on oracles and these two probably make the most of the DeFi today.

PaulRBerg commented 1 day ago

I was referring to protocols that do rely on decimals, they assume that it exists. It's really bananas. But yes, anyway, let's be explicit about this and mention it in SECURITY.

smol-ninja commented 1 day ago

Agree its bananas. This will still be mentioned as a low finding.