diff --git a/docs/Overview.md b/docs/Overview.md index e33bc2521..67e2fed83 100644 --- a/docs/Overview.md +++ b/docs/Overview.md @@ -44,9 +44,9 @@ even an upgrade system is a separate facet that can be replaced. One of the differences from the reference implementation is access freezability. Each of the facets has an associated parameter that indicates if it is possible to freeze access to the facet. Privileged actors can freeze the **diamond** -(not a specific facet!) and all facets with the marker `isFreezable` should be inaccessible until the governor unfreezes -the diamond. Note that it is a very dangerous thing since the diamond proxy can freeze the upgrade system and then the -diamond will be frozen forever. +(not a specific facet!) and all facets with the marker `isFreezable` should be inaccessible until the governor or its owner +unfreezes the diamond. Note that it is a very dangerous thing since the diamond proxy can freeze the upgrade system and then +the diamond will be frozen forever. #### DiamondInit @@ -56,41 +56,33 @@ diamond constructor and is not saved in the diamond as a facet. Implementation detail - function returns a magic value just like it is designed in [EIP-1271](https://eips.ethereum.org/EIPS/eip-1271), but the magic value is 32 bytes in size. -#### DiamondCutFacet - -These smart contracts manage the freezing/unfreezing and upgrades of the diamond proxy. That being said, the contract -must never be frozen. - -Currently, freezing and unfreezing are implemented as access control functions. It is fully controlled by the governor -but can be changed later. The governor can call `freezeDiamond` to freeze the diamond and `unfreezeDiamond` to restore -it. - -Another purpose of `DiamondCutFacet` is to upgrade the facets. The upgrading is split into 2-3 phases: - -- `proposeTransparentUpgrade`/`proposeShadowUpgrade` - propose an upgrade with visible/hidden parameters. -- `cancelUpgradeProposal` - cancel the upgrade proposal. -- `securityCouncilUpgradeApprove` - approve the upgrade by the security council. -- `executeUpgrade` - finalize the upgrade. - -The upgrade itself characterizes by three variables: - -- `facetCuts` - a set of changes to the facets (adding new facets, removing facets, and replacing them). -- pair `(address _initAddress, bytes _calldata)` for initializing the upgrade by making a delegate call to - `_initAddress` with `_calldata` inputs. - #### GettersFacet Separate facet, whose only function is providing `view` and `pure` methods. It also implements [diamond loupe](https://eips.ethereum.org/EIPS/eip-2535#diamond-loupe) which makes managing facets easier. +This contract must never be frozen. -#### GovernanceFacet +#### AdminFacet Controls changing the privileged addresses such as governor and validators or one of the system parameters (L2 -bootloader bytecode hash, verifier address, verifier parameters, etc). +bootloader bytecode hash, verifier address, verifier parameters, etc), and it also manages the freezing/unfreezing and execution of +upgrades in the diamond proxy. + +#### Governance + +This contract manages operations (calls with preconditions) for governance tasks. The contract allows for operations to be scheduled, +executed, and canceled with appropriate permissions and delays. It is used for managing and coordinating upgrades and changes in all +zkSync Era governed contracts. + +Each upgrade consists of two steps: + +- Upgrade Proposal - The governor can schedule upgrades in two different manners: + - Fully transparent data. All implementation contracts and migration contracts are known to the community. The governor must wait +for the timelock to execute the upgrade. + - Shadow upgrade. The governor only shows the commitment for the upgrade. The upgrade can be executed only with security council +approval without timelock. +- Upgrade execution - perform the upgrade that was proposed. -At the current stage, the governor has permission to instantly change the key system parameters with `GovernanceFacet`. -Later such functionality will be removed and changing system parameters will be possible only via Diamond upgrade (see -_DiamondCutFacet_). #### MailboxFacet @@ -257,6 +249,16 @@ investigation and mitigation before resuming normal operations. It is a temporary solution to prevent any significant impact of the validator hot key leakage, while the network is in the Alpha stage. +This contract consists of four main functions `commitBatches`, `proveBatches`, `executeBatches`, and `revertBatches`, that +can be called only by the validator. + +When the validator calls `commitBatches`, the same calldata will be propogated to the zkSync contract (`DiamondProxy` through +`call` where it invokes the `ExecutorFacet` through `delegatecall`), and also a timestamp is assigned to these batches to track +the time these batches are commited by the validator to enforce a delay between committing and execution of batches. Then, the +validator can prove the already commited batches regardless of the mentioned timestamp, and again the same calldata (related +to the `proveBatches` function) will be propogated to the zkSync contract. After, the `delay` is elapsed, the validator +is allowed to call `executeBatches` to propogate the same calldata to zkSync contract. + #### Allowlist The auxiliary contract controls the permission access list. It is used in bridges and diamond proxies to control which diff --git a/ethereum/contracts/dev-contracts/test/GovernanceFacetTest.sol b/ethereum/contracts/dev-contracts/test/AdminFacetTest.sol similarity index 87% rename from ethereum/contracts/dev-contracts/test/GovernanceFacetTest.sol rename to ethereum/contracts/dev-contracts/test/AdminFacetTest.sol index bb19c9843..7d2460b77 100644 --- a/ethereum/contracts/dev-contracts/test/GovernanceFacetTest.sol +++ b/ethereum/contracts/dev-contracts/test/AdminFacetTest.sol @@ -2,9 +2,9 @@ pragma solidity ^0.8.13; -import "../../zksync/facets/Governance.sol"; +import "../../zksync/facets/Admin.sol"; -contract GovernanceFacetTest is GovernanceFacet { +contract AdminFacetTest is AdminFacet { constructor() { s.governor = msg.sender; } diff --git a/ethereum/contracts/dev-contracts/test/DiamondUpgradeSecurityCouncil.sol b/ethereum/contracts/dev-contracts/test/DiamondUpgradeSecurityCouncil.sol deleted file mode 100644 index 5f611656f..000000000 --- a/ethereum/contracts/dev-contracts/test/DiamondUpgradeSecurityCouncil.sol +++ /dev/null @@ -1,14 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.13; - -import "../../zksync/libraries/Diamond.sol"; -import "../../zksync/facets/Base.sol"; - -contract DiamondUpgradeSecurityCouncil is Base { - function upgrade(address _securityCouncil) external payable returns (bytes32) { - s.upgrades.securityCouncil = _securityCouncil; - - return Diamond.DIAMOND_INIT_SUCCESS_RETURN_VALUE; - } -} diff --git a/ethereum/contracts/governance/Governance.sol b/ethereum/contracts/governance/Governance.sol new file mode 100644 index 000000000..ccc60ba9c --- /dev/null +++ b/ethereum/contracts/governance/Governance.sol @@ -0,0 +1,264 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.13; + +import {Ownable2Step} from "@openzeppelin/contracts/access/Ownable2Step.sol"; +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; +import {IGovernance} from "./IGovernance.sol"; + +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev +/// @dev Contract design is inspired by OpenZeppelin TimelockController and in-house Diamond Proxy upgrade mechanism. +/// @notice This contract manages operations (calls with preconditions) for governance tasks. +/// The contract allows for operations to be scheduled, executed, and canceled with +/// appropriate permissions and delays. It is used for managing and coordinating upgrades +/// and changes in all zkSync Era governed contracts. +/// +/// Operations can be proposed as either fully transparent upgrades with on-chain data, +/// or "shadow" upgrades where upgrade data is not published on-chain before execution. Proposed operations +/// are subject to a delay before they can be executed, but they can be executed instantly +/// with the security council’s permission. +contract Governance is IGovernance, Ownable2Step { + /// @notice A constant representing the timestamp for completed operations. + uint256 internal constant EXECUTED_PROPOSAL_TIMESTAMP = uint256(1); + + /// @notice The address of the security council. + /// @dev It is supposed to be multisig contract. + address public securityCouncil; + + /// @notice A mapping to store timestamps where each operation will be ready for execution. + /// @dev - 0 means the operation is not created. + /// @dev - 1 (EXECUTED_PROPOSAL_TIMESTAMP) means the operation is already executed. + /// @dev - any other value means timestamp in seconds when the operation will be ready for execution. + mapping(bytes32 => uint256) public timestamps; + + /// @notice The minimum delay in seconds for operations to be ready for execution. + uint256 public minDelay; + + /// @notice Initializes the contract with the admin address, security council address, and minimum delay. + /// @param _admin The address to be assigned as the admin of the contract. + /// @param _securityCouncil The address to be assigned as the security council of the contract. + /// @param _minDelay The initial minimum delay (in seconds) to be set for operations. + constructor(address _admin, address _securityCouncil, uint256 _minDelay) { + require(_admin != address(0), "Admin should be non zero address"); + + _transferOwnership(_admin); + + securityCouncil = _securityCouncil; + emit ChangeSecurityCouncil(address(0), _securityCouncil); + + minDelay = _minDelay; + emit ChangeMinDelay(0, _minDelay); + } + + /*////////////////////////////////////////////////////////////// + MODIFIERS + //////////////////////////////////////////////////////////////*/ + + /// @notice Checks that the message sender is contract itself. + modifier onlySelf() { + require(msg.sender == address(this), "Only governance contract itself allowed to call this function"); + _; + } + + /// @notice Checks that the message sender is an active security council. + modifier onlySecurityCouncil() { + require(msg.sender == securityCouncil, "Only security council allowed to call this function"); + _; + } + + /// @notice Checks that the message sender is an active owner or an active security council. + modifier onlyOwnerOrSecurityCouncil() { + require( + msg.sender == owner() || msg.sender == securityCouncil, + "Only the owner and security council are allowed to call this function" + ); + _; + } + + /*////////////////////////////////////////////////////////////// + OPERATION GETTERS + //////////////////////////////////////////////////////////////*/ + + /// @dev Returns whether an id corresponds to a registered operation. This + /// includes both Waiting, Ready, and Done operations. + function isOperation(bytes32 _id) public view returns (bool) { + return getOperationState(_id) != OperationState.Unset; + } + + /// @dev Returns whether an operation is pending or not. Note that a "pending" operation may also be "ready". + function isOperationPending(bytes32 _id) public view returns (bool) { + OperationState state = getOperationState(_id); + return state == OperationState.Waiting || state == OperationState.Ready; + } + + /// @dev Returns whether an operation is ready for execution. Note that a "ready" operation is also "pending". + function isOperationReady(bytes32 _id) public view returns (bool) { + return getOperationState(_id) == OperationState.Ready; + } + + /// @dev Returns whether an operation is done or not. + function isOperationDone(bytes32 _id) public view returns (bool) { + return getOperationState(_id) == OperationState.Done; + } + + /// @dev Returns operation state. + function getOperationState(bytes32 _id) public view returns (OperationState) { + uint256 timestamp = timestamps[_id]; + if (timestamp == 0) { + return OperationState.Unset; + } else if (timestamp == EXECUTED_PROPOSAL_TIMESTAMP) { + return OperationState.Done; + } else if (timestamp > block.timestamp) { + return OperationState.Waiting; + } else { + return OperationState.Ready; + } + } + + /*////////////////////////////////////////////////////////////// + SCHEDULING CALLS + //////////////////////////////////////////////////////////////*/ + + /// @notice Propose a fully transparent upgrade, providing upgrade data on-chain. + /// @notice The owner will be able to execute the proposal either: + /// - With a `delay` timelock on its own. + /// - With security council instantly. + /// @dev Only the current owner can propose an upgrade. + /// @param _operation The operation parameters will be executed with the upgrade. + /// @param _delay The delay time (in seconds) after which the proposed upgrade can be executed by the owner. + function scheduleTransparent(Operation calldata _operation, uint256 _delay) external onlyOwner { + bytes32 id = hashOperation(_operation); + _schedule(id, _delay); + emit TransparentOperationScheduled(id, _delay, _operation); + } + + /// @notice Propose "shadow" upgrade, upgrade data is not publishing on-chain. + /// @notice The owner will be able to execute the proposal either: + /// - With a `delay` timelock on its own. + /// - With security council instantly. + /// @dev Only the current owner can propose an upgrade. + /// @param _id The operation hash (see `hashOperation` function) + /// @param _delay The delay time (in seconds) after which the proposed upgrade may be executed by the owner. + function scheduleShadow(bytes32 _id, uint256 _delay) external onlyOwner { + _schedule(_id, _delay); + emit ShadowOperationScheduled(_id, _delay); + } + + /*////////////////////////////////////////////////////////////// + CANCELING CALLS + //////////////////////////////////////////////////////////////*/ + + /// @dev Cancel the scheduled operation. + /// @dev Both the owner and security council may cancel an operation. + /// @param _id Proposal id value (see `hashOperation`) + function cancel(bytes32 _id) external onlyOwnerOrSecurityCouncil { + require(isOperationPending(_id)); + delete timestamps[_id]; + emit OperationCancelled(_id); + } + + /*////////////////////////////////////////////////////////////// + EXECUTING CALLS + //////////////////////////////////////////////////////////////*/ + + /// @notice Executes the scheduled operation after the delay passed. + /// @dev Both the owner and security council may execute delayed operations. + /// @param _operation The operation parameters will be executed with the upgrade. + function execute(Operation calldata _operation) external onlyOwnerOrSecurityCouncil { + bytes32 id = hashOperation(_operation); + // Check if the predecessor operation is completed. + _checkPredecessorDone(_operation.predecessor); + // Ensure that the operation is ready to proceed. + require(isOperationReady(id), "Operation must be ready before execution"); + // Execute operation. + _execute(_operation.calls); + // Reconfirming that the operation is still ready after execution. + // This is needed to avoid unexpected reentrancy attacks of re-executing the same operation. + require(isOperationReady(id), "Operation must be ready after execution"); + // Set operation to be done + timestamps[id] = EXECUTED_PROPOSAL_TIMESTAMP; + emit OperationExecuted(id); + } + + /// @notice Executes the scheduled operation with the security council instantly. + /// @dev Only the security council may execute an operation instantly. + /// @param _operation The operation parameters will be executed with the upgrade. + function executeInstant(Operation calldata _operation) external onlySecurityCouncil { + bytes32 id = hashOperation(_operation); + // Check if the predecessor operation is completed. + _checkPredecessorDone(_operation.predecessor); + // Ensure that the operation is in a pending state before proceeding. + require(isOperationPending(id), "Operation must be pending before execution"); + // Execute operation. + _execute(_operation.calls); + // Reconfirming that the operation is still pending before execution. + // This is needed to avoid unexpected reentrancy attacks of re-executing the same operation. + require(isOperationPending(id), "Operation must be pending after execution"); + // Set operation to be done + timestamps[id] = EXECUTED_PROPOSAL_TIMESTAMP; + emit OperationExecuted(id); + } + + /// @dev Returns the identifier of an operation. + /// @param _operation The operation object to compute the identifier for. + function hashOperation(Operation calldata _operation) public pure returns (bytes32) { + return keccak256(abi.encode(_operation)); + } + + /*////////////////////////////////////////////////////////////// + HELPERS + //////////////////////////////////////////////////////////////*/ + + /// @dev Schedule an operation that is to become valid after a given delay. + /// @param _id The operation hash (see `hashOperation` function) + /// @param _delay The delay time (in seconds) after which the proposed upgrade can be executed by the owner. + function _schedule(bytes32 _id, uint256 _delay) internal { + require(!isOperation(_id), "Operation with this proposal id already exists"); + require(_delay >= minDelay, "Proposed delay is less than minimum delay"); + + timestamps[_id] = block.timestamp + _delay; + } + + /// @dev Execute an operation's calls. + /// @param _calls The array of calls to be executed. + function _execute(Call[] calldata _calls) internal { + for (uint256 i = 0; i < _calls.length; ++i) { + (bool success, bytes memory returnData) = _calls[i].target.call{value: _calls[i].value}(_calls[i].data); + if (!success) { + // Propage an error if the call fails. + assembly { + revert(add(returnData, 0x20), mload(returnData)) + } + } + } + } + + /// @notice Verifies if the predecessor operation is completed. + /// @param _predecessorId The hash of the operation that should be completed. + /// @dev Doesn't check the operation to be complete if the input is zero. + function _checkPredecessorDone(bytes32 _predecessorId) internal view { + require(_predecessorId == bytes32(0) || isOperationDone(_predecessorId), "Predecessor operation not completed"); + } + + /*////////////////////////////////////////////////////////////// + SELF UPGRADES + //////////////////////////////////////////////////////////////*/ + + /// @dev Changes the minimum timelock duration for future operations. + /// @param _newDelay The new minimum delay time (in seconds) for future operations. + function updateDelay(uint256 _newDelay) external onlySelf { + emit ChangeMinDelay(minDelay, _newDelay); + minDelay = _newDelay; + } + + /// @dev Updates the address of the security council. + /// @param _newSecurityCouncil The address of the new security council. + function updateSecurityCouncil(address _newSecurityCouncil) external onlySelf { + emit ChangeSecurityCouncil(securityCouncil, _newSecurityCouncil); + securityCouncil = _newSecurityCouncil; + } + + /// @dev Contract might receive/hold ETH as part of the maintenance process. + receive() external payable {} +} diff --git a/ethereum/contracts/governance/IGovernance.sol b/ethereum/contracts/governance/IGovernance.sol new file mode 100644 index 000000000..e106b54a9 --- /dev/null +++ b/ethereum/contracts/governance/IGovernance.sol @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.13; + +interface IGovernance { + /// @dev This enumeration includes the following states: + /// @param Unset Default state, indicating the operation has not been set. + /// @param Waiting The operation is scheduled but not yet ready to be executed. + /// @param Ready The operation is ready to be executed. + /// @param Done The operation has been successfully executed. + enum OperationState { + Unset, + Waiting, + Ready, + Done + } + + /// @dev Represents a call to be made during an operation. + /// @param target The address to which the call will be made. + /// @param value The amount of Ether (in wei) to be sent along with the call. + /// @param data The calldata to be executed on the `target` address. + struct Call { + address target; + uint256 value; + bytes data; + } + + /// @dev Defines the structure of an operation that Governance executes. + /// @param calls An array of `Call` structs, each representing a call to be made during the operation. + /// @param predecessor The hash of the predecessor operation, that should be executed before this operation. + /// @param salt A bytes32 value used for creating unique operation hashes. + struct Operation { + Call[] calls; + bytes32 predecessor; + bytes32 salt; + } + + function isOperation(bytes32 _id) external view returns (bool); + + function isOperationPending(bytes32 _id) external view returns (bool); + + function isOperationReady(bytes32 _id) external view returns (bool); + + function isOperationDone(bytes32 _id) external view returns (bool); + + function getOperationState(bytes32 _id) external view returns (OperationState); + + function scheduleTransparent(Operation calldata _operation, uint256 _delay) external; + + function scheduleShadow(bytes32 _id, uint256 _delay) external; + + function cancel(bytes32 _id) external; + + function execute(Operation calldata _operation) external; + + function executeInstant(Operation calldata _operation) external; + + function hashOperation(Operation calldata _operation) external pure returns (bytes32); + + function updateDelay(uint256 _newDelay) external; + + function updateSecurityCouncil(address _newSecurityCouncil) external; + + /// @notice Emitted when transparent operation is scheduled. + event TransparentOperationScheduled(bytes32 indexed _id, uint256 delay, Operation _operation); + + /// @notice Emitted when shadow operation is scheduled. + event ShadowOperationScheduled(bytes32 indexed _id, uint256 delay); + + /// @notice Emitted when the operation is executed with delay or instantly. + event OperationExecuted(bytes32 indexed _id); + + /// @notice Emitted when the security council address is changed. + event ChangeSecurityCouncil(address _securityCouncilBefore, address _securityCouncilAfter); + + /// @notice Emitted when the minimum delay for future operations is modified. + event ChangeMinDelay(uint256 _delayBefore, uint256 _delayAfter); + + /// @notice Emitted when the operation with specified id is cancelled. + event OperationCancelled(bytes32 indexed _id); +} diff --git a/ethereum/contracts/zksync/DiamondInit.sol b/ethereum/contracts/zksync/DiamondInit.sol index d5be51581..9140af05e 100644 --- a/ethereum/contracts/zksync/DiamondInit.sol +++ b/ethereum/contracts/zksync/DiamondInit.sol @@ -22,7 +22,8 @@ contract DiamondInit is Base { /// @notice zkSync contract initialization /// @param _verifier address of Verifier contract - /// @param _governor address who can manage the contract + /// @param _governor address who can manage critical updates in the contract + /// @param _admin address who can manage non-critical updates in the contract /// @param _genesisBatchHash Batch hash of the genesis (initial) batch /// @param _genesisIndexRepeatedStorageChanges The serial number of the shortcut storage key for genesis batch /// @param _genesisBatchCommitment The zk-proof commitment for the genesis batch @@ -37,6 +38,7 @@ contract DiamondInit is Base { function initialize( IVerifier _verifier, address _governor, + address _admin, bytes32 _genesisBatchHash, uint64 _genesisIndexRepeatedStorageChanges, bytes32 _genesisBatchCommitment, @@ -49,10 +51,12 @@ contract DiamondInit is Base { ) external reentrancyGuardInitializer returns (bytes32) { require(address(_verifier) != address(0), "vt"); require(_governor != address(0), "vy"); + require(_admin != address(0), "hc"); require(_priorityTxMaxGasLimit <= L2_TX_MAX_GAS_LIMIT, "vu"); s.verifier = _verifier; s.governor = _governor; + s.admin = _admin; // We need to initialize the state hash because it is used in the commitment of the next batch IExecutor.StoredBatchInfo memory storedBatchZero = IExecutor.StoredBatchInfo( diff --git a/ethereum/contracts/zksync/Storage.sol b/ethereum/contracts/zksync/Storage.sol index df9f31009..77be67ee7 100644 --- a/ethereum/contracts/zksync/Storage.sol +++ b/ethereum/contracts/zksync/Storage.sol @@ -79,7 +79,7 @@ struct VerifierParams { struct AppStorage { /// @dev Storage of variables needed for deprecated diamond cut facet uint256[7] __DEPRECATED_diamondCutStorage; - /// @notice Address which will exercise governance over the network i.e. change validator set, conduct upgrades + /// @notice Address which will exercise critical changes to the Diamond Proxy (upgrades, freezing & unfreezing) address governor; /// @notice Address that the governor proposed as one that will replace it address pendingGovernor; @@ -119,7 +119,7 @@ struct AppStorage { /// without overhead for proving the batch. uint256 priorityTxMaxGasLimit; /// @dev Storage of variables needed for upgrade facet - UpgradeStorage upgrades; + UpgradeStorage __DEPRECATED_upgrades; /// @dev A mapping L2 batch number => message number => flag. /// @dev The L2 -> L1 log is sent for every withdrawal, so this mapping is serving as /// a flag to indicate that the message was already processed. @@ -139,4 +139,8 @@ struct AppStorage { /// @dev Batch number where the upgrade transaction has happened. If 0, then no upgrade transaction has happened /// yet. uint256 l2SystemContractsUpgradeBatchNumber; + /// @dev Address which will exercise non-critical changes to the Diamond Proxy (changing validator set & unfreezing) + address admin; + /// @notice Address that the governor or admin proposed as one that will replace admin role + address pendingAdmin; } diff --git a/ethereum/contracts/zksync/facets/Admin.sol b/ethereum/contracts/zksync/facets/Admin.sol new file mode 100644 index 000000000..48f870ba1 --- /dev/null +++ b/ethereum/contracts/zksync/facets/Admin.sol @@ -0,0 +1,128 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.13; + +import "../interfaces/IAdmin.sol"; +import "../libraries/Diamond.sol"; +import "../../common/libraries/L2ContractHelper.sol"; +import {L2_TX_MAX_GAS_LIMIT} from "../Config.sol"; +import "./Base.sol"; + +/// @title Admin Contract controls access rights for contract management. +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev +contract AdminFacet is Base, IAdmin { + string public constant override getName = "AdminFacet"; + + /// @notice Starts the transfer of governor rights. Only the current governor can propose a new pending one. + /// @notice New governor can accept governor rights by calling `acceptGovernor` function. + /// @param _newPendingGovernor Address of the new governor + function setPendingGovernor(address _newPendingGovernor) external onlyGovernor { + // Save previous value into the stack to put it into the event later + address oldPendingGovernor = s.pendingGovernor; + // Change pending governor + s.pendingGovernor = _newPendingGovernor; + emit NewPendingGovernor(oldPendingGovernor, _newPendingGovernor); + } + + /// @notice Accepts transfer of governor rights. Only pending governor can accept the role. + function acceptGovernor() external { + address pendingGovernor = s.pendingGovernor; + require(msg.sender == pendingGovernor, "n4"); // Only proposed by current governor address can claim the governor rights + + address previousGovernor = s.governor; + s.governor = pendingGovernor; + delete s.pendingGovernor; + + emit NewPendingGovernor(pendingGovernor, address(0)); + emit NewGovernor(previousGovernor, pendingGovernor); + } + + /// @notice Starts the transfer of admin rights. Only the current governor or admin can propose a new pending one. + /// @notice New admin can accept admin rights by calling `acceptAdmin` function. + /// @param _newPendingAdmin Address of the new admin + function setPendingAdmin(address _newPendingAdmin) external onlyGovernorOrAdmin { + // Save previous value into the stack to put it into the event later + address oldPendingAdmin = s.pendingAdmin; + // Change pending admin + s.pendingAdmin = _newPendingAdmin; + emit NewPendingGovernor(oldPendingAdmin, _newPendingAdmin); + } + + /// @notice Accepts transfer of admin rights. Only pending admin can accept the role. + function acceptAdmin() external { + address pendingAdmin = s.pendingAdmin; + require(msg.sender == pendingAdmin, "n4"); // Only proposed by current admin address can claim the admin rights + + address previousAdmin = s.admin; + s.admin = pendingAdmin; + delete s.pendingAdmin; + + emit NewPendingAdmin(pendingAdmin, address(0)); + emit NewAdmin(previousAdmin, pendingAdmin); + } + + /// @notice Change validator status (active or not active) + /// @param _validator Validator address + /// @param _active Active flag + function setValidator(address _validator, bool _active) external onlyGovernorOrAdmin { + s.validators[_validator] = _active; + emit ValidatorStatusUpdate(_validator, _active); + } + + /// @notice Change zk porter availability + /// @param _zkPorterIsAvailable The availability of zk porter shard + function setPorterAvailability(bool _zkPorterIsAvailable) external onlyGovernor { + // Change the porter availability + s.zkPorterIsAvailable = _zkPorterIsAvailable; + emit IsPorterAvailableStatusUpdate(_zkPorterIsAvailable); + } + + /// @notice Change the max L2 gas limit for L1 -> L2 transactions + /// @param _newPriorityTxMaxGasLimit The maximum number of L2 gas that a user can request for L1 -> L2 transactions + function setPriorityTxMaxGasLimit(uint256 _newPriorityTxMaxGasLimit) external onlyGovernor { + require(_newPriorityTxMaxGasLimit <= L2_TX_MAX_GAS_LIMIT, "n5"); + + uint256 oldPriorityTxMaxGasLimit = s.priorityTxMaxGasLimit; + s.priorityTxMaxGasLimit = _newPriorityTxMaxGasLimit; + emit NewPriorityTxMaxGasLimit(oldPriorityTxMaxGasLimit, _newPriorityTxMaxGasLimit); + } + + /*////////////////////////////////////////////////////////////// + UPGRADE EXECUTION + //////////////////////////////////////////////////////////////*/ + + /// @notice Executes a proposed governor upgrade + /// @dev Only the current governor can execute the upgrade + /// @param _diamondCut The diamond cut parameters to be executed + function executeUpgrade(Diamond.DiamondCutData calldata _diamondCut) external onlyGovernor { + Diamond.diamondCut(_diamondCut); + emit ExecuteUpgrade(_diamondCut); + } + + /*////////////////////////////////////////////////////////////// + CONTRACT FREEZING + //////////////////////////////////////////////////////////////*/ + + /// @notice Instantly pause the functionality of all freezable facets & their selectors + /// @dev Only the governance mechanism may freeze Diamond Proxy + function freezeDiamond() external onlyGovernor { + Diamond.DiamondStorage storage diamondStorage = Diamond.getDiamondStorage(); + + require(!diamondStorage.isFrozen, "a9"); // diamond proxy is frozen already + diamondStorage.isFrozen = true; + + emit Freeze(); + } + + /// @notice Unpause the functionality of all freezable facets & their selectors + /// @dev Both the governor and its owner can unfreeze Diamond Proxy + function unfreezeDiamond() external onlyGovernorOrAdmin { + Diamond.DiamondStorage storage diamondStorage = Diamond.getDiamondStorage(); + + require(diamondStorage.isFrozen, "a7"); // diamond proxy is not frozen + diamondStorage.isFrozen = false; + + emit Unfreeze(); + } +} diff --git a/ethereum/contracts/zksync/facets/Base.sol b/ethereum/contracts/zksync/facets/Base.sol index af92b93d8..b7ae39739 100644 --- a/ethereum/contracts/zksync/facets/Base.sol +++ b/ethereum/contracts/zksync/facets/Base.sol @@ -6,6 +6,8 @@ import "../Storage.sol"; import "../../common/ReentrancyGuard.sol"; import "../../common/AllowListed.sol"; +import "@openzeppelin/contracts/access/Ownable.sol"; + /// @title Base contract containing functions accessible to the other facets. /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -18,15 +20,15 @@ contract Base is ReentrancyGuard, AllowListed { _; } - /// @notice Checks if validator is active - modifier onlyValidator() { - require(s.validators[msg.sender], "1h"); // validator is not active + /// @notice Checks that the message sender is an active governor or admin + modifier onlyGovernorOrAdmin() { + require(msg.sender == s.governor || msg.sender == s.admin, "Only by governor or admin"); _; } - /// @notice Checks if `msg.sender` is the security council - modifier onlySecurityCouncil() { - require(msg.sender == s.upgrades.securityCouncil, "a5"); // not a security council + /// @notice Checks if validator is active + modifier onlyValidator() { + require(s.validators[msg.sender], "1h"); // validator is not active _; } } diff --git a/ethereum/contracts/zksync/facets/DiamondCut.sol b/ethereum/contracts/zksync/facets/DiamondCut.sol deleted file mode 100644 index 68cdf108d..000000000 --- a/ethereum/contracts/zksync/facets/DiamondCut.sol +++ /dev/null @@ -1,201 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.13; - -import "../../common/libraries/UncheckedMath.sol"; -import "../interfaces/IDiamondCut.sol"; -import "../libraries/Diamond.sol"; -import "../Config.sol"; -import "./Base.sol"; - -/// @title DiamondCutFacet - contract responsible for the management of upgrades. -/// @author Matter Labs -/// @custom:security-contact security@matterlabs.dev -contract DiamondCutFacet is Base, IDiamondCut { - using UncheckedMath for uint256; - - string public constant override getName = "DiamondCutFacet"; - - /// @notice Ensures an upgrade proposal already exists. - modifier upgradeProposed() { - require(s.upgrades.state != UpgradeState.None, "a3"); // Proposal doesn't exist - _; - } - - /// @notice Ensures no proposal has been made yet. - modifier noUpgradeProposed() { - require(s.upgrades.state == UpgradeState.None, "a8"); // Proposal already exists - _; - } - - /*////////////////////////////////////////////////////////////// - UPGRADE PROPOSING - //////////////////////////////////////////////////////////////*/ - - /// @notice Propose a fully transparent upgrade, providing upgrade data on-chain - /// @notice The governor will be able to execute the proposal either - /// - With a `UPGRADE_NOTICE_PERIOD` timelock on its own - /// - With security council approvals instantly - /// @dev Only the current governor can propose an upgrade - /// @param _diamondCut The diamond cut parameters will be executed with the upgrade - function proposeTransparentUpgrade(Diamond.DiamondCutData calldata _diamondCut, uint40 _proposalId) - external - onlyGovernor - noUpgradeProposed - { - // Set the default value for proposal salt, since the proposal is fully transparent it doesn't change anything - bytes32 proposalSalt; - uint40 expectedProposalId = s.upgrades.currentProposalId + 1; - // Input proposal ID should be equal to the expected proposal id - require(_proposalId == expectedProposalId, "yb"); - s.upgrades.proposedUpgradeHash = upgradeProposalHash(_diamondCut, expectedProposalId, proposalSalt); - s.upgrades.proposedUpgradeTimestamp = uint40(block.timestamp); - s.upgrades.currentProposalId = expectedProposalId; - s.upgrades.state = UpgradeState.Transparent; - - emit ProposeTransparentUpgrade(_diamondCut, expectedProposalId, proposalSalt); - } - - /// @notice Propose "shadow" upgrade, upgrade data is not publishing on-chain - /// @notice The governor will be able to execute the proposal only: - /// - With security council approvals instantly - /// @dev Only the current governor can propose an upgrade - /// @param _proposalHash Upgrade proposal hash (see `upgradeProposalHash` function) - /// @param _proposalId An expected value for the current proposal Id - function proposeShadowUpgrade(bytes32 _proposalHash, uint40 _proposalId) external onlyGovernor noUpgradeProposed { - require(_proposalHash != bytes32(0), "mi"); - - s.upgrades.proposedUpgradeHash = _proposalHash; - s.upgrades.proposedUpgradeTimestamp = uint40(block.timestamp); // Safe to cast - s.upgrades.state = UpgradeState.Shadow; - - uint256 currentProposalId = s.upgrades.currentProposalId; - // Expected proposal ID should be one more than the current saved proposal ID value - require(_proposalId == currentProposalId.uncheckedInc(), "ya"); - s.upgrades.currentProposalId = _proposalId; - - emit ProposeShadowUpgrade(_proposalId, _proposalHash); - } - - /*////////////////////////////////////////////////////////////// - UPGRADE CANCELING - //////////////////////////////////////////////////////////////*/ - - /// @notice Cancel the proposed upgrade - /// @dev Only the current governor can remove the proposal - /// @param _proposedUpgradeHash Expected upgrade hash value to be canceled - function cancelUpgradeProposal(bytes32 _proposedUpgradeHash) external onlyGovernor upgradeProposed { - bytes32 currentUpgradeHash = s.upgrades.proposedUpgradeHash; - // Soft check that the governor is not mistaken about canceling proposals - require(_proposedUpgradeHash == currentUpgradeHash, "rx"); - - _resetProposal(); - emit CancelUpgradeProposal(s.upgrades.currentProposalId, currentUpgradeHash); - } - - /*////////////////////////////////////////////////////////////// - SECURITY COUNCIL - //////////////////////////////////////////////////////////////*/ - - /// @notice Approves the instant upgrade by the security council - /// @param _upgradeProposalHash The upgrade proposal hash that security council members want to approve. Needed to prevent unintentional approvals, including reorg attacks - function securityCouncilUpgradeApprove(bytes32 _upgradeProposalHash) external onlySecurityCouncil upgradeProposed { - require(s.upgrades.proposedUpgradeHash == _upgradeProposalHash, "un"); - s.upgrades.approvedBySecurityCouncil = true; - - emit SecurityCouncilUpgradeApprove(s.upgrades.currentProposalId, _upgradeProposalHash); - } - - /*////////////////////////////////////////////////////////////// - UPGRADE EXECUTION - //////////////////////////////////////////////////////////////*/ - - /// @notice Executes a proposed governor upgrade - /// @dev Only the current governor can execute the upgrade - /// @param _diamondCut The diamond cut parameters to be executed - /// @param _proposalSalt The committed 32 bytes salt for upgrade proposal data - function executeUpgrade(Diamond.DiamondCutData calldata _diamondCut, bytes32 _proposalSalt) external onlyGovernor { - Diamond.DiamondStorage storage diamondStorage = Diamond.getDiamondStorage(); - - bool approvedBySecurityCouncil = s.upgrades.approvedBySecurityCouncil; - UpgradeState upgradeState = s.upgrades.state; - if (upgradeState == UpgradeState.Transparent) { - bool upgradeNoticePeriodPassed = block.timestamp >= - s.upgrades.proposedUpgradeTimestamp + UPGRADE_NOTICE_PERIOD; - require(upgradeNoticePeriodPassed || approvedBySecurityCouncil, "va"); - require(_proposalSalt == bytes32(0), "po"); // The transparent upgrade may be initiated only with zero salt - } else if (upgradeState == UpgradeState.Shadow) { - require(approvedBySecurityCouncil, "av"); - require(_proposalSalt != bytes32(0), "op"); // Shadow upgrade should be initialized with "random" salt - } else { - revert("ab"); // There is no active upgrade - } - - require(approvedBySecurityCouncil || !diamondStorage.isFrozen, "f3"); - // Should not be frozen or should have enough security council approvals - - uint256 currentProposalId = s.upgrades.currentProposalId; - bytes32 executingProposalHash = upgradeProposalHash(_diamondCut, currentProposalId, _proposalSalt); - require(s.upgrades.proposedUpgradeHash == executingProposalHash, "a4"); // Proposal should be created - _resetProposal(); - - if (diamondStorage.isFrozen) { - diamondStorage.isFrozen = false; - emit Unfreeze(); - } - - Diamond.diamondCut(_diamondCut); - emit ExecuteUpgrade(currentProposalId, executingProposalHash, _proposalSalt); - } - - /*////////////////////////////////////////////////////////////// - CONTRACT FREEZING - //////////////////////////////////////////////////////////////*/ - - /// @notice Instantly pause the functionality of all freezable facets & their selectors - function freezeDiamond() external onlyGovernor { - Diamond.DiamondStorage storage diamondStorage = Diamond.getDiamondStorage(); - - require(!diamondStorage.isFrozen, "a9"); // diamond proxy is frozen already - _resetProposal(); - diamondStorage.isFrozen = true; - - emit Freeze(); - } - - /// @notice Unpause the functionality of all freezable facets & their selectors - function unfreezeDiamond() external onlyGovernor { - Diamond.DiamondStorage storage diamondStorage = Diamond.getDiamondStorage(); - - require(diamondStorage.isFrozen, "a7"); // diamond proxy is not frozen - _resetProposal(); - diamondStorage.isFrozen = false; - - emit Unfreeze(); - } - - /*////////////////////////////////////////////////////////////// - GETTERS & HELPERS - //////////////////////////////////////////////////////////////*/ - - /// @notice Generate the upgrade proposal hash - /// @param _diamondCut The diamond cut parameters will be executed with the upgrade - /// @param _proposalId The current proposal ID, to set a unique upgrade hash depending on the upgrades order - /// @param _salt The arbitrary 32 bytes, primarily used in shadow upgrades to prevent guessing the upgrade proposal content by its hash - /// @return The upgrade proposal hash - function upgradeProposalHash( - Diamond.DiamondCutData calldata _diamondCut, - uint256 _proposalId, - bytes32 _salt - ) public pure returns (bytes32) { - return keccak256(abi.encode(_diamondCut, _proposalId, _salt)); - } - - /// @dev Set up the proposed upgrade state to the default values - function _resetProposal() internal { - delete s.upgrades.state; - delete s.upgrades.proposedUpgradeHash; - delete s.upgrades.proposedUpgradeTimestamp; - delete s.upgrades.approvedBySecurityCouncil; - } -} diff --git a/ethereum/contracts/zksync/facets/Getters.sol b/ethereum/contracts/zksync/facets/Getters.sol index 71c85a734..65a381de6 100644 --- a/ethereum/contracts/zksync/facets/Getters.sol +++ b/ethereum/contracts/zksync/facets/Getters.sol @@ -106,31 +106,6 @@ contract GettersFacet is Base, IGetters, ILegacyGetters { return s.verifierParams; } - /// @return The address of the security council multisig - function getSecurityCouncil() external view returns (address) { - return s.upgrades.securityCouncil; - } - - /// @return Current upgrade proposal state - function getUpgradeProposalState() external view returns (UpgradeState) { - return s.upgrades.state; - } - - /// @return The upgrade proposal hash if there is an active one and zero otherwise - function getProposedUpgradeHash() external view returns (bytes32) { - return s.upgrades.proposedUpgradeHash; - } - - /// @return The timestamp when the upgrade was proposed, zero if there are no active proposals - function getProposedUpgradeTimestamp() external view returns (uint256) { - return s.upgrades.proposedUpgradeTimestamp; - } - - /// @return The serial number of a proposed upgrade, increments when proposing a new one - function getCurrentProposalId() external view returns (uint256) { - return s.upgrades.currentProposalId; - } - /// @return The current protocol version function getProtocolVersion() external view returns (uint256) { return s.protocolVersion; @@ -150,11 +125,6 @@ contract GettersFacet is Base, IGetters, ILegacyGetters { return s.l2SystemContractsUpgradeBatchNumber; } - /// @return The number of received upgrade approvals from the security council - function isApprovedBySecurityCouncil() external view returns (bool) { - return s.upgrades.approvedBySecurityCouncil; - } - /// @return Whether the diamond is frozen or not function isDiamondStorageFrozen() external view returns (bool) { Diamond.DiamondStorage storage ds = Diamond.getDiamondStorage(); diff --git a/ethereum/contracts/zksync/facets/Governance.sol b/ethereum/contracts/zksync/facets/Governance.sol deleted file mode 100644 index 91cdff9ec..000000000 --- a/ethereum/contracts/zksync/facets/Governance.sol +++ /dev/null @@ -1,65 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.13; - -import "../interfaces/IGovernance.sol"; -import "../../common/libraries/L2ContractHelper.sol"; -import {L2_TX_MAX_GAS_LIMIT} from "../Config.sol"; -import "./Base.sol"; - -/// @title Governance Contract controls access rights for contract management. -/// @author Matter Labs -/// @custom:security-contact security@matterlabs.dev -contract GovernanceFacet is Base, IGovernance { - string public constant override getName = "GovernanceFacet"; - - /// @notice Starts the transfer of governor rights. Only the current governor can propose a new pending one. - /// @notice New governor can accept governor rights by calling `acceptGovernor` function. - /// @param _newPendingGovernor Address of the new governor - function setPendingGovernor(address _newPendingGovernor) external onlyGovernor { - // Save previous value into the stack to put it into the event later - address oldPendingGovernor = s.pendingGovernor; - // Change pending governor - s.pendingGovernor = _newPendingGovernor; - emit NewPendingGovernor(oldPendingGovernor, _newPendingGovernor); - } - - /// @notice Accepts transfer of admin rights. Only pending governor can accept the role. - function acceptGovernor() external { - address pendingGovernor = s.pendingGovernor; - require(msg.sender == pendingGovernor, "n4"); // Only proposed by current governor address can claim the governor rights - - address previousGovernor = s.governor; - s.governor = pendingGovernor; - delete s.pendingGovernor; - - emit NewPendingGovernor(pendingGovernor, address(0)); - emit NewGovernor(previousGovernor, pendingGovernor); - } - - /// @notice Change validator status (active or not active) - /// @param _validator Validator address - /// @param _active Active flag - function setValidator(address _validator, bool _active) external onlyGovernor { - s.validators[_validator] = _active; - emit ValidatorStatusUpdate(_validator, _active); - } - - /// @notice Change zk porter availability - /// @param _zkPorterIsAvailable The availability of zk porter shard - function setPorterAvailability(bool _zkPorterIsAvailable) external onlyGovernor { - // Change the porter availability - s.zkPorterIsAvailable = _zkPorterIsAvailable; - emit IsPorterAvailableStatusUpdate(_zkPorterIsAvailable); - } - - /// @notice Change the max L2 gas limit for L1 -> L2 transactions - /// @param _newPriorityTxMaxGasLimit The maximum number of L2 gas that a user can request for L1 -> L2 transactions - function setPriorityTxMaxGasLimit(uint256 _newPriorityTxMaxGasLimit) external onlyGovernor { - require(_newPriorityTxMaxGasLimit <= L2_TX_MAX_GAS_LIMIT, "n5"); - - uint256 oldPriorityTxMaxGasLimit = s.priorityTxMaxGasLimit; - s.priorityTxMaxGasLimit = _newPriorityTxMaxGasLimit; - emit NewPriorityTxMaxGasLimit(oldPriorityTxMaxGasLimit, _newPriorityTxMaxGasLimit); - } -} diff --git a/ethereum/contracts/zksync/interfaces/IGovernance.sol b/ethereum/contracts/zksync/interfaces/IAdmin.sol similarity index 55% rename from ethereum/contracts/zksync/interfaces/IGovernance.sol rename to ethereum/contracts/zksync/interfaces/IAdmin.sol index ace7cb6a6..bb2060d4e 100644 --- a/ethereum/contracts/zksync/interfaces/IGovernance.sol +++ b/ethereum/contracts/zksync/interfaces/IAdmin.sol @@ -4,17 +4,29 @@ pragma solidity ^0.8.13; import "./IBase.sol"; -interface IGovernance is IBase { +import {Diamond} from "../libraries/Diamond.sol"; + +interface IAdmin is IBase { function setPendingGovernor(address _newPendingGovernor) external; function acceptGovernor() external; + function setPendingAdmin(address _newPendingAdmin) external; + + function acceptAdmin() external; + function setValidator(address _validator, bool _active) external; function setPorterAvailability(bool _zkPorterIsAvailable) external; function setPriorityTxMaxGasLimit(uint256 _newPriorityTxMaxGasLimit) external; + function executeUpgrade(Diamond.DiamondCutData calldata _diamondCut) external; + + function freezeDiamond() external; + + function unfreezeDiamond() external; + /// @notice Porter availability status changes event IsPorterAvailableStatusUpdate(bool isPorterAvailable); @@ -28,6 +40,22 @@ interface IGovernance is IBase { /// @notice Governor changed event NewGovernor(address indexed oldGovernor, address indexed newGovernor); + /// @notice pendingAdmin is changed + /// @dev Also emitted when new admin is accepted and in this case, `newPendingAdmin` would be zero address + event NewPendingAdmin(address indexed oldPendingAdmin, address indexed newPendingAdmin); + + /// @notice Admin changed + event NewAdmin(address indexed oldAdmin, address indexed newAdmin); + /// @notice Priority transaction max L2 gas limit changed event NewPriorityTxMaxGasLimit(uint256 oldPriorityTxMaxGasLimit, uint256 newPriorityTxMaxGasLimit); + + /// @notice Emitted when an upgrade is executed. + event ExecuteUpgrade(Diamond.DiamondCutData diamondCut); + + /// @notice Emitted when the contract is frozen. + event Freeze(); + + /// @notice Emitted when the contract is unfrozen. + event Unfreeze(); } diff --git a/ethereum/contracts/zksync/interfaces/IDiamondCut.sol b/ethereum/contracts/zksync/interfaces/IDiamondCut.sol deleted file mode 100644 index b595f772c..000000000 --- a/ethereum/contracts/zksync/interfaces/IDiamondCut.sol +++ /dev/null @@ -1,46 +0,0 @@ -// SPDX-License-Identifier: MIT - -pragma solidity ^0.8.13; - -import "../libraries/Diamond.sol"; -import "./IBase.sol"; - -interface IDiamondCut is IBase { - function proposeTransparentUpgrade(Diamond.DiamondCutData calldata _diamondCut, uint40 _proposalId) external; - - function proposeShadowUpgrade(bytes32 _proposalHash, uint40 _proposalId) external; - - function cancelUpgradeProposal(bytes32 _proposedUpgradeHash) external; - - function securityCouncilUpgradeApprove(bytes32 _upgradeProposalHash) external; - - function executeUpgrade(Diamond.DiamondCutData calldata _diamondCut, bytes32 _proposalSalt) external; - - function freezeDiamond() external; - - function unfreezeDiamond() external; - - function upgradeProposalHash( - Diamond.DiamondCutData calldata _diamondCut, - uint256 _proposalId, - bytes32 _salt - ) external pure returns (bytes32); - - event ProposeTransparentUpgrade( - Diamond.DiamondCutData diamondCut, - uint256 indexed proposalId, - bytes32 proposalSalt - ); - - event ProposeShadowUpgrade(uint256 indexed proposalId, bytes32 indexed proposalHash); - - event CancelUpgradeProposal(uint256 indexed proposalId, bytes32 indexed proposalHash); - - event SecurityCouncilUpgradeApprove(uint256 indexed proposalId, bytes32 indexed proposalHash); - - event ExecuteUpgrade(uint256 indexed proposalId, bytes32 indexed proposalHash, bytes32 proposalSalt); - - event Freeze(); - - event Unfreeze(); -} diff --git a/ethereum/contracts/zksync/interfaces/IGetters.sol b/ethereum/contracts/zksync/interfaces/IGetters.sol index f88c3b9d7..d658a6978 100644 --- a/ethereum/contracts/zksync/interfaces/IGetters.sol +++ b/ethereum/contracts/zksync/interfaces/IGetters.sol @@ -45,24 +45,12 @@ interface IGetters is IBase { function isDiamondStorageFrozen() external view returns (bool); - function getSecurityCouncil() external view returns (address); - - function getUpgradeProposalState() external view returns (UpgradeState); - - function getProposedUpgradeHash() external view returns (bytes32); - - function getProposedUpgradeTimestamp() external view returns (uint256); - - function getCurrentProposalId() external view returns (uint256); - function getProtocolVersion() external view returns (uint256); function getL2SystemContractsUpgradeTxHash() external view returns (bytes32); function getL2SystemContractsUpgradeBatchNumber() external view returns (uint256); - function isApprovedBySecurityCouncil() external view returns (bool); - function getPriorityTxMaxGasLimit() external view returns (uint256); function getAllowList() external view returns (address); diff --git a/ethereum/contracts/zksync/interfaces/IZkSync.sol b/ethereum/contracts/zksync/interfaces/IZkSync.sol index 72f69463f..b3112a48b 100644 --- a/ethereum/contracts/zksync/interfaces/IZkSync.sol +++ b/ethereum/contracts/zksync/interfaces/IZkSync.sol @@ -3,9 +3,8 @@ pragma solidity ^0.8.13; import "./IMailbox.sol"; -import "./IGovernance.sol"; +import "./IAdmin.sol"; import "./IExecutor.sol"; -import "./IDiamondCut.sol"; import "./IGetters.sol"; -interface IZkSync is IMailbox, IGovernance, IExecutor, IDiamondCut, IGetters {} +interface IZkSync is IMailbox, IAdmin, IExecutor, IGetters {} diff --git a/ethereum/package.json b/ethereum/package.json index 9667d62cc..8abd508cc 100644 --- a/ethereum/package.json +++ b/ethereum/package.json @@ -53,7 +53,7 @@ "test:foundry": "hardhat solpp && forge test", "test:fork": "CONTRACT_TESTS=1 TEST_CONTRACTS_FORK=1 yarn run hardhat test test/unit_tests/*.fork.ts --network hardhat", "lint": "yarn lint:sol && yarn prettier:check", - "lint:sol-contracts": "solhint --max-warnings 31 contracts/**/*.sol", + "lint:sol-contracts": "solhint --max-warnings 44 contracts/**/*.sol", "lint:sol-tests": "solhint --max-warnings 0 test/**/*.sol", "lint:sol": "yarn lint:sol-contracts && yarn lint:sol-tests", "prettier:check-contracts": "prettier --check contracts/**/*.sol", @@ -70,13 +70,13 @@ "deploy-erc20": "ts-node scripts/deploy-erc20.ts", "token-info": "ts-node scripts/token-info.ts", "deploy-testkit": "ts-node scripts/deploy-testkit.ts", - "diamond-upgrade": "ts-node scripts/diamond-upgrade.ts", "verify": "hardhat run --network env scripts/verify.ts", "deploy-testnet-erc20": "ts-node scripts/deploy-testnet-token.ts", "read-variable": "ts-node scripts/read-variable.ts", "initialize-bridges": "ts-node scripts/initialize-bridges.ts", "initialize-allow-list": "ts-node scripts/initialize-l1-allow-list.ts", "initialize-validator": "ts-node scripts/initialize-validator.ts", + "initialize-governance": "ts-node scripts/initialize-governance.ts", "upgrade-1": "ts-node scripts/upgrades/upgrade-1.ts", "upgrade-2": "ts-node scripts/upgrades/upgrade-2.ts", "upgrade-3": "ts-node scripts/upgrades/upgrade-3.ts", diff --git a/ethereum/scripts/deploy-weth-bridges.ts b/ethereum/scripts/deploy-weth-bridges.ts index 6e0d7ec85..1ba6a9127 100644 --- a/ethereum/scripts/deploy-weth-bridges.ts +++ b/ethereum/scripts/deploy-weth-bridges.ts @@ -19,7 +19,6 @@ async function main() { .option('--private-key ') .option('--gas-price ') .option('--nonce ') - .option('--governor-address ') .option('--create2-salt ') .action(async (cmd) => { const deployWallet = cmd.privateKey @@ -30,9 +29,6 @@ async function main() { ).connect(provider); console.log(`Using deployer wallet: ${deployWallet.address}`); - const governorAddress = cmd.governorAddress ? cmd.governorAddress : deployWallet.address; - console.log(`Using governor address: ${governorAddress}`); - const gasPrice = cmd.gasPrice ? parseUnits(cmd.gasPrice, 'gwei') : await provider.getGasPrice(); console.log(`Using gas price: ${formatUnits(gasPrice, 'gwei')} gwei`); @@ -43,7 +39,6 @@ async function main() { const deployer = new Deployer({ deployWallet, - governorAddress, verbose: true }); diff --git a/ethereum/scripts/deploy.ts b/ethereum/scripts/deploy.ts index 51ef7541b..e12d77ba4 100644 --- a/ethereum/scripts/deploy.ts +++ b/ethereum/scripts/deploy.ts @@ -19,7 +19,7 @@ async function main() { .option('--private-key ') .option('--gas-price ') .option('--nonce ') - .option('--governor-address ') + .option('--owner-address ') .option('--create2-salt ') .option('--diamond-upgrade-init ') .option('--only-verifier') @@ -32,8 +32,8 @@ async function main() { ).connect(provider); console.log(`Using deployer wallet: ${deployWallet.address}`); - const governorAddress = cmd.governorAddress ? cmd.governorAddress : deployWallet.address; - console.log(`Using governor address: ${governorAddress}`); + const ownerAddress = cmd.ownerAddress ? cmd.ownerAddress : deployWallet.address; + console.log(`Using owner address: ${ownerAddress}`); const gasPrice = cmd.gasPrice ? parseUnits(cmd.gasPrice, 'gwei') : await provider.getGasPrice(); console.log(`Using gas price: ${formatUnits(gasPrice, 'gwei')} gwei`); @@ -45,7 +45,7 @@ async function main() { const deployer = new Deployer({ deployWallet, - governorAddress, + ownerAddress, verbose: true }); @@ -79,8 +79,9 @@ async function main() { }); nonce++; - await deployer.deployAllowList(create2Salt, { gasPrice, nonce }); - await deployer.deployZkSyncContract(create2Salt, gasPrice, nonce + 1); + await deployer.deployGovernance(create2Salt, { gasPrice, nonce }); + await deployer.deployAllowList(create2Salt, { gasPrice, nonce: nonce + 1 }); + await deployer.deployZkSyncContract(create2Salt, gasPrice, nonce + 2); await deployer.deployBridgeContracts(create2Salt, gasPrice); // Do not pass nonce, since it was increment after deploying zkSync contracts await deployer.deployWethBridgeContracts(create2Salt, gasPrice); await deployer.deployValidatorTimelock(create2Salt, { gasPrice }); diff --git a/ethereum/scripts/diamond-upgrade.ts b/ethereum/scripts/diamond-upgrade.ts deleted file mode 100644 index 2f7b5301b..000000000 --- a/ethereum/scripts/diamond-upgrade.ts +++ /dev/null @@ -1,113 +0,0 @@ -import * as hardhat from 'hardhat'; -import { Command } from 'commander'; -import { diamondCut } from '../src.ts/diamondCut'; -import { ethers } from 'hardhat'; -import { Deployer } from '../src.ts/deploy'; -import { print, web3Provider } from './utils'; -import { FacetCut, getAllSelectors } from '../src.ts/diamondCut'; - -const provider = web3Provider(); -const ZERO_ADDRESS = ethers.constants.AddressZero; - -function getZkSyncContract() { - // Create the dummy wallet with provider to get contracts from `Deployer` - const dummyWallet = ethers.Wallet.createRandom().connect(provider); - const deployer = new Deployer({ deployWallet: dummyWallet }); - - return deployer.zkSyncContract(dummyWallet); -} - -async function main() { - const program = new Command(); - - program.version('0.1.0').name('upgrade-diamond'); - - program.command('get-contract-selectors ').action(async (contractName: string) => { - const contract = await hardhat.ethers.getContractAt(contractName, ZERO_ADDRESS); - const selectors = getAllSelectors(contract.interface); - - print('Contract selectors', selectors); - }); - - program.command('diamond-loupe-view').action(async () => { - const facets = await getZkSyncContract().facets(); - - print('Facets', facets); - }); - - program - .command('legacy-prepare-upgrade-calldata ') - .option('--init-address ') - .option('--init-data ') - .action(async (facetCutsData: string, cmd) => { - const diamondCutFacet = await hardhat.ethers.getContractAt('IOldDiamondCut', ZERO_ADDRESS); - - // Encode data for the upgrade call - const facetCuts: Array = JSON.parse(facetCutsData); - - const initAddress = cmd.initAddress ? cmd.initAddress : ZERO_ADDRESS; - const initData = cmd.initData ? cmd.initData : '0x'; - - const upgradeParam = diamondCut(facetCuts, initAddress, initData); - print('DiamondCutData', upgradeParam); - - // Get transaction data of the `proposeDiamondCut` - const proposeUpgrade = await diamondCutFacet.interface.encodeFunctionData('proposeDiamondCut', [ - upgradeParam.facetCuts, - upgradeParam.initAddress - ]); - - // Get transaction data of the `executeDiamondCutProposal` - const executeUpgrade = await diamondCutFacet.interface.encodeFunctionData('executeDiamondCutProposal', [ - upgradeParam - ]); - - print('proposeUpgrade', proposeUpgrade); - print('executeUpgrade', executeUpgrade); - }); - - program - .command('prepare-upgrade-calldata ') - .option('--init-address ') - .option('--init-data ') - .option('--proposal-id ') - .action(async (facetCutsData: string, cmd) => { - const diamondCutFacet = await hardhat.ethers.getContractAt('DiamondCutFacet', ZERO_ADDRESS); - - // Encode data for the upgrade call - const facetCuts: Array = JSON.parse(facetCutsData); - - const initAddress = cmd.initAddress ? cmd.initAddress : ZERO_ADDRESS; - const initData = cmd.initData ? cmd.initData : '0x'; - const proposalId = cmd.proposalId - ? cmd.proposalId - : (await getZkSyncContract().getCurrentProposalId()).add(1); - - const upgradeParam = diamondCut(facetCuts, initAddress, initData); - print('DiamondCut', upgradeParam); - - // Get transaction data of the `proposeTransparentUpgrade` - const proposeUpgrade = await diamondCutFacet.interface.encodeFunctionData('proposeTransparentUpgrade', [ - upgradeParam, - proposalId - ]); - - // Get transaction data of the `executeDiamondCutProposal` - const executeUpgrade = await diamondCutFacet.interface.encodeFunctionData('executeUpgrade', [ - upgradeParam, - ethers.constants.HashZero - ]); - - print('proposeUpgrade', proposeUpgrade); - print('executeUpgrade', executeUpgrade); - }); - - await program.parseAsync(process.argv); -} - -main() - .then(() => process.exit(0)) - .catch((err) => { - console.error('Error:', err); - process.exit(1); - }); diff --git a/ethereum/scripts/initialize-bridges.ts b/ethereum/scripts/initialize-bridges.ts index c87960094..361363cdc 100644 --- a/ethereum/scripts/initialize-bridges.ts +++ b/ethereum/scripts/initialize-bridges.ts @@ -78,7 +78,6 @@ async function main() { const deployer = new Deployer({ deployWallet, - governorAddress: deployWallet.address, verbose: true }); diff --git a/ethereum/scripts/initialize-governance.ts b/ethereum/scripts/initialize-governance.ts new file mode 100644 index 000000000..213475d73 --- /dev/null +++ b/ethereum/scripts/initialize-governance.ts @@ -0,0 +1,80 @@ +import { Command } from 'commander'; +import { ethers, Wallet } from 'ethers'; +import { Deployer } from '../src.ts/deploy'; +import { formatUnits, parseUnits } from 'ethers/lib/utils'; +import { + web3Provider, +} from './utils'; + +import * as fs from 'fs'; +import * as path from 'path'; + +const provider = web3Provider(); +const testConfigPath = path.join(process.env.ZKSYNC_HOME as string, `etc/test_config/constant`); +const ethTestConfig = JSON.parse(fs.readFileSync(`${testConfigPath}/eth.json`, { encoding: 'utf-8' })); + +async function main() { + const program = new Command(); + + program.version('0.1.0').name('initialize-governance'); + + program + .option('--private-key ') + .option('--owner-address ') + .option('--gas-price ') + .action(async (cmd) => { + const deployWallet = cmd.privateKey + ? new Wallet(cmd.privateKey, provider) + : Wallet.fromMnemonic( + process.env.MNEMONIC ? process.env.MNEMONIC : ethTestConfig.mnemonic, + "m/44'/60'/0'/0/1" + ).connect(provider); + console.log(`Using deployer wallet: ${deployWallet.address}`); + + const gasPrice = cmd.gasPrice ? parseUnits(cmd.gasPrice, 'gwei') : await provider.getGasPrice(); + console.log(`Using gas price: ${formatUnits(gasPrice, 'gwei')} gwei`); + + const ownerAddress = cmd.ownerAddress ? cmd.ownerAddress : deployWallet.address; + + const deployer = new Deployer({ + deployWallet, + ownerAddress, + verbose: true + }); + + const governance = deployer.governanceContract(deployWallet); + const zkSync = deployer.zkSyncContract(deployWallet); + + const erc20Bridge = deployer.transparentUpgradableProxyContract(deployer.addresses.Bridges.ERC20BridgeProxy, deployWallet); + const wethBridge = deployer.transparentUpgradableProxyContract(deployer.addresses.Bridges.WethBridgeProxy, deployWallet); + + await (await erc20Bridge.changeAdmin(governance.address)).wait(); + await (await wethBridge.changeAdmin(governance.address)).wait(); + + await (await zkSync.setPendingGovernor(governance.address)).wait(); + + const call = { + target: zkSync.address, + value: 0, + data: zkSync.interface.encodeFunctionData('acceptGovernor') + } + + const operation = { + calls: [call], + predecessor: ethers.constants.HashZero, + salt: ethers.constants.HashZero + }; + + await (await governance.scheduleTransparent(operation, 0)).wait(); + await (await governance.execute(operation)).wait(); + }); + + await program.parseAsync(process.argv); +} + +main() + .then(() => process.exit(0)) + .catch((err) => { + console.error('Error:', err); + process.exit(1); + }); diff --git a/ethereum/scripts/initialize-l2-weth-token.ts b/ethereum/scripts/initialize-l2-weth-token.ts index 335735979..d1f2736d1 100644 --- a/ethereum/scripts/initialize-l2-weth-token.ts +++ b/ethereum/scripts/initialize-l2-weth-token.ts @@ -103,7 +103,6 @@ async function main() { const deployer = new Deployer({ deployWallet, - governorAddress: deployWallet.address, verbose: true }); @@ -146,7 +145,6 @@ async function main() { const deployer = new Deployer({ deployWallet, - governorAddress: deployWallet.address, verbose: true }); diff --git a/ethereum/scripts/initialize-validator.ts b/ethereum/scripts/initialize-validator.ts index 1fea7ed4f..de12379c8 100644 --- a/ethereum/scripts/initialize-validator.ts +++ b/ethereum/scripts/initialize-validator.ts @@ -35,7 +35,6 @@ async function main() { const deployer = new Deployer({ deployWallet, - governorAddress: deployWallet.address, verbose: true }); diff --git a/ethereum/scripts/initialize-weth-bridges.ts b/ethereum/scripts/initialize-weth-bridges.ts index 982d987bf..928d787cf 100644 --- a/ethereum/scripts/initialize-weth-bridges.ts +++ b/ethereum/scripts/initialize-weth-bridges.ts @@ -58,7 +58,6 @@ async function main() { const deployer = new Deployer({ deployWallet, - governorAddress: deployWallet.address, verbose: true }); diff --git a/ethereum/scripts/verify.ts b/ethereum/scripts/verify.ts index 34fe10cec..c8cc18804 100644 --- a/ethereum/scripts/verify.ts +++ b/ethereum/scripts/verify.ts @@ -24,10 +24,9 @@ async function main() { // Contracts without constructor parameters for (const address of [ - addresses.ZkSync.DiamondCutFacet, addresses.ZkSync.GettersFacet, addresses.ZkSync.DiamondInit, - addresses.ZkSync.GovernanceFacet, + addresses.ZkSync.AdminFacet, addresses.ZkSync.MailboxFacet, addresses.ZkSync.ExecutorFacet, addresses.ZkSync.Verifier diff --git a/ethereum/src.ts/deploy.ts b/ethereum/src.ts/deploy.ts index 53872bf31..f08ddeda4 100644 --- a/ethereum/src.ts/deploy.ts +++ b/ethereum/src.ts/deploy.ts @@ -9,7 +9,8 @@ import { L1ERC20BridgeFactory } from '../typechain/L1ERC20BridgeFactory'; import { L1WethBridgeFactory } from '../typechain/L1WethBridgeFactory'; import { ValidatorTimelockFactory } from '../typechain/ValidatorTimelockFactory'; import { SingletonFactoryFactory } from '../typechain/SingletonFactoryFactory'; -import { AllowListFactory } from '../typechain'; +import { AllowListFactory, } from '../typechain'; +import { ITransparentUpgradeableProxyFactory } from '../typechain/ITransparentUpgradeableProxyFactory'; import { hexlify } from 'ethers/lib/utils'; import { readSystemContractsBytecode, @@ -21,6 +22,7 @@ import { getTokens } from '../scripts/utils'; import { deployViaCreate2 } from './deploy-utils'; +import { IGovernanceFactory } from '../typechain/IGovernanceFactory'; const L2_BOOTLOADER_BYTECODE_HASH = hexlify(hashL2Bytecode(readBatchBootloaderBytecode())); const L2_DEFAULT_ACCOUNT_BYTECODE_HASH = hexlify(hashL2Bytecode(readSystemContractsBytecode('DefaultAccount'))); @@ -28,9 +30,8 @@ const L2_DEFAULT_ACCOUNT_BYTECODE_HASH = hexlify(hashL2Bytecode(readSystemContra export interface DeployedAddresses { ZkSync: { MailboxFacet: string; - GovernanceFacet: string; + AdminFacet: string; ExecutorFacet: string; - DiamondCutFacet: string; GettersFacet: string; Verifier: string; DiamondInit: string; @@ -44,6 +45,7 @@ export interface DeployedAddresses { WethBridgeImplementation: string; WethBridgeProxy: string; }; + Governance: string; AllowList: string; ValidatorTimeLock: string; Create2Factory: string; @@ -51,7 +53,7 @@ export interface DeployedAddresses { export interface DeployerConfig { deployWallet: Wallet; - governorAddress?: string; + ownerAddress?: string; verbose?: boolean; } @@ -59,8 +61,7 @@ export function deployedAddressesFromEnv(): DeployedAddresses { return { ZkSync: { MailboxFacet: getAddressFromEnv('CONTRACTS_MAILBOX_FACET_ADDR'), - GovernanceFacet: getAddressFromEnv('CONTRACTS_GOVERNANCE_FACET_ADDR'), - DiamondCutFacet: getAddressFromEnv('CONTRACTS_DIAMOND_CUT_FACET_ADDR'), + AdminFacet: getAddressFromEnv('CONTRACTS_ADMIN_FACET_ADDR'), ExecutorFacet: getAddressFromEnv('CONTRACTS_EXECUTOR_FACET_ADDR'), GettersFacet: getAddressFromEnv('CONTRACTS_GETTERS_FACET_ADDR'), DiamondInit: getAddressFromEnv('CONTRACTS_DIAMOND_INIT_ADDR'), @@ -77,7 +78,8 @@ export function deployedAddressesFromEnv(): DeployedAddresses { }, AllowList: getAddressFromEnv('CONTRACTS_L1_ALLOW_LIST_ADDR'), Create2Factory: getAddressFromEnv('CONTRACTS_CREATE2_FACTORY_ADDR'), - ValidatorTimeLock: getAddressFromEnv('CONTRACTS_VALIDATOR_TIMELOCK_ADDR') + ValidatorTimeLock: getAddressFromEnv('CONTRACTS_VALIDATOR_TIMELOCK_ADDR'), + Governance: getAddressFromEnv('CONTRACTS_GOVERNANCE_ADDR') }; } @@ -85,23 +87,22 @@ export class Deployer { public addresses: DeployedAddresses; private deployWallet: Wallet; private verbose: boolean; - private governorAddress: string; + private ownerAddress: string; constructor(config: DeployerConfig) { this.deployWallet = config.deployWallet; this.verbose = config.verbose != null ? config.verbose : false; this.addresses = deployedAddressesFromEnv(); - this.governorAddress = config.governorAddress != null ? config.governorAddress : this.deployWallet.address; + this.ownerAddress = config.ownerAddress != null ? config.ownerAddress : this.deployWallet.address; } public async initialProxyDiamondCut() { const facetCuts = Object.values( await getCurrentFacetCutsForAdd( - this.addresses.ZkSync.DiamondCutFacet, + this.addresses.ZkSync.AdminFacet, this.addresses.ZkSync.GettersFacet, this.addresses.ZkSync.MailboxFacet, - this.addresses.ZkSync.ExecutorFacet, - this.addresses.ZkSync.GovernanceFacet + this.addresses.ZkSync.ExecutorFacet ) ); const genesisBatchHash = getHashFromEnv('CONTRACTS_GENESIS_ROOT'); // TODO: confusing name @@ -117,7 +118,8 @@ export class Deployer { const diamondInitCalldata = DiamondInit.encodeFunctionData('initialize', [ this.addresses.ZkSync.Verifier, - this.governorAddress, + this.ownerAddress, + this.ownerAddress, genesisBatchHash, genesisRollupLeafIndex, genesisBatchCommitment, @@ -173,11 +175,28 @@ export class Deployer { return result[0]; } + public async deployGovernance(create2Salt: string, ethTxOptions: ethers.providers.TransactionRequest) { + ethTxOptions.gasLimit ??= 10_000_000; + const contractAddress = await this.deployViaCreate2( + 'Governance', + // TODO: load parameters from config + [this.ownerAddress, ethers.constants.AddressZero, 0], + create2Salt, + ethTxOptions + ); + + if (this.verbose) { + console.log(`CONTRACTS_GOVERNANCE_ADDR=${contractAddress}`); + } + + this.addresses.Governance = contractAddress; + } + public async deployAllowList(create2Salt: string, ethTxOptions: ethers.providers.TransactionRequest) { ethTxOptions.gasLimit ??= 10_000_000; const contractAddress = await this.deployViaCreate2( 'AllowList', - [this.governorAddress], + [this.ownerAddress], create2Salt, ethTxOptions ); @@ -200,26 +219,15 @@ export class Deployer { this.addresses.ZkSync.MailboxFacet = contractAddress; } - public async deployGovernanceFacet(create2Salt: string, ethTxOptions: ethers.providers.TransactionRequest) { + public async deployAdminFacet(create2Salt: string, ethTxOptions: ethers.providers.TransactionRequest) { ethTxOptions.gasLimit ??= 10_000_000; - const contractAddress = await this.deployViaCreate2('GovernanceFacet', [], create2Salt, ethTxOptions); + const contractAddress = await this.deployViaCreate2('AdminFacet', [], create2Salt, ethTxOptions); if (this.verbose) { - console.log(`CONTRACTS_GOVERNANCE_FACET_ADDR=${contractAddress}`); + console.log(`CONTRACTS_ADMIN_FACET_ADDR=${contractAddress}`); } - this.addresses.ZkSync.GovernanceFacet = contractAddress; - } - - public async deployDiamondCutFacet(create2Salt: string, ethTxOptions: ethers.providers.TransactionRequest) { - ethTxOptions.gasLimit ??= 10_000_000; - const contractAddress = await this.deployViaCreate2('DiamondCutFacet', [], create2Salt, ethTxOptions); - - if (this.verbose) { - console.log(`CONTRACTS_DIAMOND_CUT_FACET_ADDR=${contractAddress}`); - } - - this.addresses.ZkSync.DiamondCutFacet = contractAddress; + this.addresses.ZkSync.AdminFacet = contractAddress; } public async deployExecutorFacet(create2Salt: string, ethTxOptions: ethers.providers.TransactionRequest) { @@ -278,7 +286,7 @@ export class Deployer { ethTxOptions.gasLimit ??= 10_000_000; const contractAddress = await this.deployViaCreate2( 'TransparentUpgradeableProxy', - [this.addresses.Bridges.ERC20BridgeImplementation, this.governorAddress, '0x'], + [this.addresses.Bridges.ERC20BridgeImplementation, this.ownerAddress, '0x'], create2Salt, ethTxOptions ); @@ -325,7 +333,7 @@ export class Deployer { ethTxOptions.gasLimit ??= 10_000_000; const contractAddress = await this.deployViaCreate2( 'TransparentUpgradeableProxy', - [this.addresses.Bridges.WethBridgeImplementation, this.governorAddress, '0x'], + [this.addresses.Bridges.WethBridgeImplementation, this.ownerAddress, '0x'], create2Salt, ethTxOptions ); @@ -405,13 +413,12 @@ export class Deployer { const independentZkSyncDeployPromises = [ this.deployMailboxFacet(create2Salt, { gasPrice, nonce }), this.deployExecutorFacet(create2Salt, { gasPrice, nonce: nonce + 1 }), - this.deployDiamondCutFacet(create2Salt, { gasPrice, nonce: nonce + 2 }), - this.deployGovernanceFacet(create2Salt, { gasPrice, nonce: nonce + 3 }), - this.deployGettersFacet(create2Salt, { gasPrice, nonce: nonce + 4 }), - this.deployDiamondInit(create2Salt, { gasPrice, nonce: nonce + 5 }) + this.deployAdminFacet(create2Salt, { gasPrice, nonce: nonce + 2 }), + this.deployGettersFacet(create2Salt, { gasPrice, nonce: nonce + 3 }), + this.deployDiamondInit(create2Salt, { gasPrice, nonce: nonce + 4 }) ]; await Promise.all(independentZkSyncDeployPromises); - nonce += 6; + nonce += 5; await this.deployDiamondProxy(create2Salt, { gasPrice, nonce }); } @@ -436,7 +443,7 @@ export class Deployer { const validatorAddress = getAddressFromEnv('ETH_SENDER_SENDER_OPERATOR_COMMIT_ETH_ADDR'); const contractAddress = await this.deployViaCreate2( 'ValidatorTimelock', - [this.governorAddress, this.addresses.ZkSync.DiamondProxy, executionDelay, validatorAddress], + [this.ownerAddress, this.addresses.ZkSync.DiamondProxy, executionDelay, validatorAddress], create2Salt, ethTxOptions ); @@ -457,10 +464,18 @@ export class Deployer { } } + public transparentUpgradableProxyContract(address, signerOrProvider: Signer | providers.Provider) { + return ITransparentUpgradeableProxyFactory.connect(address, signerOrProvider); + } + public create2FactoryContract(signerOrProvider: Signer | providers.Provider) { return SingletonFactoryFactory.connect(this.addresses.Create2Factory, signerOrProvider); } + public governanceContract(signerOrProvider: Signer | providers.Provider) { + return IGovernanceFactory.connect(this.addresses.Governance, signerOrProvider); + } + public zkSyncContract(signerOrProvider: Signer | providers.Provider) { return IZkSyncFactory.connect(this.addresses.ZkSync.DiamondProxy, signerOrProvider); } diff --git a/ethereum/src.ts/diamondCut.ts b/ethereum/src.ts/diamondCut.ts index 9e1cf6a5f..f68946719 100644 --- a/ethereum/src.ts/diamondCut.ts +++ b/ethereum/src.ts/diamondCut.ts @@ -50,20 +50,19 @@ export function getAllSelectors(contractInterface: Interface) { } export async function getCurrentFacetCutsForAdd( - diamondCutFacetAddress: string, + adminAddress: string, gettersAddress: string, mailboxAddress: string, - executorAddress: string, - governanceAddress: string + executorAddress: string ) { const facetsCuts = {}; // Some facets should always be available regardless of freezing: upgradability system, getters, etc. // And for some facets there are should be possibility to freeze them by the governor if we found a bug inside. - if (diamondCutFacetAddress) { - // Should be unfreezable. The function to unfreeze contract is located on the diamond cut facet. - // That means if the diamond cut will be freezable, the proxy can NEVER be unfrozen. - const diamondCutFacet = await hardhat.ethers.getContractAt('DiamondCutFacet', diamondCutFacetAddress); - facetsCuts['DiamondCutFacet'] = facetCut(diamondCutFacet.address, diamondCutFacet.interface, Action.Add, false); + if (adminAddress) { + // Should be unfreezable. The function to unfreeze contract is located on the admin facet. + // That means if the admin facet will be freezable, the proxy can NEVER be unfrozen. + const adminFacet = await hardhat.ethers.getContractAt('AdminFacet', adminAddress); + facetsCuts['AdminFacet'] = facetCut(adminFacet.address, adminFacet.interface, Action.Add, false); } if (gettersAddress) { // Should be unfreezable. There are getters, that users can expect to be available. @@ -79,10 +78,7 @@ export async function getCurrentFacetCutsForAdd( const executor = await hardhat.ethers.getContractAt('ExecutorFacet', executorAddress); facetsCuts['ExecutorFacet'] = facetCut(executor.address, executor.interface, Action.Add, true); } - if (governanceAddress) { - const governance = await hardhat.ethers.getContractAt('GovernanceFacet', governanceAddress); - facetsCuts['GovernanceFacet'] = facetCut(governance.address, governance.interface, Action.Add, true); - } + return facetsCuts; } @@ -109,19 +105,12 @@ export async function getDeployedFacetCutsForRemove(wallet: Wallet, zkSyncAddres export async function getFacetCutsForUpgrade( wallet: Wallet, zkSyncAddress: string, - diamondCutFacetAddress: string, + adminAddress: string, gettersAddress: string, mailboxAddress: string, - executorAddress: string, - governanceAddress: string + executorAddress: string ) { - const newFacetCuts = await getCurrentFacetCutsForAdd( - diamondCutFacetAddress, - gettersAddress, - mailboxAddress, - executorAddress, - governanceAddress - ); + const newFacetCuts = await getCurrentFacetCutsForAdd(adminAddress, gettersAddress, mailboxAddress, executorAddress); const oldFacetCuts = await getDeployedFacetCutsForRemove(wallet, zkSyncAddress, Object.keys(newFacetCuts)); return [...oldFacetCuts, ...Object.values(newFacetCuts)]; } diff --git a/ethereum/test/foundry/unit/concrete/DiamondCut/UpgradeLogic.t.sol b/ethereum/test/foundry/unit/concrete/DiamondCut/UpgradeLogic.t.sol index b3dcfde07..17b0be157 100644 --- a/ethereum/test/foundry/unit/concrete/DiamondCut/UpgradeLogic.t.sol +++ b/ethereum/test/foundry/unit/concrete/DiamondCut/UpgradeLogic.t.sol @@ -4,12 +4,11 @@ pragma solidity ^0.8.17; // solhint-disable max-line-length import {DiamondCutTest} from "./_DiamondCut_Shared.t.sol"; -import {Utils} from "../Utils/Utils.sol"; import {DiamondCutTestContract} from "../../../../../cache/solpp-generated-contracts/dev-contracts/test/DiamondCutTestContract.sol"; import {DiamondInit} from "../../../../../cache/solpp-generated-contracts/zksync/DiamondInit.sol"; import {DiamondProxy} from "../../../../../cache/solpp-generated-contracts/zksync/DiamondProxy.sol"; import {VerifierParams} from "../../../../../cache/solpp-generated-contracts/zksync/Storage.sol"; -import {DiamondCutFacet} from "../../../../../cache/solpp-generated-contracts/zksync/facets/DiamondCut.sol"; +import {AdminFacet} from "../../../../../cache/solpp-generated-contracts/zksync/facets/Admin.sol"; import {GettersFacet} from "../../../../../cache/solpp-generated-contracts/zksync/facets/Getters.sol"; import {Diamond} from "../../../../../cache/solpp-generated-contracts/zksync/libraries/Diamond.sol"; @@ -18,22 +17,24 @@ import {Diamond} from "../../../../../cache/solpp-generated-contracts/zksync/lib contract UpgradeLogicTest is DiamondCutTest { DiamondProxy private diamondProxy; DiamondInit private diamondInit; - DiamondCutFacet private diamondCutFacet; - DiamondCutFacet private proxyAsDiamondCut; + AdminFacet private adminFacet; + AdminFacet private proxyAsAdmin; GettersFacet private proxyAsGetters; address private governor; address private randomSigner; - function getDiamondCutSelectors() private view returns (bytes4[] memory) { - bytes4[] memory dcSelectors = new bytes4[](8); - dcSelectors[0] = diamondCutFacet.proposeTransparentUpgrade.selector; - dcSelectors[1] = diamondCutFacet.proposeShadowUpgrade.selector; - dcSelectors[2] = diamondCutFacet.cancelUpgradeProposal.selector; - dcSelectors[3] = diamondCutFacet.securityCouncilUpgradeApprove.selector; - dcSelectors[4] = diamondCutFacet.executeUpgrade.selector; - dcSelectors[5] = diamondCutFacet.freezeDiamond.selector; - dcSelectors[6] = diamondCutFacet.unfreezeDiamond.selector; - dcSelectors[7] = diamondCutFacet.upgradeProposalHash.selector; + function getAdminSelectors() private view returns (bytes4[] memory) { + bytes4[] memory dcSelectors = new bytes4[](10); + dcSelectors[0] = adminFacet.setPendingGovernor.selector; + dcSelectors[1] = adminFacet.acceptGovernor.selector; + dcSelectors[2] = adminFacet.setPendingAdmin.selector; + dcSelectors[3] = adminFacet.acceptAdmin.selector; + dcSelectors[4] = adminFacet.setValidator.selector; + dcSelectors[5] = adminFacet.setPorterAvailability.selector; + dcSelectors[6] = adminFacet.setPriorityTxMaxGasLimit.selector; + dcSelectors[7] = adminFacet.executeUpgrade.selector; + dcSelectors[8] = adminFacet.freezeDiamond.selector; + dcSelectors[9] = adminFacet.unfreezeDiamond.selector; return dcSelectors; } @@ -43,15 +44,15 @@ contract UpgradeLogicTest is DiamondCutTest { diamondCutTestContract = new DiamondCutTestContract(); diamondInit = new DiamondInit(); - diamondCutFacet = new DiamondCutFacet(); + adminFacet = new AdminFacet(); gettersFacet = new GettersFacet(); Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](2); facetCuts[0] = Diamond.FacetCut({ - facet: address(diamondCutFacet), + facet: address(adminFacet), action: Diamond.Action.Add, isFreezable: false, - selectors: getDiamondCutSelectors() + selectors: getAdminSelectors() }); facetCuts[1] = Diamond.FacetCut({ facet: address(gettersFacet), @@ -70,6 +71,7 @@ contract UpgradeLogicTest is DiamondCutTest { diamondInit.initialize.selector, 0x03752D8252d67f99888E741E3fB642803B29B155, governor, + governor, 0x02c775f0a90abf7a0e8043f2fdc38f0580ca9f9996a895d05a501bfeaa3b2e21, 0, 0x0000000000000000000000000000000000000000000000000000000000000000, @@ -88,7 +90,7 @@ contract UpgradeLogicTest is DiamondCutTest { }); diamondProxy = new DiamondProxy(block.chainid, diamondCutData); - proxyAsDiamondCut = DiamondCutFacet(address(diamondProxy)); + proxyAsAdmin = AdminFacet(address(diamondProxy)); proxyAsGetters = GettersFacet(address(diamondProxy)); } @@ -96,129 +98,26 @@ contract UpgradeLogicTest is DiamondCutTest { vm.startPrank(randomSigner); vm.expectRevert(abi.encodePacked("1g")); - proxyAsDiamondCut.freezeDiamond(); + proxyAsAdmin.freezeDiamond(); } function test_RevertWhen_DoubleFreezingByGovernor() public { vm.startPrank(governor); - proxyAsDiamondCut.freezeDiamond(); + proxyAsAdmin.freezeDiamond(); vm.expectRevert(abi.encodePacked("a9")); - proxyAsDiamondCut.freezeDiamond(); + proxyAsAdmin.freezeDiamond(); } function test_RevertWhen_UnfreezingWhenNotFrozen() public { vm.startPrank(governor); vm.expectRevert(abi.encodePacked("a7")); - proxyAsDiamondCut.unfreezeDiamond(); - } - - function test_RevertWhen_ExecutingUnapprovedProposalWHenDiamondStorageIsFrozen() public { - vm.startPrank(governor); - - proxyAsDiamondCut.freezeDiamond(); - - Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](1); - facetCuts[0] = Diamond.FacetCut({ - facet: address(gettersFacet), - action: Diamond.Action.Replace, - isFreezable: true, - selectors: getGettersSelectors() - }); - - Diamond.DiamondCutData memory diamondCutData = Diamond.DiamondCutData({ - facetCuts: facetCuts, - initAddress: address(0), - initCalldata: bytes("") - }); - - proxyAsDiamondCut.proposeTransparentUpgrade(diamondCutData, 1); - - vm.expectRevert(abi.encodePacked("f3")); - proxyAsDiamondCut.executeUpgrade(diamondCutData, 0); - } - - function test_RevertWhen_ExecutingProposalWithDifferentInitAddress() public { - Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](1); - facetCuts[0] = Diamond.FacetCut({ - facet: address(gettersFacet), - action: Diamond.Action.Add, - isFreezable: true, - selectors: getGettersSelectors() - }); - - Diamond.DiamondCutData memory proposedDiamondCutData = Diamond.DiamondCutData({ - facetCuts: facetCuts, - initAddress: address(0), - initCalldata: bytes("") - }); - - Diamond.DiamondCutData memory executedDiamondCutData = Diamond.DiamondCutData({ - facetCuts: facetCuts, - initAddress: address(1), - initCalldata: bytes("") - }); - - uint40 nextProposalId = uint40(proxyAsGetters.getCurrentProposalId() + 1); - - vm.startPrank(governor); - - proxyAsDiamondCut.proposeTransparentUpgrade(proposedDiamondCutData, nextProposalId); - - vm.expectRevert(abi.encodePacked("a4")); - proxyAsDiamondCut.executeUpgrade(executedDiamondCutData, 0); - } - - function test_RevertWhen_ExecutingProposalWithDifferentFacetCut() public { - Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](1); - facetCuts[0] = Diamond.FacetCut({ - facet: address(gettersFacet), - action: Diamond.Action.Replace, - isFreezable: true, - selectors: getGettersSelectors() - }); - - Diamond.FacetCut[] memory invalidFacetCuts = new Diamond.FacetCut[](1); - invalidFacetCuts[0] = Diamond.FacetCut({ - facet: address(gettersFacet), - action: Diamond.Action.Replace, - isFreezable: false, - selectors: getGettersSelectors() - }); - - Diamond.DiamondCutData memory diamondCutData = Diamond.DiamondCutData({ - facetCuts: facetCuts, - initAddress: address(0), - initCalldata: bytes("") - }); - - Diamond.DiamondCutData memory invalidDiamondCutData = Diamond.DiamondCutData({ - facetCuts: invalidFacetCuts, - initAddress: address(0), - initCalldata: bytes("") - }); - - uint40 nextProposalId = uint40(proxyAsGetters.getCurrentProposalId() + 1); - - vm.startPrank(governor); - proxyAsDiamondCut.proposeTransparentUpgrade(diamondCutData, nextProposalId); - - vm.expectRevert(abi.encodePacked("a4")); - proxyAsDiamondCut.executeUpgrade(invalidDiamondCutData, 0); - } - - function test_RevertWhen_CancelingEmptyProposal() public { - bytes32 proposalHash = proxyAsGetters.getProposedUpgradeHash(); - - vm.startPrank(governor); - - vm.expectRevert(abi.encodePacked("a3")); - proxyAsDiamondCut.cancelUpgradeProposal(proposalHash); + proxyAsAdmin.unfreezeDiamond(); } - function test_ProposeAndExecuteDiamondCut() public { + function test_ExecuteDiamondCut() public { Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](1); facetCuts[0] = Diamond.FacetCut({ facet: address(gettersFacet), @@ -233,13 +132,9 @@ contract UpgradeLogicTest is DiamondCutTest { initCalldata: bytes("") }); - uint40 nextProposalId = uint40(proxyAsGetters.getCurrentProposalId() + 1); - vm.startPrank(governor); - proxyAsDiamondCut.proposeTransparentUpgrade(diamondCutData, nextProposalId); - - proxyAsDiamondCut.executeUpgrade(diamondCutData, 0); + proxyAsAdmin.executeUpgrade(diamondCutData); bytes4[] memory gettersFacetSelectors = getGettersSelectors(); for (uint256 i = 0; i < gettersFacetSelectors.length; i++) { @@ -253,7 +148,7 @@ contract UpgradeLogicTest is DiamondCutTest { } } - function test_RevertWhen_ExecutingSameProposalTwoTimes() public { + function test_ExecutingSameProposalTwoTimes() public { Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](1); facetCuts[0] = Diamond.FacetCut({ facet: address(gettersFacet), @@ -268,155 +163,9 @@ contract UpgradeLogicTest is DiamondCutTest { initCalldata: bytes("") }); - uint40 nextProposalId = uint40(proxyAsGetters.getCurrentProposalId() + 1); - vm.startPrank(governor); - proxyAsDiamondCut.proposeTransparentUpgrade(diamondCutData, nextProposalId); - - proxyAsDiamondCut.executeUpgrade(diamondCutData, 0); - - vm.expectRevert(abi.encodePacked("ab")); - proxyAsDiamondCut.executeUpgrade(diamondCutData, 0); - } - - function test_RevertWhen_ProposingAnAlreadyPropsedUpgrade() public { - Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](1); - facetCuts[0] = Diamond.FacetCut({ - facet: address(gettersFacet), - action: Diamond.Action.Replace, - isFreezable: true, - selectors: getGettersSelectors() - }); - - Diamond.DiamondCutData memory diamondCutData = Diamond.DiamondCutData({ - facetCuts: facetCuts, - initAddress: address(0), - initCalldata: bytes("") - }); - - uint40 nextProposalId = uint40(proxyAsGetters.getCurrentProposalId() + 1); - - vm.startPrank(governor); - - proxyAsDiamondCut.proposeTransparentUpgrade(diamondCutData, nextProposalId); - - vm.expectRevert(abi.encodePacked("a8")); - proxyAsDiamondCut.proposeTransparentUpgrade(diamondCutData, nextProposalId); - } - - function test_RevertWhen_ExecutingUnapprovedShadowUpgrade() public { - Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](1); - facetCuts[0] = Diamond.FacetCut({ - facet: address(gettersFacet), - action: Diamond.Action.Replace, - isFreezable: true, - selectors: getGettersSelectors() - }); - - Diamond.DiamondCutData memory diamondCutData = Diamond.DiamondCutData({ - facetCuts: facetCuts, - initAddress: address(0), - initCalldata: bytes("") - }); - - uint40 nextProposalId = uint40(proxyAsGetters.getCurrentProposalId() + 1); - - vm.startPrank(governor); - - bytes32 executingProposalHash = proxyAsDiamondCut.upgradeProposalHash(diamondCutData, nextProposalId, 0); - - proxyAsDiamondCut.proposeShadowUpgrade(executingProposalHash, nextProposalId); - - vm.expectRevert(abi.encodePacked("av")); - proxyAsDiamondCut.executeUpgrade(diamondCutData, 0); - } - - function test_RevertWhen_ProposingShadowUpgradeWithWrongProposalId() public { - uint40 nextProposalId = uint40(proxyAsGetters.getCurrentProposalId() + 1); - - vm.startPrank(governor); - - bytes32 porposalHash = Utils.randomBytes32("porposalHash"); - - vm.expectRevert(abi.encodePacked("ya")); - proxyAsDiamondCut.proposeShadowUpgrade(porposalHash, nextProposalId + 1); - } - - function test_RevertWhen_ProposingTransparentUpgradeWithWrongProposalId() public { - Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](1); - facetCuts[0] = Diamond.FacetCut({ - facet: address(gettersFacet), - action: Diamond.Action.Replace, - isFreezable: true, - selectors: getGettersSelectors() - }); - - Diamond.DiamondCutData memory diamondCutData = Diamond.DiamondCutData({ - facetCuts: facetCuts, - initAddress: address(0), - initCalldata: bytes("") - }); - - uint40 currentProposalId = uint40(proxyAsGetters.getCurrentProposalId()); - - vm.startPrank(governor); - - vm.expectRevert(abi.encodePacked("yb")); - proxyAsDiamondCut.proposeTransparentUpgrade(diamondCutData, currentProposalId); - } - - function test_RevertWhen_CancellingUpgradeProposalWithWrongHash() public { - Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](1); - facetCuts[0] = Diamond.FacetCut({ - facet: address(gettersFacet), - action: Diamond.Action.Replace, - isFreezable: true, - selectors: getGettersSelectors() - }); - - Diamond.DiamondCutData memory diamondCutData = Diamond.DiamondCutData({ - facetCuts: facetCuts, - initAddress: address(0), - initCalldata: bytes("") - }); - - uint40 nextProposalId = uint40(proxyAsGetters.getCurrentProposalId() + 1); - - vm.startPrank(governor); - - proxyAsDiamondCut.proposeTransparentUpgrade(diamondCutData, nextProposalId); - - bytes32 proposedUpgradeHash = Utils.randomBytes32("proposedUpgradeHash"); - - vm.expectRevert(abi.encodePacked("rx")); - proxyAsDiamondCut.cancelUpgradeProposal(proposedUpgradeHash); - } - - function test_RevertWhen_ExecutingTransparentUpgradeWithNonZeroSalt() public { - Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](1); - facetCuts[0] = Diamond.FacetCut({ - facet: address(gettersFacet), - action: Diamond.Action.Replace, - isFreezable: true, - selectors: getGettersSelectors() - }); - - Diamond.DiamondCutData memory diamondCutData = Diamond.DiamondCutData({ - facetCuts: facetCuts, - initAddress: address(0), - initCalldata: bytes("") - }); - - uint40 nextProposalId = uint40(proxyAsGetters.getCurrentProposalId() + 1); - - vm.startPrank(governor); - - proxyAsDiamondCut.proposeTransparentUpgrade(diamondCutData, nextProposalId); - - bytes32 proposalSalt = Utils.randomBytes32("proposalSalt"); - - vm.expectRevert(abi.encodePacked("po")); - proxyAsDiamondCut.executeUpgrade(diamondCutData, proposalSalt); + proxyAsAdmin.executeUpgrade(diamondCutData); + proxyAsAdmin.executeUpgrade(diamondCutData); } } diff --git a/ethereum/test/foundry/unit/concrete/DiamondCut/_DiamondCut_Shared.t.sol b/ethereum/test/foundry/unit/concrete/DiamondCut/_DiamondCut_Shared.t.sol index b7da4ebe4..5c0c9400e 100644 --- a/ethereum/test/foundry/unit/concrete/DiamondCut/_DiamondCut_Shared.t.sol +++ b/ethereum/test/foundry/unit/concrete/DiamondCut/_DiamondCut_Shared.t.sol @@ -14,7 +14,7 @@ contract DiamondCutTest is Test { GettersFacet internal gettersFacet; function getGettersSelectors() public view returns (bytes4[] memory) { - bytes4[] memory selectors = new bytes4[](35); + bytes4[] memory selectors = new bytes4[](29); selectors[0] = gettersFacet.getVerifier.selector; selectors[1] = gettersFacet.getGovernor.selector; selectors[2] = gettersFacet.getPendingGovernor.selector; @@ -32,24 +32,18 @@ contract DiamondCutTest is Test { selectors[14] = gettersFacet.getL2DefaultAccountBytecodeHash.selector; selectors[15] = gettersFacet.getVerifierParams.selector; selectors[16] = gettersFacet.isDiamondStorageFrozen.selector; - selectors[17] = gettersFacet.getSecurityCouncil.selector; - selectors[18] = gettersFacet.getUpgradeProposalState.selector; - selectors[19] = gettersFacet.getProposedUpgradeHash.selector; - selectors[20] = gettersFacet.getProposedUpgradeTimestamp.selector; - selectors[21] = gettersFacet.getCurrentProposalId.selector; - selectors[22] = gettersFacet.isApprovedBySecurityCouncil.selector; - selectors[23] = gettersFacet.getPriorityTxMaxGasLimit.selector; - selectors[24] = gettersFacet.getAllowList.selector; - selectors[25] = gettersFacet.isEthWithdrawalFinalized.selector; - selectors[26] = gettersFacet.facets.selector; - selectors[27] = gettersFacet.facetFunctionSelectors.selector; - selectors[28] = gettersFacet.facetAddresses.selector; - selectors[29] = gettersFacet.facetAddress.selector; - selectors[30] = gettersFacet.isFunctionFreezable.selector; - selectors[31] = gettersFacet.isFacetFreezable.selector; - selectors[32] = gettersFacet.getTotalBatchesCommitted.selector; - selectors[33] = gettersFacet.getTotalBatchesVerified.selector; - selectors[34] = gettersFacet.getTotalBatchesExecuted.selector; + selectors[17] = gettersFacet.getPriorityTxMaxGasLimit.selector; + selectors[18] = gettersFacet.getAllowList.selector; + selectors[19] = gettersFacet.isEthWithdrawalFinalized.selector; + selectors[20] = gettersFacet.facets.selector; + selectors[21] = gettersFacet.facetFunctionSelectors.selector; + selectors[22] = gettersFacet.facetAddresses.selector; + selectors[23] = gettersFacet.facetAddress.selector; + selectors[24] = gettersFacet.isFunctionFreezable.selector; + selectors[25] = gettersFacet.isFacetFreezable.selector; + selectors[26] = gettersFacet.getTotalBatchesCommitted.selector; + selectors[27] = gettersFacet.getTotalBatchesVerified.selector; + selectors[28] = gettersFacet.getTotalBatchesExecuted.selector; return selectors; } } diff --git a/ethereum/test/foundry/unit/concrete/Executor/_Executor_Shared.t.sol b/ethereum/test/foundry/unit/concrete/Executor/_Executor_Shared.t.sol index ce24e453d..b0166ca09 100644 --- a/ethereum/test/foundry/unit/concrete/Executor/_Executor_Shared.t.sol +++ b/ethereum/test/foundry/unit/concrete/Executor/_Executor_Shared.t.sol @@ -12,7 +12,7 @@ import {DiamondProxy} from "../../../../../cache/solpp-generated-contracts/zksyn import {VerifierParams} from "../../../../../cache/solpp-generated-contracts/zksync/Storage.sol"; import {ExecutorFacet} from "../../../../../cache/solpp-generated-contracts/zksync/facets/Executor.sol"; import {GettersFacet} from "../../../../../cache/solpp-generated-contracts/zksync/facets/Getters.sol"; -import {GovernanceFacet} from "../../../../../cache/solpp-generated-contracts/zksync/facets/Governance.sol"; +import {AdminFacet} from "../../../../../cache/solpp-generated-contracts/zksync/facets/Admin.sol"; import {MailboxFacet} from "../../../../../cache/solpp-generated-contracts/zksync/facets/Mailbox.sol"; import {IExecutor} from "../../../../../cache/solpp-generated-contracts/zksync/interfaces/IExecutor.sol"; import {Diamond} from "../../../../../cache/solpp-generated-contracts/zksync/libraries/Diamond.sol"; @@ -22,7 +22,7 @@ contract ExecutorTest is Test { address internal validator; address internal randomSigner; AllowList internal allowList; - GovernanceFacet internal governance; + AdminFacet internal admin; ExecutorFacet internal executor; GettersFacet internal getters; MailboxFacet internal mailbox; @@ -35,13 +35,18 @@ contract ExecutorTest is Test { IExecutor.StoredBatchInfo internal genesisStoredBatchInfo; IExecutor.ProofInput internal proofInput; - function getGovernanceSelectors() private view returns (bytes4[] memory) { - bytes4[] memory selectors = new bytes4[](5); - selectors[0] = governance.setPendingGovernor.selector; - selectors[1] = governance.acceptGovernor.selector; - selectors[2] = governance.setValidator.selector; - selectors[3] = governance.setPorterAvailability.selector; - selectors[4] = governance.setPriorityTxMaxGasLimit.selector; + function getAdminSelectors() private view returns (bytes4[] memory) { + bytes4[] memory selectors = new bytes4[](10); + selectors[0] = admin.setPendingGovernor.selector; + selectors[1] = admin.acceptGovernor.selector; + selectors[2] = admin.setPendingAdmin.selector; + selectors[3] = admin.acceptAdmin.selector; + selectors[4] = admin.setValidator.selector; + selectors[5] = admin.setPorterAvailability.selector; + selectors[6] = admin.setPriorityTxMaxGasLimit.selector; + selectors[7] = admin.executeUpgrade.selector; + selectors[8] = admin.freezeDiamond.selector; + selectors[9] = admin.unfreezeDiamond.selector; return selectors; } @@ -55,7 +60,7 @@ contract ExecutorTest is Test { } function getGettersSelectors() public view returns (bytes4[] memory) { - bytes4[] memory selectors = new bytes4[](35); + bytes4[] memory selectors = new bytes4[](29); selectors[0] = getters.getVerifier.selector; selectors[1] = getters.getGovernor.selector; selectors[2] = getters.getPendingGovernor.selector; @@ -73,24 +78,18 @@ contract ExecutorTest is Test { selectors[14] = getters.getL2DefaultAccountBytecodeHash.selector; selectors[15] = getters.getVerifierParams.selector; selectors[16] = getters.isDiamondStorageFrozen.selector; - selectors[17] = getters.getSecurityCouncil.selector; - selectors[18] = getters.getUpgradeProposalState.selector; - selectors[19] = getters.getProposedUpgradeHash.selector; - selectors[20] = getters.getProposedUpgradeTimestamp.selector; - selectors[21] = getters.getCurrentProposalId.selector; - selectors[22] = getters.isApprovedBySecurityCouncil.selector; - selectors[23] = getters.getPriorityTxMaxGasLimit.selector; - selectors[24] = getters.getAllowList.selector; - selectors[25] = getters.isEthWithdrawalFinalized.selector; - selectors[26] = getters.facets.selector; - selectors[27] = getters.facetFunctionSelectors.selector; - selectors[28] = getters.facetAddresses.selector; - selectors[29] = getters.facetAddress.selector; - selectors[30] = getters.isFunctionFreezable.selector; - selectors[31] = getters.isFacetFreezable.selector; - selectors[32] = getters.getTotalBatchesCommitted.selector; - selectors[33] = getters.getTotalBatchesVerified.selector; - selectors[34] = getters.getTotalBatchesExecuted.selector; + selectors[17] = getters.getPriorityTxMaxGasLimit.selector; + selectors[18] = getters.getAllowList.selector; + selectors[19] = getters.isEthWithdrawalFinalized.selector; + selectors[20] = getters.facets.selector; + selectors[21] = getters.facetFunctionSelectors.selector; + selectors[22] = getters.facetAddresses.selector; + selectors[23] = getters.facetAddress.selector; + selectors[24] = getters.isFunctionFreezable.selector; + selectors[25] = getters.isFacetFreezable.selector; + selectors[26] = getters.getTotalBatchesCommitted.selector; + selectors[27] = getters.getTotalBatchesVerified.selector; + selectors[28] = getters.getTotalBatchesExecuted.selector; return selectors; } @@ -111,7 +110,7 @@ contract ExecutorTest is Test { randomSigner = makeAddr("randomSigner"); executor = new ExecutorFacet(); - governance = new GovernanceFacet(); + admin = new AdminFacet(); getters = new GettersFacet(); mailbox = new MailboxFacet(); @@ -124,6 +123,7 @@ contract ExecutorTest is Test { diamondInit.initialize.selector, dummyAddress, //verifier owner, + owner, 0, 0, 0, @@ -137,10 +137,10 @@ contract ExecutorTest is Test { Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](4); facetCuts[0] = Diamond.FacetCut({ - facet: address(governance), + facet: address(admin), action: Diamond.Action.Add, isFreezable: true, - selectors: getGovernanceSelectors() + selectors: getAdminSelectors() }); facetCuts[1] = Diamond.FacetCut({ facet: address(executor), @@ -176,10 +176,10 @@ contract ExecutorTest is Test { executor = ExecutorFacet(address(diamondProxy)); getters = GettersFacet(address(diamondProxy)); mailbox = MailboxFacet(address(diamondProxy)); - governance = GovernanceFacet(address(diamondProxy)); + admin = AdminFacet(address(diamondProxy)); vm.prank(owner); - governance.setValidator(validator, true); + admin.setValidator(validator, true); uint256[] memory recursiveAggregationInput; uint256[] memory serializedProof; diff --git a/ethereum/test/unit_tests/governance_test.spec.ts b/ethereum/test/unit_tests/governance_test.spec.ts index 9fbd3a9ff..5d107ab67 100644 --- a/ethereum/test/unit_tests/governance_test.spec.ts +++ b/ethereum/test/unit_tests/governance_test.spec.ts @@ -1,6 +1,6 @@ import { expect } from 'chai'; import * as hardhat from 'hardhat'; -import { GovernanceFacetTest, GovernanceFacetTestFactory } from '../../typechain'; +import { AdminFacetTest, AdminFacetTestFactory, GovernanceFactory } from '../../typechain'; import { getCallRevertReason } from './utils'; import * as ethers from 'ethers'; @@ -8,59 +8,65 @@ function randomAddress() { return ethers.utils.hexlify(ethers.utils.randomBytes(20)); } -describe('Governance facet tests', function () { - let governanceTest: GovernanceFacetTest; +describe('Admin facet tests', function () { + let adminFacetTest: AdminFacetTest; let randomSigner: ethers.Signer; before(async () => { - const contractFactory = await hardhat.ethers.getContractFactory('GovernanceFacetTest'); + const contractFactory = await hardhat.ethers.getContractFactory('AdminFacetTest'); const contract = await contractFactory.deploy(); - governanceTest = GovernanceFacetTestFactory.connect(contract.address, contract.signer); + adminFacetTest = AdminFacetTestFactory.connect(contract.address, contract.signer); + + const governanceFactory = await hardhat.ethers.getContractFactory('Governance'); + const governanceContract = await contractFactory.deploy(); + const governance = GovernanceFactory.connect(governanceContract.address, governanceContract.signer); + await adminFacetTest.setPendingGovernor(governance.address); + randomSigner = (await hardhat.ethers.getSigners())[1]; }); it('governor successfully set validator', async () => { const validatorAddress = randomAddress(); - await governanceTest.setValidator(validatorAddress, true); + await adminFacetTest.setValidator(validatorAddress, true); - const isValidator = await governanceTest.isValidator(validatorAddress); + const isValidator = await adminFacetTest.isValidator(validatorAddress); expect(isValidator).to.equal(true); }); it('random account fails to set validator', async () => { const validatorAddress = randomAddress(); const revertReason = await getCallRevertReason( - governanceTest.connect(randomSigner).setValidator(validatorAddress, true) + adminFacetTest.connect(randomSigner).setValidator(validatorAddress, true) ); - expect(revertReason).equal('1g'); + expect(revertReason).equal('Only by governor or admin'); }); it('governor successfully set porter availability', async () => { - await governanceTest.setPorterAvailability(true); + await adminFacetTest.setPorterAvailability(true); - const porterAvailability = await governanceTest.getPorterAvailability(); + const porterAvailability = await adminFacetTest.getPorterAvailability(); expect(porterAvailability).to.equal(true); }); it('random account fails to set porter availability', async () => { const revertReason = await getCallRevertReason( - governanceTest.connect(randomSigner).setPorterAvailability(false) + adminFacetTest.connect(randomSigner).setPorterAvailability(false) ); expect(revertReason).equal('1g'); }); it('governor successfully set priority transaction max gas limit', async () => { const gasLimit = '12345678'; - await governanceTest.setPriorityTxMaxGasLimit(gasLimit); + await adminFacetTest.setPriorityTxMaxGasLimit(gasLimit); - const newGasLimit = await governanceTest.getPriorityTxMaxGasLimit(); + const newGasLimit = await adminFacetTest.getPriorityTxMaxGasLimit(); expect(newGasLimit).to.equal(gasLimit); }); it('random account fails to priority transaction max gas limit', async () => { const gasLimit = '123456789'; const revertReason = await getCallRevertReason( - governanceTest.connect(randomSigner).setPriorityTxMaxGasLimit(gasLimit) + adminFacetTest.connect(randomSigner).setPriorityTxMaxGasLimit(gasLimit) ); expect(revertReason).equal('1g'); }); @@ -74,29 +80,29 @@ describe('Governance facet tests', function () { it('set pending governor', async () => { const proposedGovernor = await randomSigner.getAddress(); - await governanceTest.setPendingGovernor(proposedGovernor); + await adminFacetTest.setPendingGovernor(proposedGovernor); - const pendingGovernor = await governanceTest.getPendingGovernor(); + const pendingGovernor = await adminFacetTest.getPendingGovernor(); expect(pendingGovernor).equal(proposedGovernor); }); it('reset pending governor', async () => { const proposedGovernor = await newGovernor.getAddress(); - await governanceTest.setPendingGovernor(proposedGovernor); + await adminFacetTest.setPendingGovernor(proposedGovernor); - const pendingGovernor = await governanceTest.getPendingGovernor(); + const pendingGovernor = await adminFacetTest.getPendingGovernor(); expect(pendingGovernor).equal(proposedGovernor); }); it('failed to accept governor from not proposed account', async () => { - const revertReason = await getCallRevertReason(governanceTest.connect(randomSigner).acceptGovernor()); + const revertReason = await getCallRevertReason(adminFacetTest.connect(randomSigner).acceptGovernor()); expect(revertReason).equal('n4'); }); it('accept governor from proposed account', async () => { - await governanceTest.connect(newGovernor).acceptGovernor(); + await adminFacetTest.connect(newGovernor).acceptGovernor(); - const governor = await governanceTest.getGovernor(); + const governor = await adminFacetTest.getGovernor(); expect(governor).equal(await newGovernor.getAddress()); }); }); diff --git a/ethereum/test/unit_tests/l1_erc20_bridge_test.spec.ts b/ethereum/test/unit_tests/l1_erc20_bridge_test.spec.ts index 6abd65414..399e2742d 100644 --- a/ethereum/test/unit_tests/l1_erc20_bridge_test.spec.ts +++ b/ethereum/test/unit_tests/l1_erc20_bridge_test.spec.ts @@ -52,6 +52,7 @@ describe(`L1ERC20Bridge tests`, function () { const diamondInitData = diamondInit.interface.encodeFunctionData('initialize', [ dummyAddress, await owner.getAddress(), + await owner.getAddress(), ethers.constants.HashZero, 0, ethers.constants.HashZero, diff --git a/ethereum/test/unit_tests/l1_weth_bridge_test.spec.ts b/ethereum/test/unit_tests/l1_weth_bridge_test.spec.ts index 6587d7720..5306aedae 100644 --- a/ethereum/test/unit_tests/l1_weth_bridge_test.spec.ts +++ b/ethereum/test/unit_tests/l1_weth_bridge_test.spec.ts @@ -84,6 +84,7 @@ describe('WETH Bridge tests', () => { const diamondInitData = diamondInit.interface.encodeFunctionData('initialize', [ dummyAddress, await owner.getAddress(), + await owner.getAddress(), ethers.constants.HashZero, 0, ethers.constants.HashZero, diff --git a/ethereum/test/unit_tests/l2-upgrade.test.spec.ts b/ethereum/test/unit_tests/l2-upgrade.test.spec.ts index b539af3c6..460799510 100644 --- a/ethereum/test/unit_tests/l2-upgrade.test.spec.ts +++ b/ethereum/test/unit_tests/l2-upgrade.test.spec.ts @@ -7,11 +7,10 @@ import { AllowList, ExecutorFacet, ExecutorFacetFactory, - DiamondCutFacetFactory, GettersFacetFactory, - GovernanceFacetFactory, GettersFacet, - GovernanceFacet, + AdminFacet, + AdminFacetFactory, DefaultUpgradeFactory, CustomUpgradeTestFactory } from '../../typechain'; @@ -32,16 +31,14 @@ import { } from './utils'; import * as ethers from 'ethers'; import { BigNumber, BigNumberish, BytesLike } from 'ethers'; -import { DiamondCutFacet } from '../../typechain'; import { REQUIRED_L1_TO_L2_GAS_PER_PUBDATA_LIMIT, hashBytecode } from 'zksync-web3/build/src/utils'; const SYSTEM_UPGRADE_TX_TYPE = 254; describe('L2 upgrade test', function () { let proxyExecutor: ExecutorFacet; - let proxyDiamondCut: DiamondCutFacet; + let proxyAdmin: AdminFacet; let proxyGetters: GettersFacet; - let proxyGovernance: GovernanceFacet; let allowList: AllowList; let diamondProxyContract: ethers.Contract; @@ -65,17 +62,13 @@ describe('L2 upgrade test', function () { const gettersContract = await gettersFactory.deploy(); const gettersFacet = GettersFacetFactory.connect(gettersContract.address, gettersContract.signer); - const diamondCutFacetFactory = await hardhat.ethers.getContractFactory('DiamondCutFacet'); - const diamondCutFacetContract = await diamondCutFacetFactory.deploy(); - const diamondCutFacet = DiamondCutFacetFactory.connect( - diamondCutFacetContract.address, - diamondCutFacetContract.signer + const adminFacetFactory = await hardhat.ethers.getContractFactory('AdminFacet'); + const adminFacetContract = await adminFacetFactory.deploy(); + const adminFacet = AdminFacetFactory.connect( + adminFacetContract.address, + adminFacetContract.signer ); - const governanceFactory = await hardhat.ethers.getContractFactory('GovernanceFacet'); - const governanceContract = await governanceFactory.deploy(); - const governanceFacet = GovernanceFacetFactory.connect(governanceContract.address, governanceContract.signer); - const allowListFactory = await hardhat.ethers.getContractFactory('AllowList'); const allowListContract = await allowListFactory.deploy(await allowListFactory.signer.getAddress()); allowList = AllowListFactory.connect(allowListContract.address, allowListContract.signer); @@ -97,6 +90,7 @@ describe('L2 upgrade test', function () { const diamondInitData = diamondInit.interface.encodeFunctionData('initialize', [ verifier, await owner.getAddress(), + await owner.getAddress(), ethers.constants.HashZero, 0, ethers.constants.HashZero, @@ -109,13 +103,12 @@ describe('L2 upgrade test', function () { ]); const facetCuts = [ - // Should be unfreezable. The function to unfreeze contract is located on the diamond cut facet. - // That means if the diamond cut will be freezable, the proxy can NEVER be unfrozen. - facetCut(diamondCutFacet.address, diamondCutFacet.interface, Action.Add, false), + // Should be unfreezable. The function to unfreeze contract is located on the admin facet. + // That means if the admin will be freezable, the proxy can NEVER be unfrozen. + facetCut(adminFacet.address, adminFacet.interface, Action.Add, false), // Should be unfreezable. There are getters, that users can expect to be available. facetCut(gettersFacet.address, gettersFacet.interface, Action.Add, false), - facetCut(executorFacet.address, executorFacet.interface, Action.Add, true), - facetCut(governanceFacet.address, governanceFacet.interface, Action.Add, true) + facetCut(executorFacet.address, executorFacet.interface, Action.Add, true) ]; const diamondCutData = diamondCut(facetCuts, diamondInit.address, diamondInitData); @@ -128,10 +121,9 @@ describe('L2 upgrade test', function () { proxyExecutor = ExecutorFacetFactory.connect(diamondProxyContract.address, owner); proxyGetters = GettersFacetFactory.connect(diamondProxyContract.address, owner); - proxyDiamondCut = DiamondCutFacetFactory.connect(diamondProxyContract.address, owner); - proxyGovernance = GovernanceFacetFactory.connect(diamondProxyContract.address, owner); + proxyAdmin = AdminFacetFactory.connect(diamondProxyContract.address, owner); - await (await proxyGovernance.setValidator(await owner.getAddress(), true)).wait(); + await (await proxyAdmin.setValidator(await owner.getAddress(), true)).wait(); }); it('Upgrade should work even if not all batches are processed', async () => { @@ -146,7 +138,7 @@ describe('L2 upgrade test', function () { expect(await proxyGetters.getL2SystemContractsUpgradeTxHash()).to.equal(ethers.constants.HashZero); await ( - await executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + await executeUpgrade(proxyGetters, proxyAdmin, { newProtocolVersion: 1, l2ProtocolUpgradeTx: noopUpgradeTransaction }) @@ -162,21 +154,19 @@ describe('L2 upgrade test', function () { it('Timestamp should behave correctly', async () => { // Upgrade was scheduled for now should work fine const timeNow = (await hardhat.ethers.provider.getBlock('latest')).timestamp; - await executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + await executeUpgrade(proxyGetters, proxyAdmin, { upgradeTimestamp: ethers.BigNumber.from(timeNow), l2ProtocolUpgradeTx: noopUpgradeTransaction }); // Upgrade that was scheduled for the future should not work now const revertReason = await getCallRevertReason( - executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + executeUpgrade(proxyGetters, proxyAdmin, { upgradeTimestamp: ethers.BigNumber.from(timeNow).mul(2), l2ProtocolUpgradeTx: noopUpgradeTransaction }) ); - expect(revertReason).to.equal('Upgrade is not ready yet'); - - await proxyDiamondCut.cancelUpgradeProposal(await proxyGetters.getProposedUpgradeHash()); + expect(revertReason).to.equal('Upgrade is not ready yet'); }); it('Should require correct tx type for upgrade tx', async () => { @@ -184,14 +174,12 @@ describe('L2 upgrade test', function () { txType: 255 }); const revertReason = await getCallRevertReason( - executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + executeUpgrade(proxyGetters, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx }) ); expect(revertReason).to.equal('L2 system upgrade tx type is wrong'); - - await proxyDiamondCut.cancelUpgradeProposal(await proxyGetters.getProposedUpgradeHash()); }); it('Should include the new protocol version as part of nonce', async () => { @@ -201,15 +189,13 @@ describe('L2 upgrade test', function () { }); const revertReason = await getCallRevertReason( - executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + executeUpgrade(proxyGetters, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, newProtocolVersion: 3 }) ); expect(revertReason).to.equal('The new protocol version should be included in the L2 system upgrade tx'); - - await proxyDiamondCut.cancelUpgradeProposal(await proxyGetters.getProposedUpgradeHash()); }); it('Should ensure monotonic protocol version', async () => { @@ -219,15 +205,13 @@ describe('L2 upgrade test', function () { }); const revertReason = await getCallRevertReason( - executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + executeUpgrade(proxyGetters, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, newProtocolVersion: 0 }) ); expect(revertReason).to.equal('New protocol version is not greater than the current one'); - - await proxyDiamondCut.cancelUpgradeProposal(await proxyGetters.getProposedUpgradeHash()); }); it('Should validate upgrade transaction overhead', async () => { @@ -237,15 +221,13 @@ describe('L2 upgrade test', function () { }); const revertReason = await getCallRevertReason( - executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + executeUpgrade(proxyGetters, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, newProtocolVersion: 3 }) ); expect(revertReason).to.equal('my'); - - await proxyDiamondCut.cancelUpgradeProposal(await proxyGetters.getProposedUpgradeHash()); }); it('Should validate upgrade transaction gas max', async () => { @@ -255,15 +237,13 @@ describe('L2 upgrade test', function () { }); const revertReason = await getCallRevertReason( - executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + executeUpgrade(proxyGetters, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, newProtocolVersion: 3 }) ); expect(revertReason).to.equal('ui'); - - await proxyDiamondCut.cancelUpgradeProposal(await proxyGetters.getProposedUpgradeHash()); }); it('Should validate upgrade transaction cant output more pubdata than processable', async () => { @@ -274,15 +254,13 @@ describe('L2 upgrade test', function () { }); const revertReason = await getCallRevertReason( - executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + executeUpgrade(proxyGetters, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, newProtocolVersion: 3 }) ); expect(revertReason).to.equal('uk'); - - await proxyDiamondCut.cancelUpgradeProposal(await proxyGetters.getProposedUpgradeHash()); }); it('Should validate factory deps', async () => { @@ -294,7 +272,7 @@ describe('L2 upgrade test', function () { }); const revertReason = await getCallRevertReason( - executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + executeUpgrade(proxyGetters, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, factoryDeps: [myFactoryDep], newProtocolVersion: 3 @@ -302,8 +280,6 @@ describe('L2 upgrade test', function () { ); expect(revertReason).to.equal('Wrong factory dep hash'); - - await proxyDiamondCut.cancelUpgradeProposal(await proxyGetters.getProposedUpgradeHash()); }); it('Should validate factory deps length match', async () => { @@ -314,7 +290,7 @@ describe('L2 upgrade test', function () { }); const revertReason = await getCallRevertReason( - executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + executeUpgrade(proxyGetters, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, factoryDeps: [myFactoryDep], newProtocolVersion: 3 @@ -322,8 +298,6 @@ describe('L2 upgrade test', function () { ); expect(revertReason).to.equal('Wrong number of factory deps'); - - await proxyDiamondCut.cancelUpgradeProposal(await proxyGetters.getProposedUpgradeHash()); }); it('Should validate factory deps length isnt too large', async () => { @@ -336,7 +310,7 @@ describe('L2 upgrade test', function () { }); const revertReason = await getCallRevertReason( - executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + executeUpgrade(proxyGetters, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, factoryDeps: Array(33).fill(myFactoryDep), newProtocolVersion: 3 @@ -344,8 +318,6 @@ describe('L2 upgrade test', function () { ); expect(revertReason).to.equal('Factory deps can be at most 32'); - - await proxyDiamondCut.cancelUpgradeProposal(await proxyGetters.getProposedUpgradeHash()); }); let l2UpgradeTxHash: string; @@ -377,7 +349,7 @@ describe('L2 upgrade test', function () { newProtocolVersion: 4 }; - const upgradeReceipt = await (await executeTransparentUpgrade(proxyGetters, proxyDiamondCut, upgrade)).wait(); + const upgradeReceipt = await (await executeUpgrade(proxyGetters, proxyAdmin, upgrade)).wait(); const defaultUpgradeFactory = await hardhat.ethers.getContractFactory('DefaultUpgrade'); const upgradeEvents = upgradeReceipt.logs.map((log) => { @@ -461,11 +433,9 @@ describe('L2 upgrade test', function () { newProtocolVersion: 5 }; const revertReason = await getCallRevertReason( - executeTransparentUpgrade(proxyGetters, proxyDiamondCut, upgrade) + executeUpgrade(proxyGetters, proxyAdmin, upgrade) ); - await proxyDiamondCut.cancelUpgradeProposal(await proxyGetters.getProposedUpgradeHash()); - expect(revertReason).to.equal('Previous upgrade has not been finalized'); }); @@ -633,7 +603,7 @@ describe('L2 upgrade test', function () { it('Should successfully commit a sequential upgrade', async () => { expect(await proxyGetters.getL2SystemContractsUpgradeBatchNumber()).to.equal(0); await ( - await executeTransparentUpgrade(proxyGetters, proxyDiamondCut, { + await executeUpgrade(proxyGetters, proxyAdmin, { newProtocolVersion: 5, l2ProtocolUpgradeTx: noopUpgradeTransaction }) @@ -674,7 +644,7 @@ describe('L2 upgrade test', function () { it('Should successfully commit custom upgrade', async () => { const upgradeReceipt = await ( - await executeCustomTransparentUpgrade(proxyGetters, proxyDiamondCut, { + await executeCustomUpgrade(proxyGetters, proxyAdmin, { newProtocolVersion: 6, l2ProtocolUpgradeTx: noopUpgradeTransaction }) @@ -737,22 +707,6 @@ interface L2ToL1Log { value: string; } -function contextLog(timestamp: number, prevBatchHash: BytesLike): L2ToL1Log { - return { - sender: L2_SYSTEM_CONTEXT_ADDRESS, - key: packBatchTimestampAndBatchTimestamp(timestamp, timestamp), - value: ethers.utils.hexlify(prevBatchHash) - }; -} - -function bootloaderLog(txHash: BytesLike): L2ToL1Log { - return { - sender: L2_BOOTLOADER_ADDRESS, - key: ethers.utils.hexlify(txHash), - value: ethers.utils.hexlify(BigNumber.from(1)) - }; -} - function encodeLog(log: L2ToL1Log): string { return ethers.utils.hexConcat([ `0x00000000`, @@ -937,9 +891,9 @@ function buildProposeUpgrade(proposedUpgrade: PartialProposedUpgrade): ProposedU }; } -async function executeTransparentUpgrade( +async function executeUpgrade( proxyGetters: GettersFacet, - proxyDiamondCut: DiamondCutFacet, + proxyAdmin: AdminFacet, partialUpgrade: Partial, contractFactory?: ethers.ethers.ContractFactory ) { @@ -948,7 +902,6 @@ async function executeTransparentUpgrade( partialUpgrade.newProtocolVersion = newVersion; } const upgrade = buildProposeUpgrade(partialUpgrade); - const proposalId = (await proxyGetters.getCurrentProposalId()).add(1); const defaultUpgradeFactory = contractFactory ? contractFactory @@ -961,15 +914,13 @@ async function executeTransparentUpgrade( const diamondCutData = diamondCut([], diamondUpgradeInit.address, upgradeCalldata); - await (await proxyDiamondCut.proposeTransparentUpgrade(diamondCutData, proposalId)).wait(); - // This promise will be handled in the tests - return proxyDiamondCut.executeUpgrade(diamondCutData, ethers.constants.HashZero); + return proxyAdmin.executeUpgrade(diamondCutData); } -async function executeCustomTransparentUpgrade( +async function executeCustomUpgrade( proxyGetters: GettersFacet, - proxyDiamondCut: DiamondCutFacet, + proxyAdmin: AdminFacet, partialUpgrade: Partial, contractFactory?: ethers.ethers.ContractFactory ) { @@ -978,7 +929,6 @@ async function executeCustomTransparentUpgrade( partialUpgrade.newProtocolVersion = newVersion; } const upgrade = buildProposeUpgrade(partialUpgrade); - const proposalId = (await proxyGetters.getCurrentProposalId()).add(1); const upgradeFactory = contractFactory ? contractFactory @@ -991,10 +941,8 @@ async function executeCustomTransparentUpgrade( const diamondCutData = diamondCut([], diamondUpgradeInit.address, upgradeCalldata); - await (await proxyDiamondCut.proposeTransparentUpgrade(diamondCutData, proposalId)).wait(); - // This promise will be handled in the tests - return proxyDiamondCut.executeUpgrade(diamondCutData, ethers.constants.HashZero); + return proxyAdmin.executeUpgrade(diamondCutData); } async function makeExecutedEqualCommitted( diff --git a/ethereum/test/unit_tests/mailbox_test.spec.ts b/ethereum/test/unit_tests/mailbox_test.spec.ts index fc279915b..10199c18d 100644 --- a/ethereum/test/unit_tests/mailbox_test.spec.ts +++ b/ethereum/test/unit_tests/mailbox_test.spec.ts @@ -60,6 +60,7 @@ describe('Mailbox tests', function () { dummyHash.set([1, 0, 0, 1]); const dummyAddress = ethers.utils.hexlify(ethers.utils.randomBytes(20)); const diamondInitData = diamondInit.interface.encodeFunctionData('initialize', [ + dummyAddress, dummyAddress, dummyAddress, ethers.constants.HashZero, diff --git a/ethereum/test/unit_tests/proxy_test.spec.ts b/ethereum/test/unit_tests/proxy_test.spec.ts index 88da78827..ce52454a1 100644 --- a/ethereum/test/unit_tests/proxy_test.spec.ts +++ b/ethereum/test/unit_tests/proxy_test.spec.ts @@ -7,8 +7,8 @@ import { DiamondProxyFactory, DiamondProxyTest, DiamondProxyTestFactory, - DiamondCutFacet, - DiamondCutFacetFactory, + AdminFacet, + AdminFacetFactory, GettersFacet, GettersFacetFactory, MailboxFacet, @@ -22,7 +22,7 @@ import { getCallRevertReason } from './utils'; describe('Diamond proxy tests', function () { let proxy: DiamondProxy; let diamondInit: DiamondInit; - let diamondCutFacet: DiamondCutFacet; + let adminFacet: AdminFacet; let gettersFacet: GettersFacet; let mailboxFacet: MailboxFacet; let diamondProxyTest: DiamondProxyTest; @@ -37,9 +37,9 @@ describe('Diamond proxy tests', function () { const diamondInitContract = await diamondInitFactory.deploy(); diamondInit = DiamondInitFactory.connect(diamondInitContract.address, diamondInitContract.signer); - const diamondCutFactory = await hardhat.ethers.getContractFactory('DiamondCutFacet'); - const diamondCutContract = await diamondCutFactory.deploy(); - diamondCutFacet = DiamondCutFacetFactory.connect(diamondCutContract.address, diamondCutContract.signer); + const adminFactory = await hardhat.ethers.getContractFactory('AdminFacet'); + const adminContract = await adminFactory.deploy(); + adminFacet = AdminFacetFactory.connect(adminContract.address, adminContract.signer); const gettersFacetFactory = await hardhat.ethers.getContractFactory('GettersFacet'); const gettersFacetContract = await gettersFacetFactory.deploy(); @@ -57,7 +57,7 @@ describe('Diamond proxy tests', function () { ); const facetCuts = [ - facetCut(diamondCutFacet.address, diamondCutFacet.interface, Action.Add, false), + facetCut(adminFacet.address, adminFacet.interface, Action.Add, false), facetCut(gettersFacet.address, gettersFacet.interface, Action.Add, false), facetCut(mailboxFacet.address, mailboxFacet.interface, Action.Add, true) ]; @@ -70,6 +70,7 @@ describe('Diamond proxy tests', function () { const diamondInitCalldata = diamondInit.interface.encodeFunctionData('initialize', [ '0x03752D8252d67f99888E741E3fB642803B29B155', governorAddress, + governorAddress, '0x02c775f0a90abf7a0e8043f2fdc38f0580ca9f9996a895d05a501bfeaa3b2e21', 0, '0x0000000000000000000000000000000000000000000000000000000000000000', @@ -100,11 +101,11 @@ describe('Diamond proxy tests', function () { expect(isFreezable).equal(false); } - const diamondCutSelectors = getAllSelectors(diamondCutFacet.interface); + const diamondCutSelectors = getAllSelectors(adminFacet.interface); for (const selector of diamondCutSelectors) { const addr = await proxyAsGettersFacet.facetAddress(selector); const isFreezable = await proxyAsGettersFacet.isFunctionFreezable(selector); - expect(addr).equal(diamondCutFacet.address); + expect(addr).equal(adminFacet.address); expect(isFreezable).equal(false); } }); @@ -129,24 +130,18 @@ describe('Diamond proxy tests', function () { const diamondProxyTestCalldata = diamondProxyTest.interface.encodeFunctionData('setFreezability', [true]); const diamondCutInitData = diamondCut([], diamondProxyTest.address, diamondProxyTestCalldata); - const diamondCutFacetProposeCalldata = diamondCutFacet.interface.encodeFunctionData( - 'proposeTransparentUpgrade', - [diamondCutInitData, 1] - ); - await proxy.fallback({ data: diamondCutFacetProposeCalldata }); - const diamondCutFacetExecuteCalldata = diamondCutFacet.interface.encodeFunctionData('executeUpgrade', [ - diamondCutInitData, - ethers.constants.HashZero + const adminFacetExecuteCalldata = adminFacet.interface.encodeFunctionData('executeUpgrade', [ + diamondCutInitData ]); - await proxy.fallback({ data: diamondCutFacetExecuteCalldata }); + await proxy.fallback({ data: adminFacetExecuteCalldata }); expect(await proxyAsGettersFacet.isDiamondStorageFrozen()).equal(true); }); - it('should revert on executing a proposal when diamondStorage is frozen', async () => { + it('should not revert on executing a proposal when diamondStorage is frozen', async () => { const facetCuts = [ { - facet: diamondCutFacet.address, + facet: adminFacet.address, selectors: ['0x000000aa'], action: Action.Add, isFreezable: false @@ -154,18 +149,10 @@ describe('Diamond proxy tests', function () { ]; const diamondCutData = diamondCut(facetCuts, ethers.constants.AddressZero, '0x'); - const diamondCutFacetProposeCalldata = diamondCutFacet.interface.encodeFunctionData( - 'proposeTransparentUpgrade', - [diamondCutData, 2] - ); - await proxy.fallback({ data: diamondCutFacetProposeCalldata }); - const diamondCutFacetExecuteCalldata = diamondCutFacet.interface.encodeFunctionData('executeUpgrade', [ - diamondCutData, - ethers.constants.HashZero + const adminFacetExecuteCalldata = adminFacet.interface.encodeFunctionData('executeUpgrade', [ + diamondCutData ]); - const revertReason = await getCallRevertReason(proxy.fallback({ data: diamondCutFacetExecuteCalldata })); - - expect(revertReason).equal('f3'); + await proxy.fallback({ data: adminFacetExecuteCalldata }); }); it('should revert on calling a freezable faucet when diamondStorage is frozen', async () => { diff --git a/ethereum/test/unit_tests/utils.ts b/ethereum/test/unit_tests/utils.ts index 63250fe69..21fb548a7 100644 --- a/ethereum/test/unit_tests/utils.ts +++ b/ethereum/test/unit_tests/utils.ts @@ -131,25 +131,6 @@ export function createSystemLogs() { ]; } -export async function setSecurityCouncil( - proxyAsGetters: ethers.Contract, - proxyAsDiamondCut: ethers.Contract, - securityCouncil: ethers.Signer -) { - const diamondUpgradeSecurityCouncilFactory = await hardhat.ethers.getContractFactory( - 'DiamondUpgradeSecurityCouncil' - ); - const diamondUpgradeSecurityCouncilContract = await diamondUpgradeSecurityCouncilFactory.deploy(); - const calldata = diamondUpgradeSecurityCouncilContract.interface.encodeFunctionData('upgrade', [ - await securityCouncil.getAddress() - ]); - const diamondCutData = diamondCut([], diamondUpgradeSecurityCouncilContract.address, calldata); - - const nextProposalId = (await proxyAsGetters.getCurrentProposalId()).add(1); - await proxyAsDiamondCut.proposeTransparentUpgrade(diamondCutData, nextProposalId); - await proxyAsDiamondCut.executeUpgrade(diamondCutData, ethers.constants.HashZero); -} - export function genesisStoredBatchInfo(): StoredBatchInfo { return { batchNumber: 0, diff --git a/zksync/src/deployL2Weth.ts b/zksync/src/deployL2Weth.ts index 191bf3f1a..7c61efb6c 100644 --- a/zksync/src/deployL2Weth.ts +++ b/zksync/src/deployL2Weth.ts @@ -4,7 +4,7 @@ import { Deployer } from '../../ethereum/src.ts/deploy'; import { formatUnits, parseUnits } from 'ethers/lib/utils'; import { getTokens, web3Provider } from '../../ethereum/scripts/utils'; -import { getNumberFromEnv, create2DeployFromL1, computeL2Create2Address } from './utils'; +import { getNumberFromEnv, applyL1ToL2Alias, create2DeployFromL1, computeL2Create2Address } from './utils'; import * as fs from 'fs'; import * as path from 'path'; @@ -68,14 +68,19 @@ async function main() { const deployer = new Deployer({ deployWallet, - governorAddress: deployWallet.address, verbose: true }); const zkSync = deployer.zkSyncContract(deployWallet); const priorityTxMaxGasLimit = getNumberFromEnv('CONTRACTS_PRIORITY_TX_MAX_GAS_LIMIT'); - const governorAddress = await zkSync.getGovernor(); + const l1GovernorAddress = await zkSync.getGovernor(); + // Check whether governor is a smart contract on L1 to apply alias if needed. + const l1GovernorCodeSize = ethers.utils.hexDataLength( + await deployWallet.provider.getCode(l1GovernorAddress) + ); + const l2GovernorAddress = l1GovernorCodeSize == 0 ? l1GovernorAddress : applyL1ToL2Alias(l1GovernorAddress); + const abiCoder = new ethers.utils.AbiCoder(); const l2WethImplAddr = computeL2Create2Address( @@ -92,7 +97,7 @@ async function main() { const l2ERC20BridgeProxyConstructor = ethers.utils.arrayify( abiCoder.encode( ['address', 'address', 'bytes'], - [l2WethImplAddr, governorAddress, proxyInitializationParams] + [l2WethImplAddr, l2GovernorAddress, proxyInitializationParams] ) ); const l2WethProxyAddr = computeL2Create2Address( diff --git a/zksync/src/utils.ts b/zksync/src/utils.ts index b347a9d0f..275632f33 100644 --- a/zksync/src/utils.ts +++ b/zksync/src/utils.ts @@ -12,6 +12,12 @@ export const REQUIRED_L2_GAS_PRICE_PER_PUBDATA = require('../../SystemConfig.jso const DEPLOYER_SYSTEM_CONTRACT_ADDRESS = '0x0000000000000000000000000000000000008006'; const CREATE2_PREFIX = ethers.utils.solidityKeccak256(['string'], ['zksyncCreate2']); +const L1_TO_L2_ALIAS_OFFSET = '0x1111000000000000000000000000000000001111'; +const ADDRESS_MODULO = ethers.BigNumber.from(2).pow(160); + +export function applyL1ToL2Alias(address: string): string { + return ethers.utils.hexlify(ethers.BigNumber.from(address).add(L1_TO_L2_ALIAS_OFFSET).mod(ADDRESS_MODULO)); +} export function hashL2Bytecode(bytecode: ethers.BytesLike): Uint8Array { // For getting the consistent length we first convert the bytecode to UInt8Array