Please, consider everything described below as a general recommendation. These notes will represent potential possibilities to optimize gas consumption. It's okay, if something is not suitable in your case. Just let me know the reason in the comments section. Enjoy!
[G-01] Try ++i instead of i++
Description:
In case of i++, the compiler has to to create a temp variable to return i (if there's a need) and then i gets incremented.
In case of ++i, the compiler just simply returns already incremented value.
[G-03] Consider marking onlyOwner functions as payable
Description:
This one is a bit questionable, but you can try that out. So, the compiler adds some extra conditions in case of non-payable, but we know that onlyOwner modifier will be reverted, if the user invoke following methods.
All occurances:
Contracts:
file: src/Kernel.sol
...............................
// Lines: [76-78]
function changeKernel(Kernel newKernel_) external onlyKernel {
kernel = newKernel_;
}
// Lines: [126-128]
function setActiveStatus(bool activate_) external onlyKernel {
isActive = activate_;
}
// Lines: [439-448]
function grantRole(Role role_, address addr_) public onlyAdmin {}
// Lines: [451-460]
function revokeRole(Role role_, address addr_) public onlyAdmin {}
// Lines: [235-260]
function revokeRole(Role role_, address addr_) public onlyAdmin {}
file: src/policies/BondCallback.sol
...............................
// Lines: [61-76]
function requestPermissions()
external
view
override
onlyKernel
returns (Permissions[] memory requests)
{}
file: src/policies/Governance.sol
...............................
// Lines: [61-76]
function requestPermissions()
external
view
override
onlyKernel
returns (Permissions[] memory requests)
{}
*[G-04] Use binary shifting instead of a / 2^x, x > 0 or `a 2^x, x > 0`**
Description:
It's also pretty impactful one, especially in loops.
MLOAD costs only 3 units of gas, SLOAD(warm access) is about 100 units. Therefore, cache, when it's possible.
All occurances:
Contracts:
file: src/policies/Governance.sol
...............................
// Lines: [240-262]
// Comment: it is possible to cache `activeProposal.proposalId` in order to get MLOAD instead of warm access per each invoking.
function vote(bool for_) external {
uint256 userVotes = VOTES.balanceOf(msg.sender);
if (activeProposal.proposalId == 0) {
revert NoActiveProposalDetected();
}
if (userVotesForProposal[activeProposal.proposalId][msg.sender] > 0) {
revert UserAlreadyVoted();
}
if (for_) {
yesVotesForProposal[activeProposal.proposalId] += userVotes;
} else {
noVotesForProposal[activeProposal.proposalId] += userVotes;
}
userVotesForProposal[activeProposal.proposalId][msg.sender] = userVotes;
VOTES.transferFrom(msg.sender, address(this), userVotes);
emit WalletVoted(activeProposal.proposalId, msg.sender, for_, userVotes);
}
// Lines: [265-289]
// Comment: it is possible to cache `activeProposal.proposalId` in order to get MLOAD instead of warm access per each invoking.
function executeProposal() external {
uint256 netVotes = yesVotesForProposal[activeProposal.proposalId] -
noVotesForProposal[activeProposal.proposalId];
if (netVotes * 100 < VOTES.totalSupply() * EXECUTION_THRESHOLD) {
revert NotEnoughVotesToExecute();
}
if (block.timestamp < activeProposal.activationTimestamp + EXECUTION_TIMELOCK) {
revert ExecutionTimelockStillActive();
}
Instruction[] memory instructions = INSTR.getInstructions(activeProposal.proposalId);
for (uint256 step; step < instructions.length; ) {
kernel.executeAction(instructions[step].action, instructions[step].target);
unchecked {
++step;
}
}
emit ProposalExecuted(activeProposal.proposalId);
// deactivate the active proposal
activeProposal = ActivatedProposal(0, 0);
}
file: src/modules/RANGE.sol
...............................
// Lines: [158-178]
// Comment: here you can allocate the result of `_range.wall.low.price` computation and etc into the memory, so in event you can simply pass that precomputed value instead of taking warm access to get `_range.wal.low.price again`. The same for all computation could be done.
function updatePrices(uint256 movingAverage_) external permissioned {
// Cache the spreads
uint256 wallSpread = _range.wall.spread;
uint256 cushionSpread = _range.cushion.spread;
// Calculate new wall and cushion values from moving average and spread
_range.wall.low.price = (movingAverage_ * (FACTOR_SCALE - wallSpread)) / FACTOR_SCALE;
_range.wall.high.price = (movingAverage_ * (FACTOR_SCALE + wallSpread)) / FACTOR_SCALE;
_range.cushion.low.price = (movingAverage_ * (FACTOR_SCALE - cushionSpread)) / FACTOR_SCALE;
_range.cushion.high.price =
(movingAverage_ * (FACTOR_SCALE + cushionSpread)) /
FACTOR_SCALE;
emit PricesChanged(
_range.wall.low.price,
_range.cushion.low.price,
_range.cushion.high.price,
_range.wall.high.price
);
}
file: src/modules/PRICE.sol
...............................
// Lines: [97-100]
// Comment: Also cache computation into memory, then assign the numObservations and after that just pass this cached result into allocating array.
numObservations = uint32(movingAverageDuration_ / observationFrequency_);
observations = new uint256[](numObservations);
// Lines: [144-144]
// This line could be transformed a bit:
nextObsIndex = (nextObsIndex + 1) % numObs;
++nextObsIndex %= numObs;
// Lines: [165-171]
// Comment: cache `observationFrequency`
if (updatedAt < block.timestamp - 3 * uint256(observationFrequency))
revert Price_BadFeed(address(_ohmEthPriceFeed));
ohmEthPrice = uint256(ohmEthPriceInt);
int256 reserveEthPriceInt;
(, reserveEthPriceInt, , updatedAt, ) = _reserveEthPriceFeed.latestRoundData();
if (updatedAt < block.timestamp - uint256(observationFrequency))
// Lines: [240-262]
[G-06] Add require() before some computations, if it makes sense
Description:
Everyting above require() takes some gas for execution, therefore if the statement reverts gas will not be retrieved.
All occurances:
Contracts:
file: src/modules/INSTR.sol
...............................
// Lines: [48-48]
// Comment: Put right after getting length in order to avoid unnecessary cold/warm accesses in case of the failure.
if (length == 0) revert INSTR_InstructionsCannotBeEmpty();
file: src/policies/Governance.sol
...............................
// Lines: [183-185]
if (proposalId_ == 0) {
revert CannotEndorseNullProposal();
}
[G-07] Internal functions can be inlined
Description:
It takes some extra JUMPs which costs around 40-50 gas uints. In loops it will save significant amount of gas.
All occurances:
Contracts:
file: src/policies/Operator.sol
...............................
// Lines: [643-643]
function _updateRangePrices() internal {}
// Lines: [634-634]
function _updateCapacity(bool high_, uint256 reduceBy_) internal {}
[G-08] Use private/internal for constants/immutable variables instead of public
Description:
Optimization comes from not creating a getter function for each public instance. Try to define them as private/internal if it's possible in specific case.
All occurances:
Contracts:
file: src/Kernel.sol
...............................
// Lines: [155-158]
address public executor;
address public admin;
// Lines: [165-197]
Keycode[] public allKeycodes;
mapping(Keycode => Module) public getModuleForKeycode;
mapping(Module => Keycode) public getKeycodeForModule;
mapping(Keycode => Policy[]) public moduleDependents;
mapping(Keycode => mapping(Policy => uint256)) public getDependentIndex;
mapping(Keycode => mapping(Policy => mapping(bytes4 => bool))) public modulePermissions;
Policy[] public activePolicies;
mapping(Policy => uint256) public getPolicyIndex;
mapping(address => mapping(Role => bool)) public hasRole;
mapping(Role => bool) public isRole;
// Comment: I'll not include every single occurance, because the main message have been proposed.
[G-09] Use const values instead of type(uint256).max
Description:
Not sure about readability, but it might be tangible in loops.
[G-10] Mark functions as external instead of public, if there are no internal calls
Description:
Functions marked by external use call data to read arguments, where public will first allocate in local memory and then load them.
All occurances:
Contracts:
file: src/modules/INSTR.sol
...............................
// Lines: [28-30]
function VERSION() public pure override returns (uint8 major, uint8 minor) {
return (1, 0);
}
// Lines: [37-39]
function getInstructions(uint256 instructionsId_) public view returns (Instruction[] memory) {
return storedInstructions[instructionsId_];
}
file: src/modules/MINTR.sol
...............................
// Lines: [20-22]
function KEYCODE() public pure override returns (Keycode) {
return toKeycode("MINTR");
}
// Lines: [33-39]
function mintOhm(address to_, uint256 amount_) public permissioned {
ohm.mint(to_, amount_);
}
function burnOhm(address from_, uint256 amount_) public permissioned {
ohm.burnFrom(from_, amount_);
}
[G-11] Use calldataload instead of mload
Description:
After Berlin hard fork, to load a non-zero byte from calldata dropped from 64 units of gas to 16 units, therefore if you do not modify args, use a calldata instead of memory. Here you need to explicitly specify calldataload, or replace memory with calldata. If the args are pretty huge, allocating args in memory will cost a lot.
Table of contents
a = a + b
instead ofa += b
a / 2^x, x > 0
MLOAD
<<SLOAD
require()
before some computations, if it makes senseInternal
functions can be inlinedprivate/internal
forconstants/immutable
variables instead ofpublic
type(uint256).max
external
instead ofpublic
, if there are no internal callscalldataload
instead ofmload
immutable/const
Disclaimer
[G-01] Try ++i instead of i++
Description:
i++
, the compiler has to to create a temp variable to returni
(if there's a need) and theni
gets incremented.++i
, the compiler just simply returns already incremented value.All occurances:
Contracts:
[G-02] Consider
a = a + b
instead ofa += b
Description:
All occurances:
Contracts:
[G-03] Consider marking onlyOwner functions as payable
Description:
onlyOwner
modifier will be reverted, if the user invoke following methods.All occurances:
Contracts:
*[G-04] Use binary shifting instead of
a / 2^x, x > 0
or `a 2^x, x > 0`**Description:
All occurances:
Contracts:
[G-05] Cache state variables,
MLOAD
<<SLOAD
Description:
MLOAD
costs only 3 units of gas,SLOAD
(warm access) is about 100 units. Therefore, cache, when it's possible.All occurances:
Contracts:
[G-06] Add
require() before some computations, if it makes sense
Description:
require()
takes some gas for execution, therefore if the statement reverts gas will not be retrieved.All occurances:
Contracts:
[G-07]
Internal
functions can be inlinedDescription:
JUMP
s which costs around 40-50 gas uints. In loops it will save significant amount of gas.All occurances:
Contracts:
[G-08] Use
private/internal
forconstants/immutable
variables instead ofpublic
Description:
public
instance. Try to define them as private/internal if it's possible in specific case.All occurances:
Contracts:
[G-09] Use const values instead of
type(uint256).max
Description:
All occurances:
Contracts:
[G-10] Mark functions as
external
instead ofpublic
, if there are no internal callsDescription:
external
use call data to read arguments, wherepublic
will first allocate in local memory and then load them.All occurances:
Contracts:
[G-11] Use
calldataload
instead ofmload
Description:
calldataload
, or replacememory
withcalldata
. If the args are pretty huge, allocating args in memory will cost a lot.All occurances:
Contracts:
Kudos for the quality of the code! It's pretty easy to explore