Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #297

Open
code423n4 opened this issue May 25, 2022 · 0 comments
Open

Gas Optimizations #297

code423n4 opened this issue May 25, 2022 · 0 comments
Labels
bug Something isn't working duplicate This issue or pull request already exists G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

1. Cache array length outside of loop

Impact

Caching the array length outside a loop saves reading it on each iteration, as long as the array's length is not changed during the loop.

Proof of Concept

AuraClaimZap.sol:143:        for (uint256 i = 0; i < rewardContracts.length; i++) {
AuraClaimZap.sol:147:        for (uint256 i = 0; i < extraRewardContracts.length; i++) {
AuraClaimZap.sol:151:        for (uint256 i = 0; i < tokenRewardContracts.length; i++) {
AuraLocker.sol:696:        for (uint256 i = nextUnlockIndex; i < locks.length; i++) {
AuraVestedEscrow.sol:100:        for (uint256 i = 0; i < _recipient.length; i++) {

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to cache the array length outside of for loop.

2. Long revert error messages

Impact

Shortening revert error messages to fit in 32 bytes will decrease gas costs for deployment and gas costs when the revert condition has been met.

Proof of Concept

AuraLocker.sol:197:        require(_rewardsToken != address(stakingToken), "Cannot add StakingToken as reward");

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to decrease revert messages to maximum 32 bytes. Alternatively custom error types should be used.

3. Use custom errors instead of revert strings to save gas

Impact

Usage of Custom Errors reduces the gas cost.

Proof of Concept

Aura.sol:66:        require(msg.sender == operator, "Only operator");
Aura.sol:67:        require(totalSupply() == 0, "Only once");
Aura.sol:68:        require(_amount > 0, "Must mint something");
Aura.sol:69:        require(_minter != address(0), "Invalid minter");
Aura.sol:92:        require(totalSupply() != 0, "Not initialised");
Aura.sol:129:        require(msg.sender == minter, "Only minter");
AuraBalRewardPool.sol:77:        require(_startDelay < 2 weeks, "!delay");
AuraBalRewardPool.sol:121:        require(_amount > 0, "RewardPool : Cannot stake 0");
AuraBalRewardPool.sol:139:        require(_amount > 0, "RewardPool : Cannot stake 0");
AuraBalRewardPool.sol:157:        require(amount > 0, "RewardPool : Cannot withdraw 0");
AuraBalRewardPool.sol:206:        require(msg.sender == rewardManager || block.timestamp > startTime, "!authorized");
AuraBalRewardPool.sol:207:        require(rewardRate == 0, "!one time");
AuraBalRewardPool.sol:210:        require(rewardsAvailable > 0, "!balance");
AuraClaimZap.sol:96:        require(msg.sender == owner, "!auth");
AuraClaimZap.sol:137:        require(tokenRewardContracts.length == tokenRewardTokens.length, "!parity");
AuraLocker.sol:196:        require(rewardData[_rewardsToken].lastUpdateTime == 0, "Reward already exists");
AuraLocker.sol:197:        require(_rewardsToken != address(stakingToken), "Cannot add StakingToken as reward");
AuraLocker.sol:210:        require(rewardData[_rewardsToken].lastUpdateTime > 0, "Reward does not exist");
AuraLocker.sol:216:        require(_rate <= 500, "over max rate"); //max 5% per epoch
AuraLocker.sol:217:        require(_delay >= 2, "min delay"); //minimum 2 epochs of grace
AuraLocker.sol:232:        require(_tokenAddress != address(stakingToken), "Cannot withdraw staking token");
AuraLocker.sol:233:        require(rewardData[_tokenAddress].lastUpdateTime == 0, "Cannot withdraw reward token");
AuraLocker.sol:259:        require(_amount > 0, "Cannot stake 0");
AuraLocker.sol:260:        require(!isShutdown, "shutdown");
AuraLocker.sol:353:        require(isShutdown, "Must be shutdown");
AuraLocker.sol:359:        require(amt > 0, "Nothing locked");
AuraLocker.sol:385:        require(length > 0, "no locks");
AuraLocker.sol:431:        require(locked > 0, "no exp locks");
AuraLocker.sol:471:        require(len > 0, "Nothing to delegate");
AuraLocker.sol:472:        require(newDelegatee != address(0), "Must delegate to someone");
AuraLocker.sol:476:        require(newDelegatee != oldDelegatee, "Must choose new delegatee");
AuraLocker.sol:598:        require(timestamp <= block.timestamp, "ERC20Votes: block not yet mined");
AuraLocker.sol:616:        require(timestamp < block.timestamp, "ERC20Votes: block not yet mined");
AuraLocker.sol:655:        require(epochStart < block.timestamp, "Epoch is in the future");
AuraLocker.sol:719:        require(epochStart < block.timestamp, "Epoch is in the future");
AuraLocker.sol:821:        require(rewardDistributors[cvxCrv][msg.sender], "!authorized");
AuraLocker.sol:822:        require(_rewards > 0, "No reward");
AuraLocker.sol:849:        require(_rewardsToken != cvxCrv, "Use queueNewRewards");
AuraLocker.sol:850:        require(rewardDistributors[_rewardsToken][msg.sender], "Must be rewardsDistributor");
AuraLocker.sol:851:        require(_reward > 0, "No reward");
AuraMath.sol:40:        require(a <= type(uint224).max, "AuraMath: uint224 Overflow");
AuraMath.sol:45:        require(a <= type(uint128).max, "AuraMath: uint128 Overflow");
AuraMath.sol:50:        require(a <= type(uint112).max, "AuraMath: uint112 Overflow");
AuraMath.sol:55:        require(a <= type(uint96).max, "AuraMath: uint96 Overflow");
AuraMath.sol:60:        require(a <= type(uint32).max, "AuraMath: uint32 Overflow");
AuraMerkleDrop.sol:69:        require(_expiresAfter > 2 weeks, "!expiry");
AuraMerkleDrop.sol:78:        require(msg.sender == dao, "!auth");
AuraMerkleDrop.sol:84:        require(msg.sender == dao, "!auth");
AuraMerkleDrop.sol:85:        require(merkleRoot == bytes32(0), "already set");
AuraMerkleDrop.sol:91:        require(msg.sender == dao, "!auth");
AuraMerkleDrop.sol:97:        require(msg.sender == dao, "!auth");
AuraMerkleDrop.sol:98:        require(block.timestamp > expiryTime, "!expired");
AuraMerkleDrop.sol:105:        require(msg.sender == dao, "!auth");
AuraMerkleDrop.sol:119:        require(merkleRoot != bytes32(0), "!root");
AuraMerkleDrop.sol:120:        require(block.timestamp > startTime, "!started");
AuraMerkleDrop.sol:121:        require(block.timestamp < expiryTime, "!active");
AuraMerkleDrop.sol:122:        require(_amount > 0, "!amount");
AuraMerkleDrop.sol:123:        require(hasClaimed[msg.sender] == false, "already claimed");
AuraMerkleDrop.sol:126:        require(MerkleProof.verify(_proof, merkleRoot, leaf), "invalid proof");
AuraMerkleDrop.sol:152:        require(penaltyForwarder != address(0), "!forwarder");
AuraMinter.sol:32:        require(block.timestamp > inflationProtectionTime, "Inflation protected for now");
AuraPenaltyForwarder.sol:48:        require(block.timestamp > lastDistribution + distributionDelay, "!elapsed");
AuraPenaltyForwarder.sol:52:        require(bal > 0, "!empty");
AuraStakingProxy.sol:89:        require(msg.sender == owner, "!auth");
AuraStakingProxy.sol:90:        require(_outputBps > 9000 && _outputBps < 10000, "Invalid output bps");
AuraStakingProxy.sol:100:        require(msg.sender == owner, "!auth");
AuraStakingProxy.sol:108:        require(msg.sender == owner, "!auth");
AuraStakingProxy.sol:116:        require(msg.sender == owner, "!auth");
AuraStakingProxy.sol:117:        require(pendingOwner != address(0), "invalid owner");
AuraStakingProxy.sol:128:        require(msg.sender == owner, "!auth");
AuraStakingProxy.sol:129:        require(_incentive <= 100, "too high");
AuraStakingProxy.sol:138:        require(msg.sender == owner, "!auth");
AuraStakingProxy.sol:158:        require(msg.sender == owner, "!auth");
AuraStakingProxy.sol:159:        require(_token != crv && _token != cvx && _token != cvxCrv, "not allowed");
AuraStakingProxy.sol:172:            require(msg.sender == keeper, "!auth");
AuraStakingProxy.sol:203:        require(address(_token) != crv && address(_token) != cvxCrv, "not allowed");
AuraVestedEscrow.sol:56:        require(starttime_ >= block.timestamp, "start must be future");
AuraVestedEscrow.sol:57:        require(endtime_ > starttime_, "end must be greater");
AuraVestedEscrow.sol:66:        require(totalTime >= 16 weeks, "!short");
AuraVestedEscrow.sol:78:        require(msg.sender == admin, "!auth");
AuraVestedEscrow.sol:87:        require(msg.sender == admin, "!auth");
AuraVestedEscrow.sol:97:        require(!initialised, "initialised already");
AuraVestedEscrow.sol:117:        require(msg.sender == admin, "!auth");
AuraVestedEscrow.sol:118:        require(totalLocked[_recipient] > 0, "!funding");
AuraVestedEscrow.sol:185:            require(address(auraLocker) != address(0), "!auraLocker");
BalLiquidityProvider.sol:47:        require(msg.sender == provider, "!auth");
BalLiquidityProvider.sol:48:        require(_request.assets.length == 2 && _request.maxAmountsIn.length == 2, "!valid");
BalLiquidityProvider.sol:49:        require(pairToken.balanceOf(address(this)) > minPairAmount, "!minLiq");
BalLiquidityProvider.sol:53:            require(asset == address(startToken) || asset == address(pairToken), "!asset");
BalLiquidityProvider.sol:57:            require(bal > 0 && bal == _request.maxAmountsIn[i], "!bal");
BalLiquidityProvider.sol:65:        require(supplyBefore == 0, "!init");
BalLiquidityProvider.sol:70:        require(balAfter > 0, "!mint");
BalLiquidityProvider.sol:79:        require(msg.sender == dao, "!auth");
BalLiquidityProvider.sol:89:        require(msg.sender == provider || msg.sender == dao, "!auth");
ClaimFeesHelper.sol:45:        require(tokenTime > lastTokenTimes[address(_token)], "not time yet");
CrvDepositorWrapper.sol:42:        require(poolAddress != address(0), "!poolAddress");
CrvDepositorWrapper.sol:119:        require(IERC20(BALANCER_POOL_TOKEN).approve(crvDeposit, type(uint256).max), "!approval");
ExtraRewardsDistributor.sol:68:        require(_epoch <= latestEpoch, "Cannot assign to the future");
ExtraRewardsDistributor.sol:74:            require(len == 0 || rewardEpochs[_token][len - 1] < _epoch, "Cannot backdate to this epoch");
ExtraRewardsDistributor.sol:171:        require(_index > 0 && _index < rewardEpochs[_token].length - 1, "!past");
ExtraRewardsDistributor.sol:172:        require(_index >= userClaims[_token][msg.sender], "already claimed");

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to add Custom Errors.

4. ++i/--i costs less gas compared to i++, i += 1, i-- or i -= 1

Impact

++i or --i costs less gas compared to i++, i += 1, i-- or i -= 1 for unsigned integer as pre-increment/pre-decrement is cheaper (about 5 gas per iteration).

Proof of Concept

AuraClaimZap.sol:143:        for (uint256 i = 0; i < rewardContracts.length; i++) {
AuraClaimZap.sol:147:        for (uint256 i = 0; i < extraRewardContracts.length; i++) {
AuraClaimZap.sol:151:        for (uint256 i = 0; i < tokenRewardContracts.length; i++) {
AuraLocker.sol:174:            for (uint256 i = 0; i < rewardTokensLength; i++) {
AuraLocker.sol:306:        for (uint256 i; i < rewardTokensLength; i++) {
AuraLocker.sol:410:            for (uint256 i = nextUnlockIndex; i < length; i++) {
AuraLocker.sol:426:                nextUnlockIndex++;
AuraLocker.sol:696:        for (uint256 i = nextUnlockIndex; i < locks.length; i++) {
AuraLocker.sol:702:                idx++;
AuraLocker.sol:773:        for (uint256 i = 0; i < userRewardsLength; i++) {
AuraVestedEscrow.sol:100:        for (uint256 i = 0; i < _recipient.length; i++) {
BalLiquidityProvider.sol:51:        for (uint256 i = 0; i < 2; i++) {
ExtraRewardsDistributor.sol:233:        for (uint256 i = epochIndex; i < tokenEpochs; i++) {
AuraLocker.sol:497:                i--;
AuraLocker.sol:664:        for (uint256 i = locksLength; i > 0; i--) {
AuraLocker.sol:726:        for (uint256 i = epochIndex + 1; i > 0; i--) {

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to use ++i or --i instead of i++, i += 1, i-- or i -= 1 to increment value of an unsigned integer variable.

5. Obsolete overflow/underflow check

Impact

Starting from solidity 0.8.0 there is built-in check for overflows/underflows. This mechanism automatically checks if the variable overflows or underflows and throws an error. Multiple contracts use increments that cannot overflow but consume additional gas for checks.

Proof of Concept

AuraClaimZap.sol:143:        for (uint256 i = 0; i < rewardContracts.length; i++) {
AuraClaimZap.sol:147:        for (uint256 i = 0; i < extraRewardContracts.length; i++) {
AuraClaimZap.sol:151:        for (uint256 i = 0; i < tokenRewardContracts.length; i++) {
AuraLocker.sol:174:            for (uint256 i = 0; i < rewardTokensLength; i++) {
AuraLocker.sol:306:        for (uint256 i; i < rewardTokensLength; i++) {
AuraLocker.sol:410:            for (uint256 i = nextUnlockIndex; i < length; i++) {
AuraLocker.sol:426:                nextUnlockIndex++;
AuraLocker.sol:696:        for (uint256 i = nextUnlockIndex; i < locks.length; i++) {
AuraLocker.sol:702:                idx++;
AuraLocker.sol:773:        for (uint256 i = 0; i < userRewardsLength; i++) {
AuraVestedEscrow.sol:100:        for (uint256 i = 0; i < _recipient.length; i++) {
BalLiquidityProvider.sol:51:        for (uint256 i = 0; i < 2; i++) {
ExtraRewardsDistributor.sol:233:        for (uint256 i = epochIndex; i < tokenEpochs; i++) {
AuraLocker.sol:497:                i--;
AuraLocker.sol:664:        for (uint256 i = locksLength; i > 0; i--) {
AuraLocker.sol:726:        for (uint256 i = epochIndex + 1; i > 0; i--) {

Recommended Mitigation Steps

It is recommended wrap incrementing of i with unchecked block unchecked { ++i } or unchecked { --i }.

6. No need to explicitly initialize variables with default values

Impact

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for addresses). Explicitly initializing it with its default value is an anti-pattern and waste of gas.

Proof of Concept

AuraLocker.sol:114:    bool public isShutdown = false;
AuraVestedEscrow.sol:33:    bool public initialised = false;
AuraBalRewardPool.sol:35:    uint256 public pendingPenalty = 0;
AuraBalRewardPool.sol:38:    uint256 public periodFinish = 0;
AuraBalRewardPool.sol:39:    uint256 public rewardRate = 0;
AuraClaimZap.sol:143:        for (uint256 i = 0; i < rewardContracts.length; i++) {
AuraClaimZap.sol:147:        for (uint256 i = 0; i < extraRewardContracts.length; i++) {
AuraClaimZap.sol:151:        for (uint256 i = 0; i < tokenRewardContracts.length; i++) {
AuraLocker.sol:72:    uint256 public queuedCvxCrvRewards = 0;
AuraLocker.sol:174:            for (uint256 i = 0; i < rewardTokensLength; i++) {
AuraLocker.sol:381:        uint256 reward = 0;
AuraLocker.sol:485:        uint256 futureUnlocksSum = 0;
AuraLocker.sol:540:                    uint256 unlocksSinceLatestCkpt = 0;
AuraLocker.sol:630:        uint256 low = 0;
AuraLocker.sol:773:        for (uint256 i = 0; i < userRewardsLength; i++) {
AuraMerkleDrop.sol:29:    uint256 public pendingPenalty = 0;
AuraVestedEscrow.sol:99:        uint256 totalAmount = 0;
AuraVestedEscrow.sol:100:        for (uint256 i = 0; i < _recipient.length; i++) {
BalLiquidityProvider.sol:51:        for (uint256 i = 0; i < 2; i++) {
ExtraRewardsDistributor.sol:231:        uint256 claimableTokens = 0;

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to remove explicit initializations with default values.

7. Use != 0 instead of > 0 for unsigned integer comparison

Impact

When dealing with unsigned integer types, comparisons with != 0 are cheaper that with > 0.

Proof of Concept

Aura.sol:68:        require(_amount > 0, "Must mint something");
AuraBalRewardPool.sol:121:        require(_amount > 0, "RewardPool : Cannot stake 0");
AuraBalRewardPool.sol:139:        require(_amount > 0, "RewardPool : Cannot stake 0");
AuraBalRewardPool.sol:157:        require(amount > 0, "RewardPool : Cannot withdraw 0");
AuraBalRewardPool.sol:178:        if (reward > 0) {
AuraBalRewardPool.sol:210:        require(rewardsAvailable > 0, "!balance");
AuraClaimZap.sol:196:        if (depositCrvMaxAmount > 0) {
AuraClaimZap.sol:200:            if (crvBalance > 0) {
AuraClaimZap.sol:218:        if (depositCvxMaxAmount > 0) {
AuraClaimZap.sol:221:            if (cvxBalance > 0) {
AuraLocker.sol:210:        require(rewardData[_rewardsToken].lastUpdateTime > 0, "Reward does not exist");
AuraLocker.sol:259:        require(_amount > 0, "Cannot stake 0");
AuraLocker.sol:309:            if (reward > 0) {
AuraLocker.sol:359:        require(amt > 0, "Nothing locked");
AuraLocker.sol:385:        require(length > 0, "no locks");
AuraLocker.sol:400:            if (_checkDelay > 0) {
AuraLocker.sol:419:                if (_checkDelay > 0) {
AuraLocker.sol:431:        require(locked > 0, "no exp locks");
AuraLocker.sol:443:        if (reward > 0) {
AuraLocker.sol:471:        require(len > 0, "Nothing to delegate");
AuraLocker.sol:496:            if (i > 0) {
AuraLocker.sol:520:            if (ckpts.length > 0) {
AuraLocker.sol:664:        for (uint256 i = locksLength; i > 0; i--) {
AuraLocker.sol:726:        for (uint256 i = epochIndex + 1; i > 0; i--) {
AuraLocker.sol:822:        require(_rewards > 0, "No reward");
AuraLocker.sol:851:        require(_reward > 0, "No reward");
AuraMerkleDrop.sol:122:        require(_amount > 0, "!amount");
AuraPenaltyForwarder.sol:52:        require(bal > 0, "!empty");
AuraStakingProxy.sol:177:        if (crvBal > 0) {
AuraStakingProxy.sol:185:        if (cvxCrvBal > 0) {
AuraStakingProxy.sol:207:        if (bal > 0) {
AuraVestedEscrow.sol:118:        require(totalLocked[_recipient] > 0, "!funding");
BalLiquidityProvider.sol:57:            require(bal > 0 && bal == _request.maxAmountsIn[i], "!bal");
BalLiquidityProvider.sol:70:        require(balAfter > 0, "!mint");
ExtraRewardsDistributor.sol:149:        if (claimableTokens > 0) {
ExtraRewardsDistributor.sol:171:        require(_index > 0 && _index < rewardEpochs[_token].length - 1, "!past");

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to use != 0 instead of > 0.

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels May 25, 2022
code423n4 added a commit that referenced this issue May 25, 2022
@0xMaharishi 0xMaharishi added the duplicate This issue or pull request already exists label May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

2 participants