Gas Optimizations #71
Comments
Deploying clones would save cost when Conduits are created, however it also increases the cost to use the conduits created. That increase has a tiny impact, but I assume it was intentional to favor the end-users here - creating conduits will be relatively rare and reserved for platforms and/or power users.
This general tactic often can often result in significant savings, but I ran the recommended change and here it only saves ~100 gas on
This will save some gas, but
This is similar to the recommendation in #60 (although #60 appears to be a bit more thorough) and provides fairly significant savings on critical code paths.
This is a safe change to make since
Yes these should provide some savings.
It's unfortunate that the optimizer cannot handle scenarios like this automatically... There does appear to be a small win here, but it's debatable whether the impact to readability is worth it here.
These changes seem to be focused on getters, it's not clear it would impact gas for any transactions.
This appears to be handled by the optimizer automatically now. Testing did not change the gas results.
This recommendation causes tests to fail, suggesting this change violates the EIP-712 standard. |
Table of Contents:
ConduitController.sol#createConduit(): Help the optimizer by saving a storage variable's reference instead of repeatedly fetching itConduitController.sol#acceptOwnership(): Help the optimizer by saving a storage variable's reference instead of repeatedly fetching itOrderValidator.sol#_validateBasicOrderAndUpdateStatus(): Help the optimizer by saving a storage variable's reference instead of repeatedly fetching itOrderValidator.sol#_validateBasicOrderAndUpdateStatus(): avoid an unnecessary SSTORE by not writing a default valueOrderCombiner.sol:++totalFilteredExecutionscosts less gas compared tototalFilteredExecutions += 1OrderCombiner.sol:--maximumFulfilledcosts less gas compared tomaximumFulfilled--FulfillmentApplier.sol#_applyFulfillment(): Unchecking arithmetics operations that can't underflow/overflow/reference: Unchecking arithmetics operations that can't underflow/overflowORconditions cost less than their equivalentANDconditions ("NOT(something is false)" costs less than "everything is true")abi.encode()is less efficient thanabi.encodePacked()Cheap Contract Deployment Through Clones
See
@audittag:There's a way to save a significant amount of gas on deployment using Clones: https://www.youtube.com/watch?v=3Mw-pMmJ7TA .
This is a solution that was adopted, as an example, by Porter Finance. They realized that deploying using clones was 10x cheaper:
I suggest applying a similar pattern, here with a
cloneDeterministicmethod to mimic the currentcreate2ConduitController.sol#createConduit(): Help the optimizer by saving a storage variable's reference instead of repeatedly fetching itTo help the optimizer, declare a
storagetype variable and use it instead of repeatedly fetching the reference in a map or an array.The effect can be quite significant.
As an example, instead of repeatedly calling
someMap[someIndex], save its reference like this:SomeStruct storage someStruct = someMap[someIndex]and use it.Affected code (check the
@audittags):Notice that this optimization already exists in the solution:
ConduitController.sol#acceptOwnership(): Help the optimizer by saving a storage variable's reference instead of repeatedly fetching itThis optimization is similar to the one explained here.
Instead of repeatedly fetching the storage region, consider declaring and using a storage variable here (see
@audittags):File: ConduitController.sol 232: function acceptOwnership(address conduit) external override { ... - 237: if (msg.sender != _conduits[conduit].potentialOwner) { //@audit gas: similar to L130, should declare "ConduitProperties storage _conduitProperties = _conduits[conduit];" and use it to get potentialOwner + 236: ConduitProperties storage _conduitProperties = _conduits[conduit]; + 237: if (msg.sender != _conduitProperties.potentialOwner) { ... - 246: delete _conduits[conduit].potentialOwner;//@audit gas: should use suggested storage variable _conduitProperties to delete potentialOwner + 246: delete _conduitProperties.potentialOwner; ... 249: emit OwnershipTransferred( 250: conduit, - 251: _conduits[conduit].owner, //@audit gas: should use suggested storage variable _conduitProperties to get owner + 251: _conduitProperties.owner, 252: msg.sender 253: ); ... - 256: _conduits[conduit].owner = msg.sender; //@audit gas: should use suggested storage variable _conduitProperties to set owner + 256: _conduitProperties.owner = msg.sender;OrderValidator.sol#_validateBasicOrderAndUpdateStatus(): Help the optimizer by saving a storage variable's reference instead of repeatedly fetching itThis optimization is similar to the one explained here.
Instead of repeatedly fetching the storage region, consider declaring and using a storage variable here (see
@audittags):OrderValidator.sol#_validateBasicOrderAndUpdateStatus(): avoid an unnecessary SSTORE by not writing a default valueThe following line is not needed, as it's writing to storage a default value:
Consider removing this line completely.
OrderCombiner.sol:++totalFilteredExecutionscosts less gas compared tototalFilteredExecutions += 1For a
uint256 ivariable, the following is true with the Optimizer enabled at 10k:i += 1is the most expensive formi++costs 6 gas less thani += 1++icosts 5 gas less thani++(11 gas less thani += 1)Consider replacing
totalFilteredExecutions += 1with++totalFilteredExecutionshere:OrderCombiner.sol:--maximumFulfilledcosts less gas compared tomaximumFulfilled--For a
uint256 ivariable, the following is true with the Optimizer enabled at 10k:i -= 1is the most expensive formi--costs 11 gas less thani -= 1--icosts 5 gas less thani--(16 gas less thani -= 1)Consider replacing
maximumFulfilled--with--maximumFulfilledhere:FulfillmentApplier.sol#_applyFulfillment(): Unchecking arithmetics operations that can't underflow/overflowSolidity 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
uncheckedblock: https://docs.soliditylang.org/en/v0.8.10/control-structures.html#checked-or-unchecked-arithmeticI suggest wrapping with an
uncheckedblock here (see@audittags for more details):/reference: Unchecking arithmetics operations that can't underflow/overflowThis is similar to the optimization above, except that here, contracts under
/referenceare just readable versions of the real contracts to be deployed.The following lines should be
unchecked, please check that this is the case in their corresponding assembly code:ORconditions cost less than their equivalentANDconditions ("NOT(something is false)" costs less than "everything is true")Remember that the equivalent of
(a && b)is!(!a || !b)Even with the 10k Optimizer enabled:
ORconditions cost less than their equivalentANDconditions.POC
pragma solidity 0.8.13; contract Test { bool isOpen; bool channelPreviouslyOpen; function boolTest() external view returns (uint) { - if (isOpen && !channelPreviouslyOpen) { + if (!(!isOpen || channelPreviouslyOpen)) { return 1; - } else if (!isOpen && channelPreviouslyOpen) { + } else if (!(isOpen || !channelPreviouslyOpen)) { return 2; } } function setBools(bool _isOpen, bool _channelPreviouslyOpen) external { isOpen = _isOpen; channelPreviouslyOpen= _channelPreviouslyOpen; } }Affected code
Added together, it's possible to save a significant amount of gas by replacing the
&&conditions by their||equivalent in the solution.ConduitController.sol#updateChannel()Use
!(!isOpen || channelPreviouslyOpen)instead ofisOpen && !channelPreviouslyOpenand use!(isOpen || !channelPreviouslyOpen)instead of!isOpen && channelPreviouslyOpen:OrderValidator.sol#_validateOrderAndUpdateStatus()Use
!(!(numerator < denominator) || !_doesNotSupportPartialFills(orderParameters.orderType))instead ofnumerator < denominator && _doesNotSupportPartialFills(orderParameters.orderType:OrderValidator.sol#_cancel()Use
!(msg.sender == offerer || msg.sender == zone)instead ofmsg.sender != offerer && msg.sender != zonehere:SignatureVerification.sol#_assertValidSignature()Use
!(v == 27 || v == 28)instead ofv != 27 && v != 28:ZoneInteraction.sol#_assertRestrictedBasicOrderValidity()Use
!(!(uint256(orderType) > 1) || msg.sender == zone || msg.sender == offerer)instead ofuint256(orderType) > 1 && msg.sender != zone && msg.sender != offerer:ZoneInteraction.sol#_assertRestrictedAdvancedOrderValidity()(1)Use
!(!(uint256(orderType) > 1) || msg.sender == zone || msg.sender == offerer)instead ofuint256(orderType) > 1 && msg.sender != zone && msg.sender != offerer:ZoneInteraction.sol#_assertRestrictedAdvancedOrderValidity()(2)Use
!(advancedOrder.extraData.length != 0 || criteriaResolvers.length != 0)instead ofadvancedOrder.extraData.length == 0 && criteriaResolvers.length == 0:Bytes constants are more efficient than string constants
From the Solidity doc:
Why do Solidity examples use bytes32 type instead of string?
If data can fit into 32 bytes, then you should use bytes32 datatype rather than bytes or strings as it is cheaper in solidity. Basically, any fixed size variable in solidity is cheaper than variable size. That will save gas on the contract.
Instances of
string constantthat can be replaced bybytes(1..32) constant:An array's length should be cached to save gas in for-loops
Reading array length at each iteration of the loop consumes more gas than necessary.
In the best case scenario (length read on a memory variable), caching the array length in the stack saves around 3 gas per iteration.
In the worst case scenario (external calls at each iteration), the amount of gas wasted can be massive.
Here, lengths are only read from memory.
Consider storing the array's length in a variable before the for-loop, and use this newly created variable instead:
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
Affected code:
The code would go from:
to:
The risk of overflow is inexistant for
uint256here.Note that this is already applied at some places in the solution. As an example:
No need to explicitly initialize variables with default values
This finding is only true without the Optimizer
If a variable is not set/initialized, it is assumed to have the default value (
0foruint,falseforbool,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) {Affected code:
I suggest removing explicit initializations for default values.
abi.encode()is less efficient thanabi.encodePacked()Changing
abi.encodefunction toabi.encodePackedcan save gas since theabi.encodefunction pads extra null bytes at the end of the call data, which is unnecessary. Also, in general,abi.encodePackedis more gas-efficient (see Solidity-Encode-Gas-Comparison).Consider using
abi.encodePacked()here:Consider using the assembly equivalent for
abi.encodePacked()here:Notice that this is already used at other places for a similar situation:
The text was updated successfully, but these errors were encountered: