Booster Code Style Findings

BOO-01C: Deprecated if-revert Pattern

Description:

The linked programming pattern has been deprecated in favor of a require check.

Example:

Copy
84/// @notice Set fees
85/// @dev only fee manager
86/// Values are per 10000.
87/// Total fees are maxed to 20%
88/// @param _ecdLockedIncentive Amount of incentives to distribute to veECD holders
89/// @param _callerFees Amount of incentives to disitribute to function caller.
90/// @param _platform Amount of incentives to disitribute to the platform.
91function setFees(
92    uint256 _ecdLockedIncentive,
93    uint256 _callerFees,
94    uint256 _platform
95) external {
96    require(msg.sender == feeManager, "!auth");
97
98    uint256 total = _ecdLockedIncentive + _callerFees + _platform;
99    require(total <= MaxFees, ">MaxFees");
100
101    if (
102        _ecdLockedIncentive >= 300 &&
103        _ecdLockedIncentive <= 1000 &&
104        _callerFees <= 50 &&
105        _platform <= 1000
106    ) {
107        ecdLockedIncentive = _ecdLockedIncentive;
108        earmarkIncentive = _callerFees;
109        platformFee = _platform;
110    } else {
111        revert("fees out of bounds");
112    }
113}

Recommendation:

We advise such a check to be imposed in the codebase and the if-revert structure to be omitted.

Alleviation:

The statements were refactored to a require statement per our recommendation.

BOO-02C: Inefficient Data Location Specifier

Description:

The linked argument is declared as memory in an external function.

Example:

Copy
252function claimRewards(uint256[] memory _pids) external nonReentrant {

Recommendation:

We advise it to be set as calldata optimizing the function's gas cost.

Alleviation:

The location specifier was properly set to calldata.

BOO-03C: Inefficient Execution Guard

Description:

The linked execution guards are in the form of assert statements that will deplete all allocated gas for the transaction in case of failure.

Example:

Copy
167assert(address(pools[_pid].lpToken) != address(0x0));

Recommendation:

As the Solidity documentation states, the assert statement should solely be utilized for invariants and otherwise guaranteed-to-be-true conditions (such as the result of mathematical formulas) rather than input validation. As such, we advise a require check to be utilized in all linked instances that contains a descriptive error message.

Alleviation:

All assert instructions were replaced by require statements accordingly.

BOO-04C: Inefficient Execution of Incentive Distribution

Description:

The linked segments do not validate whether a non-zero fee has been imposed and as such can result in multiple redundant statements and external calls in such a case.

Example:

Copy
277function _distributeIncentives(uint256 amount) private returns (uint256) {
278    if (amount == 0) return (0);
279
280    uint256 _ecdLockedIncentive = (amount * ecdLockedIncentive) /
281        FEE_DENOMINATOR;
282    uint256 _callIncentive = (amount * earmarkIncentive) / FEE_DENOMINATOR;
283
284    if (
285        treasury != address(0) &&
286        treasury != address(this) &&
287        platformFee > 0
288    ) {
289        uint256 _platform = (amount * platformFee) / FEE_DENOMINATOR;
290        amount = amount - _platform;
291        IERC20(ptp).safeTransfer(treasury, _platform);
292    }
293
294    IERC20(ptp).safeTransfer(msg.sender, _callIncentive);
295
296    //send stakers's share of ptp to reward contract
297    SafeERC20.safeApprove(ptp, ecdStakerRewardPool, 0);
298    SafeERC20.safeApprove(ptp, ecdStakerRewardPool, amount);
299
300    IVeEcdRewardsPool(ecdStakerRewardPool).queueNewRewards(
301        _ecdLockedIncentive
302    );
303
304    return amount - _ecdLockedIncentive - _callIncentive;
305}

Recommendation:

We advise zero-value validation to wrap the linked statements to ensure optimal execution of the function's code.

Alleviation:

All statements relating to fee are now conditionally executed to optimize the codebase.

BOO-05C: Inexistent Error Message

Description:

The linked require check has no error message explicitly defined.

Example:

Copy
441require(poolInfo.rewarder != address(0x0));

Recommendation:

We advise one to be set so to aid in the validation of the require's condition as well as the legibility of the codebase.

Alleviation:

An error message was properly introduced to the linked require check.

BOO-06C: Inexistent Visibility Specifiers

Description:

The linked variables have no visibility specifiers explicitly set.

Example:

Copy
37mapping(uint256 => PoolInfo) public pools;
38uint256[] existingPools;
39mapping(uint256 => ExtraRewardsPool) public extraRewardsPools;
40IMasterPlatypus public masterPlatypus;
41IPlatypusProxy public depositorProxy;
42IRewardFactory public rewardFactory;
43IERC20 public ptp;

Recommendation:

We advise them to be set so to avoid potential compilation discrepancies in the future as the default behaviour is for the compiler to assign one automatically which may change in future pragma versions.

Alleviation:

The linked variables had their visibility specifiers explicitly set.

BOO-07C: Variable Mutability Specifiers

Description:

The linked variables are assigned to only once during the contract's constructor.

Example:

Copy
57constructor(
58    address _masterPlatypus,
59    address _depositorProxy,
60    address _ptp
61) {
62    masterPlatypus = IMasterPlatypus(_masterPlatypus);
63    depositorProxy = IPlatypusProxy(_depositorProxy);
64    ptp = IERC20(_ptp);
65
66    feeManager = msg.sender;
67}

Recommendation:

We advise them to be set as immutable greatly optimizing the codebase.

Alleviation:

All three variables were correctly set as immutable optimizing the codebase.