Omniscia Olympus DAO Audit

StakingDistributor Manual Review Findings

StakingDistributor Manual Review Findings

SDR-01M: Improper Accumulation of Rewards

Description:

The nextRewardFor function does not properly accumulate rewards if multiple ones are specified for a particular recipient.

Example:

contracts/StakingDistributor.sol
110/**
111    @notice view function for next reward for specified address
112    @param _recipient address
113    @return uint
114 */
115function nextRewardFor(address _recipient) public view returns (uint256) {
116    uint256 reward;
117    for (uint256 i = 0; i < info.length; i++) {
118        if (info[i].recipient == _recipient) {
119            reward = nextRewardAt(info[i].rate);
120        }
121    }
122    return reward;
123}

Recommendation:

We advise the function to properly sum the results of nextRewardAt invocations to ensure it operates as intended.

Alleviation:

Rewards are now properly accumulated in the reward entry.

SDR-02M: Ungraceful Handling of High Adjustment Rates

Description:

The adjustment.rate is meant to represent a step-by-step reduction or increase of the reward rate for a particular recipient, however, there can be a case where the info[_index].rate is smaller than the step which would render the adjust operation impossible and thus cause the full distribute hook to fail.

Example:

contracts/StakingDistributor.sol
88} else {
89    // if rate should decrease
90    info[_index].rate = info[_index].rate.sub(adjustment.rate); // lower rate
91    if (info[_index].rate <= adjustment.target) {
92        // if target met
93        adjustments[_index].rate = 0; // turn off adjustment
94    }
95}

Recommendation:

We advise the reduction of a particular rate to be gracefully handled whereby if the reduction is greater than the current rate the rate should be set to zero.

Alleviation:

The Olympus DAO team considered this exhibit but decided to retain the current behaviour of the code in place.

SDR-03M: Inexistent Validation of Entry Validity

Description:

The removeRecipient and setAdjustment functions do not actually validate whether there is an existing entry in the _index they are operating in, leading to adjustments for inexistent entries / future ones or removal of inexistent entries.

Example:

contracts/StakingDistributor.sol
137/**
138    @notice removes recipient for distributions
139    @param _index uint
140    @param _recipient address
141 */
142function removeRecipient(uint256 _index, address _recipient) external {
143    require(msg.sender == governor() || msg.sender == guardian(), "Caller is not governor or guardian");
144    require(_recipient == info[_index].recipient);
145    info[_index].recipient = address(0);
146    info[_index].rate = 0;
147}
148

149/**
150    @notice set adjustment info for a collector's reward rate
151    @param _index uint
152    @param _add bool
153    @param _rate uint
154    @param _target uint
155 */
156function setAdjustment(
157    uint256 _index,
158    bool _add,
159    uint256 _rate,
160    uint256 _target
161) external {
162    require(msg.sender == governor() || msg.sender == guardian(), "Caller is not governor or guardian");
163

164    if (msg.sender == guardian()) {
165        require(_rate <= info[_index].rate.mul(25).div(1000), "Limiter: cannot adjust by >2.5%");
166    }
167

168    adjustments[_index] = Adjust({add: _add, rate: _rate, target: _target});
169}

Recommendation:

We advise both functions to properly validate that an info entry exists by evaluating its recipient.

Alleviation:

Both functions now properly validate the existance of an entry by ensuring that the info[_index].recipient member is non-zero.

SDR-04M: Inexistent Validation of Reward Rate

Description:

The addRecipient function does not validate that the _rewardRate set does not exceed the maximum achievable which is 1000000.

Example:

contracts/StakingDistributor.sol
127/**
128    @notice adds recipient for distributions
129    @param _recipient address
130    @param _rewardRate uint
131 */
132function addRecipient(address _recipient, uint256 _rewardRate) external onlyGovernor {
133    require(_recipient != address(0));
134    info.push(Info({recipient: _recipient, rate: _rewardRate}));
135}

Recommendation:

We advise such validation to be imposed to prevent arbitrarily high reward rates.

Alleviation:

The function now properly validate that the _rewardRate set is at most equivalent to the newly declared rateDenominator.

Prev

Ownable.sol (OWN-M)

Next

StandardBondingCalculator.sol (SBC-M)

On this page