RewardEthToken Static Analysis Findings

RET-01S: Inexistent Sanitization Against Claim to Zero

Description:

The claim function does not sanitize the account argument and it is not sanitized anywhere in the MerkleDistributor implementation, meaning that it can even represent the zero-address.

Example:

Copy
262/**
263 * @dev See {IRewardEthToken-claim}.
264 */
265function claim(address account, uint256 amount) external override {
266    require(msg.sender == merkleDistributor, "RewardEthToken: access denied");
267
268    // update checkpoints, transfer amount from distributor to account
269    uint128 _rewardPerToken = rewardPerToken;
270    checkpoints[address(0)] = Checkpoint({
271        reward: _balanceOf(address(0), _rewardPerToken).sub(amount).toUint128(),
272        rewardPerToken: _rewardPerToken
273    });
274    checkpoints[account] = Checkpoint({
275        reward: _balanceOf(account, _rewardPerToken).add(amount).toUint128(),
276        rewardPerToken: _rewardPerToken
277    });
278    emit Transfer(address(0), account, amount);
279}

Recommendation:

We advise a require to be introduced ensuring that the account cannot be zero. While it does not affect the correctness of the implementation, it will emit a non-standard Transfer event from the zero address to the zero address incorrectly.

Alleviation:

A require check was introduced properly validating that the account argument is different than the zero-address.

RET-02S: Inexistent Zero-Based Input Validation

Description:

The input arguments of the linked function are of the address type, are set once and are not validated to be different from the zero-address.

Example:

Copy
54/**
55 * @dev See {IRewardEthToken-upgrade}.
56 */
57function upgrade(address _oracles) external override onlyAdmin whenPaused {
58    require(address(oracles) == 0x2f1C5E86B13a74f5A6E7B4b35DD77fe29Aa47514, "RewardEthToken: already upgraded");
59    oracles = _oracles;
60}

Recommendation:

We advise such validations to be introduced to ensure no misconfiguration can occur.

Alleviation:

The upgrade function now properly sanitizes its argument against the zero-address, however, it does not validate that it is different than the currently set oracle.

RET-03S: Usage of Accuracy Numeric Literal

Description:

The numeric literal 1e18 is meant to represent the accuracy of each stakedEthToken unit but is not documented as such.

Example:

Copy
184function _calculateNewReward(
185    uint256 currentReward,
186    uint256 stakedEthAmount,
187    uint256 periodRewardPerToken
188)
189    internal pure returns (uint256)
190{
191    return currentReward.add(stakedEthAmount.mul(periodRewardPerToken).div(1e18));
192}

Recommendation:

We advise all instances of it to be replaced by a contract-level constant variable that properly depicts its intention. This does not alter the generated bytecode of the contract and increases the legibility and maintainability of the code.

Alleviation:

The Stakewise team stated that the constant is utilized in a single place only and as such should remain as is.

RET-04S: Usage of Percentage Numeric Literal

Description:

The numeric literal 1e4 is meant to represent the protocolFee accuracy but is not documented as such.

Example:

Copy
88/**
89 * @dev See {IRewardEthToken-setProtocolFee}.
90 */
91function setProtocolFee(uint256 _protocolFee) external override onlyAdmin {
92    require(_protocolFee < 1e4, "RewardEthToken: invalid protocol fee");
93    protocolFee = _protocolFee;
94    emit ProtocolFeeUpdated(_protocolFee);
95}

Recommendation:

We advise all instances of it to be replaced by a contract-level constant variable that properly depicts its intention. This does not alter the generated bytecode of the contract and increases the legibility and maintainability of the code.

Alleviation:

The Stakewise team stated that the constant is utilized in a single place only and as such should remain as is.