We audited the matter-labs/system-contracts repository for pull request #285 at commit 44c4f98. The following files were in scope:
├── bootloader
│ └── bootloader.yul
└── contracts
├── ContractDeployer.sol
├── SystemContext.sol
└── interfaces
├── ISystemContext.sol
└── ISystemContextDeprecated.sol
Additionally, we audited the matter-labs/zksync-2-contracts repository for pull request #169 at commit 661a11c. The following file was in scope:
ethereum
└── contracts
└── zksync
└── facets
└── Executor.sol
The system upgrade introduces changes to provide more accurate and efficient information about L2 blocks for faster soft confirmations on wallets and block explorers. The naming conventions have been revised, distinguishing between batches (now referred to as "batch" as opposed to "block") and individual L2 blocks, which are now denoted as "L2 blocks". The compiler functions getBlockNumber, getBlockTimestamp, and getBlockHashEVM have been adapted to return values specific to L2 blocks.
To process and ensure consistency within L2 blocks, the system implements the setL2Block method before each transaction to provide data about the L2 block to which the transaction belongs. The hash of an L2 block is derived based on its properties, including block number, timestamp, previous L2 block hash, and transaction rolling hash. Timing invariants are maintained to ensure that each L2 block's timestamp is greater than the previous L2 block's timestamp and the timestamp of the batch it belongs to. Additionally, batches must start with a new L2 block, and the timestamp of the last miniblock in a batch must not be too far into the future. The system also accommodates an empty, fictive L2 block at the end of each batch to allow for technical requirements within the node. These upgrades aim to improve the user experience and provide more accurate and timely information for L2 block confirmations.
This upgrade does not introduce new roles to the system. As before, the operator role is in control of the batches, in addition to controlling the block frequency within them. Currently, the operator will be centralized through Matter Labs with the goal of decentralizing it in the medium-term future. The operator has the privilege to include or exclude any transaction they choose.
The SystemContext system contract is used to maintain context information that can be accessed from contract code. One of these values is the block difficulty, which is hard-coded to 25 * 1015.
This breaks EVM equivalence where block.difficulty is defined as the previous RANDAO value of the consensus layer as per the EIP-4399 specification. This can result in unexpected contract misbehavior which utilizes block difficulty as a weak source of randomness. For instance, in a naïve casino dApp this could lead to one-sided odds and hence a loss of funds.
Note that since Solidity version 0.8.18, block.difficulty is considered deprecated in favor of block.prevrandao.
Consider conforming to the EIP-4399 properties of block.difficulty / block.prevrandao to match the EVM's behavior.
Update: Acknowledged, not resolved. The Matter Labs team stated:
Due to the difficulty of the implementation of the fix, it was postponed to one of the future milestones. Note that the discrepancy is common among L2s and has been documented on the Differences from Ethereum page.
Some of the comments communicate incorrect or misleading information. For instance:
TX_OPERATOR_L2_BLOCK_INFO_SLOTS constant description says "Note, that an additional slot is required for the fictive block at the end of the block" while it should say "[...] fictive block at the end of the batch".SystemContext on line 294 mismatches the message string and comment: The condition checks "greater than or equal to" while the message string and comment say "greater than".publicBatchDataToL1 function debug logs state that block data is published to L1, although it is referring to batch data.getBatchNumberAndTimestamp function refer to block data, although batch is meant.block in several places, but it should be batch.Consider changing the comments to reflect the intention of the code.
Update: Resolved in pull request #304 at commit 3d9a4fa.
Within the scope, there are two instances where docstrings are missing:
_previousBlockTimestamp parameter is not documented for the _verifyBlockTimestamp function._baseFee parameter is not documented for the setNewBatch function.Consider thoroughly documenting all functions (and their parameters) that are part of any contract's public API. Functions implementing sensitive functionality, even if not public, should be clearly documented as well. When writing docstrings, consider following the Ethereum Natural Specification Format (NatSpec).
Update: Resolved in pull request #305 at commit 19c5200 and pull request #197 at commit dc82a21.
require StatementWithin SystemContext.sol, there is a require statement on line 17 that lacks an error message.
Consider including specific, informative error messages in require statements to improve overall code clarity and facilitate troubleshooting whenever a requirement is not satisfied.
Update: Resolved in pull request #306 at commit d44a675.
Throughout the codebase, variable naming related to "batch" and "block" terminology is mixed together. For instance:
SystemContext on lines 103-107Executor on line 84Bootloader on line 141, line 166, line 409, line 3334, and line 3414Consider consolidating terminology in all instances where "block" refers to "batch".
Update: Resolved in pull request #307 at commit daa4020 and pull request #198 at commit c4efbe7.
Within Executor.sol, global imports are being used. The use of non-explicit imports in the codebase can decrease the clarity of the code and may create naming conflicts between locally defined and imported variables. This is particularly relevant when multiple contracts exist within the same Solidity files or when inheritance chains are long.
Following the principle that clearer code is better code, consider using named import syntax (import {A, B, C} from "X") to explicitly declare which contracts are being imported.
Update: Resolved in pull request #199 at commit 154ce74.
Throughout the scope, the following typographical errors were found:
Consider fixing the above errors to improve the clarity and readability of the codebase.
Update: Resolved in pull request #308 at commit e0f6798 and pull request #200 at commit 7f7b6c6.
In the Executor facet, the following instances of gas optimization were identified:
>>) is cheaper than a division with a power of two.&) is cheaper than a modulo with a power of two.Consider changing to these bitwise operations to save gas.
Update: Resolved in pull request #201 at commit bbaa2f0.
The NUMBER or 0x43 opcode returns the current block number on Ethereum and zkSync Era. While on Ethereum blocks are produced every 12 seconds, on zkSync Era L2 blocks are produced every 1-2 seconds. However, on zkSync Era the operator controls the frequency of L2 blocks and, hence, can manipulate the block timestamp and block number. Thus, smart contracts that rely on the block number for calculations (e.g., interest rate) are exposed to unexpected behavior considering the Ethereum difference and influence through the operator.
Another peculiarity is related to the PREVRANDAO, DIFFICULTY, or 0x44 opcode which is set to a constant value on zkSync Era but on Ethereum, it gives a pseudo-random value. This might influence applications that rely on this opcode to behave unexpectedly.
Consider adding compile-time warnings or errors when the NUMBER and PREVRANDAO opcodes are used and thoroughly outlining their peculiarities in the documentation to help smart contract developers understand the differences and avoid making false assumptions.
This audit was conducted over the course of a week. One medium-severity issue as well as a few low-severity and noteworthy issues were identified. Some changes were proposed to improve code readability and ensure smart contract security best practices are being followed. We found the dedicated documentation provided by the Matter Labs team to be helpful to understand the audited code changes. The team was also supportive in answering questions in a timely manner.