ZK Stack VM1.5 Diff Audit

Table of Contents

Summary

Type

Layer 2

Timeline

From 2024-03-25

To 2024-04-08

Languages

Solidity, Yul

Total Issues

7 (7 resolved)

Critical Severity Issues

2 (2 resolved)

High Severity Issues

0 (0 resolved)

Medium Severity Issues

0 (0 resolved)

Low Severity Issues

2 (2 resolved)

Notes & Additional Information

3 (3 resolved)

Client Reported Issues

0 (0 resolved)

Scope

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

System Overview

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.

Critical Severity

Invalid Gas Accounting

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.

Skipped Transaction Processing

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.

Low Severity

Misleading Comments

We have identified the following examples of misleading comments:

• The gasBoundCall function description mentions a BOUND_CALL_OVERHEAD constant, which does not exist in the codebase.
• The sendToL1 function contains an inline comment that refers to removed code.
• The _processL2Logs function contains an outdated comment describing the wrong number of logs.
• The 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.
• The askOperatorForRefund function comment no longer describes the correct inputs.
• The 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.

Missing Docstrings

Throughout the codebase, there are several values that do not have docstrings:

• The new _pubdataToSpend parameter (1, 2).
• Both parameters for the setPubdataInfo function. The description also only covers the first parameter.
• The new gas parameters (1, 2, 3, 4, 5, 6, 7, 8, 9, 10).

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.

Notes & Additional Information

Typographical errors

We identified the following typographical errors. Consider correcting them.

• "wouldbn't" should be "wouldn't".
• both instances (1, 2) of "a most" should be "at most".
• "decomit" should be "decommit".
• "not" should be "no".
• "baseSepnt" should be "baseSpent".

Update: Resolved in pull request #347.

Naming suggestions

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.

Code simplifications

Here are some suggestions for code simplifications:

• Instead of copying values individually from calldata, a single calldatacopy instruction could be used.
• Instead of using an exclusive upper bound to identify blob hash keys, using the last relevant key as the upper bound would be clearer.
• There are multiple places (1, 2, 3) where the new saturatingSub function could be used.

Update: Resolved in pull request #349.

Conclusion

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.