From 8b5b2964fc78cd94ebea48b172647489f39b1ae3 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Thu, 5 Sep 2024 18:38:38 +0200 Subject: [PATCH] Implement restriction to allow limiting chain admin in power (#699) Co-authored-by: Vlad Bochok <41153528+vladbochok@users.noreply.github.com> Co-authored-by: Yberjon <106954795+Yberjon@users.noreply.github.com> --- .solhintignore | 1 + .../contracts/common/L1ContractErrors.sol | 22 ++ .../dev-contracts/test/ReenterGovernance.sol | 5 +- .../governance/AccessControlRestriction.sol | 72 ++++++ .../contracts/governance/ChainAdmin.sol | 114 +++++++--- l1-contracts/contracts/governance/Common.sol | 13 ++ .../contracts/governance/Governance.sol | 1 + .../governance/IAccessControlRestriction.sol | 14 ++ .../contracts/governance/IChainAdmin.sol | 39 ++-- .../contracts/governance/IGovernance.sol | 12 +- .../governance/IPermanentRestriction.sol | 17 ++ .../contracts/governance/IRestriction.sol | 15 ++ .../governance/PermanentRestriction.sol | 186 ++++++++++++++++ .../chain-deps/facets/Getters.sol | 5 + .../chain-interfaces/IGetters.sol | 3 + l1-contracts/deploy-scripts/DeployL1.s.sol | 11 +- .../deploy-scripts/RegisterHyperchain.s.sol | 10 +- l1-contracts/deploy-scripts/Utils.sol | 7 +- l1-contracts/src.ts/deploy-process.ts | 3 +- l1-contracts/src.ts/deploy.ts | 14 +- .../foundry/unit/concrete/Utils/Utils.sol | 3 +- .../Governance/AccessControlRestriction.t.sol | 186 ++++++++++++++++ .../Governance/Authorization.t.sol | 0 .../governance/Governance/ChainAdmin.t.sol | 177 +++++++++++++++ .../Governance/Executing.t.sol | 2 +- .../Governance/Fallback.t.sol | 0 .../Governance/OperationStatus.t.sol | 2 +- .../Governance/PermanentRestriction.t.sol | 209 ++++++++++++++++++ .../Governance/Reentrancy.t.sol | 0 .../Governance/SelfUpgrades.t.sol | 2 +- .../Governance/_Governance_Shared.t.sol | 5 +- .../CreateNewChain.t.sol | 4 + .../StateTransitionManager/FreezeChain.t.sol | 4 + .../RevertBatches.t.sol | 4 + .../SetChainCreationParams.t.sol | 4 + .../SetNewVersionUpgrade.t.sol | 4 + .../SetUpgradeDiamondCut.t.sol | 4 + .../SetValidatorTimelock.t.sol | 4 + .../StateTransitionOwnerZero.t.sol | 4 + .../_StateTransitionManager_Shared.t.sol | 13 +- 40 files changed, 1106 insertions(+), 89 deletions(-) create mode 100644 l1-contracts/contracts/governance/AccessControlRestriction.sol create mode 100644 l1-contracts/contracts/governance/Common.sol create mode 100644 l1-contracts/contracts/governance/IAccessControlRestriction.sol create mode 100644 l1-contracts/contracts/governance/IPermanentRestriction.sol create mode 100644 l1-contracts/contracts/governance/IRestriction.sol create mode 100644 l1-contracts/contracts/governance/PermanentRestriction.sol create mode 100644 l1-contracts/test/foundry/unit/concrete/governance/Governance/AccessControlRestriction.t.sol rename l1-contracts/test/foundry/unit/concrete/{ => governance}/Governance/Authorization.t.sol (100%) create mode 100644 l1-contracts/test/foundry/unit/concrete/governance/Governance/ChainAdmin.t.sol rename l1-contracts/test/foundry/unit/concrete/{ => governance}/Governance/Executing.t.sol (99%) rename l1-contracts/test/foundry/unit/concrete/{ => governance}/Governance/Fallback.t.sol (100%) rename l1-contracts/test/foundry/unit/concrete/{ => governance}/Governance/OperationStatus.t.sol (99%) create mode 100644 l1-contracts/test/foundry/unit/concrete/governance/Governance/PermanentRestriction.t.sol rename l1-contracts/test/foundry/unit/concrete/{ => governance}/Governance/Reentrancy.t.sol (100%) rename l1-contracts/test/foundry/unit/concrete/{ => governance}/Governance/SelfUpgrades.t.sol (97%) rename l1-contracts/test/foundry/unit/concrete/{ => governance}/Governance/_Governance_Shared.t.sol (93%) diff --git a/.solhintignore b/.solhintignore index abcb64f98..2371ed166 100644 --- a/.solhintignore +++ b/.solhintignore @@ -8,6 +8,7 @@ l1-contracts/lib l1-contracts/node_modules l1-contracts/contracts/dev-contracts l1-contracts/test +l1-contracts/deploy-scripts # l1-contracts-foundry l1-contracts-foundry/cache diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 73ff72cc9..bec5a56cd 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -1,6 +1,28 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.21; +// 0x5ecf2d7a +error AccessToFallbackDenied(address target, address invoker); +// 0x3995f750 +error AccessToFunctionDenied(address target, bytes4 selector, address invoker); +// 0x6c167909 +error OnlySelfAllowed(); +// 0x52e22c98 +error RestrictionWasNotPresent(address restriction); +// 0xf126e113 +error RestrictionWasAlreadyPresent(address restriction); +// 0x3331e9c0 +error CallNotAllowed(bytes call); +// 0x59e1b0d2 +error ChainZeroAddress(); +// 0xff4bbdf1 +error NotAHyperchain(address chainAddress); +// 0xa3decdf3 +error NotAnAdmin(address expected, address actual); +// 0xf6fd7071 +error RemovingPermanentRestriction(); +// 0xfcb9b2e1 +error UnallowedImplementation(bytes32 implementationHash); // 0x1ff9d522 error AddressAlreadyUsed(address addr); // 0x86bb51b8 diff --git a/l1-contracts/contracts/dev-contracts/test/ReenterGovernance.sol b/l1-contracts/contracts/dev-contracts/test/ReenterGovernance.sol index 0d619c5ba..193f8085f 100644 --- a/l1-contracts/contracts/dev-contracts/test/ReenterGovernance.sol +++ b/l1-contracts/contracts/dev-contracts/test/ReenterGovernance.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.24; import {IGovernance} from "../../governance/IGovernance.sol"; +import {Call} from "../../governance/Common.sol"; contract ReenterGovernance { // add this to be excluded from coverage report @@ -12,7 +13,7 @@ contract ReenterGovernance { // Store call, predecessor and salt separately, // because Operation struct can't be stored on storage. - IGovernance.Call call; + Call call; bytes32 predecessor; bytes32 salt; @@ -45,7 +46,7 @@ contract ReenterGovernance { fallback() external payable { if (!alreadyReentered) { alreadyReentered = true; - IGovernance.Call[] memory calls = new IGovernance.Call[](1); + Call[] memory calls = new Call[](1); calls[0] = call; IGovernance.Operation memory op = IGovernance.Operation({ calls: calls, diff --git a/l1-contracts/contracts/governance/AccessControlRestriction.sol b/l1-contracts/contracts/governance/AccessControlRestriction.sol new file mode 100644 index 000000000..3fc67f875 --- /dev/null +++ b/l1-contracts/contracts/governance/AccessControlRestriction.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.24; + +import {AccessToFallbackDenied, AccessToFunctionDenied} from "../common/L1ContractErrors.sol"; +import {IAccessControlRestriction} from "./IAccessControlRestriction.sol"; +import {AccessControlDefaultAdminRules} from "@openzeppelin/contracts-v4/access/AccessControlDefaultAdminRules.sol"; +import {IRestriction} from "./IRestriction.sol"; +import {Call} from "./Common.sol"; + +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev +/// @notice The Restriction that is designed to provide the access control logic for the `ChainAdmin` contract. +/// @dev It inherits from `AccessControlDefaultAdminRules` without overriding `_setRoleAdmin` functionaity. In other +/// words, the `DEFAULT_ADMIN_ROLE` is the only role that can manage roles. This is done for simplicity. +/// @dev An instance of this restriction should be deployed separately for each `ChainAdmin` contract. +/// @dev IMPORTANT: this function does not validate the ability of the invoker to use `msg.value`. Thus, +/// either all callers with access to functions should be trusted to not steal ETH from the `ChainAdmin` account +/// or not ETH should be passively stored in `ChainAdmin` account. +contract AccessControlRestriction is IRestriction, IAccessControlRestriction, AccessControlDefaultAdminRules { + /// @notice Required roles to call a specific functions. + /// @dev Note, that the role 0 means the `DEFAULT_ADMIN_ROLE` from the `AccessControlDefaultAdminRules` contract. + mapping(address target => mapping(bytes4 selector => bytes32 requiredRole)) public requiredRoles; + + /// @notice Required roles to call a fallback function. + mapping(address target => bytes32 requiredRole) public requiredRolesForFallback; + + constructor( + uint48 initialDelay, + address initialDefaultAdmin + ) AccessControlDefaultAdminRules(initialDelay, initialDefaultAdmin) {} + + /// @notice Sets the required role for a specific function call. + /// @param _target The address of the contract. + /// @param _selector The selector of the function. + /// @param _requiredRole The required role. + function setRequiredRoleForCall( + address _target, + bytes4 _selector, + bytes32 _requiredRole + ) external onlyRole(DEFAULT_ADMIN_ROLE) { + requiredRoles[_target][_selector] = _requiredRole; + + emit RoleSet(_target, _selector, _requiredRole); + } + + /// @notice Sets the required role for a fallback function call. + /// @param _target The address of the contract. + /// @param _requiredRole The required role. + function setRequiredRoleForFallback(address _target, bytes32 _requiredRole) external onlyRole(DEFAULT_ADMIN_ROLE) { + requiredRolesForFallback[_target] = _requiredRole; + + emit FallbackRoleSet(_target, _requiredRole); + } + + /// @inheritdoc IRestriction + function validateCall(Call calldata _call, address _invoker) external view { + // Note, that since `DEFAULT_ADMIN_ROLE` is 0 and the default storage value for the + // `requiredRoles` and `requiredRolesForFallback` is 0, the default admin is by default a required + // role for all the functions. + if (_call.data.length < 4) { + if (!hasRole(requiredRolesForFallback[_call.target], _invoker)) { + revert AccessToFallbackDenied(_call.target, _invoker); + } + } else { + bytes4 selector = bytes4(_call.data[:4]); + if (!hasRole(requiredRoles[_call.target][selector], _invoker)) { + revert AccessToFunctionDenied(_call.target, selector, _invoker); + } + } + } +} diff --git a/l1-contracts/contracts/governance/ChainAdmin.sol b/l1-contracts/contracts/governance/ChainAdmin.sol index 4d9ff858f..f6a93146f 100644 --- a/l1-contracts/contracts/governance/ChainAdmin.sol +++ b/l1-contracts/contracts/governance/ChainAdmin.sol @@ -2,48 +2,76 @@ pragma solidity 0.8.24; -import {Ownable2Step} from "@openzeppelin/contracts-v4/access/Ownable2Step.sol"; +// solhint-disable gas-length-in-loops + +import {NoCallsProvided, OnlySelfAllowed, RestrictionWasNotPresent, RestrictionWasAlreadyPresent} from "../common/L1ContractErrors.sol"; import {IChainAdmin} from "./IChainAdmin.sol"; -import {IAdmin} from "../state-transition/chain-interfaces/IAdmin.sol"; -import {NoCallsProvided, Unauthorized, ZeroAddress} from "../common/L1ContractErrors.sol"; +import {IRestriction} from "./IRestriction.sol"; +import {Call} from "./Common.sol"; + +import {EnumerableSet} from "@openzeppelin/contracts-v4/utils/structs/EnumerableSet.sol"; +import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev /// @notice The contract is designed to hold the `admin` role in ZKSync Chain (State Transition) contracts. /// The owner of the contract can perform any external calls and also save the information needed for -/// the blockchain node to accept the protocol upgrade. Another role - `tokenMultiplierSetter` can be used in the contract -/// to change the base token gas price in the Chain contract. -contract ChainAdmin is IChainAdmin, Ownable2Step { +/// the blockchain node to accept the protocol upgrade. +contract ChainAdmin is IChainAdmin, ReentrancyGuard { + using EnumerableSet for EnumerableSet.AddressSet; + + /// @notice Ensures that only the `ChainAdmin` contract itself can call the function. + /// @dev All functions that require access-control should use `onlySelf` modifier, while the access control logic + /// should be implemented in the restriction contracts. + modifier onlySelf() { + if (msg.sender != address(this)) { + revert OnlySelfAllowed(); + } + _; + } + + constructor(address[] memory _initialRestrictions) reentrancyGuardInitializer { + unchecked { + for (uint256 i = 0; i < _initialRestrictions.length; ++i) { + _addRestriction(_initialRestrictions[i]); + } + } + } + /// @notice Mapping of protocol versions to their expected upgrade timestamps. /// @dev Needed for the offchain node administration to know when to start building batches with the new protocol version. mapping(uint256 protocolVersion => uint256 upgradeTimestamp) public protocolVersionToUpgradeTimestamp; - /// @notice The address which can call `setTokenMultiplier` function to change the base token gas price in the Chain contract. - /// @dev The token base price can be changed quite often, so the private key for this role is supposed to be stored in the node - /// and used by the automated service in a way similar to the sequencer workflow. - address public tokenMultiplierSetter; + /// @notice The set of active restrictions. + EnumerableSet.AddressSet internal activeRestrictions; - constructor(address _initialOwner, address _initialTokenMultiplierSetter) { - if (_initialOwner == address(0)) { - revert ZeroAddress(); - } - _transferOwnership(_initialOwner); - // Can be zero if no one has this permission. - tokenMultiplierSetter = _initialTokenMultiplierSetter; - emit NewTokenMultiplierSetter(address(0), _initialTokenMultiplierSetter); + /// @notice Returns the list of active restrictions. + function getRestrictions() public view returns (address[] memory) { + return activeRestrictions.values(); + } + + /// @inheritdoc IChainAdmin + function isRestrictionActive(address _restriction) external view returns (bool) { + return activeRestrictions.contains(_restriction); } - /// @notice Updates the address responsible for setting token multipliers on the Chain contract . - /// @param _tokenMultiplierSetter The new address to be set as the token multiplier setter. - function setTokenMultiplierSetter(address _tokenMultiplierSetter) external onlyOwner { - emit NewTokenMultiplierSetter(tokenMultiplierSetter, _tokenMultiplierSetter); - tokenMultiplierSetter = _tokenMultiplierSetter; + /// @inheritdoc IChainAdmin + function addRestriction(address _restriction) external onlySelf { + _addRestriction(_restriction); + } + + /// @inheritdoc IChainAdmin + function removeRestriction(address _restriction) external onlySelf { + if (!activeRestrictions.remove(_restriction)) { + revert RestrictionWasNotPresent(_restriction); + } + emit RestrictionRemoved(_restriction); } /// @notice Set the expected upgrade timestamp for a specific protocol version. /// @param _protocolVersion The ZKsync chain protocol version. /// @param _upgradeTimestamp The timestamp at which the chain node should expect the upgrade to happen. - function setUpgradeTimestamp(uint256 _protocolVersion, uint256 _upgradeTimestamp) external onlyOwner { + function setUpgradeTimestamp(uint256 _protocolVersion, uint256 _upgradeTimestamp) external onlySelf { protocolVersionToUpgradeTimestamp[_protocolVersion] = _upgradeTimestamp; emit UpdateUpgradeTimestamp(_protocolVersion, _upgradeTimestamp); } @@ -52,12 +80,16 @@ contract ChainAdmin is IChainAdmin, Ownable2Step { /// @param _calls Array of Call structures defining target, value, and data for each call. /// @param _requireSuccess If true, reverts transaction on any call failure. /// @dev Intended for batch processing of contract interactions, managing gas efficiency and atomicity of operations. - function multicall(Call[] calldata _calls, bool _requireSuccess) external payable onlyOwner { + /// @dev Note, that this function lacks access control. It is expected that the access control is implemented in a separate restriction contract. + /// @dev Even though all the validation from external modules is executed via `staticcall`, the function + /// is marked as `nonReentrant` to prevent reentrancy attacks in case the staticcall restriction is lifted in the future. + function multicall(Call[] calldata _calls, bool _requireSuccess) external payable nonReentrant { if (_calls.length == 0) { revert NoCallsProvided(); } - // solhint-disable-next-line gas-length-in-loops for (uint256 i = 0; i < _calls.length; ++i) { + _validateCall(_calls[i]); + // slither-disable-next-line arbitrary-send-eth (bool success, bytes memory returnData) = _calls[i].target.call{value: _calls[i].value}(_calls[i].data); if (_requireSuccess && !success) { @@ -70,17 +102,27 @@ contract ChainAdmin is IChainAdmin, Ownable2Step { } } - /// @notice Sets the token multiplier in the specified Chain contract. - /// @param _chainContract The chain contract address where the token multiplier will be set. - /// @param _nominator The numerator part of the token multiplier. - /// @param _denominator The denominator part of the token multiplier. - function setTokenMultiplier(IAdmin _chainContract, uint128 _nominator, uint128 _denominator) external { - if (msg.sender != tokenMultiplierSetter) { - revert Unauthorized(msg.sender); + /// @dev Contract might receive/hold ETH as part of the maintenance process. + receive() external payable {} + + /// @notice Function that returns the current admin can perform the call. + /// @dev By default it always returns true, but can be overridden in derived contracts. + function _validateCall(Call calldata _call) internal view { + address[] memory restrictions = getRestrictions(); + + unchecked { + for (uint256 i = 0; i < restrictions.length; ++i) { + IRestriction(restrictions[i]).validateCall(_call, msg.sender); + } } - _chainContract.setTokenMultiplier(_nominator, _denominator); } - /// @dev Contract might receive/hold ETH as part of the maintenance process. - receive() external payable {} + /// @notice Adds a new restriction to the active restrictions set. + /// @param _restriction The address of the restriction contract to be added. + function _addRestriction(address _restriction) internal { + if (!activeRestrictions.add(_restriction)) { + revert RestrictionWasAlreadyPresent(_restriction); + } + emit RestrictionAdded(_restriction); + } } diff --git a/l1-contracts/contracts/governance/Common.sol b/l1-contracts/contracts/governance/Common.sol new file mode 100644 index 000000000..fd73dd793 --- /dev/null +++ b/l1-contracts/contracts/governance/Common.sol @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.24; + +/// @dev Represents a call to be made during multicall. +/// @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; +} diff --git a/l1-contracts/contracts/governance/Governance.sol b/l1-contracts/contracts/governance/Governance.sol index 790b79a26..d6e9d9b44 100644 --- a/l1-contracts/contracts/governance/Governance.sol +++ b/l1-contracts/contracts/governance/Governance.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.24; import {Ownable2Step} from "@openzeppelin/contracts-v4/access/Ownable2Step.sol"; import {IGovernance} from "./IGovernance.sol"; +import {Call} from "./Common.sol"; import {ZeroAddress, Unauthorized, OperationMustBeReady, OperationMustBePending, OperationExists, InvalidDelay, PreviousOperationNotExecuted} from "../common/L1ContractErrors.sol"; /// @author Matter Labs diff --git a/l1-contracts/contracts/governance/IAccessControlRestriction.sol b/l1-contracts/contracts/governance/IAccessControlRestriction.sol new file mode 100644 index 000000000..3c9cfb5c5 --- /dev/null +++ b/l1-contracts/contracts/governance/IAccessControlRestriction.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.24; + +/// @title AccessControlRestriction contract interface +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev +interface IAccessControlRestriction { + /// @notice Emitted when the required role for a specific function is set. + event RoleSet(address indexed target, bytes4 indexed selector, bytes32 requiredRole); + + /// @notice Emitted when the required role for a fallback function is set. + event FallbackRoleSet(address indexed target, bytes32 requiredRole); +} diff --git a/l1-contracts/contracts/governance/IChainAdmin.sol b/l1-contracts/contracts/governance/IChainAdmin.sol index d5d8f117c..1ef3144c2 100644 --- a/l1-contracts/contracts/governance/IChainAdmin.sol +++ b/l1-contracts/contracts/governance/IChainAdmin.sol @@ -2,36 +2,37 @@ pragma solidity 0.8.24; -import {IAdmin} from "../state-transition/chain-interfaces/IAdmin.sol"; +import {Call} from "./Common.sol"; /// @title ChainAdmin contract interface /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev interface IChainAdmin { - /// @dev Represents a call to be made during multicall. - /// @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; - } - /// @notice Emitted when the expected upgrade timestamp for a specific protocol version is set. - event UpdateUpgradeTimestamp(uint256 indexed _protocolVersion, uint256 _upgradeTimestamp); + event UpdateUpgradeTimestamp(uint256 indexed protocolVersion, uint256 upgradeTimestamp); /// @notice Emitted when the call is executed from the contract. - event CallExecuted(Call _call, bool _success, bytes _returnData); + event CallExecuted(Call call, bool success, bytes returnData); + + /// @notice Emitted when a new restriction is added. + event RestrictionAdded(address indexed restriction); - /// @notice Emitted when the new token multiplier address is set. - event NewTokenMultiplierSetter(address _oldTokenMultiplierSetter, address _newTokenMultiplierSetter); + /// @notice Emitted when a restriction is removed. + event RestrictionRemoved(address indexed restriction); - function setTokenMultiplierSetter(address _tokenMultiplierSetter) external; + /// @notice Returns the list of active restrictions. + function getRestrictions() external view returns (address[] memory); - function setUpgradeTimestamp(uint256 _protocolVersion, uint256 _upgradeTimestamp) external; + /// @notice Checks if the restriction is active. + /// @param _restriction The address of the restriction contract. + function isRestrictionActive(address _restriction) external view returns (bool); - function multicall(Call[] calldata _calls, bool _requireSuccess) external payable; + /// @notice Adds a new restriction to the active restrictions set. + /// @param _restriction The address of the restriction contract. + function addRestriction(address _restriction) external; - function setTokenMultiplier(IAdmin _chainContract, uint128 _nominator, uint128 _denominator) external; + /// @notice Removes a restriction from the active restrictions set. + /// @param _restriction The address of the restriction contract. + /// @dev Sometimes restrictions might need to enforce their permanence (e.g. if a chain should be a rollup forever). + function removeRestriction(address _restriction) external; } diff --git a/l1-contracts/contracts/governance/IGovernance.sol b/l1-contracts/contracts/governance/IGovernance.sol index 2b03ed4c9..0cb478573 100644 --- a/l1-contracts/contracts/governance/IGovernance.sol +++ b/l1-contracts/contracts/governance/IGovernance.sol @@ -2,6 +2,8 @@ // We use a floating point pragma here so it can be used within other projects that interact with the ZKsync ecosystem without using our exact pragma version. pragma solidity ^0.8.21; +import {Call} from "./Common.sol"; + /// @title Governance contract interface /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -18,16 +20,6 @@ interface IGovernance { 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. diff --git a/l1-contracts/contracts/governance/IPermanentRestriction.sol b/l1-contracts/contracts/governance/IPermanentRestriction.sol new file mode 100644 index 000000000..548866b9f --- /dev/null +++ b/l1-contracts/contracts/governance/IPermanentRestriction.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.24; + +/// @notice The interface for the permanent restriction contract. +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev +interface IPermanentRestriction { + /// @notice Emitted when the implementation is allowed or disallowed. + event AdminImplementationAllowed(bytes32 indexed implementationHash, bool isAllowed); + + /// @notice Emitted when a certain calldata is allowed or disallowed. + event AllowedDataChanged(bytes data, bool isAllowed); + + /// @notice Emitted when the selector is labeled as validated or not. + event SelectorValidationChanged(bytes4 indexed selector, bool isValidated); +} diff --git a/l1-contracts/contracts/governance/IRestriction.sol b/l1-contracts/contracts/governance/IRestriction.sol new file mode 100644 index 000000000..b2cc79428 --- /dev/null +++ b/l1-contracts/contracts/governance/IRestriction.sol @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.24; + +import {Call} from "./Common.sol"; + +/// @title Restriction contract interface +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev +interface IRestriction { + /// @notice Ensures that the invoker has the required role to call the function. + /// @param _call The call data. + /// @param _invoker The address of the invoker. + function validateCall(Call calldata _call, address _invoker) external view; +} diff --git a/l1-contracts/contracts/governance/PermanentRestriction.sol b/l1-contracts/contracts/governance/PermanentRestriction.sol new file mode 100644 index 000000000..acf75a67f --- /dev/null +++ b/l1-contracts/contracts/governance/PermanentRestriction.sol @@ -0,0 +1,186 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.24; + +import {CallNotAllowed, ChainZeroAddress, NotAHyperchain, NotAnAdmin, RemovingPermanentRestriction, ZeroAddress, UnallowedImplementation} from "../common/L1ContractErrors.sol"; + +import {Ownable2Step} from "@openzeppelin/contracts-v4/access/Ownable2Step.sol"; + +import {Call} from "./Common.sol"; +import {IRestriction} from "./IRestriction.sol"; +import {IChainAdmin} from "./IChainAdmin.sol"; +import {IBridgehub} from "../bridgehub/IBridgehub.sol"; +import {IZkSyncHyperchain} from "../state-transition/chain-interfaces/IZkSyncHyperchain.sol"; +import {IAdmin} from "../state-transition/chain-interfaces/IAdmin.sol"; + +import {IPermanentRestriction} from "./IPermanentRestriction.sol"; + +/// @title PermanentRestriction contract +/// @author Matter Labs +/// @custom:security-contact security@matterlabs.dev +/// @notice This contract should be used by chains that wish to guarantee that certain security +/// properties are preserved forever. +/// @dev To be deployed as a transparent upgradable proxy, owned by a trusted decentralized governance. +/// @dev Once of the instances of such contract is to ensure that a ZkSyncHyperchain is a rollup forever. +contract PermanentRestriction is IRestriction, IPermanentRestriction, Ownable2Step { + /// @notice The address of the Bridgehub contract. + IBridgehub public immutable BRIDGE_HUB; + + /// @notice The mapping of the allowed admin implementations. + mapping(bytes32 implementationCodeHash => bool isAllowed) public allowedAdminImplementations; + + /// @notice The mapping of the allowed calls. + mapping(bytes allowedCalldata => bool isAllowed) public allowedCalls; + + /// @notice The mapping of the validated selectors. + mapping(bytes4 selector => bool isValidated) public validatedSelectors; + + constructor(address _initialOwner, IBridgehub _bridgehub) { + BRIDGE_HUB = _bridgehub; + + // solhint-disable-next-line gas-custom-errors, reason-string + if (_initialOwner == address(0)) { + revert ZeroAddress(); + } + _transferOwnership(_initialOwner); + } + + /// @notice Allows a certain `ChainAdmin` implementation to be used as an admin. + /// @param _implementationHash The hash of the implementation code. + /// @param _isAllowed The flag that indicates if the implementation is allowed. + function allowAdminImplementation(bytes32 _implementationHash, bool _isAllowed) external onlyOwner { + allowedAdminImplementations[_implementationHash] = _isAllowed; + + emit AdminImplementationAllowed(_implementationHash, _isAllowed); + } + + /// @notice Allows a certain calldata for a selector to be used. + /// @param _data The calldata for the function. + /// @param _isAllowed The flag that indicates if the calldata is allowed. + function setAllowedData(bytes calldata _data, bool _isAllowed) external onlyOwner { + allowedCalls[_data] = _isAllowed; + + emit AllowedDataChanged(_data, _isAllowed); + } + + /// @notice Allows a certain selector to be validated. + /// @param _selector The selector of the function. + /// @param _isValidated The flag that indicates if the selector is validated. + function setSelectorIsValidated(bytes4 _selector, bool _isValidated) external onlyOwner { + validatedSelectors[_selector] = _isValidated; + + emit SelectorValidationChanged(_selector, _isValidated); + } + + /// @inheritdoc IRestriction + function validateCall( + Call calldata _call, + address // _invoker + ) external view override { + _validateAsChainAdmin(_call); + _validateRemoveRestriction(_call); + } + + /// @notice Validates the call as the chain admin + /// @param _call The call data. + function _validateAsChainAdmin(Call calldata _call) internal view { + if (!_isAdminOfAChain(_call.target)) { + // We only validate calls related to being an admin of a chain + return; + } + + // All calls with the length of the data below 4 will get into `receive`/`fallback` functions, + // we consider it to always be allowed. + if (_call.data.length < 4) { + return; + } + + bytes4 selector = bytes4(_call.data[:4]); + + if (selector == IAdmin.setPendingAdmin.selector) { + _validateNewAdmin(_call); + return; + } + + if (!validatedSelectors[selector]) { + // The selector is not validated, any data is allowed. + return; + } + + if (!allowedCalls[_call.data]) { + revert CallNotAllowed(_call.data); + } + } + + /// @notice Validates the correctness of the new admin. + /// @param _call The call data. + /// @dev Ensures that the admin has a whitelisted implementation and does not remove this restriction. + function _validateNewAdmin(Call calldata _call) internal view { + address newChainAdmin = abi.decode(_call.data[4:], (address)); + + bytes32 implementationCodeHash = newChainAdmin.codehash; + + if (!allowedAdminImplementations[implementationCodeHash]) { + revert UnallowedImplementation(implementationCodeHash); + } + + // Since the implementation is known to be correct (from the checks above), we + // can safely trust the returned value from the call below + if (!IChainAdmin(newChainAdmin).isRestrictionActive(address(this))) { + revert RemovingPermanentRestriction(); + } + } + + /// @notice Validates the removal of the restriction. + /// @param _call The call data. + /// @dev Ensures that this restriction is not removed. + function _validateRemoveRestriction(Call calldata _call) internal view { + if (_call.target != msg.sender) { + return; + } + + if (bytes4(_call.data[:4]) != IChainAdmin.removeRestriction.selector) { + return; + } + + address removedRestriction = abi.decode(_call.data[4:], (address)); + + if (removedRestriction == address(this)) { + revert RemovingPermanentRestriction(); + } + } + + /// @notice Checks if the `msg.sender` is an admin of a certain ZkSyncHyperchain. + /// @param _chain The address of the chain. + function _isAdminOfAChain(address _chain) internal view returns (bool) { + (bool success, ) = address(this).staticcall(abi.encodeCall(this.tryCompareAdminOfAChain, (_chain, msg.sender))); + return success; + } + + /// @notice Tries to compare the admin of a chain with the potential admin. + /// @param _chain The address of the chain. + /// @param _potentialAdmin The address of the potential admin. + /// @dev This function reverts if the `_chain` is not a ZkSyncHyperchain or the `_potentialAdmin` is not the + /// admin of the chain. + function tryCompareAdminOfAChain(address _chain, address _potentialAdmin) external view { + if (_chain == address(0)) { + revert ChainZeroAddress(); + } + + // Unfortunately there is no easy way to double check that indeed the `_chain` is a ZkSyncHyperchain. + // So we do the following: + // - Query it for `chainId`. If it reverts, it is not a ZkSyncHyperchain. + // - Query the Bridgehub for the Hyperchain with the given `chainId`. + // - We compare the corresponding addresses + uint256 chainId = IZkSyncHyperchain(_chain).getChainId(); + if (BRIDGE_HUB.getHyperchain(chainId) != _chain) { + revert NotAHyperchain(_chain); + } + + // Now, the chain is known to be a hyperchain, so it should implement the corresponding interface + address admin = IZkSyncHyperchain(_chain).getAdmin(); + if (admin != _potentialAdmin) { + revert NotAnAdmin(admin, _potentialAdmin); + } + } +} diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol index 9cb2a2da8..604eeef2e 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol @@ -52,6 +52,11 @@ contract GettersFacet is ZkSyncHyperchainBase, IGetters, ILegacyGetters { return s.bridgehub; } + /// @inheritdoc IGetters + function getChainId() external view returns (uint256) { + return s.chainId; + } + /// @inheritdoc IGetters function getStateTransitionManager() external view returns (address) { return s.stateTransitionManager; diff --git a/l1-contracts/contracts/state-transition/chain-interfaces/IGetters.sol b/l1-contracts/contracts/state-transition/chain-interfaces/IGetters.sol index 4d06f9e8e..5da8cc748 100644 --- a/l1-contracts/contracts/state-transition/chain-interfaces/IGetters.sol +++ b/l1-contracts/contracts/state-transition/chain-interfaces/IGetters.sol @@ -33,6 +33,9 @@ interface IGetters is IZkSyncHyperchainBase { /// @return The address of the base token function getBaseToken() external view returns (address); + /// @return The chain id of the ZK Chain. + function getChainId() external view returns (uint256); + /// @return The address of the base token bridge function getBaseTokenBridge() external view returns (address); diff --git a/l1-contracts/deploy-scripts/DeployL1.s.sol b/l1-contracts/deploy-scripts/DeployL1.s.sol index 098aee5bc..cef851957 100644 --- a/l1-contracts/deploy-scripts/DeployL1.s.sol +++ b/l1-contracts/deploy-scripts/DeployL1.s.sol @@ -302,10 +302,17 @@ contract DeployL1Script is Script { } function deployChainAdmin() internal { - bytes memory bytecode = abi.encodePacked( + bytes memory accessControlRestrictionBytecode = abi.encodePacked( type(ChainAdmin).creationCode, - abi.encode(config.ownerAddress, address(0)) + abi.encode(uint256(0), config.ownerAddress) ); + + address accessControlRestriction = deployViaCreate2(accessControlRestrictionBytecode); + console.log("Access control restriction deployed at:", accessControlRestriction); + address[] memory restrictions = new address[](1); + restrictions[0] = accessControlRestriction; + + bytes memory bytecode = abi.encodePacked(type(ChainAdmin).creationCode, abi.encode(restrictions)); address contractAddress = deployViaCreate2(bytecode); console.log("ChainAdmin deployed at:", contractAddress); addresses.chainAdmin = contractAddress; diff --git a/l1-contracts/deploy-scripts/RegisterHyperchain.s.sol b/l1-contracts/deploy-scripts/RegisterHyperchain.s.sol index e0039b969..94acb2f02 100644 --- a/l1-contracts/deploy-scripts/RegisterHyperchain.s.sol +++ b/l1-contracts/deploy-scripts/RegisterHyperchain.s.sol @@ -13,6 +13,7 @@ import {IZkSyncHyperchain} from "contracts/state-transition/chain-interfaces/IZk import {ValidatorTimelock} from "contracts/state-transition/ValidatorTimelock.sol"; import {Governance} from "contracts/governance/Governance.sol"; import {ChainAdmin} from "contracts/governance/ChainAdmin.sol"; +import {AccessControlRestriction} from "contracts/governance/AccessControlRestriction.sol"; import {Utils} from "./Utils.sol"; import {PubdataPricingMode} from "contracts/state-transition/chain-deps/ZkSyncHyperchainStorage.sol"; @@ -151,8 +152,13 @@ contract RegisterHyperchainScript is Script { function deployChainAdmin() internal { vm.broadcast(); - ChainAdmin chainAdmin = new ChainAdmin(config.ownerAddress, address(0)); - console.log("ChainAdmin deployed at:", address(chainAdmin)); + AccessControlRestriction restriction = new AccessControlRestriction(0, config.ownerAddress); + + address[] memory restrictions = new address[](1); + restrictions[0] = address(restriction); + + vm.broadcast(); + ChainAdmin chainAdmin = new ChainAdmin(restrictions); config.chainAdmin = address(chainAdmin); } diff --git a/l1-contracts/deploy-scripts/Utils.sol b/l1-contracts/deploy-scripts/Utils.sol index 17449da7e..440eadcac 100644 --- a/l1-contracts/deploy-scripts/Utils.sol +++ b/l1-contracts/deploy-scripts/Utils.sol @@ -8,7 +8,8 @@ import {Vm} from "forge-std/Vm.sol"; import {Bridgehub} from "contracts/bridgehub/Bridgehub.sol"; import {L2TransactionRequestDirect} from "contracts/bridgehub/IBridgehub.sol"; import {IGovernance} from "contracts/governance/IGovernance.sol"; -import {IERC20} from "@openzeppelin/contracts-v4/token/ERC20/IERC20.sol"; +import {Call} from "contracts/governance/Common.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import {REQUIRED_L2_GAS_PRICE_PER_PUBDATA} from "contracts/common/Config.sol"; import {L2_DEPLOYER_SYSTEM_CONTRACT_ADDR} from "contracts/common/L2ContractAddresses.sol"; import {L2ContractHelper} from "contracts/common/libraries/L2ContractHelper.sol"; @@ -316,8 +317,8 @@ library Utils { ) internal { IGovernance governance = IGovernance(_governor); - IGovernance.Call[] memory calls = new IGovernance.Call[](1); - calls[0] = IGovernance.Call({target: _target, value: _value, data: _data}); + Call[] memory calls = new Call[](1); + calls[0] = Call({target: _target, value: _value, data: _data}); IGovernance.Operation memory operation = IGovernance.Operation({ calls: calls, diff --git a/l1-contracts/src.ts/deploy-process.ts b/l1-contracts/src.ts/deploy-process.ts index d30762a15..285dd9e0e 100644 --- a/l1-contracts/src.ts/deploy-process.ts +++ b/l1-contracts/src.ts/deploy-process.ts @@ -58,9 +58,8 @@ export async function initialBridgehubDeployment( nonce++; await deployer.deployGovernance(create2Salt, { gasPrice, nonce }); - nonce++; - await deployer.deployChainAdmin(create2Salt, { gasPrice, nonce }); + await deployer.deployChainAdmin(create2Salt, { gasPrice }); await deployer.deployTransparentProxyAdmin(create2Salt, { gasPrice }); await deployer.deployBridgehubContract(create2Salt, gasPrice); await deployer.deployBlobVersionedHashRetriever(create2Salt, { gasPrice }); diff --git a/l1-contracts/src.ts/deploy.ts b/l1-contracts/src.ts/deploy.ts index 3ee3bb07b..d131d269c 100644 --- a/l1-contracts/src.ts/deploy.ts +++ b/l1-contracts/src.ts/deploy.ts @@ -212,9 +212,21 @@ export class Deployer { public async deployChainAdmin(create2Salt: string, ethTxOptions: ethers.providers.TransactionRequest) { ethTxOptions.gasLimit ??= 10_000_000; + // Firstly, we deploy the access control restriction for the chain admin + const accessControlRestriction = await this.deployViaCreate2( + "AccessControlRestriction", + [0, this.ownerAddress], + create2Salt, + ethTxOptions + ); + if (this.verbose) { + console.log(`CONTRACTS_ACCESS_CONTROL_RESTRICTION_ADDR=${accessControlRestriction}`); + } + + // Then we deploy the ChainAdmin contract itself const contractAddress = await this.deployViaCreate2( "ChainAdmin", - [this.ownerAddress, ethers.constants.AddressZero], + [[accessControlRestriction]], create2Salt, ethTxOptions ); diff --git a/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol b/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol index 1260334fd..3b46d212a 100644 --- a/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol +++ b/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol @@ -210,7 +210,7 @@ library Utils { } function getGettersSelectors() public pure returns (bytes4[] memory) { - bytes4[] memory selectors = new bytes4[](29); + bytes4[] memory selectors = new bytes4[](30); selectors[0] = GettersFacet.getVerifier.selector; selectors[1] = GettersFacet.getAdmin.selector; selectors[2] = GettersFacet.getPendingAdmin.selector; @@ -240,6 +240,7 @@ library Utils { selectors[26] = GettersFacet.getTotalBatchesVerified.selector; selectors[27] = GettersFacet.getTotalBatchesExecuted.selector; selectors[28] = GettersFacet.getL2SystemContractsUpgradeTxHash.selector; + selectors[29] = GettersFacet.getChainId.selector; return selectors; } diff --git a/l1-contracts/test/foundry/unit/concrete/governance/Governance/AccessControlRestriction.t.sol b/l1-contracts/test/foundry/unit/concrete/governance/Governance/AccessControlRestriction.t.sol new file mode 100644 index 000000000..e5b45975f --- /dev/null +++ b/l1-contracts/test/foundry/unit/concrete/governance/Governance/AccessControlRestriction.t.sol @@ -0,0 +1,186 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {Test} from "forge-std/Test.sol"; + +import "openzeppelin-contracts/contracts/utils/Strings.sol"; +import "forge-std/console.sol"; +import {IChainAdmin} from "contracts/governance/IChainAdmin.sol"; +import {ChainAdmin} from "contracts/governance/ChainAdmin.sol"; +import {AccessControlRestriction} from "contracts/governance/AccessControlRestriction.sol"; +import {IAccessControlRestriction} from "contracts/governance/IAccessControlRestriction.sol"; +import {Utils} from "test/foundry/unit/concrete/Utils/Utils.sol"; +import {NoCallsProvided, AccessToFallbackDenied, AccessToFunctionDenied} from "contracts/common/L1ContractErrors.sol"; +import {Call} from "contracts/governance/Common.sol"; + +contract AccessRestrictionTest is Test { + AccessControlRestriction internal restriction; + ChainAdmin internal chainAdmin; + address owner; + address randomCaller; + bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00; + + function getChainAdminSelectors() public pure returns (bytes4[] memory) { + bytes4[] memory selectors = new bytes4[](12); + selectors[0] = IChainAdmin.getRestrictions.selector; + selectors[1] = IChainAdmin.isRestrictionActive.selector; + selectors[2] = IChainAdmin.addRestriction.selector; + selectors[3] = IChainAdmin.removeRestriction.selector; + + return selectors; + } + + function setUp() public { + owner = makeAddr("random address"); + randomCaller = makeAddr("random caller"); + + restriction = new AccessControlRestriction(0, owner); + address[] memory restrictions = new address[](1); + restrictions[0] = address(restriction); + + chainAdmin = new ChainAdmin(restrictions); + } + + function test_adminAsAddressZero() public { + vm.expectRevert("AccessControl: 0 default admin"); + new AccessControlRestriction(0, address(0)); + } + + function test_setRequiredRoleForCallByNotDefaultAdmin(bytes32 role) public { + vm.assume(role != DEFAULT_ADMIN_ROLE); + + bytes4[] memory chainAdminSelectors = getChainAdminSelectors(); + string memory revertMsg = string( + abi.encodePacked( + "AccessControl: account ", + Strings.toHexString(uint160(randomCaller), 20), + " is missing role ", + Strings.toHexString(uint256(DEFAULT_ADMIN_ROLE), 32) + ) + ); + + vm.expectRevert(bytes(revertMsg)); + vm.prank(randomCaller); + restriction.setRequiredRoleForCall(address(chainAdmin), chainAdminSelectors[0], role); + } + + function test_setRequiredRoleForCallAccessToFunctionDenied(bytes32 role) public { + vm.assume(role != DEFAULT_ADMIN_ROLE); + + bytes4[] memory chainAdminSelectors = getChainAdminSelectors(); + + vm.startPrank(owner); + restriction.setRequiredRoleForCall(address(chainAdmin), chainAdminSelectors[0], role); + vm.stopPrank(); + + Call memory call = Call({ + target: address(chainAdmin), + value: 0, + data: abi.encodeCall(IChainAdmin.getRestrictions, ()) + }); + + vm.expectRevert( + abi.encodeWithSelector( + AccessToFunctionDenied.selector, + address(chainAdmin), + chainAdminSelectors[0], + randomCaller + ) + ); + restriction.validateCall(call, randomCaller); + } + + function test_setRequiredRoleForCall(bytes32 role) public { + vm.assume(role != DEFAULT_ADMIN_ROLE); + + bytes4[] memory chainAdminSelectors = getChainAdminSelectors(); + + vm.expectEmit(true, true, false, true); + emit IAccessControlRestriction.RoleSet(address(chainAdmin), chainAdminSelectors[0], role); + + vm.startPrank(owner); + restriction.setRequiredRoleForCall(address(chainAdmin), chainAdminSelectors[0], role); + restriction.grantRole(role, randomCaller); + vm.stopPrank(); + + Call memory call = Call({ + target: address(chainAdmin), + value: 0, + data: abi.encodeCall(IChainAdmin.getRestrictions, ()) + }); + restriction.validateCall(call, randomCaller); + } + + function test_setRequiredRoleForFallbackByNotDefaultAdmin(bytes32 role) public { + vm.assume(role != DEFAULT_ADMIN_ROLE); + + string memory revertMsg = string( + abi.encodePacked( + "AccessControl: account ", + Strings.toHexString(uint160(randomCaller), 20), + " is missing role ", + Strings.toHexString(uint256(DEFAULT_ADMIN_ROLE), 32) + ) + ); + + vm.expectRevert(bytes(revertMsg)); + vm.prank(randomCaller); + restriction.setRequiredRoleForFallback(address(chainAdmin), role); + } + + function test_setRequiredRoleForFallbackAccessToFallbackDenied(bytes32 role) public { + vm.assume(role != DEFAULT_ADMIN_ROLE); + + vm.startPrank(owner); + restriction.setRequiredRoleForFallback(address(chainAdmin), role); + vm.stopPrank(); + + Call memory call = Call({target: address(chainAdmin), value: 0, data: ""}); + + vm.expectRevert(abi.encodeWithSelector(AccessToFallbackDenied.selector, address(chainAdmin), randomCaller)); + restriction.validateCall(call, randomCaller); + } + + function test_setRequiredRoleForFallback(bytes32 role) public { + vm.assume(role != DEFAULT_ADMIN_ROLE); + + vm.expectEmit(true, false, false, true); + emit IAccessControlRestriction.FallbackRoleSet(address(chainAdmin), role); + + vm.startPrank(owner); + restriction.setRequiredRoleForFallback(address(chainAdmin), role); + restriction.grantRole(role, randomCaller); + vm.stopPrank(); + + Call memory call = Call({target: address(chainAdmin), value: 0, data: ""}); + restriction.validateCall(call, randomCaller); + } + + function test_validateCallFunction(bytes32 role) public { + vm.assume(role != DEFAULT_ADMIN_ROLE); + + bytes4[] memory chainAdminSelectors = getChainAdminSelectors(); + vm.startPrank(owner); + restriction.setRequiredRoleForCall(address(chainAdmin), chainAdminSelectors[0], role); + restriction.grantRole(role, randomCaller); + vm.stopPrank(); + + Call memory call = Call({ + target: address(chainAdmin), + value: 0, + data: abi.encodeCall(IChainAdmin.getRestrictions, ()) + }); + restriction.validateCall(call, randomCaller); + } + + function test_validateCallFallback(bytes32 role) public { + vm.assume(role != DEFAULT_ADMIN_ROLE); + vm.startPrank(owner); + restriction.setRequiredRoleForFallback(address(chainAdmin), role); + restriction.grantRole(role, randomCaller); + vm.stopPrank(); + + Call memory call = Call({target: address(chainAdmin), value: 0, data: ""}); + restriction.validateCall(call, randomCaller); + } +} diff --git a/l1-contracts/test/foundry/unit/concrete/Governance/Authorization.t.sol b/l1-contracts/test/foundry/unit/concrete/governance/Governance/Authorization.t.sol similarity index 100% rename from l1-contracts/test/foundry/unit/concrete/Governance/Authorization.t.sol rename to l1-contracts/test/foundry/unit/concrete/governance/Governance/Authorization.t.sol diff --git a/l1-contracts/test/foundry/unit/concrete/governance/Governance/ChainAdmin.t.sol b/l1-contracts/test/foundry/unit/concrete/governance/Governance/ChainAdmin.t.sol new file mode 100644 index 000000000..01a3c7dfd --- /dev/null +++ b/l1-contracts/test/foundry/unit/concrete/governance/Governance/ChainAdmin.t.sol @@ -0,0 +1,177 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {Test} from "forge-std/Test.sol"; + +import "openzeppelin-contracts/contracts/utils/Strings.sol"; +import {AccessControlRestriction} from "contracts/governance/AccessControlRestriction.sol"; +import {IChainAdmin} from "contracts/governance/IChainAdmin.sol"; +import {ChainAdmin} from "contracts/governance/ChainAdmin.sol"; +import {GettersFacet} from "contracts/state-transition/chain-deps/facets/Getters.sol"; +import {Call} from "contracts/governance/Common.sol"; +import {NoCallsProvided, RestrictionWasAlreadyPresent, RestrictionWasNotPresent, AccessToFallbackDenied, AccessToFunctionDenied} from "contracts/common/L1ContractErrors.sol"; +import {Utils} from "test/foundry/unit/concrete/Utils/Utils.sol"; + +contract ChainAdminTest is Test { + ChainAdmin internal chainAdmin; + AccessControlRestriction internal restriction; + GettersFacet internal gettersFacet; + + address internal owner; + uint32 internal major; + uint32 internal minor; + uint32 internal patch; + bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00; + + function setUp() public { + owner = makeAddr("random address"); + + restriction = new AccessControlRestriction(0, owner); + address[] memory restrictions = new address[](1); + restrictions[0] = address(restriction); + + chainAdmin = new ChainAdmin(restrictions); + + gettersFacet = new GettersFacet(); + } + + function test_getRestrictions() public { + address[] memory restrictions = chainAdmin.getRestrictions(); + assertEq(restrictions[0], address(restriction)); + } + + function test_isRestrictionActive() public { + bool isActive = chainAdmin.isRestrictionActive(address(restriction)); + assertEq(isActive, true); + } + + function test_addRestriction() public { + address[] memory restrictions = chainAdmin.getRestrictions(); + + vm.expectEmit(true, false, false, true); + emit IChainAdmin.RestrictionAdded(owner); + + vm.prank(address(chainAdmin)); + chainAdmin.addRestriction(owner); + } + + function test_addRestrictionRevert() public { + vm.startPrank(address(chainAdmin)); + chainAdmin.addRestriction(owner); + + vm.expectRevert(abi.encodeWithSelector(RestrictionWasAlreadyPresent.selector, owner)); + chainAdmin.addRestriction(owner); + vm.stopPrank(); + } + + function test_removeRestriction() public { + address[] memory restrictions = chainAdmin.getRestrictions(); + + vm.startPrank(address(chainAdmin)); + chainAdmin.addRestriction(owner); + + vm.expectEmit(true, false, false, true); + emit IChainAdmin.RestrictionRemoved(owner); + + chainAdmin.removeRestriction(owner); + vm.stopPrank(); + } + + function test_removeRestrictionRevert() public { + address[] memory restrictions = chainAdmin.getRestrictions(); + + vm.startPrank(address(chainAdmin)); + chainAdmin.addRestriction(owner); + chainAdmin.removeRestriction(owner); + + vm.expectRevert(abi.encodeWithSelector(RestrictionWasNotPresent.selector, owner)); + chainAdmin.removeRestriction(owner); + vm.stopPrank(); + } + + function test_setUpgradeTimestamp(uint256 semverMinorVersionMultiplier, uint256 timestamp) public { + (major, minor, patch) = gettersFacet.getSemverProtocolVersion(); + uint256 protocolVersion = packSemver(major, minor, patch + 1, semverMinorVersionMultiplier); + + vm.expectEmit(true, false, false, true); + emit IChainAdmin.UpdateUpgradeTimestamp(protocolVersion, timestamp); + + vm.prank(address(chainAdmin)); + chainAdmin.setUpgradeTimestamp(protocolVersion, timestamp); + } + + function test_multicallRevertNoCalls() public { + Call[] memory calls = new Call[](0); + + vm.expectRevert(NoCallsProvided.selector); + chainAdmin.multicall(calls, false); + } + + function test_multicallRevertFailedCall() public { + Call[] memory calls = new Call[](1); + calls[0] = Call({target: address(chainAdmin), value: 0, data: abi.encodeCall(gettersFacet.getAdmin, ())}); + + vm.expectRevert(); + vm.prank(owner); + chainAdmin.multicall(calls, true); + } + + function test_validateCallAccessToFunctionDenied(bytes32 role) public { + vm.assume(role != DEFAULT_ADMIN_ROLE); + + Call[] memory calls = new Call[](2); + calls[0] = Call({target: address(gettersFacet), value: 0, data: abi.encodeCall(gettersFacet.getAdmin, ())}); + calls[1] = Call({target: address(gettersFacet), value: 0, data: abi.encodeCall(gettersFacet.getVerifier, ())}); + + vm.prank(owner); + restriction.setRequiredRoleForCall(address(gettersFacet), gettersFacet.getAdmin.selector, role); + + vm.expectRevert( + abi.encodeWithSelector( + AccessToFunctionDenied.selector, + address(gettersFacet), + gettersFacet.getAdmin.selector, + owner + ) + ); + vm.prank(owner); + chainAdmin.multicall(calls, true); + } + + function test_validateCallAccessToFallbackDenied(bytes32 role) public { + vm.assume(role != DEFAULT_ADMIN_ROLE); + + Call[] memory calls = new Call[](2); + calls[0] = Call({target: address(gettersFacet), value: 0, data: ""}); + calls[1] = Call({target: address(gettersFacet), value: 0, data: abi.encodeCall(gettersFacet.getVerifier, ())}); + + vm.prank(owner); + restriction.setRequiredRoleForFallback(address(gettersFacet), role); + + vm.expectRevert(abi.encodeWithSelector(AccessToFallbackDenied.selector, address(gettersFacet), owner)); + vm.prank(owner); + chainAdmin.multicall(calls, true); + } + + function test_multicall() public { + Call[] memory calls = new Call[](2); + calls[0] = Call({target: address(gettersFacet), value: 0, data: abi.encodeCall(gettersFacet.getAdmin, ())}); + calls[1] = Call({target: address(gettersFacet), value: 0, data: abi.encodeCall(gettersFacet.getVerifier, ())}); + + vm.prank(owner); + chainAdmin.multicall(calls, true); + } + + function packSemver( + uint32 major, + uint32 minor, + uint32 patch, + uint256 semverMinorVersionMultiplier + ) public returns (uint256) { + if (major != 0) { + revert("Major version must be 0"); + } + + return minor * semverMinorVersionMultiplier + patch; + } +} diff --git a/l1-contracts/test/foundry/unit/concrete/Governance/Executing.t.sol b/l1-contracts/test/foundry/unit/concrete/governance/Governance/Executing.t.sol similarity index 99% rename from l1-contracts/test/foundry/unit/concrete/Governance/Executing.t.sol rename to l1-contracts/test/foundry/unit/concrete/governance/Governance/Executing.t.sol index 9a1e5eeb2..09d6c0267 100644 --- a/l1-contracts/test/foundry/unit/concrete/Governance/Executing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/governance/Governance/Executing.t.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.24; import {StdStorage, stdStorage} from "forge-std/Test.sol"; -import {Utils} from "../Utils/Utils.sol"; +import {Utils} from "../../Utils/Utils.sol"; import {GovernanceTest} from "./_Governance_Shared.t.sol"; diff --git a/l1-contracts/test/foundry/unit/concrete/Governance/Fallback.t.sol b/l1-contracts/test/foundry/unit/concrete/governance/Governance/Fallback.t.sol similarity index 100% rename from l1-contracts/test/foundry/unit/concrete/Governance/Fallback.t.sol rename to l1-contracts/test/foundry/unit/concrete/governance/Governance/Fallback.t.sol diff --git a/l1-contracts/test/foundry/unit/concrete/Governance/OperationStatus.t.sol b/l1-contracts/test/foundry/unit/concrete/governance/Governance/OperationStatus.t.sol similarity index 99% rename from l1-contracts/test/foundry/unit/concrete/Governance/OperationStatus.t.sol rename to l1-contracts/test/foundry/unit/concrete/governance/Governance/OperationStatus.t.sol index 131bb6465..9b4ee36e9 100644 --- a/l1-contracts/test/foundry/unit/concrete/Governance/OperationStatus.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/governance/Governance/OperationStatus.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.24; -import {Utils} from "../Utils/Utils.sol"; +import {Utils} from "../../Utils/Utils.sol"; import {GovernanceTest} from "./_Governance_Shared.t.sol"; diff --git a/l1-contracts/test/foundry/unit/concrete/governance/Governance/PermanentRestriction.t.sol b/l1-contracts/test/foundry/unit/concrete/governance/Governance/PermanentRestriction.t.sol new file mode 100644 index 000000000..0e9dc0bce --- /dev/null +++ b/l1-contracts/test/foundry/unit/concrete/governance/Governance/PermanentRestriction.t.sol @@ -0,0 +1,209 @@ +pragma solidity 0.8.24; + +import "openzeppelin-contracts/contracts/utils/Strings.sol"; +import {Bridgehub} from "contracts/bridgehub/Bridgehub.sol"; +import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; +import {StateTransitionManager} from "contracts/state-transition/StateTransitionManager.sol"; +import {DiamondInit} from "contracts/state-transition/chain-deps/DiamondInit.sol"; +import {PermanentRestriction} from "contracts/governance/PermanentRestriction.sol"; +import {IPermanentRestriction} from "contracts/governance/IPermanentRestriction.sol"; +import {ZeroAddress, ChainZeroAddress, NotAnAdmin, UnallowedImplementation, RemovingPermanentRestriction, CallNotAllowed} from "contracts/common/L1ContractErrors.sol"; +import {Call} from "contracts/governance/Common.sol"; +import {IZkSyncHyperchain} from "contracts/state-transition/chain-interfaces/IZkSyncHyperchain.sol"; +import {VerifierParams, FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-deps/ZkSyncHyperchainStorage.sol"; +import {IAdmin} from "contracts/state-transition/chain-interfaces/IAdmin.sol"; +import {AccessControlRestriction} from "contracts/governance/AccessControlRestriction.sol"; +import {ChainAdmin} from "contracts/governance/ChainAdmin.sol"; +import {IChainAdmin} from "contracts/governance/IChainAdmin.sol"; +import {StateTransitionManagerTest} from "test/foundry/unit/concrete/state-transition/StateTransitionManager/_StateTransitionManager_Shared.t.sol"; + +contract PermanentRestrictionTest is StateTransitionManagerTest { + ChainAdmin internal chainAdmin; + AccessControlRestriction internal restriction; + PermanentRestriction internal permRestriction; + + address internal owner; + address internal hyperchain; + + function setUp() public { + deploy(); + + createNewChainBridgehub(getDiamondCutData(address(diamondInit))); + + vm.stopPrank(); + + owner = makeAddr("owner"); + hyperchain = chainContractAddress.getHyperchain(chainId); + permRestriction = new PermanentRestriction(owner, bridgehub); + restriction = new AccessControlRestriction(0, owner); + address[] memory restrictions = new address[](1); + restrictions[0] = address(restriction); + chainAdmin = new ChainAdmin(restrictions); + } + + function test_ownerAsAddressZero() public { + vm.expectRevert(ZeroAddress.selector); + permRestriction = new PermanentRestriction(address(0), bridgehub); + } + + function test_allowAdminImplementation(bytes32 implementationHash) public { + vm.expectEmit(true, false, false, true); + emit IPermanentRestriction.AdminImplementationAllowed(implementationHash, true); + + vm.prank(owner); + permRestriction.allowAdminImplementation(implementationHash, true); + } + + function test_setAllowedData(bytes memory data) public { + vm.expectEmit(false, false, false, true); + emit IPermanentRestriction.AllowedDataChanged(data, true); + + vm.prank(owner); + permRestriction.setAllowedData(data, true); + } + + function test_setSelectorIsValidated(bytes4 selector) public { + vm.expectEmit(true, false, false, true); + emit IPermanentRestriction.SelectorValidationChanged(selector, true); + + vm.prank(owner); + permRestriction.setSelectorIsValidated(selector, true); + } + + function test_tryCompareAdminOfAChainIsAddressZero() public { + vm.expectRevert(ChainZeroAddress.selector); + permRestriction.tryCompareAdminOfAChain(address(0), owner); + } + + function test_tryCompareAdminOfAChainNotAHyperchain() public { + vm.expectRevert(); + permRestriction.tryCompareAdminOfAChain(makeAddr("random"), owner); + } + + function test_tryCompareAdminOfAChainNotAnAdmin() public { + vm.expectRevert(abi.encodeWithSelector(NotAnAdmin.selector, IZkSyncHyperchain(hyperchain).getAdmin(), owner)); + permRestriction.tryCompareAdminOfAChain(hyperchain, owner); + } + + function test_tryCompareAdminOfAChain() public { + permRestriction.tryCompareAdminOfAChain(hyperchain, newChainAdmin); + } + + function test_validateCallTooShortData() public { + Call memory call = Call({target: hyperchain, value: 0, data: ""}); + + vm.startPrank(newChainAdmin); + permRestriction.validateCall(call, owner); + vm.stopPrank(); + } + + function test_validateCallSetPendingAdminUnallowedImplementation() public { + Call memory call = Call({ + target: hyperchain, + value: 0, + data: abi.encodeWithSelector(IAdmin.setPendingAdmin.selector, owner) + }); + + vm.expectRevert(abi.encodeWithSelector(UnallowedImplementation.selector, owner.codehash)); + + vm.startPrank(newChainAdmin); + permRestriction.validateCall(call, owner); + vm.stopPrank(); + } + + function test_validateCallSetPendingAdminRemovingPermanentRestriction() public { + vm.prank(owner); + permRestriction.allowAdminImplementation(address(chainAdmin).codehash, true); + + Call memory call = Call({ + target: hyperchain, + value: 0, + data: abi.encodeWithSelector(IAdmin.setPendingAdmin.selector, address(chainAdmin)) + }); + + vm.expectRevert(RemovingPermanentRestriction.selector); + + vm.startPrank(newChainAdmin); + permRestriction.validateCall(call, owner); + vm.stopPrank(); + } + + function test_validateCallSetPendingAdmin() public { + vm.prank(owner); + permRestriction.allowAdminImplementation(address(chainAdmin).codehash, true); + + vm.prank(address(chainAdmin)); + chainAdmin.addRestriction(address(permRestriction)); + + Call memory call = Call({ + target: hyperchain, + value: 0, + data: abi.encodeWithSelector(IAdmin.setPendingAdmin.selector, address(chainAdmin)) + }); + + vm.startPrank(newChainAdmin); + permRestriction.validateCall(call, owner); + vm.stopPrank(); + } + + function test_validateCallNotValidatedSelector() public { + Call memory call = Call({ + target: hyperchain, + value: 0, + data: abi.encodeWithSelector(IAdmin.acceptAdmin.selector) + }); + + vm.startPrank(newChainAdmin); + permRestriction.validateCall(call, owner); + vm.stopPrank(); + } + + function test_validateCallCallNotAllowed() public { + vm.prank(owner); + permRestriction.setSelectorIsValidated(IAdmin.acceptAdmin.selector, true); + Call memory call = Call({ + target: hyperchain, + value: 0, + data: abi.encodeWithSelector(IAdmin.acceptAdmin.selector) + }); + + vm.expectRevert(abi.encodeWithSelector(CallNotAllowed.selector, call.data)); + + vm.startPrank(newChainAdmin); + permRestriction.validateCall(call, owner); + vm.stopPrank(); + } + + function test_validateCall() public { + vm.prank(owner); + permRestriction.setSelectorIsValidated(IAdmin.acceptAdmin.selector, true); + Call memory call = Call({ + target: hyperchain, + value: 0, + data: abi.encodeWithSelector(IAdmin.acceptAdmin.selector) + }); + + vm.prank(owner); + permRestriction.setAllowedData(call.data, true); + + vm.startPrank(newChainAdmin); + permRestriction.validateCall(call, owner); + vm.stopPrank(); + } + + function createNewChainBridgehub(Diamond.DiamondCutData memory _diamondCut) internal { + vm.stopPrank(); + vm.startPrank(address(0)); + bridgehub.addStateTransitionManager(address(chainContractAddress)); + bridgehub.addToken(baseToken); + bridgehub.setSharedBridge(sharedBridge); + bridgehub.createNewChain({ + _chainId: chainId, + _stateTransitionManager: address(chainContractAddress), + _baseToken: baseToken, + _salt: 0, + _admin: newChainAdmin, + _initData: abi.encode(_diamondCut) + }); + } +} diff --git a/l1-contracts/test/foundry/unit/concrete/Governance/Reentrancy.t.sol b/l1-contracts/test/foundry/unit/concrete/governance/Governance/Reentrancy.t.sol similarity index 100% rename from l1-contracts/test/foundry/unit/concrete/Governance/Reentrancy.t.sol rename to l1-contracts/test/foundry/unit/concrete/governance/Governance/Reentrancy.t.sol diff --git a/l1-contracts/test/foundry/unit/concrete/Governance/SelfUpgrades.t.sol b/l1-contracts/test/foundry/unit/concrete/governance/Governance/SelfUpgrades.t.sol similarity index 97% rename from l1-contracts/test/foundry/unit/concrete/Governance/SelfUpgrades.t.sol rename to l1-contracts/test/foundry/unit/concrete/governance/Governance/SelfUpgrades.t.sol index 04a909f57..a37acf150 100644 --- a/l1-contracts/test/foundry/unit/concrete/Governance/SelfUpgrades.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/governance/Governance/SelfUpgrades.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.24; -import {Utils} from "../Utils/Utils.sol"; +import {Utils} from "../../Utils/Utils.sol"; import {GovernanceTest} from "./_Governance_Shared.t.sol"; diff --git a/l1-contracts/test/foundry/unit/concrete/Governance/_Governance_Shared.t.sol b/l1-contracts/test/foundry/unit/concrete/governance/Governance/_Governance_Shared.t.sol similarity index 93% rename from l1-contracts/test/foundry/unit/concrete/Governance/_Governance_Shared.t.sol rename to l1-contracts/test/foundry/unit/concrete/governance/Governance/_Governance_Shared.t.sol index e7f499254..2a34bc2ff 100644 --- a/l1-contracts/test/foundry/unit/concrete/Governance/_Governance_Shared.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/governance/Governance/_Governance_Shared.t.sol @@ -6,6 +6,7 @@ import {Test} from "forge-std/Test.sol"; import {Governance} from "contracts/governance/Governance.sol"; import {IGovernance} from "contracts/governance/IGovernance.sol"; +import {Call} from "contracts/governance/Common.sol"; import {EventOnFallback} from "contracts/dev-contracts/EventOnFallback.sol"; import {Forwarder} from "contracts/dev-contracts/Forwarder.sol"; import {RevertFallback} from "contracts/dev-contracts/RevertFallback.sol"; @@ -58,8 +59,8 @@ contract GovernanceTest is Test, EventOnFallback { uint256 _value, bytes memory _data ) internal pure returns (IGovernance.Operation memory) { - IGovernance.Call[] memory calls = new IGovernance.Call[](1); - calls[0] = IGovernance.Call({target: _target, value: _value, data: _data}); + Call[] memory calls = new Call[](1); + calls[0] = Call({target: _target, value: _value, data: _data}); return IGovernance.Operation({calls: calls, salt: bytes32(0), predecessor: bytes32(0)}); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/CreateNewChain.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/CreateNewChain.t.sol index ba69c6bd7..0598779a4 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/CreateNewChain.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/CreateNewChain.t.sol @@ -6,6 +6,10 @@ import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; import {Unauthorized, HashMismatch} from "contracts/common/L1ContractErrors.sol"; contract createNewChainTest is StateTransitionManagerTest { + function setUp() public { + deploy(); + } + function test_RevertWhen_InitialDiamondCutHashMismatch() public { Diamond.DiamondCutData memory initialDiamondCutData = getDiamondCutData(sharedBridge); Diamond.DiamondCutData memory correctDiamondCutData = getDiamondCutData(address(diamondInit)); diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/FreezeChain.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/FreezeChain.t.sol index 1bf8c8a40..98dc2661e 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/FreezeChain.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/FreezeChain.t.sol @@ -7,6 +7,10 @@ import {IAdmin} from "contracts/state-transition/chain-interfaces/IAdmin.sol"; import {FacetIsFrozen} from "contracts/common/L1ContractErrors.sol"; contract freezeChainTest is StateTransitionManagerTest { + function setUp() public { + deploy(); + } + function test_FreezingChain() public { createNewChain(getDiamondCutData(diamondInit)); diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/RevertBatches.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/RevertBatches.t.sol index 2113f3467..78aef87c3 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/RevertBatches.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/RevertBatches.t.sol @@ -26,6 +26,10 @@ contract revertBatchesTest is StateTransitionManagerTest { ExecutorFacet internal executorFacet; GettersFacet internal gettersFacet; + function setUp() public { + deploy(); + } + function test_SuccessfulBatchReverting() public { createNewChain(getDiamondCutData(diamondInit)); diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetChainCreationParams.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetChainCreationParams.t.sol index 85fa1a316..eeee06beb 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetChainCreationParams.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetChainCreationParams.t.sol @@ -8,6 +8,10 @@ import {IExecutor} from "contracts/state-transition/chain-interfaces/IExecutor.s import {EMPTY_STRING_KECCAK, DEFAULT_L2_LOGS_TREE_ROOT_HASH} from "contracts/common/Config.sol"; contract SetChainCreationParamsTest is StateTransitionManagerTest { + function setUp() public { + deploy(); + } + function test_SettingInitialCutHash() public { bytes32 initialCutHash = keccak256(abi.encode(getDiamondCutData(address(diamondInit)))); address randomDiamondInit = address(0x303030303030303030303); diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetNewVersionUpgrade.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetNewVersionUpgrade.t.sol index ced7e3f7d..ceac175df 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetNewVersionUpgrade.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetNewVersionUpgrade.t.sol @@ -5,6 +5,10 @@ import {StateTransitionManagerTest} from "./_StateTransitionManager_Shared.t.sol import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; contract setNewVersionUpgradeTest is StateTransitionManagerTest { + function setUp() public { + deploy(); + } + function test_SettingNewVersionUpgrade() public { assertEq(chainContractAddress.protocolVersion(), 0, "Initial protocol version is not correct"); diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetUpgradeDiamondCut.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetUpgradeDiamondCut.t.sol index a71f35d2e..32d7614b5 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetUpgradeDiamondCut.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetUpgradeDiamondCut.t.sol @@ -5,6 +5,10 @@ import {StateTransitionManagerTest} from "./_StateTransitionManager_Shared.t.sol import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; contract setUpgradeDiamondCutTest is StateTransitionManagerTest { + function setUp() public { + deploy(); + } + function test_SettingUpgradeDiamondCut() public { assertEq(chainContractAddress.protocolVersion(), 0, "Initial protocol version is not correct"); diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetValidatorTimelock.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetValidatorTimelock.t.sol index d290a8767..2741b3f72 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetValidatorTimelock.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/SetValidatorTimelock.t.sol @@ -4,6 +4,10 @@ pragma solidity 0.8.24; import {StateTransitionManagerTest} from "./_StateTransitionManager_Shared.t.sol"; contract setValidatorTimelockTest is StateTransitionManagerTest { + function setUp() public { + deploy(); + } + function test_SettingValidatorTimelock() public { assertEq( chainContractAddress.validatorTimelock(), diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/StateTransitionOwnerZero.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/StateTransitionOwnerZero.t.sol index d8fb6e187..8b0e0ce09 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/StateTransitionOwnerZero.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/StateTransitionOwnerZero.t.sol @@ -8,6 +8,10 @@ import {StateTransitionManagerInitializeData, ChainCreationParams} from "contrac import {ZeroAddress} from "contracts/common/L1ContractErrors.sol"; contract initializingSTMOwnerZeroTest is StateTransitionManagerTest { + function setUp() public { + deploy(); + } + function test_InitializingSTMWithGovernorZeroShouldRevert() public { ChainCreationParams memory chainCreationParams = ChainCreationParams({ genesisUpgrade: address(genesisUpgradeContract), diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/_StateTransitionManager_Shared.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/_StateTransitionManager_Shared.t.sol index 999336642..918cfa476 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/_StateTransitionManager_Shared.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/_StateTransitionManager_Shared.t.sol @@ -7,6 +7,7 @@ import {Test} from "forge-std/Test.sol"; import {TransparentUpgradeableProxy} from "@openzeppelin/contracts-v4/proxy/transparent/TransparentUpgradeableProxy.sol"; import {Utils} from "foundry-test/unit/concrete/Utils/Utils.sol"; +import {Bridgehub} from "contracts/bridgehub/Bridgehub.sol"; import {UtilsFacet} from "foundry-test/unit/concrete/Utils/UtilsFacet.sol"; import {AdminFacet} from "contracts/state-transition/chain-deps/facets/Admin.sol"; import {ExecutorFacet} from "contracts/state-transition/chain-deps/facets/Executor.sol"; @@ -24,7 +25,7 @@ contract StateTransitionManagerTest is Test { StateTransitionManager internal stateTransitionManager; StateTransitionManager internal chainContractAddress; GenesisUpgrade internal genesisUpgradeContract; - address internal bridgehub; + Bridgehub internal bridgehub; address internal diamondInit; address internal constant governor = address(0x1010101); address internal constant admin = address(0x2020202); @@ -37,12 +38,12 @@ contract StateTransitionManagerTest is Test { Diamond.FacetCut[] internal facetCuts; - function setUp() public { - bridgehub = makeAddr("bridgehub"); + function deploy() public { + bridgehub = new Bridgehub(); newChainAdmin = makeAddr("chainadmin"); - vm.startPrank(bridgehub); - stateTransitionManager = new StateTransitionManager(bridgehub, type(uint256).max); + vm.startPrank(address(bridgehub)); + stateTransitionManager = new StateTransitionManager(address(bridgehub), type(uint256).max); diamondInit = address(new DiamondInit()); genesisUpgradeContract = new GenesisUpgrade(); @@ -129,7 +130,7 @@ contract StateTransitionManagerTest is Test { function createNewChain(Diamond.DiamondCutData memory _diamondCut) internal { vm.stopPrank(); - vm.startPrank(bridgehub); + vm.startPrank(address(bridgehub)); chainContractAddress.createNewChain({ _chainId: chainId,