Open code423n4 opened 2 years ago
processExpiredLocks
CvxCrvRewardsLocker.processExpiredLocks; L133-L145
Gas could be saved by changing:
function processExpiredLocks(bool relock) external override returns (bool) { if (relock) { require(!prepareWithdrawal, Error.PREPARED_WITHDRAWAL); } if (relock) { ICvxLocker(CVX_LOCKER).processExpiredLocks(relock); } else { ICvxLocker(CVX_LOCKER).withdrawExpiredLocksTo(treasury); } return true; }
To:
function processExpiredLocks(bool relock) external override returns (bool) { if (relock) { require(!prepareWithdrawal, Error.PREPARED_WITHDRAWAL); ICvxLocker(CVX_LOCKER).processExpiredLocks(relock); } else { ICvxLocker(CVX_LOCKER).withdrawExpiredLocksTo(treasury); } return true; }
TopUpActionFeeHandler.payFees#L81-L109
If lpToken.safeTransferFrom reverts keeperAmount and treasuryAmount will be unnecessarily calculated.
lpToken.safeTransferFrom
keeperAmount
treasuryAmount
uint256 keeperAmount = amount.scaledMul(getKeeperFeeFraction()); uint256 treasuryAmount = amount.scaledMul(getTreasuryFeeFraction()); LpToken lpToken = LpToken(lpTokenAddress); lpToken.safeTransferFrom(msg.sender, address(this), amount);
I suggest setting keeperAmount and treasuryAmount after lpToken.safeTransferFrom as seen below.
LpToken lpToken = LpToken(lpTokenAddress); lpToken.safeTransferFrom(msg.sender, address(this), amount); uint256 keeperAmount = amount.scaledMul(getKeeperFeeFraction()); uint256 treasuryAmount = amount.scaledMul(getTreasuryFeeFraction());
hasAnyRole
RoleManager.hasAnyRole#L73-L86
i = 0
roles.length
++i
i++
In summary I suggest changing it from:
for (uint256 i = 0; i < roles.length; i++) { if (hasRole(roles[i], account)) { return true; } }
size = roles.length; for (uint256 i; i < size; ++i) { if (hasRole(roles[i], account)) { return true; } }
Also valid for(list not exhaustive):
GAS
G-01: Unnecessary if statement in
processExpiredLocks
CvxCrvRewardsLocker.processExpiredLocks; L133-L145
Gas could be saved by changing:
To:
G-02: Changing parameters order could save some gas
TopUpActionFeeHandler.payFees#L81-L109
If
lpToken.safeTransferFrom
revertskeeperAmount
andtreasuryAmount
will be unnecessarily calculated.I suggest setting
keeperAmount
andtreasuryAmount
afterlpToken.safeTransferFrom
as seen below.Ga-03: For loop in
hasAnyRole
can be more gas efficientRoleManager.hasAnyRole#L73-L86
i = 0
as zero is already the default value.roles.length
would save gas from calculating it every iteration++i
is more effecient than using a postfixi++
In summary I suggest changing it from:
To:
Also valid for(list not exhaustive):