Skip to content

Commit

Permalink
(fix): merge conflicts from all commits
Browse files Browse the repository at this point in the history
  • Loading branch information
Raid Ateir committed Oct 29, 2024
1 parent 9bcd15b commit bf8e072
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 140 deletions.
2 changes: 1 addition & 1 deletion l1-contracts/contracts/dev-contracts/DummyRestriction.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ contract DummyRestriction is 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 virtual view {
function validateCall(Call calldata _call, address _invoker) external view virtual {
// nothing
}
}
10 changes: 2 additions & 8 deletions l1-contracts/contracts/governance/AccessControlRestriction.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,9 @@ import {Call} from "./Common.sol";
/// @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
<<<<<<< HEAD
/// or not ETH should be passively stored in `ChainAdmin` account.
contract AccessControlRestriction is Restriction, IAccessControlRestriction, AccessControlDefaultAdminRules {
/// @notice Required roles to call a specific functions.
=======
/// or no ETH should be passively stored in `ChainAdmin` account.
contract AccessControlRestriction is IRestriction, IAccessControlRestriction, AccessControlDefaultAdminRules {
contract AccessControlRestriction is Restriction, IAccessControlRestriction, AccessControlDefaultAdminRules {
/// @notice Required roles to call a specific function.
>>>>>>> origin/vb-governance-n05
/// @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;

Expand Down Expand Up @@ -66,7 +60,7 @@ contract AccessControlRestriction is IRestriction, IAccessControlRestriction, Ac
}

/// @inheritdoc Restriction
function validateCall(Call calldata _call, address _invoker) external override view {
function validateCall(Call calldata _call, address _invoker) external view override {
// 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.
Expand Down
23 changes: 5 additions & 18 deletions l1-contracts/contracts/governance/ChainAdmin.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ 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.
/// @dev Note, that it does not implement any form of access control by default, but instead utilizes
/// so called "restrictions": contracts that implement the `IRestriction` interface and ensure that
/// @dev Note, that it does not implement any form of access control by default, but instead utilizes
/// so called "restrictions": contracts that implement the `IRestriction` interface and ensure that
/// particular restrictions are ensured for the contract, including access control, security invariants, etc.
contract ChainAdmin is IChainAdmin, ReentrancyGuard {
using EnumerableSet for EnumerableSet.AddressSet;
Expand Down Expand Up @@ -107,15 +107,9 @@ contract ChainAdmin is IChainAdmin, ReentrancyGuard {
/// @dev Contract might receive/hold ETH as part of the maintenance process.
receive() external payable {}

<<<<<<< HEAD
/// @notice Function that ensures that the current admin can perform the call.
/// @dev Reverts in case the call can not be performed. Successfully executes otherwise.
function _validateCall(Call calldata _call) internal view {
=======
/// @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) private view {
>>>>>>> origin/vb-governance-n03
address[] memory restrictions = getRestrictions();

unchecked {
Expand All @@ -127,19 +121,12 @@ contract ChainAdmin is IChainAdmin, ReentrancyGuard {

/// @notice Adds a new restriction to the active restrictions set.
/// @param _restriction The address of the restriction contract to be added.
<<<<<<< HEAD
function _addRestriction(address _restriction) internal {
<<<<<<< HEAD
RestrictionValidator.validateRestriction(_restriction);

=======
function _addRestriction(address _restriction) private {
if (_restriction == address(0)) {
revert ZeroAddress();
}
>>>>>>> origin/sb-governance-l02
=======
function _addRestriction(address _restriction) private {
>>>>>>> origin/vb-governance-n03
RestrictionValidator.validateRestriction(_restriction);

if (!activeRestrictions.add(_restriction)) {
revert RestrictionWasAlreadyPresent(_restriction);
}
Expand Down
53 changes: 20 additions & 33 deletions l1-contracts/contracts/governance/L2AdminFactory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,8 @@
pragma solidity 0.8.24;

import {ChainAdmin} from "./ChainAdmin.sol";
<<<<<<< HEAD
import {RestrictionValidator} from "./restriction/RestrictionValidator.sol";
=======
import {ZeroAddress} from "../common/L1ContractErrors.sol";
>>>>>>> origin/sb-governance-l02

/// @author Matter Labs
/// @custom:security-contact [email protected]
Expand All @@ -28,72 +25,62 @@ contract L2AdminFactory {
address[] public requiredRestrictions;

constructor(address[] memory _requiredRestrictions) {
<<<<<<< HEAD
_validateRestrctions(_requiredRestrictions);
=======
_validateZeroAddress(_requiredRestrictions);
>>>>>>> origin/sb-governance-l02
_validateRestrctions(_requiredRestrictions);
requiredRestrictions = _requiredRestrictions;
}

/// @notice Deploys a new L2 admin contract.
/// @return admin The address of the deployed admin contract.
<<<<<<< HEAD
// solhint-disable-next-line gas-calldata-parameters
function deployAdmin(address[] memory _additionalRestrictions, bytes32 _salt) external returns (address admin) {
// Even though the chain admin will likely perform similar checks,
// Even though the chain admin will likely perform similar checks,
// we keep those here just in case, since it is not expensive, while allowing to fail fast.
<<<<<<< HEAD
_validateRestrctions(_additionalRestrictions);
=======
_validateZeroAddress(_additionalRestrictions);
_validateRestrctions(_additionalRestrictions);

>>>>>>> origin/sb-governance-l02
address[] memory restrictions = new address[](requiredRestrictions.length + _additionalRestrictions.length);
=======
function deployAdmin(address[] calldata _additionalRestrictions, bytes32 _salt) external returns (address admin) {
>>>>>>> origin/sb-governance-n01
uint256 cachedRequired = requiredRestrictions.length;
uint256 cachedAdditional = _additionalRestrictions.length;
address[] memory restrictions = new address[](cachedRequired + cachedRequired);

address[] memory restrictions = new address[](cachedRequired + cachedAdditional);

unchecked {
for (uint256 i = 0; i < cachedRequired; ++i) {
restrictions[i] = requiredRestrictions[i];
}
for (uint256 i = 0; i < cachedAdditional; ++i) {
restrictions[cachedRequired + i] = _additionalRestrictions[i];
}
}
}

admin = address(new ChainAdmin{salt: _salt}(restrictions));

emit AdminDeployed(admin);
}

<<<<<<< HEAD
/// @notice Checks that the provided list of restrictions is correct.
/// @param _restrictions List of the restrictions to check.
/// @dev In case either of the restrictions is not correct, the function reverts.
function _validateRestrctions(address[] memory _restrictions) internal view {
unchecked {
uint256 length = _restrictions.length;
for(uint256 i = 0; i < length; ++i) {
RestrictionValidator.validateRestriction(_restrictions[i]);
=======
/// @notice Checks that the provided list of restrictions does not contain
/// any zero addresses.
/// @param _restrictions List of the restrictions to check.
/// @dev In case either of the restrictions is zero address, the function reverts.
function _validateZeroAddress(address[] memory _restrictions) internal view {
function _validateZeroAddress(address[] memory _restrictions) private pure {
unchecked {
uint256 length = _restrictions.length;
for(uint256 i = 0; i < length; ++i) {
for (uint256 i = 0; i < length; ++i) {
if (_restrictions[i] == address(0)) {
revert ZeroAddress();
}
>>>>>>> origin/sb-governance-l02
}
}
}

/// @notice Checks that the provided list of restrictions is correct.
/// @param _restrictions List of the restrictions to check.
/// @dev In case either of the restrictions is not correct, the function reverts.
function _validateRestrctions(address[] memory _restrictions) private view {
unchecked {
uint256 length = _restrictions.length;
for (uint256 i = 0; i < length; ++i) {
RestrictionValidator.validateRestriction(_restrictions[i]);
}
}
}
Expand Down
75 changes: 16 additions & 59 deletions l1-contracts/contracts/governance/PermanentRestriction.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,8 @@ import {IPermanentRestriction} from "./IPermanentRestriction.sol";
/// @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.
<<<<<<< HEAD
/// @dev Once of the instances of such contract is to ensure that a ZkSyncHyperchain is a rollup forever.
contract PermanentRestriction is Restriction, IPermanentRestriction, Ownable2StepUpgradeable {
=======
/// @dev One of the instances of such contract is enough to ensure that a ZkSyncHyperchain is a rollup forever.
contract PermanentRestriction is IRestriction, IPermanentRestriction, Ownable2StepUpgradeable {
>>>>>>> origin/vb-governance-n05
contract PermanentRestriction is Restriction, IPermanentRestriction, Ownable2StepUpgradeable {
/// @notice The address of the Bridgehub contract.
IBridgehub public immutable BRIDGE_HUB;

Expand Down Expand Up @@ -134,17 +129,10 @@ contract PermanentRestriction is IRestriction, IPermanentRestriction, Ownable2St
/// @param _call The call data.
/// @dev Note that we do not need to validate the migration to the L1 layer as the admin
/// is not changed in this case.
<<<<<<< HEAD
function _validateMigrationToL2(Call calldata _call) internal view {
(address admin, bool isMigration) = _getNewAdminFromMigration(_call);
if(isMigration) {
if(!allowedL2Admins[admin]) {
=======
function _validateMigrationToL2(Call calldata _call) private view {
_ensureEnoughGas();
try this.tryGetNewAdminFromMigration(_call) returns (address admin) {
(address admin, bool isMigration) = _getNewAdminFromMigration(_call);
if (isMigration) {
if (!allowedL2Admins[admin]) {
>>>>>>> origin/vb-governance-n03
revert NotAllowed(admin);
}
}
Expand Down Expand Up @@ -220,23 +208,9 @@ contract PermanentRestriction is IRestriction, IPermanentRestriction, Ownable2St
}

/// @notice Checks if the `msg.sender` is an admin of a certain ZkSyncHyperchain.
/// @notice Function is internal for testing purposes only.
/// @param _chain The address of the chain.
<<<<<<< HEAD
function _isAdminOfAChain(address _chain) internal view returns (bool) {
=======
function _isAdminOfAChain(address _chain) private view returns (bool) {
_ensureEnoughGas();
(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 {
>>>>>>> origin/vb-governance-n03
if (_chain == address(0)) {
return false;
}
Expand All @@ -252,7 +226,7 @@ contract PermanentRestriction is IRestriction, IPermanentRestriction, Ownable2St
if (!chainIdQuerySuccess) {
// It is not a hyperchain, so we can return `false` here.
return false;
}
}

// Note, that here it is important to use the legacy `getHyperchain` function, so that the contract
// is compatible with the legacy ones.
Expand All @@ -270,25 +244,18 @@ contract PermanentRestriction is IRestriction, IPermanentRestriction, Ownable2St
/// @notice Tries to call `IGetters.getChainId()` function on the `_chain`.
/// It ensures that the returndata is of correct format and if not, it returns false.
/// @param _chain The address of the potential chain
/// @return chainId The chainId of the chain.
/// @return chainId The chainId of the chain.
/// @return success Whether the call was successful.
/// If the second item is `false`, the caller should ignore the first value.
/// If the second item is `false`, the caller should ignore the first value.
function _getChainIdUnffallibleCall(address _chain) internal view returns (uint256 chainId, bool success) {
bytes4 selector = IGetters.getChainId.selector;

// Note, that we do use assembly here to ensure that the function does not panic in case of
// either incorrect `_chain` address or in case the returndata is too large
assembly {
// We use scratch space here, so it is safe
mstore(0, selector)
success := staticcall(
gas(),
_chain,
0,
4,
0,
0
)
success := staticcall(gas(), _chain, 0, 4, 0, 0)

let isReturndataSizeCorrect := eq(returndatasize(), 32)

Expand All @@ -306,15 +273,15 @@ contract PermanentRestriction is IRestriction, IPermanentRestriction, Ownable2St
/// @notice Tries to get the new admin from the migration.
/// @param _call The call data.
/// @return Returns a tuple of the new admin and whether the transaction is indeed the migration.
/// If the second item is `false`, the caller should ignore the first value.
/// @dev If any other error is returned, it is assumed to be out of gas or some other unexpected
/// If the second item is `false`, the caller should ignore the first value.
/// @dev If any other error is returned, it is assumed to be out of gas or some other unexpected
/// error that should be bubbled up by the caller.
function _getNewAdminFromMigration(Call calldata _call) internal view returns (address, bool) {
if (_call.target != address(BRIDGE_HUB)) {
return (address(0), false);
}

if(_call.data.length < 4) {
if (_call.data.length < 4) {
return (address(0), false);
}

Expand All @@ -325,7 +292,7 @@ contract PermanentRestriction is IRestriction, IPermanentRestriction, Ownable2St
address sharedBridge = BRIDGE_HUB.sharedBridge();

// Assuming that correctly encoded calldata is provided, the following line must never fail,
// since the correct selector was checked before.
// since the correct selector was checked before.
L2TransactionRequestTwoBridgesOuter memory request = abi.decode(
_call.data[4:],
(L2TransactionRequestTwoBridgesOuter)
Expand All @@ -349,10 +316,10 @@ contract PermanentRestriction is IRestriction, IPermanentRestriction, Ownable2St
}

// From now on, we know that the used encoding version is `NEW_ENCODING_VERSION` that is
// supported only in the new protocol version with Gateway support, so we can assume
// supported only in the new protocol version with Gateway support, so we can assume
// that the methods like e.g. Bridgehub.ctmAssetIdToAddress must exist.

// This is the format of the `secondBridgeData` under the `NEW_ENCODING_VERSION`.
// This is the format of the `secondBridgeData` under the `NEW_ENCODING_VERSION`.
// If it fails, it would mean that the data is not correct and the call would eventually fail anyway.
(bytes32 chainAssetId, bytes memory bridgehubData) = abi.decode(encodedData, (bytes32, bytes));

Expand All @@ -371,20 +338,10 @@ contract PermanentRestriction is IRestriction, IPermanentRestriction, Ownable2St
return (address(0), false);
}

// The asset handler of CTM is the bridgehub and so the following decoding should work
// The asset handler of CTM is the bridgehub and so the following decoding should work
BridgehubBurnCTMAssetData memory burnData = abi.decode(bridgehubData, (BridgehubBurnCTMAssetData));
(address l2Admin, ) = abi.decode(burnData.ctmData, (address, bytes));

<<<<<<< HEAD
return (l2Admin, true);
=======
return l2Admin;
}

function _ensureEnoughGas() private view {
if (gasleft() < MIN_GAS_FOR_FALLABLE_CALL) {
revert NotEnoughGas();
}
>>>>>>> origin/vb-governance-n03
}
}
4 changes: 2 additions & 2 deletions l1-contracts/contracts/governance/restriction/Restriction.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ import {IRestriction, RESTRICTION_MAGIC} from "./IRestriction.sol";
abstract contract Restriction is IRestriction {
/// @notice A method used to check that the contract supports this interface.
/// @return Returns the `RESTRICTION_MAGIC`
function getSupportsRestrictionMagic() external view returns (bytes32) {
function getSupportsRestrictionMagic() external pure returns (bytes32) {
return RESTRICTION_MAGIC;
}

/// @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 virtual view;
function validateCall(Call calldata _call, address _invoker) external view virtual;
}
6 changes: 3 additions & 3 deletions l1-contracts/contracts/governance/restriction/RestrictionValidator.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
pragma solidity 0.8.24;

import {NotARestriction} from "../../common/L1ContractErrors.sol";
import { IRestriction, RESTRICTION_MAGIC } from "./IRestriction.sol";
import {IRestriction, RESTRICTION_MAGIC} from "./IRestriction.sol";

/// @title Restriction validator
/// @author Matter Labs
Expand All @@ -12,8 +12,8 @@ import { IRestriction, RESTRICTION_MAGIC } from "./IRestriction.sol";
library RestrictionValidator {
/// @notice Ensures that the provided address implements the restriction interface
/// @dev Note that it *can not guarantee* that the corresponding address indeed implements
/// the interface completely or that it is implemented correctly. It is mainly used to
/// ensure that invalid restrictions can not be accidentally added.
/// the interface completely or that it is implemented correctly. It is mainly used to
/// ensure that invalid restrictions can not be accidentally added.
function validateRestriction(address _restriction) internal view {
if (IRestriction(_restriction).getSupportsRestrictionMagic() != RESTRICTION_MAGIC) {
revert NotARestriction(_restriction);
Expand Down
Loading

0 comments on commit bf8e072

Please sign in to comment.