BaseRewardPool Manual Review Findings

BRP-01M: Improper Invocation of EIP-20 transferFrom

Description:

The linked statement does not properly validate the returned bool of the EIP-20 standard transferFrom function. As the standard dictates, callers must not assume that false is never returned, however, certain tokens such as USDT (Tether) are non-standard and yield no bool causing such checks to fail.

Example:

Copy
230require(
231    IERC20(_rewardToken).transferFrom(
232        msg.sender,
233        address(this),
234        _amountReward
235    )
236);

Recommendation:

We advise a safe wrapper library to be utilized instead such as SafeERC20 by OpenZeppelin to opportunistically validate the returned bool only if it exists.

Alleviation:

The safeTransferFrom function is now correctly utilized.

BRP-02M: Improper Unit Neutralization

Description:

The linked unit neutralization appears incorrect as the rewardPerTokenStored value already represents the reward token's deicmals and the calculation performs a multiplication of the staking and reward token decimals. Given that earned is meant to represent the reward token's decimals at the end, the staking decimals should be neutralized.

Example:

Copy
162function earned(address _account, address _rewardToken)
163    public
164    view
165    returns (uint256)
166{
167    return (
168        (((balanceOf(_account) *
169            (rewardPerToken(_rewardToken) -
170                userRewardPerTokenPaid[_rewardToken][_account])) /
171            (10**rewardDecimals(_rewardToken))) +
172            userRewards[_rewardToken][_account])
173    );
174}

Recommendation:

We advise the codebase to be updated accordingly to avoid incorrect rewards being distributed by the contract.

Alleviation:

The correct decimal units are now utilized by the earned function.