Open code423n4 opened 1 year ago
It can be as high quality report!
Defined Variables Used Only Once Minor memory savings (Storage is packed into 1 slot), most likely 3 for memory * 2 + some extra math for unpacking, maybe 20 gas 26
Redundant Use of Variable 6 gas
Struct Member can be Packed into Fewer Storage Slots 4k
Unchanging State Variable Should be Immutable Ignoring as mostly off (1 / 5 is correct)
X = X + Y costs less gass than X += Y 34
Use require instead of && 3
Change Function Visibility Public to External Skipping
Internal Function Called Only Once can be Inlined 21*24 = 504
Use Calldata instead of Memory for Read Only Function Parameters Some will not compile, skipping for lack of detail
Using Elements Smaller than 32 bytes (256 bits) Might Use More Gas Skipping
Empty Blocks Should Emit Something or be Removed Same
4573
GalloDaSballo marked the issue as grade-b
Table of Contents
Defined Variables Used Only Once
Issue
Certain variables is defined even though they are used only once. Remove these unnecessary variables to save gas. For cases where it will reduce the readability, one can use comments to help describe what the code is doing.
PoC
Remove
oldFacet
variable of _addFunctions function of Diamond.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/libraries/Diamond.sol#L134-L135 Mitigation: Delete line 134 and replace line 135 with below codeRemove
selectorsLength
variable of _saveFacetIfNew function of Diamond.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/libraries/Diamond.sol#L187-L189 Mitigation: Delete line 187 and replace line 189 with below codeMitigation
Don't define variable that is used only once. Details are listed on above PoC.
Redundant Use of Variable
Issue
Below has redundant use of variables. Instead of defining a new variable, emit the event first and then update the variable.
PoC
Total of 2 instances found.
acceptGovernor function of Governance.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Governance.sol#L28-L40 Change it to
setVerifierParams function of Governance.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Governance.sol#L104-L109 Change it to
Mitigation
Instead of defining a new variable, emit the event first and then update the variable like shown in above PoC.
Struct Member can be Packed into Fewer Storage Slots
Issue
The order of struct member can be reordered in a way to reduce the usage amount of storage slots.
https://docs.soliditylang.org/en/v0.8.15/internals/layout_in_storage.html#layout-of-state-variables-in-storage
PoC
Total of 2 instances found.
struct
StoredBlockInfo
of IExecutor.sol Originally it uses 8 slots but this can be reduced to 7 slots by reordering the struct member like below. SwitchindexRepeatedStorageChanges
(8 bytes) andblockHash
(32 bytes) order, so thatblockNumber
(8 bytes) andindexRepeatedStorageChanges
can fit into 1 slot. https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/interfaces/IExecutor.sol#L17-L18struct
AppStorage
of Storage.sol Originally it uses 16 slots but this can be reduced to 15 slots by reordering the struct member like below. Move bool variablezkPorterIsAvailable
(1 bytes) to bottom of address varialbependingGovernor
(20 bytes), so that these 2 variable can be packed into 1 slot. https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/Storage.sol#L69-L105Mitigation
Reorder struct member like shown in above PoC.
X = X + Y costs less gass than X += Y
Issue
X = X + Y costs less gass than X += Y for state variables
PoC
Total of 1 instance found.
Unchanging State Variable Should be Immutable
Issue
State variable that is only set in the constructor and can't be changed afterwards, should be declared as immutable.
PoC
Total of 5 instances found.
l2Bridge variable of L1ERC20Bridge.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/bridge/L1ERC20Bridge.sol#L47
l2TokenFactory variable of L1ERC20Bridge.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/bridge/L1ERC20Bridge.sol#L50
l2ProxyTokenBytecodeHash variable of L1ERC20Bridge.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/bridge/L1ERC20Bridge.sol#L53
l2Bridge variable of L1EthBridge.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/bridge/L1EthBridge.sol#L44
l2TokenFactory variable of L2ERC20Bridge.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/zksync/contracts/bridge/L2ERC20Bridge.sol#L23
Mitigation
Change to immutable.
Use require instead of &&
Issue
When there are multiple conditions in require statement, break down the require statement into multiple require statements instead of using && can save gas.
PoC
Mitigation
Break down into several require statement.
Change Function Visibility Public to External
Issue
If the function is not called internally, it is cheaper to set your function visibility to external instead of public.
PoC
Total of 1 instance found.
Mailbox.sol:l2TransactionBaseCost() https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Mailbox.sol#L89
Mitigation
Change the function visibility to external.
Internal Function Called Only Once Can be Inlined
Issue
Certain function is defined even though it is called only once. Inline it instead to where it is called to avoid usage of extra gas.
PoC
Total of 21 instances found.
uncheckedAdd() of UncheckedMath.sol This function called only once at line 111 of Executor.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/common/libraries/UncheckedMath.sol#L12 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L111
_L2MessageToLog() of Mailbox.sol This function called only once at line 32 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Mailbox.sol#L75 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Mailbox.sol#L32
_requestL2Transaction() of Mailbox.sol This function called only once at line 113 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Mailbox.sol#L116 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Mailbox.sol#L113
_writePriorityOp() of Mailbox.sol This function called only once at line 131 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Mailbox.sol#L144 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Mailbox.sol#L131
calculateRoot() of Merkle.sol This function called only once at line 68 of Mailbox.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/libraries/Merkle.sol#L17 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Mailbox.sol#L68
_depositFunds() of L1ERC20Bridge.sol This function called only once at line 116 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/bridge/L1ERC20Bridge.sol#L136 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/bridge/L1ERC20Bridge.sol#L116
_getERC20Getters() of L1ERC20Bridge.sol This function called only once at line 155 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/bridge/L1ERC20Bridge.sol#L164 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/bridge/L1ERC20Bridge.sol#L155
getSize() of PriorityQueue.sol This function called only once at line 65 of Getters.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/libraries/PriorityQueue.sol#L44 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Getters.sol#L65
pushBack() of PriorityQueue.sol This function called only once at line 165 of Mailbox.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/libraries/PriorityQueue.sol#L54 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Mailbox.sol#L165
front() of PriorityQueue.sol This function called only once at line 70 of Getters.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/libraries/PriorityQueue.sol#L63 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Getters.sol#L70
popFront() of PriorityQueue.sol This function called only once at line 181 of Executor.sol https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/libraries/PriorityQueue.sol#L71 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L181
_deployL2Token() of L2ERC20Bridge.sol This function called only once at line 62 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/zksync/contracts/bridge/L2ERC20Bridge.sol#L73 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/zksync/contracts/bridge/L2ERC20Bridge.sol#L62
_collectOperationsFromPriorityQueue() of Executor.sol This function called only once at line 198 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L177 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L198
_executeOneBlock() of Executor.sol This function called only once at line 211 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L190 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L211
_getBlockProofPublicInput() of Executor.sol This function called only once at line 248 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L274 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L248
_verifyRecursivePartOfProof() of Executor.sol This function called only once at line 264 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L296 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L264
_maxU256() of Executor.sol This function called only once at line 338 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L349 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L338
_createBlockCommitment() of Executor.sol This function called only once at line 65 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L354 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L65
_blockPassThroughData() of Executor.sol This function called only once at line 355 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L362 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L355
_blockMetaParameters() of Executor.sol This function called only once at line 356 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L372 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L356
_blockAuxilaryOutput() of Executor.sol This function called only once at line 357 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L376 https://github.com/code-423n4/2022-10-zksync/blob/456078b53a6d09636b84522ac8f3e8049e4e3af5/ethereum/contracts/zksync/facets/Executor.sol#L357
Mitigation
I recommend to not define above functions and instead inline it at place it is called.
Use Calldata instead of Memory for Read Only Function Parameters
Issue
It is cheaper gas to use calldata than memory if the function parameter is read only. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory. More details on following link. link: https://docs.soliditylang.org/en/v0.8.15/types.html#data-location
PoC
Total of 5 instances found.
Mitigation
Change memory to calldata
Using Elements Smaller than 32 bytes (256 bits) Might Use More Gas
Issue
Since EVM operates on 32 bytes at a time, if the element is smaller than that, the EVM must use more operations in order to reduce the elements from 32 bytes to specified size. Therefore it is more gas saving to use 32 bytes unless it is used in a struct or state variable in order to reduce storage slot.
Reference: https://docs.soliditylang.org/en/v0.8.15/internals/layout_in_storage.html
PoC
Total of 13 instances found.
Mitigation
I suggest using uint256 instead of anything smaller and downcast where needed.
Empty Blocks Should Emit Something or be Removed
Issue
There are function with empty blocks. These should do something like emit an event or reverting. If not it should be removed from the contract.
PoC
Total of 2 found.
Save 23877 gas by deleting empty constructor() of DiamondInit.sol Before gas avg: 293178 After gas avg: 269301
Delete IZkySync.sol contract since it is empty and unnecessary contract.