From 1d531f2cca64e28bc436b431303f96f3b16f9c62 Mon Sep 17 00:00:00 2001 From: vladbochok Date: Fri, 25 Oct 2024 13:22:06 +0200 Subject: [PATCH 01/29] Fix M-01 --- l1-contracts/contracts/upgrades/GatewayUpgrade.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol index 9ce428f96..6212b7005 100644 --- a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol +++ b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol @@ -32,6 +32,7 @@ contract GatewayUpgrade is BaseZkSyncUpgrade, Initializable { /// @notice The main function that will be called by the upgrade proxy. /// @param _proposedUpgrade The upgrade to be executed. + /// @dev Doesn't require any access-control restrictions as the contract is used in the delegate call. function upgrade(ProposedUpgrade calldata _proposedUpgrade) public override returns (bytes32) { (bytes memory l2TxDataStart, bytes memory l2TxDataFinish) = abi.decode( _proposedUpgrade.postUpgradeCalldata, @@ -58,9 +59,8 @@ contract GatewayUpgrade is BaseZkSyncUpgrade, Initializable { } /// @notice The function that will be called from this same contract, we need an external call to be able to modify _proposedUpgrade (memory/calldata). + /// @dev Doesn't require any access-control restrictions as the contract is used in the delegate call. function upgradeExternal(ProposedUpgrade calldata _proposedUpgrade) external { - // solhint-disable-next-line gas-custom-errors - require(msg.sender == address(this), "GatewayUpgrade: upgradeExternal"); super.upgrade(_proposedUpgrade); } } From 8487b79461747d268d693b4bc1eb95f4f4605bca Mon Sep 17 00:00:00 2001 From: vladbochok Date: Fri, 25 Oct 2024 14:40:59 +0200 Subject: [PATCH 02/29] Fix L-01 --- l1-contracts/contracts/upgrades/GatewayUpgrade.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol index 9ce428f96..354ef6383 100644 --- a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol +++ b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol @@ -2,8 +2,6 @@ pragma solidity 0.8.24; -import {Initializable} from "@openzeppelin/contracts-upgradeable-v4/proxy/utils/Initializable.sol"; - import {BaseZkSyncUpgrade, ProposedUpgrade} from "./BaseZkSyncUpgrade.sol"; import {DataEncoding} from "../common/libraries/DataEncoding.sol"; @@ -20,7 +18,7 @@ import {IBridgehub} from "../bridgehub/IBridgehub.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev /// @notice This upgrade will be used to migrate Era to be part of the ZK chain ecosystem contracts. -contract GatewayUpgrade is BaseZkSyncUpgrade, Initializable { +contract GatewayUpgrade is BaseZkSyncUpgrade { using PriorityQueue for PriorityQueue.Queue; using PriorityTree for PriorityTree.Tree; From e52430484a00f3450f69699d15611a41a1d442b3 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Fri, 25 Oct 2024 15:19:06 +0200 Subject: [PATCH 03/29] fix clarity --- .../contracts/common/libraries/DataEncoding.sol | 10 ++++++++-- .../contracts/dev-contracts/test/CustomUpgradeTest.sol | 2 +- l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol | 2 +- l1-contracts/contracts/upgrades/DefaultUpgrade.sol | 2 +- l1-contracts/contracts/upgrades/GatewayUpgrade.sol | 2 +- l1-contracts/contracts/upgrades/L1GenesisUpgrade.sol | 2 +- 6 files changed, 13 insertions(+), 7 deletions(-) diff --git a/l1-contracts/contracts/common/libraries/DataEncoding.sol b/l1-contracts/contracts/common/libraries/DataEncoding.sol index 9df83d67a..ee22cbfab 100644 --- a/l1-contracts/contracts/common/libraries/DataEncoding.sol +++ b/l1-contracts/contracts/common/libraries/DataEncoding.sol @@ -120,7 +120,12 @@ library DataEncoding { } } - /// @notice Decodes the token data by combining chain id, asset deployment tracker and asset data. + /// @notice Decodes the token data + /// @dev Note that all the returned metadata of the token is ABI encoded. + /// @return chainId The chainId of the origin of the token + /// @return name The name of the token. + /// @return symbol The symbol of the token. + /// @return decimals The decimals of the token. function decodeTokenData( bytes calldata _tokenData ) internal pure returns (uint256 chainId, bytes memory name, bytes memory symbol, bytes memory decimals) { @@ -135,7 +140,8 @@ library DataEncoding { } } - /// @notice Encodes the token data by combining chain id, asset deployment tracker and asset data. + /// @notice Encodes the token data by combining chain id, and its metadata. + /// @dev Note that all the metadata of the token is expected to be ABI encoded. /// @param _chainId The id of the chain token is native to. /// @param _name The name of the token. /// @param _symbol The symbol of the token. diff --git a/l1-contracts/contracts/dev-contracts/test/CustomUpgradeTest.sol b/l1-contracts/contracts/dev-contracts/test/CustomUpgradeTest.sol index 7055ce557..0a3958724 100644 --- a/l1-contracts/contracts/dev-contracts/test/CustomUpgradeTest.sol +++ b/l1-contracts/contracts/dev-contracts/test/CustomUpgradeTest.sol @@ -25,7 +25,7 @@ contract CustomUpgradeTest is BaseZkSyncUpgrade { /// upgrade. function _postUpgrade(bytes calldata _customCallDataForUpgrade) internal override {} - /// @notice The main function that will be called by the upgrade proxy. + /// @notice The main function that will be delegate-called by the chain. /// @param _proposedUpgrade The upgrade to be executed. function upgrade(ProposedUpgrade calldata _proposedUpgrade) public override returns (bytes32) { (uint32 newMinorVersion, bool isPatchOnly) = _setNewProtocolVersion(_proposedUpgrade.newProtocolVersion); diff --git a/l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol b/l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol index edae3870b..32dc186c4 100644 --- a/l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol +++ b/l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol @@ -63,7 +63,7 @@ abstract contract BaseZkSyncUpgrade is ZKChainBase { /// @notice Notifies about complete upgrade event UpgradeComplete(uint256 indexed newProtocolVersion, bytes32 indexed l2UpgradeTxHash, ProposedUpgrade upgrade); - /// @notice The main function that will be provided by the upgrade proxy + /// @notice The main function that will be delegate-called by the chain. /// @dev This is a virtual function and should be overridden by custom upgrade implementations. /// @param _proposedUpgrade The upgrade to be executed. /// @return txHash The hash of the L2 system contract upgrade transaction. diff --git a/l1-contracts/contracts/upgrades/DefaultUpgrade.sol b/l1-contracts/contracts/upgrades/DefaultUpgrade.sol index c6ebb18dc..87c6dd220 100644 --- a/l1-contracts/contracts/upgrades/DefaultUpgrade.sol +++ b/l1-contracts/contracts/upgrades/DefaultUpgrade.sol @@ -8,7 +8,7 @@ import {BaseZkSyncUpgrade, ProposedUpgrade} from "./BaseZkSyncUpgrade.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev contract DefaultUpgrade is BaseZkSyncUpgrade { - /// @notice The main function that will be called by the upgrade proxy. + /// @notice The main function that will be delegate-called by the chain. /// @param _proposedUpgrade The upgrade to be executed. function upgrade(ProposedUpgrade calldata _proposedUpgrade) public override returns (bytes32) { super.upgrade(_proposedUpgrade); diff --git a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol index 9ce428f96..1348d5f74 100644 --- a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol +++ b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol @@ -30,7 +30,7 @@ contract GatewayUpgrade is BaseZkSyncUpgrade, Initializable { THIS_ADDRESS = address(this); } - /// @notice The main function that will be called by the upgrade proxy. + /// @notice The main function that will be delegate-called by the chain. /// @param _proposedUpgrade The upgrade to be executed. function upgrade(ProposedUpgrade calldata _proposedUpgrade) public override returns (bytes32) { (bytes memory l2TxDataStart, bytes memory l2TxDataFinish) = abi.decode( diff --git a/l1-contracts/contracts/upgrades/L1GenesisUpgrade.sol b/l1-contracts/contracts/upgrades/L1GenesisUpgrade.sol index 4637c535d..1feaf77a5 100644 --- a/l1-contracts/contracts/upgrades/L1GenesisUpgrade.sol +++ b/l1-contracts/contracts/upgrades/L1GenesisUpgrade.sol @@ -21,7 +21,7 @@ import {L2ContractHelper} from "../common/libraries/L2ContractHelper.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev contract L1GenesisUpgrade is IL1GenesisUpgrade, BaseZkSyncUpgradeGenesis { - /// @notice The main function that will be called by the upgrade proxy. + /// @notice The main function that will be delegate-called by the chain. function genesisUpgrade( address _l1GenesisUpgrade, uint256 _chainId, From cca49de2a648bc76b23d899d1f72531d8472ab75 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Fri, 25 Oct 2024 14:58:34 +0100 Subject: [PATCH 04/29] fix: N-06 Typographical Errors --- l1-contracts/contracts/common/L2ContractAddresses.sol | 2 +- system-contracts/contracts/L2GenesisUpgrade.sol | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/l1-contracts/contracts/common/L2ContractAddresses.sol b/l1-contracts/contracts/common/L2ContractAddresses.sol index a8fba013c..0d9e59936 100644 --- a/l1-contracts/contracts/common/L2ContractAddresses.sol +++ b/l1-contracts/contracts/common/L2ContractAddresses.sol @@ -69,7 +69,7 @@ interface IL2Messenger { /// if the assetId can be calculated with this address then it is in fact an NTV asset address constant L2_NATIVE_TOKEN_VAULT_ADDR = address(0x10004); -/// @dev the address of the l2 asse3t router. +/// @dev the address of the l2 asset router. address constant L2_MESSAGE_ROOT_ADDR = address(0x10005); /// @dev the offset for the system contracts diff --git a/system-contracts/contracts/L2GenesisUpgrade.sol b/system-contracts/contracts/L2GenesisUpgrade.sol index aab22ff34..fbb3a1e30 100644 --- a/system-contracts/contracts/L2GenesisUpgrade.sol +++ b/system-contracts/contracts/L2GenesisUpgrade.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.24; -import {DEPLOYER_SYSTEM_CONTRACT, SYSTEM_CONTEXT_CONTRACT, L2_BRIDDGE_HUB, L2_ASSET_ROUTER, L2_MESSAGE_ROOT} from "./Constants.sol"; +import {DEPLOYER_SYSTEM_CONTRACT, SYSTEM_CONTEXT_CONTRACT, L2_BRIDGE_HUB, L2_ASSET_ROUTER, L2_MESSAGE_ROOT} from "./Constants.sol"; import {IContractDeployer, ForceDeployment} from "./interfaces/IContractDeployer.sol"; import {SystemContractHelper} from "./libraries/SystemContractHelper.sol"; import {ISystemContext} from "./interfaces/ISystemContext.sol"; @@ -28,20 +28,20 @@ contract L2GenesisUpgrade is IL2GenesisUpgrade { // (The comment does not mention the exact order in case it changes) // However, there is still some follow up finalization that needs to be done. - address bridgehubOwner = L2_BRIDDGE_HUB.owner(); + address bridgehubOwner = L2_BRIDGE_HUB.owner(); bytes memory data = abi.encodeCall( - L2_BRIDDGE_HUB.setAddresses, + L2_BRIDGE_HUB.setAddresses, (L2_ASSET_ROUTER, _ctmDeployer, address(L2_MESSAGE_ROOT)) ); (bool success, bytes memory returnData) = SystemContractHelper.mimicCall( - address(L2_BRIDDGE_HUB), + address(L2_BRIDGE_HUB), bridgehubOwner, data ); if (!success) { - // Progapatate revert reason + // Propagate revert reason assembly { revert(add(returnData, 0x20), returndatasize()) } From fcec3d33b0a32f067c76cc64c0ded8a1a26d8125 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Fri, 25 Oct 2024 15:08:27 +0100 Subject: [PATCH 05/29] fix: N-05 Unnecessary Cast --- .../contracts/common/libraries/SystemContractsCaller.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/common/libraries/SystemContractsCaller.sol b/l1-contracts/contracts/common/libraries/SystemContractsCaller.sol index b6bf0c54a..30dbf3a81 100644 --- a/l1-contracts/contracts/common/libraries/SystemContractsCaller.sol +++ b/l1-contracts/contracts/common/libraries/SystemContractsCaller.sol @@ -48,7 +48,7 @@ library SystemContractsCaller { assembly { dataStart := add(data, 0x20) } - uint32 dataLength = uint32(Utils.safeCastToU32(data.length)); + uint32 dataLength = Utils.safeCastToU32(data.length); uint256 farCallAbi = getFarCallABI({ dataOffset: 0, From 780b0277f0c09d262b0774cc51f6db03a4efeed0 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Fri, 25 Oct 2024 15:18:54 +0100 Subject: [PATCH 06/29] fix: N-04 Unsafe ABI Encoding --- l1-contracts/contracts/governance/PermanentRestriction.sol | 2 +- l1-contracts/contracts/upgrades/GatewayUpgrade.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/governance/PermanentRestriction.sol b/l1-contracts/contracts/governance/PermanentRestriction.sol index 153ce369e..7cf01a5d8 100644 --- a/l1-contracts/contracts/governance/PermanentRestriction.sol +++ b/l1-contracts/contracts/governance/PermanentRestriction.sol @@ -235,7 +235,7 @@ contract PermanentRestriction is IRestriction, IPermanentRestriction, Ownable2St // Note, that we do not use an explicit call here to ensure that the function does not panic in case of // incorrect `_chain` address. - (bool success, bytes memory data) = _chain.staticcall(abi.encodeWithSelector(IGetters.getChainId.selector)); + (bool success, bytes memory data) = _chain.staticcall(abi.encodeCall(IGetters.getChainId)); if (!success || data.length < 32) { revert NotAHyperchain(_chain); } diff --git a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol index 9ce428f96..cf75e8d76 100644 --- a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol +++ b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol @@ -50,7 +50,7 @@ contract GatewayUpgrade is BaseZkSyncUpgrade, Initializable { ); // slither-disable-next-line controlled-delegatecall (bool success, ) = THIS_ADDRESS.delegatecall( - abi.encodeWithSelector(IGatewayUpgrade.upgradeExternal.selector, proposedUpgrade) + abi.encodeCall(IGatewayUpgrade.upgradeExternal, proposedUpgrade) ); // solhint-disable-next-line gas-custom-errors require(success, "GatewayUpgrade: upgrade failed"); From c5934cefa7bfe18aee337bab6ebad7fa8ce775c3 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Fri, 25 Oct 2024 15:30:35 +0100 Subject: [PATCH 07/29] fix: N-03 Unnecessary Code --- .../contracts/bridge/L2SharedBridgeLegacy.sol | 4 +-- .../bridge/ntv/L2NativeTokenVault.sol | 4 +-- .../contracts/common/L2ContractAddresses.sol | 3 -- .../common/interfaces/IL2ContractDeployer.sol | 32 ------------------- .../test/foundry/l2/unit/utils/L2Utils.sol | 8 ++--- 5 files changed, 8 insertions(+), 43 deletions(-) delete mode 100644 l1-contracts/contracts/common/interfaces/IL2ContractDeployer.sol diff --git a/l1-contracts/contracts/bridge/L2SharedBridgeLegacy.sol b/l1-contracts/contracts/bridge/L2SharedBridgeLegacy.sol index 61e6141c2..8753703b0 100644 --- a/l1-contracts/contracts/bridge/L2SharedBridgeLegacy.sol +++ b/l1-contracts/contracts/bridge/L2SharedBridgeLegacy.sol @@ -7,7 +7,7 @@ import {UpgradeableBeacon} from "@openzeppelin/contracts-v4/proxy/beacon/Upgrade import {BridgedStandardERC20} from "./BridgedStandardERC20.sol"; -import {DEPLOYER_SYSTEM_CONTRACT, L2_ASSET_ROUTER_ADDR, L2_NATIVE_TOKEN_VAULT_ADDR} from "../common/L2ContractAddresses.sol"; +import {L2_DEPLOYER_SYSTEM_CONTRACT_ADDR, L2_ASSET_ROUTER_ADDR, L2_NATIVE_TOKEN_VAULT_ADDR} from "../common/L2ContractAddresses.sol"; import {SystemContractsCaller} from "../common/libraries/SystemContractsCaller.sol"; import {L2ContractHelper, IContractDeployer} from "../common/libraries/L2ContractHelper.sol"; @@ -143,7 +143,7 @@ contract L2SharedBridgeLegacy is IL2SharedBridgeLegacy, Initializable { function deployBeaconProxy(bytes32 salt) external onlyNTV returns (address proxy) { (bool success, bytes memory returndata) = SystemContractsCaller.systemCallWithReturndata( uint32(gasleft()), - DEPLOYER_SYSTEM_CONTRACT, + L2_DEPLOYER_SYSTEM_CONTRACT_ADDR, 0, abi.encodeCall( IContractDeployer.create2, diff --git a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol index e96a6d289..b308b8d7b 100644 --- a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol @@ -16,7 +16,7 @@ import {NativeTokenVault} from "./NativeTokenVault.sol"; import {IL2SharedBridgeLegacy} from "../interfaces/IL2SharedBridgeLegacy.sol"; import {BridgedStandardERC20} from "../BridgedStandardERC20.sol"; -import {DEPLOYER_SYSTEM_CONTRACT, L2_ASSET_ROUTER_ADDR} from "../../common/L2ContractAddresses.sol"; +import {L2_DEPLOYER_SYSTEM_CONTRACT_ADDR, L2_ASSET_ROUTER_ADDR} from "../../common/L2ContractAddresses.sol"; import {L2ContractHelper, IContractDeployer} from "../../common/libraries/L2ContractHelper.sol"; import {SystemContractsCaller} from "../../common/libraries/SystemContractsCaller.sol"; @@ -136,7 +136,7 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { (bool success, bytes memory returndata) = SystemContractsCaller.systemCallWithReturndata( uint32(gasleft()), - DEPLOYER_SYSTEM_CONTRACT, + L2_DEPLOYER_SYSTEM_CONTRACT_ADDR, 0, abi.encodeCall( IContractDeployer.create2, diff --git a/l1-contracts/contracts/common/L2ContractAddresses.sol b/l1-contracts/contracts/common/L2ContractAddresses.sol index a8fba013c..d2459e443 100644 --- a/l1-contracts/contracts/common/L2ContractAddresses.sol +++ b/l1-contracts/contracts/common/L2ContractAddresses.sol @@ -75,9 +75,6 @@ address constant L2_MESSAGE_ROOT_ADDR = address(0x10005); /// @dev the offset for the system contracts uint160 constant SYSTEM_CONTRACTS_OFFSET = 0x8000; // 2^15 -/// @dev the address of the deployer system contract -address constant DEPLOYER_SYSTEM_CONTRACT = address(SYSTEM_CONTRACTS_OFFSET + 0x06); - /// @dev the address of the l2 messenger system contract IL2Messenger constant L2_MESSENGER = IL2Messenger(address(SYSTEM_CONTRACTS_OFFSET + 0x08)); diff --git a/l1-contracts/contracts/common/interfaces/IL2ContractDeployer.sol b/l1-contracts/contracts/common/interfaces/IL2ContractDeployer.sol deleted file mode 100644 index 015442dd9..000000000 --- a/l1-contracts/contracts/common/interfaces/IL2ContractDeployer.sol +++ /dev/null @@ -1,32 +0,0 @@ -// SPDX-License-Identifier: MIT -// 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; - -/** - * @author Matter Labs - * @notice System smart contract that is responsible for deploying other smart contracts on a ZK chain. - */ -interface IL2ContractDeployer { - /// @notice A struct that describes a forced deployment on an address. - /// @param bytecodeHash The bytecode hash to put on an address. - /// @param newAddress The address on which to deploy the bytecodehash to. - /// @param callConstructor Whether to run the constructor on the force deployment. - /// @param value The `msg.value` with which to initialize a contract. - /// @param input The constructor calldata. - struct ForceDeployment { - bytes32 bytecodeHash; - address newAddress; - bool callConstructor; - uint256 value; - bytes input; - } - - /// @notice This method is to be used only during an upgrade to set bytecodes on specific addresses. - function forceDeployOnAddresses(ForceDeployment[] calldata _deployParams) external; - - /// @notice Deploys a contract with similar address derivation rules to the EVM's `CREATE2` opcode. - /// @param _salt The create2 salt. - /// @param _bytecodeHash The correctly formatted hash of the bytecode. - /// @param _input The constructor calldata. - function create2(bytes32 _salt, bytes32 _bytecodeHash, bytes calldata _input) external; -} diff --git a/l1-contracts/test/foundry/l2/unit/utils/L2Utils.sol b/l1-contracts/test/foundry/l2/unit/utils/L2Utils.sol index 2c46774f4..7f1fec1ea 100644 --- a/l1-contracts/test/foundry/l2/unit/utils/L2Utils.sol +++ b/l1-contracts/test/foundry/l2/unit/utils/L2Utils.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.20; import {Vm} from "forge-std/Vm.sol"; -import {DEPLOYER_SYSTEM_CONTRACT, L2_ASSET_ROUTER_ADDR, L2_NATIVE_TOKEN_VAULT_ADDR} from "contracts/common/L2ContractAddresses.sol"; +import {L2_DEPLOYER_SYSTEM_CONTRACT_ADDR, L2_ASSET_ROUTER_ADDR, L2_NATIVE_TOKEN_VAULT_ADDR} from "contracts/common/L2ContractAddresses.sol"; import {IContractDeployer, L2ContractHelper} from "contracts/common/libraries/L2ContractHelper.sol"; import {L2AssetRouter} from "contracts/bridge/asset-router/L2AssetRouter.sol"; @@ -55,7 +55,7 @@ library L2Utils { */ function initSystemContracts() internal { bytes memory contractDeployerBytecode = readSystemContractsBytecode("ContractDeployer"); - vm.etch(DEPLOYER_SYSTEM_CONTRACT, contractDeployerBytecode); + vm.etch(L2_DEPLOYER_SYSTEM_CONTRACT_ADDR, contractDeployerBytecode); } /// @notice Deploys the L2AssetRouter contract. @@ -89,7 +89,7 @@ library L2Utils { }); vm.prank(L2_FORCE_DEPLOYER_ADDR); - IContractDeployer(DEPLOYER_SYSTEM_CONTRACT).forceDeployOnAddresses(deployments); + IContractDeployer(L2_DEPLOYER_SYSTEM_CONTRACT_ADDR).forceDeployOnAddresses(deployments); } /// @notice Deploys the L2NativeTokenVault contract. @@ -146,7 +146,7 @@ library L2Utils { }); vm.prank(L2_FORCE_DEPLOYER_ADDR); - IContractDeployer(DEPLOYER_SYSTEM_CONTRACT).forceDeployOnAddresses(deployments); + IContractDeployer(L2_DEPLOYER_SYSTEM_CONTRACT_ADDR).forceDeployOnAddresses(deployments); } /// @notice Encodes the token data. From ec21e78b2e6b692d2cd35403fb13282ae9f742a5 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Fri, 25 Oct 2024 15:32:08 +0100 Subject: [PATCH 08/29] fix: N-02 Legacy Naming --- .../transactionFilterer/GatewayTransactionFilterer.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol b/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol index 81556f221..9d5c39933 100644 --- a/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol +++ b/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol @@ -90,8 +90,8 @@ contract GatewayTransactionFilterer is ITransactionFilterer, ReentrancyGuard, Ow } (, bytes32 decodedAssetId, ) = abi.decode(l2Calldata[4:], (uint256, bytes32, bytes)); - address stmAddress = BRIDGE_HUB.ctmAssetIdToAddress(decodedAssetId); - return (stmAddress != address(0)); + address ctmAddress = BRIDGE_HUB.ctmAssetIdToAddress(decodedAssetId); + return (ctmAddress != address(0)); } return whitelistedSenders[sender]; From 752cb868492a4d43ed959fac8cb05f506eba207c Mon Sep 17 00:00:00 2001 From: kelemeno Date: Fri, 25 Oct 2024 16:06:36 +0100 Subject: [PATCH 09/29] fix: N-01 TODO Comments in the Code --- l1-contracts/contracts/common/libraries/DataEncoding.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/l1-contracts/contracts/common/libraries/DataEncoding.sol b/l1-contracts/contracts/common/libraries/DataEncoding.sol index 9df83d67a..abfe5831d 100644 --- a/l1-contracts/contracts/common/libraries/DataEncoding.sol +++ b/l1-contracts/contracts/common/libraries/DataEncoding.sol @@ -125,7 +125,6 @@ library DataEncoding { bytes calldata _tokenData ) internal pure returns (uint256 chainId, bytes memory name, bytes memory symbol, bytes memory decimals) { bytes1 encodingVersion = _tokenData[0]; - // kl todo check correct if (encodingVersion == LEGACY_ENCODING_VERSION) { (name, symbol, decimals) = abi.decode(_tokenData, (bytes, bytes, bytes)); } else if (encodingVersion == NEW_ENCODING_VERSION) { From 095b1208ff2868ceb1109dc965fe31ac485372f8 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Fri, 25 Oct 2024 16:20:19 +0100 Subject: [PATCH 10/29] fix: L-05 Unused Code --- l1-contracts/contracts/common/L1ContractErrors.sol | 10 ---------- .../contracts/common/libraries/UnsafeBytes.sol | 7 ------- l2-contracts/contracts/errors/L2ContractErrors.sol | 13 ------------- .../contracts/PubdataChunkPublisher.sol | 3 +-- 4 files changed, 1 insertion(+), 32 deletions(-) diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 48c90d540..8c9bd1602 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -138,8 +138,6 @@ error HashedLogIsDefault(); error HashMismatch(bytes32 expected, bytes32 actual); // 0xb615c2b1 error ZKChainLimitReached(); -// -error InsufficientAllowance(uint256 providedAllowance, uint256 requiredAmount); // 0xdd381a4c error IncorrectBridgeHubAddress(address bridgehub); // 0x826fb11e @@ -154,8 +152,6 @@ error InvalidChainId(); error InvalidDelay(); // 0x0af806e0 error InvalidHash(); -// -error InvalidInput(); // 0xc1780bd6 error InvalidLogSender(address sender, uint256 logKey); // 0xd8e9405c @@ -358,14 +354,10 @@ error UndefinedDiamondCutAction(); error UnexpectedNumberOfFactoryDeps(); // 0x6aa39880 error UnexpectedSystemLog(uint256 logKey); -// -error UnimplementedMessage(string); // 0xf093c2e5 error UpgradeBatchNumberIsNotZero(); // error UnsupportedEncodingVersion(); -// -error UnsupportedPaymasterFlow(); // 0x47b3b145 error ValidateTxnNotEnoughGas(); // 0x626ade30 @@ -388,8 +380,6 @@ error ZeroAddress(); error ZeroBalance(); // 0xc84885d4 error ZeroChainId(); -// 0x520aa59c -error PubdataIsEmpty(); // 0x99d8fec9 error EmptyData(); // 0xc99a8360 diff --git a/l1-contracts/contracts/common/libraries/UnsafeBytes.sol b/l1-contracts/contracts/common/libraries/UnsafeBytes.sol index 4edf94004..e2680d9e0 100644 --- a/l1-contracts/contracts/common/libraries/UnsafeBytes.sol +++ b/l1-contracts/contracts/common/libraries/UnsafeBytes.sol @@ -30,13 +30,6 @@ library UnsafeBytes { } } - function readUint128(bytes memory _bytes, uint256 _start) internal pure returns (uint128 result, uint256 offset) { - assembly { - offset := add(_start, 16) - result := mload(add(_bytes, offset)) - } - } - function readUint256(bytes memory _bytes, uint256 _start) internal pure returns (uint256 result, uint256 offset) { assembly { offset := add(_start, 32) diff --git a/l2-contracts/contracts/errors/L2ContractErrors.sol b/l2-contracts/contracts/errors/L2ContractErrors.sol index bb16f38c6..c29ba4781 100644 --- a/l2-contracts/contracts/errors/L2ContractErrors.sol +++ b/l2-contracts/contracts/errors/L2ContractErrors.sol @@ -2,33 +2,20 @@ // 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.20; -// 0x1f73225f -error AddressMismatch(address expected, address supplied); -error AssetIdMismatch(bytes32 expected, bytes32 supplied); // 0x5e85ae73 error AmountMustBeGreaterThanZero(); -// 0xb4f54111 -error DeployFailed(); // 0x7138356f error EmptyAddress(); -// 0x1c25715b -error EmptyBytes32(); // 0x1bdfd505 error FailedToTransferTokens(address tokenContract, address to, uint256 amount); // 0x2a1b2dd8 error InsufficientAllowance(uint256 providedAllowance, uint256 requiredAmount); -// 0xcbd9d2e0 -error InvalidCaller(address); // 0xb4fa3fb3 error InvalidInput(); -// 0x0ac76f01 -error NonSequentialVersion(); // 0x8e4a23d6 error Unauthorized(address); // 0x6e128399 error Unimplemented(); -// 0xa4dde386 -error UnimplementedMessage(string message); // 0xff15b069 error UnsupportedPaymasterFlow(); // 0x750b219c diff --git a/system-contracts/contracts/PubdataChunkPublisher.sol b/system-contracts/contracts/PubdataChunkPublisher.sol index f61f0b5ac..7c2abf2e1 100644 --- a/system-contracts/contracts/PubdataChunkPublisher.sol +++ b/system-contracts/contracts/PubdataChunkPublisher.sol @@ -2,7 +2,6 @@ pragma solidity 0.8.24; import {IPubdataChunkPublisher} from "./interfaces/IPubdataChunkPublisher.sol"; -import {SystemContractBase} from "./abstract/SystemContractBase.sol"; import {BLOB_SIZE_BYTES, MAX_NUMBER_OF_BLOBS} from "./Constants.sol"; import {TooMuchPubdata} from "./SystemContractErrors.sol"; @@ -11,7 +10,7 @@ import {TooMuchPubdata} from "./SystemContractErrors.sol"; * @custom:security-contact security@matterlabs.dev * @notice Smart contract for chunking pubdata into the appropriate size for EIP-4844 blobs. */ -contract PubdataChunkPublisher is IPubdataChunkPublisher, SystemContractBase { +contract PubdataChunkPublisher is IPubdataChunkPublisher { /// @notice Chunks pubdata into pieces that can fit into blobs. /// @param _pubdata The total l2 to l1 pubdata that will be sent via L1 blobs. /// @dev Note: This is an early implementation, in the future we plan to support up to 16 blobs per l1 batch. From ba2cff6f5761e1fd0396b546a2878ea7da94f03b Mon Sep 17 00:00:00 2001 From: kelemeno Date: Fri, 25 Oct 2024 16:21:24 +0100 Subject: [PATCH 11/29] extra error message --- l1-contracts/test/foundry/l2/unit/weth/WETH.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/test/foundry/l2/unit/weth/WETH.t.sol b/l1-contracts/test/foundry/l2/unit/weth/WETH.t.sol index 6cbc44fa7..1786c5839 100644 --- a/l1-contracts/test/foundry/l2/unit/weth/WETH.t.sol +++ b/l1-contracts/test/foundry/l2/unit/weth/WETH.t.sol @@ -7,7 +7,7 @@ import {Test} from "forge-std/Test.sol"; import {L2WrappedBaseToken} from "contracts/bridge/L2WrappedBaseToken.sol"; import {TransparentUpgradeableProxy} from "@openzeppelin/contracts-v4/proxy/transparent/TransparentUpgradeableProxy.sol"; -import {Unauthorized, UnimplementedMessage, BridgeMintNotImplemented} from "contracts/common/L1ContractErrors.sol"; +import {Unauthorized, BridgeMintNotImplemented} from "contracts/common/L1ContractErrors.sol"; contract WethTest is Test { L2WrappedBaseToken internal weth; From 6f681a8f98f8227f593c138ef1eab10c209263a5 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Fri, 25 Oct 2024 17:01:16 +0100 Subject: [PATCH 12/29] fix: L-04 Missing or Incomplete Docstrings --- .../contracts/bridgehub/IMessageRoot.sol | 7 +++++-- .../common/interfaces/IL2ContractDeployer.sol | 1 + .../contracts/upgrades/GatewayUpgrade.sol | 2 ++ .../contracts/upgrades/IGatewayUpgrade.sol | 7 +++++++ .../contracts/upgrades/IL1GenesisUpgrade.sol | 16 ++++++++++++++++ .../contracts/upgrades/L1GenesisUpgrade.sol | 8 +++++++- l2-contracts/contracts/L2ContractHelper.sol | 11 +++++++++++ system-contracts/contracts/L1Messenger.sol | 1 + system-contracts/contracts/L2GenesisUpgrade.sol | 7 ++++++- .../contracts/interfaces/IMessageRoot.sol | 7 +++++++ 10 files changed, 63 insertions(+), 4 deletions(-) diff --git a/l1-contracts/contracts/bridgehub/IMessageRoot.sol b/l1-contracts/contracts/bridgehub/IMessageRoot.sol index 2e15e6f63..cf76f8313 100644 --- a/l1-contracts/contracts/bridgehub/IMessageRoot.sol +++ b/l1-contracts/contracts/bridgehub/IMessageRoot.sol @@ -4,8 +4,11 @@ pragma solidity 0.8.24; import {IBridgehub} from "./IBridgehub.sol"; -/// @author Matter Labs -/// @custom:security-contact security@matterlabs.dev +/** +* @author Matter Labs +* @notice MessageRoot contract is responsible for storing and aggregating the roots of the batches from different chains into the MessageRoot. +* @custom:security-contact security@matterlabs.dev + */ interface IMessageRoot { function BRIDGE_HUB() external view returns (IBridgehub); diff --git a/l1-contracts/contracts/common/interfaces/IL2ContractDeployer.sol b/l1-contracts/contracts/common/interfaces/IL2ContractDeployer.sol index 015442dd9..f28c12c72 100644 --- a/l1-contracts/contracts/common/interfaces/IL2ContractDeployer.sol +++ b/l1-contracts/contracts/common/interfaces/IL2ContractDeployer.sol @@ -22,6 +22,7 @@ interface IL2ContractDeployer { } /// @notice This method is to be used only during an upgrade to set bytecodes on specific addresses. + /// @param _deployParams the force deployments function forceDeployOnAddresses(ForceDeployment[] calldata _deployParams) external; /// @notice Deploys a contract with similar address derivation rules to the EVM's `CREATE2` opcode. diff --git a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol index 9ce428f96..03320ab68 100644 --- a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol +++ b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol @@ -24,6 +24,8 @@ contract GatewayUpgrade is BaseZkSyncUpgrade, Initializable { using PriorityQueue for PriorityQueue.Queue; using PriorityTree for PriorityTree.Tree; + /// @notice The address of this contract. + /// @dev needed as this address is delegateCalled, and we delegateCall it again. address public immutable THIS_ADDRESS; constructor() { diff --git a/l1-contracts/contracts/upgrades/IGatewayUpgrade.sol b/l1-contracts/contracts/upgrades/IGatewayUpgrade.sol index eaa74c75b..ccd9941b1 100644 --- a/l1-contracts/contracts/upgrades/IGatewayUpgrade.sol +++ b/l1-contracts/contracts/upgrades/IGatewayUpgrade.sol @@ -4,6 +4,13 @@ pragma solidity 0.8.24; import {ProposedUpgrade} from "./BaseZkSyncUpgrade.sol"; +/** + * @author Matter Labs + * @notice Gateway upgrade interface. Used for the protocol upgrade that introduces the Gateway. + */ interface IGatewayUpgrade { + /// @notice The upgrade function called from within this same contract + /// @dev This is needed for memory -> calldata conversion of the _upgrade arg. + /// @param _upgrade The upgrade to be executed. function upgradeExternal(ProposedUpgrade calldata _upgrade) external returns (bytes32); } diff --git a/l1-contracts/contracts/upgrades/IL1GenesisUpgrade.sol b/l1-contracts/contracts/upgrades/IL1GenesisUpgrade.sol index 57dd40131..e9b5d1273 100644 --- a/l1-contracts/contracts/upgrades/IL1GenesisUpgrade.sol +++ b/l1-contracts/contracts/upgrades/IL1GenesisUpgrade.sol @@ -4,8 +4,17 @@ pragma solidity 0.8.24; import {L2CanonicalTransaction} from "../common/Messaging.sol"; +/** + * @author Matter Labs + * @notice L1 genesis upgrade interface. Every chain has to process an upgrade txs at its genesis. + * @notice This is needed to set system params like the chainId and to deploy some system contracts. + */ interface IL1GenesisUpgrade { /// @dev emitted when a chain registers and a GenesisUpgrade happens + /// @param _zkChain the address of the zk chain + /// @param _l2Transaction the l2 genesis upgrade transaction + /// @param _protocolVersion the current protocol version + /// @param _factoryDeps the factory dependencies needed for the upgrade event GenesisUpgrade( address indexed _zkChain, L2CanonicalTransaction _l2Transaction, @@ -13,6 +22,13 @@ interface IL1GenesisUpgrade { bytes[] _factoryDeps ); + /// @notice The main function that will be called by the Admin facet at genesis. + /// @param _l1GenesisUpgrade the address of the l1 genesis upgrade + /// @param _chainId the chain id + /// @param _protocolVersion the current protocol version + /// @param _l1CtmDeployerAddress the address of the l1 ctm deployer + /// @param _forceDeployments the force deployments + /// @param _factoryDeps the factory dependencies function genesisUpgrade( address _l1GenesisUpgrade, uint256 _chainId, diff --git a/l1-contracts/contracts/upgrades/L1GenesisUpgrade.sol b/l1-contracts/contracts/upgrades/L1GenesisUpgrade.sol index 4637c535d..5c85f409d 100644 --- a/l1-contracts/contracts/upgrades/L1GenesisUpgrade.sol +++ b/l1-contracts/contracts/upgrades/L1GenesisUpgrade.sol @@ -21,7 +21,13 @@ import {L2ContractHelper} from "../common/libraries/L2ContractHelper.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev contract L1GenesisUpgrade is IL1GenesisUpgrade, BaseZkSyncUpgradeGenesis { - /// @notice The main function that will be called by the upgrade proxy. + /// @notice The main function that will be called by the Admin facet. + /// @param _l1GenesisUpgrade the address of the l1 genesis upgrade + /// @param _chainId the chain id + /// @param _protocolVersion the current protocol version + /// @param _l1CtmDeployerAddress the address of the l1 ctm deployer + /// @param _forceDeploymentsData the force deployments data + /// @param _factoryDeps the factory dependencies function genesisUpgrade( address _l1GenesisUpgrade, uint256 _chainId, diff --git a/l2-contracts/contracts/L2ContractHelper.sol b/l2-contracts/contracts/L2ContractHelper.sol index 620e9b3ee..199370e9d 100644 --- a/l2-contracts/contracts/L2ContractHelper.sol +++ b/l2-contracts/contracts/L2ContractHelper.sol @@ -55,6 +55,12 @@ interface IContractDeployer { /// @param _input the calldata to be sent to the constructor of the new contract function create2(bytes32 _salt, bytes32 _bytecodeHash, bytes calldata _input) external returns (address); + /// @notice Calculates the address of a create2 contract deployment + /// @param _sender The address of the sender. + /// @param _bytecodeHash The bytecode hash of the new contract to be deployed. + /// @param _salt a unique value to create the deterministic address of the new contract + /// @param _input the calldata to be sent to the constructor of the new contract + /// @return newAddress The derived address of the account. function getNewAddressCreate2( address _sender, bytes32 _bytecodeHash, @@ -82,6 +88,11 @@ interface IBaseToken { * the compression of the state diffs and bytecodes. */ interface ICompressor { + /// @notice Verifies that the compression of state diffs has been done correctly for the {_stateDiffs} param. + /// @param _numberOfStateDiffs The number of state diffs being checked. + /// @param _enumerationIndexSize Number of bytes used to represent an enumeration index for repeated writes. + /// @param _stateDiffs Encoded full state diff structs. See the first dev comment below for encoding. + /// @param _compressedStateDiffs The compressed state diffs function verifyCompressedStateDiffs( uint256 _numberOfStateDiffs, uint256 _enumerationIndexSize, diff --git a/system-contracts/contracts/L1Messenger.sol b/system-contracts/contracts/L1Messenger.sol index 0f9242ef1..1f0cfe8e5 100644 --- a/system-contracts/contracts/L1Messenger.sol +++ b/system-contracts/contracts/L1Messenger.sol @@ -186,6 +186,7 @@ contract L1Messenger is IL1Messenger, SystemContractBase { /// @notice Verifies that the {_operatorInput} reflects what occurred within the L1Batch and that /// the compressed statediffs are equivalent to the full state diffs. + /// @param _l2DAValidator the address of the l2 da validator /// @param _operatorInput The total pubdata and uncompressed state diffs of transactions that were /// processed in the current L1 Batch. Pubdata consists of L2 to L1 Logs, messages, deployed bytecode, and state diffs. /// @dev Function that should be called exactly once per L1 Batch by the bootloader. diff --git a/system-contracts/contracts/L2GenesisUpgrade.sol b/system-contracts/contracts/L2GenesisUpgrade.sol index aab22ff34..8554d876c 100644 --- a/system-contracts/contracts/L2GenesisUpgrade.sol +++ b/system-contracts/contracts/L2GenesisUpgrade.sol @@ -10,8 +10,13 @@ import {IL2GenesisUpgrade} from "./interfaces/IL2GenesisUpgrade.sol"; /// @custom:security-contact security@matterlabs.dev /// @author Matter Labs -/// @notice The contract that can be used for deterministic contract deployment. +/// @notice The l2 component of the genesis upgrade. contract L2GenesisUpgrade is IL2GenesisUpgrade { + /// @notice The funciton that is delegateCalled from the complex upgrader. + /// @dev It is used to set the chainId and to deploy the force deployments. + /// @param _chainId the chain id + /// @param _ctmDeployer the address of the ctm deployer + /// @param _forceDeploymentsData the force deployments data function genesisUpgrade( uint256 _chainId, address _ctmDeployer, diff --git a/system-contracts/contracts/interfaces/IMessageRoot.sol b/system-contracts/contracts/interfaces/IMessageRoot.sol index 854508eb1..7bc35266a 100644 --- a/system-contracts/contracts/interfaces/IMessageRoot.sol +++ b/system-contracts/contracts/interfaces/IMessageRoot.sol @@ -2,6 +2,13 @@ // 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.20; +/** +* @author Matter Labs +* @notice MessageRoot contract is responsible for storing and aggregating the roots of the batches from different chains into the MessageRoot. +* @custom:security-contact security@matterlabs.dev + */ interface IMessageRoot { + /// @notice The aggregated root of the batches from different chains. + /// @return aggregatedRoot of the batches from different chains. function getAggregatedRoot() external view returns (bytes32 aggregatedRoot); } From 9ccd217d6a68913694d1784323f6a03738d56793 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Fri, 25 Oct 2024 17:02:44 +0100 Subject: [PATCH 13/29] fix: L-02 Unnecessary Legacy Check For Chain Migration --- system-contracts/contracts/L2GenesisUpgrade.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/system-contracts/contracts/L2GenesisUpgrade.sol b/system-contracts/contracts/L2GenesisUpgrade.sol index aab22ff34..ca234edbd 100644 --- a/system-contracts/contracts/L2GenesisUpgrade.sol +++ b/system-contracts/contracts/L2GenesisUpgrade.sol @@ -17,6 +17,7 @@ contract L2GenesisUpgrade is IL2GenesisUpgrade { address _ctmDeployer, bytes calldata _forceDeploymentsData ) external payable { + // solhint-disable-next-line gas-custom-errors require(_chainId != 0, "Invalid chainId"); ISystemContext(SYSTEM_CONTEXT_CONTRACT).setChainId(_chainId); From 0067c0a039eab46077537f1987fc8027ffc05e0e Mon Sep 17 00:00:00 2001 From: kelemeno Date: Fri, 25 Oct 2024 17:19:43 +0100 Subject: [PATCH 14/29] remove IL2Bridge --- .../contracts/bridge/interfaces/IL2Bridge.sol | 15 --------------- .../dev-contracts/test/DummySharedBridge.sol | 1 - .../GatewayTransactionFilterer.sol | 4 +--- .../CheckTransaction.sol | 10 +++++----- system-contracts/contracts/L2GenesisUpgrade.sol | 1 - 5 files changed, 6 insertions(+), 25 deletions(-) delete mode 100644 l1-contracts/contracts/bridge/interfaces/IL2Bridge.sol diff --git a/l1-contracts/contracts/bridge/interfaces/IL2Bridge.sol b/l1-contracts/contracts/bridge/interfaces/IL2Bridge.sol deleted file mode 100644 index 7fe7b7a97..000000000 --- a/l1-contracts/contracts/bridge/interfaces/IL2Bridge.sol +++ /dev/null @@ -1,15 +0,0 @@ -// SPDX-License-Identifier: MIT -// 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; - -/// @author Matter Labs -/// @custom:security-contact security@matterlabs.dev -interface IL2Bridge { - function withdraw(bytes32 _assetId, bytes memory _assetData) external; - - function finalizeDeposit(bytes32 _assetId, bytes calldata _transferData) external; - - function l1Bridge() external view returns (address); - - function setAssetHandlerAddress(bytes32 _assetId, address _assetAddress) external; -} diff --git a/l1-contracts/contracts/dev-contracts/test/DummySharedBridge.sol b/l1-contracts/contracts/dev-contracts/test/DummySharedBridge.sol index c75ec4530..0033fac3c 100644 --- a/l1-contracts/contracts/dev-contracts/test/DummySharedBridge.sol +++ b/l1-contracts/contracts/dev-contracts/test/DummySharedBridge.sol @@ -10,7 +10,6 @@ import {TWO_BRIDGES_MAGIC_VALUE, ETH_TOKEN_ADDRESS} from "../../common/Config.so import {IL1NativeTokenVault} from "../../bridge/ntv/L1NativeTokenVault.sol"; import {L2_NATIVE_TOKEN_VAULT_ADDR} from "../../common/L2ContractAddresses.sol"; import {SafeERC20} from "@openzeppelin/contracts-v4/token/ERC20/utils/SafeERC20.sol"; -import {IL2Bridge} from "../../bridge/interfaces/IL2Bridge.sol"; import {IL2SharedBridgeLegacy} from "../../bridge/interfaces/IL2SharedBridgeLegacy.sol"; import {IL2SharedBridgeLegacyFunctions} from "../../bridge/interfaces/IL2SharedBridgeLegacyFunctions.sol"; diff --git a/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol b/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol index 81556f221..775364fdd 100644 --- a/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol +++ b/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol @@ -8,7 +8,6 @@ import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; import {AlreadyWhitelisted, InvalidSelector, NotWhitelisted, ZeroAddress} from "../common/L1ContractErrors.sol"; import {ITransactionFilterer} from "../state-transition/chain-interfaces/ITransactionFilterer.sol"; import {IBridgehub} from "../bridgehub/IBridgehub.sol"; -import {IL2Bridge} from "../bridge/interfaces/IL2Bridge.sol"; import {IAssetRouterBase} from "../bridge/asset-router/IAssetRouterBase.sol"; /// @author Matter Labs @@ -83,8 +82,7 @@ contract GatewayTransactionFilterer is ITransactionFilterer, ReentrancyGuard, Ow if (sender == L1_ASSET_ROUTER) { bytes4 l2TxSelector = bytes4(l2Calldata[:4]); if ( - (IAssetRouterBase.finalizeDeposit.selector != l2TxSelector) && - (IL2Bridge.finalizeDeposit.selector != l2TxSelector) + (IAssetRouterBase.finalizeDeposit.selector != l2TxSelector) ) { revert InvalidSelector(l2TxSelector); } diff --git a/l1-contracts/test/foundry/unit/concrete/GatewayTransactionFilterer/CheckTransaction.sol b/l1-contracts/test/foundry/unit/concrete/GatewayTransactionFilterer/CheckTransaction.sol index 3231a7144..6ea6fd2c8 100644 --- a/l1-contracts/test/foundry/unit/concrete/GatewayTransactionFilterer/CheckTransaction.sol +++ b/l1-contracts/test/foundry/unit/concrete/GatewayTransactionFilterer/CheckTransaction.sol @@ -4,13 +4,13 @@ pragma solidity 0.8.24; import {GatewayTransactionFiltererTest} from "./_GatewayTransactionFilterer_Shared.t.sol"; import {IGetters} from "contracts/state-transition/chain-interfaces/IGetters.sol"; -import {IL2Bridge} from "contracts/bridge/interfaces/IL2Bridge.sol"; import {IBridgehub} from "contracts/bridgehub/IBridgehub.sol"; +import {IAssetRouterBase} from "contracts/bridge/asset-router/IAssetRouterBase.sol"; import {AlreadyWhitelisted, InvalidSelector, NotWhitelisted} from "contracts/common/L1ContractErrors.sol"; contract CheckTransactionTest is GatewayTransactionFiltererTest { function test_TransactionAllowedOnlyFromWhitelistedSenderWhichIsNotAssetRouter() public { - bytes memory txCalladata = abi.encodeCall(IL2Bridge.finalizeDeposit, (bytes32("0x12345"), bytes("0x23456"))); + bytes memory txCalladata = abi.encodeCall(IAssetRouterBase.finalizeDeposit, (uint256(10), bytes32("0x12345"), bytes("0x23456"))); vm.startPrank(owner); vm.mockCall( bridgehub, @@ -50,7 +50,7 @@ contract CheckTransactionTest is GatewayTransactionFiltererTest { function test_TransactionAllowedFromWhitelistedSenderForChainBridging() public { address stm = address(0x6060606); - bytes memory txCalladata = abi.encodeCall(IL2Bridge.finalizeDeposit, (bytes32("0x12345"), bytes("0x23456"))); + bytes memory txCalladata = abi.encodeCall(IAssetRouterBase.finalizeDeposit, (uint256(10), bytes32("0x12345"), bytes("0x23456"))); vm.startPrank(owner); vm.mockCall( bridgehub, @@ -74,9 +74,9 @@ contract CheckTransactionTest is GatewayTransactionFiltererTest { } function test_TransactionFailsWithInvalidSelectorEvenIfTheSenderIsAR() public { - bytes memory txCalladata = abi.encodeCall(IL2Bridge.withdraw, (bytes32("0x12345"), bytes("0x23456"))); + bytes memory txCalladata = abi.encodeCall(IAssetRouterBase.setAssetHandlerAddressThisChain, (bytes32("0x12345"), address(0x01234567890123456789))); vm.prank(owner); - vm.expectRevert(abi.encodeWithSelector(InvalidSelector.selector, IL2Bridge.withdraw.selector)); + vm.expectRevert(abi.encodeWithSelector(InvalidSelector.selector, IAssetRouterBase.setAssetHandlerAddressThisChain.selector)); bool isTxAllowed = transactionFiltererProxy.isTransactionAllowed( assetRouter, address(0), diff --git a/system-contracts/contracts/L2GenesisUpgrade.sol b/system-contracts/contracts/L2GenesisUpgrade.sol index ca234edbd..aab22ff34 100644 --- a/system-contracts/contracts/L2GenesisUpgrade.sol +++ b/system-contracts/contracts/L2GenesisUpgrade.sol @@ -17,7 +17,6 @@ contract L2GenesisUpgrade is IL2GenesisUpgrade { address _ctmDeployer, bytes calldata _forceDeploymentsData ) external payable { - // solhint-disable-next-line gas-custom-errors require(_chainId != 0, "Invalid chainId"); ISystemContext(SYSTEM_CONTEXT_CONTRACT).setChainId(_chainId); From 821749627c45523f4009e994caa5bb13fa5300cf Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 14:22:50 +0400 Subject: [PATCH 15/29] (fix): fmt + comments --- .../contracts/bridgehub/IMessageRoot.sol | 6 +++--- .../GatewayTransactionFilterer.sol | 4 +--- .../contracts/upgrades/IGatewayUpgrade.sol | 1 + .../contracts/upgrades/IL1GenesisUpgrade.sol | 1 + .../CheckTransaction.sol | 19 +++++++++++++++---- .../contracts/interfaces/IMessageRoot.sol | 6 +++--- 6 files changed, 24 insertions(+), 13 deletions(-) diff --git a/l1-contracts/contracts/bridgehub/IMessageRoot.sol b/l1-contracts/contracts/bridgehub/IMessageRoot.sol index cf76f8313..d4a3c7d7b 100644 --- a/l1-contracts/contracts/bridgehub/IMessageRoot.sol +++ b/l1-contracts/contracts/bridgehub/IMessageRoot.sol @@ -5,9 +5,9 @@ pragma solidity 0.8.24; import {IBridgehub} from "./IBridgehub.sol"; /** -* @author Matter Labs -* @notice MessageRoot contract is responsible for storing and aggregating the roots of the batches from different chains into the MessageRoot. -* @custom:security-contact security@matterlabs.dev + * @author Matter Labs + * @notice MessageRoot contract is responsible for storing and aggregating the roots of the batches from different chains into the MessageRoot. + * @custom:security-contact security@matterlabs.dev */ interface IMessageRoot { function BRIDGE_HUB() external view returns (IBridgehub); diff --git a/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol b/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol index 10fed4f46..46a7e8cd5 100644 --- a/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol +++ b/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol @@ -88,9 +88,7 @@ contract GatewayTransactionFilterer is ITransactionFilterer, ReentrancyGuard, Ow return _checkSTMAssetId(decodedAssetId); } - if ( - IAssetRouterBase.finalizeDeposit.selector != l2TxSelector - ) { + if (IAssetRouterBase.finalizeDeposit.selector != l2TxSelector) { revert InvalidSelector(l2TxSelector); } diff --git a/l1-contracts/contracts/upgrades/IGatewayUpgrade.sol b/l1-contracts/contracts/upgrades/IGatewayUpgrade.sol index ccd9941b1..c2c972a5e 100644 --- a/l1-contracts/contracts/upgrades/IGatewayUpgrade.sol +++ b/l1-contracts/contracts/upgrades/IGatewayUpgrade.sol @@ -6,6 +6,7 @@ import {ProposedUpgrade} from "./BaseZkSyncUpgrade.sol"; /** * @author Matter Labs + * @custom:security-contact security@matterlabs.dev * @notice Gateway upgrade interface. Used for the protocol upgrade that introduces the Gateway. */ interface IGatewayUpgrade { diff --git a/l1-contracts/contracts/upgrades/IL1GenesisUpgrade.sol b/l1-contracts/contracts/upgrades/IL1GenesisUpgrade.sol index e9b5d1273..c217e3be1 100644 --- a/l1-contracts/contracts/upgrades/IL1GenesisUpgrade.sol +++ b/l1-contracts/contracts/upgrades/IL1GenesisUpgrade.sol @@ -6,6 +6,7 @@ import {L2CanonicalTransaction} from "../common/Messaging.sol"; /** * @author Matter Labs + * @custom:security-contact security@matterlabs.dev * @notice L1 genesis upgrade interface. Every chain has to process an upgrade txs at its genesis. * @notice This is needed to set system params like the chainId and to deploy some system contracts. */ diff --git a/l1-contracts/test/foundry/unit/concrete/GatewayTransactionFilterer/CheckTransaction.sol b/l1-contracts/test/foundry/unit/concrete/GatewayTransactionFilterer/CheckTransaction.sol index 6ea6fd2c8..f9e22907f 100644 --- a/l1-contracts/test/foundry/unit/concrete/GatewayTransactionFilterer/CheckTransaction.sol +++ b/l1-contracts/test/foundry/unit/concrete/GatewayTransactionFilterer/CheckTransaction.sol @@ -10,7 +10,10 @@ import {AlreadyWhitelisted, InvalidSelector, NotWhitelisted} from "contracts/com contract CheckTransactionTest is GatewayTransactionFiltererTest { function test_TransactionAllowedOnlyFromWhitelistedSenderWhichIsNotAssetRouter() public { - bytes memory txCalladata = abi.encodeCall(IAssetRouterBase.finalizeDeposit, (uint256(10), bytes32("0x12345"), bytes("0x23456"))); + bytes memory txCalladata = abi.encodeCall( + IAssetRouterBase.finalizeDeposit, + (uint256(10), bytes32("0x12345"), bytes("0x23456")) + ); vm.startPrank(owner); vm.mockCall( bridgehub, @@ -50,7 +53,10 @@ contract CheckTransactionTest is GatewayTransactionFiltererTest { function test_TransactionAllowedFromWhitelistedSenderForChainBridging() public { address stm = address(0x6060606); - bytes memory txCalladata = abi.encodeCall(IAssetRouterBase.finalizeDeposit, (uint256(10), bytes32("0x12345"), bytes("0x23456"))); + bytes memory txCalladata = abi.encodeCall( + IAssetRouterBase.finalizeDeposit, + (uint256(10), bytes32("0x12345"), bytes("0x23456")) + ); vm.startPrank(owner); vm.mockCall( bridgehub, @@ -74,9 +80,14 @@ contract CheckTransactionTest is GatewayTransactionFiltererTest { } function test_TransactionFailsWithInvalidSelectorEvenIfTheSenderIsAR() public { - bytes memory txCalladata = abi.encodeCall(IAssetRouterBase.setAssetHandlerAddressThisChain, (bytes32("0x12345"), address(0x01234567890123456789))); + bytes memory txCalladata = abi.encodeCall( + IAssetRouterBase.setAssetHandlerAddressThisChain, + (bytes32("0x12345"), address(0x01234567890123456789)) + ); vm.prank(owner); - vm.expectRevert(abi.encodeWithSelector(InvalidSelector.selector, IAssetRouterBase.setAssetHandlerAddressThisChain.selector)); + vm.expectRevert( + abi.encodeWithSelector(InvalidSelector.selector, IAssetRouterBase.setAssetHandlerAddressThisChain.selector) + ); bool isTxAllowed = transactionFiltererProxy.isTransactionAllowed( assetRouter, address(0), diff --git a/system-contracts/contracts/interfaces/IMessageRoot.sol b/system-contracts/contracts/interfaces/IMessageRoot.sol index 7bc35266a..3966caa15 100644 --- a/system-contracts/contracts/interfaces/IMessageRoot.sol +++ b/system-contracts/contracts/interfaces/IMessageRoot.sol @@ -3,9 +3,9 @@ pragma solidity ^0.8.20; /** -* @author Matter Labs -* @notice MessageRoot contract is responsible for storing and aggregating the roots of the batches from different chains into the MessageRoot. -* @custom:security-contact security@matterlabs.dev + * @author Matter Labs + * @notice MessageRoot contract is responsible for storing and aggregating the roots of the batches from different chains into the MessageRoot. + * @custom:security-contact security@matterlabs.dev */ interface IMessageRoot { /// @notice The aggregated root of the batches from different chains. From d8eb4ae7560592ca9ae7f808ce7fb3364a3b5310 Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 14:28:07 +0400 Subject: [PATCH 16/29] (fix): remove TODO --- l1-contracts/contracts/common/Config.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/l1-contracts/contracts/common/Config.sol b/l1-contracts/contracts/common/Config.sol index 1aa26ba4f..0e3ed1f61 100644 --- a/l1-contracts/contracts/common/Config.sol +++ b/l1-contracts/contracts/common/Config.sol @@ -18,7 +18,6 @@ uint256 constant MAX_L2_TO_L1_LOGS_COMMITMENT_BYTES = 4 + L2_TO_L1_LOG_SERIALIZE /// @dev Actually equal to the `keccak256(new bytes(L2_TO_L1_LOG_SERIALIZE_SIZE))` bytes32 constant L2_L1_LOGS_TREE_DEFAULT_LEAF_HASH = 0x72abee45b59e344af8a6e520241c4744aff26ed411f4c4b00f8af09adada43ba; -// TODO: change constant to the real root hash of empty Merkle tree (SMA-184) bytes32 constant DEFAULT_L2_LOGS_TREE_ROOT_HASH = bytes32(0); /// @dev Denotes the type of the ZKsync transaction that came from L1. From 00fc57da67e25028fedce0f3ddf0ef18623f4759 Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 15:13:29 +0400 Subject: [PATCH 17/29] (fix): build --- l1-contracts/contracts/governance/PermanentRestriction.sol | 7 ------- l1-contracts/contracts/upgrades/GatewayUpgrade.sol | 4 +--- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/l1-contracts/contracts/governance/PermanentRestriction.sol b/l1-contracts/contracts/governance/PermanentRestriction.sol index 1ea60b79c..fc03364e2 100644 --- a/l1-contracts/contracts/governance/PermanentRestriction.sol +++ b/l1-contracts/contracts/governance/PermanentRestriction.sol @@ -219,13 +219,6 @@ contract PermanentRestriction is Restriction, IPermanentRestriction, Ownable2Ste // - Query the Bridgehub for the Hyperchain with the given `chainId`. // - We compare the corresponding addresses - // Note, that we do not use an explicit call here to ensure that the function does not panic in case of - // incorrect `_chain` address. - (bool success, bytes memory data) = _chain.staticcall(abi.encodeCall(IGetters.getChainId)); - if (!success || data.length < 32) { - revert NotAHyperchain(_chain); - } - // 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 (uint256 chainId, bool chainIdQuerySuccess) = _getChainIdUnffallibleCall(_chain); diff --git a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol index 7df523bad..5274c44c3 100644 --- a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol +++ b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol @@ -75,9 +75,7 @@ contract GatewayUpgrade is BaseZkSyncUpgrade { ); // slither-disable-next-line controlled-delegatecall - (bool success, ) = THIS_ADDRESS.delegatecall( - abi.encodeCall(IGatewayUpgrade.upgradeExternal, proposedUpgrade) - ); + (bool success, ) = THIS_ADDRESS.delegatecall(abi.encodeCall(IGatewayUpgrade.upgradeExternal, proposedUpgrade)); if (!success) { revert GatewayUpgradeFailed(); } From 797f1fc830e5069b2651fc5e7fd3fd2f26aa0308 Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 15:19:59 +0400 Subject: [PATCH 18/29] (fix): system contracts build --- system-contracts/contracts/L2GenesisUpgrade.sol | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/system-contracts/contracts/L2GenesisUpgrade.sol b/system-contracts/contracts/L2GenesisUpgrade.sol index e2652fcff..e8bceb48a 100644 --- a/system-contracts/contracts/L2GenesisUpgrade.sol +++ b/system-contracts/contracts/L2GenesisUpgrade.sol @@ -15,11 +15,12 @@ import {L2GatewayUpgradeHelper} from "./L2GatewayUpgradeHelper.sol"; /// @author Matter Labs /// @notice The l2 component of the genesis upgrade. contract L2GenesisUpgrade is IL2GenesisUpgrade { - /// @notice The funciton that is delegateCalled from the complex upgrader. + /// @notice The function that is delegateCalled from the complex upgrader. /// @dev It is used to set the chainId and to deploy the force deployments. /// @param _chainId the chain id /// @param _ctmDeployer the address of the ctm deployer - /// @param _forceDeploymentsData the force deployments data + /// @param _fixedForceDeploymentsData the force deployments data + /// @param _additionalForceDeploymentsData the additional force deployments data function genesisUpgrade( uint256 _chainId, address _ctmDeployer, @@ -31,7 +32,7 @@ contract L2GenesisUpgrade is IL2GenesisUpgrade { revert InvalidChainId(); } ISystemContext(SYSTEM_CONTEXT_CONTRACT).setChainId(_chainId); - ForceDeployment[] memory forceDeployments = abi.decode(_forceDeploymentsData, (ForceDeployment[])); + ForceDeployment[] memory forceDeployments = abi.decode(_fixedForceDeploymentsData, (ForceDeployment[])); IContractDeployer(DEPLOYER_SYSTEM_CONTRACT).forceDeployOnAddresses{value: msg.value}(forceDeployments); // It is expected that either via to the force deployments above From 5f8f2872cb318253b71d75a2ca0b31c42d25199d Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 15:23:44 +0400 Subject: [PATCH 19/29] (fix): lint --- l1-contracts/contracts/common/L1ContractErrors.sol | 4 ++-- l2-contracts/contracts/errors/L2ContractErrors.sol | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 190d23412..7697d30b9 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -119,8 +119,6 @@ error InsufficientChainBalance(); error InvalidCaller(address); // 0x4fbe5dba error InvalidDelay(); -// 0x0af806e0 -error InvalidHash(); // 0xc1780bd6 error InvalidLogSender(address sender, uint256 logKey); // 0xd8e9405c @@ -316,6 +314,8 @@ error IncorrectBatchBounds( ); // 0x64107968 error AssetHandlerNotRegistered(bytes32 assetId); +// 0xff4bbdf1 +error NotAHyperchain(address chainAddress); // 0x64846fe4 error NotARestriction(address addr); // 0xfa5cd00f diff --git a/l2-contracts/contracts/errors/L2ContractErrors.sol b/l2-contracts/contracts/errors/L2ContractErrors.sol index fd4b3536b..89e548ea0 100644 --- a/l2-contracts/contracts/errors/L2ContractErrors.sol +++ b/l2-contracts/contracts/errors/L2ContractErrors.sol @@ -18,8 +18,6 @@ error InsufficientAllowance(uint256 providedAllowance, uint256 requiredAmount); error InvalidInput(); // 0x8e4a23d6 error Unauthorized(address); -// 0x6e128399 -error Unimplemented(); // 0xff15b069 error UnsupportedPaymasterFlow(); // 0x750b219c From 824240d314ff9e128e92d7497bd98ca55e505ad4 Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 15:26:26 +0400 Subject: [PATCH 20/29] (fix): lint --- l1-contracts/contracts/common/L1ContractErrors.sol | 2 -- 1 file changed, 2 deletions(-) diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 7697d30b9..5aa1c0a50 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -314,8 +314,6 @@ error IncorrectBatchBounds( ); // 0x64107968 error AssetHandlerNotRegistered(bytes32 assetId); -// 0xff4bbdf1 -error NotAHyperchain(address chainAddress); // 0x64846fe4 error NotARestriction(address addr); // 0xfa5cd00f From 533ac4c007a658ef2d14788ccbb4aab5948b6e4a Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 15:41:45 +0400 Subject: [PATCH 21/29] (fix): small fixes --- .../governance/PermanentRestriction.sol | 7 ++++ system-contracts/SystemContractsHashes.json | 42 +++++++++---------- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/l1-contracts/contracts/governance/PermanentRestriction.sol b/l1-contracts/contracts/governance/PermanentRestriction.sol index fc03364e2..627870069 100644 --- a/l1-contracts/contracts/governance/PermanentRestriction.sol +++ b/l1-contracts/contracts/governance/PermanentRestriction.sol @@ -219,6 +219,13 @@ contract PermanentRestriction is Restriction, IPermanentRestriction, Ownable2Ste // - Query the Bridgehub for the Hyperchain with the given `chainId`. // - We compare the corresponding addresses + // Note, that we do not use an explicit call here to ensure that the function does not panic in case of + // incorrect `_chain` address. + (bool success, bytes memory data) = _chain.staticcall(abi.encodeWithSelector(IGetters.getChainId.selector)); + if (!success || data.length < 32) { + revert(); + } + // 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 (uint256 chainId, bool chainIdQuerySuccess) = _getChainIdUnffallibleCall(_chain); diff --git a/system-contracts/SystemContractsHashes.json b/system-contracts/SystemContractsHashes.json index 60a5b5c83..599fd11b4 100644 --- a/system-contracts/SystemContractsHashes.json +++ b/system-contracts/SystemContractsHashes.json @@ -3,49 +3,49 @@ "contractName": "AccountCodeStorage", "bytecodePath": "artifacts-zk/contracts-preprocessed/AccountCodeStorage.sol/AccountCodeStorage.json", "sourceCodePath": "contracts-preprocessed/AccountCodeStorage.sol", - "bytecodeHash": "0x0100005d279e00e3781eba9b8813f8de7bc5d652c848d583d0062b36b2cae29e", + "bytecodeHash": "0x0100005df535e7d1e6f3933b26076778d9c44fd6e7faf546732f08290d8c8f94", "sourceCodeHash": "0x2e0e09d57a04bd1e722d8bf8c6423fdf3f8bca44e5e8c4f6684f987794be066e" }, { "contractName": "BootloaderUtilities", "bytecodePath": "artifacts-zk/contracts-preprocessed/BootloaderUtilities.sol/BootloaderUtilities.json", "sourceCodePath": "contracts-preprocessed/BootloaderUtilities.sol", - "bytecodeHash": "0x010007c7d96a06465d57858ea13b4bb612215046e127974b5f5b5808f9e3c788", + "bytecodeHash": "0x010007c72b7f29a0e1954ee4c65e6598d0934d33c692faedd7ac4fe30b508fa3", "sourceCodeHash": "0x0f1213c4b95acb71f4ab5d4082cc1aeb2bd5017e1cccd46afc66e53268609d85" }, { "contractName": "ComplexUpgrader", "bytecodePath": "artifacts-zk/contracts-preprocessed/ComplexUpgrader.sol/ComplexUpgrader.json", "sourceCodePath": "contracts-preprocessed/ComplexUpgrader.sol", - "bytecodeHash": "0x010000bf784451a15f1bdcaa8ed5770cc25235984e44ce605779b79849386467", + "bytecodeHash": "0x010000bf6fcec0995b82b1c51133a507c8f63111234530b69fe7dadaae0c8172", "sourceCodeHash": "0xfcc74aefbc7cbde7945c29bad0e47527ac443bd6b75251a4ae520e28c714af37" }, { "contractName": "Compressor", "bytecodePath": "artifacts-zk/contracts-preprocessed/Compressor.sol/Compressor.json", "sourceCodePath": "contracts-preprocessed/Compressor.sol", - "bytecodeHash": "0x0100014b6c51a439159f8e86a891e3515eeea6b782efcba60b7b2ff0008a6869", + "bytecodeHash": "0x0100014b2cac967629cb05fb59a5c77cb5a077b74c50521ed9216a59511bf182", "sourceCodeHash": "0x7240b5fb2ea8e184522e731fb14f764ebae52b8a69d1870a55daedac9a3ed617" }, { "contractName": "ContractDeployer", "bytecodePath": "artifacts-zk/contracts-preprocessed/ContractDeployer.sol/ContractDeployer.json", "sourceCodePath": "contracts-preprocessed/ContractDeployer.sol", - "bytecodeHash": "0x010004e51f0fb3af074c21411a299285b9b6760d577a79bfd98a6d15bde5bd2c", + "bytecodeHash": "0x010004e5a266e697bb45bc90ff310dcb293725006146ff83e46bde8f3c6b44fa", "sourceCodeHash": "0x92bc09da23ed9d86ba7a84f0dbf48503c99582ae58cdbebbdcc5f14ea1fcf014" }, { "contractName": "Create2Factory", "bytecodePath": "artifacts-zk/contracts-preprocessed/Create2Factory.sol/Create2Factory.json", "sourceCodePath": "contracts-preprocessed/Create2Factory.sol", - "bytecodeHash": "0x0100004979f4353b7edd11ad71b4e0435ae74dc669248f12646e06a95ae5eeec", + "bytecodeHash": "0x010000493a391e65a70dea42442132cf7c7001dac94388b9c4218ce9b1491b57", "sourceCodeHash": "0x97392413259e6aae5187768cefd734507460ae818d6975709cc9b4e15a9af906" }, { "contractName": "DefaultAccount", "bytecodePath": "artifacts-zk/contracts-preprocessed/DefaultAccount.sol/DefaultAccount.json", "sourceCodePath": "contracts-preprocessed/DefaultAccount.sol", - "bytecodeHash": "0x0100055d7dee7cec6611cf68fa483883fedccd32592e5b91418d5e8338880fc1", + "bytecodeHash": "0x0100055d74f7387e03ecbb5209bea7e0318aea05cfaaa1c195a85df100115cea", "sourceCodeHash": "0xebffe840ebbd9329edb1ebff8ca50f6935e7dabcc67194a896fcc2e968d46dfb" }, { @@ -59,71 +59,71 @@ "contractName": "ImmutableSimulator", "bytecodePath": "artifacts-zk/contracts-preprocessed/ImmutableSimulator.sol/ImmutableSimulator.json", "sourceCodePath": "contracts-preprocessed/ImmutableSimulator.sol", - "bytecodeHash": "0x01000039bbbb1e91691c8c36672cd0b57adb505bf485b1aeea7b1e1f41d592ef", + "bytecodeHash": "0x0100003946a9e538157e73717201b8cd17af70998602a3692b0ac1eff6ad850e", "sourceCodeHash": "0x9659e69f7db09e8f60a8bb95314b1ed26afcc689851665cf27f5408122f60c98" }, { "contractName": "KnownCodesStorage", "bytecodePath": "artifacts-zk/contracts-preprocessed/KnownCodesStorage.sol/KnownCodesStorage.json", "sourceCodePath": "contracts-preprocessed/KnownCodesStorage.sol", - "bytecodeHash": "0x0100006f68de5d0154a31ff1e889f0623c2b9bfaed2109547c0bbc93df82d6c3", + "bytecodeHash": "0x0100006f1ab2c7415de3914a2b9c53942cd3ff6471f698e7383b59f51e33e4d3", "sourceCodeHash": "0xb39b5b81168653e0c5062f7b8e1d6d15a4e186df3317f192f0cb2fc3a74f5448" }, { "contractName": "L1Messenger", "bytecodePath": "artifacts-zk/contracts-preprocessed/L1Messenger.sol/L1Messenger.json", "sourceCodePath": "contracts-preprocessed/L1Messenger.sol", - "bytecodeHash": "0x010001f7efd57d106ffdacde139e11ae13590509ee55d8ba15573e4410ac092a", - "sourceCodeHash": "0x8d22a4019347a45cb0c27bed9e98f7033637a7bdcd90fafb1922caa48f2b05de" + "bytecodeHash": "0x010001f74f7e45f40e1acbae30507ef94ea2775026a6ba0d0eb38cce10e4a472", + "sourceCodeHash": "0xe97846e4ff5f1cfffd6a454f5ad278deecf6fd7a67525908dea9af877dc822a9" }, { "contractName": "L2BaseToken", "bytecodePath": "artifacts-zk/contracts-preprocessed/L2BaseToken.sol/L2BaseToken.json", "sourceCodePath": "contracts-preprocessed/L2BaseToken.sol", - "bytecodeHash": "0x010001033bf67c0464dce4ed878664840d0b37d25756f6c6c2fb439a253b3017", + "bytecodeHash": "0x01000103bbfa393b49b9f8a7adcfedf1273b7928750f3ea8798347dfd8ca0d6f", "sourceCodeHash": "0x8bdd2b4d0b53dba84c9f0af250bbaa2aad10b3de6747bba957f0bd3721090dfa" }, { "contractName": "L2GatewayUpgrade", "bytecodePath": "artifacts-zk/contracts-preprocessed/L2GatewayUpgrade.sol/L2GatewayUpgrade.json", "sourceCodePath": "contracts-preprocessed/L2GatewayUpgrade.sol", - "bytecodeHash": "0x0100038b37fe650a1b83b23cdcf35cb2701bb5f952f0a63d8f080718788ed4a4", + "bytecodeHash": "0x0100038b3b4065d2682996020e14177a9b4632e054b6718f68d46ff13c012b20", "sourceCodeHash": "0x9248f46f491b8853da77e8f9787cfc1a136abee90fde18a3b8f47dcb8859c63c" }, { "contractName": "L2GatewayUpgradeHelper", "bytecodePath": "artifacts-zk/contracts-preprocessed/L2GatewayUpgradeHelper.sol/L2GatewayUpgradeHelper.json", "sourceCodePath": "contracts-preprocessed/L2GatewayUpgradeHelper.sol", - "bytecodeHash": "0x01000007535b44016390e041915eadc958c713a1a8f5bc27e35ad444ef546fad", + "bytecodeHash": "0x010000071330ec1656098ed33e28b475e101394550c02907d7ee2abbae9b762e", "sourceCodeHash": "0xd1c42c4d338697b8effbfe22a0f07d8d9c5a06c8ec8f45deae77765af48a355b" }, { "contractName": "L2GenesisUpgrade", "bytecodePath": "artifacts-zk/contracts-preprocessed/L2GenesisUpgrade.sol/L2GenesisUpgrade.json", "sourceCodePath": "contracts-preprocessed/L2GenesisUpgrade.sol", - "bytecodeHash": "0x010001b3d649bf9be5b9ed5cc8bc7ae0c7b9664ce63c33e74d7c0674a369521b", - "sourceCodeHash": "0xeb8583c1b31bd66d71f253cab8f8c38d789b7a5986a0ae0a1807b69532678343" + "bytecodeHash": "0x0100024b2e6a7646d229d8900c24945448a4b7c33a9203d459bd905ce47e38bf", + "sourceCodeHash": "0x5e608318c256fbde1db1a3cc72ca230cb42423a44410c72bf0889e5fc72c9c0c" }, { "contractName": "MsgValueSimulator", "bytecodePath": "artifacts-zk/contracts-preprocessed/MsgValueSimulator.sol/MsgValueSimulator.json", "sourceCodePath": "contracts-preprocessed/MsgValueSimulator.sol", - "bytecodeHash": "0x0100005d23f184324cb35d3e9c76e70a898a94e347eea5ddfeea2055b372ec8a", + "bytecodeHash": "0x0100005df63cf8940e407a67346b406dcddf4788cba9792ecd6a0edb8d8b3bd8", "sourceCodeHash": "0x082f3dcbc2fe4d93706c86aae85faa683387097d1b676e7ebd00f71ee0f13b71" }, { "contractName": "NonceHolder", "bytecodePath": "artifacts-zk/contracts-preprocessed/NonceHolder.sol/NonceHolder.json", "sourceCodePath": "contracts-preprocessed/NonceHolder.sol", - "bytecodeHash": "0x010000d9fbee5cbf613421094d193a1a012eb071565a311c548080b6db5f8157", + "bytecodeHash": "0x010000d9e79c30aeda9b823f1a0161c7637ed50848e6287e2a34e37cf2e7e4e8", "sourceCodeHash": "0xcd0c0366effebf2c98c58cf96322cc242a2d1c675620ef5514b7ed1f0a869edc" }, { "contractName": "PubdataChunkPublisher", "bytecodePath": "artifacts-zk/contracts-preprocessed/PubdataChunkPublisher.sol/PubdataChunkPublisher.json", "sourceCodePath": "contracts-preprocessed/PubdataChunkPublisher.sol", - "bytecodeHash": "0x01000049fe72fd8726473cc9fd892b9a6aa02d12f3db1bc20452c5fd0b1c4cc8", - "sourceCodeHash": "0x04d3d2e4019081c87aae5c22a060d84ae2e9d631ebce59801ecce37b9c87e4c7" + "bytecodeHash": "0x01000049377ba719b2d7493420854f12ebe67b75e21338777fb22b73e58ec057", + "sourceCodeHash": "0x398b1b9325b39d4c31e672866d4cbdf1cab453fae8d29f438262d921d427f094" }, { "contractName": "SloadContract", @@ -136,7 +136,7 @@ "contractName": "SystemContext", "bytecodePath": "artifacts-zk/contracts-preprocessed/SystemContext.sol/SystemContext.json", "sourceCodePath": "contracts-preprocessed/SystemContext.sol", - "bytecodeHash": "0x0100017f3a6ec3b05bcfa216590e25bdfde2ac07c22c6ec2c7fb82ac54187a45", + "bytecodeHash": "0x0100017f235b172e9a808764229a777b027e179eacc88a7ea48ef81cb193630a", "sourceCodeHash": "0x22406893d61abd477ce071dce506cf2534cca7b7717d015769fc8af1f1b80e06" }, { From 9824a3dd129b5311d47030898ce42e718adf3583 Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 16:14:34 +0400 Subject: [PATCH 22/29] (fix): tests --- .../contracts/common/L1ContractErrors.sol | 6 ++++ .../governance/PermanentRestriction.sol | 36 ++++++++++--------- .../Governance/PermanentRestriction.t.sol | 12 ++++--- .../contracts/L2GenesisUpgrade.sol | 28 --------------- 4 files changed, 34 insertions(+), 48 deletions(-) diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 5aa1c0a50..7cfbfdfe3 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -67,6 +67,8 @@ error ChainIdMismatch(); error ChainIdNotRegistered(uint256 chainId); // 0x8f620a06 error ChainIdTooBig(); +// 0x59e1b0d2 +error ChainZeroAddress(); // 0xf7a01e4d error DelegateCallFailed(bytes returnData); // 0x0a8ed92c @@ -314,6 +316,10 @@ error IncorrectBatchBounds( ); // 0x64107968 error AssetHandlerNotRegistered(bytes32 assetId); +// 0xff4bbdf1 +error NotAHyperchain(address chainAddress); +// 0xa3decdf3 +error NotAnAdmin(address expected, address actual); // 0x64846fe4 error NotARestriction(address addr); // 0xfa5cd00f diff --git a/l1-contracts/contracts/governance/PermanentRestriction.sol b/l1-contracts/contracts/governance/PermanentRestriction.sol index 627870069..6be880229 100644 --- a/l1-contracts/contracts/governance/PermanentRestriction.sol +++ b/l1-contracts/contracts/governance/PermanentRestriction.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.24; -import {CallNotAllowed, RemovingPermanentRestriction, ZeroAddress, UnallowedImplementation, AlreadyWhitelisted, NotAllowed} from "../common/L1ContractErrors.sol"; +import {CallNotAllowed, ChainZeroAddress, RemovingPermanentRestriction, ZeroAddress, UnallowedImplementation, AlreadyWhitelisted, NotAHyperchain, NotAnAdmin, NotAllowed} from "../common/L1ContractErrors.sol"; import {L2TransactionRequestTwoBridgesOuter, BridgehubBurnCTMAssetData} from "../bridgehub/IBridgehub.sol"; import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable-v4/access/Ownable2StepUpgradeable.sol"; @@ -209,8 +209,18 @@ contract PermanentRestriction is Restriction, IPermanentRestriction, Ownable2Ste /// @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)) { - return false; + revert ChainZeroAddress(); } // Unfortunately there is no easy way to double check that indeed the `_chain` is a ZkSyncHyperchain. @@ -223,29 +233,23 @@ contract PermanentRestriction is Restriction, IPermanentRestriction, Ownable2Ste // incorrect `_chain` address. (bool success, bytes memory data) = _chain.staticcall(abi.encodeWithSelector(IGetters.getChainId.selector)); if (!success || data.length < 32) { - revert(); + revert NotAHyperchain(_chain); } - // 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 - (uint256 chainId, bool chainIdQuerySuccess) = _getChainIdUnffallibleCall(_chain); - - if (!chainIdQuerySuccess) { - // It is not a hyperchain, so we can return `false` here. - return false; - } + // Can not fail + uint256 chainId = abi.decode(data, (uint256)); // Note, that here it is important to use the legacy `getHyperchain` function, so that the contract // is compatible with the legacy ones. if (BRIDGE_HUB.getHyperchain(chainId) != _chain) { - // It is not a hyperchain, so we can return `false` here. - return false; + revert NotAHyperchain(_chain); } - // Now, the chain is known to be a hyperchain, so it must implement the corresponding interface + // Now, the chain is known to be a hyperchain, so it should implement the corresponding interface address admin = IZKChain(_chain).getAdmin(); - - return admin == msg.sender; + if (admin != _potentialAdmin) { + revert NotAnAdmin(admin, _potentialAdmin); + } } /// @notice Tries to call `IGetters.getChainId()` function on the `_potentialChainAddress`. diff --git a/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol index 525f89f60..c76cb0e84 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol @@ -9,7 +9,7 @@ import {ChainTypeManager} from "contracts/state-transition/ChainTypeManager.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 {NotAllowed, NotEnoughGas, UnsupportedEncodingVersion, InvalidSelector, ZeroAddress, UnallowedImplementation, RemovingPermanentRestriction, CallNotAllowed} from "contracts/common/L1ContractErrors.sol"; +import {ChainZeroAddress, NotAllowed, NotAHyperchain, NotEnoughGas, NotAnAdmin, UnsupportedEncodingVersion, InvalidSelector, ZeroAddress, UnallowedImplementation, RemovingPermanentRestriction, CallNotAllowed} from "contracts/common/L1ContractErrors.sol"; import {Call} from "contracts/governance/Common.sol"; import {IZKChain} from "contracts/state-transition/chain-interfaces/IZKChain.sol"; import {VerifierParams, FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-deps/ZKChainStorage.sol"; @@ -122,15 +122,19 @@ contract PermanentRestrictionTest is ChainTypeManagerTest { } function test_tryCompareAdminOfAChainIsAddressZero() public { - assertFalse(isAddressAdmin(address(0), owner)); + vm.expectRevert(abi.encodeWithSelector(ChainZeroAddress.selector)); + permRestriction.tryCompareAdminOfAChain(address(0), owner); } function test_tryCompareAdminOfAChainNotAHyperchain() public { - assertFalse(isAddressAdmin(makeAddr("random"), owner)); + address chain = makeAddr("random"); + vm.expectRevert(abi.encodeWithSelector(NotAHyperchain.selector, chain)); + permRestriction.tryCompareAdminOfAChain(chain, owner); } function test_tryCompareAdminOfAChainNotAnAdmin() public { - assertFalse(isAddressAdmin(hyperchain, owner)); + vm.expectRevert(abi.encodeWithSelector(NotAnAdmin.selector, IZKChain(hyperchain).getAdmin(), owner)); + permRestriction.tryCompareAdminOfAChain(hyperchain, owner); } function test_tryCompareAdminOfAChain() public { diff --git a/system-contracts/contracts/L2GenesisUpgrade.sol b/system-contracts/contracts/L2GenesisUpgrade.sol index e8bceb48a..abc887ba3 100644 --- a/system-contracts/contracts/L2GenesisUpgrade.sol +++ b/system-contracts/contracts/L2GenesisUpgrade.sol @@ -27,38 +27,10 @@ contract L2GenesisUpgrade is IL2GenesisUpgrade { bytes calldata _fixedForceDeploymentsData, bytes calldata _additionalForceDeploymentsData ) external payable { - // solhint-disable-next-line gas-custom-errors if (_chainId == 0) { revert InvalidChainId(); } ISystemContext(SYSTEM_CONTEXT_CONTRACT).setChainId(_chainId); - ForceDeployment[] memory forceDeployments = abi.decode(_fixedForceDeploymentsData, (ForceDeployment[])); - IContractDeployer(DEPLOYER_SYSTEM_CONTRACT).forceDeployOnAddresses{value: msg.value}(forceDeployments); - - // It is expected that either via to the force deployments above - // or upon init both the L2 deployment of Bridgehub, AssetRouter and MessageRoot are deployed. - // (The comment does not mention the exact order in case it changes) - // However, there is still some follow up finalization that needs to be done. - - address bridgehubOwner = L2_BRIDGE_HUB.owner(); - - bytes memory data = abi.encodeCall( - L2_BRIDGE_HUB.setAddresses, - (L2_ASSET_ROUTER, _ctmDeployer, address(L2_MESSAGE_ROOT)) - ); - - (bool success, bytes memory returnData) = SystemContractHelper.mimicCall( - address(L2_BRIDGE_HUB), - bridgehubOwner, - data - ); - if (!success) { - // Propagate revert reason - assembly { - revert(add(returnData, 0x20), returndatasize()) - } - } - ISystemContext(SYSTEM_CONTEXT_CONTRACT).setChainId(_chainId); L2GatewayUpgradeHelper.performForceDeployedContractsInit( _ctmDeployer, From 269ef381f7f0d34be0192067eb99ff123c9b0350 Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 16:42:47 +0400 Subject: [PATCH 23/29] (fix): system contracts --- system-contracts/SystemContractsHashes.json | 38 ++++++++++----------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/system-contracts/SystemContractsHashes.json b/system-contracts/SystemContractsHashes.json index 599fd11b4..53599d047 100644 --- a/system-contracts/SystemContractsHashes.json +++ b/system-contracts/SystemContractsHashes.json @@ -3,49 +3,49 @@ "contractName": "AccountCodeStorage", "bytecodePath": "artifacts-zk/contracts-preprocessed/AccountCodeStorage.sol/AccountCodeStorage.json", "sourceCodePath": "contracts-preprocessed/AccountCodeStorage.sol", - "bytecodeHash": "0x0100005df535e7d1e6f3933b26076778d9c44fd6e7faf546732f08290d8c8f94", + "bytecodeHash": "0x0100005d697cd710ca06e038048178b67e2c75fbd4bb2799359baed6e51d2c57", "sourceCodeHash": "0x2e0e09d57a04bd1e722d8bf8c6423fdf3f8bca44e5e8c4f6684f987794be066e" }, { "contractName": "BootloaderUtilities", "bytecodePath": "artifacts-zk/contracts-preprocessed/BootloaderUtilities.sol/BootloaderUtilities.json", "sourceCodePath": "contracts-preprocessed/BootloaderUtilities.sol", - "bytecodeHash": "0x010007c72b7f29a0e1954ee4c65e6598d0934d33c692faedd7ac4fe30b508fa3", + "bytecodeHash": "0x010007c7a0cad933f039823230d66da3d922ee9b4575badc9e093e148e5ebf7c", "sourceCodeHash": "0x0f1213c4b95acb71f4ab5d4082cc1aeb2bd5017e1cccd46afc66e53268609d85" }, { "contractName": "ComplexUpgrader", "bytecodePath": "artifacts-zk/contracts-preprocessed/ComplexUpgrader.sol/ComplexUpgrader.json", "sourceCodePath": "contracts-preprocessed/ComplexUpgrader.sol", - "bytecodeHash": "0x010000bf6fcec0995b82b1c51133a507c8f63111234530b69fe7dadaae0c8172", + "bytecodeHash": "0x010000bffc60af04f8d65efce2cfdd0d3879cea1b1683d23dcf59ee5baf3049b", "sourceCodeHash": "0xfcc74aefbc7cbde7945c29bad0e47527ac443bd6b75251a4ae520e28c714af37" }, { "contractName": "Compressor", "bytecodePath": "artifacts-zk/contracts-preprocessed/Compressor.sol/Compressor.json", "sourceCodePath": "contracts-preprocessed/Compressor.sol", - "bytecodeHash": "0x0100014b2cac967629cb05fb59a5c77cb5a077b74c50521ed9216a59511bf182", + "bytecodeHash": "0x0100014b7686d4939ec1f60de9b7f8330468d5482139d492c8710857f010e2ff", "sourceCodeHash": "0x7240b5fb2ea8e184522e731fb14f764ebae52b8a69d1870a55daedac9a3ed617" }, { "contractName": "ContractDeployer", "bytecodePath": "artifacts-zk/contracts-preprocessed/ContractDeployer.sol/ContractDeployer.json", "sourceCodePath": "contracts-preprocessed/ContractDeployer.sol", - "bytecodeHash": "0x010004e5a266e697bb45bc90ff310dcb293725006146ff83e46bde8f3c6b44fa", + "bytecodeHash": "0x010004e72ec4babd55a31ffd2a413c58f72c13f801996511757cdbde511b8523", "sourceCodeHash": "0x92bc09da23ed9d86ba7a84f0dbf48503c99582ae58cdbebbdcc5f14ea1fcf014" }, { "contractName": "Create2Factory", "bytecodePath": "artifacts-zk/contracts-preprocessed/Create2Factory.sol/Create2Factory.json", "sourceCodePath": "contracts-preprocessed/Create2Factory.sol", - "bytecodeHash": "0x010000493a391e65a70dea42442132cf7c7001dac94388b9c4218ce9b1491b57", + "bytecodeHash": "0x010000494e9df2482601b694e37ba166e23a049c444faa6fc7dba4f7ac1f0bb9", "sourceCodeHash": "0x97392413259e6aae5187768cefd734507460ae818d6975709cc9b4e15a9af906" }, { "contractName": "DefaultAccount", "bytecodePath": "artifacts-zk/contracts-preprocessed/DefaultAccount.sol/DefaultAccount.json", "sourceCodePath": "contracts-preprocessed/DefaultAccount.sol", - "bytecodeHash": "0x0100055d74f7387e03ecbb5209bea7e0318aea05cfaaa1c195a85df100115cea", + "bytecodeHash": "0x0100055d838340f08184a24f6abeba7d279b0fab0922e838bc54695b9ac349f2", "sourceCodeHash": "0xebffe840ebbd9329edb1ebff8ca50f6935e7dabcc67194a896fcc2e968d46dfb" }, { @@ -59,70 +59,70 @@ "contractName": "ImmutableSimulator", "bytecodePath": "artifacts-zk/contracts-preprocessed/ImmutableSimulator.sol/ImmutableSimulator.json", "sourceCodePath": "contracts-preprocessed/ImmutableSimulator.sol", - "bytecodeHash": "0x0100003946a9e538157e73717201b8cd17af70998602a3692b0ac1eff6ad850e", + "bytecodeHash": "0x010000395a88359d8e3241d38a9b996d62a12807a767b264e55926b39b5bfbc5", "sourceCodeHash": "0x9659e69f7db09e8f60a8bb95314b1ed26afcc689851665cf27f5408122f60c98" }, { "contractName": "KnownCodesStorage", "bytecodePath": "artifacts-zk/contracts-preprocessed/KnownCodesStorage.sol/KnownCodesStorage.json", "sourceCodePath": "contracts-preprocessed/KnownCodesStorage.sol", - "bytecodeHash": "0x0100006f1ab2c7415de3914a2b9c53942cd3ff6471f698e7383b59f51e33e4d3", + "bytecodeHash": "0x0100006f5f8bcafa51c48c2686fcaf770c5cd0faf33ee9b67e356480ce861aca", "sourceCodeHash": "0xb39b5b81168653e0c5062f7b8e1d6d15a4e186df3317f192f0cb2fc3a74f5448" }, { "contractName": "L1Messenger", "bytecodePath": "artifacts-zk/contracts-preprocessed/L1Messenger.sol/L1Messenger.json", "sourceCodePath": "contracts-preprocessed/L1Messenger.sol", - "bytecodeHash": "0x010001f74f7e45f40e1acbae30507ef94ea2775026a6ba0d0eb38cce10e4a472", + "bytecodeHash": "0x010001f759bd56145860e9302ca8712b50293ed35ce5a4ed62b69c099f30b864", "sourceCodeHash": "0xe97846e4ff5f1cfffd6a454f5ad278deecf6fd7a67525908dea9af877dc822a9" }, { "contractName": "L2BaseToken", "bytecodePath": "artifacts-zk/contracts-preprocessed/L2BaseToken.sol/L2BaseToken.json", "sourceCodePath": "contracts-preprocessed/L2BaseToken.sol", - "bytecodeHash": "0x01000103bbfa393b49b9f8a7adcfedf1273b7928750f3ea8798347dfd8ca0d6f", + "bytecodeHash": "0x01000103365edfcc3816f124b7dff844801827279c7188791bbc6f93efe3ed36", "sourceCodeHash": "0x8bdd2b4d0b53dba84c9f0af250bbaa2aad10b3de6747bba957f0bd3721090dfa" }, { "contractName": "L2GatewayUpgrade", "bytecodePath": "artifacts-zk/contracts-preprocessed/L2GatewayUpgrade.sol/L2GatewayUpgrade.json", "sourceCodePath": "contracts-preprocessed/L2GatewayUpgrade.sol", - "bytecodeHash": "0x0100038b3b4065d2682996020e14177a9b4632e054b6718f68d46ff13c012b20", + "bytecodeHash": "0x0100038bcf51634e23f6854d9d01ab20edb0c36fec82360e2fe71e63a93a13f9", "sourceCodeHash": "0x9248f46f491b8853da77e8f9787cfc1a136abee90fde18a3b8f47dcb8859c63c" }, { "contractName": "L2GatewayUpgradeHelper", "bytecodePath": "artifacts-zk/contracts-preprocessed/L2GatewayUpgradeHelper.sol/L2GatewayUpgradeHelper.json", "sourceCodePath": "contracts-preprocessed/L2GatewayUpgradeHelper.sol", - "bytecodeHash": "0x010000071330ec1656098ed33e28b475e101394550c02907d7ee2abbae9b762e", + "bytecodeHash": "0x010000074346067c9488fcdf8f9cadccf0014566760bca73f87d545b7dec87ed", "sourceCodeHash": "0xd1c42c4d338697b8effbfe22a0f07d8d9c5a06c8ec8f45deae77765af48a355b" }, { "contractName": "L2GenesisUpgrade", "bytecodePath": "artifacts-zk/contracts-preprocessed/L2GenesisUpgrade.sol/L2GenesisUpgrade.json", "sourceCodePath": "contracts-preprocessed/L2GenesisUpgrade.sol", - "bytecodeHash": "0x0100024b2e6a7646d229d8900c24945448a4b7c33a9203d459bd905ce47e38bf", - "sourceCodeHash": "0x5e608318c256fbde1db1a3cc72ca230cb42423a44410c72bf0889e5fc72c9c0c" + "bytecodeHash": "0x0100024b9f1de8f353981b79ed13f48caffa7a0cd77f4502c59cffd8c48e330a", + "sourceCodeHash": "0x43ed45b1ad53d98164c9b791d19b07d1ff064385d700695be59e17c6ec28da34" }, { "contractName": "MsgValueSimulator", "bytecodePath": "artifacts-zk/contracts-preprocessed/MsgValueSimulator.sol/MsgValueSimulator.json", "sourceCodePath": "contracts-preprocessed/MsgValueSimulator.sol", - "bytecodeHash": "0x0100005df63cf8940e407a67346b406dcddf4788cba9792ecd6a0edb8d8b3bd8", + "bytecodeHash": "0x0100005d0cbec2ea3498470c99758a6290364cb3c16cb3ee0b029b82146387f8", "sourceCodeHash": "0x082f3dcbc2fe4d93706c86aae85faa683387097d1b676e7ebd00f71ee0f13b71" }, { "contractName": "NonceHolder", "bytecodePath": "artifacts-zk/contracts-preprocessed/NonceHolder.sol/NonceHolder.json", "sourceCodePath": "contracts-preprocessed/NonceHolder.sol", - "bytecodeHash": "0x010000d9e79c30aeda9b823f1a0161c7637ed50848e6287e2a34e37cf2e7e4e8", + "bytecodeHash": "0x010000d9b6dec0f36e3896883c8d4100f585d79b7b4df02ccd699618ac6bdafe", "sourceCodeHash": "0xcd0c0366effebf2c98c58cf96322cc242a2d1c675620ef5514b7ed1f0a869edc" }, { "contractName": "PubdataChunkPublisher", "bytecodePath": "artifacts-zk/contracts-preprocessed/PubdataChunkPublisher.sol/PubdataChunkPublisher.json", "sourceCodePath": "contracts-preprocessed/PubdataChunkPublisher.sol", - "bytecodeHash": "0x01000049377ba719b2d7493420854f12ebe67b75e21338777fb22b73e58ec057", + "bytecodeHash": "0x01000049615ece265d67427e684a665de82466e4065085201b7670f19c67df7c", "sourceCodeHash": "0x398b1b9325b39d4c31e672866d4cbdf1cab453fae8d29f438262d921d427f094" }, { @@ -136,7 +136,7 @@ "contractName": "SystemContext", "bytecodePath": "artifacts-zk/contracts-preprocessed/SystemContext.sol/SystemContext.json", "sourceCodePath": "contracts-preprocessed/SystemContext.sol", - "bytecodeHash": "0x0100017f235b172e9a808764229a777b027e179eacc88a7ea48ef81cb193630a", + "bytecodeHash": "0x0100017fb9147ea6f138ef0acbd9cfbc713eeaa7e355c6c4b3404cfb77e7488c", "sourceCodeHash": "0x22406893d61abd477ce071dce506cf2534cca7b7717d015769fc8af1f1b80e06" }, { From 416c2aa11da4ad8cb32ec3166231f24fa4b9962e Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 16:52:10 +0400 Subject: [PATCH 24/29] (fix): imports --- system-contracts/contracts/L2GenesisUpgrade.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/system-contracts/contracts/L2GenesisUpgrade.sol b/system-contracts/contracts/L2GenesisUpgrade.sol index abc887ba3..00a22c799 100644 --- a/system-contracts/contracts/L2GenesisUpgrade.sol +++ b/system-contracts/contracts/L2GenesisUpgrade.sol @@ -2,9 +2,7 @@ pragma solidity 0.8.24; -import {DEPLOYER_SYSTEM_CONTRACT, SYSTEM_CONTEXT_CONTRACT, L2_BRIDGE_HUB, L2_ASSET_ROUTER, L2_MESSAGE_ROOT} from "./Constants.sol"; -import {IContractDeployer, ForceDeployment} from "./interfaces/IContractDeployer.sol"; -import {SystemContractHelper} from "./libraries/SystemContractHelper.sol"; +import {SYSTEM_CONTEXT_CONTRACT} from "./Constants.sol"; import {ISystemContext} from "./interfaces/ISystemContext.sol"; import {InvalidChainId} from "contracts/SystemContractErrors.sol"; import {IL2GenesisUpgrade} from "./interfaces/IL2GenesisUpgrade.sol"; From 71ec9c8c547a2738e3ce29b80d465532457eebf0 Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 17:05:18 +0400 Subject: [PATCH 25/29] (fix): slight refactoring --- .../contracts/common/L1ContractErrors.sol | 4 --- .../governance/PermanentRestriction.sol | 35 +++++++++---------- .../Governance/PermanentRestriction.t.sol | 16 ++++----- 3 files changed, 23 insertions(+), 32 deletions(-) diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 7cfbfdfe3..7697d30b9 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -67,8 +67,6 @@ error ChainIdMismatch(); error ChainIdNotRegistered(uint256 chainId); // 0x8f620a06 error ChainIdTooBig(); -// 0x59e1b0d2 -error ChainZeroAddress(); // 0xf7a01e4d error DelegateCallFailed(bytes returnData); // 0x0a8ed92c @@ -318,8 +316,6 @@ error IncorrectBatchBounds( error AssetHandlerNotRegistered(bytes32 assetId); // 0xff4bbdf1 error NotAHyperchain(address chainAddress); -// 0xa3decdf3 -error NotAnAdmin(address expected, address actual); // 0x64846fe4 error NotARestriction(address addr); // 0xfa5cd00f diff --git a/l1-contracts/contracts/governance/PermanentRestriction.sol b/l1-contracts/contracts/governance/PermanentRestriction.sol index 6be880229..bd4c78261 100644 --- a/l1-contracts/contracts/governance/PermanentRestriction.sol +++ b/l1-contracts/contracts/governance/PermanentRestriction.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.24; -import {CallNotAllowed, ChainZeroAddress, RemovingPermanentRestriction, ZeroAddress, UnallowedImplementation, AlreadyWhitelisted, NotAHyperchain, NotAnAdmin, NotAllowed} from "../common/L1ContractErrors.sol"; +import {CallNotAllowed, RemovingPermanentRestriction, ZeroAddress, UnallowedImplementation, AlreadyWhitelisted, NotAHyperchain, NotAllowed} from "../common/L1ContractErrors.sol"; import {L2TransactionRequestTwoBridgesOuter, BridgehubBurnCTMAssetData} from "../bridgehub/IBridgehub.sol"; import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable-v4/access/Ownable2StepUpgradeable.sol"; @@ -209,18 +209,8 @@ contract PermanentRestriction is Restriction, IPermanentRestriction, Ownable2Ste /// @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(); + return false; } // Unfortunately there is no easy way to double check that indeed the `_chain` is a ZkSyncHyperchain. @@ -236,20 +226,27 @@ contract PermanentRestriction is Restriction, IPermanentRestriction, Ownable2Ste revert NotAHyperchain(_chain); } - // Can not fail - uint256 chainId = abi.decode(data, (uint256)); + // 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 + + (uint256 chainId, bool chainIdQuerySuccess) = _getChainIdUnffallibleCall(_chain); + + 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. if (BRIDGE_HUB.getHyperchain(chainId) != _chain) { - revert NotAHyperchain(_chain); + // It is not a hyperchain, so we can return `false` here. + return false; } - // Now, the chain is known to be a hyperchain, so it should implement the corresponding interface + // Now, the chain is known to be a hyperchain, so it must implement the corresponding interface address admin = IZKChain(_chain).getAdmin(); - if (admin != _potentialAdmin) { - revert NotAnAdmin(admin, _potentialAdmin); - } + + return admin == msg.sender; } /// @notice Tries to call `IGetters.getChainId()` function on the `_potentialChainAddress`. diff --git a/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol index c76cb0e84..4682802ad 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol @@ -9,7 +9,7 @@ import {ChainTypeManager} from "contracts/state-transition/ChainTypeManager.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 {ChainZeroAddress, NotAllowed, NotAHyperchain, NotEnoughGas, NotAnAdmin, UnsupportedEncodingVersion, InvalidSelector, ZeroAddress, UnallowedImplementation, RemovingPermanentRestriction, CallNotAllowed} from "contracts/common/L1ContractErrors.sol"; +import {NotAllowed, NotAHyperchain, NotEnoughGas, UnsupportedEncodingVersion, InvalidSelector, ZeroAddress, UnallowedImplementation, RemovingPermanentRestriction, CallNotAllowed} from "contracts/common/L1ContractErrors.sol"; import {Call} from "contracts/governance/Common.sol"; import {IZKChain} from "contracts/state-transition/chain-interfaces/IZKChain.sol"; import {VerifierParams, FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-deps/ZKChainStorage.sol"; @@ -121,20 +121,18 @@ contract PermanentRestrictionTest is ChainTypeManagerTest { return permRestriction.isAdminOfAChain(chainAddr); } - function test_tryCompareAdminOfAChainIsAddressZero() public { - vm.expectRevert(abi.encodeWithSelector(ChainZeroAddress.selector)); - permRestriction.tryCompareAdminOfAChain(address(0), owner); + function test_isAdminOfAChainIsAddressZero() public { + assertFalse(permRestriction.isAdminOfAChain(address(0))); } - function test_tryCompareAdminOfAChainNotAHyperchain() public { + function test_isAdminOfAChainNotAHyperchain() public { address chain = makeAddr("random"); vm.expectRevert(abi.encodeWithSelector(NotAHyperchain.selector, chain)); - permRestriction.tryCompareAdminOfAChain(chain, owner); + permRestriction.isAdminOfAChain(chain); } - function test_tryCompareAdminOfAChainNotAnAdmin() public { - vm.expectRevert(abi.encodeWithSelector(NotAnAdmin.selector, IZKChain(hyperchain).getAdmin(), owner)); - permRestriction.tryCompareAdminOfAChain(hyperchain, owner); + function test_isAdminOfAChainOfAChainNotAnAdmin() public { + assertFalse(permRestriction.isAdminOfAChain(hyperchain)); } function test_tryCompareAdminOfAChain() public { From 3fef0bb44f670cd7e4edf643e8a6ae8b71b90843 Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 17:19:52 +0400 Subject: [PATCH 26/29] (fix): foundry tests and system-contract hashes --- .../Governance/PermanentRestriction.t.sol | 3 ++ system-contracts/SystemContractsHashes.json | 38 +++++++++---------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol index 4682802ad..6dbd65ae1 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol @@ -12,6 +12,7 @@ import {IPermanentRestriction} from "contracts/governance/IPermanentRestriction. import {NotAllowed, NotAHyperchain, NotEnoughGas, UnsupportedEncodingVersion, InvalidSelector, ZeroAddress, UnallowedImplementation, RemovingPermanentRestriction, CallNotAllowed} from "contracts/common/L1ContractErrors.sol"; import {Call} from "contracts/governance/Common.sol"; import {IZKChain} from "contracts/state-transition/chain-interfaces/IZKChain.sol"; +import {IGetters} from "contracts/state-transition/chain-interfaces/IGetters.sol"; import {VerifierParams, FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-deps/ZKChainStorage.sol"; import {IAdmin} from "contracts/state-transition/chain-interfaces/IAdmin.sol"; import {AccessControlRestriction} from "contracts/governance/AccessControlRestriction.sol"; @@ -341,6 +342,7 @@ contract PermanentRestrictionTest is ChainTypeManagerTest { } function test_validateMigrationToL2RevertNotAllowed() public { + vm.mockCall(address(bridgehub), abi.encodeWithSelector(IGetters.getChainId.selector), abi.encode(271)); Call memory call = _encodeMigraationCall(true, true, true, true, true, address(0)); vm.expectRevert(abi.encodeWithSelector(NotAllowed.selector, address(0))); @@ -348,6 +350,7 @@ contract PermanentRestrictionTest is ChainTypeManagerTest { } function test_validateMigrationToL2() public { + vm.mockCall(address(bridgehub), abi.encodeWithSelector(IGetters.getChainId.selector), abi.encode(271)); address expectedAddress = L2ContractHelper.computeCreate2Address( L2_FACTORY_ADDR, bytes32(0), diff --git a/system-contracts/SystemContractsHashes.json b/system-contracts/SystemContractsHashes.json index 53599d047..a6dec1ede 100644 --- a/system-contracts/SystemContractsHashes.json +++ b/system-contracts/SystemContractsHashes.json @@ -3,49 +3,49 @@ "contractName": "AccountCodeStorage", "bytecodePath": "artifacts-zk/contracts-preprocessed/AccountCodeStorage.sol/AccountCodeStorage.json", "sourceCodePath": "contracts-preprocessed/AccountCodeStorage.sol", - "bytecodeHash": "0x0100005d697cd710ca06e038048178b67e2c75fbd4bb2799359baed6e51d2c57", + "bytecodeHash": "0x0100005df535e7d1e6f3933b26076778d9c44fd6e7faf546732f08290d8c8f94", "sourceCodeHash": "0x2e0e09d57a04bd1e722d8bf8c6423fdf3f8bca44e5e8c4f6684f987794be066e" }, { "contractName": "BootloaderUtilities", "bytecodePath": "artifacts-zk/contracts-preprocessed/BootloaderUtilities.sol/BootloaderUtilities.json", "sourceCodePath": "contracts-preprocessed/BootloaderUtilities.sol", - "bytecodeHash": "0x010007c7a0cad933f039823230d66da3d922ee9b4575badc9e093e148e5ebf7c", + "bytecodeHash": "0x010007c72b7f29a0e1954ee4c65e6598d0934d33c692faedd7ac4fe30b508fa3", "sourceCodeHash": "0x0f1213c4b95acb71f4ab5d4082cc1aeb2bd5017e1cccd46afc66e53268609d85" }, { "contractName": "ComplexUpgrader", "bytecodePath": "artifacts-zk/contracts-preprocessed/ComplexUpgrader.sol/ComplexUpgrader.json", "sourceCodePath": "contracts-preprocessed/ComplexUpgrader.sol", - "bytecodeHash": "0x010000bffc60af04f8d65efce2cfdd0d3879cea1b1683d23dcf59ee5baf3049b", + "bytecodeHash": "0x010000bf6fcec0995b82b1c51133a507c8f63111234530b69fe7dadaae0c8172", "sourceCodeHash": "0xfcc74aefbc7cbde7945c29bad0e47527ac443bd6b75251a4ae520e28c714af37" }, { "contractName": "Compressor", "bytecodePath": "artifacts-zk/contracts-preprocessed/Compressor.sol/Compressor.json", "sourceCodePath": "contracts-preprocessed/Compressor.sol", - "bytecodeHash": "0x0100014b7686d4939ec1f60de9b7f8330468d5482139d492c8710857f010e2ff", + "bytecodeHash": "0x0100014b2cac967629cb05fb59a5c77cb5a077b74c50521ed9216a59511bf182", "sourceCodeHash": "0x7240b5fb2ea8e184522e731fb14f764ebae52b8a69d1870a55daedac9a3ed617" }, { "contractName": "ContractDeployer", "bytecodePath": "artifacts-zk/contracts-preprocessed/ContractDeployer.sol/ContractDeployer.json", "sourceCodePath": "contracts-preprocessed/ContractDeployer.sol", - "bytecodeHash": "0x010004e72ec4babd55a31ffd2a413c58f72c13f801996511757cdbde511b8523", + "bytecodeHash": "0x010004e5a266e697bb45bc90ff310dcb293725006146ff83e46bde8f3c6b44fa", "sourceCodeHash": "0x92bc09da23ed9d86ba7a84f0dbf48503c99582ae58cdbebbdcc5f14ea1fcf014" }, { "contractName": "Create2Factory", "bytecodePath": "artifacts-zk/contracts-preprocessed/Create2Factory.sol/Create2Factory.json", "sourceCodePath": "contracts-preprocessed/Create2Factory.sol", - "bytecodeHash": "0x010000494e9df2482601b694e37ba166e23a049c444faa6fc7dba4f7ac1f0bb9", + "bytecodeHash": "0x010000493a391e65a70dea42442132cf7c7001dac94388b9c4218ce9b1491b57", "sourceCodeHash": "0x97392413259e6aae5187768cefd734507460ae818d6975709cc9b4e15a9af906" }, { "contractName": "DefaultAccount", "bytecodePath": "artifacts-zk/contracts-preprocessed/DefaultAccount.sol/DefaultAccount.json", "sourceCodePath": "contracts-preprocessed/DefaultAccount.sol", - "bytecodeHash": "0x0100055d838340f08184a24f6abeba7d279b0fab0922e838bc54695b9ac349f2", + "bytecodeHash": "0x0100055d74f7387e03ecbb5209bea7e0318aea05cfaaa1c195a85df100115cea", "sourceCodeHash": "0xebffe840ebbd9329edb1ebff8ca50f6935e7dabcc67194a896fcc2e968d46dfb" }, { @@ -59,70 +59,70 @@ "contractName": "ImmutableSimulator", "bytecodePath": "artifacts-zk/contracts-preprocessed/ImmutableSimulator.sol/ImmutableSimulator.json", "sourceCodePath": "contracts-preprocessed/ImmutableSimulator.sol", - "bytecodeHash": "0x010000395a88359d8e3241d38a9b996d62a12807a767b264e55926b39b5bfbc5", + "bytecodeHash": "0x0100003946a9e538157e73717201b8cd17af70998602a3692b0ac1eff6ad850e", "sourceCodeHash": "0x9659e69f7db09e8f60a8bb95314b1ed26afcc689851665cf27f5408122f60c98" }, { "contractName": "KnownCodesStorage", "bytecodePath": "artifacts-zk/contracts-preprocessed/KnownCodesStorage.sol/KnownCodesStorage.json", "sourceCodePath": "contracts-preprocessed/KnownCodesStorage.sol", - "bytecodeHash": "0x0100006f5f8bcafa51c48c2686fcaf770c5cd0faf33ee9b67e356480ce861aca", + "bytecodeHash": "0x0100006f1ab2c7415de3914a2b9c53942cd3ff6471f698e7383b59f51e33e4d3", "sourceCodeHash": "0xb39b5b81168653e0c5062f7b8e1d6d15a4e186df3317f192f0cb2fc3a74f5448" }, { "contractName": "L1Messenger", "bytecodePath": "artifacts-zk/contracts-preprocessed/L1Messenger.sol/L1Messenger.json", "sourceCodePath": "contracts-preprocessed/L1Messenger.sol", - "bytecodeHash": "0x010001f759bd56145860e9302ca8712b50293ed35ce5a4ed62b69c099f30b864", + "bytecodeHash": "0x010001f74f7e45f40e1acbae30507ef94ea2775026a6ba0d0eb38cce10e4a472", "sourceCodeHash": "0xe97846e4ff5f1cfffd6a454f5ad278deecf6fd7a67525908dea9af877dc822a9" }, { "contractName": "L2BaseToken", "bytecodePath": "artifacts-zk/contracts-preprocessed/L2BaseToken.sol/L2BaseToken.json", "sourceCodePath": "contracts-preprocessed/L2BaseToken.sol", - "bytecodeHash": "0x01000103365edfcc3816f124b7dff844801827279c7188791bbc6f93efe3ed36", + "bytecodeHash": "0x01000103bbfa393b49b9f8a7adcfedf1273b7928750f3ea8798347dfd8ca0d6f", "sourceCodeHash": "0x8bdd2b4d0b53dba84c9f0af250bbaa2aad10b3de6747bba957f0bd3721090dfa" }, { "contractName": "L2GatewayUpgrade", "bytecodePath": "artifacts-zk/contracts-preprocessed/L2GatewayUpgrade.sol/L2GatewayUpgrade.json", "sourceCodePath": "contracts-preprocessed/L2GatewayUpgrade.sol", - "bytecodeHash": "0x0100038bcf51634e23f6854d9d01ab20edb0c36fec82360e2fe71e63a93a13f9", + "bytecodeHash": "0x0100038b3b4065d2682996020e14177a9b4632e054b6718f68d46ff13c012b20", "sourceCodeHash": "0x9248f46f491b8853da77e8f9787cfc1a136abee90fde18a3b8f47dcb8859c63c" }, { "contractName": "L2GatewayUpgradeHelper", "bytecodePath": "artifacts-zk/contracts-preprocessed/L2GatewayUpgradeHelper.sol/L2GatewayUpgradeHelper.json", "sourceCodePath": "contracts-preprocessed/L2GatewayUpgradeHelper.sol", - "bytecodeHash": "0x010000074346067c9488fcdf8f9cadccf0014566760bca73f87d545b7dec87ed", + "bytecodeHash": "0x010000071330ec1656098ed33e28b475e101394550c02907d7ee2abbae9b762e", "sourceCodeHash": "0xd1c42c4d338697b8effbfe22a0f07d8d9c5a06c8ec8f45deae77765af48a355b" }, { "contractName": "L2GenesisUpgrade", "bytecodePath": "artifacts-zk/contracts-preprocessed/L2GenesisUpgrade.sol/L2GenesisUpgrade.json", "sourceCodePath": "contracts-preprocessed/L2GenesisUpgrade.sol", - "bytecodeHash": "0x0100024b9f1de8f353981b79ed13f48caffa7a0cd77f4502c59cffd8c48e330a", - "sourceCodeHash": "0x43ed45b1ad53d98164c9b791d19b07d1ff064385d700695be59e17c6ec28da34" + "bytecodeHash": "0x010001b386e0ed48ce9fbaad09c7865a58c28c8350d9bc9446b3beaee4aee999", + "sourceCodeHash": "0x2aaddd8a8ef3f56b4f4e6ba52c0035572145b0ea562fbf218a2eb5fc462f988d" }, { "contractName": "MsgValueSimulator", "bytecodePath": "artifacts-zk/contracts-preprocessed/MsgValueSimulator.sol/MsgValueSimulator.json", "sourceCodePath": "contracts-preprocessed/MsgValueSimulator.sol", - "bytecodeHash": "0x0100005d0cbec2ea3498470c99758a6290364cb3c16cb3ee0b029b82146387f8", + "bytecodeHash": "0x0100005df63cf8940e407a67346b406dcddf4788cba9792ecd6a0edb8d8b3bd8", "sourceCodeHash": "0x082f3dcbc2fe4d93706c86aae85faa683387097d1b676e7ebd00f71ee0f13b71" }, { "contractName": "NonceHolder", "bytecodePath": "artifacts-zk/contracts-preprocessed/NonceHolder.sol/NonceHolder.json", "sourceCodePath": "contracts-preprocessed/NonceHolder.sol", - "bytecodeHash": "0x010000d9b6dec0f36e3896883c8d4100f585d79b7b4df02ccd699618ac6bdafe", + "bytecodeHash": "0x010000d9e79c30aeda9b823f1a0161c7637ed50848e6287e2a34e37cf2e7e4e8", "sourceCodeHash": "0xcd0c0366effebf2c98c58cf96322cc242a2d1c675620ef5514b7ed1f0a869edc" }, { "contractName": "PubdataChunkPublisher", "bytecodePath": "artifacts-zk/contracts-preprocessed/PubdataChunkPublisher.sol/PubdataChunkPublisher.json", "sourceCodePath": "contracts-preprocessed/PubdataChunkPublisher.sol", - "bytecodeHash": "0x01000049615ece265d67427e684a665de82466e4065085201b7670f19c67df7c", + "bytecodeHash": "0x01000049377ba719b2d7493420854f12ebe67b75e21338777fb22b73e58ec057", "sourceCodeHash": "0x398b1b9325b39d4c31e672866d4cbdf1cab453fae8d29f438262d921d427f094" }, { @@ -136,7 +136,7 @@ "contractName": "SystemContext", "bytecodePath": "artifacts-zk/contracts-preprocessed/SystemContext.sol/SystemContext.json", "sourceCodePath": "contracts-preprocessed/SystemContext.sol", - "bytecodeHash": "0x0100017fb9147ea6f138ef0acbd9cfbc713eeaa7e355c6c4b3404cfb77e7488c", + "bytecodeHash": "0x0100017f235b172e9a808764229a777b027e179eacc88a7ea48ef81cb193630a", "sourceCodeHash": "0x22406893d61abd477ce071dce506cf2534cca7b7717d015769fc8af1f1b80e06" }, { From 4474d53950b577a9a3d8e0ca984bd36b9ad28f42 Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 17:55:39 +0400 Subject: [PATCH 27/29] (fix): change to encodeWithCall --- l1-contracts/contracts/governance/PermanentRestriction.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/governance/PermanentRestriction.sol b/l1-contracts/contracts/governance/PermanentRestriction.sol index bd4c78261..8e51d15d1 100644 --- a/l1-contracts/contracts/governance/PermanentRestriction.sol +++ b/l1-contracts/contracts/governance/PermanentRestriction.sol @@ -221,7 +221,7 @@ contract PermanentRestriction is Restriction, IPermanentRestriction, Ownable2Ste // Note, that we do not use an explicit call here to ensure that the function does not panic in case of // incorrect `_chain` address. - (bool success, bytes memory data) = _chain.staticcall(abi.encodeWithSelector(IGetters.getChainId.selector)); + (bool success, bytes memory data) = _chain.staticcall(abi.encodeCall(IGetters.getChainId, ())); if (!success || data.length < 32) { revert NotAHyperchain(_chain); } From 9fe61213c5306339afb2cac0747ac040af85f980 Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 18:02:18 +0400 Subject: [PATCH 28/29] (fix): remove duplicate check --- l1-contracts/contracts/governance/PermanentRestriction.sol | 7 ------- .../l1/unit/concrete/Governance/PermanentRestriction.t.sol | 7 +------ 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/l1-contracts/contracts/governance/PermanentRestriction.sol b/l1-contracts/contracts/governance/PermanentRestriction.sol index 8e51d15d1..e3b879c8b 100644 --- a/l1-contracts/contracts/governance/PermanentRestriction.sol +++ b/l1-contracts/contracts/governance/PermanentRestriction.sol @@ -219,13 +219,6 @@ contract PermanentRestriction is Restriction, IPermanentRestriction, Ownable2Ste // - Query the Bridgehub for the Hyperchain with the given `chainId`. // - We compare the corresponding addresses - // Note, that we do not use an explicit call here to ensure that the function does not panic in case of - // incorrect `_chain` address. - (bool success, bytes memory data) = _chain.staticcall(abi.encodeCall(IGetters.getChainId, ())); - if (!success || data.length < 32) { - revert NotAHyperchain(_chain); - } - // 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 diff --git a/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol index 6dbd65ae1..0a4be438e 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol @@ -12,7 +12,6 @@ import {IPermanentRestriction} from "contracts/governance/IPermanentRestriction. import {NotAllowed, NotAHyperchain, NotEnoughGas, UnsupportedEncodingVersion, InvalidSelector, ZeroAddress, UnallowedImplementation, RemovingPermanentRestriction, CallNotAllowed} from "contracts/common/L1ContractErrors.sol"; import {Call} from "contracts/governance/Common.sol"; import {IZKChain} from "contracts/state-transition/chain-interfaces/IZKChain.sol"; -import {IGetters} from "contracts/state-transition/chain-interfaces/IGetters.sol"; import {VerifierParams, FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-deps/ZKChainStorage.sol"; import {IAdmin} from "contracts/state-transition/chain-interfaces/IAdmin.sol"; import {AccessControlRestriction} from "contracts/governance/AccessControlRestriction.sol"; @@ -127,9 +126,7 @@ contract PermanentRestrictionTest is ChainTypeManagerTest { } function test_isAdminOfAChainNotAHyperchain() public { - address chain = makeAddr("random"); - vm.expectRevert(abi.encodeWithSelector(NotAHyperchain.selector, chain)); - permRestriction.isAdminOfAChain(chain); + assertFalse(permRestriction.isAdminOfAChain(makeAddr("random"))); } function test_isAdminOfAChainOfAChainNotAnAdmin() public { @@ -342,7 +339,6 @@ contract PermanentRestrictionTest is ChainTypeManagerTest { } function test_validateMigrationToL2RevertNotAllowed() public { - vm.mockCall(address(bridgehub), abi.encodeWithSelector(IGetters.getChainId.selector), abi.encode(271)); Call memory call = _encodeMigraationCall(true, true, true, true, true, address(0)); vm.expectRevert(abi.encodeWithSelector(NotAllowed.selector, address(0))); @@ -350,7 +346,6 @@ contract PermanentRestrictionTest is ChainTypeManagerTest { } function test_validateMigrationToL2() public { - vm.mockCall(address(bridgehub), abi.encodeWithSelector(IGetters.getChainId.selector), abi.encode(271)); address expectedAddress = L2ContractHelper.computeCreate2Address( L2_FACTORY_ADDR, bytes32(0), From 623e1314939d2f18769026441ec82ff7c39018b3 Mon Sep 17 00:00:00 2001 From: Raid Ateir Date: Sun, 27 Oct 2024 18:08:29 +0400 Subject: [PATCH 29/29] (fix): lint --- l1-contracts/contracts/common/L1ContractErrors.sol | 2 -- l1-contracts/contracts/governance/PermanentRestriction.sol | 2 +- .../l1/unit/concrete/Governance/PermanentRestriction.t.sol | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 7697d30b9..5aa1c0a50 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -314,8 +314,6 @@ error IncorrectBatchBounds( ); // 0x64107968 error AssetHandlerNotRegistered(bytes32 assetId); -// 0xff4bbdf1 -error NotAHyperchain(address chainAddress); // 0x64846fe4 error NotARestriction(address addr); // 0xfa5cd00f diff --git a/l1-contracts/contracts/governance/PermanentRestriction.sol b/l1-contracts/contracts/governance/PermanentRestriction.sol index e3b879c8b..30b586086 100644 --- a/l1-contracts/contracts/governance/PermanentRestriction.sol +++ b/l1-contracts/contracts/governance/PermanentRestriction.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.24; -import {CallNotAllowed, RemovingPermanentRestriction, ZeroAddress, UnallowedImplementation, AlreadyWhitelisted, NotAHyperchain, NotAllowed} from "../common/L1ContractErrors.sol"; +import {CallNotAllowed, RemovingPermanentRestriction, ZeroAddress, UnallowedImplementation, AlreadyWhitelisted, NotAllowed} from "../common/L1ContractErrors.sol"; import {L2TransactionRequestTwoBridgesOuter, BridgehubBurnCTMAssetData} from "../bridgehub/IBridgehub.sol"; import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable-v4/access/Ownable2StepUpgradeable.sol"; diff --git a/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol index 0a4be438e..54a576cb7 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/Governance/PermanentRestriction.t.sol @@ -9,7 +9,7 @@ import {ChainTypeManager} from "contracts/state-transition/ChainTypeManager.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 {NotAllowed, NotAHyperchain, NotEnoughGas, UnsupportedEncodingVersion, InvalidSelector, ZeroAddress, UnallowedImplementation, RemovingPermanentRestriction, CallNotAllowed} from "contracts/common/L1ContractErrors.sol"; +import {NotAllowed, NotEnoughGas, UnsupportedEncodingVersion, InvalidSelector, ZeroAddress, UnallowedImplementation, RemovingPermanentRestriction, CallNotAllowed} from "contracts/common/L1ContractErrors.sol"; import {Call} from "contracts/governance/Common.sol"; import {IZKChain} from "contracts/state-transition/chain-interfaces/IZKChain.sol"; import {VerifierParams, FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-deps/ZKChainStorage.sol";