NB: Some functions have been truncated where neccessary to just show affected parts of the code
The gas estimates are the exact results from running the tests included with an exception of internal functions(we estimate based on number of SLOADS saved)
The optimizer is set to run with the default runs(200).
Throught the report some places might be denoted with audit tags to show the actual place affected.
Using immutable on variables that are only set in the constructor and never after
Use immutable if you want to assign a permanent value at construction. Use constants if you already know the permanent value. Both get directly embedded in bytecode, saving SLOAD.
Variables only set in the constructor and never edited afterwards should be marked as immutable, as it would avoid the expensive storage-writing operation in the constructor (around 20 000 gas per variable) and replace the expensive storage-reading operations (around 2100 gas per reading) to a less expensive value reading (3 gas)
Use calldata instead of memory for function parameters
When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * .length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution.When arguments are read-only on external functions, the data location should be calldata:
PRICE.sol.initialize(): uint256[] memory startObservations should be uint256[] calldata startObservations(Saves ~ 1933 gas)
Average Gas Before: 432495 Average Gas After: 430562
File: /src/modules/PRICE.sol
function initialize(uint256[] memory startObservations_, uint48 lastObservationTime_)
external
permissioned
{
if (initialized) revert Price_AlreadyInitialized();
// Cache numObservations to save gas.
uint256 numObs = observations.length;
// Check that the number of start observations matches the number expected
if (startObservations_.length != numObs || lastObservationTime_ > uint48(block.timestamp))
revert Price_InvalidParams();
// Push start observations into storage and total up observations
uint256 total;
for (uint256 i; i < numObs; ) {
if (startObservations_[i] == 0) revert Price_InvalidParams();
total += startObservations_[i];
observations[i] = startObservations_[i];
unchecked {
++i;
}
}
// Set moving average, last observation time, and initialized flag
_movingAverage = total / numObs;
lastObservationTime = lastObservationTime_;
initialized = true;
}
startObservations_ should be declared calldata as it is readonly on this function
File: /src/policies/TreasuryCustodian.sol
function revokePolicyApprovals(address policy_, ERC20[] memory tokens_) external {
if (Policy(policy_).isActive()) revert PolicyStillActive();
// TODO Make sure `policy_` is an actual policy and not a random address.
uint256 len = tokens_.length;
for (uint256 j; j < len; ) {
TRSRY.setApprovalFor(policy_, tokens_[j], 0);
unchecked {
++j;
}
}
emit ApprovalRevoked(policy_, tokens_);
}
ERC20[] memory tokens_ should be declared as ERC20[] calldata tokens_ as it is readonly in this function
string memory proposalURI_ should be declared as string calldata proposalURI_ as it is readonly in this function
Caching storage values in memory to minimize SLOADs
The code can be optimized by minimizing the number of SLOADs.
SLOADs are expensive (100 gas after the 1st one) compared to MLOADs/MSTOREs (3 gas each). Storage values read multiple times should instead be cached in memory the first time (costing 1 SLOAD) and then read from this cache to avoid multiple SLOADs.
File: /src/modules/PRICE.sol
240: function changeMovingAverageDuration(uint48 movingAverageDuration_) external permissioned {
241: // Moving Average Duration should be divisible by Observation Frequency to get a whole number of observations
242: if (movingAverageDuration_ == 0 || movingAverageDuration_ % observationFrequency != 0) //@audit: SLOAD 1
243: revert Price_InvalidParams();
...
245: // Calculate the new number of observations
246: uint256 newObservations = uint256(movingAverageDuration_ / observationFrequency); //@audit: SLOAD 2
PRICE.sol.changeObservationFrequency(): movingAverageDuration should be cached(Saves 1 sload)
File: /src/modules/PRICE.sol
266: function changeObservationFrequency(uint48 observationFrequency_) external permissioned {
267: // Moving Average Duration should be divisible by Observation Frequency to get a whole number of observations
268: if (observationFrequency_ == 0 || movingAverageDuration % observationFrequency_ != 0) //@audit: SLOAD 1
269: revert Price_InvalidParams();
...
271: // Calculate the new number of observations
272: uint256 newObservations = uint256(movingAverageDuration / observationFrequency_); //@audit: SLOAD 2
Heart.sol.beat(): lastBeat should be cached(Saves 1 sload ~40gas , saves ~399 if we cache the result of the external call frequency() )
Estimations without caching the frequency function: only cache lastBeat
Average Gas before:29228 Average Gas After: 29188
Estimations after caching the frequency function: lastBeat and frequency
Average Gas before:29228 Average Gas After: 28829
File: /src/policies/Heart.sol
92: function beat() external nonReentrant {
93: if (!active) revert Heart_BeatStopped();
94: if (block.timestamp < lastBeat + frequency()) revert Heart_OutOfCycle(); //@audit: lastBeat SLOAD 1
...
102: // Update the last beat timestamp
103: lastBeat += frequency(); //@audit: lastBeat SLOAD 2
...
109: }
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.
As an example, instead of repeatedly calling someMap[someIndex], save its reference like this: SomeStruct storage somestruct = someMap[someIndex] and use it.
TRSRY.sol.repayLoan(): reserveDebt[token_][msg.sender] should be cached
Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate
Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.
File: /src/modules/TRSRY.sol
36: mapping(ERC20 => uint256) public totalDebt;
38: /// @notice Debt for particular token and debtor address
39: mapping(ERC20 => mapping(address => uint256)) public reserveDebt;
Use Shift Right/Left instead of Division/Multiplication
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, so the calculation can be unchecked in Solidity version 0.8+
Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead
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.
File: /src/policies/PriceConfig.sol
//@audit: uint48 lastObservationTime_
45: function initialize(uint256[] memory startObservations_, uint48 lastObservationTime_)
//@audit: uint48 movingAverageDuration_
58: function changeMovingAverageDuration(uint48 movingAverageDuration_)
//@audit: uint48 observationFrequency_
69: function changeObservationFrequency(uint48 observationFrequency_)
Using unchecked blocks to save gas
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
see resource
The operation earliestPrice - currentPrice cannot underflow due to the check on Line 135 which ensures that this operation would only be perfomened if earliestPrice is greter than currentPrice
The operation nextObsIndex - 1 cannot underflow as it would only be perfomed if nextObsIndex is not equal to 0. As nextObsIndex is a uint if it's not equal to 0 then it must be greater than 0
The operation amount_ - oldDebt cannot underlow due the check if (oldDebt < amount_) that ensures that amount is greater than oldDebt before performng the operation
The operation oldDebt - amount_ cannot underlow due the check if (oldDebt < amount_) that ensures that this operation would only be perfomed if oldDebt is greater than amount_
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:
++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:
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
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:
File: /src/policies/Governance.sol
223: if (proposalHasBeenActivated[proposalId_] == true) {
306: if (tokenClaimsForProposal[proposalId_][msg.sender] == true) {
Using bools for storage incurs overhead
Booleans are more expensive than uint256 or any type that takes up a full word because each write operation emits an extra SLOAD to first read the slot's contents, replace the bits taken up by the boolean, and then write back. This is the compiler's defense against contract upgrades and pointer aliasing, and it cannot be disabled.
See source
Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past
File: /src/policies/Governance.sol
105: mapping(uint256 => bool) public proposalHasBeenActivated;
117: mapping(uint256 => mapping(address => bool)) public tokenClaimsForProposal;
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.
FINDINGS
NB: Some functions have been truncated where neccessary to just show affected parts of the code The gas estimates are the exact results from running the tests included with an exception of internal functions(we estimate based on number of SLOADS saved) The optimizer is set to run with the default runs(200). Throught the report some places might be denoted with audit tags to show the actual place affected.
Using immutable on variables that are only set in the constructor and never after
Use immutable if you want to assign a permanent value at construction. Use constants if you already know the permanent value. Both get directly embedded in bytecode, saving SLOAD. Variables only set in the constructor and never edited afterwards should be marked as immutable, as it would avoid the expensive storage-writing operation in the constructor (around 20 000 gas per variable) and replace the expensive storage-reading operations (around 2100 gas per reading) to a less expensive value reading (3 gas)
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/BondCallback.sol#L28
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/BondCallback.sol#L32
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Heart.sol#L48
The result of a function call should be cached rather than re-calling the function
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Heart.sol#L92-L109
Heart.sol.beat(): frequency() should be cached(Saves ~351 gas)
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/PriceConfig.sol#L25-L35
PriceConfig.sol.requestPermissions():PRICE.KEYCODE() should be cached( Saves ~1472 gas)
The above can be rewriten as follows:
Other Instance: https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/VoterRegistration.sol#L27-L36
VoterRegistration.sol.requestPermissions():VOTES.KEYCODE() should be cached( Saves ~758 gas)
See an existing implementation already on Line 34 for how to implement the above function
Use calldata instead of memory for function parameters
When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 *.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution.When arguments are read-only on external functions, the data location should be calldata:
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L205
PRICE.sol.initialize(): uint256[] memory startObservations should be uint256[] calldata startObservations(Saves ~ 1933 gas)
startObservations_should be declared calldata as it is readonly on this functionhttps://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/TreasuryCustodian.sol#L53-L67
TreasuryCustodian.sol.revokePolicyApprovals(): ERC20[] memory tokensshould be ERC20[] calldata tokens(Saves ~ 114 gas)
ERC20[] memory tokens_should be declared asERC20[] calldata tokens_as it is readonly in this functionhttps://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/BondCallback.sol#L152-L166
BondCallback.sol.batchToTreasury(): ERC20[] memory tokensshould be ERC20[] calldata tokens(Saves ~ 186 gas)
ERC20[] memory tokens_should be declared asERC20[] calldata tokens_as it is readonly in this functionhttps://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/PriceConfig.sol#L45-L50
PriceConfig.sol.initialize(): uint256[] memory startObservations should be uint256[] calldata startObservations(Saves ~ 3580 gas)
uint256[] memory startObservations_should be declared asuint256[] calldata startObservations_as it is readonly in this functionhttps://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L159-L163
Governance.sol.submitProposal(): string memory proposalURI should be string memory proposalURI (Saves ~ 3580 gas)
string memory proposalURI_should be declared asstring calldata proposalURI_as it is readonly in this functionCaching storage values in memory to minimize SLOADs
The code can be optimized by minimizing the number of SLOADs.
SLOADs are expensive (100 gas after the 1st one) compared to MLOADs/MSTOREs (3 gas each). Storage values read multiple times should instead be cached in memory the first time (costing 1 SLOAD) and then read from this cache to avoid multiple SLOADs.
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L122-L147
PRICE.sol.updateMovingAverage(): nextObsIndex should be cached(Saves ~123 gas)
The gas saved ends up being higher than the estimates if we optimize the functions that are also called inside this one (~259 gas)
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L154-L180
PRICE.sol.getCurrentPrice(): observationFrequency should be cached(Saves ~134 gas)
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L122-L147
PRICE.sol.updateMovingAverage(): _movingAverage should be cached(Saves 1 sload ~ 99 gas )
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L183-L187
PRICE.sol.getLastPrice(): nextObsIndex should be cached(Saves 1 sload ~94 gas)
PRICE.sol.changeMovingAverageDuration(): observationFrequency should be cached(Saves 1 sload)
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L240-L246
PRICE.sol.changeObservationFrequency(): movingAverageDuration should be cached(Saves 1 sload)
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L266-L272
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Heart.sol#L92-L109
Heart.sol.beat(): lastBeat should be cached(Saves 1 sload ~40gas , saves ~399 if we cache the result of the external call
frequency())https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Heart.sol#L111-L114
Heart.sol._issueReward(): reward should be cached(Saves 1 sload ~94 gas)
Other interesting places that we can utilize caching.
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L240-L262
Governance.sol.vote(): activeProposal.proposalId should be cached(Saves ~281 gas)
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L265-L289
Governance.sol.executeProposal(): activeProposal.proposalId should be cached(Saves ~147gas)
Internal/Private functions only called once can be inlined to save gas
Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
Affected code:
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Heart.sol#L111-L114
The above function is only called on Line 106
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. As an example, instead of repeatedly calling
someMap[someIndex], save its reference like this:SomeStruct storage somestruct = someMap[someIndex]and use it.TRSRY.sol.repayLoan(): reserveDebt[token_][msg.sender] should be cached
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/TRSRY.sol#L105-L119
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L205-L236
Governance.sol.activateProposal(): proposalHasBeenActivated[proposalId_]
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L295-L314
Governance.sol.reclaimVotes():tokenClaimsForProposal[proposalId_][msg.sender] should be cached
Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate
Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/modules/TRSRY.sol#L36-L39
Use Shift Right/Left instead of Division/Multiplication
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, so the calculation can be unchecked in Solidity version 0.8+
relevant source
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Operator.sol#L372
x += y costs more gas than x = x + y for state variables
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Heart.sol#L103
The above should be modified to
Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead
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.
https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L205
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Operator.sol#L527-L531
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/PriceConfig.sol#L45
Using unchecked blocks to save gas
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 see resource
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L136
The operation
currentPrice - earliestPricecannot underflow due to the check on Line 135 which ensures thatcurrentPriceis greater thanearliestPricehttps://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L138The operation
earliestPrice - currentPricecannot underflow due to the check on Line 135 which ensures that this operation would only be perfomened ifearliestPriceis greter thancurrentPricehttps://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L185
The operation
nextObsIndex - 1cannot underflow as it would only be perfomed ifnextObsIndexis not equal to 0. AsnextObsIndexis a uint if it's not equal to 0 then it must be greater than 0https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/modules/TRSRY.sol#L131
The operation
amount_ - oldDebtcannot underlow due the checkif (oldDebt < amount_)that ensures that amount is greater than oldDebt before performng the operation The operationoldDebt - amount_cannot underlow due the checkif (oldDebt < amount_)that ensures that this operation would only be perfomed ifoldDebtis greater thanamount_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:
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L276-L278
The above should be modified to
++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:
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/utils/KernelUtils.sol#L49
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Operator.sol#L488
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:
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L223
Using bools for storage incurs overhead
Booleans are more expensive than uint256 or any type that takes up a full word because each write operation emits an extra SLOAD to first read the slot's contents, replace the bits taken up by the boolean, and then write back. This is the compiler's defense against contract upgrades and pointer aliasing, and it cannot be disabled.
See source Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past
Instances affected include
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L113
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L62
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Operator.sol#L63
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/BondCallback.sol#L24
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Heart.sol#L33
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L105
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.
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L121-L137
https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Operator.sol#L89
https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L65