You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
SafeMath is not needed when using Solidity version 0.8+
Solidity version 0.8+ already implements overflow and underflow checks by default.
Using the SafeMath library from OpenZeppelin (which is more gas expensive than the 0.8+ overflow checks) is therefore redundant.
I suggest using the built-in checks instead of SafeMath and remove SafeMath here:
contracts/BaseRewardPool.sol:
2: pragma solidity0.8.7;
42: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
52: using SafeMathforuint256;
contracts/Booster.sol:
2: pragma solidity0.8.7;
4: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
21: using SafeMathforuint256;
contracts/DepositToken.sol:
2: pragma solidity0.8.7;
4: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
13: using SafeMathforuint256;
contracts/ExtraRewardStashV1.sol:
2: pragma solidity0.8.7;
4: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
18: using SafeMathforuint256;
contracts/ExtraRewardStashV2.sol:
2: pragma solidity0.8.7;
4: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
18: using SafeMathforuint256;
contracts/ExtraRewardStashV3.sol:
2: pragma solidity0.8.7;
4: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
18: using SafeMathforuint256;
contracts/PoolManager.sol:
2: pragma solidity0.8.7;
4: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
17: using SafeMathforuint256;
contracts/RewardFactory.sol:
2: pragma solidity0.8.7;
4: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
contracts/StashFactory.sol:
2: pragma solidity0.8.7;
4: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
contracts/TokenFactory.sol:
2: pragma solidity0.8.7;
4: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
contracts/VE3DRewardPool.sol:
2: pragma solidity0.8.7;
42: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
55: using SafeMathforuint256;
contracts/VeAssetDepositor.sol:
2: pragma solidity0.8.7;
4: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
16: using SafeMathforuint256;
contracts/VeTokenMinter.sol:
2: pragma solidity0.8.7;
8: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
12: using SafeMathforuint256;
contracts/VirtualBalanceRewardPool.sol:
2: pragma solidity0.8.7;
42: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
51: using SafeMathforuint256;
66: using SafeMathforuint256;
contracts/VoterProxy.sol:
2: pragma solidity0.8.7;
4: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
19: using SafeMathforuint256;
contracts/token/VE3Token.sol:
2: pragma solidity0.8.7;
4: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
13: using SafeMathforuint256;
contracts/token/VeToken.sol:
2: pragma solidity0.8.7;
5: import"@openzeppelin/contracts/utils/math/SafeMath.sol";
8: using SafeMathforuint256;
Boolean comparisons
Comparing to a constant (true or false) is a bit more expensive than directly checking the returned boolean value.
I suggest using if(directValue) instead of if(directValue == true) and if(!directValue) instead of if(directValue == false) here:
Booster.sol:352: require(pool.shutdown ==false, "pool is closed");
Booster.sol:498: require(pool.shutdown ==false, "pool is closed");
RewardFactory.sol:40: require(rewardAccess[msg.sender] ==true, "!auth");
RewardFactory.sol:57: require(rewardAccess[msg.sender] ==true, "!auth");
RewardFactory.sol:103: require(operators.contains(_msgSender()) || rewardAccess[_msgSender()] ==true, "!auth");
VoterProxy.sol:70: operator ==address(0) ||IDeposit(operator).isShutdown() ==true,
VoterProxy.sol:93: if (protectedTokens[_token] ==false) {
VoterProxy.sol:96: if (protectedTokens[_gauge] ==false) {
VoterProxy.sol:110: require(stashPool[msg.sender] ==true, "!auth");
VoterProxy.sol:113: if (protectedTokens[address(_asset)] ==true) {
> 0 is less efficient than != 0 for unsigned integers (with proof)
!= 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas)
Proof: While it may seem that > 0 is cheaper than !=, this is only true without the optimizer enabled and outside a require statement. If you enable the optimizer at 10k AND you're in a require statement, this will save gas. You can see this tweet for more proofs: https://twitter.com/gzeon/status/1485428085885640706
Strict inequalities (>) are more expensive than non-strict ones (>=). This is due to some supplementary checks (ISZERO, 3 gas). This also holds true between <= and <.
Consider replacing strict inequalities with non-strict ones to save some gas here:
helper/Babylonian.sol:51: return (r < r1 ? r : r1);
helper/FixedPoint.sol:58: uint256 z = FullMath.mulDiv(self._x, uint256(y <0?-y : y), Q112);
helper/FixedPoint.sol:60: return y <0?-int256(z) : int256(z);
helper/MathUtil.sol:12: return a < b ? a : b;
Splitting require() statements that use && saves gas
If you're using the Optimizer at 200, instead of using the && operator in a single require statement to check multiple conditions, I suggest using multiple require statements with 1 condition per require statement:
Please, note that this might not hold true at a higher number of runs for the Optimizer (10k). However, it indeed is true at 200.
Use shift right/left instead of division/multiplication if possible
While the DIV / MUL opcode uses 5 gas, the SHR / SHL opcode only uses 3 gas. Furthermore, beware that Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting. Eventually, overflow checks are never performed for shift operations as they are done for arithmetic operations. Instead, the result is always truncated.
An array's length should be cached to save gas in for-loops
Reading array length at each iteration of the loop consumes more gas than necessary.
In the best case scenario (length read on a memory variable), caching the array length in the stack saves around 3 gas per iteration.
In the worst case scenario (external calls at each iteration, like at VE3DRewardPool.sol:148 or VE3DRewardPool.sol:281), the amount of gas wasted can be massive.
Here, I suggest storing the array's length in a variable before the for-loop, and use this new variable instead:
BaseRewardPool.sol:176: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:199: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:218: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:245: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:282: for (uint256 i =0; i < extraRewards.length; i++) {
Booster.sol:329: for (uint256 i =0; i < poolInfo.length; i++) {
VE3DLocker.sol:207: for (uint256 i; i < rewardTokens.length; i++) {
VE3DLocker.sol:286: for (uint256 i =0; i < userRewards.length; i++) {
VE3DLocker.sol:315: for (uint256 i = locks.length-1; i +1!=0; i--) {
VE3DLocker.sol:360: for (uint256 i = locks.length-1; i +1!=0; i--) {
VE3DLocker.sol:457: for (uint256 i = nextUnlockIndex; i < locks.length; i++) {
VE3DLocker.sol:720: for (uint256 i; i < rewardTokens.length; i++) {
VE3DLocker.sol:803: for (uint256 i =0; i < rewardTokens.length; i++) {
VE3DRewardPool.sol:148: for (uint256 i =0; i < rewardTokens.length(); i++) {
VE3DRewardPool.sol:281: for (uint256 i =0; i < rewardTokens.length(); i++) {
VoterProxy.sol:217: for (uint256 i =0; i < _tokenVote.length; i++) {
++i costs less gas compared to i++ or i += 1 (same for --i vs i-- or i -= 1)
Pre-increments and pre-decrements are cheaper.
For a uint256 i variable, the following is true with the Optimizer enabled at 10k:
Increment:
i += 1 is the most expensive form
i++ costs 6 gas less than i += 1
++i costs 5 gas less than i++ (11 gas less than i += 1)
Decrement:
i -= 1 is the most expensive form
i-- costs 11 gas less than i -= 1
--i costs 5 gas less than i-- (16 gas less than i -= 1)
Note that post-increments (or post-decrements) return the old value before incrementing or decrementing, hence the name post-increment:
uint i =1;
uint j =2;
require(j == i++, "This will be false as i is incremented after the comparison");
However, pre-increments (or pre-decrements) return the new value:
uint i =1;
uint j =2;
require(j ==++i, "This will be true as i is incremented before the comparison");
In the pre-increment case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2.
Affected code:
helper/BitMath.sol:38: if (x >=0x2) r +=1;
helper/BitMath.sol:83: if (x &0x1>0) r -=1;
helper/FullMath.sol:11: if (mm < l) h -=1;
helper/FullMath.sol:44: if (mm > l) h -=1;
BaseRewardPool.sol:176: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:199: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:218: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:245: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:282: for (uint256 i =0; i < extraRewards.length; i++) {
Booster.sol:329: for (uint256 i =0; i < poolInfo.length; i++) {
ExtraRewardStashV2.sol:71: for (uint256 i =0; i < length; i++) {
ExtraRewardStashV2.sol:78: for (uint256 i =0; i < length; i++) {
ExtraRewardStashV2.sol:137: for (uint256 i =0; i < maxRewards; i++) {
ExtraRewardStashV2.sol:140: for (uint256 x = i; x < tokenCount; x++) {
ExtraRewardStashV2.sol:181: for (uint256 i =0; i < tokenCount; i++) {
ExtraRewardStashV2.sol:213: for (uint256 i =0; i < tokenCount; i++) {
ExtraRewardStashV3.sol:84: for (uint256 i =0; i < maxRewards; i++) {
ExtraRewardStashV3.sol:126: for (uint256 i =0; i < tokenCount; i++) {
RewardFactory.sol:49: for (uint256 i =0; i < length; i++) {
RewardFactory.sol:66: for (uint256 i =0; i < length; i++) {
VE3DLocker.sol:207: for (uint256 i; i < rewardTokens.length; i++) {
VE3DLocker.sol:286: for (uint256 i =0; i < userRewards.length; i++) {
VE3DLocker.sol:315: for (uint256 i = locks.length-1; i +1!=0; i--) {
VE3DLocker.sol:360: for (uint256 i = locks.length-1; i +1!=0; i--) {
VE3DLocker.sol:383: epochindex--;
VE3DLocker.sol:387: for (uint256 i = epochindex -1; i +1!=0; i--) {
VE3DLocker.sol:406: for (uint256 i = _epoch; i +1!=0; i--) {
VE3DLocker.sol:425: for (uint256 i =0; i <128; i++) {
VE3DLocker.sol:457: for (uint256 i = nextUnlockIndex; i < locks.length; i++) {
VE3DLocker.sol:463: idx++;
VE3DLocker.sol:555: idx--;
VE3DLocker.sol:593: eIndex--;
VE3DLocker.sol:640: for (uint256 i = nextUnlockIndex; i < length; i++) {
VE3DLocker.sol:665: nextUnlockIndex++;
VE3DLocker.sol:720: for (uint256 i; i < rewardTokens.length; i++) {
VE3DLocker.sol:803: for (uint256 i =0; i < rewardTokens.length; i++) {
VE3DRewardPool.sol:148: for (uint256 i =0; i < rewardTokens.length(); i++) {
VE3DRewardPool.sol:214: for (uint256 i =0; i < length; i++) {
VE3DRewardPool.sol:238: for (uint256 i =0; i < length; i++) {
VE3DRewardPool.sol:257: for (uint256 i =0; i < length; i++) {
VE3DRewardPool.sol:281: for (uint256 i =0; i < rewardTokens.length(); i++) {
VE3DRewardPool.sol:326: for (uint256 i =0; i < length; i++) {
VoterProxy.sol:217: for (uint256 i =0; i < _tokenVote.length; i++) {
Consider using pre-increments and pre-decrements where they are relevant (meaning: not where post-increments/decrements logic are relevant).
Increments/decrements can be unchecked in for-loops
In Solidity 0.8+, there's a default overflow check on unsigned integers. It's possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.
BaseRewardPool.sol:176: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:199: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:218: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:245: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:282: for (uint256 i =0; i < extraRewards.length; i++) {
Booster.sol:329: for (uint256 i =0; i < poolInfo.length; i++) {
ExtraRewardStashV2.sol:71: for (uint256 i =0; i < length; i++) {
ExtraRewardStashV2.sol:78: for (uint256 i =0; i < length; i++) {
ExtraRewardStashV2.sol:137: for (uint256 i =0; i < maxRewards; i++) {
ExtraRewardStashV2.sol:140: for (uint256 x = i; x < tokenCount; x++) {
ExtraRewardStashV2.sol:181: for (uint256 i =0; i < tokenCount; i++) {
ExtraRewardStashV2.sol:213: for (uint256 i =0; i < tokenCount; i++) {
ExtraRewardStashV3.sol:84: for (uint256 i =0; i < maxRewards; i++) {
ExtraRewardStashV3.sol:126: for (uint256 i =0; i < tokenCount; i++) {
RewardFactory.sol:49: for (uint256 i =0; i < length; i++) {
RewardFactory.sol:66: for (uint256 i =0; i < length; i++) {
VE3DLocker.sol:207: for (uint256 i; i < rewardTokens.length; i++) {
VE3DLocker.sol:286: for (uint256 i =0; i < userRewards.length; i++) {
VE3DLocker.sol:425: for (uint256 i =0; i <128; i++) {
VE3DLocker.sol:457: for (uint256 i = nextUnlockIndex; i < locks.length; i++) {
VE3DLocker.sol:640: for (uint256 i = nextUnlockIndex; i < length; i++) {
VE3DLocker.sol:720: for (uint256 i; i < rewardTokens.length; i++) {
VE3DLocker.sol:803: for (uint256 i =0; i < rewardTokens.length; i++) {
VE3DRewardPool.sol:148: for (uint256 i =0; i < rewardTokens.length(); i++) {
VE3DRewardPool.sol:214: for (uint256 i =0; i < length; i++) {
VE3DRewardPool.sol:238: for (uint256 i =0; i < length; i++) {
VE3DRewardPool.sol:257: for (uint256 i =0; i < length; i++) {
VE3DRewardPool.sol:281: for (uint256 i =0; i < rewardTokens.length(); i++) {
VE3DRewardPool.sol:326: for (uint256 i =0; i < length; i++) {
VoterProxy.sol:217: for (uint256 i =0; i < _tokenVote.length; i++) {
The change would be:
- for (uint256 i; i < numIterations; i++) {+ for (uint256 i; i < numIterations;) {
// ...
+ unchecked { ++i; }
}
The same can be applied with decrements (which should use break when i == 0).
The risk of overflow is non-existant for uint256 here.
abi.encode() is less efficient than abi.encodePacked()
Changing abi.encode function to abi.encodePacked can save gas since the abi.encode function pads extra null bytes at the end of the call data, which is unnecessary. Also, in general, abi.encodePacked is more gas-efficient (see Solidity-Encode-Gas-Comparison).
Consider using abi.encodePacked() here:
StashFactory.sol:92: bytesmemory data =abi.encode(rewarded_token);
Public functions to external
An external call cost is less expensive than one of a public function.
The following functions could be set external to save gas and improve code quality (extracted from Slither).
No need to explicitly initialize variables with default values
If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas.
As an example: for (uint256 i = 0; i < numIterations; ++i) { should be replaced with for (uint256 i; i < numIterations; ++i) {
Affected code:
helper/FixedPoint.sol:50: uint256 z =0;
BaseRewardPool.sol:66: uint256public periodFinish =0;
BaseRewardPool.sol:67: uint256public rewardRate =0;
BaseRewardPool.sol:70: uint256public queuedRewards =0;
BaseRewardPool.sol:71: uint256public currentRewards =0;
BaseRewardPool.sol:72: uint256public historicalRewards =0;
BaseRewardPool.sol:176: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:199: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:218: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:245: for (uint256 i =0; i < extraRewards.length; i++) {
BaseRewardPool.sol:282: for (uint256 i =0; i < extraRewards.length; i++) {
Booster.sol:329: for (uint256 i =0; i < poolInfo.length; i++) {
ExtraRewardStashV1.sol:29: uint256public historicalRewards =0;
ExtraRewardStashV2.sol:71: for (uint256 i =0; i < length; i++) {
ExtraRewardStashV2.sol:78: for (uint256 i =0; i < length; i++) {
ExtraRewardStashV2.sol:137: for (uint256 i =0; i < maxRewards; i++) {
ExtraRewardStashV2.sol:181: for (uint256 i =0; i < tokenCount; i++) {
ExtraRewardStashV2.sol:213: for (uint256 i =0; i < tokenCount; i++) {
ExtraRewardStashV3.sol:84: for (uint256 i =0; i < maxRewards; i++) {
ExtraRewardStashV3.sol:126: for (uint256 i =0; i < tokenCount; i++) {
RewardFactory.sol:49: for (uint256 i =0; i < length; i++) {
RewardFactory.sol:66: for (uint256 i =0; i < length; i++) {
VE3DLocker.sol:106: boolpublic isShutdown =false;
VE3DLocker.sol:286: for (uint256 i =0; i < userRewards.length; i++) {
VE3DLocker.sol:420: uint256 min =0;
VE3DLocker.sol:425: for (uint256 i =0; i <128; i++) {
VE3DLocker.sol:613: uint256 reward =0;
VE3DLocker.sol:803: for (uint256 i =0; i < rewardTokens.length; i++) {
VE3DRewardPool.sol:148: for (uint256 i =0; i < rewardTokens.length(); i++) {
VE3DRewardPool.sol:214: for (uint256 i =0; i < length; i++) {
VE3DRewardPool.sol:238: for (uint256 i =0; i < length; i++) {
VE3DRewardPool.sol:257: for (uint256 i =0; i < length; i++) {
VE3DRewardPool.sol:281: for (uint256 i =0; i < rewardTokens.length(); i++) {
VE3DRewardPool.sol:326: for (uint256 i =0; i < length; i++) {
VeAssetDepositor.sol:28: uint256public incentiveVeAsset =0;
VirtualBalanceRewardPool.sol:74: uint256public periodFinish =0;
VirtualBalanceRewardPool.sol:75: uint256public rewardRate =0;
VirtualBalanceRewardPool.sol:78: uint256public queuedRewards =0;
VirtualBalanceRewardPool.sol:79: uint256public currentRewards =0;
VirtualBalanceRewardPool.sol:80: uint256public historicalRewards =0;
VoterProxy.sol:217: for (uint256 i =0; i < _tokenVote.length; i++) {
VoterProxy.sol:227: uint256 _balance =0;
I suggest removing explicit initializations for default values.
Reduce the size of error messages (Long revert Strings)
Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.
Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.
Revert strings > 32 bytes:
helper/BitMath.sol:8: require(x >0, "BitMath::mostSignificantBit: zero");
helper/BitMath.sol:45: require(x >0, "BitMath::leastSignificantBit: zero");
helper/FixedPoint.sol:85: require(upper <=type(uint112).max, "FixedPoint::muluq: upper overflow");
helper/FixedPoint.sol:105: require(other._x >0, "FixedPoint::divuq: division by zero");
helper/FixedPoint.sol:131: require(denominator >0, "FixedPoint::fraction: division by zero");
helper/FixedPoint.sol:149: require(self._x !=0, "FixedPoint::reciprocal: reciprocal of zero");
Migrations.sol:9: require(msg.sender== owner, "This function is restricted to the contract's owner");
I suggest shortening the revert strings to fit in 32 bytes.
Use Custom Errors instead of Revert Strings to save Gas
Solidity 0.8.4 introduced custom errors. They are more gas efficient than revert strings, when it comes to deploy cost as well as runtime cost when the revert condition is met. Use custom errors instead of revert strings for gas savings.
Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)
Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.
Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).
I suggest replacing all revert strings with custom errors in the solution. Affected files:
helper/BitMath.sol
helper/BoringMath.sol
helper/FixedPoint.sol
helper/FullMath.sol
helper/SmartWalletWhitelist.sol
token/VE3Token.sol
token/VeToken.sol
BaseRewardPool.sol
Booster.sol
DepositToken.sol
ExtraRewardStashV3.sol
Migrations.sol
PoolManager.sol
RewardFactory.sol
StashFactory.sol
TokenFactory.sol
VE3DLocker.sol
VE3DRewardPool.sol
VeAssetDepositor.sol
VeTokenMinter.sol
VirtualBalanceRewardPool.sol
VoterProxy.sol
Functions guaranteed to revert when called by normal users can be marked payable
If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.
PoolManager.sol:27: ) external onlyOwner returns (bool) {
PoolManager.sol:40: function shutdownPool(address_pools, uint256_pid) external onlyOwner returns (bool) {
RewardFactory.sol:24: function addOperator(address_newOperator, address_veAsset) public onlyOwner {
RewardFactory.sol:29: function removeOperator(address_operator) public onlyOwner {
StashFactory.sol:30: function addOperator(address_newOperator) public onlyOwner {
StashFactory.sol:34: function removeOperator(address_operator) public onlyOwner {
TokenFactory.sol:19: function addOperator(address_newOperator) public onlyOwner {
TokenFactory.sol:23: function removeOperator(address_operator) public onlyOwner {
VE3DLocker.sol:179: ) external onlyOwner {
VE3DLocker.sol:185: function setKickIncentive(uint256_rate, uint256_delay) external onlyOwner {
VE3DLocker.sol:193: function shutdown() external onlyOwner {
VE3DLocker.sol:197: function addOperator(address_newOperator) public onlyOwner {
VE3DLocker.sol:201: function removeOperator(address_operator) public onlyOwner {
VE3DLocker.sol:789: function recoverERC20(address_tokenAddress, uint256_tokenAmount) external onlyOwner {
VE3DRewardPool.sol:107: ) external onlyOwner {
VE3DRewardPool.sol:114: function addOperator(address_newOperator) public onlyOwner {
VE3DRewardPool.sol:118: function removeOperator(address_operator) public onlyOwner {
VeTokenMinter.sol:32: function addOperator(address_newOperator) public onlyOwner {
VeTokenMinter.sol:36: function removeOperator(address_operator) public onlyOwner {
VeTokenMinter.sol:41: function updateveAssetWeight(addressveAssetOperator, uint256newWeight) external onlyOwner {
VeTokenMinter.sol:77: function withdraw(address_destination, uint256_amount) external onlyOwner {
The text was updated successfully, but these errors were encountered:
Table of Contents:
> 0
is less efficient than!= 0
for unsigned integers (with proof)>=
is cheaper than>
(and<=
cheaper than<
)require()
statements that use&&
saves gas++i
costs less gas compared toi++
ori += 1
(same for--i
vsi--
ori -= 1
)abi.encode()
is less efficient thanabi.encodePacked()
payable
Cheap Contract Deployment Through Clones
See
@audit
tag:There's a way to save a significant amount of gas on deployment using Clones: https://www.youtube.com/watch?v=3Mw-pMmJ7TA .
This is a solution that was adopted, as an example, by Porter Finance. They realized that deploying using clones was 10x cheaper:
I suggest applying a similar pattern.
SafeMath is not needed when using Solidity version 0.8+
Solidity version 0.8+ already implements overflow and underflow checks by default.
Using the SafeMath library from OpenZeppelin (which is more gas expensive than the 0.8+ overflow checks) is therefore redundant.
I suggest using the built-in checks instead of SafeMath and remove SafeMath here:
Boolean comparisons
Comparing to a constant (
true
orfalse
) is a bit more expensive than directly checking the returned boolean value.I suggest using
if(directValue)
instead ofif(directValue == true)
andif(!directValue)
instead ofif(directValue == false)
here:> 0
is less efficient than!= 0
for unsigned integers (with proof)!= 0
costs less gas compared to> 0
for unsigned integers inrequire
statements with the optimizer enabled (6 gas)Proof: While it may seem that
> 0
is cheaper than!=
, this is only true without the optimizer enabled and outside a require statement. If you enable the optimizer at 10k AND you're in arequire
statement, this will save gas. You can see this tweet for more proofs: https://twitter.com/gzeon/status/1485428085885640706I suggest changing
> 0
with!= 0
here:Also, please enable the Optimizer.
>=
is cheaper than>
(and<=
cheaper than<
)Strict inequalities (
>
) are more expensive than non-strict ones (>=
). This is due to some supplementary checks (ISZERO, 3 gas). This also holds true between<=
and<
.Consider replacing strict inequalities with non-strict ones to save some gas here:
Splitting
require()
statements that use&&
saves gasIf you're using the Optimizer at 200, instead of using the
&&
operator in a single require statement to check multiple conditions, I suggest using multiple require statements with 1 condition per require statement:Please, note that this might not hold true at a higher number of runs for the Optimizer (10k). However, it indeed is true at 200.
Use shift right/left instead of division/multiplication if possible
While the
DIV
/MUL
opcode uses 5 gas, theSHR
/SHL
opcode only uses 3 gas. Furthermore, beware that Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting. Eventually, overflow checks are never performed for shift operations as they are done for arithmetic operations. Instead, the result is always truncated.>> 1
instead of/ 2
>> 2
instead of/ 4
<< 3
instead of* 8
Affected code:
An array's length should be cached to save gas in for-loops
Reading array length at each iteration of the loop consumes more gas than necessary.
In the best case scenario (length read on a memory variable), caching the array length in the stack saves around 3 gas per iteration.
In the worst case scenario (external calls at each iteration, like at
VE3DRewardPool.sol:148
orVE3DRewardPool.sol:281
), the amount of gas wasted can be massive.Here, I suggest storing the array's length in a variable before the for-loop, and use this new variable instead:
++i
costs less gas compared toi++
ori += 1
(same for--i
vsi--
ori -= 1
)Pre-increments and pre-decrements are cheaper.
For a
uint256 i
variable, the following is true with the Optimizer enabled at 10k:Increment:
i += 1
is the most expensive formi++
costs 6 gas less thani += 1
++i
costs 5 gas less thani++
(11 gas less thani += 1
)Decrement:
i -= 1
is the most expensive formi--
costs 11 gas less thani -= 1
--i
costs 5 gas less thani--
(16 gas less thani -= 1
)Note that post-increments (or post-decrements) return the old value before incrementing or decrementing, hence the name post-increment:
However, pre-increments (or pre-decrements) return the new value:
In the pre-increment case, the compiler has to create a temporary variable (when used) for returning
1
instead of2
.Affected code:
Consider using pre-increments and pre-decrements where they are relevant (meaning: not where post-increments/decrements logic are relevant).
Increments/decrements can be unchecked in for-loops
In Solidity 0.8+, there's a default overflow check on unsigned integers. It's possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.
ethereum/solidity#10695
Affected code:
The change would be:
The same can be applied with decrements (which should use
break
wheni == 0
).The risk of overflow is non-existant for
uint256
here.abi.encode()
is less efficient thanabi.encodePacked()
Changing
abi.encode
function toabi.encodePacked
can save gas since theabi.encode
function pads extra null bytes at the end of the call data, which is unnecessary. Also, in general,abi.encodePacked
is more gas-efficient (see Solidity-Encode-Gas-Comparison).Consider using
abi.encodePacked()
here:Public functions to external
An external call cost is less expensive than one of a public function.
The following functions could be set external to save gas and improve code quality (extracted from Slither).
No need to explicitly initialize variables with default values
If a variable is not set/initialized, it is assumed to have the default value (
0
foruint
,false
forbool
,address(0)
for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas.As an example:
for (uint256 i = 0; i < numIterations; ++i) {
should be replaced withfor (uint256 i; i < numIterations; ++i) {
Affected code:
I suggest removing explicit initializations for default values.
Variable that should be constant
This variable is never edited:
Consider marking it as constant.
Reduce the size of error messages (Long revert Strings)
Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.
Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.
Revert strings > 32 bytes:
I suggest shortening the revert strings to fit in 32 bytes.
Use Custom Errors instead of Revert Strings to save Gas
Solidity 0.8.4 introduced custom errors. They are more gas efficient than revert strings, when it comes to deploy cost as well as runtime cost when the revert condition is met. Use custom errors instead of revert strings for gas savings.
Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)
Source: https://blog.soliditylang.org/2021/04/21/custom-errors/:
Custom errors are defined using the
error
statement, which can be used inside and outside of contracts (including interfaces and libraries).I suggest replacing all revert strings with custom errors in the solution. Affected files:
Functions guaranteed to revert when called by normal users can be marked
payable
If a function modifier such as
onlyOwner
is used, the function will revert if a normal user tries to pay the function. Marking the function aspayable
will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.The text was updated successfully, but these errors were encountered: