Platform: Code4rena
Start Date: 16/09/2021
Pot Size: $200,000 SUSHI
Total HM: 26
Participants: 16
Period: 14 days
Judge: alcueca
Total Solo HM: 13
Id: 29
League: ETH
Rank: 7/16
Findings: 3
Award: $5,813.91
🌟 Selected for report: 5
🚀 Solo Findings: 1
🌟 Selected for report: tensors
211.7953 SUSHI - $2,647.44
tensors
Some rare tokens have 0 decimals: https://etherscan.io/token/0xcc8fa225d80b9c7d42f96e9570156c65d6caaa25
For these tokens, small losses of precision will be amplified by the lack of decimals.
Consider a constant product pool with 1000 of token0 (with no decimals), and 1000 of token1 (also with no decimals). Suppose I swap n= 1,2,3,4 of token0 to token1. Then my output amount of token1 will be 0,1,2,3.
If token0/1 are valuable than I will be losing 100%, 50%, 33%, 25% of my trade to rounding. Currently there is no valuable token with 0 decimals, but there may be in the future.
Rounding the final getAmountOut division upwards would fix this.
#0 - maxsam4
2021-10-19T11:57:34Z
Acceptable risk. We can't do anything if the token itself doesn't have decimals. We don't create synthetic assets and fractionalize such tokens ourselves.
🌟 Selected for report: tensors
tensors
The oracle timestampLast is configured to 1 by the pool deployment:
Looking at the _update() function, this means that the first swap after deployment will be given an undue amount of weight because the difference between it and the next timestamp will be very large. This will break TWAP oracles that use the pool data which includes the first swap.
I recommend using the actual block.timestamp in the pool constructor instead.
#0 - maxsam4
2021-10-19T11:59:23Z
🌟 Selected for report: tensors
tensors
Consider using a more recent version instead of using solidity 0.8.0. More recent versions of solidity have compiler optimizations, user defined types, overriding interface functions, reading from immutables, among other things. This could help reading and writing safe and clean code.
#0 - maxsam4
2021-10-19T12:04:18Z
(new) compiler version is fixed in hardhat config.
🌟 Selected for report: tensors
70.5984 SUSHI - $882.48
tensors
The competition docs state that index pools should have equal weights for all tokens:
"So, if the pool creator makes a pool of four tokens, each token will have 25% of the pool; five tokens, each token will have 20% of the pool; and so on. "
but the actual code is more general than that, allowing arbitrary combinations of weights:
I recommend a brief fix to the documentation.
#0 - maxsam4
2021-10-05T07:16:52Z
Apparently the competition docs for IndexPool are completely wrong. Apologies.
The code is right though.
#1 - alcueca
2021-10-25T06:20:34Z
Wrong documentation is a Sev 1, I don't make the rules :man_shrugging:
🌟 Selected for report: tensors
32.2581 SUSHI - $403.23
tensors
Constants of form 10**X can be rewritten as 1eX and this will save gas.
9.2639 SUSHI - $115.80
tensors
It is widely considered best practice to lock the pragma statements to a specific compiler. See, for example, https://swcregistry.io/docs/SWC-103
Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.
##Proof of concept https://github.com/sushiswap/trident/blob/7098ef9ad97fa11d350a15d97740e97ad2ca4649/contracts/pool/ConstantProductPool.sol#L3
#0 - maxsam4
2021-10-19T12:03:58Z
compiler version is fixed in hardhat config.
#1 - alcueca
2021-10-25T05:15:11Z
Duplicated with #49