Caching the array length is more gas efficient.
This is because access to a local variable in solidity is more efficient than query storage / calldata / memory.
We recommend to change from:
for (uint256 i=0; i<array.length; i++) { ... }
to:
uint len = array.length
for (uint256 i=0; i<len; i++) { ... }
Prefix increments are cheaper than postfix increments
Prefix increments are cheaper than postfix increments.
Further more, using unchecked {++x} is even more gas efficient, and the gas saving accumulates every iteration and can make a real change
There is no risk of overflow caused by increamenting the iteration index in for loops (the ++i in for (uint256 i = 0; i < numIterations; ++i)).
But increments perform overflow checks that are not necessary in this case.
These functions use not using prefix increments (++x) or not using the unchecked keyword:
Code instances:
change to prefix increment and unchecked: Basket.sol, i, 93
change to prefix increment and unchecked: NibblVault.sol, i, 547
change to prefix increment and unchecked: UpgradedNibblVault.sol, i, 469
change to prefix increment and unchecked: UpgradedNibblVault.sol, i, 486
change to prefix increment and unchecked: Basket.sol, i, 70
change to prefix increment and unchecked: Basket.sol, i, 43
change to prefix increment and unchecked: NibblVault.sol, i, 525
change to prefix increment and unchecked: NibblVault.sol, i, 506
change to prefix increment and unchecked: UpgradedNibblVault.sol, i, 452
Unnecessary index init
In for loops you initialize the index to start from 0, but it already initialized to 0 in default and this assignment cost gas.
It is more clear and gas efficient to declare without assigning 0 and will have the same meaning:
Reading a storage variable is gas costly (SLOAD). In cases of multiple read of a storage variable in the same scope, caching the first read (i.e saving as a local variable) can save gas and decrease the
overall gas uses. The following is a list of functions and the storage variables that you read twice:
Code instances:
UpgradedNibblVault.sol: SCALE is read twice in getCurrentValuation
NibblVault.sol: SCALE is read twice in _chargeFeeSecondaryCurve
UpgradedNibblVault.sol: primaryReserveRatio is read twice in initialize
NibblVault.sol: SCALE is read twice in _chargeFee
NibblVault.sol: SCALE is read twice in getCurrentValuation
NibblVault.sol: minBuyoutTime is read twice in initiateBuyout
UpgradedNibblVault.sol: SCALE is read twice in _chargeFee
Rearrange state variables
You can change the order of the storage variables to decrease memory uses.
Code instances:
In UpgradedNibblVault.sol,rearranging the storage fields can optimize to: 27 slots from: 28 slots.
The new order of types (you choose the actual variables):
uint256
uint256
uint256
uint256
uint256
bytes32
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
Status
uint
uint256
address
uint32
uint32
address
address
address
In NibblVault.sol,rearranging the storage fields can optimize to: 28 slots from: 29 slots.
The new order of types (you choose the actual variables):
uint256
uint256
uint256
uint256
uint256
uint256
uint256
bytes32
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
uint256
Status
uint256
address
uint32
uint32
address
address
address
Short the following require messages
The following require messages are of length more than 32 and we think are short enough to short
them into exactly 32 characters such that it will be placed in one slot of memory and the require
function will cost less gas.
The list:
Code instances:
Solidity file: NibblVaultFactory.sol, In line 49, Require message length to shorten: 33, The message: NibblVaultFactory: Invalid sender
Solidity file: MaliciousFactory.sol, In line 46, Require message length to shorten: 33, The message: NibblVaultFactory: Invalid sender
Consider inline the following functions to save gas
You can inline the following functions instead of writing a specific function to save gas.
(see https://github.com/code-423n4/2021-11-nested-findings/issues/167 for a similar issue.)
'transferFrom(address(this), *, *)' could be replaced by the following more gas efficient 'transfer(, **)'
This replacement is more gas efficient and improves the code quality.
Unnecessary array boundaries check when loading an array element twice
There are places in the code (especially in for-each loops) that loads the same array element more than once.
In such cases, only one array boundaries check should take place, and the rest are unnecessary.
Therefore, this array element should be cached in a local variable and then be loaded
again using this local variable, skipping the redundant second array boundaries check:
Code instances:
Basket.sol.withdrawMultipleERC20 - double load of _tokens[i]
NibblVault.sol.withdrawMultipleERC20 - double load of _assets[i]
UpgradedNibblVault.sol.withdrawMultipleERC20 - double load of _assets[i]
Unused state variables
Unused state variables are gas consuming at deployment (since they are located in storage) and are
a bad code practice. Removing those variables will decrease deployment gas cost and improve code quality.
This is a full list of all the unused storage variables we found in your code base.
In the following files there are state variables that could be set immutable to save gas.
Code instances:
initialTokenPrice in NibblVault.sol
minBuyoutTime in UpgradedNibblVault.sol
curatorFee in NibblVault.sol
fictitiousPrimaryReserveBalance in UpgradedNibblVault.sol
minBuyoutTime in NibblVault.sol
initialTokenPrice in UpgradedNibblVault.sol
curatorFee in UpgradedNibblVault.sol
fictitiousPrimaryReserveBalance in NibblVault.sol
Caching array length can save gas
Caching the array length is more gas efficient. This is because access to a local variable in solidity is more efficient than query storage / calldata / memory. We recommend to change from:
to:
Code instances:
Prefix increments are cheaper than postfix increments
Prefix increments are cheaper than postfix increments. Further more, using unchecked {++x} is even more gas efficient, and the gas saving accumulates every iteration and can make a real change There is no risk of overflow caused by increamenting the iteration index in for loops (the
++i
infor (uint256 i = 0; i < numIterations; ++i)
). But increments perform overflow checks that are not necessary in this case. These functions use not using prefix increments (++x
) or not using the unchecked keyword:Code instances:
Unnecessary index init
In for loops you initialize the index to start from 0, but it already initialized to 0 in default and this assignment cost gas. It is more clear and gas efficient to declare without assigning 0 and will have the same meaning:
Code instances:
Storage double reading. Could save SLOAD
Reading a storage variable is gas costly (SLOAD). In cases of multiple read of a storage variable in the same scope, caching the first read (i.e saving as a local variable) can save gas and decrease the overall gas uses. The following is a list of functions and the storage variables that you read twice:
Code instances:
Rearrange state variables
You can change the order of the storage variables to decrease memory uses.
Code instances:
In UpgradedNibblVault.sol,rearranging the storage fields can optimize to: 27 slots from: 28 slots. The new order of types (you choose the actual variables):
In NibblVault.sol,rearranging the storage fields can optimize to: 28 slots from: 29 slots. The new order of types (you choose the actual variables):
Short the following require messages
The following require messages are of length more than 32 and we think are short enough to short them into exactly 32 characters such that it will be placed in one slot of memory and the require function will cost less gas. The list:
Code instances:
Use != 0 instead of > 0
Using != 0 is slightly cheaper than > 0. (see https://github.com/code-423n4/2021-12-maple-findings/issues/75 for similar issue)
Code instances:
Use unchecked to save gas for certain additive calculations that cannot overflow
You can use unchecked in the following calculations since there is no risk to overflow:
Code instances:
Consider inline the following functions to save gas
Code instances
Inline one time use functions
The following functions are used exactly once. Therefore you can inline them and save gas and improve code clearness.
Code instances:
Do not cache msg.sender
We recommend not to cache msg.sender since calling it is 2 gas while reading a variable is more.
Code instances:
Change transferFrom to transfer
'transferFrom(address(this), *, *)' could be replaced by the following more gas efficient 'transfer(, **)' This replacement is more gas efficient and improves the code quality.
Code instances:
Unnecessary array boundaries check when loading an array element twice
Code instances:
Unused state variables
Unused state variables are gas consuming at deployment (since they are located in storage) and are a bad code practice. Removing those variables will decrease deployment gas cost and improve code quality. This is a full list of all the unused storage variables we found in your code base.
Code instances:
State variables that could be set immutable
In the following files there are state variables that could be set immutable to save gas.
Code instances: