We performed a diff audit of the zksync-association/zk-governance repository at commit 27763f1 against commit 08ec4e7.
In scope were the following files:
src
├── ZkMerkleDistributor.sol
├── ZkTokenV1.sol
├── interfaces
│ ├── IMintable.sol
│ └── IMintableAndDelegatable.sol
└── lib
└── Nonces.sol
We also performed a full audit of the zksync-association/zk-governance repository at commit 410452d for the following file:
src
└── ZkTokenV2.sol
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 addition to the functionalities described in the previous audit report, the system contains the following updates.
ZkTokenV1 ContractA new function was added to allow delegating votes from a signer to a delegatee, adding EIP-1271 support in addition to the ECDSA signature.
ZkMerkleDistributor ContractThe claiming approach has been updated. Users now have more options while claiming their tokens. Previously, users could claim and delegate simultaneously or they could claim on behalf of others and delegate simultaneously. The updated version adds two new separate functions to claim tokens, one to claim on behalf of others and one to claim for yourself, but without any voting rights delegation.
ZkTokenV2 ContractThis additional token contract extends from the V1 contract and was introduced with the sole purpose of renaming the token from zkSync to ZKsync, since the token was already deployed at the time of reviewing, according to the ZKsync Association team. No further features were added to it.
When claiming tokens on behalf of another account using claimOnBehalf or claimAndDelegateOnBehalf, the claim information includes an expiry field which indicates at which block.timestamp this signature would expire. In case the block.timestamp is equal to or larger than the expiry ([1] [2]), the claiming operation will fail.
When delegating tokens using claimAndDelegate or claimAndDelegateOnBehalf, the delegate information includes a similar expiry field. However, in this case, the delegation will only fail in case the block.timestamp is strictly greater than the expiry.
Consider modifying the signature expiry verification process for either the claiming signature or the delegation signature in order to make it consistent for both types of signatures.
Update: Resolved at commit 410452d. The signature expiry verification processes of both the claiming signature and delegation signature are now consistent with each other. They both check whether the expiration timestamp is strictly smaller than the current block.timestamp.
Throughout the codebase, a few instances of incorrect or misleading documentation were identified:
ZkTokenV1 contract points out that the nonce used in both the delegateBySig and permit functions is the same. Consider mentioning that the same nonce is also used in the delegateOnBehalf function._claim function suggests that this function is only called internally by claim and claimOnBehalf. However, it is also called by claimAndDelegate and claimAndDelegateOnBehalf. Consider mentioning this in the documentation.ZkMerkleDistributor__ClaimAmountExceedsMaximum error states that this error is thrown when the total amount of claimed tokens exceeds the total amount claimed. However, the error should be thrown when the total amount claimed exceeds the maximum claimable amount.claimAndDelegate function suggests that this method cannot be called by smart accounts because it is using the signature parameter that is passed to the delegateOnBehalf function of the ZkTokenV1 contract. However, the delegateOnBehalf function does support EIP-1271 and ECDSA signatures, thereby supporting smart accounts as well. Additionally, consider changing "smart contract accounts" to "smart accounts" to be in conformity with the official zkSync documentation.ZkMerkleDistributor__ClaimWindowNotOpen error, "or" should be removed.Clear and accurate documentation helps users and developers understand the codebase. Consider reviewing all the documentation and updating any incorrect and/or misleading statements.
Update: Resolved at commit 410452d. All the instances of incorrect or misleading documentation have been addressed.
The following instance of unused imports was identified and can be removed:
ERC20PermitUpgradeable import in ZkTokenV1.solConsider removing any unused imports to improve the overall clarity and readability of the codebase.
Update: Resolved at commit 410452d. The ERC20PermitUpgradeable import has been removed.
Throughout the codebase, the following instances of code could benefit from gas cost optimization:
delegateOnBehalf function in ZkTokenV1 will copy the _signature data to memory.claimAndDelegate and claimAndDelegateOnBehalf functions in ZkMerkleDistributor will copy the DelegateInfo struct to memory.The arguments mentioned above are read-only as none of their values are being modified within the functions. Consider changing the locations of these arguments from memory to calldata in order to reduce the gas amount needed to execute the functions.
Update: Resolved at commit 410452d. All the suggested gas optimizations have been applied on the codebase.
This update adds EIP-1271 support to the ZkTokenV1 and ZkMerkleDistributor contracts in order to expand claiming and voting delegation options using the signatures of smart accounts. In addition, as a result of updates made to the claiming procedure, users now have more flexibility when claiming their tokens.
The audit yielded one low-severity issue along with a few recommendations for code improvement. The codebase is well-written, straightforward to follow, and well-documented. The codebase is a collaborative effort between ZKsync Association and ScopeLift. Both teams were very responsive throughout the engagement and answered all our questions.