RewardsPoolBase Code Style Findings

RPB-01C: Data Location Optimization

Description:

The linked array declarations are stored in memory whilst their associated function signatures are declared as external.

Example:

Copy
447function extend(uint256 _endBlock, uint256[] memory _rewardsPerBlock)
448    external
449    virtual
450    onlyFactory
451{

Recommendation:

We advise that the data locations are instead set to calldata to optimize the gas cost of the relevant code segments.

Alleviation:

This particular function was adjusted, however, the same optimization can be applied to the new arguments and we advise them to be adjusted properly.

RPB-02C: Duplicate Calculation

Description:

The linked calculations are also carried out by the factory implementation thus causing them to be evaluated twice redundantly.

Example:

Copy
466for (uint256 i = 0; i < _rewardsPerBlock.length; i++) {
467    uint256 currentRemainingReward =
468        calculateRewardsAmount(
469            _getBlock(),
470            endBlock,
471            rewardPerBlock[i]
472        );
473    uint256 newRemainingReward =
474        calculateRewardsAmount(
475            _getBlock(),
476            _endBlock,
477            _rewardsPerBlock[i]
478        );
479
480    address rewardsToken = rewardsTokens[i];
481
482    if (currentRemainingReward > newRemainingReward) {
483        // Some reward leftover needs to be returned
484        IERC20Detailed(rewardsToken).safeTransfer(
485            msg.sender,
486            currentRemainingReward.sub(newRemainingReward)
487        );
488    }
489
490    rewardPerBlock[i] = _rewardsPerBlock[i];
491}

Recommendation:

We advise that these values are relayed to the extend function from the factory to reduce the gas cost involved in executing the function.

Alleviation:

Adjustments were made across the codebase to properly pass in all the pre-calculated rewards greatly optimizing the gas cost involved in invoking the function.

RPB-03C: Duplication of require Checks

Description:

The linked require checks are already validated by the factory that deploys the RewardPoolBase and thus are considered redundant.

Example:

Copy
50require(
51    address(_stakingToken) != address(0),
52    "Constructor::Invalid staking token address"
53);
54require(
55    _rewardPerBlock.length > 0,
56    "Constructor::Rewards per block are not provided"
57);
58require(
59    _rewardsTokens.length > 0,
60    "Constructor::Rewards tokens are not provided"
61);
62require(
63    _startBlock > _getBlock(),
64    "Constructor::The starting block must be in the future."
65);
66require(
67    _endBlock > _getBlock(),
68    "Constructor::The end block must be in the future."
69);
70require(
71    _rewardPerBlock.length == _rewardsTokens.length,
72    "Constructor::Rewards per block and rewards tokens must be with the same length."
73);
74require(_stakeLimit != 0, "Constructor::Stake limit needs to be more than 0");

Recommendation:

We advise that either the checks imposed by the factory or the contract are omitted the former of which we recommend to ensure optimal gas utilization.

Alleviation:

The linked require checks were omitted from the contract's constructor thus optimizing the gas cost of deployment.

RPB-04C: Ineffectual Check

Description:

The require check condition of the linked segment is guaranteed if the check of L54-L57 and L70-L73 succeed.

Example:

Copy
50require(
51    address(_stakingToken) != address(0),
52    "Constructor::Invalid staking token address"
53);
54require(
55    _rewardPerBlock.length > 0,
56    "Constructor::Rewards per block are not provided"
57);
58require(
59    _rewardsTokens.length > 0,
60    "Constructor::Rewards tokens are not provided"
61);
62require(
63    _startBlock > _getBlock(),
64    "Constructor::The starting block must be in the future."
65);
66require(
67    _endBlock > _getBlock(),
68    "Constructor::The end block must be in the future."
69);
70require(
71    _rewardPerBlock.length == _rewardsTokens.length,
72    "Constructor::Rewards per block and rewards tokens must be with the same length."
73);
74require(_stakeLimit != 0, "Constructor::Stake limit needs to be more than 0");

Recommendation:

We advise that the ineffectual require check is safely omitted from the codebase.

Alleviation:

The linked require check was omitted from the codebase.

RPB-05C: Inefficient Loop Limit Evaluation

Description:

The linked for loops all extract the limit until which they are meant to iterate from the length member of an in-storage array thus causing a redundant load on each iteration.

Example:

Copy
84for (uint256 i = 0; i < rewardsTokens.length; i++) {
85    accumulatedRewardMultiplier.push(0);
86}

Recommendation:

We advise that the limits are stored to temporary in-memory variables greatly optimizing the gas cost involved in iterating within these loops.

Alleviation:

All linked statements apart from the first one from the constructor were adjusted to cache the length member of the arrays first preventing redundant lookups.

RPB-06C: Redundant SafeMath Utilization

Description:

The linked sub statement is considered redundant as the conditionals applied on L244 and L248 ensure that the subtraction will never underflow.

Example:

Copy
242uint256 currentBlock = _getBlock();
243
244if (currentBlock <= lastRewardBlock) {
245    return;
246}
247
248uint256 applicableBlock = (currentBlock < endBlock) ? currentBlock : endBlock;
249
250uint256 blocksSinceLastReward = applicableBlock.sub(lastRewardBlock);

Recommendation:

We advise that the sub utilization is safely omitted from the codebase thus optimizing the gas cost of the surrounding code block.

Alleviation:

The usage of sub was replaced by a literal subtraction as advised.

RPB-07C: Redundant SafeMath Utilization

Description:

The linked sub invocation performs an underflow-bound subtraction between currentRemainingReward and newRemainingReward whereas the surrounding if clause guarantees that currentRemainingReward is strictly greater than newRemainingReward.

Example:

Copy
482if (currentRemainingReward > newRemainingReward) {
483    // Some reward leftover needs to be returned
484    IERC20Detailed(rewardsToken).safeTransfer(
485        msg.sender,
486        currentRemainingReward.sub(newRemainingReward)
487    );
488}

Recommendation:

We advise that the sub invocation is safely omitted to optimize the gas cost of the surrounding code block.

Alleviation:

The SafeMath subtraction was replaced by its literal counterpart according to our recommendation.