Skip to content
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions l1-contracts/contracts/governance/IChainAdmin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ interface IChainAdmin {
/// @notice The EVM emulator has been enabled
event EnableEvmEmulator();

/// @notice Returns the upgrade timestamp for a specific protocol version.
/// @param _protocolVersion The protocol version to query.
/// @return The timestamp at which the upgrade is expected.
function protocolVersionToUpgradeTimestamp(uint256 _protocolVersion) external view returns (uint256);

/// @notice Returns the list of active restrictions.
function getRestrictions() external view returns (address[] memory);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,8 @@ abstract contract ChainTypeManagerBase is IChainTypeManager, ReentrancyGuard, Ow
uint256 _oldProtocolVersion,
Diamond.DiamondCutData calldata _diamondCut
) external onlyOwner {
IZKChain(getZKChain(_chainId)).upgradeChainFromVersion(_oldProtocolVersion, _diamondCut);
address chainAddress = getZKChain(_chainId);
IZKChain(chainAddress).upgradeChainFromVersion(chainAddress, _oldProtocolVersion, _diamondCut);
}

/// @dev executes upgrade on chain
Expand Down
14 changes: 14 additions & 0 deletions l1-contracts/contracts/state-transition/IValidatorTimelock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pragma solidity 0.8.28;

import {IL1Bridgehub} from "../core/bridgehub/IL1Bridgehub.sol";
import {IExecutor} from "./chain-interfaces/IExecutor.sol";
import {Diamond} from "./libraries/Diamond.sol";

/// @author Matter Labs
/// @custom:security-contact security@matterlabs.dev
Expand All @@ -13,12 +14,14 @@ interface IValidatorTimelock is IExecutor {
/// @param rotateReverterRole Whether to rotate the REVERTER_ROLE.
/// @param rotateProverRole Whether to rotate the PROVER_ROLE.
/// @param rotateExecutorRole Whether to rotate the EXECUTOR_ROLE.
/// @param rotateUpgraderRole Whether to rotate the UPGRADER_ROLE.
struct ValidatorRotationParams {
bool rotatePrecommitterRole;
bool rotateCommitterRole;
bool rotateReverterRole;
bool rotateProverRole;
bool rotateExecutorRole;
bool rotateUpgraderRole;
}

/// @notice The delay between committing and executing batches is changed.
Expand All @@ -34,6 +37,8 @@ interface IValidatorTimelock is IExecutor {
function PROVER_ROLE() external view returns (bytes32);
/// @notice Role hash for addresses allowed to execute batches on a chain.
function EXECUTOR_ROLE() external view returns (bytes32);
/// @notice Role hash for addresses allowed to upgrade chains.
function UPGRADER_ROLE() external view returns (bytes32);
/// @notice Optional admin role hash for managing PRECOMMITTER_ROLE assignments.
/// @dev Note, that it is optional, meaning that by default the admin role is held by the chain admin
function OPTIONAL_PRECOMMITTER_ADMIN_ROLE() external view returns (bytes32);
Expand All @@ -49,6 +54,9 @@ interface IValidatorTimelock is IExecutor {
/// @notice Optional admin role hash for managing EXECUTOR_ROLE assignments.
/// @dev Note, that it is optional, meaning that by default the admin role is held by the chain admin
function OPTIONAL_EXECUTOR_ADMIN_ROLE() external view returns (bytes32);
/// @notice Optional admin role hash for managing UPGRADER_ROLE assignments.
/// @dev Note, that it is optional, meaning that by default the admin role is held by the chain admin
function OPTIONAL_UPGRADER_ADMIN_ROLE() external view returns (bytes32);

/// @notice The address of the bridgehub
function BRIDGE_HUB() external view returns (IL1Bridgehub);
Expand Down Expand Up @@ -140,4 +148,10 @@ interface IValidatorTimelock is IExecutor {
uint256 _processBatchTo,
bytes calldata _batchData
) external;
/// @dev Make a call to the zkChain diamond contract to upgrade from a specific protocol version.
function upgradeChainFromVersion(
address _chainAddress,
uint256 _oldProtocolVersion,
Diamond.DiamondCutData calldata _diamondCut
) external;
}
28 changes: 26 additions & 2 deletions l1-contracts/contracts/state-transition/ValidatorTimelock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity 0.8.28;
import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable-v4/access/Ownable2StepUpgradeable.sol";
import {AccessControlEnumerablePerChainAddressUpgradeable} from "./AccessControlEnumerablePerChainAddressUpgradeable.sol";
import {LibMap} from "./libraries/LibMap.sol";
import {Diamond} from "./libraries/Diamond.sol";
import {IZKChain} from "./chain-interfaces/IZKChain.sol";
import {NotAZKChain, TimeNotReached} from "../common/L1ContractErrors.sol";
import {IL1Bridgehub} from "../core/bridgehub/IL1Bridgehub.sol";
Expand Down Expand Up @@ -47,6 +48,9 @@ contract ValidatorTimelock is
/// @inheritdoc IValidatorTimelock
bytes32 public constant override EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE");

/// @inheritdoc IValidatorTimelock
bytes32 public constant override UPGRADER_ROLE = keccak256("UPGRADER_ROLE");

/// @inheritdoc IValidatorTimelock
bytes32 public constant override OPTIONAL_PRECOMMITTER_ADMIN_ROLE = keccak256("OPTIONAL_PRECOMMITTER_ADMIN_ROLE");

Expand All @@ -62,6 +66,9 @@ contract ValidatorTimelock is
/// @inheritdoc IValidatorTimelock
bytes32 public constant override OPTIONAL_EXECUTOR_ADMIN_ROLE = keccak256("OPTIONAL_EXECUTOR_ADMIN_ROLE");

/// @inheritdoc IValidatorTimelock
bytes32 public constant override OPTIONAL_UPGRADER_ADMIN_ROLE = keccak256("OPTIONAL_UPGRADER_ADMIN_ROLE");

/// @inheritdoc IValidatorTimelock
IL1Bridgehub public immutable override BRIDGE_HUB;

Expand Down Expand Up @@ -122,6 +129,9 @@ contract ValidatorTimelock is
if (params.rotateExecutorRole) {
revokeRole(_chainAddress, EXECUTOR_ROLE, _validator);
}
if (params.rotateUpgraderRole) {
revokeRole(_chainAddress, UPGRADER_ROLE, _validator);
}
}

/// @inheritdoc IValidatorTimelock
Expand All @@ -134,7 +144,8 @@ contract ValidatorTimelock is
rotateCommitterRole: true,
rotateReverterRole: true,
rotateProverRole: true,
rotateExecutorRole: true
rotateExecutorRole: true,
rotateUpgraderRole: true
})
);
}
Expand Down Expand Up @@ -165,6 +176,9 @@ contract ValidatorTimelock is
if (params.rotateExecutorRole) {
grantRole(_chainAddress, EXECUTOR_ROLE, _validator);
}
if (params.rotateUpgraderRole) {
grantRole(_chainAddress, UPGRADER_ROLE, _validator);
}
}

/// @inheritdoc IValidatorTimelock
Expand All @@ -177,7 +191,8 @@ contract ValidatorTimelock is
rotateCommitterRole: true,
rotateReverterRole: true,
rotateProverRole: true,
rotateExecutorRole: true
rotateExecutorRole: true,
rotateUpgraderRole: true
})
);
}
Expand Down Expand Up @@ -272,6 +287,15 @@ contract ValidatorTimelock is
_propagateToZKChain(_chainAddress);
}

/// @inheritdoc IValidatorTimelock
function upgradeChainFromVersion(
address _chainAddress,
uint256, // _oldProtocolVersion (unused in this specific implementation)
Diamond.DiamondCutData calldata // _diamondCut (unused in this specific implementation)
) external onlyRole(_chainAddress, UPGRADER_ROLE) {
_propagateToZKChain(_chainAddress);
}

/// @dev Call the zkChain diamond contract with the same calldata as this contract was called.
/// Note: it is called the zkChain diamond contract, not delegatecalled!
function _propagateToZKChain(address _chainAddress) internal {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {RollupDAManager} from "../../data-availability/RollupDAManager.sol";
import {L2_DEPLOYER_SYSTEM_CONTRACT_ADDR} from "../../../common/l2-helpers/L2ContractAddresses.sol";
import {AllowedBytecodeTypes, IL2ContractDeployer} from "../../../common/interfaces/IL2ContractDeployer.sol";
import {IL1AssetTracker} from "../../../bridge/asset-tracker/IL1AssetTracker.sol";
import {IChainAdmin} from "../../../governance/IChainAdmin.sol";

// While formally the following import is not used, it is needed to inherit documentation from it
import {IZKChainBase} from "../../chain-interfaces/IZKChainBase.sol";
Expand Down Expand Up @@ -241,9 +242,10 @@ contract AdminFacet is ZKChainBase, IAdmin {

/// @inheritdoc IAdmin
function upgradeChainFromVersion(
address, // _chainAddress (unused in this specific implementation)
uint256 _oldProtocolVersion,
Diamond.DiamondCutData calldata _diamondCut
) external onlyAdminOrChainTypeManager {
) external onlyAdminOrChainTypeManagerOrValidator {
bytes32 cutHashInput = keccak256(abi.encode(_diamondCut));
bytes32 upgradeCutHash = IChainTypeManager(s.chainTypeManager).upgradeCutHash(_oldProtocolVersion);
if (cutHashInput != upgradeCutHash) {
Expand All @@ -253,6 +255,15 @@ contract AdminFacet is ZKChainBase, IAdmin {
if (s.protocolVersion != _oldProtocolVersion) {
revert ProtocolIdMismatch(s.protocolVersion, _oldProtocolVersion);
}

// Check that the auto upgrade timestamp has passed if the sender is not admin or chainTypeManager
if (msg.sender != s.admin && msg.sender != s.chainTypeManager) {
uint256 timestamp = IChainAdmin(s.admin).protocolVersionToUpgradeTimestamp(_oldProtocolVersion);
if (block.timestamp < timestamp) {
revert Unauthorized(msg.sender);
}
}

Diamond.diamondCut(_diamondCut);
emit ExecuteUpgrade(_diamondCut);
if (s.protocolVersion <= _oldProtocolVersion) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,84 +25,144 @@ contract ZKChainBase is ReentrancyGuard {

/// @notice Checks that the message sender is an active admin
modifier onlyAdmin() {
_onlyAdmin();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did we introduce so many changes in this file?

_;
}

function _onlyAdmin() internal view {
if (msg.sender != s.admin) {
revert Unauthorized(msg.sender);
}
_;
}

/// @notice Checks if validator is active
modifier onlyValidator() {
_onlyValidator();
_;
}

function _onlyValidator() internal view {
if (!s.validators[msg.sender]) {
revert Unauthorized(msg.sender);
}
_;
}

modifier onlyChainTypeManager() {
_onlyChainTypeManager();
_;
}

function _onlyChainTypeManager() internal view {
if (msg.sender != s.chainTypeManager) {
revert Unauthorized(msg.sender);
}
_;
}

modifier onlyBridgehub() {
_onlyBridgehub();
_;
}

function _onlyBridgehub() internal view {
if (msg.sender != s.bridgehub) {
revert Unauthorized(msg.sender);
}
_;
}

modifier onlyBridgehubOrInteropCenter() {
_onlyBridgehubOrInteropCenter();
_;
}

function _onlyBridgehubOrInteropCenter() internal view {
if ((msg.sender != s.bridgehub) && (msg.sender != L2_INTEROP_CENTER_ADDR)) {
revert Unauthorized(msg.sender);
}
_;
}

modifier onlyGatewayAssetTracker() {
_onlyGatewayAssetTracker();
_;
}

function _onlyGatewayAssetTracker() internal view {
if (msg.sender != GW_ASSET_TRACKER_ADDR) {
revert Unauthorized(msg.sender);
}
_;
}

modifier onlyChainAssetHandler() {
_onlyChainAssetHandler();
_;
}

function _onlyChainAssetHandler() internal view {
if (msg.sender != IL1Bridgehub(s.bridgehub).chainAssetHandler()) {
revert Unauthorized(msg.sender);
}
_;
}

modifier onlyAdminOrChainTypeManager() {
_onlyAdminOrChainTypeManager();
_;
}

function _onlyAdminOrChainTypeManager() internal view {
if (msg.sender != s.admin && msg.sender != s.chainTypeManager) {
revert Unauthorized(msg.sender);
}
}

modifier onlyAdminOrChainTypeManagerOrValidator() {
_onlyAdminOrChainTypeManagerOrValidator();
_;
}

function _onlyAdminOrChainTypeManagerOrValidator() internal view {
if (msg.sender != s.admin && msg.sender != s.chainTypeManager && !s.validators[msg.sender]) {
revert Unauthorized(msg.sender);
}
}

modifier onlyValidatorOrChainTypeManager() {
_onlyValidatorOrChainTypeManager();
_;
}

function _onlyValidatorOrChainTypeManager() internal view {
if (!s.validators[msg.sender] && msg.sender != s.chainTypeManager) {
revert Unauthorized(msg.sender);
}
_;
}

modifier onlySettlementLayer() {
_onlySettlementLayer();
_;
}

function _onlySettlementLayer() internal view {
if (s.settlementLayer != address(0)) {
revert NotSettlementLayer();
}
_;
}

modifier onlySelf() {
_onlySelf();
_;
}

function _onlySelf() internal view {
if (msg.sender != address(this)) {
revert Unauthorized(msg.sender);
}
_;
}

modifier onlyServiceTransaction() {
_onlyServiceTransaction();
_;
}

function _onlyServiceTransaction() internal view {
IBridgehubBase bridgehub = IBridgehubBase(s.bridgehub);
if (
/// Purposes.
Expand All @@ -119,7 +179,6 @@ contract ZKChainBase is ReentrancyGuard {
) {
revert Unauthorized(msg.sender);
}
_;
}

/// @notice Returns whether the priority queue is still active, i.e.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,14 @@ interface IAdmin is IZKChainBase {
function allowEvmEmulation() external returns (bytes32 canonicalTxHash);

/// @notice Perform the upgrade from the current protocol version with the corresponding upgrade data
/// @param _chainAddress The address of the chain being upgraded
/// @param _protocolVersion The current protocol version from which upgrade is executed
/// @param _cutData The diamond cut parameters that is executed in the upgrade
function upgradeChainFromVersion(uint256 _protocolVersion, Diamond.DiamondCutData calldata _cutData) external;
function upgradeChainFromVersion(
address _chainAddress,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that you have not yet worked on this change on the server side, but this is a breaking change for zkstack cli etc

uint256 _protocolVersion,
Diamond.DiamondCutData calldata _cutData
) external;

/// @notice Executes a proposed governor upgrade
/// @dev Only the ChainTypeManager contract can execute the upgrade
Expand Down
Loading
Loading