Merge pull request #777 from matter-labs/sb-merge-dev-kl-sync-layer-r…

…eorg

Merge dev into kl sync layer reorg

.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

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

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,

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 [email protected]
+/// @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);
+            }
+        }
+    }
+}

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 [email protected]
 /// @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);
+    }
 }

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;
+}

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

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 [email protected]
+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);
+}

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 [email protected]
 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;
 }