Gas Optimizations #578
Labels
bug
Warden finding
G (Gas Optimization)
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
FINDINGS
Using immutable on variables that are only set in the constructor and never after
File: Migration.sol line 39
The above is only set in the constructor and never set again
File: Migration.sol line 37
File: Buyout.sol line 29
File: Buyout.sol line 31
File: Buyout.sol line 33
File: BaseVault.sol line 19
Using unchecked blocks to save gas - Increments in for loop can be unchecked ( save 30-40 gas per loop iteration)
The majority of Solidity for loops increment a uint256 variable that starts at 0. These increment operations never need to be checked for over/underflow because the variable will never reach the max number of uint256 (will run out of gas long before that happens). The default over/underflow check wastes gas in every iteration of virtually every for loop . eg.
e.g Let's work with a sample loop below.
can be written as shown below.
We can also write it as an inlined function like below.
Affected code
File: Vault.sol line 78
The above should be modified to:
Other Instances to modify
File: Vault.sol line 104
see resource
Cache storage values in memory to minimize SLOADs
The code can be optimized by minimizing the number of SLOADs. SLOADs are expensive 100 gas compared to MLOADs/MSTOREs(3gas)
Storage value should get cached in memory
NB: Some functions have been truncated where necessary to just show affected parts of the code
Migration.sol.propose() : registry should be cached (saves ~ 92 gas)
File: Migration.sol line 81-95
In the above registry should be cached in memory to reduce number of SLOADs.
SLOAD 1: line 81
SLOAD 2: line 95
Migration.sol.migrateFractions() : registry should be cached (saves ~ 372 gas)
File: Migration.sol line 435,467,470
In the above registry should be cached in memory to reduce number of SLOADs.
SLOAD 1: line 435
SLOAD 2: line 467
SLOAD 3: line 470
Migration.sol.migrateFractions() : registry should be cached (saves ~ 101 gas)
File:Migration.sol line 184,200
In the above registry should be cached in memory to reduce number of SLOADs.
SLOAD 1: line 184
SLOAD 2: line 200
Buyout.sol.start() : registry should be cached (saves ~ 83 gas)
File:Buyout.sol line 61-71
In the above registry should be cached in memory to reduce number of SLOADs.
SLOAD 1: line 61 and the SLOAD 2: line 71
Buyout.sol.cash() : registry should be cached (saves ~ 84 gas)
File: Buyout.sol line 246-267
In the above registry should be cached in memory to reduce number of SLOADs.
SLOAD 1: line 246 and the SLOAD 2: line 267
Buyout.sol.redeem() : registry should be cached (saves ~97 gas)
File: Buyout.sol line 280-288
In the above registry should be cached in memory to reduce number of SLOADs.
SLOAD 1 : line 280 and the SLOAD 2 : line 288
FERC1155.sol.uri() : metadata[_uri] should be cached
File: FERC1155.sol line 297-298
SLOAD 1: in the require statement line 297 costing 100 gas
SLOAD 2: in the return statement line 298 again costing 100gas
We can cache
metadata[_id]
in memory and read the value from memory instead of from storageHelp the optimizer by saving a storage variable's reference instead of repeatedly fetching migrationInfo[_vault][_proposalId]
Declare a Storage variable and use it instead of fetching the reference in the map repeatedly.
Instead of calling
migrationInfo[_vault][_proposalId]
everytime save it's reference like shown below and use the reference.File: Migration.sol line 454-456
In the above function,
migrationInfo[_vault][_proposalId]
is being fetched 3 times in the following lines1: line 454
2: line 455
3: line 466
Something similar to my proposal has already been implemented on line 266
Cache the length of arrays in loops (saves ~6 gas per iteration)
Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.
The solidity compiler will always read the length of the array during each iteration. That is,
1.if it is a storage array, this is an extra sload operation (100 additional extra gas (EIP-2929 2) for each iteration except for the first),
2.if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first),
3.if it is a calldata array, this is an extra calldataload operation (3 additional gas for each iteration except for the first)
This extra costs can be avoided by caching the array length (in stack):
When reading the length of an array, sload or mload or calldataload operation is only called once and subsequently replaced by a cheap dupN instruction. Even though mload , calldataload and dupN have the same gas cost, mload and calldataload needs an additional dupN to put the offset in the stack, i.e., an extra 3 gas. which brings this to 6 gas
Here, I suggest storing the array’s length in a variable before the for-loop, and use it instead:
File: MerkleBase.sol line 51
The above should be modified to
Other instances to modify
File: MerkleBase.sol line 110
File: BaseVault.sol line 64
File: BaseVault.sol line 83
File: BaseVault.sol line 107
File: BaseVault.sol line 130
File: BaseVault.sol line 132
File: Buyout.sol line 454
File: MerkleBase.sol line 78
The following shows all instances where
_data.length
is being accessed in the function getProof()++i costs less gas compared to i++ or i += 1 (~5 gas per iteration)
++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:
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 of 2
Instances include:
File: Vault.sol line 78
The above should be modified to:
File: Vault.sol line 104
use shorter revert strings(less than 32 bytes)
Every reason string takes at least 32 bytes so make sure your string fits in 32 bytes or it will become more expensive.
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.
File: MerkleBase.sol line 62
Other instances to modify
File MerkleBase.sol line 78
I suggest shortening the revert strings to fit in 32 bytes, or using custom errors.
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)
see Source
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).
Instances to modify
File: FERC1155.sol line 263-268
File: FERC1155.sol line 275
File: FERC1155.sol line 297
Use Shift Right/Left instead of Division/Multiplication
A division/multiplication by any number x being a power of 2 can be calculated by shifting log2(x) to the right/left.
While the DIV opcode uses 5 gas, the SHR opcode only uses 3 gas. Furthermore, Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting.
relevant source
File: MerkleBase.sol line 100
The above should be modified to
File: MerkleBase.sol line 142
Expressions for constant values such as a call to keccak256(), should use immutable rather than constant
Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas.
If the variable was immutable instead: the calculation would only be done once at deploy time (in the constructor), and then the result would be saved and read directly at runtime rather than being recalculated.
consequences:
Each usage of a "constant" costs ~100gas more on each access (it is still a little better than storing the result in storage, but not much..)
Since these are not real constants, they can't be referenced from a real constant environment (e.g. from assembly, or from another library )
See: ethereum/solidity#9232
File: Permit.sol line 5
File: Permit.sol line 10
File: Permit.sol line 15
constants should be defined rather than using magic numbers
There are several occurrences of literal values with unexplained meaning .Literal values in the codebase without an explained meaning make the code harder to read, understand and maintain, thus hindering the experience of developers, auditors and external contributors alike.
Developers should define a constant variable for every magic value used , giving it a clear and self-explanatory name. Additionally, for complex values, inline comments explaining how they were calculated or why they were chosen are highly recommended. Following Solidity’s style guide, constants should be named in UPPER_CASE_WITH_UNDERSCORES format, and specific public getters should be defined to read each one of them.
File: Buyout.sol line 209
File: Buyout.sol line 211
File: Buyout.sol line 86-87
File: FERC1155.sol line 247
Using Private Rather than Public for constants saves gas
If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non payable getter functions for deployment calldata, and not adding another entry to the method ID table
File: FERC1155.sol line 15
File: FERC1155.sol line 17
File: Buyout.sol line 35
File: Buyout.sol line 37
File: Migration.sol line 43
Use CALLDATA Instead of Memory
File: FERC1155.sol line 68-72
When arguments are read only on external functions, the data location should be calldata avoiding the cost of allocating memory or storage.
string memory _uri
should be modified tostring calldata _uri
File: FERC1155.sol line 79-87
Since
_data
is only being read and not modified here, we can modify ourbytes memory _data
tobytes calldata _data
File: Metadata.sol line 24-31
Since
_uri
is only being read we can use calldata instead of memory : fromstring memory _uri
tostring calldata _uri
The text was updated successfully, but these errors were encountered: