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) {
Instances include:
UniswapV2PathPriceOracle.sol:34: for (uint i = 0; i < path.length - 1; i++) {
UniswapV2PathPriceOracle.sol:49: for (uint i = 0; i < path.length - 1; i++) {
I suggest removing explicit initializations for default values.
Some variables should be immutable
These variables are only set in the constructor and are never edited after that:
File: PhuturePriceOracle.sol
23: /// @notice Base asset address
24: address public base; //@audit gas: should be immutable
25:
26: /// @notice Index registry address
27: address public registry; //@audit gas: should be immutable
Consider marking them as immutable.
Caching storage values in memory
The code can be optimized by minimising the number of SLOADs. SLOADs are expensive (100 gas) compared to MLOADs/MSTOREs (3 gas). Here, storage values should get cached in memory (see the @audit tags for further details):
contracts/PhuturePriceOracle.sol:
83: require(priceOracleOf[_asset] != address(0), "PhuturePriceOracle: UNSET"); //@audit gas: should cache priceOracleOf[_asset]
84: return IPriceOracle(priceOracleOf[_asset]).refreshedAssetPerBaseInUQ(_asset); //@audit gas: should use cached priceOracleOf[_asset]
93: require(priceOracleOf[_asset] != address(0), "PhuturePriceOracle: UNSET"); //@audit gas: should cache priceOracleOf[_asset]
94: return IPriceOracle(priceOracleOf[_asset]).lastAssetPerBaseInUQ(_asset); //@audit gas: should use cached priceOracleOf[_asset]
> 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
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:
BaseIndex.sol:78: for (uint i; i < _assets.length; ++i) {
IndexLogic.sol:39: for (uint i; i < assets.length(); ++i) {
IndexLogic.sol:60: for (uint i; i < inactiveAssets.length(); ++i) {
IndexLogic.sol:125: for (uint i; i < length + inactiveAssets.length(); ++i) {
ManagedIndex.sol:30: for (uint i; i < _assets.length; ++i) {
ManagedIndexReweightingLogic.sol:38: for (uint i; i < assets.length(); ++i) {
ManagedIndexReweightingLogic.sol:50: for (uint i; i < _updatedAssets.length; ++i) {
ManagedIndexReweightingLogic.sol:96: for (uint i; i < _inactiveAssets.length; ++i) {
TopNMarketCapIndex.sol:48: for (uint i; i < _assets.length; ++i) {
TopNMarketCapReweightingLogic.sol:37: for (uint i; i < assets.length(); ++i) {
TopNMarketCapReweightingLogic.sol:51: for (uint _i; _i < diff.assetCount; ++_i) {
TopNMarketCapReweightingLogic.sol:104: for (uint i; i < _inactiveAssets.length; ++i) {
TrackedIndex.sol:35: for (uint i; i < _assets.length; ++i) {
TrackedIndexReweightingLogic.sol:37: for (uint i; i < assets.length(); ++i) {
TrackedIndexReweightingLogic.sol:66: for (uint i; i < assets.length(); ++i) {
UniswapV2PathPriceOracle.sol:34: for (uint i = 0; i < path.length - 1; i++) {
UniswapV2PathPriceOracle.sol:49: for (uint i = 0; i < path.length - 1; 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:
libraries/FullMath.sol:124: result++;
UniswapV2PathPriceOracle.sol:34: for (uint i = 0; i < path.length - 1; i++) {
UniswapV2PathPriceOracle.sol:49: for (uint i = 0; i < path.length - 1; i++) {
I suggest using ++i instead of i++ to increment the value of an uint variable.
Increments can be unchecked
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.
BaseIndex.sol:78: for (uint i; i < _assets.length; ++i) {
IndexLogic.sol:39: for (uint i; i < assets.length(); ++i) {
IndexLogic.sol:60: for (uint i; i < inactiveAssets.length(); ++i) {
IndexLogic.sol:102: for (uint i; i < length; ++i) {
IndexLogic.sol:125: for (uint i; i < length + inactiveAssets.length(); ++i) {
ManagedIndex.sol:30: for (uint i; i < _assets.length; ++i) {
ManagedIndexReweightingLogic.sol:38: for (uint i; i < assets.length(); ++i) {
ManagedIndexReweightingLogic.sol:50: for (uint i; i < _updatedAssets.length; ++i) {
ManagedIndexReweightingLogic.sol:96: for (uint i; i < _inactiveAssets.length; ++i) {
TopNMarketCapIndex.sol:48: for (uint i; i < _assets.length; ++i) {
TopNMarketCapReweightingLogic.sol:37: for (uint i; i < assets.length(); ++i) {
TopNMarketCapReweightingLogic.sol:51: for (uint _i; _i < diff.assetCount; ++_i) {
TopNMarketCapReweightingLogic.sol:104: for (uint i; i < _inactiveAssets.length; ++i) {
TrackedIndex.sol:35: for (uint i; i < _assets.length; ++i) {
TrackedIndexReweightingLogic.sol:37: for (uint i; i < assets.length(); ++i) {
TrackedIndexReweightingLogic.sol:66: for (uint i; i < assets.length(); ++i) {
UniswapV2PathPriceOracle.sol:34: for (uint i = 0; i < path.length - 1; i++) {
UniswapV2PathPriceOracle.sol:49: for (uint i = 0; i < path.length - 1; i++) {
The code would go from:
for (uint256 i; i < numIterations; i++) {
// ...
}
to:
for (uint256 i; i < numIterations;) {
// ...
unchecked { ++i; }
}
The risk of overflow is inexistant for a uint256 here.
Unchecking arithmetics operations that can't underflow/overflow
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:
> 0
is less efficient than!= 0
for unsigned integers (with proof)++i
costs less gas compared toi++
ori += 1
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.
Some variables should be immutable
These variables are only set in the constructor and are never edited after that:
Consider marking them as immutable.
Caching storage values in memory
The code can be optimized by minimising the number of SLOADs. SLOADs are expensive (100 gas) compared to MLOADs/MSTOREs (3 gas). Here, storage values should get cached in memory (see the
@audit
tags for further details):> 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.
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.Increments can be unchecked
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
Instances include:
The code would go from:
to:
The risk of overflow is inexistant for a
uint256
here.Unchecking arithmetics operations that can't underflow/overflow
Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn't possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an
unchecked
block: https://docs.soliditylang.org/en/v0.8.10/control-structures.html#checked-or-unchecked-arithmeticI suggest wrapping with an
unchecked
block here (see@audit
tags for more details):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, or using custom errors as described next.
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.