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
Help the optimizer by saving a storage variable's reference instead of repeatedly fetching it
To help the optimizer, declare a storage type variable and use it instead of repeatedly fetching the reference in a map or an array.
The effect can be quite significant.
As an example, instead of repeatedly calling someMap[someIndex], save its reference like this: SomeStruct storage someStruct = someMap[someIndex] and use it.
Instances include (check the @audit tags):
core/contracts/dex/DexAddressProvider.sol:
53: return (_dexMapping[index].proxy, _dexMapping[index].router); //@audit gas: should declare a storage variable "Dex storage _dex = _dexMapping[index]"
Caching external values in memory
See the @audit tags for further details:
core/contracts/dex/DexAddressProvider.sol:
22: require(_a.controller().hasRole(_a.controller().MANAGER_ROLE(), msg.sender), "LM010"); //@audit gas: should cache _a.controller()
supervaults/contracts/SuperVault.sol:
369: if (ga.mimo().balanceOf(address(this)) >0) { //@audit gas: should cache ga.mimo()
Using an existing memory variable instead of reading storage
See the @audit tags for further details:
core/contracts/oracles/BalancerV2LPOracle.sol:
41: (address_pool, IBalancerVault.PoolSpecialization tokensNum) = vault.getPool(poolId); //@audit gas: should use memory variable _poolId: "vault.getPool(_poolId)"
File: BalancerV2LPOracle.sol
14: contractBalancerV2LPOracleisAggregatorV3Interface, BNum {
15: using SafeMathforuint256;
16:
17: stringpublicoverride description; //@audit gas: 32 bytes18: uint256publicoverride version =3; //@audit gas: 32 bytes19: uint8publicoverride decimals; //@audit gas: 1 byte, can be tightly packed by being moved further down or by moving an address closer20:
21: bytes32public poolId; //@audit gas: 32 bytes22: IBalancerVault public vault; //@audit gas: 20 bytes23: IBalancerPool public pool; //@audit gas: 20 bytes24: AggregatorV3Interface public oracleA; //@audit gas: 20 bytes25: AggregatorV3Interface public oracleB; //@audit gas: 20 bytes
to
contractBalancerV2LPOracleisAggregatorV3Interface, BNum {
using SafeMathforuint256;
stringpublicoverride description; //@audit gas: 32 bytes (slot 1)uint256publicoverride version =3; //@audit gas: 32 bytes (slot 2)uint8publicoverride decimals; //@audit gas: 1 byte (slot 3)
IBalancerVault public vault; //@audit gas: 20 bytes (slot 3)
IBalancerPool public pool; //@audit gas: 20 bytes (slot 4)bytes32public poolId; //@audit gas: 32 bytes <= this is the one we moved
AggregatorV3Interface public oracleA; //@audit gas: 20 bytes
AggregatorV3Interface public oracleB; //@audit gas: 20 bytes
> 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)
I suggest using <= instead of < here:
core/contracts/libraries/ABDKMath64x64.sol:697: returnuint128(r < r1 ? r : r1);
Splitting require() statements that use && saves gas
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 (saving 3 gas per &):
require() should be used for checking error conditions on inputs and return values while assert() should be used for invariant checking
Properly functioning code should never reach a failing assert statement, unless there is a bug in your contract you should fix. Here, I believe the assert should be a require or a revert:
core/contracts/libraries/ABDKMath64x64.sol:641: assert(xh == hi >>128);
As the Solidity version is 0.6.12 < 0.8.0, the remaining gas would not be refunded in case of failure.
Amounts should be checked for 0 before calling a transfer
Checking non-zero transfer values can avoid an expensive external call and save gas.
While this is done at some places, it's not consistently done in the solution.
An array's length should be cached to save gas in for-loops
Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.
Caching the array length in the stack saves around 3 gas per iteration.
Here, I suggest storing the array's length in a variable before the for-loop, and use it instead:
core/contracts/dex/DexAddressProvider.sol:16: for (uint256 i; i < dexes.length; i++) {
++i costs less gas compared to i++ or i += 1
++i costs less gas compared to i++ or i += 1 for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.
i++ increments i and returns the initial value of i. Which means:
uint i =1;
i++; // == 1 but i == 2
But ++i returns the actual incremented value:
uint i =1;
++i; // == 2 and i == 2 too, so no need for a temporary variable
In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2
Instances include:
core/contracts/dex/DexAddressProvider.sol:16: for (uint256 i; i < dexes.length; i++) {
core/contracts/inception/AdminInceptionVault.sol:108: for (uint8 i =1; i < _collateralCount +1; i++) {
core/contracts/libraries/ABDKMath64x64.sol:396: resultShift +=1;
core/contracts/libraries/ABDKMath64x64.sol:403: absXShift +=1;
core/contracts/libraries/ABDKMath64x64.sol:463: if (xc >=0x2) msb +=1; // No need to shift xc anymore
core/contracts/libraries/ABDKMath64x64.sol:624: if (xc >=0x2) msb +=1; // No need to shift xc anymore
I suggest using ++i instead of i++ to increment the value of an uint variable.
Usage of a non-native 256 bits uint as a counter in for-loops increases gas cost
Due to how the EVM natively works on 256 bit numbers, using a 8 bit number in for-loops introduces additional costs as the EVM has to properly enforce the limits of this smaller type.
When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.
It is only beneficial to use reduced-size arguments if you are dealing with storage values because the compiler will pack multiple elements into one storage slot, and thus, combine multiple reads or writes into a single operation. When dealing with function arguments or memory values, there is no inherent benefit because the compiler does not pack these values.
Affected code:
core/contracts/inception/AdminInceptionVault.sol:108: for (uint8 i =1; i < _collateralCount +1; i++) {
Consider manually checking for the upper bound before the for-loop and using the uint256 type as a counter in the mentioned for-loops.
Public functions to external
The following functions could be set external to save gas and improve code quality.
External call cost is less expensive than of public functions.
clone(bytes) should be declared external:
- SuperVaultFactory.clone(bytes) (contracts/SuperVaultFactory.sol#23-28)
parallel() should be declared external:
- DexAddressProvider.parallel() (contracts/dex/DexAddressProvider.sol#43-45)
dexMapping(uint256) should be declared external:
- DexAddressProvider.dexMapping(uint256) (contracts/dex/DexAddressProvider.sol#52-54)
deposit(address,uint256) should be declared external:
- AdminInceptionVault.deposit(address,uint256) (contracts/inception/AdminInceptionVault.sol#149-154)
borrow(uint256,uint256) should be declared external:
- AdminInceptionVault.borrow(uint256,uint256) (contracts/inception/AdminInceptionVault.sol#164-173)
a() should be declared external:
- AdminInceptionVault.a() (contracts/inception/AdminInceptionVault.sol#175-177)
debtNotifier() should be declared external:
- AdminInceptionVault.debtNotifier() (contracts/inception/AdminInceptionVault.sol#179-181)
weth() should be declared external:
- AdminInceptionVault.weth() (contracts/inception/AdminInceptionVault.sol#183-185)
mimo() should be declared external:
- AdminInceptionVault.mimo() (contracts/inception/AdminInceptionVault.sol#187-189)
inceptionCore() should be declared external:
- AdminInceptionVault.inceptionCore() (contracts/inception/AdminInceptionVault.sol#191-193)
collateralCount() should be declared external:
- AdminInceptionVault.collateralCount() (contracts/inception/AdminInceptionVault.sol#195-197)
collaterals(uint8) should be declared external:
- AdminInceptionVault.collaterals(uint8) (contracts/inception/AdminInceptionVault.sol#199-201)
collateralId(address) should be declared external:
- AdminInceptionVault.collateralId(address) (contracts/inception/AdminInceptionVault.sol#203-205)
a() should be declared external:
- InceptionVaultFactory.a() (contracts/inception/InceptionVaultFactory.sol#138-140)
debtNotifier() should be declared external:
- InceptionVaultFactory.debtNotifier() (contracts/inception/InceptionVaultFactory.sol#142-144)
weth() should be declared external:
- InceptionVaultFactory.weth() (contracts/inception/InceptionVaultFactory.sol#146-148)
mimo() should be declared external:
- InceptionVaultFactory.mimo() (contracts/inception/InceptionVaultFactory.sol#150-152)
adminInceptionVaultBase() should be declared external:
- InceptionVaultFactory.adminInceptionVaultBase() (contracts/inception/InceptionVaultFactory.sol#154-156)
inceptionVaultsCoreBase() should be declared external:
- InceptionVaultFactory.inceptionVaultsCoreBase() (contracts/inception/InceptionVaultFactory.sol#158-160)
inceptionVaultsDataProviderBase() should be declared external:
- InceptionVaultFactory.inceptionVaultsDataProviderBase() (contracts/inception/InceptionVaultFactory.sol#162-164)
inceptionVaultCount() should be declared external:
- InceptionVaultFactory.inceptionVaultCount() (contracts/inception/InceptionVaultFactory.sol#166-168)
priceFeedCount() should be declared external:
- InceptionVaultFactory.priceFeedCount() (contracts/inception/InceptionVaultFactory.sol#170-172)
inceptionVaults(uint256) should be declared external:
- InceptionVaultFactory.inceptionVaults(uint256) (contracts/inception/InceptionVaultFactory.sol#174-176)
priceFeeds(uint8) should be declared external:
- InceptionVaultFactory.priceFeeds(uint8) (contracts/inception/InceptionVaultFactory.sol#178-180)
priceFeedIds(address) should be declared external:
- InceptionVaultFactory.priceFeedIds(address) (contracts/inception/InceptionVaultFactory.sol#182-184)
cumulativeRate() should be declared external:
- InceptionVaultsCore.cumulativeRate() (contracts/inception/InceptionVaultsCore.sol#243-245)
lastRefresh() should be declared external:
- InceptionVaultsCore.lastRefresh() (contracts/inception/InceptionVaultsCore.sol#247-249)
vaultConfig() should be declared external:
- InceptionVaultsCore.vaultConfig() (contracts/inception/InceptionVaultsCore.sol#251-253)
a() should be declared external:
- InceptionVaultsCore.a() (contracts/inception/InceptionVaultsCore.sol#255-257)
inceptionCollateral() should be declared external:
- InceptionVaultsCore.inceptionCollateral() (contracts/inception/InceptionVaultsCore.sol#259-261)
adminInceptionVault() should be declared external:
- InceptionVaultsCore.adminInceptionVault() (contracts/inception/InceptionVaultsCore.sol#263-265)
inceptionVaultsData() should be declared external:
- InceptionVaultsCore.inceptionVaultsData() (contracts/inception/InceptionVaultsCore.sol#267-269)
inceptionPriceFeed() should be declared external:
- InceptionVaultsCore.inceptionPriceFeed() (contracts/inception/InceptionVaultsCore.sol#271-273)
a() should be declared external:
- InceptionVaultsDataProvider.a() (contracts/inception/InceptionVaultsDataProvider.sol#161-163)
inceptionVaultsCore() should be declared external:
- InceptionVaultsDataProvider.inceptionVaultsCore() (contracts/inception/InceptionVaultsDataProvider.sol#165-167)
inceptionVaultCount() should be declared external:
- InceptionVaultsDataProvider.inceptionVaultCount() (contracts/inception/InceptionVaultsDataProvider.sol#169-171)
baseDebt() should be declared external:
- InceptionVaultsDataProvider.baseDebt() (contracts/inception/InceptionVaultsDataProvider.sol#173-175)
a() should be declared external:
- ChainlinkInceptionPriceFeed.a() (contracts/inception/priceFeed/ChainlinkInceptionPriceFeed.sol#87-89)
inceptionCollateral() should be declared external:
- ChainlinkInceptionPriceFeed.inceptionCollateral() (contracts/inception/priceFeed/ChainlinkInceptionPriceFeed.sol#91-93)
assetOracle() should be declared external:
- ChainlinkInceptionPriceFeed.assetOracle() (contracts/inception/priceFeed/ChainlinkInceptionPriceFeed.sol#95-97)
eurOracle() should be declared external:
- ChainlinkInceptionPriceFeed.eurOracle() (contracts/inception/priceFeed/ChainlinkInceptionPriceFeed.sol#99-101)
deposit(uint256) should be declared external:
- DemandMinerV2.deposit(uint256) (contracts/liquidityMining/v2/DemandMinerV2.sol#66-76)
withdraw(uint256) should be declared external:
- DemandMinerV2.withdraw(uint256) (contracts/liquidityMining/v2/DemandMinerV2.sol#82-92)
token() should be declared external:
- DemandMinerV2.token() (contracts/liquidityMining/v2/DemandMinerV2.sol#94-96)
feeCollector() should be declared external:
- DemandMinerV2.feeCollector() (contracts/liquidityMining/v2/DemandMinerV2.sol#98-100)
feeConfig() should be declared external:
- DemandMinerV2.feeConfig() (contracts/liquidityMining/v2/DemandMinerV2.sol#102-104)
releaseRewards(address) should be declared external:
- GenericMinerV2.releaseRewards(address) (contracts/liquidityMining/v2/GenericMinerV2.sol#80-86)
- PARMinerV2.releaseRewards(address) (contracts/liquidityMining/v2/PARMinerV2.sol#136-142)
updateBoost(address) should be declared external:
- GenericMinerV2.updateBoost(address) (contracts/liquidityMining/v2/GenericMinerV2.sol#91-94)
stake(address) should be declared external:
- GenericMinerV2.stake(address) (contracts/liquidityMining/v2/GenericMinerV2.sol#101-103)
- PARMinerV2.stake(address) (contracts/liquidityMining/v2/PARMinerV2.sol#172-174)
stakeWithBoost(address) should be declared external:
- GenericMinerV2.stakeWithBoost(address) (contracts/liquidityMining/v2/GenericMinerV2.sol#110-112)
- PARMinerV2.stakeWithBoost(address) (contracts/liquidityMining/v2/PARMinerV2.sol#181-183)
pendingMIMO(address) should be declared external:
- GenericMinerV2.pendingMIMO(address) (contracts/liquidityMining/v2/GenericMinerV2.sol#119-122)
- PARMinerV2.pendingMIMO(address) (contracts/liquidityMining/v2/PARMinerV2.sol#190-193)
pendingPAR(address) should be declared external:
- GenericMinerV2.pendingPAR(address) (contracts/liquidityMining/v2/GenericMinerV2.sol#129-132)
- PARMinerV2.pendingPAR(address) (contracts/liquidityMining/v2/PARMinerV2.sol#200-207)
par() should be declared external:
- GenericMinerV2.par() (contracts/liquidityMining/v2/GenericMinerV2.sol#134-136)
- PARMinerV2.par() (contracts/liquidityMining/v2/PARMinerV2.sol#209-211)
a() should be declared external:
- GenericMinerV2.a() (contracts/liquidityMining/v2/GenericMinerV2.sol#138-140)
- PARMinerV2.a() (contracts/liquidityMining/v2/PARMinerV2.sol#213-215)
boostConfig() should be declared external:
- GenericMinerV2.boostConfig() (contracts/liquidityMining/v2/GenericMinerV2.sol#142-144)
- PARMinerV2.boostConfig() (contracts/liquidityMining/v2/PARMinerV2.sol#217-219)
totalStake() should be declared external:
- GenericMinerV2.totalStake() (contracts/liquidityMining/v2/GenericMinerV2.sol#146-148)
- PARMinerV2.totalStake() (contracts/liquidityMining/v2/PARMinerV2.sol#221-223)
totalStakeWithBoost() should be declared external:
- GenericMinerV2.totalStakeWithBoost() (contracts/liquidityMining/v2/GenericMinerV2.sol#150-152)
- PARMinerV2.totalStakeWithBoost() (contracts/liquidityMining/v2/PARMinerV2.sol#225-227)
userInfo(address) should be declared external:
- GenericMinerV2.userInfo(address) (contracts/liquidityMining/v2/GenericMinerV2.sol#164-166)
- PARMinerV2.userInfo(address) (contracts/liquidityMining/v2/PARMinerV2.sol#243-245)
deposit(uint256) should be declared external:
- PARMinerV2.deposit(uint256) (contracts/liquidityMining/v2/PARMinerV2.sol#91-94)
withdraw(uint256) should be declared external:
- PARMinerV2.withdraw(uint256) (contracts/liquidityMining/v2/PARMinerV2.sol#100-103)
liquidate(uint256,uint256,uint256,bytes) should be declared external:
- PARMinerV2.liquidate(uint256,uint256,uint256,bytes) (contracts/liquidityMining/v2/PARMinerV2.sol#112-130)
restakePAR(address) should be declared external:
- PARMinerV2.restakePAR(address) (contracts/liquidityMining/v2/PARMinerV2.sol#148-157)
updateBoost(address) should be declared external:
- PARMinerV2.updateBoost(address) (contracts/liquidityMining/v2/PARMinerV2.sol#162-165)
liquidateCallerReward() should be declared external:
- PARMinerV2.liquidateCallerReward() (contracts/liquidityMining/v2/PARMinerV2.sol#229-231)
baseDebtChanged(address,uint256) should be declared external:
- SupplyMinerV2.baseDebtChanged(address,uint256) (contracts/liquidityMining/v2/SupplyMinerV2.sol#45-47)
collateral() should be declared external:
- SupplyMinerV2.collateral() (contracts/liquidityMining/v2/SupplyMinerV2.sol#49-51)
syncStake(address) should be declared external:
- VotingMinerV2.syncStake(address) (contracts/liquidityMining/v2/VotingMinerV2.sol#56-60)
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) {
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).
Table of Contents:
BalancerV2LPOracle.sol
: Tighly pack storage variables> 0
is less efficient than!= 0
for unsigned integers (with proof)<=
is cheaper than<
require()
statements that use&&
saves gasrequire()
should be used for checking error conditions on inputs and return values whileassert()
should be used for invariant checking++i
costs less gas compared toi++
ori += 1
Help the optimizer by saving a storage variable's reference instead of repeatedly fetching it
To help the optimizer, declare a
storage
type variable and use it instead of repeatedly fetching the reference in a map or an array.The effect can be quite significant.
As an example, instead of repeatedly calling
someMap[someIndex]
, save its reference like this:SomeStruct storage someStruct = someMap[someIndex]
and use it.Instances include (check the
@audit
tags):Caching external values in memory
See the
@audit
tags for further details:Using an existing memory variable instead of reading storage
See the
@audit
tags for further details:BalancerV2LPOracle.sol
: Tighly pack storage variablesI suggest going from (see
@audit
tags):to
Which would save 1 storage slot.
Variables that should be constant
According to slither:
> 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<
Strict inequalities (
<
) are more expensive than non-strict ones (<=
). This is due to some supplementary checks (ISZERO, 3 gas)I suggest using
<=
instead of<
here:Splitting
require()
statements that use&&
saves gasInstead 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 (saving 3 gas per&
):require()
should be used for checking error conditions on inputs and return values whileassert()
should be used for invariant checkingProperly functioning code should never reach a failing assert statement, unless there is a bug in your contract you should fix. Here, I believe the assert should be a require or a revert:
As the Solidity version is
0.6.12 < 0.8.0
, the remaining gas would not be refunded in case of failure.Amounts should be checked for 0 before calling a transfer
Checking non-zero transfer values can avoid an expensive external call and save gas.
While this is done at some places, it's not consistently done in the solution.
I suggest adding a non-zero-value check here:
An array's length should be cached to save gas in for-loops
Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.
Caching the array length in the stack saves around 3 gas per iteration.
Here, I suggest storing the array's length in a variable before the for-loop, and use it instead:
++i
costs less gas compared toi++
ori += 1
++i
costs less gas compared toi++
ori += 1
for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.i++
incrementsi
and returns the initial value ofi
. Which means:But
++i
returns the actual incremented value:In the first case, the compiler has to create a temporary variable (when used) for returning
1
instead of2
Instances include:
I suggest using
++i
instead ofi++
to increment the value of an uint variable.Usage of a non-native 256 bits uint as a counter in for-loops increases gas cost
Due to how the EVM natively works on 256 bit numbers, using a 8 bit number in for-loops introduces additional costs as the EVM has to properly enforce the limits of this smaller type.
See the warning at this link: https://docs.soliditylang.org/en/v0.8.0/internals/layout_in_storage.html#layout-of-state-variables-in-storage :
Affected code:
Consider manually checking for the upper bound before the for-loop and using the
uint256
type as a counter in the mentioned for-loops.Public functions to external
The following functions could be set external to save gas and improve code quality.
External call cost is less expensive than of public functions.
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) {
Instances include:
I suggest removing explicit initializations for default values.
Use Custom Errors instead of Revert Strings to save Gas
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).Instances include:
I suggest replacing revert strings with custom errors.
The text was updated successfully, but these errors were encountered: