Omniscia Alliance Block Audit

RewardsPoolFactory Code Style Findings

RewardsPoolFactory Code Style Findings

RPF-01C: Data Location Optimization

TypeSeverityLocation
Gas OptimizationInformationalRewardsPoolFactory.sol:L35, L36, L103

Description:

The deploy function contains two dynamic array arguments that are stored in memory and is declared as external. The extendRewardPool function contains a single dynamic array argument stored in memory and is declared external as well

Example:

contracts/RewardsPoolFactory.sol
31function deploy(
32 address _stakingToken,
33 uint256 _startBlock,
34 uint256 _endBlock,
35 address[] memory _rewardsTokens,
36 uint256[] memory _rewardPerBlock,
37 uint256 _stakeLimit
38) external onlyOwner {

Recommendation:

We advise that the data location the arrays are stored in is changed to calldata to optimize the gas cost of the code segments.

Alleviation:

The variables involved in the deploy function were indeed optimized, however, the latter of the three remains unoptimized and we advise it to be done so.

RPF-02C: Redundant Usage of SafeMath

TypeSeverityLocation
Gas OptimizationInformationalRewardsPoolFactory.sol:L118

Description:

The linked safeTransfer invocation passes in the result of a SafeMath subtraction between the variables newRemainingReward and currentRemainingReward whereas the whole statement is included within an if clause that ensures newRemainingReward is strictly greater-than currentRemainingReward.

Example:

contracts/RewardsPoolFactory.sol
110for (uint256 i = 0; i < _rewardsPerBlock.length; i++) {
111 uint256 currentRemainingReward = calculateRewardsAmount(block.number, currentEndBlock, pool.rewardPerBlock(i));
112 uint256 newRemainingReward = calculateRewardsAmount(block.number, _endBlock, _rewardsPerBlock[i]);
113
114 address rewardsToken = RewardsPoolBase(_rewardsPoolAddress).rewardsTokens(i);
115
116 if (newRemainingReward > currentRemainingReward) {
117 // Some more reward needs to be transferred to the rewards pool contract
118 IERC20Detailed(rewardsToken).safeTransfer(_rewardsPoolAddress, newRemainingReward.sub(currentRemainingReward));
119 }
120}

Recommendation:

We advise that the sub invocation is safely omitted as the subtraction is guaranteed to never underflow, optimizing the gas cost of the function.

Alleviation:

The redundant SafeMath utilization was replaced by its raw counterpart optimizing the gas cost of the function.

RPF-03C: Redundant Validation Loop

TypeSeverityLocation
Gas OptimizationInformationalRewardsPoolFactory.sol:L52-L61

Description:

Within Solidity gas costs are of great concern and as such code should be developed as optimal as possible. The linked for loop validates the input _rewardsTokens and _rewardPerBlock prior to their utilization in the ensuing for loop of L79-L90 redundantly so.

Example:

contracts/RewardsPoolFactory.sol
52for (uint256 i = 0; i < _rewardsTokens.length; i++) {
53 require(
54 _rewardsTokens[i] != address(0),
55 "RewardsPoolFactory::deploy: Reward token address could not be invalid"
56 );
57 require(
58 _rewardPerBlock[i] != 0,
59 "RewardsPoolFactory::deploy: Reward per block must be greater than zero"
60 );
61}
62 require(
63 _stakeLimit != 0,
64 "RewardsPoolFactory::deploy: Stake limit must be more than 0"
65);
66
67address rewardsPoolBase =
68 address(
69 new RewardsPoolBase(
70 IERC20Detailed(_stakingToken),
71 _startBlock,
72 _endBlock,
73 _rewardsTokens,
74 _rewardPerBlock,
75 _stakeLimit
76 )
77 );
78
79for (uint256 i = 0; i < _rewardsTokens.length; i++) {
80 uint256 rewardsAmount =
81 calculateRewardsAmount(
82 _startBlock,
83 _endBlock,
84 _rewardPerBlock[i]
85 );
86 IERC20Detailed(_rewardsTokens[i]).safeTransfer(
87 rewardsPoolBase,
88 rewardsAmount
89 );
90}

Recommendation:

We advise that the validation is performed directly on the for loop that utilizes the variables so as to reduce the gas cost involved in utilizing the function.

Alleviation:

The redundant for sanitization loop was omitted and its inner statements were placed under the for loop they are directly utilized in.