ZK Token, Capped Minter, and Merkle Distributor Audit

Table of Contents

Summary

Type
Token
Timeline
From 2024-03-11
To 2024-03-22
Languages
Solidity
Total Issues
13 (10 resolved, 1 partially resolved)
Critical Severity Issues
0 (0 resolved)
High Severity Issues
0 (0 resolved)
Medium Severity Issues
0 (0 resolved)
Low Severity Issues
6 (6 resolved)
Notes & Additional Information
7 (4 resolved, 1 partially resolved)

Scope

We audited the zksync-association/zk-governance repository at commit 08ec4e7.

In scope were the following files:

 src
├── ZkCappedMinter.sol
├── ZkMerkleDistributor.sol
└── ZkTokenV1.sol

Note that some issues might refer to the f37e3dc commit which was the audit's early commit until the final one was delivered.

System Overview

The ZkTokenV1 contract is the core of the system and is going to be the governance token of ZKsync Era. It is an ERC-20 token with permit functionality which incorporates a voting delegation system so that token holders can delegate their voting power to a trusted representative while preserving the actual token value. In order to activate their voting power, each token holder must set a delegatee even if they are delegating to their own account address. Upon a token transfer, the corresponding voting power is also automatically transferred from and to the corresponding delegatees, respectively.

The token contract is a transparent upgradeable proxy and also has an access control mechanism. The default admin role is allowed to create other access roles, assign or revoke these roles, and also set a separate and dedicated admin role for each role. By default, all defined roles are managed by the default admin role.

In order to bootstrap the token's circulation, a specific amount of tokens will be minted and distributed among Matter Labs, the Foundation, and the Association, with an airdrop taking place as well. The token distribution involves two more contracts, namely ZkCappedMinter and ZkMerkleDistributor. These contracts are going to be assigned the MINTER role in the access control system of ZkTokenV1 and will control the token minting among the beneficiaries.

More specifically, two instances of the ZkCappedMinter contract will be deployed: one for the Foundation and another for the Association. ZkCappedMinter defines an immutable admin and an immutable cap value. The admin is able to mint tokens to any address at any frequency, up to the token mint cap.

The ZkMerkleDistributor contract will be used to control token distribution among the beneficiaries of the public airdrop. An immutable Merkle root value will be set upon the contract's deployment, committing to all information about the qualifying addresses and amounts. Users will be allowed to claim their airdropped tokens within a specific time window. In order to claim successfully, users should provide a valid Merkle proof and a delegatee account address. When the specified claiming period is over, the contract admin is allowed to sweep any airdrop amount left unclaimed and transfer it to an address of their preference.

Security Model and Trust Assumptions

The system's security ultimately relies on the honest behavior of the initial admin of ZkTokenV1. Upon the contract's initialization, the admin is granted the DEFAULT_ADMIN_ROLE, the MINTER_ADMIN_ROLE, and the BURNER_ADMIN_ROLE roles, along with the ownership of the proxy. As a consequence, the admin is allowed to grant or revoke token minting and burning rights to any address. The admin also can upgrade the token by changing the implementation address.

The ZKsync Association team clarified that the initial admin will be the Association's 4-out-of-7 multisig with a security precaution of having one key per person, with each key being a hardware key and not being used for any other purpose. According to the plan, once the governance system has been successfully bootstrapped and the proposals procedure is functional, all three admin roles will be handed over to a governance contract and the Association will subsequently lose all three roles. During the audit, we considered that the Association will act honestly and in accordance with the interests of the community until the admin roles are handed over to the governance.

Regarding the distribution of the token's supply, an initial amount of funds will be minted upon contract initialization for Matter Labs. There will be two instances of the ZkCappedMinter contract, one with the Association being the admin and the other with the Foundation being the admin. The ZkCappedMinter contract will control the minting for these two entities, allowing them to mint up to a defined cap amount at the frequency and the recipient(s) of their preference. The airdrop distribution will be fully controlled by the ZkMerkleDistributor contract, where an immutable Merkle root will be the only source of truth regarding the eligible accounts and the corresponding amounts. The ZKsync Association team shared their willingness to publish the data required to reconstruct the Merkle tree so that anyone can verify the correctness of the Merkle root.

 

Low Severity

The Domain of The Permit Functionality Is Not Initialized

The initialize function of the ZkTokenV1 contract does not call the initializer of the ERC20PermitUpgradeable base contract. This could impact user experience when approving using the permit functionality as there will be no readable name for the signing domain.

Consider initializing the ERC20PermitUpgradeable base contract.

Update: Resolved at commit 63bdcc9.

Claim Cap Is Not Enforced on Merkle Distributor

The _claim function is supposed to verify that the total claimed amount is below the maximum total claimable amount according to the specification. However, it checks a particular amount instead.

Consider checking the total claimed amount instead of a particular amount.

Update: Resolved at commit f9b1389.

Claim Can Be Prevented by Anyone if Front-Ran

Both the claim and claimOnBehalf functions delegate the claimed amount at the end of their execution. However, it is possible to extract the _delegateInfo parameter and front-run this transaction by calling the delegateBySig function on the token directly. This way, the nonce is incremented due to which the whole claim transaction reverts when the delegation is attempted.

Consider redesigning the requirement for delegation upon claim or calling the delegateBySig function in a try-catch block.

Update: Resolved at commit e2f3209.

Not All Delegation Parameters Are Signed

From all the DelegateInfo parameters, only the delegatee parameter is signed. As such, it is possible to front-run the transaction, change the signature to oneself while keeping the delegatee, and claim the airdrop but skip the original delegation. This way, the original transaction is fulfilled only partially because the airdrop is correctly claimed but the delegation is not correctly set.

Consider signing all the DelegateInfo parameters.

Update: Resolved at commits d2116b3 and 36c6145. All the DelegateInfo parameters are signed except for the expiry which is ignored in favour of the claim signature's expiry.

Voting Checkpoints Refer to Block Numbers Despite Unstable Block Production Frequency

The ZkTokenV1 is the governance token that will be used for voting during an open governance proposal. For vote counting, the governor will read the users' voting power checkpoint with respect to the associated voting period. According to the current default, checkpoints are created per block numbers which will, in turn, enforce the proposals' voting period to be expressed in block numbers. However, in ZKsync Era, the block production time is not stable and it may be confusing for users to predict the voting checkpoint so as to vote in time.

In order to avoid confusion and enhance user experience, consider modifying the vote checkpoint unit to block timestamp.

Update: Resolved at commit a4a45fe.

Problematic Edge Cases When Using delegateBySig

To encourage governance participation through delegation, the delegateBySig function is called together with a token claim. However, delegateBySig works only for EOA signatures and does not support ERC-1271 nor account abstraction due to backwards compatibility reasons. Given that account abstraction is common on ZKsync Era, the feature might not work as expected for some accounts.

Consider choosing another way of encouraging governance participation that will support EOAs as well as smart contracts and account abstraction.

Update: Resolved at commit 4587190.

Notes & Additional Information

Initializer Is Not Disabled on Implementation

The ZkTokenV1 implementation is left uninitialized which allows anyone to initialize it with arbitrary values.

While this does not lead to security issues in this case, consider calling the _disableInitializers function in ZkTokenV1's constructor to disable the initializer.

Update: Acknowledged, not resolved. The ScopeLift team stated:

When deployed, the token contract will be initialized in the same transaction.

Same nonces Mapping Is Used for Vote Delegation and Permit

The ERC20VotesUpgradeable and ERC20PermitUpgradeable base contracts of ZkTokenV1 use the same nonces mapping. As a consequence, it would be invalid to include the same nonce in a delegate-by-signature message and in a subsequent permit message or vice versa.

Consider clearly documenting that the nonces produced for both delegation and permit actions should respect a single order so as to avoid failing transactions.

Update: Resolved at commit 604fdf9. Further docstrings were added to clarify how to use the nonces of both actions correctly.

Missing Input Validation

The _windowStart timestamp must be less than _windowEnd. Otherwise, the ZkMerkleDistributor contract will not work according to the specification. However, this is not enforced in the contract.

Consider verifying that the start timestamp is less than the end timestamp in the constructor.

Update: Acknowledged, not resolved. The ScopeLift team stated:

If misconfigured, which is unlikely, we will deploy a new contract.

Code Clarity Suggestions

Throughout the codebase, a number of instances were identified where code clarity could be improved:

Update: Partially resolved at commit 604fdf9. The claimAndDelegateOnBehalf function does not use the toTypedDataHash function.

Incomplete Docstrings

Within ZkMerkleDistributor.sol, not all return values of the isClaimed function are documented.

Consider thoroughly documenting all functions/events (and their parameters or return values) that are part of a contract's public API. When writing docstrings, consider following the Ethereum Natural Specification Format (NatSpec).

Update: Resolved at commit 604fdf9.

Floating Pragma

Pragma directives should be fixed to clearly identify the Solidity version with which the contracts will be compiled.

Throughout the codebase, there are multiple floating pragma directives:

Consider using fixed pragma directives.

Update: Resolved at commit 604fdf9.

Lack of Security Contact

Providing a specific security contact (such as an email or ENS name) within a smart contract significantly simplifies the process for individuals to communicate if they identify a vulnerability in the code. This practice proves beneficial as it permits the code owners to dictate the communication channel for vulnerability disclosure, eliminating the risk of miscommunication or failure to report due to a lack of knowledge on how to do so. In addition, if the contract incorporates third-party libraries and a bug surfaces in those, it becomes easier for their maintainers to contact the appropriate person about the problem and provide mitigation instructions.

Throughout the codebase, there are contracts that do not have a security contact:

Consider adding a NatSpec comment containing a security contact above the contract definitions. Using the @custom:security-contact convention is recommended as it has been adopted by the OpenZeppelin Wizard and the ethereum-lists.

Update: Resolved at commit 604fdf9.

Recommendations

Deployment Suggestions

The system uses the OpenZeppelin/openzeppelin-foundry-upgrades library for deployments. This library was developed to be compatible with EVM chains but was not thoroughly tested for ZKsync Era deployments.

Given that ZKsync Era has some differences compared to Ethereum and uses its own versions of the Foundry toolkit and the Solidity compiler, consider thoroughly testing the deployment on ZKsync Era testnet before deploying to the ZKsync Era mainnet.

The ScopeLift team responded:

We modified our deployment scripts to use hardhat with ZKsync's plugins (at commit a26da8a). This change required us to downgrade to OZ Contracts v4.9.

As a result of the downgrade to OZ Contracts v4.9, OpenZeppelin assessed the degree of the changes and did not identify any issues. The code level changes of the library contracts used in this project are backwards compatible.

Conclusion

The audit only yielded low-severity issues which indicates an overall healthy design and implementation. Several fixes were suggested to address edge cases and improve the usability, robustness, and clarity of the codebase. The in-scope contracts were inspected, taking into consideration the integrated libraries and the specifics of the ZKsync Era ecosystem.

The ScopeLift team, who developed the codebase for ZKsync Association, provided a detailed specification of the system and sufficiently covered the codebase with tests. Both ZKsync Association and ScopeLift teams were very responsive and engaged in discussions with the audit team.

Request Audit