From 58ae2cad41dd4c248d7563d71f50c1c1faea7ada Mon Sep 17 00:00:00 2001 From: Zach Kolodny Date: Thu, 16 May 2024 20:15:07 -0400 Subject: [PATCH] timelock and diamond init --- .codespell/wordlist.txt | 1 + .../contracts/common/L1ContractErrors.sol | 2 + .../state-transition/ValidatorTimelock.sol | 13 +++++-- .../chain-deps/DiamondInit.sol | 37 ++++++++++++++----- 4 files changed, 41 insertions(+), 12 deletions(-) diff --git a/.codespell/wordlist.txt b/.codespell/wordlist.txt index eb14cf834..797a49365 100644 --- a/.codespell/wordlist.txt +++ b/.codespell/wordlist.txt @@ -4,3 +4,4 @@ ue tha bu crate +DNE diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index e0d532cf5..c897c040b 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -41,6 +41,8 @@ error InvalidDelay(); error PreviousOperationNotExecuted(); error HashMismatch(bytes32 expected, bytes32 actual); error HyperchainLimitReached(); +error TimeNotReached(); +error TooMuchGas(); enum SharedBridgeKey { PostUpgradeFirstBatch, diff --git a/l1-contracts/contracts/state-transition/ValidatorTimelock.sol b/l1-contracts/contracts/state-transition/ValidatorTimelock.sol index 75e5c2176..d2ad9de5f 100644 --- a/l1-contracts/contracts/state-transition/ValidatorTimelock.sol +++ b/l1-contracts/contracts/state-transition/ValidatorTimelock.sol @@ -6,6 +6,7 @@ import {Ownable2Step} from "@openzeppelin/contracts/access/Ownable2Step.sol"; import {LibMap} from "./libraries/LibMap.sol"; import {IExecutor} from "./chain-interfaces/IExecutor.sol"; import {IStateTransitionManager} from "./IStateTransitionManager.sol"; +import {Unauthorized, TimeNotReached} from "../common/L1ContractErrors.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -62,13 +63,17 @@ contract ValidatorTimelock is IExecutor, Ownable2Step { /// @notice Checks if the caller is the admin of the chain. modifier onlyChainAdmin(uint256 _chainId) { - require(msg.sender == stateTransitionManager.getChainAdmin(_chainId), "ValidatorTimelock: only chain admin"); + if (msg.sender != stateTransitionManager.getChainAdmin(_chainId)) { + revert Unauthorized(msg.sender); + } _; } /// @notice Checks if the caller is a validator. modifier onlyValidator(uint256 _chainId) { - require(validators[_chainId][msg.sender], "ValidatorTimelock: only validator"); + if (!validators[_chainId][msg.sender]) { + revert Unauthorized(msg.sender); + } _; } @@ -203,7 +208,9 @@ contract ValidatorTimelock is IExecutor, Ownable2Step { // * The batch wasn't committed at all, so execution will fail in the zkSync contract. // We allow executing such batches. - require(block.timestamp >= commitBatchTimestamp + delay, "5c"); // The delay is not passed + if (block.timestamp < commitBatchTimestamp + delay) { + revert TimeNotReached(); + } } } _propagateToZkSyncHyperchain(_chainId); diff --git a/l1-contracts/contracts/state-transition/chain-deps/DiamondInit.sol b/l1-contracts/contracts/state-transition/chain-deps/DiamondInit.sol index 424223396..663cf260a 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/DiamondInit.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/DiamondInit.sol @@ -6,6 +6,7 @@ import {Diamond} from "../libraries/Diamond.sol"; import {ZkSyncHyperchainBase} from "./facets/ZkSyncHyperchainBase.sol"; import {L2_TO_L1_LOG_SERIALIZE_SIZE, MAX_GAS_PER_TRANSACTION} from "../../common/Config.sol"; import {InitializeData, IDiamondInit} from "../chain-interfaces/IDiamondInit.sol"; +import {ZeroAddress, TooMuchGas} from "../../common/L1ContractErrors.sol"; /// @author Matter Labs /// @dev The contract is used only once to initialize the diamond proxy. @@ -18,15 +19,33 @@ contract DiamondInit is ZkSyncHyperchainBase, IDiamondInit { /// @return Magic 32 bytes, which indicates that the contract logic is expected to be used as a diamond proxy /// initializer function initialize(InitializeData calldata _initializeData) external reentrancyGuardInitializer returns (bytes32) { - require(address(_initializeData.verifier) != address(0), "vt"); - require(_initializeData.admin != address(0), "vy"); - require(_initializeData.validatorTimelock != address(0), "hc"); - require(_initializeData.priorityTxMaxGasLimit <= MAX_GAS_PER_TRANSACTION, "vu"); - require(_initializeData.bridgehub != address(0), "DiamondInit: b0"); - require(_initializeData.stateTransitionManager != address(0), "DiamondInit: stm0"); - require(_initializeData.baseToken != address(0), "DiamondInit: bt0"); - require(_initializeData.baseTokenBridge != address(0), "DiamondInit: btb0"); - require(_initializeData.blobVersionedHashRetriever != address(0), "DiamondInit: bvhr0"); + if (address(_initializeData.verifier) == address(0)) { + revert ZeroAddress(); + } + if (_initializeData.admin == address(0)) { + revert ZeroAddress(); + } + if (_initializeData.validatorTimelock == address(0)) { + revert ZeroAddress(); + } + if (_initializeData.priorityTxMaxGasLimit > MAX_GAS_PER_TRANSACTION) { + revert TooMuchGas(); + } + if (_initializeData.bridgehub == address(0)) { + revert ZeroAddress(); + } + if (_initializeData.stateTransitionManager == address(0)) { + revert ZeroAddress(); + } + if (_initializeData.baseToken == address(0)) { + revert ZeroAddress(); + } + if (_initializeData.baseTokenBridge == address(0)) { + revert ZeroAddress(); + } + if (_initializeData.blobVersionedHashRetriever == address(0)) { + revert ZeroAddress(); + } s.chainId = _initializeData.chainId; s.bridgehub = _initializeData.bridgehub;