code-423n4 / 2022-05-backd-findings

0 stars 0 forks source link

QA Report #89

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Summary

It was easy to understand the logic for me because the codes have detailed explanations. I recommended adding some more require() for better performance.

Low Risk Issues

  1. Add additional require() for better performance.

i) contracts\BkdLocker.sol#L49 require(_govToken != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED); require(_rewardToken != address(0), Error.ZERO_ADDRESS_NOT_ALLOWED);

ii) contracts\BkdLocker.sol#L60 "maxBoost" must be greater than "startBoost", otherwise L276-L278 will be revoked. require(maxBoost >= startBoost, Error.INVALID_AMOUNT);

iii) contracts\BkdLocker.sol#L60 "increasePeriod" must be positive, otherwise L276-L278 will be revoked. require(increasePeriod != 0, Error.INVALID_AMOUNT);

iv) contracts\BkdLocker.sol#L123 "amount" must be positive, otherwise the user can prepare unlock endlessly. require(amount != 0, Error.INVALID_AMOUNT);

v) contracts\BkdLocker.sol#L215 require(claimable != 0, Error.INVALID_AMOUNT);

vi) contracts\tokenomics\VestedEscrow.sol#L60 In this setFundAdmin() function at L75, it checks the first require() already. require(fundadmin_ != address(0), Error.ZERO_ADDRESS_NOTALLOWED); require(rewardToken != address(0), , Error.ZERO_ADDRESS_NOT_ALLOWED);

  1. Wrong comment contracts\BkdLocker.sol#L87

There are no "deposit" or "depositFor" functions in this contract. You need to write "lock" or "lockFor" instead.

  1. executeKeeperPoolWeight() and batchExecuteKeeperPoolWeights() functions in InflationManager.sol must have some role restrictions. There are 2 other functions. executeLpPoolWeight(), executeAmmTokenWeight().

contracts\tokenomics\InflationManager.sol#L151 contracts\tokenomics\InflationManager.sol#L241 contracts\tokenomics\InflationManager.sol#L326

With the above functions, the batch functions have the modifier "onlyRoles2(Roles.GOVERNANCE, Roles.INFLATION_MANAGER)". I think the above functions should have the same modifier also.

  1. claim() function in VestedEscrow.sol should have nonReentrant modifier same as other claim() function at L138.

contracts\tokenomics\VestedEscrow.sol#L113-L115

Otherwise, you can change the function like this(same as VestedEscrowRevocable.sol).

function claim() external virtual override {     claim(msg.sener); }

GalloDaSballo commented 2 years ago

claim() function in VestedEscrow.sol should have nonReentrant modifier same as other claim() function at L138.

Valid

rest seem minor, personally the formatting takes away from what could be a pretty decent report.