We audited the change to the matter-labs/era-contracts repository between base commit f29f2b7 and head commit 705a4c8.
In scope were the following files:
era-contracts
├── l1-contracts
│ └── contracts
│ ├── common
│ │ └── libraries
│ │ └── L2ContractHelper.sol
│ └── state-transition
│ └── chain-deps
│ └── facets
│ └── Executor.sol
└── system-contracts
├── bootloader
│ └── bootloader.yul
└── contracts
├── Constants.sol
├── ContractDeployer.sol
├── GasBoundCaller.sol
├── L1Messenger.sol
├── MsgValueSimulator.sol
├── PubdataChunkPublisher.sol
├── SystemContext.sol
├── libraries
│ └── SystemContractHelper.sol
└── precompiles
├── CodeOracle.yul
└── P256Verify.yul
This update includes several incremental changes to the codebase. Most importantly, the gas accounting for data published to L1 (pubdata) has been redesigned. This is used for all state changes and contracts deployed to ZKsync Era. Since the operator cost of publishing to L1 depends on the current L1 gas price, operations that necessitate pubdata incur a variable cost. Previously, this was deducted from the call frame's gas limit, like all other opcode gas charges. Under the new scheme, messages sent to L1 are charged a fixed gas cost during execution, but the variable amount is only charged at the end of the transaction.
This removes the variability in gas charges during regular execution, reduces overhead for transactions that change and later restore storage values, and also decouples the pubdata gas from the execution gas. However, this means that excess pubdata would not be detected until after it is used. In this scenario, the corresponding call frame is reverted, so the data no longer needs to be published.
The new mechanic means that only computation is limited by a call frame's gas limit. The new GasBoundCaller system contract has been introduced to support the previous behavior. To achieve this, it wraps the target call, measures the pubdata consumed, and ensures both computation and pubdata costs fit within the limit.
Two new precompiles were introduced: CodeOracle and P256Verify. CodeOracle retrieves the code associated with a specified code hash, if it is known. Since it is already possible to retrieve the code hash at a specified address, this introduces behavior similar to the EVM's extcodecopy instruction. P256Verify implements the RIP-7212 precompile.
All value-bearing calls are directed through the MsgValueSimulator system contract. Since this contract is the one that invokes the actual target contract, it is also charged the gas costs associated with loading the target code into memory (decommitting it). The MsgValueSimulator now automatically receives a gas stipend for this purpose. It also forwards an extra 2300 gas to the target, recreating the corresponding EVM behavior.
Lastly, the codebase has been updated to accept 6 (instead of 2) data blobs per batch of L2 transactions.
It is worth noting that the codebase has independently been generalized to support the hyperchain use case. The code under review contains some associated incidental changes, such as moving administrator privileges to a StateTransitionManager contract and renaming "ETH" to "BASE_TOKEN" where relevant. These changes are explored in a parallel audit.
The bootloader reverses the last two parameters when calling ZKSYNC_NEAR_CALL_callPostOp. This causes the pubdata allowance check to significantly overestimate the pubdata cost and underestimate the gas limit. Consequently, transactions that specify a paymaster and consume pubdata would likely fail this check, incorrectly reverting the postTransaction changes.
Consider correcting the parameter order.
Update: Resolved in pull request #316.
The audit commit inadvertently removed the processL2Tx function invocation. If deployed, the bootloader would not process any L2 transactions. Consider restoring the invocation.
Update: Resolved in pull request #316.
We have identified the following examples of misleading comments:
gasBoundCall function description mentions a BOUND_CALL_OVERHEAD constant, which does not exist in the codebase.sendToL1 function contains an inline comment that refers to removed code._processL2Logs function contains an outdated comment describing the wrong number of logs.computeGas parameter to the isNotEnoughGasForPubdata function is described as "The amount of gas spent on the computation", but it is actually the amount of execution gas remaining that can still be spent on future computation.askOperatorForRefund function comment no longer describes the correct inputs.getPubdataPublishedFromMeta function comment has correctly renamed the return value but still describes the old value. Similarly, the offset constant it uses also refers to the old value.Consider correcting or clarifying these comments.
Update: Resolved in pull request #344.
Throughout the codebase, there are several values that do not have docstrings:
_pubdataToSpend parameter (1, 2).setPubdataInfo function. The description also only covers the first parameter.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 #346.
We identified the following typographical errors. Consider correcting them.
Update: Resolved in pull request #347.
The pubdataPrice and pubdataCost variables are measured in gas, not ETH. For clarity, consider renaming them to pubdataGasRate and pubdataGas respectively.
Update: Resolved in pull request #348.
Here are some suggestions for code simplifications:
calldatacopy instruction could be used.saturatingSub function could be used.Update: Resolved in pull request #349.
Despite the two simple oversights leading to serious issues, we found the changes to be well-motivated and implemented clearly. We appreciate the thorough documentation and Matter Labs' prompt responses to our questions throughout this engagement.