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 #24

Open
code423n4 opened this issue May 12, 2022 · 2 comments
Open

Gas Optimizations #24

code423n4 opened this issue May 12, 2022 · 2 comments
Labels
bug Something isn't working G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

Gas optimization

File Aura.sol

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/Aura.sol

Remove unnecesarry libraries and external calls, Aura math do plain math operations, there is no need to use this libraries. You also arent using Address lib and SafeERC20

--- a/contracts/Aura.sol
+++ b/contracts/Aura.sol
@@ -3,8 +3,8 @@ pragma solidity ^0.8.11;
 
 import { IERC20 } from "@openzeppelin/contracts-0.8/token/ERC20/IERC20.sol";
 import { ERC20 } from "@openzeppelin/contracts-0.8/token/ERC20/ERC20.sol";
-import { SafeERC20 } from "@openzeppelin/contracts-0.8/token/ERC20/utils/SafeERC20.sol";
-import { Address } from "@openzeppelin/contracts-0.8/utils/Address.sol";
+// import { SafeERC20 } from "@openzeppelin/contracts-0.8/token/ERC20/utils/SafeERC20.sol";
+// import { Address } from "@openzeppelin/contracts-0.8/utils/Address.sol";
 import { AuraMath } from "./AuraMath.sol";
 
@@ -19,8 +19,8 @@ interface IStaker {
  */
 contract AuraToken is ERC20 {
-    using SafeERC20 for IERC20;
-    using Address for address;
-    using AuraMath for uint256;
+    // using SafeERC20 for IERC20;
+    // using Address for address;
+    // using AuraMath for uint256;
 
     address public operator;
     address public immutable vecrvProxy;
@@ -49,7 +49,7 @@ contract AuraToken is ERC20 {
     ) ERC20(_nameArg, _symbolArg) {
         operator = msg.sender;
         vecrvProxy = _proxy;
-        reductionPerCliff = EMISSIONS_MAX_SUPPLY.div(totalCliffs);
+        reductionPerCliff = EMISSIONS_MAX_SUPPLY / totalCliffs;
     }
 
     /**
@@ -101,20 +101,20 @@ contract AuraToken is ERC20 {
         uint256 emissionsMinted = totalSupply() - EMISSIONS_MAX_SUPPLY - minterMinted;
         // e.g. reductionPerCliff = 5e25 / 500 = 1e23
         // e.g. cliff = 1e25 / 1e23 = 100
-        uint256 cliff = emissionsMinted.div(reductionPerCliff);
+        uint256 cliff = emissionsMinted / reductionPerCliff;
 
         // e.g. 100 < 500
         if (cliff < totalCliffs) {
             // e.g. (new) reduction = (500 - 100) * 2.5 + 700 = 1700;
             // e.g. (new) reduction = (500 - 250) * 2.5 + 700 = 1325;
             // e.g. (new) reduction = (500 - 400) * 2.5 + 700 = 950;
-            uint256 reduction = totalCliffs.sub(cliff).mul(5).div(2).add(700);
+            uint256 reduction = ((((totalCliffs - cliff) * 5) / 2) + 700);
             // e.g. (new) amount = 1e19 * 1700 / 500 =  34e18;
             // e.g. (new) amount = 1e19 * 1325 / 500 =  26.5e18;
             // e.g. (new) amount = 1e19 * 950 / 500  =  19e17;
-            uint256 amount = _amount.mul(reduction).div(totalCliffs);
+            uint256 amount = ((_amount * reduction) / totalCliffs);
             // e.g. amtTillMax = 5e25 - 1e25 = 4e25
-            uint256 amtTillMax = EMISSIONS_MAX_SUPPLY.sub(emissionsMinted);
+            uint256 amtTillMax = EMISSIONS_MAX_SUPPLY - emissionsMinted;
             if (amount > amtTillMax) {
                 amount = amtTillMax;
             }

File AuraLocker.sol

Extract to variable on loop;

         //check to add
         if (epochs[epochindex - 1].date < currentEpoch) {
             //fill any epoch gaps until the next epoch date.
-            while (epochs[epochs.length - 1].date != currentEpoch) {
-                uint256 nextEpochDate = uint256(epochs[epochs.length - 1].date).add(rewardsDuration);
+            uint256 nextEpochDate;
+            while (uint32(nextEpochDate) != currentEpoch) {
+                nextEpochDate = uint256(epochs[epochs.length - 1].date).add(rewardsDuration);
                 epochs.push(Epoch({ supply: 0, date: uint32(nextEpochDate) }));
             }
         }

Remove unnecesarry libraries and external calls, Aura math do plain math operations, there is no need to use this libraries.

--- a/contracts/AuraLocker.sol
+++ b/contracts/AuraLocker.sol
@@ -22,10 +22,10 @@ interface IRewardStaking {
  * @dev     Invdividual and delegatee vote power lookups both use independent accounting mechanisms.
  */
 contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
-    using AuraMath for uint256;
-    using AuraMath224 for uint224;
-    using AuraMath112 for uint112;
-    using AuraMath32 for uint32;
+    // using AuraMath for uint256;
+    // using AuraMath224 for uint224;
+    // using AuraMath112 for uint112;
+    // using AuraMath32 for uint32;
     using SafeERC20 for IERC20;
 
     /* ==========     STRUCTS     ========== */
@@ -159,7 +159,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         cvxCrv = _cvxCrv;
         cvxcrvStaking = _cvxCrvStaking;
 
-        uint256 currentEpoch = block.timestamp.div(rewardsDuration).mul(rewardsDuration);
+        uint256 currentEpoch = ((block.timestamp / rewardsDuration) / rewardsDuration);
         epochs.push(Epoch({ supply: 0, date: uint32(currentEpoch) }));
     }
 
@@ -174,12 +174,12 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
             for (uint256 i = 0; i < rewardTokensLength; i++) {
                 address token = rewardTokens[i];
                 uint256 newRewardPerToken = _rewardPerToken(token);
-                rewardData[token].rewardPerTokenStored = newRewardPerToken.to96();
-                rewardData[token].lastUpdateTime = _lastTimeRewardApplicable(rewardData[token].periodFinish).to32();
+                rewardData[token].rewardPerTokenStored = AuraMath.to96(newRewardPerToken);
+                rewardData[token].lastUpdateTime = AuraMath.to32(_lastTimeRewardApplicable(rewardData[token].periodFinish));
                 if (_account != address(0)) {
                     userData[_account][token] = UserData({
-                        rewardPerTokenPaid: newRewardPerToken.to128(),
-                        rewards: _earned(_account, token, userBalance.locked).to128()
+                        rewardPerTokenPaid: AuraMath.to128(newRewardPerToken),
+                        rewards: AuraMath.to128(_earned(_account, token, userBalance.locked))
                     });
                 }
             }
@@ -265,21 +265,21 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         _checkpointEpoch();
 
         //add user balances
-        uint112 lockAmount = _amount.to112();
-        bal.locked = bal.locked.add(lockAmount);
+        uint112 lockAmount = AuraMath.to112(_amount);
+        bal.locked = bal.locked + lockAmount;
 
         //add to total supplies
-        lockedSupply = lockedSupply.add(_amount);
+        lockedSupply = lockedSupply + _amount;
 
         //add user lock records or add to current
-        uint256 currentEpoch = block.timestamp.div(rewardsDuration).mul(rewardsDuration);
-        uint256 unlockTime = currentEpoch.add(lockDuration);
+        uint256 currentEpoch = ((block.timestamp / rewardsDuration) / rewardsDuration);
+        uint256 unlockTime = currentEpoch + lockDuration;
         uint256 idx = userLocks[_account].length;
         if (idx == 0 || userLocks[_account][idx - 1].unlockTime < unlockTime) {
             userLocks[_account].push(LockedBalance({ amount: lockAmount, unlockTime: uint32(unlockTime) }));
         } else {
             LockedBalance storage userL = userLocks[_account][idx - 1];
-            userL.amount = userL.amount.add(lockAmount);
+            userL.amount = userL.amount + lockAmount;
         }
 
         address delegatee = delegates(_account);
@@ -290,7 +290,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
 
         //update epoch supply, epoch checkpointed above so safe to add to latest
         Epoch storage e = epochs[epochs.length - 1];
-        e.supply = e.supply.add(lockAmount);
+        e.supply = e.supply + lockAmount;
 
         emit Staked(_account, lockAmount, lockAmount);
     }
@@ -324,7 +324,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
 
     //insert a new epoch if needed. fill in any gaps
     function _checkpointEpoch() internal {
-        uint256 currentEpoch = block.timestamp.div(rewardsDuration).mul(rewardsDuration);
+        uint256 currentEpoch = ((block.timestamp / rewardsDuration) / rewardsDuration);
         uint256 epochindex = epochs.length;
 
         //first epoch add in constructor, no need to check 0 length
@@ -332,7 +332,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         if (epochs[epochindex - 1].date < currentEpoch) {
             //fill any epoch gaps until the next epoch date.
             while (epochs[epochs.length - 1].date != currentEpoch) {
-                uint256 nextEpochDate = uint256(epochs[epochs.length - 1].date).add(rewardsDuration);
+                uint256 nextEpochDate = (uint256(epochs[epochs.length - 1].date) + rewardsDuration);
                 epochs.push(Epoch({ supply: 0, date: uint32(nextEpochDate) }));
             }
         }
@@ -345,7 +345,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
 
     function kickExpiredLocks(address _account) external nonReentrant {
         //allow kick after grace period of 'kickRewardEpochDelay'
-        _processExpiredLocks(_account, false, msg.sender, rewardsDuration.mul(kickRewardEpochDelay));
+        _processExpiredLocks(_account, false, msg.sender, rewardsDuration / kickRewardEpochDelay);
     }
 
     // Withdraw without checkpointing or accruing any rewards, providing system is shutdown
@@ -359,7 +359,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         require(amt > 0, "Nothing locked");
 
         userBalance.locked = 0;
-        userBalance.nextUnlockIndex = locks.length.to32();
+        userBalance.nextUnlockIndex = AuraMath.to32(locks.length);
         lockedSupply -= amt;
 
         emit Withdrawn(msg.sender, amt, false);
@@ -380,8 +380,8 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         uint256 length = locks.length;
         uint256 reward = 0;
         uint256 expiryTime = _checkDelay == 0 && _relock
-            ? block.timestamp.add(rewardsDuration)
-            : block.timestamp.sub(_checkDelay);
+            ? block.timestamp + rewardsDuration
+            : block.timestamp - _checkDelay;
         require(length > 0, "no locks");
         // e.g. now = 16
         // if contract is shutdown OR latest lock unlock time (e.g. 17) <= now - (1)
@@ -391,17 +391,17 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
             locked = userBalance.locked;
 
             //dont delete, just set next index
-            userBalance.nextUnlockIndex = length.to32();
+            userBalance.nextUnlockIndex = AuraMath.to32(length);
 
             //check for kick reward
             //this wont have the exact reward rate that you would get if looped through
             //but this section is supposed to be for quick and easy low gas processing of all locks
             //we'll assume that if the reward was good enough someone would have processed at an earlier epoch
             if (_checkDelay > 0) {
-                uint256 currentEpoch = block.timestamp.sub(_checkDelay).div(rewardsDuration).mul(rewardsDuration);
-                uint256 epochsover = currentEpoch.sub(uint256(locks[length - 1].unlockTime)).div(rewardsDuration);
-                uint256 rRate = AuraMath.min(kickRewardPerEpoch.mul(epochsover + 1), denominator);
-                reward = uint256(locks[length - 1].amount).mul(rRate).div(denominator);
+                uint256 currentEpoch = ((block.timestamp - _checkDelay) / rewardsDuration) / rewardsDuration;
+                uint256 epochsover = (currentEpoch - uint256(locks[length - 1].unlockTime)) / rewardsDuration;
+                uint256 rRate = AuraMath.min(kickRewardPerEpoch / epochsover + 1, denominator);
+                reward = uint256(((locks[length - 1].amount) / rRate) / denominator);
             }
         } else {
             //use a processed index(nextUnlockIndex) to not loop as much
@@ -412,15 +412,15 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
                 if (locks[i].unlockTime > expiryTime) break;
 
                 //add to cumulative amounts
-                locked = locked.add(locks[i].amount);
+                locked = locked + locks[i].amount;
 
                 //check for kick reward
                 //each epoch over due increases reward
                 if (_checkDelay > 0) {
-                    uint256 currentEpoch = block.timestamp.sub(_checkDelay).div(rewardsDuration).mul(rewardsDuration);
-                    uint256 epochsover = currentEpoch.sub(uint256(locks[i].unlockTime)).div(rewardsDuration);
-                    uint256 rRate = AuraMath.min(kickRewardPerEpoch.mul(epochsover + 1), denominator);
-                    reward = reward.add(uint256(locks[i].amount).mul(rRate).div(denominator));
+                    uint256 currentEpoch = (((block.timestamp - _checkDelay) / rewardsDuration) / rewardsDuration);
+                    uint256 epochsover = (currentEpoch - uint256(locks[i].unlockTime)) / rewardsDuration;
+                    uint256 rRate = AuraMath.min(kickRewardPerEpoch / epochsover + 1, denominator);
+                    reward = reward + uint256(((locks[i].amount) / rRate) / denominator);
                 }
                 //set next unlock index
                 nextUnlockIndex++;
@@ -431,8 +431,8 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         require(locked > 0, "no exp locks");
 
         //update user balances and total supplies
-        userBalance.locked = userBalance.locked.sub(locked);
-        lockedSupply = lockedSupply.sub(locked);
+        userBalance.locked = userBalance.locked - locked;
+        lockedSupply = lockedSupply - locked;
 
         //checkpoint the delegatee
         _checkpointDelegate(delegates(_account), 0, 0);
@@ -442,7 +442,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         //send process incentive
         if (reward > 0) {
             //reduce return amount by the kick reward
-            locked = locked.sub(reward.to112());
+            locked = locked - AuraMath.to112(reward);
 
             //transfer reward
             stakingToken.safeTransfer(_rewardAddress, reward);
@@ -480,7 +480,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
 
         // Step 3: Move balances around
         //         Delegate for the upcoming epoch
-        uint256 upcomingEpoch = block.timestamp.add(rewardsDuration).div(rewardsDuration).mul(rewardsDuration);
+        uint256 upcomingEpoch = (((block.timestamp + rewardsDuration) / rewardsDuration) / rewardsDuration);
         uint256 i = len - 1;
         uint256 futureUnlocksSum = 0;
         LockedBalance memory currentLock = locks[i];
@@ -515,15 +515,15 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
     ) internal {
         // This would only skip on first checkpointing
         if (_account != address(0)) {
-            uint256 upcomingEpoch = block.timestamp.add(rewardsDuration).div(rewardsDuration).mul(rewardsDuration);
+            uint256 upcomingEpoch = (((block.timestamp + rewardsDuration) / rewardsDuration) / rewardsDuration);
             DelegateeCheckpoint[] storage ckpts = _checkpointedVotes[_account];
             if (ckpts.length > 0) {
                 DelegateeCheckpoint memory prevCkpt = ckpts[ckpts.length - 1];
                 // If there has already been a record for the upcoming epoch, no need to deduct the unlocks
                 if (prevCkpt.epochStart == upcomingEpoch) {
                     ckpts[ckpts.length - 1] = DelegateeCheckpoint({
-                        votes: (prevCkpt.votes + _upcomingAddition - _upcomingDeduction).to224(),
-                        epochStart: upcomingEpoch.to32()
+                        votes: AuraMath.to224(prevCkpt.votes + _upcomingAddition - _upcomingDeduction),
+                        epochStart: AuraMath.to32(upcomingEpoch)
                     });
                 }
                 // else if it has been over 16 weeks since the previous checkpoint, all locks have since expired
@@ -531,8 +531,8 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
                 else if (prevCkpt.epochStart + lockDuration <= upcomingEpoch) {
                     ckpts.push(
                         DelegateeCheckpoint({
-                            votes: (_upcomingAddition - _upcomingDeduction).to224(),
-                            epochStart: upcomingEpoch.to32()
+                            votes: AuraMath.to224(_upcomingAddition - _upcomingDeduction),
+                            epochStart: AuraMath.to32(upcomingEpoch)
                         })
                     );
                 } else {
@@ -545,17 +545,16 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
                     }
                     ckpts.push(
                         DelegateeCheckpoint({
-                            votes: (prevCkpt.votes - unlocksSinceLatestCkpt + _upcomingAddition - _upcomingDeduction)
-                                .to224(),
-                            epochStart: upcomingEpoch.to32()
+                            votes: AuraMath.to224(prevCkpt.votes - unlocksSinceLatestCkpt + _upcomingAddition - _upcomingDeduction),
+                            epochStart: AuraMath.to32(upcomingEpoch)
                         })
                     );
                 }
             } else {
                 ckpts.push(
                     DelegateeCheckpoint({
-                        votes: (_upcomingAddition - _upcomingDeduction).to224(),
-                        epochStart: upcomingEpoch.to32()
+                        votes: AuraMath.to224(_upcomingAddition - _upcomingDeduction),
+                        epochStart: AuraMath.to32(upcomingEpoch)
                     })
                 );
             }
@@ -588,7 +587,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
      * @dev Get number of checkpoints for `account`.
      */
     function numCheckpoints(address account) external view virtual returns (uint32) {
-        return _checkpointedVotes[account].length.to32();
+        return AuraMath.to32(_checkpointedVotes[account].length);
     }
 
     /**
@@ -596,7 +595,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
      */
     function getPastVotes(address account, uint256 timestamp) public view returns (uint256 votes) {
         require(timestamp <= block.timestamp, "ERC20Votes: block not yet mined");
-        uint256 epoch = timestamp.div(rewardsDuration).mul(rewardsDuration);
+        uint256 epoch = (timestamp / rewardsDuration) / rewardsDuration;
         DelegateeCheckpoint memory ckpt = _checkpointsLookup(_checkpointedVotes[account], epoch);
         votes = ckpt.votes;
         if (votes == 0 || ckpt.epochStart + lockDuration <= epoch) {
@@ -651,10 +650,10 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
 
     // Balance of an account which only includes properly locked tokens at the given epoch
     function balanceAtEpochOf(uint256 _epoch, address _user) public view returns (uint256 amount) {
-        uint256 epochStart = uint256(epochs[0].date).add(uint256(_epoch).mul(rewardsDuration));
+        uint256 epochStart = uint256(epochs[0].date) + uint256(_epoch) / rewardsDuration;
         require(epochStart < block.timestamp, "Epoch is in the future");
 
-        uint256 cutoffEpoch = epochStart.sub(lockDuration);
+        uint256 cutoffEpoch = epochStart - lockDuration;
 
         LockedBalance[] storage locks = userLocks[_user];
 
@@ -662,12 +661,12 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         //traverse inversely to make more current queries more gas efficient
         uint256 locksLength = locks.length;
         for (uint256 i = locksLength; i > 0; i--) {
-            uint256 lockEpoch = uint256(locks[i - 1].unlockTime).sub(lockDuration);
+            uint256 lockEpoch = uint256(locks[i - 1].unlockTime) - lockDuration;
             //lock epoch must be less or equal to the epoch we're basing from.
             //also not include the current epoch
             if (lockEpoch < epochStart) {
                 if (lockEpoch > cutoffEpoch) {
-                    amount = amount.add(locks[i - 1].amount);
+                    amount = amount + locks[i - 1].amount;
                 } else {
                     //stop now as no futher checks matter
                     break;
@@ -700,9 +699,9 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
                 }
                 lockData[idx] = locks[i];
                 idx++;
-                locked = locked.add(locks[i].amount);
+                locked = locked + locks[i].amount;
             } else {
-                unlockable = unlockable.add(locks[i].amount);
+                unlockable = unlockable + locks[i].amount;
             }
         }
         return (userBalance.locked, unlockable, locked, lockData);
@@ -715,10 +714,10 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
 
     // Supply of all properly locked balances at the given epoch
     function totalSupplyAtEpoch(uint256 _epoch) public view returns (uint256 supply) {
-        uint256 epochStart = uint256(epochs[0].date).add(uint256(_epoch).mul(rewardsDuration));
+        uint256 epochStart = uint256(epochs[0].date) + uint256(_epoch) / rewardsDuration;
         require(epochStart < block.timestamp, "Epoch is in the future");
 
-        uint256 cutoffEpoch = epochStart.sub(lockDuration);
+        uint256 cutoffEpoch = epochStart - lockDuration;
         uint256 lastIndex = epochs.length - 1;
 
         uint256 epochIndex = _epoch > lastIndex ? lastIndex : _epoch;
@@ -737,7 +736,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
 
     // Get an epoch index based on timestamp
     function findEpochId(uint256 _time) public view returns (uint256 epoch) {
-        return _time.sub(epochs[0].date).div(rewardsDuration);
+        return (_time - epochs[0].date) / rewardsDuration;
     }
 
     /***************************************
@@ -792,7 +791,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         uint256 _balance
     ) internal view returns (uint256) {
         UserData memory data = userData[_user][_rewardsToken];
-        return _balance.mul(_rewardPerToken(_rewardsToken).sub(data.rewardPerTokenPaid)).div(1e18).add(data.rewards);
+        return (((_balance / _rewardPerToken(_rewardsToken) - data.rewardPerTokenPaid)) / 1e18) + data.rewards;
     }
 
     function _lastTimeRewardApplicable(uint256 _finishTime) internal view returns (uint256) {
@@ -804,12 +803,12 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
             return rewardData[_rewardsToken].rewardPerTokenStored;
         }
         return
-            uint256(rewardData[_rewardsToken].rewardPerTokenStored).add(
+            uint256((((((rewardData[_rewardsToken].rewardPerTokenStored) + 
                 _lastTimeRewardApplicable(rewardData[_rewardsToken].periodFinish)
-                    .sub(rewardData[_rewardsToken].lastUpdateTime)
-                    .mul(rewardData[_rewardsToken].rewardRate)
-                    .mul(1e18)
-                    .div(lockedSupply)
+                     - rewardData[_rewardsToken].lastUpdateTime)
+                     / rewardData[_rewardsToken].rewardRate)
+                     / 1e18)
+                     / lockedSupply)
             );
     }
 
@@ -825,7 +824,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
 
         IERC20(cvxCrv).safeTransferFrom(msg.sender, address(this), _rewards);
 
-        _rewards = _rewards.add(queuedCvxCrvRewards);
+        _rewards = _rewards + queuedCvxCrvRewards;
         if (block.timestamp >= rdata.periodFinish) {
             _notifyReward(cvxCrv, _rewards);
             queuedCvxCrvRewards = 0;
@@ -833,10 +832,10 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         }
 
         //et = now - (finish-duration)
-        uint256 elapsedTime = block.timestamp.sub(rdata.periodFinish.sub(rewardsDuration.to32()));
+        uint256 elapsedTime = block.timestamp - rdata.periodFinish - AuraMath.to32(rewardsDuration);
         //current at now: rewardRate * elapsedTime
         uint256 currentAtNow = rdata.rewardRate * elapsedTime;
-        uint256 queuedRatio = currentAtNow.mul(1000).div(_rewards);
+        uint256 queuedRatio = ((currentAtNow / 1000) / _rewards);
         if (queuedRatio < newRewardRatio) {
             _notifyReward(cvxCrv, _rewards);
             queuedCvxCrvRewards = 0;
@@ -861,15 +860,15 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         RewardData storage rdata = rewardData[_rewardsToken];
 
         if (block.timestamp >= rdata.periodFinish) {
-            rdata.rewardRate = _reward.div(rewardsDuration).to96();
+            rdata.rewardRate = AuraMath.to96(_reward / rewardsDuration);
         } else {
-            uint256 remaining = uint256(rdata.periodFinish).sub(block.timestamp);
-            uint256 leftover = remaining.mul(rdata.rewardRate);
-            rdata.rewardRate = _reward.add(leftover).div(rewardsDuration).to96();
+            uint256 remaining = uint256(rdata.periodFinish) - block.timestamp;
+            uint256 leftover = (remaining / rdata.rewardRate);
+            rdata.rewardRate = AuraMath.to96((_reward + leftover) / rewardsDuration);
         }
 
-        rdata.lastUpdateTime = block.timestamp.to32();
-        rdata.periodFinish = block.timestamp.add(rewardsDuration).to32();
+        rdata.lastUpdateTime = AuraMath.to32(block.timestamp);
+        rdata.periodFinish = AuraMath.to32(block.timestamp + rewardsDuration);
 
         emit RewardAdded(_rewardsToken, _reward);
     }

Use unchecked to increment or decrement the iterator variable. You could safely save gas.

--- a/contracts/AuraLocker.sol
+++ b/contracts/AuraLocker.sol
@@ -171,7 +171,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         {
             Balances storage userBalance = balances[_account];
             uint256 rewardTokensLength = rewardTokens.length;
-            for (uint256 i = 0; i < rewardTokensLength; i++) {
+            for (uint256 i = 0; i < rewardTokensLength;) {
                 address token = rewardTokens[i];
                 uint256 newRewardPerToken = _rewardPerToken(token);
                 rewardData[token].rewardPerTokenStored = newRewardPerToken.to96();
@@ -182,6 +182,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
                         rewards: _earned(_account, token, userBalance.locked).to128()
                     });
                 }
+                unchecked{ i++; }
             }
         }
         _;
@@ -303,7 +304,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
     // Claim all pending rewards
     function getReward(address _account, bool _stake) public nonReentrant updateReward(_account) {
         uint256 rewardTokensLength = rewardTokens.length;
-        for (uint256 i; i < rewardTokensLength; i++) {
+        for (uint256 i; i < rewardTokensLength;) {
             address _rewardsToken = rewardTokens[i];
             uint256 reward = userData[_account][_rewardsToken].rewards;
             if (reward > 0) {
@@ -315,6 +316,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
                 }
                 emit RewardPaid(_account, _rewardsToken, reward);
             }
+            unchecked{ i++; }
         }
     }
 
@@ -407,7 +409,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
             //use a processed index(nextUnlockIndex) to not loop as much
             //deleting does not change array length
             uint32 nextUnlockIndex = userBalance.nextUnlockIndex;
-            for (uint256 i = nextUnlockIndex; i < length; i++) {
+            for (uint256 i = nextUnlockIndex; i < length;) {
                 //unlock time must be less or equal to time
                 if (locks[i].unlockTime > expiryTime) break;
 
@@ -424,6 +426,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
                 }
                 //set next unlock index
                 nextUnlockIndex++;
+                unchecked{ i++; }
             }
             //update next unlock index
             userBalance.nextUnlockIndex = nextUnlockIndex;
@@ -661,7 +664,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         //need to add up since the range could be in the middle somewhere
         //traverse inversely to make more current queries more gas efficient
         uint256 locksLength = locks.length;
-        for (uint256 i = locksLength; i > 0; i--) {
+        for (uint256 i = locksLength; i > 0;) {
             uint256 lockEpoch = uint256(locks[i - 1].unlockTime).sub(lockDuration);
             //lock epoch must be less or equal to the epoch we're basing from.
             //also not include the current epoch
@@ -673,6 +676,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
                     break;
                 }
             }
+            unchecked{ i--; }
         }
 
         return amount;
@@ -693,17 +697,18 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         Balances storage userBalance = balances[_user];
         uint256 nextUnlockIndex = userBalance.nextUnlockIndex;
         uint256 idx;
-        for (uint256 i = nextUnlockIndex; i < locks.length; i++) {
+        for (uint256 i = nextUnlockIndex; i < locks.length;) {
             if (locks[i].unlockTime > block.timestamp) {
                 if (idx == 0) {
                     lockData = new LockedBalance[](locks.length - i);
                 }
                 lockData[idx] = locks[i];
-                idx++;
+                unchecked{ idx++; }
                 locked = locked.add(locks[i].amount);
             } else {
                 unlockable = unlockable.add(locks[i].amount);
             }
+            unchecked{ i++; }
         }
         return (userBalance.locked, unlockable, locked, lockData);
     }
@@ -723,7 +728,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
 
         uint256 epochIndex = _epoch > lastIndex ? lastIndex : _epoch;
 
-        for (uint256 i = epochIndex + 1; i > 0; i--) {
+        for (uint256 i = epochIndex + 1; i > 0;) {
             Epoch memory e = epochs[i - 1];
             if (e.date == epochStart) {
                 continue;
@@ -732,6 +737,8 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
             } else {
                 supply += e.supply;
             }
+
+            unchecked{ i--; }
         }
     }
 
@@ -770,10 +777,11 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
         userRewards = new EarnedData[](rewardTokens.length);
         Balances storage userBalance = balances[_account];
         uint256 userRewardsLength = userRewards.length;
-        for (uint256 i = 0; i < userRewardsLength; i++) {
+        for (uint256 i = 0; i < userRewardsLength; ) {
             address token = rewardTokens[i];
             userRewards[i].token = token;
             userRewards[i].amount = _earned(_account, token, userBalance.locked);
+            unchecked{ i++; }
         }
         return userRewards;
     }

Save some gas in loops using storage

--- a/contracts/AuraLocker.sol
+++ b/contracts/AuraLocker.sol
@@ -325,16 +325,18 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
     //insert a new epoch if needed. fill in any gaps
     function _checkpointEpoch() internal {
         uint256 currentEpoch = block.timestamp.div(rewardsDuration).mul(rewardsDuration);
-        uint256 epochindex = epochs.length;
+        Epoch[] memory _epochs = epochs;
+        uint256 epochindex = _epochs.length;
 
         //first epoch add in constructor, no need to check 0 length
         //check to add
-        if (epochs[epochindex - 1].date < currentEpoch) {
+        if (_epochs[epochindex - 1].date < currentEpoch) {
             //fill any epoch gaps until the next epoch date.
-            while (epochs[epochs.length - 1].date != currentEpoch) {
-                uint256 nextEpochDate = uint256(epochs[epochs.length - 1].date).add(rewardsDuration);
-                epochs.push(Epoch({ supply: 0, date: uint32(nextEpochDate) }));
+            while (_epochs[_epochs.length - 1].date != currentEpoch) {
+                uint256 nextEpochDate = uint256(_epochs[_epochs.length - 1].date).add(rewardsDuration);
+                _epochs.push(Epoch({ supply: 0, date: uint32(nextEpochDate) }));
             }
+            epochs = _epochs;
         }
     }
 
@@ -494,7 +496,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
             delegateeUnlocks[newDelegatee][currentLock.unlockTime] += currentLock.amount;
 
             if (i > 0) {
-                i--;
+                unchecked { i--; }
                 currentLock = locks[i];
             } else {
                 break;
@@ -633,7 +635,7 @@ contract AuraLocker is ReentrancyGuard, Ownable, IAuraLocker {
             if (ckpts[mid].epochStart > epochStart) {
                 high = mid;
             } else {
-                low = mid + 1;
+                unchecked { low = mid + 1; }
             }
         }
 

File AuraBalRewardPool.sol

Remove SafeMath;
NOTE: SafeMath is no longer needed starting with Solidity 0.8. The compiler now has built in overflow checking.

--- a/contracts/AuraBalRewardPool.sol
+++ b/contracts/AuraBalRewardPool.sol
@@ -21,7 +21,7 @@ import { IAuraLocker } from "./Interfaces.sol";
  *            - Penalty on claim at 20%
  */
 contract AuraBalRewardPool {
-    using SafeMath for uint256;
+    // using SafeMath for uint256;
     using SafeERC20 for IERC20;
 
     IERC20 public immutable rewardToken;

File AuraVestedEscrow.sol

Uso unchecked for increment i var in for loop

--- a/contracts/AuraVestedEscrow.sol
+++ b/contracts/AuraVestedEscrow.sol
@@ -97,13 +97,15 @@ contract AuraVestedEscrow is ReentrancyGuard {
         require(!initialised, "initialised already");
 
         uint256 totalAmount = 0;
-        for (uint256 i = 0; i < _recipient.length; i++) {
+        for (uint256 i = 0; i < _recipient.length;) {
             uint256 amount = _amount[i];
 
             totalLocked[_recipient[i]] += amount;
             totalAmount += amount;
 
             emit Funded(_recipient[i], amount);
+
+            unchecked{ i++; }
         }
         rewardToken.safeTransferFrom(msg.sender, address(this), totalAmount);
         initialised = true;
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels May 12, 2022
code423n4 added a commit that referenced this issue May 12, 2022
@phijfry
Copy link
Collaborator

phijfry commented May 17, 2022

Agree but we are prioritising having a small diff from the original convex contracts.

@0xMaharishi 0xMaharishi added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label May 25, 2022
@0xMaharishi 0xMaharishi added the resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) label May 30, 2022
@0xMaharishi
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

3 participants