Platform: Code4rena
Start Date: 14/10/2021
Pot Size: $50,000 USDC
Total HM: 3
Participants: 14
Period: 7 days
Judge: 0xean
Total Solo HM: 3
Id: 37
League: ETH
Rank: 3/14
Findings: 1
Award: $4,545.46
🌟 Selected for report: 2
🚀 Solo Findings: 0
🌟 Selected for report: chenyu
2272.7273 USDC - $2,272.73
chenyu
PermanentlyOwnable does not prevent transferring to a dead address. It's possible to have a human error that transfers the contract ownership to a address not owned by the old owner.
Recommend a two step transfer that owner nominates an account, then the nominated account call an accept function to ensure the nominated account is valid.
#0 - mijovic
2021-10-19T13:25:09Z
This is a good point. We decided that we will not support the transfer of ownership, so we added the owner as immutable at the end.
#1 - mijovic
2021-10-21T08:20:27Z
In TempusPool, we ended up with an immutable owner (https://github.com/tempus-finance/tempus-protocol/pull/362).
However, as we needed the owner to set whitelisted contracts for the controller, we added Ownable contract that does exactly what is explained here. I would say it is fair to say that we did it because of this finding. This was done in https://github.com/tempus-finance/tempus-protocol/pull/365
🌟 Selected for report: chenyu
2272.7273 USDC - $2,272.73
chenyu
In TempusAMM constructor L138, the scaling factor 0 always maps to yieldShare, and scaling factor 1 always maps to principalShare, even though in L134 the two token might swap if principalShare < yieldShare, which makes _token0 = principalShare and _token1 = yieldShare, but scaling factor 0 is based on yieldShare.
Later _scalingFactor is based on _token0, so it might get the wrong scaling factor if principalShare and yieldShare had swapped.
Update the lines to
_scalingFactor0 = _computeScalingFactor(IERC20(address(_token0))); _scalingFactor1 = _computeScalingFactor(IERC20(address(_token1)));
#0 - mijovic
2021-10-17T09:51:03Z
While this works with Tempus Principals and Tempus Yields (have the same scaling factor), AMM was made to be able to be reused for some other tokens with similar correlation, so this must be fixed.
#1 - mijovic
2021-10-21T08:03:57Z