Yield micro contest #1 - hickuphh3's results

Handle

hickuphh3

Vulnerability details

Impact

Given a configuration of target, contacts and permissions, calling terminate() will permanently prevent this configuration from being used again because the state becomes State.TERMINATED. All other functions require the configuration to be in the other states (UNKNOWN, PLANNED, or EXECUTED).

In other words, the removal of the restoring option for the configuration through EmergencyBrake is permanent.

Recommended Mitigation Steps

Since EmergencyBrake cannot reinstate permissions after termination, it would be better to have terminate change its state to UNKNOWN. The TERMINATED state can therefore be removed.

Handle

hickuphh3

Vulnerability details

Impact

This would be equivalent to Unipool's rewardPerToken() function. Note that rewardsPerToken.accumulated only reflects the latest stored accumulated value, but does not account for pending accumulation like Unipool, and is therefore not the same. It possibly might be mistaken to be so, hence the low risk classification.

Recommended Mitigation Steps

A possible implementation is given below.

function latestRewardPerToken() external view returns (uint256) {
	RewardsPerToken memory rewardsPerToken_ = rewardsPerToken;
	if (_totalSupply == 0) return rewardsPerToken_.accumulated;
	uint32 end = earliest(block.timestamp.u32(), rewardsPeriod.end);
	uint256 timeSinceLastUpdated = end - rewardsPerToken_.lastUpdated;
	return rewardsPerToken_.accumulated + 1e18 * timeSinceLastUpdated * rewardsPerToken_.rate / _totalSupply;
}

Handle

hickuphh3

Vulnerability details

Impact

As brought up in a previous audit issue, "the suggestion of changing all public auth functions to external auth will be applied". The same should therefore be done for the new contracts Strategy.sol and ERC20Rewards.sol, since all public methods in it aren't called internally.

Handle

hickuphh3

Vulnerability details

Impact

The uint128 output by _updateRewardsPerToken() isn't used by any function. Furthermore, L107 returns a wrong value.

if (_totalSupply == 0 || block.timestamp.u32() < rewardsPeriod.start) return 0;

It should return rewardsPerToken_.accumulated instead, because in the case of a new rewards schedule being set, rewardsPeriod.start is updated to a new timestamp. Hence, the current accumulated value of all previous reward schedules thus far should be returned.

Recommended Mitigation Steps

Since the return value isn't used anywhere, remove it.

Handle

hickuphh3

Vulnerability details

Impact

The latest() function is unused and can be removed.

Recommended Mitigation Steps

Remove L68-L71