Open code423n4 opened 2 years ago
Unnecessary MLOADs in for-each loops Prefix decrements are cheaper than postfix decrements ++ is cheaper than += 1
These recommendations can offer some savings.
Unnecessary CALLDATALOAD in BasicOrderFulfiller._validateAndFulfillBasicOrder()
This may result in a small savings.
Unused named returns can be removed Don't use named returns when it isn't being used as a variable
This may offer small savings, but sometimes mixing patterns like this is done for improved readability.
Use smaller unsigned integer types for timestamp fields in structures
Agree using uint40 may be appropriate to consider for timestamp fields.
Default value initialization
The compile seems to handle most of these automatically. In my testing, these optimizations made very little difference.
Place enums next to addresses in structs
This tactic can be worth considering, particularly for any struct which is saved in storage.
Use strict comparisons when comparing timestamps
When now == endTime users may expect the order to go through. It's not clear that the small gas savings here warrants changing the behavior. 1 second won't be common but it would impact some users.
Unnecessary MLOAD in OrderCombiner._validateOrdersAndPrepareToFulfill() Unnecessary array boundaries check when loading an array element twice
These are straightforward changes that should provide small savings.
Use if instead of else if whenever possible
This can provide small savings so long as it's very clear that the else
is redundant in each of these examples.
[1] Unnecessary MLOADs in for-each loops
There are many for loops that follows this for-each pattern:
In such for loops, the
array.length
is read on every iteration, instead of caching it once in a local variable and read it from there. Memory reads are a bit more expensive than reading local variables.Recommendation
Read these values from memory once, cache them in local variables and then read them again from the local variables. For example:
Code References
OrderCombiner.sol
: lines 291, 598, 621OrderFulfiller.sol
: lines 217, 306[2] Unnecessary CALLDATALOAD in
BasicOrderFulfiller._validateAndFulfillBasicOrder()
The following assembly code from
BasicOrderFulfiller._validateAndFulfillBasicOrder()
invokes the CALLDATALOAD opcode twice using the same argument:Recommendation
Optimize by invoking that opcode only once and caching the result in a local variable:
[3] Unused named returns can be removed
Removing unused named return variables can reduce gas usage and improve code clarity. For example:
can be optimized to:
Code References
Consideration.sol
: lines 225, 311, 352, 402, 522-525, 561-563OrderCombiner.sol
: lines 135, 547, 665, 708, 791OrderValidator.sol
: lines 113-114, 422-425[4] Use smaller unsigned integer types for timestamp fields in structures
The structures
OrderComponents
,BasicOrderParameters
andOrderParameters
contains fields calledstartTime
andendTime
of typeuint256
. These fields holds timestamp values which are much smaller thantype(uint40).max
for example. Therefore, changing the type of these fields to a smaller unsigned integer type (likeuint40
) can save significant amount of gas because in such case, these two fields will share the same 32-byte slot and the entire structure will use one less slot.[5] Default value initialization
If a variable is not set/initialized, it is assumed to have the default value (
0
,false
,0x0
etc. depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes gas. For example:can be optimized to:
[6] Place enums next to addresses in structs
The structures
OrderComponents
,BasicOrderParameters
andOrderParameters
contains fields of typesOrderType
andBasicOrderType
(enums), as well as fields of typeaddress
. These fields should be places next to each other so they can share the same 32-byte slot and the entire structure will use one less slot. This is because the enum type don't use more than 12 bytes.[7] Use strict comparisons when comparing timestamps
The function
Verifiers._verifyTime()
validates that the current time doesn't exceed a given range of timestamps:The EVM doesn't have an opcode for
<=
comparison, and therefore it performs that comparison by splitting it to a<
comparison AND a==
comparison.Recommendation
Change that if condition to:
This won't change the functionality because we deal with timestamps. One second won't affect the protocol.
[8] Don't use named returns when it isn't being used as a variable
Explicit returns use less gas than implicit returns (named returns). Therefore, whenever you don't use the benefits of implicit returns (the use of the return variables), you should change the function to use explicit return instead.
Take
Consideration.fulfillBasicOrder
as an example:Here you don't use
fulfilled
as a variable, but only assigns it the return value. It can easily be optimized to:[9] Unnecessary MLOAD in
OrderCombiner._validateOrdersAndPrepareToFulfill()
The following code can be optimized by reading a local variable (
endAmount
) instead of a struct field located on memory (offerItem.endAmount
):The optimized code is as follows:
The same optimization can be applied to the code bellow as well (from the same function):
[10] Prefix decrements are cheaper than postfix decrements
Use prefix decrements (
--x
) instead of postfix decrements (x--
).Recommendation
Change all postfix decrements to prefix decrements.
Code References
[11] Unnecessary array boundaries check when loading an array element twice
Some functions loads the same array element (e.g.,
orderHashes[i]
) 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 and saving gas.Recommendation
Load the array elements once, cache them in local variables and then read them again using the local variables. For example:
Code References
CriteriaResolution.sol
: lines 168+177 (advancedOrders[i]
)OrderCombiner.sol
: lines 375+386 (orderHashes[i]
)[12] Use
if
instead ofelse if
whenever possibleif
statements are cheaper thanelse if
statements.Recommendation
Modify any
else if
statement toif
statement when it doesn't affect functionality (when the conditions can pass together).Code References
Conduit.sol
: lines 179, 192ConduitController.sol
: line 149BasicOrderFulfiller.sol
: lines 220, 242Executor.sol
: lines 64, 74[13]
++
is cheaper than+= 1
Use prefix increments (
++x
) instead of increments by 1 (x += 1
).Recommendation
Change all increments by 1 to prefix increments.
Code References