Short-Term Fee Model Changes Audit

OpenZeppelin Security | January 29, 2024

Table of Contents

Summary

Type
Layer 2
Timeline
From 2023-12-06
To 2023-12-13
Languages
Solidity, Yul
Total Issues
7 (7 resolved)
Critical Severity Issues
0 (0 resolved)
High Severity Issues
0 (0 resolved)
Medium Severity Issues
0 (0 resolved)
Low Severity Issues
4 (4 resolved)
Notes & Additional Information
3 (3 resolved)

Scope

We audited the changes made to the short-term fee model in:

In scope were the following contracts:

Repository era-contracts:

 ethereum
└── contracts
    └── zksync
        ├── facets
        │   ├── Admin.sol
        │   ├── Base.sol
        │   ├── Executor.sol
        │   ├── Getters.sol
        │   └── Mailbox.sol
        ├── interfaces
        │   ├── IAdmin.sol
        │   ├── IExecutor.sol
        │   ├── IGetters.sol
        │   ├── ILegacyGetters.sol
        │   ├── IMailbox.sol
        │   ├── IVerifier.sol
        │   └── IZkSync.sol
        └── libraries
            └── TransactionValidator.sol

Repository era-system-contracts:

 ├── bootloader
│   └── bootloader.yul
└── contracts
    ├── Compressor.sol
    ├── L1Messenger.sol
    └── interfaces
        └── IMailbox.sol

System Overview

The system upgrade introduces changes to the short-term fee model. With the new implementation on Layer 2 (L2), the operator is responsible for providing two variable values: L2 gas price and pubdata price. On Layer 1 (L1), these values are calculated within the contracts. The L2 gas price is expected to include the potential contribution of the usage of a single gas unit for sealing the batch. It is calculated based on the configurable minimal calculation cost, batch overhead, and the gas needed to seal the batch.

The pubdata price is expected to include the potential contribution of the usage of a single pubdata byte for sealing the batch. It is calculated based on the gas price on L1 and the maximum number of pubdata that can be published in a single batch. This provides a high degree of flexibility to the operator and can be adjusted according to market conditions.

Trust Assumptions

This upgrade does not introduce new roles to the system. A new administrative functionality of changing fee parameters for L1-to-L2 transactions has been added, and can only be executed by an account having the Governor role. Thus, any account possessing the Governor role is considered to be a trusted party.

 

Low Severity

Issues Discovered in v1.4.1-integration Diff Audit

The short-term fee model changes are derived from the code present in the era-contract repository, in the v1.4.1-integration branch, at commit 518bfff, and in the era-system-contracts repository, at commit ef0eb0c. Consequently, all issues identified in the v1.4.1-integration audit must be incorporated into the short-term fee model.

Consider merging all fixes from the v1.4.1-integration branch audit into the sb-short-term-fee-model branch of the era-contracts and era-system-contracts repositories.

Update: Resolved in pull request #160 at commit 4e1dfc7 and pull request #105 at commit d85d7d0.

Outdated Version of OpenZeppelin Contracts Library Used

The short-term fee model upgrade involves updating the OpenZeppelin Contracts library from version 4.8.0 to 4.9.2. However, the latest version of the OpenZeppelin Contracts library is 4.9.5 which addresses several security issues.

While these security issues do not directly impact the current implementation of the contracts in scope, consider updating the library to the newest version.

Update: Resolved in pull request #128 at commit bf5905d.

Missing Input Validation

In the Admin contract, the changeFeeParams function, callable by the Governor, modifies the parameters for deriving the gas price in L1-to-L2 transactions. To prevent potential system failure due to misconfiguration, it is recommended to validate whether the provided value for maxPubDataPerBatch exceeds priorityTxMaxPubdata.

Consider validating the input values for maxPubDataPerBatch and priorityTxMaxPubdata to prevent misconfigurations.

Update: Resolved in pull request #129 at commit d59b22d.

Missing Tests

The proposed changes to the short-term fee model lack tests that could confirm the correctness of the implementation. The following areas require additional testing:

Consider adding tests to enhance the quality and safety of the codebase.

Update: Resolved in pull request #95 at commit 80b1869, pull request #131 at commit 85a1b12.

Notes & Additional Information

Base Fee Calculated Outside of Proved Batch

The logic of calling getFeeParams function and calculating the baseFee is implemented outside of the proved batch section. This leads to a scenario whereby the baseFee is calculated for the playground batch as well.

Consider moving the initialization of baseFee and the calling of getFeeParams to the proved batch section.

Update: Resolved in pull request #93 at commit 2546b0a.

Unused Code

Throughout the codebase, there are multiple instances of unused code:

Consider removing all unused code to improve the readability and clarity of the codebase.

Update: Resolved in pull request #94 at commit fc9ba75.

Naming Suggestions

In the Mailbox contract, the parameter _gasPricePerPubdata of the _deriveL2GasPrice function has a misleading name. Despite its current name, it does not represent a price in wei but rather a gas value per pubdata byte. The given name is rather unintuitive and makes the code harder to read.

Consider renaming the _gasPricePerPubdata parameter for improved readability.

Update: Resolved in pull request #130 at commit 9e45fb4.

 

Conclusion

The system upgrade alters the short-term fee model on L2, shifting responsibility to the operator for determining L2 gas price and pubdata values. Unlike L1, these values on L2 factor in sealing batch costs, providing flexibility based on market conditions.

The audit did not reveal any significant issues with the changes made to the short-term fee model. Various recommendations have been made to enhance the quality and documentation of the codebase. We found the dedicated documentation provided by Matter Labs team to be very helpful in understanding the audited code changes. Furthermore, the Matter Labs team was very responsive throughout the audit period and answered any questions we had in a timely manner.

Related Posts