From fedf677426d7298bbc1be83f7ba2d82a84b61494 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 3 Dec 2024 01:20:11 +0100 Subject: [PATCH 01/57] fix error lint --- l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol | 3 --- l1-contracts/contracts/common/L1ContractErrors.sol | 2 -- 2 files changed, 5 deletions(-) diff --git a/l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol b/l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol index 56b0cb75b..2418e5b1d 100644 --- a/l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol +++ b/l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol @@ -8,9 +8,6 @@ error NotRelayedSender(address msgSender, address settlementLayerRelaySender); // 0xf306a770 error NotAssetRouter(address msgSender, address sharedBridge); -// 0x4b62f013 -error TokenNotSet(); - // 0xff514c10 error ChainIdAlreadyPresent(); diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 1b9dff66d..2cc93bc28 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -5,8 +5,6 @@ pragma solidity ^0.8.21; error AccessToFallbackDenied(address target, address invoker); // 0x3995f750 error AccessToFunctionDenied(address target, bytes4 selector, address invoker); -// 0x8164f842 -error ApprovalFailed(); // 0x6c167909 error OnlySelfAllowed(); // 0x52e22c98 From c944c09ba8b68f4f15631ec1cb4f4ecc34e36baa Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 3 Dec 2024 02:10:00 +0100 Subject: [PATCH 02/57] hardhat testing --- .github/workflows/l1-contracts-ci.yaml | 45 -------------------------- 1 file changed, 45 deletions(-) diff --git a/.github/workflows/l1-contracts-ci.yaml b/.github/workflows/l1-contracts-ci.yaml index 0217e5185..ec546bb5c 100644 --- a/.github/workflows/l1-contracts-ci.yaml +++ b/.github/workflows/l1-contracts-ci.yaml @@ -184,51 +184,6 @@ jobs: working-directory: ./l1-contracts run: FOUNDRY_PROFILE=default yarn test:zkfoundry - test-hardhat: - needs: [build, lint] - runs-on: ubuntu-latest - - steps: - - name: Checkout the repository - uses: actions/checkout@v4 - - - name: Use Node.js - uses: actions/setup-node@v3 - with: - node-version: 18.18.0 - cache: yarn - - - name: Install dependencies - run: yarn - - - name: Install l1 deps - working-directory: ./l1-contracts - run: yarn - - - name: Build contracts with hardhat - working-directory: ./l1-contracts - run: yarn build - - - name: Restore artifacts cache - uses: actions/cache/restore@v3 - with: - fail-on-cache-miss: true - key: artifacts-l1-${{ github.sha }} - path: | - da-contracts/out - l1-contracts/cache-forge - l1-contracts/out - l1-contracts/zkout - l2-contracts/cache-forge - l2-contracts/zkout - system-contracts/zkout - - - name: Build L2 contracts - run: yarn l2 build - - - name: Run tests - run: yarn l1 test - check-verifier-generator-l1: runs-on: ubuntu-latest From a84855747bc320b31cb106c698b0fe7e99a45c20 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 13:35:36 +0100 Subject: [PATCH 03/57] track migrations + fix comment --- l1-contracts/contracts/common/L1ContractErrors.sol | 2 ++ .../contracts/state-transition/ChainTypeManager.sol | 6 +++++- system-contracts/contracts/MsgValueSimulator.sol | 6 +++--- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 2cc93bc28..7a3702b1a 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -376,6 +376,8 @@ error BurningNativeWETHNotSupported(); error NoLegacySharedBridge(); // 0x78d2ed02 error ChainAlreadyLive(); +// 0x4e98b356 +error MigrationsNotPaused(); enum SharedBridgeKey { PostUpgradeFirstBatch, diff --git a/l1-contracts/contracts/state-transition/ChainTypeManager.sol b/l1-contracts/contracts/state-transition/ChainTypeManager.sol index 196e5731f..a9d4bf31f 100644 --- a/l1-contracts/contracts/state-transition/ChainTypeManager.sol +++ b/l1-contracts/contracts/state-transition/ChainTypeManager.sol @@ -17,7 +17,7 @@ import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable-v4/ac import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; import {L2_TO_L1_LOG_SERIALIZE_SIZE, DEFAULT_L2_LOGS_TREE_ROOT_HASH, EMPTY_STRING_KECCAK} from "../common/Config.sol"; import {InitialForceDeploymentMismatch, AdminZero, OutdatedProtocolVersion} from "./L1StateTransitionErrors.sol"; -import {ChainAlreadyLive, Unauthorized, ZeroAddress, HashMismatch, GenesisUpgradeZero, GenesisBatchHashZero, GenesisIndexStorageZero, GenesisBatchCommitmentZero} from "../common/L1ContractErrors.sol"; +import {ChainAlreadyLive, Unauthorized, ZeroAddress, HashMismatch, GenesisUpgradeZero, GenesisBatchHashZero, GenesisIndexStorageZero, GenesisBatchCommitmentZero, MigrationsNotPaused} from "../common/L1ContractErrors.sol"; import {SemVer} from "../common/libraries/SemVer.sol"; import {IBridgehub} from "../bridgehub/IBridgehub.sol"; @@ -227,6 +227,10 @@ contract ChainTypeManager is IChainTypeManager, ReentrancyGuard, Ownable2StepUpg uint256 _oldProtocolVersionDeadline, uint256 _newProtocolVersion ) external onlyOwner { + if(!IBridgehub(BRIDGE_HUB).migrationPaused()){ + revert MigrationsNotPaused(); + } + bytes32 newCutHash = keccak256(abi.encode(_cutData)); uint256 previousProtocolVersion = protocolVersion; upgradeCutHash[_oldProtocolVersion] = newCutHash; diff --git a/system-contracts/contracts/MsgValueSimulator.sol b/system-contracts/contracts/MsgValueSimulator.sol index 5fcd0f2d9..fe74f8683 100644 --- a/system-contracts/contracts/MsgValueSimulator.sol +++ b/system-contracts/contracts/MsgValueSimulator.sol @@ -20,9 +20,9 @@ import {InvalidCall} from "./SystemContractErrors.sol"; contract MsgValueSimulator is SystemContractBase { /// @notice Extract value, isSystemCall and to from the extraAbi params. /// @dev The contract accepts value, the callee and whether the call should be a system one via its ABI params. - /// @dev The first ABI param contains the value in the [0..127] bits. The 128th contains - /// the flag whether or not the call should be a system one. - /// The second ABI params contains the callee. + /// @dev The first ABI param contains the value. The second one contains + /// the address to call, while the third one contains whether the `systemCall` flag should + /// be used (this can be helpful when e.g. calling `ContractDeployer`). function _getAbiParams() internal view returns (uint256 value, bool isSystemCall, address to) { value = SystemContractHelper.getExtraAbiData(0); uint256 addressAsUint = SystemContractHelper.getExtraAbiData(1); From c3fcbfc111e9aa459fbda5b909384c9431c6ca97 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 13:37:08 +0100 Subject: [PATCH 04/57] correctly track gas before preparation --- system-contracts/bootloader/bootloader.yul | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/system-contracts/bootloader/bootloader.yul b/system-contracts/bootloader/bootloader.yul index 36d2697f2..b189aedc4 100644 --- a/system-contracts/bootloader/bootloader.yul +++ b/system-contracts/bootloader/bootloader.yul @@ -1094,13 +1094,13 @@ object "Bootloader" { gasPerPubdata, basePubdataSpent ) -> canonicalL1TxHash, gasUsedOnPreparation { + let gasBeforePreparation := gas() + debugLog("gasBeforePreparation", gasBeforePreparation) + let innerTxDataOffset := add(txDataOffset, 32) setPubdataInfo(gasPerPubdata, basePubdataSpent) - let gasBeforePreparation := gas() - debugLog("gasBeforePreparation", gasBeforePreparation) - // Even though the smart contracts on L1 should make sure that the L1->L2 provide enough gas to generate the hash // we should still be able to do it even if this protection layer fails. canonicalL1TxHash := getCanonicalL1TxHash(txDataOffset) From ce74cafdf1fbfed6c0bd639ff16a7a61a897e761 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 13:38:08 +0100 Subject: [PATCH 05/57] correct error in init --- .../contracts/state-transition/chain-deps/DiamondInit.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/state-transition/chain-deps/DiamondInit.sol b/l1-contracts/contracts/state-transition/chain-deps/DiamondInit.sol index 69a646308..65637aadc 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/DiamondInit.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/DiamondInit.sol @@ -8,7 +8,7 @@ import {L2_TO_L1_LOG_SERIALIZE_SIZE, MAX_GAS_PER_TRANSACTION} from "../../common import {InitializeData, IDiamondInit} from "../chain-interfaces/IDiamondInit.sol"; import {PriorityQueue} from "../libraries/PriorityQueue.sol"; import {PriorityTree} from "../libraries/PriorityTree.sol"; -import {ZeroAddress, TooMuchGas} from "../../common/L1ContractErrors.sol"; +import {ZeroAddress, EmptyAssetId, TooMuchGas} from "../../common/L1ContractErrors.sol"; /// @author Matter Labs /// @dev The contract is used only once to initialize the diamond proxy. @@ -43,7 +43,7 @@ contract DiamondInit is ZKChainBase, IDiamondInit { revert ZeroAddress(); } if (_initializeData.baseTokenAssetId == bytes32(0)) { - revert ZeroAddress(); + revert EmptyAssetId(); } if (_initializeData.blobVersionedHashRetriever == address(0)) { revert ZeroAddress(); From 564c5eb067138d640a9fffdf994e5ba357deef66 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 13:41:00 +0100 Subject: [PATCH 06/57] emit event upon deployment --- l1-contracts/contracts/governance/L2AdminFactory.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/l1-contracts/contracts/governance/L2AdminFactory.sol b/l1-contracts/contracts/governance/L2AdminFactory.sol index c8196c616..14048b1e6 100644 --- a/l1-contracts/contracts/governance/L2AdminFactory.sol +++ b/l1-contracts/contracts/governance/L2AdminFactory.sol @@ -17,7 +17,7 @@ import {RestrictionValidator} from "./restriction/RestrictionValidator.sol"; contract L2AdminFactory { /// @notice Emitted when an admin is deployed on the L2. /// @param admin The address of the newly deployed admin. - event AdminDeployed(address admin); + event AdminDeployed(address indexed admin); /// @dev We use storage instead of immutable variables due to the /// specifics of the zkEVM environment, where storage is actually cheaper. @@ -49,6 +49,8 @@ contract L2AdminFactory { } admin = address(new ChainAdmin{salt: _salt}(restrictions)); + + emit AdminDeployed(address(admin)); } /// @notice Checks that the provided list of restrictions is correct. From 352fde6952750df5e492c43e3a01cd2629c19632 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 14:57:29 +0100 Subject: [PATCH 07/57] fix comment --- .../contracts/state-transition/libraries/PriorityTree.sol | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol b/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol index b65255b63..a0939682d 100644 --- a/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol +++ b/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol @@ -66,6 +66,12 @@ library PriorityTree { } /// @notice Process the priority operations of a batch. + /// @dev Note, that the function below only checks that a certain segment of items is present in the tree. + /// It does not check that e.g. there are no zero items inside the provided `itemHashes`, so in theory proofs + /// that include non-existing priority operations could be created. This function relies on the fact + /// that the `itemHashes` of `_priorityOpsData` are hashes of valid priority transactions. + /// This fact is ensures by the fact the rolling hash of those is sent to the Executor by the bootloader + /// and so assuming that zero knowledge proofs are correct, so is the structure of the `itemHashes`. function processBatch(Tree storage _tree, PriorityOpsBatchInfo memory _priorityOpsData) internal { if (_priorityOpsData.itemHashes.length > 0) { bytes32 expectedRoot = Merkle.calculateRootPaths( From 8bebbd04cbad64927839475c43c67ba51653a4af Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 15:05:54 +0100 Subject: [PATCH 08/57] fix historical root --- .../contracts/state-transition/libraries/PriorityTree.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol b/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol index a0939682d..398de6fd0 100644 --- a/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol +++ b/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol @@ -50,7 +50,8 @@ library PriorityTree { /// @notice Set up the tree function setup(Tree storage _tree, uint256 _startIndex) internal { - _tree.tree.setup(ZERO_LEAF_HASH); + bytes32 initialRoot = _tree.tree.setup(ZERO_LEAF_HASH); + _tree.historicalRoots[initialRoot] = true; _tree.startIndex = _startIndex; } From a4d7d6a6b6c31d5176fc6671ef49e59822b83baf Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 15:47:05 +0100 Subject: [PATCH 09/57] remove reentrancy guard --- l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol | 7 +++---- l1-contracts/contracts/bridgehub/MessageRoot.sol | 7 +++---- .../contracts/state-transition/ChainTypeManager.sol | 7 +++---- .../transactionFilterer/GatewayTransactionFilterer.sol | 7 +++---- .../ChainTypeManager/SetNewVersionUpgrade.t.sol | 5 +++++ 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol b/l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol index 856fe6e8b..945561f6b 100644 --- a/l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol +++ b/l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol @@ -8,7 +8,6 @@ import {IBridgehub, L2TransactionRequestTwoBridgesInner} from "./IBridgehub.sol" import {ICTMDeploymentTracker} from "./ICTMDeploymentTracker.sol"; import {IAssetRouterBase} from "../bridge/asset-router/IAssetRouterBase.sol"; -import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; import {TWO_BRIDGES_MAGIC_VALUE} from "../common/Config.sol"; import {L2_BRIDGEHUB_ADDR} from "../common/L2ContractAddresses.sol"; import {OnlyBridgehub, CTMNotRegistered, NotOwnerViaRouter, NoEthAllowed, NotOwner, WrongCounterPart} from "./L1BridgehubErrors.sol"; @@ -20,7 +19,7 @@ bytes1 constant CTM_DEPLOYMENT_TRACKER_ENCODING_VERSION = 0x01; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev /// @dev Contract to be deployed on L1, can link together other contracts based on AssetInfo. -contract CTMDeploymentTracker is ICTMDeploymentTracker, ReentrancyGuard, Ownable2StepUpgradeable { +contract CTMDeploymentTracker is ICTMDeploymentTracker, Ownable2StepUpgradeable { /// @dev Bridgehub smart contract that is used to operate with L2 via asynchronous L2 <-> L1 communication. IBridgehub public immutable override BRIDGE_HUB; @@ -45,7 +44,7 @@ contract CTMDeploymentTracker is ICTMDeploymentTracker, ReentrancyGuard, Ownable /// @dev Contract is expected to be used as proxy implementation on L1. /// @dev Initialize the implementation to prevent Parity hack. - constructor(IBridgehub _bridgehub, IAssetRouterBase _l1AssetRouter) reentrancyGuardInitializer { + constructor(IBridgehub _bridgehub, IAssetRouterBase _l1AssetRouter) { _disableInitializers(); BRIDGE_HUB = _bridgehub; L1_ASSET_ROUTER = _l1AssetRouter; @@ -53,7 +52,7 @@ contract CTMDeploymentTracker is ICTMDeploymentTracker, ReentrancyGuard, Ownable /// @notice used to initialize the contract /// @param _owner the owner of the contract - function initialize(address _owner) external reentrancyGuardInitializer { + function initialize(address _owner) external { _transferOwnership(_owner); } diff --git a/l1-contracts/contracts/bridgehub/MessageRoot.sol b/l1-contracts/contracts/bridgehub/MessageRoot.sol index 62303b8dc..1ecc36d8b 100644 --- a/l1-contracts/contracts/bridgehub/MessageRoot.sol +++ b/l1-contracts/contracts/bridgehub/MessageRoot.sol @@ -6,7 +6,6 @@ import {DynamicIncrementalMerkle} from "../common/libraries/DynamicIncrementalMe import {IBridgehub} from "./IBridgehub.sol"; import {IMessageRoot} from "./IMessageRoot.sol"; -import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; import {OnlyBridgehub, OnlyChain, ChainExists, MessageRootNotRegistered, TooManyChains} from "./L1BridgehubErrors.sol"; import {FullMerkle} from "../common/libraries/FullMerkle.sol"; @@ -27,7 +26,7 @@ bytes32 constant SHARED_ROOT_TREE_EMPTY_HASH = bytes32( /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev /// @dev The MessageRoot contract is responsible for storing the cross message roots of the chains and the aggregated root of all chains. -contract MessageRoot is IMessageRoot, ReentrancyGuard { +contract MessageRoot is IMessageRoot { using FullMerkle for FullMerkle.FullTree; using DynamicIncrementalMerkle for DynamicIncrementalMerkle.Bytes32PushTree; @@ -75,13 +74,13 @@ contract MessageRoot is IMessageRoot, ReentrancyGuard { /// @dev Contract is expected to be used as proxy implementation on L1, but as a system contract on L2. /// This means we call the _initialize in both the constructor and the initialize functions. /// @dev Initialize the implementation to prevent Parity hack. - constructor(IBridgehub _bridgehub) reentrancyGuardInitializer { + constructor(IBridgehub _bridgehub) { BRIDGE_HUB = _bridgehub; _initialize(); } /// @dev Initializes a contract for later use. Expected to be used in the proxy on L1, on L2 it is a system contract without a proxy. - function initialize() external reentrancyGuardInitializer { + function initialize() external { _initialize(); } diff --git a/l1-contracts/contracts/state-transition/ChainTypeManager.sol b/l1-contracts/contracts/state-transition/ChainTypeManager.sol index a9d4bf31f..f5cd55b81 100644 --- a/l1-contracts/contracts/state-transition/ChainTypeManager.sol +++ b/l1-contracts/contracts/state-transition/ChainTypeManager.sol @@ -14,7 +14,6 @@ import {IChainTypeManager, ChainTypeManagerInitializeData, ChainCreationParams} import {IZKChain} from "./chain-interfaces/IZKChain.sol"; import {FeeParams} from "./chain-deps/ZKChainStorage.sol"; import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable-v4/access/Ownable2StepUpgradeable.sol"; -import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; import {L2_TO_L1_LOG_SERIALIZE_SIZE, DEFAULT_L2_LOGS_TREE_ROOT_HASH, EMPTY_STRING_KECCAK} from "../common/Config.sol"; import {InitialForceDeploymentMismatch, AdminZero, OutdatedProtocolVersion} from "./L1StateTransitionErrors.sol"; import {ChainAlreadyLive, Unauthorized, ZeroAddress, HashMismatch, GenesisUpgradeZero, GenesisBatchHashZero, GenesisIndexStorageZero, GenesisBatchCommitmentZero, MigrationsNotPaused} from "../common/L1ContractErrors.sol"; @@ -24,7 +23,7 @@ import {IBridgehub} from "../bridgehub/IBridgehub.sol"; /// @title State Transition Manager contract /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev -contract ChainTypeManager is IChainTypeManager, ReentrancyGuard, Ownable2StepUpgradeable { +contract ChainTypeManager is IChainTypeManager, Ownable2StepUpgradeable { using EnumerableMap for EnumerableMap.UintToAddressMap; /// @notice Address of the bridgehub @@ -65,7 +64,7 @@ contract ChainTypeManager is IChainTypeManager, ReentrancyGuard, Ownable2StepUpg /// @dev Contract is expected to be used as proxy implementation. /// @dev Initialize the implementation to prevent Parity hack. - constructor(address _bridgehub) reentrancyGuardInitializer { + constructor(address _bridgehub) { BRIDGE_HUB = _bridgehub; // While this does not provide a protection in the production, it is needed for local testing @@ -115,7 +114,7 @@ contract ChainTypeManager is IChainTypeManager, ReentrancyGuard, Ownable2StepUpg } /// @dev initialize - function initialize(ChainTypeManagerInitializeData calldata _initializeData) external reentrancyGuardInitializer { + function initialize(ChainTypeManagerInitializeData calldata _initializeData) external { if (_initializeData.owner == address(0)) { revert ZeroAddress(); } diff --git a/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol b/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol index 25d57ce4f..39bd69d1d 100644 --- a/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol +++ b/l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol @@ -4,7 +4,6 @@ pragma solidity 0.8.24; import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable-v4/access/Ownable2StepUpgradeable.sol"; -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"; @@ -15,7 +14,7 @@ import {IL2AssetRouter} from "../bridge/asset-router/IL2AssetRouter.sol"; /// @custom:security-contact security@matterlabs.dev /// @dev Filters transactions received by the Mailbox /// @dev Only allows whitelisted senders to deposit to Gateway -contract GatewayTransactionFilterer is ITransactionFilterer, ReentrancyGuard, Ownable2StepUpgradeable { +contract GatewayTransactionFilterer is ITransactionFilterer, Ownable2StepUpgradeable { /// @notice Event emitted when sender is whitelisted event WhitelistGranted(address indexed sender); @@ -33,7 +32,7 @@ contract GatewayTransactionFilterer is ITransactionFilterer, ReentrancyGuard, Ow /// @dev Contract is expected to be used as proxy implementation. /// @dev Initialize the implementation to prevent Parity hack. - constructor(IBridgehub _bridgeHub, address _assetRouter) reentrancyGuardInitializer { + constructor(IBridgehub _bridgeHub, address _assetRouter) { BRIDGE_HUB = _bridgeHub; L1_ASSET_ROUTER = _assetRouter; _disableInitializers(); @@ -41,7 +40,7 @@ contract GatewayTransactionFilterer is ITransactionFilterer, ReentrancyGuard, Ow /// @notice Initializes a contract filterer for later use. Expected to be used in the proxy. /// @param _owner The address which can upgrade the implementation. - function initialize(address _owner) external reentrancyGuardInitializer initializer { + function initialize(address _owner) external initializer { if (_owner == address(0)) { revert ZeroAddress(); } diff --git a/l1-contracts/test/foundry/l1/unit/concrete/state-transition/ChainTypeManager/SetNewVersionUpgrade.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/state-transition/ChainTypeManager/SetNewVersionUpgrade.t.sol index 1dbaa2462..014deded1 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/state-transition/ChainTypeManager/SetNewVersionUpgrade.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/state-transition/ChainTypeManager/SetNewVersionUpgrade.t.sol @@ -11,6 +11,11 @@ contract setNewVersionUpgradeTest is ChainTypeManagerTest { function test_SettingNewVersionUpgrade() public { assertEq(chainContractAddress.protocolVersion(), 0, "Initial protocol version is not correct"); + vm.mockCall( + address(bridgehub), + abi.encodeWithSelector(bridgehub.migrationPaused.selector), + abi.encode(true) + ); address randomDiamondInit = address(0x303030303030303030303); Diamond.DiamondCutData memory newDiamondCutData = getDiamondCutData(address(randomDiamondInit)); From ddfe592e2486f5c522e0faf8e047f767ed7529ec Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 15:50:17 +0100 Subject: [PATCH 10/57] gas optimization --- system-contracts/contracts/SystemContext.sol | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/system-contracts/contracts/SystemContext.sol b/system-contracts/contracts/SystemContext.sol index b4e696350..3e989bdda 100644 --- a/system-contracts/contracts/SystemContext.sol +++ b/system-contracts/contracts/SystemContext.sol @@ -485,9 +485,7 @@ contract SystemContext is ISystemContext, ISystemContextDeprecated, SystemContra batchHashes[previousBatchNumber] = _prevBatchHash; // Setting new block number and timestamp - BlockInfo memory newBlockInfo = BlockInfo({number: previousBatchNumber + 1, timestamp: _newTimestamp}); - - currentBatchInfo = newBlockInfo; + currentBatchInfo = BlockInfo({number: previousBatchNumber + 1, timestamp: _newTimestamp}); baseFee = _baseFee; @@ -502,8 +500,7 @@ contract SystemContext is ISystemContext, ISystemContextDeprecated, SystemContra uint256 _number, uint256 _baseFee ) external onlyCallFromBootloader { - BlockInfo memory newBlockInfo = BlockInfo({number: uint128(_number), timestamp: uint128(_newTimestamp)}); - currentBatchInfo = newBlockInfo; + currentBatchInfo = BlockInfo({number: uint128(_number), timestamp: uint128(_newTimestamp)}); baseFee = _baseFee; } From 472f972f01ee4bd0a592f8a5ce013a14f296dcb6 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 15:51:14 +0100 Subject: [PATCH 11/57] upd comment --- .../contracts/state-transition/chain-interfaces/IAdmin.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol b/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol index ef21f4051..2b6ee340d 100644 --- a/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol +++ b/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol @@ -62,7 +62,7 @@ interface IAdmin is IZKChainBase { function freezeDiamond() external; /// @notice Unpause the functionality of all freezable facets & their selectors - /// @dev Both the admin and the CTM can unfreeze Diamond Proxy + /// @dev Only the CTM can unfreeze Diamond Proxy function unfreezeDiamond() external; function genesisUpgrade( From 5e07a0f5fed6a6b4b277bf9d453e96f4e3a20b1d Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 15:51:55 +0100 Subject: [PATCH 12/57] fix interface --- l1-contracts/contracts/upgrades/GatewayUpgrade.sol | 2 +- l1-contracts/contracts/upgrades/IGatewayUpgrade.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol index 3ba61c228..ed33da9f1 100644 --- a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol +++ b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol @@ -80,7 +80,7 @@ contract GatewayUpgrade is BaseZkSyncUpgrade, L1GatewayBase { /// @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 { + function upgradeExternal(ProposedUpgrade calldata _proposedUpgrade) external override { super.upgrade(_proposedUpgrade); } } diff --git a/l1-contracts/contracts/upgrades/IGatewayUpgrade.sol b/l1-contracts/contracts/upgrades/IGatewayUpgrade.sol index c2c972a5e..bc4a3873c 100644 --- a/l1-contracts/contracts/upgrades/IGatewayUpgrade.sol +++ b/l1-contracts/contracts/upgrades/IGatewayUpgrade.sol @@ -13,5 +13,5 @@ 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); + function upgradeExternal(ProposedUpgrade calldata _upgrade) external; } From 08891b95592e24f249a961d783551bbc04cf85d6 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 15:53:22 +0100 Subject: [PATCH 13/57] rename + initializers --- l1-contracts/contracts/state-transition/ChainTypeManager.sol | 2 ++ system-contracts/bootloader/bootloader.yul | 2 +- system-contracts/scripts/preprocess-bootloader.ts | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/state-transition/ChainTypeManager.sol b/l1-contracts/contracts/state-transition/ChainTypeManager.sol index f5cd55b81..b17e01992 100644 --- a/l1-contracts/contracts/state-transition/ChainTypeManager.sol +++ b/l1-contracts/contracts/state-transition/ChainTypeManager.sol @@ -70,6 +70,8 @@ contract ChainTypeManager is IChainTypeManager, Ownable2StepUpgradeable { // While this does not provide a protection in the production, it is needed for local testing // Length of the L2Log encoding should not be equal to the length of other L2Logs' tree nodes preimages assert(L2_TO_L1_LOG_SERIALIZE_SIZE != 2 * 32); + + _disableInitializers(); } /// @notice only the bridgehub can call diff --git a/system-contracts/bootloader/bootloader.yul b/system-contracts/bootloader/bootloader.yul index b189aedc4..5b43e290b 100644 --- a/system-contracts/bootloader/bootloader.yul +++ b/system-contracts/bootloader/bootloader.yul @@ -2473,7 +2473,7 @@ object "Bootloader" { let ptr := NEW_FACTORY_DEPS_BEGIN_BYTE() // Selector - mstore(ptr, {{MARK_BATCH_AS_REPUBLISHED_SELECTOR}}) + mstore(ptr, {{MARK_FACTORY_DEPS_SELECTOR}}) ptr := add(ptr, 32) // Saving whether the dependencies should be sent on L1 diff --git a/system-contracts/scripts/preprocess-bootloader.ts b/system-contracts/scripts/preprocess-bootloader.ts index e3dc18aaf..29454dcd2 100644 --- a/system-contracts/scripts/preprocess-bootloader.ts +++ b/system-contracts/scripts/preprocess-bootloader.ts @@ -63,7 +63,7 @@ function getSystemContextCodeHash() { // Maybe in the future some of these params will be passed // in a JSON file. For now, a simple object is ok here. const params = { - MARK_BATCH_AS_REPUBLISHED_SELECTOR: getSelector("KnownCodesStorage", "markFactoryDeps"), + MARK_FACTORY_DEPS_SELECTOR: getSelector("KnownCodesStorage", "markFactoryDeps"), VALIDATE_TX_SELECTOR: getSelector("IAccount", "validateTransaction"), EXECUTE_TX_SELECTOR: getSelector("DefaultAccount", "executeTransaction"), RIGHT_PADDED_GET_ACCOUNT_VERSION_SELECTOR: getPaddedSelector("ContractDeployer", "extendedAccountVersion"), From 806b5ec4163c042f70671114ac30e40fcc81c468 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 17:11:23 +0100 Subject: [PATCH 14/57] fix asset router send --- l1-contracts/contracts/bridge/L1Nullifier.sol | 2 +- .../contracts/bridge/asset-router/L1AssetRouter.sol | 8 +++++++- l1-contracts/contracts/upgrades/GatewayUpgrade.sol | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/l1-contracts/contracts/bridge/L1Nullifier.sol b/l1-contracts/contracts/bridge/L1Nullifier.sol index defde97c7..1e9687b1d 100644 --- a/l1-contracts/contracts/bridge/L1Nullifier.sol +++ b/l1-contracts/contracts/bridge/L1Nullifier.sol @@ -246,7 +246,7 @@ contract L1Nullifier is IL1Nullifier, ReentrancyGuard, Ownable2StepUpgradeable, /// @param _l1AssetRouter The address of the asset router. function setL1AssetRouter(address _l1AssetRouter) external onlyOwner { if (address(l1AssetRouter) != address(0)) { - revert AddressAlreadySet(address(_l1AssetRouter)); + revert AddressAlreadySet(address(l1AssetRouter)); } if (_l1AssetRouter == address(0)) { revert ZeroAddress(); diff --git a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol index f052cb28b..17049267c 100644 --- a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol @@ -22,7 +22,7 @@ import {DataEncoding} from "../../common/libraries/DataEncoding.sol"; import {AddressAliasHelper} from "../../vendor/AddressAliasHelper.sol"; import {TWO_BRIDGES_MAGIC_VALUE, ETH_TOKEN_ADDRESS} from "../../common/Config.sol"; import {NativeTokenVaultAlreadySet} from "../L1BridgeContractErrors.sol"; -import {LegacyBridgeUsesNonNativeToken, NonEmptyMsgValue, UnsupportedEncodingVersion, AssetIdNotSupported, AssetHandlerDoesNotExist, Unauthorized, ZeroAddress, TokenNotSupported, AddressAlreadyUsed} from "../../common/L1ContractErrors.sol"; +import {LegacyBridgeUsesNonNativeToken, NonEmptyMsgValue, UnsupportedEncodingVersion, AssetIdNotSupported, AssetHandlerDoesNotExist, Unauthorized, ZeroAddress, TokenNotSupported, AddressAlreadyUsed, TokensWithFeesNotSupported} from "../../common/L1ContractErrors.sol"; import {L2_ASSET_ROUTER_ADDR} from "../../common/L2ContractAddresses.sol"; import {IBridgehub, L2TransactionRequestTwoBridgesInner, L2TransactionRequestDirect} from "../../bridgehub/IBridgehub.sol"; @@ -433,8 +433,14 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { weCanTransfer = true; } if (weCanTransfer) { + uint256 balanceBefore = l1Token.balanceOf(address(nativeTokenVault)); // slither-disable-next-line arbitrary-send-erc20 l1Token.safeTransferFrom(_originalCaller, address(nativeTokenVault), _amount); + uint256 balanceAfter = l1Token.balanceOf(address(nativeTokenVault)); + + if (balanceAfter - balanceBefore != _amount) { + revert TokensWithFeesNotSupported(); + } return true; } return false; diff --git a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol index ed33da9f1..b1a327eb3 100644 --- a/l1-contracts/contracts/upgrades/GatewayUpgrade.sol +++ b/l1-contracts/contracts/upgrades/GatewayUpgrade.sol @@ -29,7 +29,7 @@ struct GatewayUpgradeEncodedInput { /// @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, L1GatewayBase { +contract GatewayUpgrade is BaseZkSyncUpgrade, L1GatewayBase, IGatewayUpgrade { using PriorityQueue for PriorityQueue.Queue; using PriorityTree for PriorityTree.Tree; From 96208c4f10319a88f7939d7ddaede72d075d048d Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 17:15:07 +0100 Subject: [PATCH 15/57] remove msg root limitation --- l1-contracts/contracts/bridgehub/MessageRoot.sol | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/l1-contracts/contracts/bridgehub/MessageRoot.sol b/l1-contracts/contracts/bridgehub/MessageRoot.sol index 1ecc36d8b..110ba337d 100644 --- a/l1-contracts/contracts/bridgehub/MessageRoot.sol +++ b/l1-contracts/contracts/bridgehub/MessageRoot.sol @@ -6,13 +6,11 @@ import {DynamicIncrementalMerkle} from "../common/libraries/DynamicIncrementalMe import {IBridgehub} from "./IBridgehub.sol"; import {IMessageRoot} from "./IMessageRoot.sol"; -import {OnlyBridgehub, OnlyChain, ChainExists, MessageRootNotRegistered, TooManyChains} from "./L1BridgehubErrors.sol"; +import {OnlyBridgehub, OnlyChain, ChainExists, MessageRootNotRegistered} from "./L1BridgehubErrors.sol"; import {FullMerkle} from "../common/libraries/FullMerkle.sol"; import {MessageHashing} from "../common/libraries/MessageHashing.sol"; -import {MAX_NUMBER_OF_ZK_CHAINS} from "../common/Config.sol"; - // Chain tree consists of batch commitments as their leaves. We use hash of "new bytes(96)" as the hash of an empty leaf. bytes32 constant CHAIN_TREE_EMPTY_ENTRY_HASH = bytes32( 0x46700b4d40ac5c35af2c22dda2787a91eb567b06c924a8fb8ae9a05b20c08c21 @@ -150,10 +148,9 @@ contract MessageRoot is IMessageRoot { /// @param _chainId the chainId of the chain function _addNewChain(uint256 _chainId) internal { uint256 cachedChainCount = chainCount; - if (cachedChainCount >= MAX_NUMBER_OF_ZK_CHAINS) { - revert TooManyChains(cachedChainCount, MAX_NUMBER_OF_ZK_CHAINS); - } + // Since only the bridgehub can add new chains to the message root, it is expected that + // it will be responsible for ensuring that the number of chains does not exceed the limit. ++chainCount; chainIndex[_chainId] = cachedChainCount; chainIndexToId[cachedChainCount] = _chainId; From 701fd2a019d136913118cb5d190b2f0a33f481b4 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 17:19:17 +0100 Subject: [PATCH 16/57] remove unneeded force approve --- l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol | 2 -- 1 file changed, 2 deletions(-) diff --git a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol index 17049267c..c7d175cf9 100644 --- a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol @@ -540,8 +540,6 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { revert LegacyBridgeUsesNonNativeToken(); } - IERC20(_l1Token).forceApprove(address(nativeTokenVault), _amount); - bridgeMintCalldata = _burn({ _chainId: ERA_CHAIN_ID, _nextMsgValue: 0, From 5ebf38c78f67d42fa85a9885189bdb2e159c1ccb Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 17:21:47 +0100 Subject: [PATCH 17/57] add comment --- system-contracts/contracts/libraries/SystemContractHelper.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/system-contracts/contracts/libraries/SystemContractHelper.sol b/system-contracts/contracts/libraries/SystemContractHelper.sol index 7205ac86d..e901af3d4 100644 --- a/system-contracts/contracts/libraries/SystemContractHelper.sol +++ b/system-contracts/contracts/libraries/SystemContractHelper.sol @@ -436,6 +436,8 @@ library SystemContractHelper { /// 1. Force-deploy `SloadContract` to the address. /// 2. Read the required slot. /// 3. Force-deploy the previous bytecode back. + /// @dev Note, that the function will overwrite the account states of the `_addr`, i.e. + /// this function should NEVER be used against custom accounts. function forcedSload(address _addr, bytes32 _key) internal returns (bytes32 result) { bytes32 sloadContractBytecodeHash; address sloadContractAddress = SLOAD_CONTRACT_ADDRESS; From 93b71bc8e23017a5a7136c43c72f8058c943298e Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 17:30:59 +0100 Subject: [PATCH 18/57] fix comment --- l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol index 9066691a1..e43913836 100644 --- a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol @@ -273,6 +273,8 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken uint256 _amount, bool _isNative ) internal override { + // Note, that we do not update balances for chains where the assetId comes from, + // since these chains can mint new instances of the token. if ((_isNative) || (originChainId[_assetId] != _chainId)) { chainBalance[_chainId][_assetId] += _amount; } @@ -284,6 +286,8 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken uint256 _amount, bool _isNative ) internal override { + // Note, that we do not update balances for chains where the assetId comes from, + // since these chains can mint new instances of the token. if ((_isNative) || (originChainId[_assetId] != _chainId)) { // Check that the chain has sufficient balance if (chainBalance[_chainId][_assetId] < _amount) { From 53f43c58ce50284c6434b5bf78781f73dccdf634 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 17:33:46 +0100 Subject: [PATCH 19/57] small consistency --- da-contracts/contracts/RollupL1DAValidator.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/da-contracts/contracts/RollupL1DAValidator.sol b/da-contracts/contracts/RollupL1DAValidator.sol index cf3f36bd8..815343274 100644 --- a/da-contracts/contracts/RollupL1DAValidator.sol +++ b/da-contracts/contracts/RollupL1DAValidator.sol @@ -150,7 +150,7 @@ contract RollupL1DAValidator is IL1DAValidator, CalldataDA { for (uint256 i = 0; i < _blobsProvided; ++i) { bytes calldata commitmentData = _operatorDAInput[:PUBDATA_COMMITMENT_SIZE]; bytes32 prepublishedCommitment = bytes32( - _operatorDAInput[PUBDATA_COMMITMENT_SIZE:PUBDATA_COMMITMENT_SIZE + 32] + _operatorDAInput[PUBDATA_COMMITMENT_SIZE:BLOB_DA_INPUT_SIZE] ); if (prepublishedCommitment != bytes32(0)) { From 920a53033fde93699c965a727cb92561c1445c83 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 17:35:59 +0100 Subject: [PATCH 20/57] add comment --- l1-contracts/contracts/governance/AccessControlRestriction.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/l1-contracts/contracts/governance/AccessControlRestriction.sol b/l1-contracts/contracts/governance/AccessControlRestriction.sol index 6052d1e6a..fb430fd7d 100644 --- a/l1-contracts/contracts/governance/AccessControlRestriction.sol +++ b/l1-contracts/contracts/governance/AccessControlRestriction.sol @@ -65,6 +65,8 @@ contract AccessControlRestriction is Restriction, IAccessControlRestriction, Acc // `requiredRoles` and `requiredRolesForFallback` is 0, the default admin is by default a required // role for all the functions. if (_call.data.length < 4) { + // Note, that the following restriction protects only for targets that were compiled after + // Solidity v0.4.18, since before a substring of selector could still call the function. if (!hasRole(requiredRolesForFallback[_call.target], _invoker)) { revert AccessToFallbackDenied(_call.target, _invoker); } From 541bbbc7fb74c61ab8d02c2c555405629cb5b298 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 17:41:27 +0100 Subject: [PATCH 21/57] event and bootloader comment --- .../state-transition/chain-deps/facets/Admin.sol | 3 ++- .../state-transition/chain-interfaces/IAdmin.sol | 2 +- l1-contracts/selectors | 8 ++++---- system-contracts/bootloader/bootloader.yul | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol index 8a3927e11..9acf3ac3e 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol @@ -137,7 +137,7 @@ contract AdminFacet is ZKChainBase, IAdmin { } s.feeParams.pubdataPricingMode = _pricingMode; - emit ValidiumModeStatusUpdate(_pricingMode); + emit PubdataPricingModeUpdaate(_pricingMode); } /// @inheritdoc IAdmin @@ -242,6 +242,7 @@ contract AdminFacet is ZKChainBase, IAdmin { }); Diamond.diamondCut(cutData); + emit ExecuteUpgrade(cutData); } /*////////////////////////////////////////////////////////////// diff --git a/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol b/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol index 2b6ee340d..2a10acf08 100644 --- a/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol +++ b/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol @@ -107,7 +107,7 @@ interface IAdmin is IZKChainBase { event NewFeeParams(FeeParams oldFeeParams, FeeParams newFeeParams); /// @notice Validium mode status changed - event ValidiumModeStatusUpdate(PubdataPricingMode validiumMode); + event PubdataPricingModeUpdaate(PubdataPricingMode validiumMode); /// @notice The transaction filterer has been updated event NewTransactionFilterer(address oldTransactionFilterer, address newTransactionFilterer); diff --git a/l1-contracts/selectors b/l1-contracts/selectors index 1fecf6de9..95d5ada8d 100644 --- a/l1-contracts/selectors +++ b/l1-contracts/selectors @@ -2815,7 +2815,7 @@ AdminFacetTest |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| | Event | ValidatorStatusUpdate(address,bool) | 0x065b77b53864e46fda3d8986acb51696223d6dde7ced42441eb150bae6d48136 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| -| Event | ValidiumModeStatusUpdate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | +| Event | PubdataPricingModeUpdaate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| | Error | AddressHasNoCode(address) | 0x86bb51b8 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| @@ -5002,7 +5002,7 @@ AdminFacet |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| | Event | ValidatorStatusUpdate(address,bool) | 0x065b77b53864e46fda3d8986acb51696223d6dde7ced42441eb150bae6d48136 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| -| Event | ValidiumModeStatusUpdate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | +| Event | PubdataPricingModeUpdaate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| | Error | AddressHasNoCode(address) | 0x86bb51b8 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| @@ -5390,7 +5390,7 @@ IAdmin |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| | Event | ValidatorStatusUpdate(address,bool) | 0x065b77b53864e46fda3d8986acb51696223d6dde7ced42441eb150bae6d48136 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| -| Event | ValidiumModeStatusUpdate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | +| Event | PubdataPricingModeUpdaate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | +----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------+ IDiamondInit @@ -5681,7 +5681,7 @@ IZKChain |----------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| | Event | ValidatorStatusUpdate(address,bool) | 0x065b77b53864e46fda3d8986acb51696223d6dde7ced42441eb150bae6d48136 | |----------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| -| Event | ValidiumModeStatusUpdate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | +| Event | PubdataPricingModeUpdaate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | +----------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------+ Diamond diff --git a/system-contracts/bootloader/bootloader.yul b/system-contracts/bootloader/bootloader.yul index 5b43e290b..8933c492b 100644 --- a/system-contracts/bootloader/bootloader.yul +++ b/system-contracts/bootloader/bootloader.yul @@ -403,7 +403,7 @@ object "Bootloader" { ret := add(TX_DESCRIPTION_BEGIN_BYTE(), mul(MAX_TRANSACTIONS_IN_BATCH(), TX_DESCRIPTION_SIZE())) } - /// @dev The memory page consists of 59000000 / 32 VM words. + /// @dev The memory page consists of 63800000 / 32 VM words. /// Each execution result is a single boolean, but /// for the sake of simplicity we will spend 32 bytes on each /// of those for now. From 832024290db2d32ff17e44b599b497a0a4334878 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 18:05:17 +0100 Subject: [PATCH 22/57] set of minor nits + wrapeed base token check --- da-contracts/contracts/CalldataDA.sol | 2 +- da-contracts/contracts/DAContractsErrors.sol | 4 ++-- da-contracts/contracts/IL1Messenger.sol | 2 +- l1-contracts/contracts/bridge/L2WrappedBaseTokenStore.sol | 5 ++++- l1-contracts/contracts/common/L1ContractErrors.sol | 2 ++ l1-contracts/contracts/common/L2ContractAddresses.sol | 2 +- l1-contracts/contracts/common/interfaces/IL1Messenger.sol | 2 +- .../contracts/state-transition/L1StateTransitionErrors.sol | 4 ++-- .../state-transition/data-availability/CalldataDA.sol | 2 +- .../state-transition/data-availability/CalldataDAGateway.sol | 2 +- .../state-transition/data-availability/CalldataDA.t.sol | 2 +- l2-contracts/contracts/L2ContractHelper.sol | 2 +- system-contracts/contracts/interfaces/IL1Messenger.sol | 2 +- 13 files changed, 19 insertions(+), 14 deletions(-) diff --git a/da-contracts/contracts/CalldataDA.sol b/da-contracts/contracts/CalldataDA.sol index 776cd912d..3841f0c5e 100644 --- a/da-contracts/contracts/CalldataDA.sol +++ b/da-contracts/contracts/CalldataDA.sol @@ -70,7 +70,7 @@ abstract contract CalldataDA { // Now, we need to double check that the provided input was indeed returned by the L2 DA validator. if (keccak256(_operatorDAInput[:ptr]) != _l2DAValidatorOutputHash) { - revert InvalidL2DAOutputHash(); + revert InvalidL2DAOutputHash(_l2DAValidatorOutputHash); } // The rest of the output was provided specifically by the operator diff --git a/da-contracts/contracts/DAContractsErrors.sol b/da-contracts/contracts/DAContractsErrors.sol index 0923b3579..039748629 100644 --- a/da-contracts/contracts/DAContractsErrors.sol +++ b/da-contracts/contracts/DAContractsErrors.sol @@ -25,8 +25,8 @@ error InvalidNumberOfBlobs(uint256 blobsProvided, uint256 maxBlobsSupported); // 0xcd384e46 error InvalidBlobsHashes(uint256 operatorDAInputLength, uint256 blobsProvided); -// 0xe9e79528 -error InvalidL2DAOutputHash(); +// 0xd2531c15 +error InvalidL2DAOutputHash(bytes32 l2DAValidatorOutputHash); // 0x3db6e664 error OneBlobWithCalldata(); diff --git a/da-contracts/contracts/IL1Messenger.sol b/da-contracts/contracts/IL1Messenger.sol index f0557487b..a2aa35fa0 100644 --- a/da-contracts/contracts/IL1Messenger.sol +++ b/da-contracts/contracts/IL1Messenger.sol @@ -7,5 +7,5 @@ pragma solidity 0.8.24; * @notice The interface of the L1 Messenger contract, responsible for sending messages to L1. */ interface IL1Messenger { - function sendToL1(bytes memory _message) external returns (bytes32); + function sendToL1(bytes calldata _message) external returns (bytes32); } diff --git a/l1-contracts/contracts/bridge/L2WrappedBaseTokenStore.sol b/l1-contracts/contracts/bridge/L2WrappedBaseTokenStore.sol index 06e7a9769..d52a7599c 100644 --- a/l1-contracts/contracts/bridge/L2WrappedBaseTokenStore.sol +++ b/l1-contracts/contracts/bridge/L2WrappedBaseTokenStore.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.0; import {Ownable2Step} from "@openzeppelin/contracts-v4/access/Ownable2Step.sol"; -import {ZeroAddress, Unauthorized} from "../common/L1ContractErrors.sol"; +import {ZeroAddress, Unauthorized, WrappedBaseTokenAlreadyRegistered} from "../common/L1ContractErrors.sol"; /// @title L2WrappedBaseTokenStore /// @author Matter Labs @@ -69,6 +69,9 @@ contract L2WrappedBaseTokenStore is Ownable2Step { if (_l2WBaseToken == address(0)) { revert ZeroAddress(); } + if (l2WBaseTokenAddress[_chainId] != address(0)) { + revert WrappedBaseTokenAlreadyRegistered(); + } _setWBaseTokenAddress(_chainId, _l2WBaseToken); } diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 7a3702b1a..1cd8c3186 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -378,6 +378,8 @@ error NoLegacySharedBridge(); error ChainAlreadyLive(); // 0x4e98b356 error MigrationsNotPaused(); +// 0xf20c5c2a +error WrappedBaseTokenAlreadyRegistered(); enum SharedBridgeKey { PostUpgradeFirstBatch, diff --git a/l1-contracts/contracts/common/L2ContractAddresses.sol b/l1-contracts/contracts/common/L2ContractAddresses.sol index 2f656acd9..3a130b73d 100644 --- a/l1-contracts/contracts/common/L2ContractAddresses.sol +++ b/l1-contracts/contracts/common/L2ContractAddresses.sol @@ -61,7 +61,7 @@ interface IL2Messenger { /// @notice Sends an arbitrary length message to L1. /// @param _message The variable length message to be sent to L1. /// @return Returns the keccak256 hashed value of the message. - function sendToL1(bytes memory _message) external returns (bytes32); + function sendToL1(bytes calldata _message) external returns (bytes32); } /// @dev An l2 system contract address, used in the assetId calculation for native assets. diff --git a/l1-contracts/contracts/common/interfaces/IL1Messenger.sol b/l1-contracts/contracts/common/interfaces/IL1Messenger.sol index f0557487b..a2aa35fa0 100644 --- a/l1-contracts/contracts/common/interfaces/IL1Messenger.sol +++ b/l1-contracts/contracts/common/interfaces/IL1Messenger.sol @@ -7,5 +7,5 @@ pragma solidity 0.8.24; * @notice The interface of the L1 Messenger contract, responsible for sending messages to L1. */ interface IL1Messenger { - function sendToL1(bytes memory _message) external returns (bytes32); + function sendToL1(bytes calldata _message) external returns (bytes32); } diff --git a/l1-contracts/contracts/state-transition/L1StateTransitionErrors.sol b/l1-contracts/contracts/state-transition/L1StateTransitionErrors.sol index 55bb7ac16..767005991 100644 --- a/l1-contracts/contracts/state-transition/L1StateTransitionErrors.sol +++ b/l1-contracts/contracts/state-transition/L1StateTransitionErrors.sol @@ -83,8 +83,8 @@ error PubdataTooSmall(uint256 pubdataInputLength, uint256 blobCommitmentSize); // 0xcba35a08 error PubdataTooLong(uint256 pubdataLength, uint256 blobSizeBytes); -// 0x1b0f562e -error InvalidPubdataHash(); +// 0x5513177c +error InvalidPubdataHash(bytes32 fullPubdataHash, bytes32 pubdata); // 0x5717f940 error InvalidPubdataSource(uint8 pubdataSource); diff --git a/l1-contracts/contracts/state-transition/data-availability/CalldataDA.sol b/l1-contracts/contracts/state-transition/data-availability/CalldataDA.sol index 423511523..d319d92df 100644 --- a/l1-contracts/contracts/state-transition/data-availability/CalldataDA.sol +++ b/l1-contracts/contracts/state-transition/data-availability/CalldataDA.sol @@ -107,7 +107,7 @@ abstract contract CalldataDA { revert PubdataTooLong(_pubdata.length, BLOB_SIZE_BYTES); } if (_fullPubdataHash != keccak256(_pubdata)) { - revert InvalidPubdataHash(); + revert InvalidPubdataHash(_fullPubdataHash, keccak256(_pubdata)); } blobCommitments[0] = bytes32(_pubdataInput[_pubdataInput.length - BLOB_COMMITMENT_SIZE:_pubdataInput.length]); } diff --git a/l1-contracts/contracts/state-transition/data-availability/CalldataDAGateway.sol b/l1-contracts/contracts/state-transition/data-availability/CalldataDAGateway.sol index 0a072638f..90ec4a3ba 100644 --- a/l1-contracts/contracts/state-transition/data-availability/CalldataDAGateway.sol +++ b/l1-contracts/contracts/state-transition/data-availability/CalldataDAGateway.sol @@ -29,7 +29,7 @@ abstract contract CalldataDAGateway is CalldataDA { revert PubdataTooLong(_pubdata.length, _blobsProvided * BLOB_SIZE_BYTES); } if (_fullPubdataHash != keccak256(_pubdata)) { - revert InvalidPubdataHash(); + revert InvalidPubdataHash(_fullPubdataHash, keccak256(_pubdata)); } bytes calldata providedCommitments = _pubdataInput[_pubdataInput.length - diff --git a/l1-contracts/test/foundry/l1/unit/concrete/state-transition/data-availability/CalldataDA.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/state-transition/data-availability/CalldataDA.t.sol index 7838ba693..255c2de60 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/state-transition/data-availability/CalldataDA.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/state-transition/data-availability/CalldataDA.t.sol @@ -155,7 +155,7 @@ contract CalldataDATest is Test { bytes memory pubdataInput = abi.encodePacked(pubdataInputWithoutBlobCommitment, blobCommitment); bytes32 fullPubdataHash = keccak256(pubdataInput); - vm.expectRevert(InvalidPubdataHash.selector); + vm.expectRevert(abi.encodeWithSelector(InvalidPubdataHash.selector, fullPubdataHash, keccak256(pubdataInputWithoutBlobCommitment))); calldataDA.processCalldataDA(blobsProvided, fullPubdataHash, maxBlobsSupported, pubdataInput); } diff --git a/l2-contracts/contracts/L2ContractHelper.sol b/l2-contracts/contracts/L2ContractHelper.sol index f367e33ab..bbb615327 100644 --- a/l2-contracts/contracts/L2ContractHelper.sol +++ b/l2-contracts/contracts/L2ContractHelper.sol @@ -22,7 +22,7 @@ interface IL2Messenger { /// @notice Sends an arbitrary length message to L1. /// @param _message The variable length message to be sent to L1. /// @return Returns the keccak256 hashed value of the message. - function sendToL1(bytes memory _message) external returns (bytes32); + function sendToL1(bytes calldata _message) external returns (bytes32); } /** diff --git a/system-contracts/contracts/interfaces/IL1Messenger.sol b/system-contracts/contracts/interfaces/IL1Messenger.sol index 88e2c81d8..51535d8d8 100644 --- a/system-contracts/contracts/interfaces/IL1Messenger.sol +++ b/system-contracts/contracts/interfaces/IL1Messenger.sol @@ -46,7 +46,7 @@ interface IL1Messenger { event BytecodeL1PublicationRequested(bytes32 _bytecodeHash); - function sendToL1(bytes memory _message) external returns (bytes32); + function sendToL1(bytes calldata _message) external returns (bytes32); function sendL2ToL1Log(bool _isService, bytes32 _key, bytes32 _value) external returns (uint256 logIdInMerkleTree); From b26a2e730e3a2c0d09776e085bec0fd87ed42d05 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 18:06:51 +0100 Subject: [PATCH 23/57] public -> external --- l1-contracts/contracts/upgrades/BytecodesSupplier.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/upgrades/BytecodesSupplier.sol b/l1-contracts/contracts/upgrades/BytecodesSupplier.sol index c27c1a1ea..4415ad5e0 100644 --- a/l1-contracts/contracts/upgrades/BytecodesSupplier.sol +++ b/l1-contracts/contracts/upgrades/BytecodesSupplier.sol @@ -34,7 +34,7 @@ contract BytecodesSupplier { /// @notice Publishes multiple bytecodes. /// @param _bytecodes Array of bytecodes to be published. - function publishBytecodes(bytes[] calldata _bytecodes) public { + function publishBytecodes(bytes[] calldata _bytecodes) external { // solhint-disable-next-line gas-length-in-loops for (uint256 i = 0; i < _bytecodes.length; ++i) { publishBytecode(_bytecodes[i]); From 8c00cbb2e743fe2bc1a5f8cb48abcd91d378748d Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 18:08:22 +0100 Subject: [PATCH 24/57] lint --- da-contracts/contracts/RollupL1DAValidator.sol | 4 +--- l1-contracts/contracts/bridgehub/MessageRoot.sol | 2 +- .../contracts/state-transition/ChainTypeManager.sol | 6 +++--- .../contracts/state-transition/libraries/PriorityTree.sol | 2 +- .../ChainTypeManager/SetNewVersionUpgrade.t.sol | 6 +----- .../state-transition/data-availability/CalldataDA.t.sol | 8 +++++++- .../contracts/libraries/SystemContractHelper.sol | 2 +- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/da-contracts/contracts/RollupL1DAValidator.sol b/da-contracts/contracts/RollupL1DAValidator.sol index 815343274..d03b8127e 100644 --- a/da-contracts/contracts/RollupL1DAValidator.sol +++ b/da-contracts/contracts/RollupL1DAValidator.sol @@ -149,9 +149,7 @@ contract RollupL1DAValidator is IL1DAValidator, CalldataDA { // we iterate over the `_operatorDAInput`, while advancing the pointer by `BLOB_DA_INPUT_SIZE` each time for (uint256 i = 0; i < _blobsProvided; ++i) { bytes calldata commitmentData = _operatorDAInput[:PUBDATA_COMMITMENT_SIZE]; - bytes32 prepublishedCommitment = bytes32( - _operatorDAInput[PUBDATA_COMMITMENT_SIZE:BLOB_DA_INPUT_SIZE] - ); + bytes32 prepublishedCommitment = bytes32(_operatorDAInput[PUBDATA_COMMITMENT_SIZE:BLOB_DA_INPUT_SIZE]); if (prepublishedCommitment != bytes32(0)) { // We double check that this commitment has indeed been published. diff --git a/l1-contracts/contracts/bridgehub/MessageRoot.sol b/l1-contracts/contracts/bridgehub/MessageRoot.sol index 110ba337d..25ca5e3e5 100644 --- a/l1-contracts/contracts/bridgehub/MessageRoot.sol +++ b/l1-contracts/contracts/bridgehub/MessageRoot.sol @@ -149,7 +149,7 @@ contract MessageRoot is IMessageRoot { function _addNewChain(uint256 _chainId) internal { uint256 cachedChainCount = chainCount; - // Since only the bridgehub can add new chains to the message root, it is expected that + // Since only the bridgehub can add new chains to the message root, it is expected that // it will be responsible for ensuring that the number of chains does not exceed the limit. ++chainCount; chainIndex[_chainId] = cachedChainCount; diff --git a/l1-contracts/contracts/state-transition/ChainTypeManager.sol b/l1-contracts/contracts/state-transition/ChainTypeManager.sol index b17e01992..e7dea6220 100644 --- a/l1-contracts/contracts/state-transition/ChainTypeManager.sol +++ b/l1-contracts/contracts/state-transition/ChainTypeManager.sol @@ -228,9 +228,9 @@ contract ChainTypeManager is IChainTypeManager, Ownable2StepUpgradeable { uint256 _oldProtocolVersionDeadline, uint256 _newProtocolVersion ) external onlyOwner { - if(!IBridgehub(BRIDGE_HUB).migrationPaused()){ - revert MigrationsNotPaused(); - } + if (!IBridgehub(BRIDGE_HUB).migrationPaused()) { + revert MigrationsNotPaused(); + } bytes32 newCutHash = keccak256(abi.encode(_cutData)); uint256 previousProtocolVersion = protocolVersion; diff --git a/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol b/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol index 398de6fd0..b39eed029 100644 --- a/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol +++ b/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol @@ -68,7 +68,7 @@ library PriorityTree { /// @notice Process the priority operations of a batch. /// @dev Note, that the function below only checks that a certain segment of items is present in the tree. - /// It does not check that e.g. there are no zero items inside the provided `itemHashes`, so in theory proofs + /// It does not check that e.g. there are no zero items inside the provided `itemHashes`, so in theory proofs /// that include non-existing priority operations could be created. This function relies on the fact /// that the `itemHashes` of `_priorityOpsData` are hashes of valid priority transactions. /// This fact is ensures by the fact the rolling hash of those is sent to the Executor by the bootloader diff --git a/l1-contracts/test/foundry/l1/unit/concrete/state-transition/ChainTypeManager/SetNewVersionUpgrade.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/state-transition/ChainTypeManager/SetNewVersionUpgrade.t.sol index 014deded1..c62773bab 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/state-transition/ChainTypeManager/SetNewVersionUpgrade.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/state-transition/ChainTypeManager/SetNewVersionUpgrade.t.sol @@ -11,11 +11,7 @@ contract setNewVersionUpgradeTest is ChainTypeManagerTest { function test_SettingNewVersionUpgrade() public { assertEq(chainContractAddress.protocolVersion(), 0, "Initial protocol version is not correct"); - vm.mockCall( - address(bridgehub), - abi.encodeWithSelector(bridgehub.migrationPaused.selector), - abi.encode(true) - ); + vm.mockCall(address(bridgehub), abi.encodeWithSelector(bridgehub.migrationPaused.selector), abi.encode(true)); address randomDiamondInit = address(0x303030303030303030303); Diamond.DiamondCutData memory newDiamondCutData = getDiamondCutData(address(randomDiamondInit)); diff --git a/l1-contracts/test/foundry/l1/unit/concrete/state-transition/data-availability/CalldataDA.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/state-transition/data-availability/CalldataDA.t.sol index 255c2de60..fe510176a 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/state-transition/data-availability/CalldataDA.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/state-transition/data-availability/CalldataDA.t.sol @@ -155,7 +155,13 @@ contract CalldataDATest is Test { bytes memory pubdataInput = abi.encodePacked(pubdataInputWithoutBlobCommitment, blobCommitment); bytes32 fullPubdataHash = keccak256(pubdataInput); - vm.expectRevert(abi.encodeWithSelector(InvalidPubdataHash.selector, fullPubdataHash, keccak256(pubdataInputWithoutBlobCommitment))); + vm.expectRevert( + abi.encodeWithSelector( + InvalidPubdataHash.selector, + fullPubdataHash, + keccak256(pubdataInputWithoutBlobCommitment) + ) + ); calldataDA.processCalldataDA(blobsProvided, fullPubdataHash, maxBlobsSupported, pubdataInput); } diff --git a/system-contracts/contracts/libraries/SystemContractHelper.sol b/system-contracts/contracts/libraries/SystemContractHelper.sol index e901af3d4..e53fe1145 100644 --- a/system-contracts/contracts/libraries/SystemContractHelper.sol +++ b/system-contracts/contracts/libraries/SystemContractHelper.sol @@ -436,7 +436,7 @@ library SystemContractHelper { /// 1. Force-deploy `SloadContract` to the address. /// 2. Read the required slot. /// 3. Force-deploy the previous bytecode back. - /// @dev Note, that the function will overwrite the account states of the `_addr`, i.e. + /// @dev Note, that the function will overwrite the account states of the `_addr`, i.e. /// this function should NEVER be used against custom accounts. function forcedSload(address _addr, bytes32 _key) internal returns (bytes32 result) { bytes32 sloadContractBytecodeHash; From 750fbfce88e72418a1aaaaff9d35da39711bed27 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 18:36:36 +0100 Subject: [PATCH 25/57] detach pricing from is permanent rollup --- .../state-transition/chain-deps/ZKChainStorage.sol | 3 ++- .../state-transition/chain-deps/facets/Admin.sol | 9 --------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/l1-contracts/contracts/state-transition/chain-deps/ZKChainStorage.sol b/l1-contracts/contracts/state-transition/chain-deps/ZKChainStorage.sol index 4d3d2f4ff..1ad3d72ce 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/ZKChainStorage.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/ZKChainStorage.sol @@ -169,6 +169,7 @@ struct ZKChainStorage { address settlementLayer; /// @dev Priority tree, the new data structure for priority queue PriorityTree.Tree priorityTree; - /// @dev Whether the chain is a permanent rollup + /// @dev Whether the chain is a permanent rollup. Note, that it only enforces the DA validator pair, but + /// it does not enforce any other parameters, e.g. `pubdataPricingMode` bool isPermanentRollup; } diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol index 9acf3ac3e..c65836f9e 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol @@ -132,10 +132,6 @@ contract AdminFacet is ZKChainBase, IAdmin { /// @inheritdoc IAdmin function setPubdataPricingMode(PubdataPricingMode _pricingMode) external onlyAdmin onlyL1 { - if (s.isPermanentRollup && _pricingMode != PubdataPricingMode.Rollup) { - revert IncorrectPricingMode(); - } - s.feeParams.pubdataPricingMode = _pricingMode; emit PubdataPricingModeUpdaate(_pricingMode); } @@ -185,11 +181,6 @@ contract AdminFacet is ZKChainBase, IAdmin { revert InvalidDAForPermanentRollup(); } - if (s.feeParams.pubdataPricingMode != PubdataPricingMode.Rollup) { - // The correct pubdata pricing mode should be set beforehand. - revert IncorrectPricingMode(); - } - s.isPermanentRollup = true; } From b0ce40bc7d57de0fed688b51889119a8fdf5fa38 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 18:40:50 +0100 Subject: [PATCH 26/57] fix lint --- .../contracts/state-transition/chain-deps/ZKChainStorage.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/state-transition/chain-deps/ZKChainStorage.sol b/l1-contracts/contracts/state-transition/chain-deps/ZKChainStorage.sol index 1ad3d72ce..b390cf752 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/ZKChainStorage.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/ZKChainStorage.sol @@ -169,7 +169,7 @@ struct ZKChainStorage { address settlementLayer; /// @dev Priority tree, the new data structure for priority queue PriorityTree.Tree priorityTree; - /// @dev Whether the chain is a permanent rollup. Note, that it only enforces the DA validator pair, but + /// @dev Whether the chain is a permanent rollup. Note, that it only enforces the DA validator pair, but /// it does not enforce any other parameters, e.g. `pubdataPricingMode` bool isPermanentRollup; } From d9a27170401d769b20146037e8f249eb9eb1f708 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 18:42:27 +0100 Subject: [PATCH 27/57] remove unused error --- l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol | 3 --- l1-contracts/contracts/common/L1ContractErrors.sol | 2 -- .../contracts/state-transition/chain-deps/facets/Admin.sol | 2 +- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol b/l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol index 2418e5b1d..9481f2ff8 100644 --- a/l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol +++ b/l1-contracts/contracts/bridgehub/L1BridgehubErrors.sol @@ -53,9 +53,6 @@ error ChainExists(); // 0x913183d8 error MessageRootNotRegistered(); -// 0x8b7d939c -error TooManyChains(uint256 cachedChainCount, uint256 maxNumberOfChains); - // 0x7f4316f3 error NoEthAllowed(); diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 1cd8c3186..58bfbc458 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -330,8 +330,6 @@ error NewDeadlineExceedsMaxDeadline(); error AlreadyPermanentRollup(); // 0x92daded2 error InvalidDAForPermanentRollup(); -// 0x6e3331f5 -error IncorrectPricingMode(); // 0xd0266e26 error NotSettlementLayer(); // 0x7a4902ad diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol index c65836f9e..fa7508743 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol @@ -11,7 +11,7 @@ import {PriorityQueue} from "../../../state-transition/libraries/PriorityQueue.s import {ZKChainBase} from "./ZKChainBase.sol"; import {IChainTypeManager} from "../../IChainTypeManager.sol"; import {IL1GenesisUpgrade} from "../../../upgrades/IL1GenesisUpgrade.sol"; -import {Unauthorized, TooMuchGas, PriorityTxPubdataExceedsMaxPubDataPerBatch, InvalidPubdataPricingMode, ProtocolIdMismatch, HashMismatch, ProtocolIdNotGreater, DenominatorIsZero, DiamondAlreadyFrozen, DiamondNotFrozen, IncorrectPricingMode, InvalidDAForPermanentRollup, AlreadyPermanentRollup} from "../../../common/L1ContractErrors.sol"; +import {Unauthorized, TooMuchGas, PriorityTxPubdataExceedsMaxPubDataPerBatch, InvalidPubdataPricingMode, ProtocolIdMismatch, HashMismatch, ProtocolIdNotGreater, DenominatorIsZero, DiamondAlreadyFrozen, DiamondNotFrozen, InvalidDAForPermanentRollup, AlreadyPermanentRollup} from "../../../common/L1ContractErrors.sol"; import {NotL1, L1DAValidatorAddressIsZero, L2DAValidatorAddressIsZero, AlreadyMigrated, NotChainAdmin, ProtocolVersionNotUpToDate, ExecutedIsNotConsistentWithVerified, VerifiedIsNotConsistentWithCommitted, InvalidNumberOfBatchHashes, PriorityQueueNotReady, VerifiedIsNotConsistentWithCommitted, NotAllBatchesExecuted, OutdatedProtocolVersion, NotHistoricalRoot, ContractNotDeployed, NotMigrated} from "../../L1StateTransitionErrors.sol"; import {RollupDAManager} from "../../data-availability/RollupDAManager.sol"; From 502ee80e230548b3e72bcf9416e37c5419d3c213 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Mon, 9 Dec 2024 21:54:06 +0100 Subject: [PATCH 28/57] Support quick registration of NTV assets (#56) For better UX: - Allow NTV have bridgeBurnData with optional last tokenAddress. - If the `tokenAddress` is non zero, the NTV may attempt to register the token if asset handler is not yet present --- l1-contracts/contracts/bridge/L1Nullifier.sol | 9 +- .../bridge/asset-router/AssetRouterBase.sol | 20 +++- .../bridge/asset-router/IL2AssetRouter.sol | 2 - .../bridge/asset-router/L1AssetRouter.sol | 30 ++++-- .../bridge/asset-router/L2AssetRouter.sol | 30 ++---- .../bridge/ntv/INativeTokenVault.sol | 3 + .../bridge/ntv/L1NativeTokenVault.sol | 18 +++- .../bridge/ntv/L2NativeTokenVault.sol | 36 ++++++- .../contracts/bridge/ntv/NativeTokenVault.sol | 102 ++++++++++++++---- .../contracts/common/L1ContractErrors.sol | 3 + .../common/libraries/DataEncoding.sol | 39 ++++++- l1-contracts/deploy-scripts/Utils.sol | 30 +++--- .../upgrade/EcosystemUpgrade.s.sol | 16 ++- .../upgrade/FinalizeUpgrade.s.sol | 35 +++--- .../l1/integration/AssetRouterTest.t.sol | 2 +- .../L2Erc20TestAbstract.t.sol | 10 +- .../L1SharedBridge/L1SharedBridgeBase.t.sol | 4 +- .../L1SharedBridge/L1SharedBridgeFails.t.sol | 4 +- 18 files changed, 284 insertions(+), 109 deletions(-) diff --git a/l1-contracts/contracts/bridge/L1Nullifier.sol b/l1-contracts/contracts/bridge/L1Nullifier.sol index defde97c7..a4bd5f489 100644 --- a/l1-contracts/contracts/bridge/L1Nullifier.sol +++ b/l1-contracts/contracts/bridge/L1Nullifier.sol @@ -646,8 +646,8 @@ contract L1Nullifier is IL1Nullifier, ReentrancyGuard, Ownable2StepUpgradeable, assetId = DataEncoding.encodeNTVAssetId(block.chainid, _l1Token); } // For legacy deposits, the l2 receiver is not required to check tx data hash - // bytes memory transferData = abi.encode(_amount, _depositSender); - bytes memory assetData = abi.encode(_amount, address(0)); + // The token address does not have to be provided for this functionality either. + bytes memory assetData = DataEncoding.encodeBridgeBurnData(_amount, address(0), address(0)); _verifyAndClearFailedTransfer({ _checkedInLegacyBridge: false, @@ -696,7 +696,10 @@ contract L1Nullifier is IL1Nullifier, ReentrancyGuard, Ownable2StepUpgradeable, uint16 _l2TxNumberInBatch, bytes32[] calldata _merkleProof ) external override onlyLegacyBridge { - bytes memory assetData = abi.encode(_amount, _depositSender); + // For legacy deposits, the l2 receiver is not required to check tx data hash + // The token address does not have to be provided for this functionality either. + bytes memory assetData = DataEncoding.encodeBridgeBurnData(_amount, address(0), address(0)); + /// the legacy bridge can only be used with L1 native tokens. bytes32 assetId = DataEncoding.encodeNTVAssetId(block.chainid, _l1Asset); diff --git a/l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol b/l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol index c57c3a8f9..854c86b18 100644 --- a/l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol +++ b/l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol @@ -15,7 +15,8 @@ import {DataEncoding} from "../../common/libraries/DataEncoding.sol"; import {L2_NATIVE_TOKEN_VAULT_ADDR} from "../../common/L2ContractAddresses.sol"; import {IBridgehub} from "../../bridgehub/IBridgehub.sol"; -import {Unauthorized, AssetHandlerDoesNotExist} from "../../common/L1ContractErrors.sol"; +import {Unauthorized} from "../../common/L1ContractErrors.sol"; +import {INativeTokenVault} from "../ntv/INativeTokenVault.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -133,6 +134,7 @@ abstract contract AssetRouterBase is IAssetRouterBase, Ownable2StepUpgradeable, /// @param _originalCaller The `msg.sender` address from the external call that initiated current one. /// @param _transferData The encoded data, which is used by the asset handler to determine L2 recipient and amount. Might include extra information. /// @param _passValue Boolean indicating whether to pass msg.value in the call. + /// @param _nativeTokenVault The address of the native token vault. /// @return bridgeMintCalldata The calldata used by remote asset handler to mint tokens for recipient. function _burn( uint256 _chainId, @@ -140,11 +142,23 @@ abstract contract AssetRouterBase is IAssetRouterBase, Ownable2StepUpgradeable, bytes32 _assetId, address _originalCaller, bytes memory _transferData, - bool _passValue + bool _passValue, + address _nativeTokenVault ) internal returns (bytes memory bridgeMintCalldata) { address l1AssetHandler = assetHandlerAddress[_assetId]; if (l1AssetHandler == address(0)) { - revert AssetHandlerDoesNotExist(_assetId); + // As a UX feature, whenever an asset handler is not present, we always try to register asset within native token vault. + // The Native Token Vault is trusted to revert in an asset does not belong to it. + // + // Note, that it may "pollute" error handling a bit: instead of getting error for asset handler not being + // present, the user will get whatever error the native token vault will return, however, providing + // more advanced error handling requires more extensive code and will be added in the future releases. + INativeTokenVault(_nativeTokenVault).tryRegisterTokenFromBurnData(_transferData, _assetId); + + // We do not do any additional transformations here (like setting `assetHandler` in the mapping), + // because we expect that all those happened inside `tryRegisterTokenFromBurnData` + + l1AssetHandler = _nativeTokenVault; } uint256 msgValue = _passValue ? msg.value : 0; diff --git a/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol index 1839a4e77..8867c8856 100644 --- a/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol @@ -36,6 +36,4 @@ interface IL2AssetRouter is IAssetRouterBase { /// a legacy asset. /// @param _assetId The assetId of the legacy token. function setLegacyTokenAssetHandler(bytes32 _assetId) external; - - function withdrawToken(address _l2NativeToken, bytes memory _assetData) external returns (bytes32); } diff --git a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol index f052cb28b..e8a8ded18 100644 --- a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol @@ -215,7 +215,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { _msgValue: 0, _assetId: _assetId, _originalCaller: _originalCaller, - _data: abi.encode(_amount, address(0)) + _data: DataEncoding.encodeBridgeBurnData(_amount, address(0), address(0)) }); // Note that we don't save the deposited amount, as this is for the base token, which gets sent to the refundRecipient if the tx fails @@ -267,17 +267,20 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { revert AssetIdNotSupported(assetId); } + address ntvCached = address(nativeTokenVault); + bytes memory bridgeMintCalldata = _burn({ _chainId: _chainId, _nextMsgValue: _value, _assetId: assetId, _originalCaller: _originalCaller, _transferData: transferData, - _passValue: true + _passValue: true, + _nativeTokenVault: ntvCached }); bytes32 txDataHash = DataEncoding.encodeTxDataHash({ - _nativeTokenVault: address(nativeTokenVault), + _nativeTokenVault: ntvCached, _encodingVersion: encodingVersion, _originalCaller: _originalCaller, _assetId: assetId, @@ -392,7 +395,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { } } - return (assetId, abi.encode(_depositAmount, _l2Receiver)); + return (assetId, DataEncoding.encodeBridgeBurnData(_depositAmount, _l2Receiver, _l1Token)); } /// @notice Ensures that token is registered with native token vault. @@ -526,7 +529,12 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { bytes32 _assetId; { - bytes memory bridgeMintCalldata; + // Note, that to keep the code simple, while avoiding "stack too deep" error, + // this `bridgeData` variable is reused in two places with different meanings: + // - Firstly, it denotes the bridgeBurn data to be used for the NativeTokenVault + // - Secondly, after the call to `_burn` function, it denotes the `bridgeMint` data that + // will be sent to the L2 counterpart of the L1NTV. + bytes memory bridgeData = DataEncoding.encodeBridgeBurnData(_amount, _l2Receiver, _l1Token); // Inner call to encode data to decrease local var numbers _assetId = _ensureTokenRegisteredWithNTV(_l1Token); // Legacy bridge is only expected to use native tokens for L1. @@ -536,16 +544,18 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { IERC20(_l1Token).forceApprove(address(nativeTokenVault), _amount); - bridgeMintCalldata = _burn({ + // Note, that starting from here `bridgeData` starts denoting bridgeMintData. + bridgeData = _burn({ _chainId: ERA_CHAIN_ID, _nextMsgValue: 0, _assetId: _assetId, _originalCaller: _originalCaller, - _transferData: abi.encode(_amount, _l2Receiver), - _passValue: false + _transferData: bridgeData, + _passValue: false, + _nativeTokenVault: address(nativeTokenVault) }); - bytes memory l2TxCalldata = getDepositCalldata(_originalCaller, _assetId, bridgeMintCalldata); + bytes memory l2TxCalldata = getDepositCalldata(_originalCaller, _assetId, bridgeData); // If the refund recipient is not specified, the refund will be sent to the sender of the transaction. // Otherwise, the refund will be sent to the specified address. @@ -567,7 +577,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { } { - bytes memory transferData = abi.encode(_amount, _l2Receiver); + bytes memory transferData = DataEncoding.encodeBridgeBurnData(_amount, _l2Receiver, _l1Token); // Save the deposited amount to claim funds on L1 if the deposit failed on L2 L1_NULLIFIER.bridgehubConfirmL2TransactionForwarded( ERA_CHAIN_ID, diff --git a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol index 382524147..0339829e6 100644 --- a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol @@ -7,9 +7,7 @@ import {IAssetRouterBase} from "./IAssetRouterBase.sol"; import {AssetRouterBase} from "./AssetRouterBase.sol"; import {IL2NativeTokenVault} from "../ntv/IL2NativeTokenVault.sol"; -import {INativeTokenVault} from "../ntv/INativeTokenVault.sol"; import {IL2SharedBridgeLegacy} from "../interfaces/IL2SharedBridgeLegacy.sol"; -import {IAssetHandler} from "../interfaces/IAssetHandler.sol"; import {IBridgedStandardToken} from "../interfaces/IBridgedStandardToken.sol"; import {IL1ERC20Bridge} from "../interfaces/IL1ERC20Bridge.sol"; @@ -152,18 +150,6 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { return _withdrawSender(_assetId, _assetData, msg.sender, true); } - /// @dev IMPORTANT: this method will be deprecated in one of the future releases, so contracts - /// that rely on it must be upgradeable. - function withdrawToken(address _l2NativeToken, bytes memory _assetData) public returns (bytes32) { - bytes32 recordedAssetId = INativeTokenVault(L2_NATIVE_TOKEN_VAULT_ADDR).assetId(_l2NativeToken); - uint256 recordedOriginChainId = INativeTokenVault(L2_NATIVE_TOKEN_VAULT_ADDR).originChainId(recordedAssetId); - if (recordedOriginChainId != block.chainid && recordedOriginChainId != 0) { - revert AssetIdNotSupported(recordedAssetId); - } - bytes32 assetId = _ensureTokenRegisteredWithNTV(_l2NativeToken); - return _withdrawSender(assetId, _assetData, msg.sender, true); - } - /*////////////////////////////////////////////////////////////// Internal & Helpers //////////////////////////////////////////////////////////////*/ @@ -185,18 +171,19 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { address _sender, bool _alwaysNewMessageFormat ) internal returns (bytes32 txHash) { - address assetHandler = assetHandlerAddress[_assetId]; - bytes memory _l1bridgeMintData = IAssetHandler(assetHandler).bridgeBurn({ + bytes memory l1bridgeMintData = _burn({ _chainId: L1_CHAIN_ID, - _msgValue: 0, + _nextMsgValue: 0, _assetId: _assetId, _originalCaller: _sender, - _data: _assetData + _transferData: _assetData, + _passValue: false, + _nativeTokenVault: L2_NATIVE_TOKEN_VAULT_ADDR }); bytes memory message; if (_alwaysNewMessageFormat || L2_LEGACY_SHARED_BRIDGE == address(0)) { - message = _getAssetRouterWithdrawMessage(_assetId, _l1bridgeMintData); + message = _getAssetRouterWithdrawMessage(_assetId, l1bridgeMintData); // slither-disable-next-line unused-return txHash = L2ContractHelper.sendMessageToL1(message); } else { @@ -206,7 +193,8 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { if (l1Token == address(0)) { revert AssetIdNotSupported(_assetId); } - (uint256 amount, address l1Receiver) = abi.decode(_assetData, (uint256, address)); + // slither-disable-next-line unused-return + (uint256 amount, address l1Receiver, ) = DataEncoding.decodeBridgeBurnData(_assetData); message = _getSharedBridgeWithdrawMessage(l1Receiver, l1Token, amount); txHash = IL2SharedBridgeLegacy(L2_LEGACY_SHARED_BRIDGE).sendMessageToL1(message); } @@ -325,7 +313,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { revert TokenNotLegacy(); } bytes32 assetId = DataEncoding.encodeNTVAssetId(L1_CHAIN_ID, l1Address); - bytes memory data = abi.encode(_amount, _l1Receiver); + bytes memory data = DataEncoding.encodeBridgeBurnData(_amount, _l1Receiver, _l2Token); _withdrawSender(assetId, data, _sender, false); } diff --git a/l1-contracts/contracts/bridge/ntv/INativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/INativeTokenVault.sol index 12718bd6f..39ca0e20f 100644 --- a/l1-contracts/contracts/bridge/ntv/INativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/INativeTokenVault.sol @@ -44,4 +44,7 @@ interface INativeTokenVault { /// @notice Used to get the expected bridged token address corresponding to its native counterpart function calculateCreate2TokenAddress(uint256 _originChainId, address _originToken) external view returns (address); + + /// @notice Tries to register a token from the provided `_burnData` and reverts if it is not possible. + function tryRegisterTokenFromBurnData(bytes calldata _burnData, bytes32 _expectedAssetId) external; } diff --git a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol index 9066691a1..e93511b7b 100644 --- a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol @@ -164,10 +164,10 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken address _originalCaller, // solhint-disable-next-line no-unused-vars bool _depositChecked, - bytes calldata _data + uint256 _depositAmount, + address _receiver, + address _nativeToken ) internal override returns (bytes memory _bridgeMintData) { - uint256 _depositAmount; - (_depositAmount, ) = abi.decode(_data, (uint256, address)); bool depositChecked = IL1AssetRouter(address(ASSET_ROUTER)).transferFundsToNTV( _assetId, _depositAmount, @@ -178,7 +178,9 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken _assetId: _assetId, _originalCaller: _originalCaller, _depositChecked: depositChecked, - _data: _data + _depositAmount: _depositAmount, + _receiver: _receiver, + _nativeToken: _nativeToken }); } @@ -193,7 +195,8 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken address _depositSender, bytes calldata _data ) external payable override requireZeroValue(msg.value) onlyAssetRouter whenNotPaused { - (uint256 _amount, ) = abi.decode(_data, (uint256, address)); + // slither-disable-next-line unused-return + (uint256 _amount, , ) = DataEncoding.decodeBridgeBurnData(_data); address l1Token = tokenAddress[_assetId]; if (_amount == 0) { revert NoFundsTransferred(); @@ -228,6 +231,11 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken INTERNAL & HELPER FUNCTIONS //////////////////////////////////////////////////////////////*/ + function _registerTokenIfBridgedLegacy(address) internal override returns (bytes32) { + // There are no legacy tokens present on L1. + return bytes32(0); + } + // get the computed address before the contract DeployWithCreate2 deployed using Bytecode of contract DeployWithCreate2 and salt specified by the sender function calculateCreate2TokenAddress( uint256 _originChainId, diff --git a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol index 4a822719a..af4708169 100644 --- a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol @@ -23,7 +23,7 @@ import {L2ContractHelper, IContractDeployer} from "../../common/libraries/L2Cont import {SystemContractsCaller} from "../../common/libraries/SystemContractsCaller.sol"; import {DataEncoding} from "../../common/libraries/DataEncoding.sol"; -import {NoLegacySharedBridge, TokenIsLegacy, TokenIsNotLegacy, EmptyAddress, EmptyBytes32, AddressMismatch, DeployFailed, AssetIdNotSupported} from "../../common/L1ContractErrors.sol"; +import {AssetIdAlreadyRegistered, NoLegacySharedBridge, TokenIsLegacy, TokenIsNotLegacy, EmptyAddress, EmptyBytes32, AddressMismatch, DeployFailed, AssetIdNotSupported} from "../../common/L1ContractErrors.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -84,8 +84,29 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { } } + function _registerTokenIfBridgedLegacy(address _tokenAddress) internal override returns (bytes32) { + // In zkEVM immutables are stored in a storage of a system contract, + // so it makes sense to cache them for efficiency. + IL2SharedBridgeLegacy legacyBridge = L2_LEGACY_SHARED_BRIDGE; + if (address(legacyBridge) == address(0)) { + // No legacy bridge, the token must be native + return bytes32(0); + } + + address l1TokenAddress = legacyBridge.l1TokenAddress(_tokenAddress); + if (l1TokenAddress == address(0)) { + // The token is not legacy + return bytes32(0); + } + + return _registerLegacyTokenAssetId(_tokenAddress, l1TokenAddress); + } + /// @notice Sets the legacy token asset ID for the given L2 token address. function setLegacyTokenAssetId(address _l2TokenAddress) public { + if (assetId[_l2TokenAddress] != bytes32(0)) { + revert AssetIdAlreadyRegistered(); + } if (address(L2_LEGACY_SHARED_BRIDGE) == address(0)) { revert NoLegacySharedBridge(); } @@ -93,8 +114,15 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { if (l1TokenAddress == address(0)) { revert TokenIsNotLegacy(); } - bytes32 newAssetId = DataEncoding.encodeNTVAssetId(L1_CHAIN_ID, l1TokenAddress); + _registerLegacyTokenAssetId(_l2TokenAddress, l1TokenAddress); + } + + function _registerLegacyTokenAssetId( + address _l2TokenAddress, + address _l1TokenAddress + ) internal returns (bytes32 newAssetId) { + newAssetId = DataEncoding.encodeNTVAssetId(L1_CHAIN_ID, _l1TokenAddress); IL2AssetRouter(L2_ASSET_ROUTER_ADDR).setLegacyTokenAssetHandler(newAssetId); tokenAddress[newAssetId] = _l2TokenAddress; assetId[_l2TokenAddress] = newAssetId; @@ -253,7 +281,7 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { // on L2s we don't track the balance } - function _registerToken(address _nativeToken) internal override { + function _registerToken(address _nativeToken) internal override returns (bytes32) { if ( address(L2_LEGACY_SHARED_BRIDGE) != address(0) && L2_LEGACY_SHARED_BRIDGE.l1TokenAddress(_nativeToken) != address(0) @@ -261,7 +289,7 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { // Legacy tokens should be registered via `setLegacyTokenAssetId`. revert TokenIsLegacy(); } - super._registerToken(_nativeToken); + return super._registerToken(_nativeToken); } /*////////////////////////////////////////////////////////////// diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index f760660f2..65b048e3d 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -93,7 +93,7 @@ abstract contract NativeTokenVault is _registerToken(_nativeToken); } - function _registerToken(address _nativeToken) internal virtual { + function _registerToken(address _nativeToken) internal virtual returns (bytes32 newAssetId) { // We allow registering `WETH_TOKEN` inside `NativeTokenVault` only for L1 native token vault. // It is needed to allow withdrawing such assets. We restrict all WETH-related // operations to deposits from L1 only to be able to upgrade their logic more easily in the @@ -107,7 +107,7 @@ abstract contract NativeTokenVault is if (assetId[_nativeToken] != bytes32(0)) { revert AssetIdAlreadyRegistered(); } - _unsafeRegisterNativeToken(_nativeToken); + newAssetId = _unsafeRegisterNativeToken(_nativeToken); } /// @inheritdoc INativeTokenVault @@ -200,33 +200,98 @@ abstract contract NativeTokenVault is whenNotPaused returns (bytes memory _bridgeMintData) { + (uint256 amount, address receiver, address tokenAddress) = _decodeBurnAndCheckAssetId(_data, _assetId); if (originChainId[_assetId] != block.chainid) { - _bridgeMintData = _bridgeBurnBridgedToken(_chainId, _assetId, _originalCaller, _data); + _bridgeMintData = _bridgeBurnBridgedToken({ + _chainId: _chainId, + _assetId: _assetId, + _originalCaller: _originalCaller, + _amount: amount, + _receiver: receiver, + _tokenAddress: tokenAddress + }); } else { _bridgeMintData = _bridgeBurnNativeToken({ _chainId: _chainId, _assetId: _assetId, _originalCaller: _originalCaller, _depositChecked: false, - _data: _data + _depositAmount: amount, + _receiver: receiver, + _nativeToken: tokenAddress }); } } + function tryRegisterTokenFromBurnData(bytes calldata _data, bytes32 _expectedAssetId) external { + (uint256 amount, address receiver, address tokenAddress) = DataEncoding.decodeBridgeBurnData(_data); + + if (tokenAddress == address(0)) { + revert ZeroAddress(); + } + + bytes32 storedAssetId = assetId[tokenAddress]; + if (storedAssetId != bytes32(0)) { + revert AssetIdAlreadyRegistered(); + } + + // This token has not been registered within this NTV yet. Usually this means that the + // token is native to the chain and the user would prefer to get it registered as such. + // However, there are exceptions (e.g. bridged legacy ERC20 tokens on L2) when the + // assetId has not been stored yet. We will ask the implementor to double check that the token + // is not legacy. + + // We try to register it as legacy token. If it fails, we know + // it is a native one and so register it as a native token. + bytes32 newAssetId = _registerTokenIfBridgedLegacy(tokenAddress); + if (newAssetId == bytes32(0)) { + newAssetId = _registerToken(tokenAddress); + } + + if (newAssetId != _expectedAssetId) { + revert AssetIdMismatch(_expectedAssetId, newAssetId); + } + } + + function _decodeBurnAndCheckAssetId( + bytes calldata _data, + bytes32 _suppliedAssetId + ) internal returns (uint256 amount, address receiver, address parsedTokenAddress) { + (amount, receiver, parsedTokenAddress) = DataEncoding.decodeBridgeBurnData(_data); + + if (parsedTokenAddress == address(0)) { + // This means that the user wants the native token vault to resolve the + // address. In this case, it is assumed that the assetId is already registered. + parsedTokenAddress = tokenAddress[_suppliedAssetId]; + } + + // If it is still zero, it means that the token has not been registered. + if (parsedTokenAddress == address(0)) { + revert ZeroAddress(); + } + + bytes32 storedAssetId = assetId[parsedTokenAddress]; + if (_suppliedAssetId != storedAssetId) { + revert AssetIdMismatch(storedAssetId, _suppliedAssetId); + } + } + + function _registerTokenIfBridgedLegacy(address _token) internal virtual returns (bytes32); + function _bridgeBurnBridgedToken( uint256 _chainId, bytes32 _assetId, address _originalCaller, - bytes calldata _data + uint256 _amount, + address _receiver, + address _tokenAddress ) internal requireZeroValue(msg.value) returns (bytes memory _bridgeMintData) { - (uint256 _amount, address _receiver) = abi.decode(_data, (uint256, address)); if (_amount == 0) { // "Amount cannot be zero"); revert AmountMustBeGreaterThanZero(); } - address bridgedToken = tokenAddress[_assetId]; - IBridgedStandardToken(bridgedToken).bridgeBurn(_originalCaller, _amount); + IBridgedStandardToken(_tokenAddress).bridgeBurn(_originalCaller, _amount); _handleChainBalanceIncrease(_chainId, _assetId, _amount, false); emit BridgeBurn({ @@ -244,11 +309,11 @@ abstract contract NativeTokenVault is if (originChainId == 0) { revert ZeroAddress(); } - erc20Metadata = getERC20Getters(bridgedToken, originChainId); + erc20Metadata = getERC20Getters(_tokenAddress, originChainId); } address originToken; { - originToken = IBridgedStandardToken(bridgedToken).originToken(); + originToken = IBridgedStandardToken(_tokenAddress).originToken(); if (originToken == address(0)) { revert ZeroAddress(); } @@ -268,16 +333,17 @@ abstract contract NativeTokenVault is bytes32 _assetId, address _originalCaller, bool _depositChecked, - bytes calldata _data + uint256 _depositAmount, + address _receiver, + address _nativeToken ) internal virtual returns (bytes memory _bridgeMintData) { - (uint256 _depositAmount, address _receiver) = abi.decode(_data, (uint256, address)); - address nativeToken = tokenAddress[_assetId]; if (nativeToken == WETH_TOKEN) { // This ensures that WETH_TOKEN can never be bridged from chains it is native to. // It can only be withdrawn from the chain where it has already gotten. revert BurningNativeWETHNotSupported(); } + if (_assetId == BASE_TOKEN_ASSET_ID) { if (_depositAmount != msg.value) { revert ValueMismatch(_depositAmount, msg.value); @@ -291,7 +357,7 @@ abstract contract NativeTokenVault is } _handleChainBalanceIncrease(_chainId, _assetId, _depositAmount, true); if (!_depositChecked) { - uint256 expectedDepositAmount = _depositFunds(_originalCaller, IERC20(nativeToken), _depositAmount); // note if _originalCaller is this contract, this will return 0. This does not happen. + uint256 expectedDepositAmount = _depositFunds(_originalCaller, IERC20(_nativeToken), _depositAmount); // note if _originalCaller is this contract, this will return 0. This does not happen. // The token has non-standard transfer logic if (_depositAmount != expectedDepositAmount) { revert TokensWithFeesNotSupported(); @@ -305,12 +371,12 @@ abstract contract NativeTokenVault is bytes memory erc20Metadata; { - erc20Metadata = getERC20Getters(nativeToken, originChainId[_assetId]); + erc20Metadata = getERC20Getters(_nativeToken, originChainId[_assetId]); } _bridgeMintData = DataEncoding.encodeBridgeMintData({ _originalCaller: _originalCaller, _remoteReceiver: _receiver, - _originToken: nativeToken, + _originToken: _nativeToken, _amount: _depositAmount, _erc20Metadata: erc20Metadata }); @@ -351,8 +417,8 @@ abstract contract NativeTokenVault is /// @notice Registers a native token address for the vault. /// @dev It does not perform any checks for the correctnesss of the token contract. /// @param _nativeToken The address of the token to be registered. - function _unsafeRegisterNativeToken(address _nativeToken) internal { - bytes32 newAssetId = DataEncoding.encodeNTVAssetId(block.chainid, _nativeToken); + function _unsafeRegisterNativeToken(address _nativeToken) internal returns (bytes32 newAssetId) { + newAssetId = DataEncoding.encodeNTVAssetId(block.chainid, _nativeToken); tokenAddress[newAssetId] = _nativeToken; assetId[_nativeToken] = newAssetId; originChainId[newAssetId] = block.chainid; diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 2cc93bc28..06aaf0fda 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -377,6 +377,9 @@ error NoLegacySharedBridge(); // 0x78d2ed02 error ChainAlreadyLive(); +// 0xde4c0b96 +error InvalidNTVBurnData(); + enum SharedBridgeKey { PostUpgradeFirstBatch, LegacyBridgeFirstBatch, diff --git a/l1-contracts/contracts/common/libraries/DataEncoding.sol b/l1-contracts/contracts/common/libraries/DataEncoding.sol index 46441fccc..4c623febb 100644 --- a/l1-contracts/contracts/common/libraries/DataEncoding.sol +++ b/l1-contracts/contracts/common/libraries/DataEncoding.sol @@ -5,7 +5,7 @@ pragma solidity 0.8.24; import {L2_NATIVE_TOKEN_VAULT_ADDR} from "../L2ContractAddresses.sol"; import {LEGACY_ENCODING_VERSION, NEW_ENCODING_VERSION} from "../../bridge/asset-router/IAssetRouterBase.sol"; import {INativeTokenVault} from "../../bridge/ntv/INativeTokenVault.sol"; -import {IncorrectTokenAddressFromNTV, UnsupportedEncodingVersion} from "../L1ContractErrors.sol"; +import {IncorrectTokenAddressFromNTV, UnsupportedEncodingVersion, InvalidNTVBurnData} from "../L1ContractErrors.sol"; /** * @author Matter Labs @@ -13,6 +13,41 @@ import {IncorrectTokenAddressFromNTV, UnsupportedEncodingVersion} from "../L1Con * @notice Helper library for transfer data encoding and decoding to reduce possibility of errors. */ library DataEncoding { + /// @notice Abi.encodes the data required for bridgeBurn for NativeTokenVault. + /// @param _amount The amount of token to be transferred. + /// @param _remoteReceiver The address which to receive tokens on remote chain. + /// @param _maybeTokenAddress The helper field that should be either equal to 0 (in this case + /// it is assumed that the token has been registered within NativeTokenVault already) or it + /// can be equal to the address of the token on the current chain. Providing non-zero address + /// allows it to be automatically registered in case it is not yet a part of NativeTokenVault. + /// @return The encoded bridgeBurn data + function encodeBridgeBurnData( + uint256 _amount, + address _remoteReceiver, + address _maybeTokenAddress + ) internal pure returns (bytes memory) { + return abi.encode(_amount, _remoteReceiver, _maybeTokenAddress); + } + + /// @notice Function decoding bridgeBurn data previously encoded with this library. + /// @param _data The encoded data for bridgeBurn + /// @return amount The amount of token to be transferred. + /// @return receiver The address which to receive tokens on remote chain. + /// @return maybeTokenAddress The helper field that should be either equal to 0 (in this case + /// it is assumed that the token has been registered within NativeTokenVault already) or it + /// can be equal to the address of the token on the current chain. Providing non-zero address + /// allows it to be automatically registered in case it is not yet a part of NativeTokenVault. + function decodeBridgeBurnData( + bytes memory _data + ) internal pure returns (uint256 amount, address receiver, address maybeTokenAddress) { + if (_data.length != 96) { + // For better error handling + revert InvalidNTVBurnData(); + } + + (amount, receiver, maybeTokenAddress) = abi.decode(_data, (uint256, address, address)); + } + /// @notice Abi.encodes the data required for bridgeMint on remote chain. /// @param _originalCaller The address which initiated the transfer. /// @param _remoteReceiver The address which to receive tokens on remote chain. @@ -116,7 +151,7 @@ library DataEncoding { revert IncorrectTokenAddressFromNTV(_assetId, tokenAddress); } - (uint256 depositAmount, ) = abi.decode(_transferData, (uint256, address)); + (uint256 depositAmount, , ) = decodeBridgeBurnData(_transferData); txDataHash = keccak256(abi.encode(_originalCaller, tokenAddress, depositAmount)); } else if (_encodingVersion == NEW_ENCODING_VERSION) { // Similarly to calldata, the txDataHash is collision-resistant. diff --git a/l1-contracts/deploy-scripts/Utils.sol b/l1-contracts/deploy-scripts/Utils.sol index 2fca6c2b9..bde1dd959 100644 --- a/l1-contracts/deploy-scripts/Utils.sol +++ b/l1-contracts/deploy-scripts/Utils.sol @@ -211,24 +211,26 @@ library Utils { * @dev Returns the bytecode of a given system contract. */ function readPrecompileBytecode(string memory filename) internal view returns (bytes memory) { + string memory path = string.concat( + "/../system-contracts/zkout/", + filename, + ".yul/contracts-preprocessed/precompiles/", + filename, + ".yul.json" + ); + // It is the only exceptional case if (keccak256(abi.encodePacked(filename)) == keccak256(abi.encodePacked("EventWriter"))) { - return - vm.readFileBinary( - // solhint-disable-next-line func-named-parameters - string.concat("../system-contracts/contracts-preprocessed/artifacts/", filename, ".yul.zbin") - ); + path = string.concat( + "/../system-contracts/zkout/", + filename, + ".yul/contracts-preprocessed/", + filename, + ".yul.json" + ); } - return - vm.readFileBinary( - // solhint-disable-next-line func-named-parameters - string.concat( - "../system-contracts/contracts-preprocessed/precompiles/artifacts/", - filename, - ".yul.zbin" - ) - ); + return readFoundryBytecode(path); } /** diff --git a/l1-contracts/deploy-scripts/upgrade/EcosystemUpgrade.s.sol b/l1-contracts/deploy-scripts/upgrade/EcosystemUpgrade.s.sol index 3f21fdff2..e0bc48627 100644 --- a/l1-contracts/deploy-scripts/upgrade/EcosystemUpgrade.s.sol +++ b/l1-contracts/deploy-scripts/upgrade/EcosystemUpgrade.s.sol @@ -352,7 +352,7 @@ contract EcosystemUpgrade is Script { maxFeePerGas: 0, maxPriorityFeePerGas: 0, paymaster: uint256(uint160(address(0))), - nonce: 25, + nonce: getProtocolUpgradeNonce(), value: 0, reserved: [uint256(0), uint256(0), uint256(0), uint256(0)], // Note, that the data is empty, it will be fully composed inside the `GatewayUpgrade` contract @@ -368,7 +368,11 @@ contract EcosystemUpgrade is Script { } function getNewProtocolVersion() public returns (uint256) { - return 0x1900000000; + return 0x1b00000000; + } + + function getProtocolUpgradeNonce() public returns (uint256) { + return (getNewProtocolVersion() >> 32); } function getOldProtocolDeadline() public returns (uint256) { @@ -378,7 +382,12 @@ contract EcosystemUpgrade is Script { } function getOldProtocolVersion() public returns (uint256) { - return 0x1800000002; + // Mainnet is the only network that has not been upgraded. + if (block.chainid == 1) { + return 0x1800000002; + } else { + return 0x1900000000; + } } function provideSetNewVersionUpgradeCall() public returns (Call[] memory calls) { @@ -1144,7 +1153,6 @@ contract EcosystemUpgrade is Script { abi.encode( config.tokens.tokenWethAddress, addresses.bridges.sharedBridgeProxy, - config.eraChainId, config.contracts.oldSharedBridgeProxyAddress ) ); diff --git a/l1-contracts/deploy-scripts/upgrade/FinalizeUpgrade.s.sol b/l1-contracts/deploy-scripts/upgrade/FinalizeUpgrade.s.sol index 00a730194..6d2eb7c49 100644 --- a/l1-contracts/deploy-scripts/upgrade/FinalizeUpgrade.s.sol +++ b/l1-contracts/deploy-scripts/upgrade/FinalizeUpgrade.s.sol @@ -41,12 +41,7 @@ contract FinalizeUpgrade is Script { if (bh.baseTokenAssetId(chains[i]) == bytes32(0)) { vm.broadcast(); - Bridgehub(bridgehub).setLegacyBaseTokenAssetId(chains[i]); - } - - if (bh.getZKChain(chains[i]) == address(0)) { - vm.broadcast(); - Bridgehub(bridgehub).setLegacyChainAddress(chains[i]); + Bridgehub(bridgehub).registerLegacyChain(chains[i]); } } } @@ -63,18 +58,26 @@ contract FinalizeUpgrade is Script { for (uint256 i = 0; i < tokens.length; i++) { uint256 tokenBalance; - if (tokens[i] != ETH_TOKEN_ADDRESS) { - uint256 balance = IERC20(tokens[i]).balanceOf(nullifier); - if (balance != 0) { + if (vault.assetId(tokens[i]) == bytes32(0)) { + if (tokens[i] != ETH_TOKEN_ADDRESS) { + uint256 balance = IERC20(tokens[i]).balanceOf(nullifier); + if (balance != 0) { + vm.broadcast(); + vault.transferFundsFromSharedBridge(tokens[i]); + } else { + vm.broadcast(); + vault.registerToken(tokens[i]); + } + } else { vm.broadcast(); - vault.transferFundsFromSharedBridge(tokens[i]); - } + vault.registerEthToken(); - vm.broadcast(); - vault.registerToken(tokens[i]); - } else { - vm.broadcast(); - vault.registerEthToken(); + uint256 balance = address(nullifier).balance; + if (balance != 0) { + vm.broadcast(); + vault.transferFundsFromSharedBridge(tokens[i]); + } + } } // TODO: we need to reduce complexity of this one diff --git a/l1-contracts/test/foundry/l1/integration/AssetRouterTest.t.sol b/l1-contracts/test/foundry/l1/integration/AssetRouterTest.t.sol index 881c38e88..512dd203a 100644 --- a/l1-contracts/test/foundry/l1/integration/AssetRouterTest.t.sol +++ b/l1-contracts/test/foundry/l1/integration/AssetRouterTest.t.sol @@ -169,7 +169,7 @@ contract AssetRouterTest is L1ContractDeployer, ZKChainDeployer, TokenDeployer, depositToL1(ETH_TOKEN_ADDRESS); bytes memory secondBridgeCalldata = bytes.concat( NEW_ENCODING_VERSION, - abi.encode(l2TokenAssetId, abi.encode(uint256(100), address(this))) + abi.encode(l2TokenAssetId, abi.encode(uint256(100), address(this), tokenL1Address)) ); IERC20(tokenL1Address).approve(address(l1NativeTokenVault), 100); bridgehub.requestL2TransactionTwoBridges{value: 250000000000100}( diff --git a/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/L2Erc20TestAbstract.t.sol b/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/L2Erc20TestAbstract.t.sol index 049f7304f..c7e4d5ba5 100644 --- a/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/L2Erc20TestAbstract.t.sol +++ b/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/L2Erc20TestAbstract.t.sol @@ -12,6 +12,7 @@ import {IL2NativeTokenVault} from "contracts/bridge/ntv/IL2NativeTokenVault.sol" import {UpgradeableBeacon} from "@openzeppelin/contracts-v4/proxy/beacon/UpgradeableBeacon.sol"; import {BeaconProxy} from "@openzeppelin/contracts-v4/proxy/beacon/BeaconProxy.sol"; +import {DataEncoding} from "contracts/common/libraries/DataEncoding.sol"; import {L2_ASSET_ROUTER_ADDR, L2_NATIVE_TOKEN_VAULT_ADDR, L2_BRIDGEHUB_ADDR, L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR} from "contracts/common/L2ContractAddresses.sol"; import {ETH_TOKEN_ADDRESS, SETTLEMENT_LAYER_RELAY_SENDER} from "contracts/common/Config.sol"; @@ -99,7 +100,7 @@ abstract contract L2Erc20TestAbstract is Test, SharedL2ContractDeployer { BridgedStandardERC20(l2TokenAddress).reinitializeToken(getters, "TestTokenNewName", "TTN", 20); } - function test_withdrawToken() external { + function test_withdrawTokenNoRegistration() public { TestnetERC20Token l2NativeToken = new TestnetERC20Token("token", "T", 18); l2NativeToken.mint(address(this), 100); @@ -112,6 +113,11 @@ abstract contract L2Erc20TestAbstract is Test, SharedL2ContractDeployer { abi.encode(bytes32(uint256(1))) ); - IL2AssetRouter(L2_ASSET_ROUTER_ADDR).withdrawToken(address(l2NativeToken), abi.encode(100, address(1))); + bytes32 assetId = DataEncoding.encodeNTVAssetId(block.chainid, address(l2NativeToken)); + + IL2AssetRouter(L2_ASSET_ROUTER_ADDR).withdraw( + assetId, + DataEncoding.encodeBridgeBurnData(100, address(1), address(l2NativeToken)) + ); } } diff --git a/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeBase.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeBase.t.sol index 9f4c45d67..bd32d0430 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeBase.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeBase.t.sol @@ -206,7 +206,7 @@ contract L1AssetRouterTestBase is L1AssetRouterTest { } function test_bridgeRecoverFailedTransfer_Eth() public { - bytes memory transferData = abi.encode(amount, alice); + bytes memory transferData = abi.encode(amount, alice, ETH_TOKEN_ADDRESS); bytes32 txDataHash = keccak256(abi.encode(alice, ETH_TOKEN_ADDRESS, amount)); _setSharedBridgeDepositHappened(chainId, txHash, txDataHash); require(l1Nullifier.depositHappened(chainId, txHash) == txDataHash, "Deposit not set"); @@ -502,7 +502,7 @@ contract L1AssetRouterTestBase is L1AssetRouterTest { _encodingVersion: LEGACY_ENCODING_VERSION, _originalCaller: alice, _assetId: nativeTokenVault.BASE_TOKEN_ASSET_ID(), - _transferData: abi.encode(amount, bob) + _transferData: abi.encode(amount, bob, ETH_TOKEN_ADDRESS) }); assertEq(request.txDataHash, expectedTxHash); diff --git a/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeFails.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeFails.t.sol index eae01e406..5c892cccb 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeFails.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeFails.t.sol @@ -278,7 +278,7 @@ contract L1AssetRouterFailTest is L1AssetRouterTest { function test_bridgeRecoverFailedTransfer_Eth_claimFailedDepositFailed() public { vm.deal(address(nativeTokenVault), 0); - bytes memory transferData = abi.encode(amount, alice); + bytes memory transferData = abi.encode(amount, alice, ETH_TOKEN_ADDRESS); bytes32 txDataHash = keccak256(abi.encode(alice, ETH_TOKEN_ADDRESS, amount)); _setSharedBridgeDepositHappened(chainId, txHash, txDataHash); require(l1Nullifier.depositHappened(chainId, txHash) == txDataHash, "Deposit not set"); @@ -357,7 +357,7 @@ contract L1AssetRouterFailTest is L1AssetRouterTest { vm.store(address(l1Nullifier), bytes32(isWithdrawalFinalizedStorageLocation - 5), bytes32(uint256(2))); uint256 l2BatchNumber = 0; - bytes memory transferData = abi.encode(amount, alice); + bytes memory transferData = abi.encode(amount, alice, ETH_TOKEN_ADDRESS); bytes32 txDataHash = keccak256(abi.encode(alice, ETH_TOKEN_ADDRESS, amount)); _setSharedBridgeDepositHappened(eraChainId, txHash, txDataHash); require(l1Nullifier.depositHappened(eraChainId, txHash) == txDataHash, "Deposit not set"); From b350a58357d40a2d8083ee03e0c3579d09c9a1d2 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 9 Dec 2024 22:19:14 +0100 Subject: [PATCH 29/57] respond to comments --- .../bridge/ntv/L1NativeTokenVault.sol | 18 ++++++++++++++++-- .../chain-deps/facets/Admin.sol | 2 +- .../chain-interfaces/IAdmin.sol | 2 +- l1-contracts/selectors | 8 ++++---- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol index c0ad76d97..7bd2ba688 100644 --- a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol @@ -283,7 +283,7 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken ) internal override { // Note, that we do not update balances for chains where the assetId comes from, // since these chains can mint new instances of the token. - if ((_isNative) || (originChainId[_assetId] != _chainId)) { + if (!_hasInfiniteBalance(_isNative, _assetId, _chainId)) { chainBalance[_chainId][_assetId] += _amount; } } @@ -296,7 +296,7 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken ) internal override { // Note, that we do not update balances for chains where the assetId comes from, // since these chains can mint new instances of the token. - if ((_isNative) || (originChainId[_assetId] != _chainId)) { + if (!_hasInfiniteBalance(_isNative, _assetId, _chainId)) { // Check that the chain has sufficient balance if (chainBalance[_chainId][_assetId] < _amount) { revert InsufficientChainBalance(); @@ -304,4 +304,18 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken chainBalance[_chainId][_assetId] -= _amount; } } + + /// @dev Returns whether a chain `_chainId` has infinite balance for an asset `_assetId`, i.e. + /// it can be minted by it. + /// @param _isNative Whether the asset is native to the L1 chain. + /// @param _assetId The asset id + /// @param _chainId An id of a chain which we test against. + /// @return Whether The chain `_chainId` has infinite balance of the token + function _hasInfiniteBalance( + bool _isNative, + bytes32 _assetId, + uint256 _chainId + ) private view returns (bool) { + return !_isNative && originChainId[_assetId] == _chainId; + } } diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol index fa7508743..fc546c86b 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol @@ -133,7 +133,7 @@ contract AdminFacet is ZKChainBase, IAdmin { /// @inheritdoc IAdmin function setPubdataPricingMode(PubdataPricingMode _pricingMode) external onlyAdmin onlyL1 { s.feeParams.pubdataPricingMode = _pricingMode; - emit PubdataPricingModeUpdaate(_pricingMode); + emit PubdataPricingModeUpdate(_pricingMode); } /// @inheritdoc IAdmin diff --git a/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol b/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol index 2a10acf08..4ca0810fc 100644 --- a/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol +++ b/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol @@ -107,7 +107,7 @@ interface IAdmin is IZKChainBase { event NewFeeParams(FeeParams oldFeeParams, FeeParams newFeeParams); /// @notice Validium mode status changed - event PubdataPricingModeUpdaate(PubdataPricingMode validiumMode); + event PubdataPricingModeUpdate(PubdataPricingMode validiumMode); /// @notice The transaction filterer has been updated event NewTransactionFilterer(address oldTransactionFilterer, address newTransactionFilterer); diff --git a/l1-contracts/selectors b/l1-contracts/selectors index 95d5ada8d..ba2b254d7 100644 --- a/l1-contracts/selectors +++ b/l1-contracts/selectors @@ -2815,7 +2815,7 @@ AdminFacetTest |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| | Event | ValidatorStatusUpdate(address,bool) | 0x065b77b53864e46fda3d8986acb51696223d6dde7ced42441eb150bae6d48136 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| -| Event | PubdataPricingModeUpdaate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | +| Event | PubdataPricingModeUpdate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| | Error | AddressHasNoCode(address) | 0x86bb51b8 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| @@ -5002,7 +5002,7 @@ AdminFacet |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| | Event | ValidatorStatusUpdate(address,bool) | 0x065b77b53864e46fda3d8986acb51696223d6dde7ced42441eb150bae6d48136 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| -| Event | PubdataPricingModeUpdaate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | +| Event | PubdataPricingModeUpdate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| | Error | AddressHasNoCode(address) | 0x86bb51b8 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| @@ -5390,7 +5390,7 @@ IAdmin |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| | Event | ValidatorStatusUpdate(address,bool) | 0x065b77b53864e46fda3d8986acb51696223d6dde7ced42441eb150bae6d48136 | |----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| -| Event | PubdataPricingModeUpdaate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | +| Event | PubdataPricingModeUpdate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | +----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------+ IDiamondInit @@ -5681,7 +5681,7 @@ IZKChain |----------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| | Event | ValidatorStatusUpdate(address,bool) | 0x065b77b53864e46fda3d8986acb51696223d6dde7ced42441eb150bae6d48136 | |----------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------| -| Event | PubdataPricingModeUpdaate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | +| Event | PubdataPricingModeUpdate(uint8) | 0xaa01146df0a628c6b214e79a281f7524b792de4a52ad6f07c78e6e035d58c896 | +----------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------+ Diamond From 60f5badacd8cf03aba8834995624b692f5001ef5 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 00:50:40 +0100 Subject: [PATCH 30/57] fix lint/slither --- l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol | 6 +----- l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol | 3 ++- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol index 7bd2ba688..a012728c9 100644 --- a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol @@ -311,11 +311,7 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken /// @param _assetId The asset id /// @param _chainId An id of a chain which we test against. /// @return Whether The chain `_chainId` has infinite balance of the token - function _hasInfiniteBalance( - bool _isNative, - bytes32 _assetId, - uint256 _chainId - ) private view returns (bool) { + function _hasInfiniteBalance(bool _isNative, bytes32 _assetId, uint256 _chainId) private view returns (bool) { return !_isNative && originChainId[_assetId] == _chainId; } } diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index 65b048e3d..fff304789 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -224,7 +224,8 @@ abstract contract NativeTokenVault is } function tryRegisterTokenFromBurnData(bytes calldata _data, bytes32 _expectedAssetId) external { - (uint256 amount, address receiver, address tokenAddress) = DataEncoding.decodeBridgeBurnData(_data); + // slither-disable-next-line unused-return + (, , address tokenAddress) = DataEncoding.decodeBridgeBurnData(_data); if (tokenAddress == address(0)) { revert ZeroAddress(); From 7d809ddf4df08db2cf827f7c0a4ecb9bcfbb876e Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 10 Dec 2024 00:51:30 +0100 Subject: [PATCH 31/57] Restore loadtest final merge (#61) Clone of https://github.com/matter-labs/era-contracts-private/pull/10, but targeted towards the gateway release candidate --- .../dev-contracts/L2SharedBridgeLegacyDev.sol | 51 ++++++++++ .../config-deploy-l1.toml | 1 + l1-contracts/deploy-scripts/DeployL1.s.sol | 23 ++++- .../deploy-scripts/DeployL2Contracts.sol | 8 +- l1-contracts/deploy-scripts/DeployUtils.s.sol | 6 ++ .../L2ContractsBytecodesLib.sol | 6 ++ .../L2LegacySharedBridgeTestHelper.sol | 97 +++++++++++++++++++ .../deploy-scripts/RegisterZKChain.s.sol | 89 ++++++----------- .../dev/SetupLegacyBridge.s.sol | 67 +++++++------ .../upgrade/EcosystemUpgrade.s.sol | 7 +- .../script-config/config-deploy-l1.toml | 1 + .../upgrade-envs/script-config/mainnet.toml | 1 + .../contracts/L2GatewayUpgradeHelper.sol | 13 ++- .../interfaces/IL2GenesisUpgrade.sol | 4 + .../test/L2GenesisUpgrade.spec.ts | 3 +- 15 files changed, 272 insertions(+), 105 deletions(-) create mode 100644 l1-contracts/contracts/dev-contracts/L2SharedBridgeLegacyDev.sol create mode 100644 l1-contracts/deploy-scripts/L2LegacySharedBridgeTestHelper.sol diff --git a/l1-contracts/contracts/dev-contracts/L2SharedBridgeLegacyDev.sol b/l1-contracts/contracts/dev-contracts/L2SharedBridgeLegacyDev.sol new file mode 100644 index 000000000..c80685a81 --- /dev/null +++ b/l1-contracts/contracts/dev-contracts/L2SharedBridgeLegacyDev.sol @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.24; + +import {UpgradeableBeacon} from "@openzeppelin/contracts-v4/proxy/beacon/UpgradeableBeacon.sol"; + +import {BridgedStandardERC20} from "../bridge/BridgedStandardERC20.sol"; + +import {L2SharedBridgeLegacy} from "../bridge/L2SharedBridgeLegacy.sol"; +import {InvalidCaller, ZeroAddress, EmptyBytes32, Unauthorized, AmountMustBeGreaterThanZero, DeployFailed} from "../common/L1ContractErrors.sol"; + +contract L2SharedBridgeLegacyDev is L2SharedBridgeLegacy { + constructor() L2SharedBridgeLegacy() {} + + /// @notice Initializes the bridge contract for later use. Expected to be used in the proxy. + /// @param _legacyBridge The address of the L1 Bridge contract. + /// @param _l1SharedBridge The address of the L1 Bridge contract. + /// @param _l2TokenProxyBytecodeHash The bytecode hash of the proxy for tokens deployed by the bridge. + /// @param _aliasedOwner The address of the governor contract. + function initializeDevBridge( + address _legacyBridge, + address _l1SharedBridge, + bytes32 _l2TokenProxyBytecodeHash, + address _aliasedOwner + ) external reinitializer(2) { + if (_l1SharedBridge == address(0)) { + revert ZeroAddress(); + } + + if (_l2TokenProxyBytecodeHash == bytes32(0)) { + revert EmptyBytes32(); + } + + if (_aliasedOwner == address(0)) { + revert ZeroAddress(); + } + + l1SharedBridge = _l1SharedBridge; + l1Bridge = _legacyBridge; + + // The following statement is true only in freshly deployed environments. However, + // for those environments we do not need to deploy this contract at all. + // This check is primarily for local testing purposes. + if (l2TokenProxyBytecodeHash == bytes32(0) && address(l2TokenBeacon) == address(0)) { + address l2StandardToken = address(new BridgedStandardERC20{salt: bytes32(0)}()); + l2TokenBeacon = new UpgradeableBeacon{salt: bytes32(0)}(l2StandardToken); + l2TokenProxyBytecodeHash = _l2TokenProxyBytecodeHash; + l2TokenBeacon.transferOwnership(_aliasedOwner); + } + } +} diff --git a/l1-contracts/deploy-script-config-template/config-deploy-l1.toml b/l1-contracts/deploy-script-config-template/config-deploy-l1.toml index 4429fbf12..1802c8e0e 100644 --- a/l1-contracts/deploy-script-config-template/config-deploy-l1.toml +++ b/l1-contracts/deploy-script-config-template/config-deploy-l1.toml @@ -1,6 +1,7 @@ era_chain_id = 9 owner_address = "0x70997970C51812dc3A010C7d01b50e0d17dc79C8" testnet_verifier = true +support_l2_legacy_shared_bridge_test = false [contracts] governance_security_council_address = "0x0000000000000000000000000000000000000000" diff --git a/l1-contracts/deploy-scripts/DeployL1.s.sol b/l1-contracts/deploy-scripts/DeployL1.s.sol index 541a00eea..8f357decf 100644 --- a/l1-contracts/deploy-scripts/DeployL1.s.sol +++ b/l1-contracts/deploy-scripts/DeployL1.s.sol @@ -57,6 +57,7 @@ import {L2ContractsBytecodesLib} from "./L2ContractsBytecodesLib.sol"; import {ValidiumL1DAValidator} from "contracts/state-transition/data-availability/ValidiumL1DAValidator.sol"; import {RollupDAManager} from "contracts/state-transition/data-availability/RollupDAManager.sol"; import {BytecodesSupplier} from "contracts/upgrades/BytecodesSupplier.sol"; +import {L2LegacySharedBridgeTestHelper} from "./L2LegacySharedBridgeTestHelper.sol"; import {DeployUtils, GeneratedData, Config, DeployedAddresses, FixedForceDeploymentsData} from "./DeployUtils.s.sol"; @@ -310,9 +311,15 @@ contract DeployL1Script is Script, DeployUtils { } function deployL1NullifierImplementation() internal { - // TODO(EVM-743): allow non-dev nullifier in the local deployment + bytes memory bytecode; + if (config.supportL2LegacySharedBridgeTest) { + bytecode = type(L1NullifierDev).creationCode; + } else { + bytecode = type(L1Nullifier).creationCode; + } + address contractAddress = deployViaCreate2( - type(L1NullifierDev).creationCode, + bytecode, // solhint-disable-next-line func-named-parameters abi.encode(addresses.bridgehub.bridgehubProxy, config.eraChainId, addresses.stateTransition.diamondProxy) ); @@ -688,6 +695,15 @@ contract DeployL1Script is Script, DeployUtils { function prepareForceDeploymentsData() internal view returns (bytes memory) { require(addresses.governance != address(0), "Governance address is not set"); + address dangerousTestOnlyForcedBeacon; + if (config.supportL2LegacySharedBridgeTest) { + (dangerousTestOnlyForcedBeacon, ) = L2LegacySharedBridgeTestHelper.calculateTestL2TokenBeaconAddress( + addresses.bridges.erc20BridgeProxy, + addresses.bridges.l1NullifierProxy, + addresses.governance + ); + } + FixedForceDeploymentsData memory data = FixedForceDeploymentsData({ l1ChainId: config.l1ChainId, eraChainId: config.eraChainId, @@ -708,7 +724,8 @@ contract DeployL1Script is Script, DeployUtils { // For newly created chains it it is expected that the following bridges are not present at the moment // of creation of the chain l2SharedBridgeLegacyImpl: address(0), - l2BridgedStandardERC20Impl: address(0) + l2BridgedStandardERC20Impl: address(0), + dangerousTestOnlyForcedBeacon: dangerousTestOnlyForcedBeacon }); return abi.encode(data); diff --git a/l1-contracts/deploy-scripts/DeployL2Contracts.sol b/l1-contracts/deploy-scripts/DeployL2Contracts.sol index 00b29ecd0..dbd83f5ee 100644 --- a/l1-contracts/deploy-scripts/DeployL2Contracts.sol +++ b/l1-contracts/deploy-scripts/DeployL2Contracts.sol @@ -39,14 +39,10 @@ contract DeployL2Script is Script { } function run() public { - deploy(false); + deploy(); } - function runWithLegacyBridge() public { - deploy(true); - } - - function deploy(bool legacyBridge) public { + function deploy() public { initializeConfig(); // Note, that it is important that the first transaction is for setting the L2 DA validator diff --git a/l1-contracts/deploy-scripts/DeployUtils.s.sol b/l1-contracts/deploy-scripts/DeployUtils.s.sol index 3f4717414..ac26ddba5 100644 --- a/l1-contracts/deploy-scripts/DeployUtils.s.sol +++ b/l1-contracts/deploy-scripts/DeployUtils.s.sol @@ -69,6 +69,10 @@ struct FixedForceDeploymentsData { bytes32 messageRootBytecodeHash; address l2SharedBridgeLegacyImpl; address l2BridgedStandardERC20Impl; + // The forced beacon address. It is needed only for internal testing. + // MUST be equal to 0 in production. + // It will be the job of the governance to ensure that this value is set correctly. + address dangerousTestOnlyForcedBeacon; } // solhint-disable-next-line gas-struct-packing @@ -128,6 +132,7 @@ struct Config { uint256 eraChainId; address ownerAddress; bool testnetVerifier; + bool supportL2LegacySharedBridgeTest; ContractsConfig contracts; TokensConfig tokens; } @@ -190,6 +195,7 @@ contract DeployUtils is Script { config.eraChainId = toml.readUint("$.era_chain_id"); config.ownerAddress = toml.readAddress("$.owner_address"); config.testnetVerifier = toml.readBool("$.testnet_verifier"); + config.supportL2LegacySharedBridgeTest = toml.readBool("$.support_l2_legacy_shared_bridge_test"); config.contracts.governanceSecurityCouncilAddress = toml.readAddress( "$.contracts.governance_security_council_address" diff --git a/l1-contracts/deploy-scripts/L2ContractsBytecodesLib.sol b/l1-contracts/deploy-scripts/L2ContractsBytecodesLib.sol index 832911f37..499592082 100644 --- a/l1-contracts/deploy-scripts/L2ContractsBytecodesLib.sol +++ b/l1-contracts/deploy-scripts/L2ContractsBytecodesLib.sol @@ -188,6 +188,12 @@ library L2ContractsBytecodesLib { return Utils.readZKFoundryBytecodeL1("L2SharedBridgeLegacy.sol", "L2SharedBridgeLegacy"); } + /// @notice Reads the bytecode of the L2SharedBridgeLegacy contract. + /// @return The bytecode of the L2SharedBridgeLegacy contract. + function readL2LegacySharedBridgeDevBytecode() internal view returns (bytes memory) { + return Utils.readZKFoundryBytecodeL1("L2SharedBridgeLegacyDev.sol", "L2SharedBridgeLegacyDev"); + } + /// @notice Reads the bytecode of the L2GatewayUpgrade contract. /// @return The bytecode of the L2GatewayUpgrade contract. function readGatewayUpgradeBytecode() internal view returns (bytes memory) { diff --git a/l1-contracts/deploy-scripts/L2LegacySharedBridgeTestHelper.sol b/l1-contracts/deploy-scripts/L2LegacySharedBridgeTestHelper.sol new file mode 100644 index 000000000..14b1285f4 --- /dev/null +++ b/l1-contracts/deploy-scripts/L2LegacySharedBridgeTestHelper.sol @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {L2ContractsBytecodesLib} from "./L2ContractsBytecodesLib.sol"; +import {L2ContractHelper} from "contracts/common/libraries/L2ContractHelper.sol"; +import {Utils} from "./Utils.sol"; +import {L2SharedBridgeLegacyDev} from "contracts/dev-contracts/L2SharedBridgeLegacyDev.sol"; +import {AddressAliasHelper} from "contracts/vendor/AddressAliasHelper.sol"; + +library L2LegacySharedBridgeTestHelper { + function calculateL2LegacySharedBridgeProxyAddr( + address l1Erc20BridgeProxy, + address l1NullifierProxy, + address ecosystemL1Governance + ) internal view returns (address) { + // During local testing, we will deploy `L2SharedBridgeLegacyDev` to each chain + // that supports the legacy bridge. + + bytes32 implHash = L2ContractHelper.hashL2Bytecode( + L2ContractsBytecodesLib.readL2LegacySharedBridgeDevBytecode() + ); + address implAddress = Utils.getL2AddressViaCreate2Factory(bytes32(0), implHash, hex""); + + bytes32 proxyHash = L2ContractHelper.hashL2Bytecode( + L2ContractsBytecodesLib.readTransparentUpgradeableProxyBytecode() + ); + + return + Utils.getL2AddressViaCreate2Factory( + bytes32(0), + proxyHash, + getLegacySharedBridgeProxyConstructorParams( + implAddress, + l1Erc20BridgeProxy, + l1NullifierProxy, + ecosystemL1Governance + ) + ); + } + + function getLegacySharedBridgeProxyConstructorParams( + address _implAddress, + address _l1Erc20BridgeProxy, + address _l1NullifierProxy, + address _ecosystemL1Governance + ) internal view returns (bytes memory) { + bytes32 beaconProxyBytecodeHash = L2ContractHelper.hashL2Bytecode( + L2ContractsBytecodesLib.readBeaconProxyBytecode() + ); + + bytes memory initializeData = abi.encodeCall( + L2SharedBridgeLegacyDev.initializeDevBridge, + ( + _l1Erc20BridgeProxy, + // While the variable is named `sharedBridge`, in reality it will have the same + // address as the nullifier + _l1NullifierProxy, + beaconProxyBytecodeHash, + AddressAliasHelper.applyL1ToL2Alias(_ecosystemL1Governance) + ) + ); + + return abi.encode(_implAddress, AddressAliasHelper.applyL1ToL2Alias(_ecosystemL1Governance), initializeData); + } + + function calculateTestL2TokenBeaconAddress( + address l1Erc20BridgeProxy, + address l1NullifierProxy, + address ecosystemL1Governance + ) internal view returns (address tokenBeaconAddress, bytes32 tokenBeaconBytecodeHash) { + address l2SharedBridgeAddress = calculateL2LegacySharedBridgeProxyAddr( + l1Erc20BridgeProxy, + l1NullifierProxy, + ecosystemL1Governance + ); + + bytes32 bridgedL2ERC20Hash = L2ContractHelper.hashL2Bytecode( + L2ContractsBytecodesLib.readStandardERC20Bytecode() + ); + address bridgeL2ERC20ImplAddress = L2ContractHelper.computeCreate2Address( + l2SharedBridgeAddress, + bytes32(0), + bridgedL2ERC20Hash, + keccak256(hex"") + ); + + tokenBeaconBytecodeHash = L2ContractHelper.hashL2Bytecode( + L2ContractsBytecodesLib.readUpgradeableBeaconBytecode() + ); + tokenBeaconAddress = L2ContractHelper.computeCreate2Address( + l2SharedBridgeAddress, + bytes32(0), + tokenBeaconBytecodeHash, + keccak256(abi.encode(bridgeL2ERC20ImplAddress)) + ); + } +} diff --git a/l1-contracts/deploy-scripts/RegisterZKChain.s.sol b/l1-contracts/deploy-scripts/RegisterZKChain.s.sol index cc124a138..36bbbfae8 100644 --- a/l1-contracts/deploy-scripts/RegisterZKChain.s.sol +++ b/l1-contracts/deploy-scripts/RegisterZKChain.s.sol @@ -25,7 +25,7 @@ import {L2ContractHelper} from "contracts/common/libraries/L2ContractHelper.sol" import {L1NullifierDev} from "contracts/dev-contracts/L1NullifierDev.sol"; import {L2SharedBridgeLegacy} from "contracts/bridge/L2SharedBridgeLegacy.sol"; import {AddressAliasHelper} from "contracts/vendor/AddressAliasHelper.sol"; - +import {L2LegacySharedBridgeTestHelper} from "./L2LegacySharedBridgeTestHelper.sol"; import {ETH_TOKEN_ADDRESS} from "contracts/common/Config.sol"; // solhint-disable-next-line gas-struct-packing @@ -52,6 +52,8 @@ struct Config { address governanceSecurityCouncilAddress; uint256 governanceMinDelay; address l1Nullifier; + address l1Erc20Bridge; + bool initializeLegacyBridge; } contract RegisterZKChainScript is Script { @@ -86,22 +88,21 @@ contract RegisterZKChainScript is Script { initializeConfig(); // TODO: some chains may not want to have a legacy shared bridge - runInner("/script-out/output-register-zk-chain.toml", false); + runInner("/script-out/output-register-zk-chain.toml"); } function runForTest() public { console.log("Deploying ZKChain"); initializeConfigTest(); - // TODO: Yes, it is the same as for prod since it is never read from down the line - runInner(vm.envString("ZK_CHAIN_OUT"), false); + runInner(vm.envString("ZK_CHAIN_OUT")); } - function runInner(string memory outputPath, bool initializeL2LegacyBridge) internal { + function runInner(string memory outputPath) internal { string memory root = vm.projectRoot(); outputPath = string.concat(root, outputPath); - if (initializeL2LegacyBridge) { + if (config.initializeLegacyBridge) { // This must be run before the chain is deployed setUpLegacySharedBridgeParams(); } @@ -117,7 +118,7 @@ contract RegisterZKChainScript is Script { configureZkSyncStateTransition(); setPendingAdmin(); - if (initializeL2LegacyBridge) { + if (config.initializeLegacyBridge) { deployLegacySharedBridge(); } @@ -145,6 +146,7 @@ contract RegisterZKChainScript is Script { config.nativeTokenVault = toml.readAddress("$.deployed_addresses.native_token_vault_addr"); config.sharedBridgeProxy = toml.readAddress("$.deployed_addresses.bridges.shared_bridge_proxy_addr"); config.l1Nullifier = toml.readAddress("$.deployed_addresses.bridges.l1_nullifier_proxy_addr"); + config.l1Erc20Bridge = toml.readAddress("$.deployed_addresses.bridges.erc20_bridge_proxy_addr"); config.diamondCutData = toml.readBytes("$.contracts_config.diamond_cut_data"); config.forceDeployments = toml.readBytes("$.contracts_config.force_deployments_data"); @@ -165,6 +167,7 @@ contract RegisterZKChainScript is Script { config.validiumMode = toml.readBool("$.chain.validium_mode"); config.validatorSenderOperatorCommitEth = toml.readAddress("$.chain.validator_sender_operator_commit_eth"); config.validatorSenderOperatorBlobsEth = toml.readAddress("$.chain.validator_sender_operator_blobs_eth"); + config.initializeLegacyBridge = toml.readBool("$.initialize_legacy_bridge"); } function getConfig() public view returns (Config memory) { @@ -240,49 +243,15 @@ contract RegisterZKChainScript is Script { } function setUpLegacySharedBridgeParams() internal { - bytes memory implementationConstructorParams = hex""; - - address legacyBridgeImplementationAddress = L2ContractHelper.computeCreate2Address( - msg.sender, - "", - L2ContractHelper.hashL2Bytecode(L2ContractsBytecodesLib.readL2LegacySharedBridgeBytecode()), - keccak256(implementationConstructorParams) - ); - - bytes memory proxyInitializationParams = abi.encodeCall( - L2SharedBridgeLegacy.initialize, - ( - config.sharedBridgeProxy, - L2ContractHelper.hashL2Bytecode(L2ContractsBytecodesLib.readBeaconProxyBytecode()), - // This is not exactly correct, this should be ecosystem governance and not chain governance - msg.sender - ) - ); - - bytes memory proxyConstructorParams = abi.encode( - legacyBridgeImplementationAddress, - // In real production, this would be aliased ecosystem governance. - // But in real production we also do not initialize legacy shared bridge - msg.sender, - proxyInitializationParams + // Ecosystem governance is the owner of the L1Nullifier + address ecosystemGovernance = L1NullifierDev(config.l1Nullifier).owner(); + address bridgeAddress = L2LegacySharedBridgeTestHelper.calculateL2LegacySharedBridgeProxyAddr( + config.l1Erc20Bridge, + config.l1Nullifier, + ecosystemGovernance ); - - address proxyAddress = L2ContractHelper.computeCreate2Address( - msg.sender, - "", - L2ContractHelper.hashL2Bytecode(L2ContractsBytecodesLib.readTransparentUpgradeableProxyBytecode()), - keccak256(proxyConstructorParams) - ); - vm.broadcast(); - L1NullifierDev(config.l1Nullifier).setL2LegacySharedBridge(config.chainChainId, proxyAddress); - - legacySharedBridgeParams = LegacySharedBridgeParams({ - implementationConstructorParams: implementationConstructorParams, - implementationAddress: legacyBridgeImplementationAddress, - proxyConstructorParams: proxyConstructorParams, - proxyAddress: proxyAddress - }); + L1NullifierDev(config.l1Nullifier).setL2LegacySharedBridge(config.chainChainId, bridgeAddress); } function registerAssetIdOnBridgehub() internal { @@ -447,9 +416,9 @@ contract RegisterZKChainScript is Script { function deployLegacySharedBridge() internal { bytes[] memory emptyDeps = new bytes[](0); - address correctLegacyBridgeImplAddr = Utils.deployThroughL1({ - bytecode: L2ContractsBytecodesLib.readL2LegacySharedBridgeBytecode(), - constructorargs: legacySharedBridgeParams.implementationConstructorParams, + address legacyBridgeImplAddr = Utils.deployThroughL1Deterministic({ + bytecode: L2ContractsBytecodesLib.readL2LegacySharedBridgeDevBytecode(), + constructorargs: hex"", create2salt: "", l2GasLimit: Utils.MAX_PRIORITY_TX_GAS, factoryDeps: emptyDeps, @@ -458,9 +427,15 @@ contract RegisterZKChainScript is Script { l1SharedBridgeProxy: config.sharedBridgeProxy }); - address correctProxyAddress = Utils.deployThroughL1({ + output.l2LegacySharedBridge = Utils.deployThroughL1Deterministic({ bytecode: L2ContractsBytecodesLib.readTransparentUpgradeableProxyBytecode(), - constructorargs: legacySharedBridgeParams.proxyConstructorParams, + constructorargs: L2LegacySharedBridgeTestHelper.getLegacySharedBridgeProxyConstructorParams( + legacyBridgeImplAddr, + config.l1Erc20Bridge, + config.l1Nullifier, + // Ecosystem governance is the owner of the L1Nullifier + L1NullifierDev(config.l1Nullifier).owner() + ), create2salt: "", l2GasLimit: Utils.MAX_PRIORITY_TX_GAS, factoryDeps: emptyDeps, @@ -468,14 +443,6 @@ contract RegisterZKChainScript is Script { bridgehubAddress: config.bridgehub, l1SharedBridgeProxy: config.sharedBridgeProxy }); - - require( - correctLegacyBridgeImplAddr == legacySharedBridgeParams.implementationAddress, - "Legacy bridge implementation address mismatch" - ); - require(correctProxyAddress == legacySharedBridgeParams.proxyAddress, "Legacy bridge proxy address mismatch"); - - output.l2LegacySharedBridge = correctProxyAddress; } function getFactoryDeps() internal view returns (bytes[] memory) { diff --git a/l1-contracts/deploy-scripts/dev/SetupLegacyBridge.s.sol b/l1-contracts/deploy-scripts/dev/SetupLegacyBridge.s.sol index c7b89b27e..0e6652ce9 100644 --- a/l1-contracts/deploy-scripts/dev/SetupLegacyBridge.s.sol +++ b/l1-contracts/deploy-scripts/dev/SetupLegacyBridge.s.sol @@ -5,11 +5,14 @@ import {Script} from "forge-std/Script.sol"; import {stdToml} from "forge-std/StdToml.sol"; import {Utils} from "./../Utils.sol"; import {L2ContractsBytecodesLib} from "../L2ContractsBytecodesLib.sol"; -import {L1SharedBridge} from "contracts/bridge/L1SharedBridge.sol"; +import {L1AssetRouter} from "contracts/bridge/asset-router/L1AssetRouter.sol"; import {DummyL1ERC20Bridge} from "contracts/dev-contracts/DummyL1ERC20Bridge.sol"; import {ProxyAdmin} from "@openzeppelin/contracts-v4/proxy/transparent/ProxyAdmin.sol"; import {ITransparentUpgradeableProxy} from "@openzeppelin/contracts-v4/proxy/transparent/TransparentUpgradeableProxy.sol"; import {L2ContractHelper} from "contracts/common/libraries/L2ContractHelper.sol"; +import {L2LegacySharedBridgeTestHelper} from "../L2LegacySharedBridgeTestHelper.sol"; +import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable-v4/access/Ownable2StepUpgradeable.sol"; +import {L1NullifierDev} from "contracts/dev-contracts/L1NullifierDev.sol"; /// This scripts is only for developer contract SetupLegacyBridge is Script { @@ -27,13 +30,16 @@ contract SetupLegacyBridge is Script { struct Addresses { address create2FactoryAddr; address bridgehub; + address l1Nullifier; address diamondProxy; address sharedBridgeProxy; + address l1NativeTokenVault; address transparentProxyAdmin; address erc20BridgeProxy; address tokenWethAddress; address erc20BridgeProxyImpl; address sharedBridgeProxyImpl; + address l1NullifierProxyImpl; } function run() public { @@ -43,6 +49,8 @@ contract SetupLegacyBridge is Script { deployDummyErc20Bridge(); upgradeImplementation(addresses.erc20BridgeProxy, addresses.erc20BridgeProxyImpl); setParamsForDummyBridge(); + deployL1NullifierImplementation(); + upgradeImplementation(addresses.l1Nullifier, addresses.l1NullifierProxyImpl); } function initializeConfig() internal { @@ -52,7 +60,9 @@ contract SetupLegacyBridge is Script { addresses.bridgehub = toml.readAddress("$.bridgehub"); addresses.diamondProxy = toml.readAddress("$.diamond_proxy"); + addresses.l1Nullifier = toml.readAddress("$.l1_nullifier_proxy"); addresses.sharedBridgeProxy = toml.readAddress("$.shared_bridge_proxy"); + addresses.l1NativeTokenVault = toml.readAddress("$.l1_native_token_vault"); addresses.transparentProxyAdmin = toml.readAddress("$.transparent_proxy_admin"); addresses.erc20BridgeProxy = toml.readAddress("$.erc20bridge_proxy"); addresses.tokenWethAddress = toml.readAddress("$.token_weth_address"); @@ -65,9 +75,15 @@ contract SetupLegacyBridge is Script { // We need to deploy new shared bridge for changing chain id and diamond proxy address function deploySharedBridgeImplementation() internal { bytes memory bytecode = abi.encodePacked( - type(L1SharedBridge).creationCode, + type(L1AssetRouter).creationCode, // solhint-disable-next-line func-named-parameters - abi.encode(addresses.tokenWethAddress, addresses.bridgehub, config.chainId, addresses.diamondProxy) + abi.encode( + addresses.tokenWethAddress, + addresses.bridgehub, + addresses.l1Nullifier, + config.chainId, + addresses.diamondProxy + ) ); address contractAddress = deployViaCreate2(bytecode); @@ -78,12 +94,22 @@ contract SetupLegacyBridge is Script { bytes memory bytecode = abi.encodePacked( type(DummyL1ERC20Bridge).creationCode, // solhint-disable-next-line func-named-parameters - abi.encode(addresses.sharedBridgeProxy) + abi.encode(addresses.l1Nullifier, addresses.sharedBridgeProxy, addresses.l1NativeTokenVault, config.chainId) ); address contractAddress = deployViaCreate2(bytecode); addresses.erc20BridgeProxyImpl = contractAddress; } + function deployL1NullifierImplementation() internal { + bytes memory bytecode = abi.encodePacked( + type(L1NullifierDev).creationCode, + abi.encode(addresses.bridgehub, config.chainId, addresses.diamondProxy) + ); + address contractAddress = deployViaCreate2(bytecode); + + addresses.l1NullifierProxyImpl = contractAddress; + } + function upgradeImplementation(address proxy, address implementation) internal { bytes memory proxyAdminUpgradeData = abi.encodeCall( ProxyAdmin.upgrade, @@ -103,37 +129,18 @@ contract SetupLegacyBridge is Script { } function setParamsForDummyBridge() internal { - (address l2TokenBeacon, bytes32 l2TokenBeaconHash) = calculateTokenBeaconAddress(); + (address l2TokenBeacon, bytes32 l2TokenBeaconHash) = L2LegacySharedBridgeTestHelper + .calculateTestL2TokenBeaconAddress( + addresses.erc20BridgeProxy, + addresses.l1Nullifier, + Ownable2StepUpgradeable(addresses.l1Nullifier).owner() + ); + DummyL1ERC20Bridge bridge = DummyL1ERC20Bridge(addresses.erc20BridgeProxy); vm.broadcast(); bridge.setValues(config.l2SharedBridgeAddress, l2TokenBeacon, l2TokenBeaconHash); } - function calculateTokenBeaconAddress() - internal - returns (address tokenBeaconAddress, bytes32 tokenBeaconBytecodeHash) - { - bytes memory l2StandardTokenCode = L2ContractsBytecodesLib.readStandardERC20Bytecode(); - (address l2StandardToken, ) = calculateL2Create2Address( - config.l2SharedBridgeAddress, - l2StandardTokenCode, - bytes32(0), - "" - ); - - bytes memory beaconProxy = L2ContractsBytecodesLib.readBeaconProxyBytecode(); - tokenBeaconBytecodeHash = L2ContractHelper.hashL2Bytecode(beaconProxy); - - bytes memory upgradableBeacon = L2ContractsBytecodesLib.readUpgradeableBeaconBytecode(); - - (tokenBeaconAddress, ) = calculateL2Create2Address( - config.l2SharedBridgeAddress, - upgradableBeacon, - bytes32(0), - abi.encode(l2StandardToken) - ); - } - function calculateL2Create2Address( address sender, bytes memory bytecode, diff --git a/l1-contracts/deploy-scripts/upgrade/EcosystemUpgrade.s.sol b/l1-contracts/deploy-scripts/upgrade/EcosystemUpgrade.s.sol index e0bc48627..156250d3e 100644 --- a/l1-contracts/deploy-scripts/upgrade/EcosystemUpgrade.s.sol +++ b/l1-contracts/deploy-scripts/upgrade/EcosystemUpgrade.s.sol @@ -87,6 +87,10 @@ struct FixedForceDeploymentsData { bytes32 messageRootBytecodeHash; address l2SharedBridgeLegacyImpl; address l2BridgedStandardERC20Impl; + // The forced beacon address. It is needed only for internal testing. + // MUST be equal to 0 in production. + // It will be the job of the governance to ensure that this value is set correctly. + address dangerousTestOnlyForcedBeacon; } // A subset of the ones used for tests @@ -1505,7 +1509,8 @@ contract EcosystemUpgrade is Script { ), messageRootBytecodeHash: L2ContractHelper.hashL2Bytecode(L2ContractsBytecodesLib.readMessageRootBytecode()), l2SharedBridgeLegacyImpl: addresses.expectedL2Addresses.l2SharedBridgeLegacyImpl, - l2BridgedStandardERC20Impl: addresses.expectedL2Addresses.l2BridgedStandardERC20Impl + l2BridgedStandardERC20Impl: addresses.expectedL2Addresses.l2BridgedStandardERC20Impl, + dangerousTestOnlyForcedBeacon: address(0) }); return abi.encode(data); diff --git a/l1-contracts/test/foundry/l1/integration/deploy-scripts/script-config/config-deploy-l1.toml b/l1-contracts/test/foundry/l1/integration/deploy-scripts/script-config/config-deploy-l1.toml index d52a6b704..8bcd27fa6 100644 --- a/l1-contracts/test/foundry/l1/integration/deploy-scripts/script-config/config-deploy-l1.toml +++ b/l1-contracts/test/foundry/l1/integration/deploy-scripts/script-config/config-deploy-l1.toml @@ -1,6 +1,7 @@ era_chain_id = 9 owner_address = "0x70997970C51812dc3A010C7d01b50e0d17dc79C8" testnet_verifier = true +support_l2_legacy_shared_bridge_test = false [contracts] governance_security_council_address = "0x0000000000000000000000000000000000000000" diff --git a/l1-contracts/test/foundry/l1/integration/upgrade-envs/script-config/mainnet.toml b/l1-contracts/test/foundry/l1/integration/upgrade-envs/script-config/mainnet.toml index ec72115a2..4089eb029 100644 --- a/l1-contracts/test/foundry/l1/integration/upgrade-envs/script-config/mainnet.toml +++ b/l1-contracts/test/foundry/l1/integration/upgrade-envs/script-config/mainnet.toml @@ -1,6 +1,7 @@ era_chain_id = 324 owner_address = "8f7a9912416e8adc4d9c21fae1415d3318a11897" testnet_verifier = false +support_l2_legacy_shared_bridge_test = false [contracts] max_number_of_chains = 100 diff --git a/system-contracts/contracts/L2GatewayUpgradeHelper.sol b/system-contracts/contracts/L2GatewayUpgradeHelper.sol index 5c226e623..c9ebc2ac7 100644 --- a/system-contracts/contracts/L2GatewayUpgradeHelper.sol +++ b/system-contracts/contracts/L2GatewayUpgradeHelper.sol @@ -144,9 +144,16 @@ library L2GatewayUpgradeHelper { address deployedTokenBeacon; bool contractsDeployedAlready; if (additionalForceDeploymentsData.l2LegacySharedBridge != address(0)) { - deployedTokenBeacon = address( - IL2SharedBridgeLegacy(additionalForceDeploymentsData.l2LegacySharedBridge).l2TokenBeacon() - ); + // In production, the `fixedForceDeploymentsData.dangerousTestOnlyForcedBeacon` must always + // be equal to 0. It is only for simplifying testing. + if (fixedForceDeploymentsData.dangerousTestOnlyForcedBeacon == address(0)) { + deployedTokenBeacon = address( + IL2SharedBridgeLegacy(additionalForceDeploymentsData.l2LegacySharedBridge).l2TokenBeacon() + ); + } else { + deployedTokenBeacon = fixedForceDeploymentsData.dangerousTestOnlyForcedBeacon; + } + contractsDeployedAlready = true; } diff --git a/system-contracts/contracts/interfaces/IL2GenesisUpgrade.sol b/system-contracts/contracts/interfaces/IL2GenesisUpgrade.sol index 14f05304e..d86cb70a6 100644 --- a/system-contracts/contracts/interfaces/IL2GenesisUpgrade.sol +++ b/system-contracts/contracts/interfaces/IL2GenesisUpgrade.sol @@ -31,6 +31,10 @@ struct FixedForceDeploymentsData { bytes32 messageRootBytecodeHash; address l2SharedBridgeLegacyImpl; address l2BridgedStandardERC20Impl; + // The forced beacon address. It is needed only for internal testing. + // MUST be equal to 0 in production. + // It will be the job of the governance to ensure that this value is set correctly. + address dangerousTestOnlyForcedBeacon; } interface IL2GenesisUpgrade { diff --git a/system-contracts/test/L2GenesisUpgrade.spec.ts b/system-contracts/test/L2GenesisUpgrade.spec.ts index f2db34fb2..a76100dc5 100644 --- a/system-contracts/test/L2GenesisUpgrade.spec.ts +++ b/system-contracts/test/L2GenesisUpgrade.spec.ts @@ -97,7 +97,7 @@ describe("L2GenesisUpgrade tests", function () { fixedForceDeploymentsData = ethers.utils.defaultAbiCoder.encode( [ - "tuple(uint256 l1ChainId, uint256 eraChainId, address l1AssetRouter, bytes32 l2TokenProxyBytecodeHash, address aliasedL1Governance, uint256 maxNumberOfZKChains, bytes32 bridgehubBytecodeHash, bytes32 l2AssetRouterBytecodeHash, bytes32 l2NtvBytecodeHash, bytes32 messageRootBytecodeHash, address l2SharedBridgeLegacyImpl, address l2BridgedStandardERC20Impl)", + "tuple(uint256 l1ChainId, uint256 eraChainId, address l1AssetRouter, bytes32 l2TokenProxyBytecodeHash, address aliasedL1Governance, uint256 maxNumberOfZKChains, bytes32 bridgehubBytecodeHash, bytes32 l2AssetRouterBytecodeHash, bytes32 l2NtvBytecodeHash, bytes32 messageRootBytecodeHash, address l2SharedBridgeLegacyImpl, address l2BridgedStandardERC20Impl, address dangerousTestOnlyForcedBeacon)", ], [ { @@ -114,6 +114,7 @@ describe("L2GenesisUpgrade tests", function () { // For genesis upgrade these values will always be zero l2SharedBridgeLegacyImpl: ethers.constants.AddressZero, l2BridgedStandardERC20Impl: ethers.constants.AddressZero, + dangerousTestOnlyForcedBeacon: ethers.constants.AddressZero, }, ] ); From 02d6fbb23baab553f266795c3018f3c40d0cc8d2 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 10 Dec 2024 10:39:46 +0100 Subject: [PATCH 32/57] Fix audittens L07 (#33) --- .../bridge/asset-router/L2AssetRouter.sol | 2 +- .../contracts/common/L1ContractErrors.sol | 2 ++ .../common/libraries/L2ContractHelper.sol | 17 +++++++++++++ .../contracts/governance/L2AdminFactory.sol | 8 +++++-- .../governance/PermanentRestriction.sol | 24 ++++++++++--------- .../Governance/PermanentRestriction.t.sol | 9 ++----- .../unit/L2AdminFactory/L2AdminFactory.t.sol | 8 +++---- 7 files changed, 44 insertions(+), 26 deletions(-) diff --git a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol index 74b0a5393..55cf39280 100644 --- a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol @@ -157,7 +157,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { function withdrawToken(address _l2NativeToken, bytes memory _assetData) public returns (bytes32) { bytes32 recordedAssetId = INativeTokenVault(L2_NATIVE_TOKEN_VAULT_ADDR).assetId(_l2NativeToken); uint256 recordedOriginChainId = INativeTokenVault(L2_NATIVE_TOKEN_VAULT_ADDR).originChainId(recordedAssetId); - if (recordedOriginChainId != block.chaind && recordedOriginChainId != 0) { + if (recordedOriginChainId != block.chainid && recordedOriginChainId != 0) { revert AssetIdNotSupported(recordedAssetId); } bytes32 assetId = _ensureTokenRegisteredWithNTV(_l2NativeToken); diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 973baa5a0..d15671b5a 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -436,6 +436,8 @@ error AssetRouterAllowanceNotZero(); error BurningNativeWETHNotSupported(); // 0xb20b58ce error NoLegacySharedBridge(); +// 0x8e3ce3cb +error TooHighDeploymentNonce(); enum SharedBridgeKey { PostUpgradeFirstBatch, diff --git a/l1-contracts/contracts/common/libraries/L2ContractHelper.sol b/l1-contracts/contracts/common/libraries/L2ContractHelper.sol index 2d1a26c1f..b81bc99d3 100644 --- a/l1-contracts/contracts/common/libraries/L2ContractHelper.sol +++ b/l1-contracts/contracts/common/libraries/L2ContractHelper.sol @@ -49,6 +49,9 @@ library L2ContractHelper { /// @dev The prefix used to create CREATE2 addresses. bytes32 private constant CREATE2_PREFIX = keccak256("zksyncCreate2"); + /// @dev Prefix used during derivation of account addresses using CREATE + bytes32 private constant CREATE_PREFIX = 0x63bae3a9951d38e8a3fbb7b70909afc1200610fc5bc55ade242f815974674f23; + /// @notice Sends L2 -> L1 arbitrary-long message through the system contract messenger. /// @param _message Data to be sent to L1. /// @return keccak256 hash of the sent message. @@ -131,6 +134,20 @@ library L2ContractHelper { return address(uint160(uint256(data))); } + /// @notice Calculates the address of a deployed contract via create + /// @param _sender The account that deploys the contract. + /// @param _senderNonce The deploy nonce of the sender's account. + /// NOTE: L2 create derivation is different from L1 derivation! + function computeCreateAddress(address _sender, uint256 _senderNonce) internal pure returns (address) { + // No collision is possible with the Ethereum's CREATE, since + // the prefix begins with 0x63.... + bytes32 hash = keccak256( + bytes.concat(CREATE_PREFIX, bytes32(uint256(uint160(_sender))), bytes32(_senderNonce)) + ); + + return address(uint160(uint256(hash))); + } + /// @notice Hashes the L2 bytecodes and returns them in the format in which they are processed by the bootloader function hashFactoryDeps(bytes[] memory _factoryDeps) internal pure returns (uint256[] memory hashedFactoryDeps) { uint256 factoryDepsLen = _factoryDeps.length; diff --git a/l1-contracts/contracts/governance/L2AdminFactory.sol b/l1-contracts/contracts/governance/L2AdminFactory.sol index 44515c117..ae81b9f97 100644 --- a/l1-contracts/contracts/governance/L2AdminFactory.sol +++ b/l1-contracts/contracts/governance/L2AdminFactory.sol @@ -31,7 +31,7 @@ contract L2AdminFactory { /// @notice Deploys a new L2 admin contract. /// @return admin The address of the deployed admin contract. // solhint-disable-next-line gas-calldata-parameters - function deployAdmin(address[] memory _additionalRestrictions, bytes32 _salt) external returns (address admin) { + function deployAdmin(address[] memory _additionalRestrictions) external returns (address admin) { // Even though the chain admin will likely perform similar checks, // we keep those here just in case, since it is not expensive, while allowing to fail fast. _validateRestrctions(_additionalRestrictions); @@ -45,7 +45,11 @@ contract L2AdminFactory { restrictions[requiredRestrictions.length + i] = _additionalRestrictions[i]; } - admin = address(new ChainAdmin{salt: _salt}(restrictions)); + // Note, that we are using CREATE instead of CREATE2 to prevent + // an attack where malicious deployer could select malicious `seed1` and `seed2` where + // this factory with `seed1` produces the same address as some other random factory with `seed2`, + // allowing to deploy a malicious contract. + admin = address(new ChainAdmin(restrictions)); } /// @notice Checks that the provided list of restrictions is correct. diff --git a/l1-contracts/contracts/governance/PermanentRestriction.sol b/l1-contracts/contracts/governance/PermanentRestriction.sol index 71f92e12b..85d8d1e87 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 {TooHighDeploymentNonce, 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"; @@ -24,6 +24,11 @@ import {IPermanentRestriction} from "./IPermanentRestriction.sol"; /// has at least this amount. uint256 constant MIN_GAS_FOR_FALLABLE_CALL = 5_000_000; +/// @dev The value up to which the nonces of the L2AdminDeployer could be used. This is needed +/// to limit the impact of the birthday paradox attack, where an attack could craft a malicious +/// address on L1. +uint256 constant MAX_ALLOWED_NONCE = (1 << 48); + /// @title PermanentRestriction contract /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -98,18 +103,15 @@ contract PermanentRestriction is Restriction, IPermanentRestriction, Ownable2Ste } /// @notice Whitelists a certain L2 admin. - /// @param deploymentSalt The salt for the deployment. - /// @param l2BytecodeHash The hash of the L2 bytecode. - /// @param constructorInputHash The hash of the constructor data for the deployment. - function allowL2Admin(bytes32 deploymentSalt, bytes32 l2BytecodeHash, bytes32 constructorInputHash) external { + /// @param deploymentNonce The deployment nonce of the `L2_ADMIN_FACTORY` used for the deployment. + function allowL2Admin(uint256 deploymentNonce) external { + if (deploymentNonce > MAX_ALLOWED_NONCE) { + revert TooHighDeploymentNonce(); + } + // We do not do any additional validations for constructor data or the bytecode, // we expect that only admins of the allowed format are to be deployed. - address expectedAddress = L2ContractHelper.computeCreate2Address( - L2_ADMIN_FACTORY, - deploymentSalt, - l2BytecodeHash, - constructorInputHash - ); + address expectedAddress = L2ContractHelper.computeCreateAddress(L2_ADMIN_FACTORY, deploymentNonce); if (allowedL2Admins[expectedAddress]) { revert AlreadyWhitelisted(expectedAddress); 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 2ed7a489d..32759bea2 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 @@ -345,16 +345,11 @@ contract PermanentRestrictionTest is ChainTypeManagerTest { } function test_validateMigrationToL2() public { - address expectedAddress = L2ContractHelper.computeCreate2Address( - L2_FACTORY_ADDR, - bytes32(0), - bytes32(0), - bytes32(0) - ); + address expectedAddress = L2ContractHelper.computeCreateAddress(L2_FACTORY_ADDR, uint256(0)); vm.expectEmit(true, false, false, true); emit IPermanentRestriction.AllowL2Admin(expectedAddress); - permRestriction.allowL2Admin(bytes32(0), bytes32(0), bytes32(0)); + permRestriction.allowL2Admin(uint256(0)); Call memory call = _encodeMigraationCall(true, true, true, true, true, expectedAddress); diff --git a/l1-contracts/test/foundry/l2/unit/L2AdminFactory/L2AdminFactory.t.sol b/l1-contracts/test/foundry/l2/unit/L2AdminFactory/L2AdminFactory.t.sol index 95bcb8304..7da200111 100644 --- a/l1-contracts/test/foundry/l2/unit/L2AdminFactory/L2AdminFactory.t.sol +++ b/l1-contracts/test/foundry/l2/unit/L2AdminFactory/L2AdminFactory.t.sol @@ -42,7 +42,7 @@ contract L2AdminFactoryTest is Test { additionalRestrictions[0] = invalidRestriction; vm.expectRevert(abi.encodeWithSelector(NotARestriction.selector, address(invalidRestriction))); - factory.deployAdmin(additionalRestrictions, bytes32(0)); + factory.deployAdmin(additionalRestrictions); } function testL2AdminFactory() public { @@ -58,9 +58,7 @@ contract L2AdminFactoryTest is Test { allRestrictions[0] = requiredRestrictions[0]; allRestrictions[1] = additionalRestrictions[0]; - bytes32 salt = keccak256("salt"); - - address admin = factory.deployAdmin(additionalRestrictions, salt); + address admin = factory.deployAdmin(additionalRestrictions); // Now, we need to check whether it would be able to accept such an admin PermanentRestriction restriction = new PermanentRestriction(IBridgehub(address(0)), address(factory)); @@ -72,6 +70,6 @@ contract L2AdminFactoryTest is Test { vm.expectEmit(true, false, false, true); emit IPermanentRestriction.AllowL2Admin(admin); - restriction.allowL2Admin(salt, codeHash, keccak256(abi.encode(allRestrictions))); + restriction.allowL2Admin(uint256(0)); } } From d947519c28242a246b40b975c6becde51ef85f88 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 10 Dec 2024 10:40:14 +0100 Subject: [PATCH 33/57] Fix audittens i26 (#62) --- l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol b/l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol index c57c3a8f9..0be1a0e01 100644 --- a/l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol +++ b/l1-contracts/contracts/bridge/asset-router/AssetRouterBase.sol @@ -50,7 +50,7 @@ abstract contract AssetRouterBase is IAssetRouterBase, Ownable2StepUpgradeable, * variables without shifting down storage in the inheritance chain. * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps */ - uint256[47] private __gap; + uint256[48] private __gap; /// @notice Checks that the message sender is the bridgehub. modifier onlyBridgehub() { From 924bf427f00343fe7d9b793a340c377b072abce3 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 12:13:02 +0100 Subject: [PATCH 34/57] fix i24 --- l1-contracts/contracts/common/L1ContractErrors.sol | 2 ++ .../state-transition/chain-deps/facets/Executor.sol | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 3c49c4976..f4f0289e4 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -383,6 +383,8 @@ error WrappedBaseTokenAlreadyRegistered(); // 0xde4c0b96 error InvalidNTVBurnData(); +// 0xbe7193d4 +error InvalidSystemLogsLength(); enum SharedBridgeKey { PostUpgradeFirstBatch, diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol index 90d84bf70..22b5cb70a 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol @@ -15,7 +15,7 @@ import {L2_BOOTLOADER_ADDRESS, L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, L2_SYSTE import {IChainTypeManager} from "../../IChainTypeManager.sol"; import {PriorityTree, PriorityOpsBatchInfo} from "../../libraries/PriorityTree.sol"; import {IL1DAValidator, L1DAValidatorOutput} from "../../chain-interfaces/IL1DAValidator.sol"; -import {MissingSystemLogs, BatchNumberMismatch, TimeNotReached, ValueMismatch, HashMismatch, NonIncreasingTimestamp, TimestampError, InvalidLogSender, TxHashMismatch, UnexpectedSystemLog, LogAlreadyProcessed, InvalidProtocolVersion, CanOnlyProcessOneBatch, BatchHashMismatch, UpgradeBatchNumberIsNotZero, NonSequentialBatch, CantExecuteUnprovenBatches, SystemLogsSizeTooBig, InvalidNumberOfBlobs, VerifiedBatchesExceedsCommittedBatches, InvalidProof, RevertedBatchNotAfterNewLastBatch, CantRevertExecutedBatch, L2TimestampTooBig, PriorityOperationsRollingHashMismatch} from "../../../common/L1ContractErrors.sol"; +import {InvalidSystemLogsLength, MissingSystemLogs, BatchNumberMismatch, TimeNotReached, ValueMismatch, HashMismatch, NonIncreasingTimestamp, TimestampError, InvalidLogSender, TxHashMismatch, UnexpectedSystemLog, LogAlreadyProcessed, InvalidProtocolVersion, CanOnlyProcessOneBatch, BatchHashMismatch, UpgradeBatchNumberIsNotZero, NonSequentialBatch, CantExecuteUnprovenBatches, SystemLogsSizeTooBig, InvalidNumberOfBlobs, VerifiedBatchesExceedsCommittedBatches, InvalidProof, RevertedBatchNotAfterNewLastBatch, CantRevertExecutedBatch, L2TimestampTooBig, PriorityOperationsRollingHashMismatch} from "../../../common/L1ContractErrors.sol"; import {ChainWasMigrated, InvalidBatchesDataLength, MismatchL2DAValidator, MismatchNumberOfLayer1Txs, PriorityOpsDataLeftPathLengthIsNotZero, PriorityOpsDataRightPathLengthIsNotZero, PriorityOpsDataItemHashesLengthIsNotZero} from "../../L1StateTransitionErrors.sol"; // While formally the following import is not used, it is needed to inherit documentation from it @@ -163,6 +163,11 @@ contract ExecutorFacet is ZKChainBase, IExecutor { // linear traversal of the logs uint256 logsLength = emittedL2Logs.length; + + if (logsLength % L2_TO_L1_LOG_SERIALIZE_SIZE != 0) { + revert InvalidSystemLogsLength(); + } + for (uint256 i = 0; i < logsLength; i = i.uncheckedAdd(L2_TO_L1_LOG_SERIALIZE_SIZE)) { // Extract the values to be compared to/used such as the log sender, key, and value // slither-disable-next-line unused-return From 96e3a9b866ce5c21fd2a1fb9bd23959481728d23 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 12:17:46 +0100 Subject: [PATCH 35/57] fix i28 --- .../contracts/state-transition/chain-deps/facets/Mailbox.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol index 17a55f0aa..f27ca7005 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol @@ -412,7 +412,7 @@ contract MailboxFacet is ZKChainBase, IMailbox { ) internal view returns (BridgehubL2TransactionRequest memory) { // solhint-disable-next-line func-named-parameters bytes memory data = abi.encodeCall( - IBridgehub(s.bridgehub).forwardTransactionOnGateway, + IBridgehub.forwardTransactionOnGateway, (_chainId, _canonicalTxHash, _expirationTimestamp) ); return From 28829113b2972231a0c8866a7621b9e6f4236401 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Tue, 10 Dec 2024 12:49:41 +0100 Subject: [PATCH 36/57] Fix ci (#65) --- l1-contracts/contracts/governance/L2AdminFactory.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/governance/L2AdminFactory.sol b/l1-contracts/contracts/governance/L2AdminFactory.sol index 2b38ac887..95a2a2312 100644 --- a/l1-contracts/contracts/governance/L2AdminFactory.sol +++ b/l1-contracts/contracts/governance/L2AdminFactory.sol @@ -53,7 +53,7 @@ contract L2AdminFactory { // this factory with `seed1` produces the same address as some other random factory with `seed2`, // allowing to deploy a malicious contract. admin = address(new ChainAdmin(restrictions)); - + emit AdminDeployed(address(admin)); } From 51363b9c55d91de38e086132b7cfc0cf013cae1f Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 14:25:06 +0100 Subject: [PATCH 37/57] fix i29 --- .../contracts/bridgehub/Bridgehub.sol | 72 +++++++++---------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/l1-contracts/contracts/bridgehub/Bridgehub.sol b/l1-contracts/contracts/bridgehub/Bridgehub.sol index d583be353..d0385800d 100644 --- a/l1-contracts/contracts/bridgehub/Bridgehub.sol +++ b/l1-contracts/contracts/bridgehub/Bridgehub.sol @@ -718,20 +718,20 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus revert SLNotWhitelisted(); } - BridgehubBurnCTMAssetData memory bridgehubData = abi.decode(_data, (BridgehubBurnCTMAssetData)); - if (_assetId != ctmAssetIdFromChainId(bridgehubData.chainId)) { - revert IncorrectChainAssetId(_assetId, ctmAssetIdFromChainId(bridgehubData.chainId)); + BridgehubBurnCTMAssetData memory bridgehubBurnData = abi.decode(_data, (BridgehubBurnCTMAssetData)); + if (_assetId != ctmAssetIdFromChainId(bridgehubBurnData.chainId)) { + revert IncorrectChainAssetId(_assetId, ctmAssetIdFromChainId(bridgehubBurnData.chainId)); } - if (settlementLayer[bridgehubData.chainId] != block.chainid) { - revert NotCurrentSL(settlementLayer[bridgehubData.chainId], block.chainid); + if (settlementLayer[bridgehubBurnData.chainId] != block.chainid) { + revert NotCurrentSL(settlementLayer[bridgehubBurnData.chainId], block.chainid); } - settlementLayer[bridgehubData.chainId] = _settlementChainId; + settlementLayer[bridgehubBurnData.chainId] = _settlementChainId; - if (whitelistedSettlementLayers[bridgehubData.chainId]) { + if (whitelistedSettlementLayers[bridgehubBurnData.chainId]) { revert SettlementLayersMustSettleOnL1(); } - address zkChain = zkChainMap.get(bridgehubData.chainId); + address zkChain = zkChainMap.get(bridgehubBurnData.chainId); if (zkChain == address(0)) { revert HyperchainNotRegistered(); } @@ -739,26 +739,26 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus revert IncorrectSender(_originalCaller, IZKChain(zkChain).getAdmin()); } - bytes memory ctmMintData = IChainTypeManager(chainTypeManager[bridgehubData.chainId]).forwardedBridgeBurn( - bridgehubData.chainId, - bridgehubData.ctmData + bytes memory ctmMintData = IChainTypeManager(chainTypeManager[bridgehubBurnData.chainId]).forwardedBridgeBurn( + bridgehubBurnData.chainId, + bridgehubBurnData.ctmData ); bytes memory chainMintData = IZKChain(zkChain).forwardedBridgeBurn( _settlementChainId == L1_CHAIN_ID ? L1_SETTLEMENT_LAYER_VIRTUAL_ADDRESS : zkChainMap.get(_settlementChainId), _originalCaller, - bridgehubData.chainData + bridgehubBurnData.chainData ); BridgehubMintCTMAssetData memory bridgeMintStruct = BridgehubMintCTMAssetData({ - chainId: bridgehubData.chainId, - baseTokenAssetId: baseTokenAssetId[bridgehubData.chainId], + chainId: bridgehubBurnData.chainId, + baseTokenAssetId: baseTokenAssetId[bridgehubBurnData.chainId], ctmData: ctmMintData, chainData: chainMintData }); bridgehubMintData = abi.encode(bridgeMintStruct); - emit MigrationStarted(bridgehubData.chainId, _assetId, _settlementChainId); + emit MigrationStarted(bridgehubBurnData.chainId, _assetId, _settlementChainId); } /// @dev IL1AssetHandler interface, used to receive a chain on the settlement layer. @@ -769,38 +769,38 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus bytes32 _assetId, bytes calldata _bridgehubMintData ) external payable override requireZeroValue(msg.value) onlyAssetRouter whenMigrationsNotPaused { - BridgehubMintCTMAssetData memory bridgehubData = abi.decode(_bridgehubMintData, (BridgehubMintCTMAssetData)); + BridgehubMintCTMAssetData memory bridgehubMintData = abi.decode(_bridgehubMintData, (BridgehubMintCTMAssetData)); address ctm = ctmAssetIdToAddress[_assetId]; if (ctm == address(0)) { revert NoCTMForAssetId(_assetId); } - if (settlementLayer[bridgehubData.chainId] == block.chainid) { + if (settlementLayer[bridgehubMintData.chainId] == block.chainid) { revert AlreadyCurrentSL(block.chainid); } - settlementLayer[bridgehubData.chainId] = block.chainid; - chainTypeManager[bridgehubData.chainId] = ctm; - baseTokenAssetId[bridgehubData.chainId] = bridgehubData.baseTokenAssetId; + settlementLayer[bridgehubMintData.chainId] = block.chainid; + chainTypeManager[bridgehubMintData.chainId] = ctm; + baseTokenAssetId[bridgehubMintData.chainId] = bridgehubMintData.baseTokenAssetId; // To keep `assetIdIsRegistered` consistent, we'll also automatically register the base token. // It is assumed that if the bridging happened, the token was approved on L1 already. - assetIdIsRegistered[bridgehubData.baseTokenAssetId] = true; + assetIdIsRegistered[bridgehubMintData.baseTokenAssetId] = true; - address zkChain = getZKChain(bridgehubData.chainId); + address zkChain = getZKChain(bridgehubMintData.chainId); bool contractAlreadyDeployed = zkChain != address(0); if (!contractAlreadyDeployed) { - zkChain = IChainTypeManager(ctm).forwardedBridgeMint(bridgehubData.chainId, bridgehubData.ctmData); + zkChain = IChainTypeManager(ctm).forwardedBridgeMint(bridgehubMintData.chainId, bridgehubMintData.ctmData); if (zkChain == address(0)) { - revert ChainIdNotRegistered(bridgehubData.chainId); + revert ChainIdNotRegistered(bridgehubMintData.chainId); } // We want to allow any chain to be migrated, - _registerNewZKChain(bridgehubData.chainId, zkChain, false); - messageRoot.addNewChain(bridgehubData.chainId); + _registerNewZKChain(bridgehubMintData.chainId, zkChain, false); + messageRoot.addNewChain(bridgehubMintData.chainId); } - IZKChain(zkChain).forwardedBridgeMint(bridgehubData.chainData, contractAlreadyDeployed); + IZKChain(zkChain).forwardedBridgeMint(bridgehubMintData.chainData, contractAlreadyDeployed); - emit MigrationFinalized(bridgehubData.chainId, _assetId, zkChain); + emit MigrationFinalized(bridgehubMintData.chainId, _assetId, zkChain); } /// @dev IL1AssetHandler interface, used to undo a failed migration of a chain. @@ -813,22 +813,22 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus address _depositSender, bytes calldata _data ) external payable override requireZeroValue(msg.value) onlyAssetRouter onlyL1 { - BridgehubBurnCTMAssetData memory bridgehubData = abi.decode(_data, (BridgehubBurnCTMAssetData)); + BridgehubBurnCTMAssetData memory bridgehubBurnData = abi.decode(_data, (BridgehubBurnCTMAssetData)); - settlementLayer[bridgehubData.chainId] = block.chainid; + settlementLayer[bridgehubBurnData.chainId] = block.chainid; - IChainTypeManager(chainTypeManager[bridgehubData.chainId]).forwardedBridgeRecoverFailedTransfer({ - _chainId: bridgehubData.chainId, + IChainTypeManager(chainTypeManager[bridgehubBurnData.chainId]).forwardedBridgeRecoverFailedTransfer({ + _chainId: bridgehubBurnData.chainId, _assetInfo: _assetId, _depositSender: _depositSender, - _ctmData: bridgehubData.ctmData + _ctmData: bridgehubBurnData.ctmData }); - IZKChain(getZKChain(bridgehubData.chainId)).forwardedBridgeRecoverFailedTransfer({ - _chainId: bridgehubData.chainId, + IZKChain(getZKChain(bridgehubBurnData.chainId)).forwardedBridgeRecoverFailedTransfer({ + _chainId: bridgehubBurnData.chainId, _assetInfo: _assetId, _originalCaller: _depositSender, - _chainData: bridgehubData.chainData + _chainData: bridgehubBurnData.chainData }); } From 77ea5daa02e9cf6d345f46f498c82736e0ad5333 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 14:27:38 +0100 Subject: [PATCH 38/57] fix i30 --- l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol index a012728c9..192d44e2d 100644 --- a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol @@ -62,7 +62,7 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken /// @dev Accepts ether only from the contract that was the shared Bridge. receive() external payable { - if ((address(L1_NULLIFIER) != msg.sender) && (address(ASSET_ROUTER) != msg.sender)) { + if (address(L1_NULLIFIER) != msg.sender) { revert Unauthorized(msg.sender); } } From 8409737f85de15a5d0e08358321dd8fb55c1d373 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 14:29:43 +0100 Subject: [PATCH 39/57] fix i31 --- l1-contracts/contracts/bridgehub/Bridgehub.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/bridgehub/Bridgehub.sol b/l1-contracts/contracts/bridgehub/Bridgehub.sol index d0385800d..f8f716268 100644 --- a/l1-contracts/contracts/bridgehub/Bridgehub.sol +++ b/l1-contracts/contracts/bridgehub/Bridgehub.sol @@ -334,7 +334,7 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus revert CTMNotRegistered(); } - bytes32 ctmAssetId = keccak256(abi.encode(L1_CHAIN_ID, sender, _additionalData)); + bytes32 ctmAssetId = DataEncoding.encodeAssetId(L1_CHAIN_ID, _additionalData, sender); ctmAssetIdToAddress[ctmAssetId] = _assetAddress; ctmAssetIdFromAddress[_assetAddress] = ctmAssetId; emit AssetRegistered(ctmAssetId, _assetAddress, _additionalData, msg.sender); From d994088c5c9846cfba37b5c15b7e15fb4f8a7ded Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 14:30:11 +0100 Subject: [PATCH 40/57] fix i32 --- l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index fff304789..bf2ce8a0c 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -352,7 +352,6 @@ abstract contract NativeTokenVault is _handleChainBalanceIncrease(_chainId, _assetId, _depositAmount, true); } else { - // The Bridgehub also checks this, but we want to be sure if (msg.value != 0) { revert NonEmptyMsgValue(); } From b8970e5b9246d8c256c2565a724581205ef1d6aa Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 14:43:43 +0100 Subject: [PATCH 41/57] fix i35 --- l1-contracts/contracts/bridge/L1Nullifier.sol | 6 +++--- .../contracts/bridge/asset-router/IL1AssetRouter.sol | 2 +- .../contracts/bridge/asset-router/L1AssetRouter.sol | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/l1-contracts/contracts/bridge/L1Nullifier.sol b/l1-contracts/contracts/bridge/L1Nullifier.sol index b0bd0449b..d623fd24e 100644 --- a/l1-contracts/contracts/bridge/L1Nullifier.sol +++ b/l1-contracts/contracts/bridge/L1Nullifier.sol @@ -679,7 +679,7 @@ contract L1Nullifier is IL1Nullifier, ReentrancyGuard, Ownable2StepUpgradeable, /// method in `L1ERC20Bridge`. /// /// @param _depositSender The address of the deposit initiator. - /// @param _l1Asset The address of the deposited L1 ERC20 token. + /// @param _l1Token The address of the deposited L1 ERC20 token. /// @param _amount The amount of the deposit that failed. /// @param _l2TxHash The L2 transaction hash of the failed deposit finalization. /// @param _l2BatchNumber The L2 batch number where the deposit finalization was processed. @@ -688,7 +688,7 @@ contract L1Nullifier is IL1Nullifier, ReentrancyGuard, Ownable2StepUpgradeable, /// @param _merkleProof The Merkle proof of the processing L1 -> L2 transaction with deposit finalization. function claimFailedDepositLegacyErc20Bridge( address _depositSender, - address _l1Asset, + address _l1Token, uint256 _amount, bytes32 _l2TxHash, uint256 _l2BatchNumber, @@ -701,7 +701,7 @@ contract L1Nullifier is IL1Nullifier, ReentrancyGuard, Ownable2StepUpgradeable, bytes memory assetData = DataEncoding.encodeBridgeBurnData(_amount, address(0), address(0)); /// the legacy bridge can only be used with L1 native tokens. - bytes32 assetId = DataEncoding.encodeNTVAssetId(block.chainid, _l1Asset); + bytes32 assetId = DataEncoding.encodeNTVAssetId(block.chainid, _l1Token); _verifyAndClearFailedTransfer({ _checkedInLegacyBridge: true, diff --git a/l1-contracts/contracts/bridge/asset-router/IL1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/IL1AssetRouter.sol index 5d4c4fc5f..a9ee26525 100644 --- a/l1-contracts/contracts/bridge/asset-router/IL1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/IL1AssetRouter.sol @@ -33,7 +33,7 @@ interface IL1AssetRouter is IAssetRouterBase, IL1SharedBridgeLegacy { bytes32 indexed l2DepositTxHash, address indexed from, address to, - address l1Asset, + address l1Token, uint256 amount ); diff --git a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol index f334c468d..db9838704 100644 --- a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol @@ -601,7 +601,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { l2DepositTxHash: txHash, from: _originalCaller, to: _l2Receiver, - l1Asset: _l1Token, + l1Token: _l1Token, amount: _amount }); } From 0be70a20cc3b4db4da5999413370ca552e6f7223 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 14:44:57 +0100 Subject: [PATCH 42/57] fix i36 --- .../contracts/state-transition/libraries/BatchDecoder.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/state-transition/libraries/BatchDecoder.sol b/l1-contracts/contracts/state-transition/libraries/BatchDecoder.sol index a8af4b7ab..bca183f0d 100644 --- a/l1-contracts/contracts/state-transition/libraries/BatchDecoder.sol +++ b/l1-contracts/contracts/state-transition/libraries/BatchDecoder.sol @@ -167,7 +167,7 @@ library BatchDecoder { } uint8 encodingVersion = uint8(_executeData[0]); - if (encodingVersion == 0) { + if (encodingVersion == SUPPORTED_ENCODING_VERSION) { (executeData, priorityOpsData) = abi.decode( _executeData[1:], (IExecutor.StoredBatchInfo[], PriorityOpsBatchInfo[]) From 067b33ca0145d7083327521fa4451e9941147bcd Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 14:59:43 +0100 Subject: [PATCH 43/57] fix i37 --- l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index bf2ce8a0c..16d0bff01 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -145,7 +145,7 @@ abstract contract NativeTokenVault is } function _bridgeMintBridgedToken( - uint256 _originChainId, + uint256 _chainId, bytes32 _assetId, bytes calldata _data ) internal virtual returns (address receiver, uint256 amount) { @@ -159,12 +159,12 @@ abstract contract NativeTokenVault is if (token == address(0)) { token = _ensureAndSaveTokenDeployed(_assetId, originToken, erc20Data); } - _handleChainBalanceDecrease(_originChainId, _assetId, amount, false); + _handleChainBalanceDecrease(_chainId, _assetId, amount, false); IBridgedStandardToken(token).bridgeMint(receiver, amount); } function _bridgeMintNativeToken( - uint256 _originChainId, + uint256 _chainId, bytes32 _assetId, bytes calldata _data ) internal returns (address receiver, uint256 amount) { @@ -172,7 +172,7 @@ abstract contract NativeTokenVault is // slither-disable-next-line unused-return (, receiver, , amount, ) = DataEncoding.decodeBridgeMintData(_data); - _handleChainBalanceDecrease(_originChainId, _assetId, amount, true); + _handleChainBalanceDecrease(_chainId, _assetId, amount, true); _withdrawFunds(_assetId, receiver, token, amount); } From 2a7723f186e5e9ea3c5f772f4a5fb674802b90fc Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 15:01:14 +0100 Subject: [PATCH 44/57] fix i38 --- l1-contracts/contracts/bridge/asset-router/IAssetRouterBase.sol | 2 +- l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/bridge/asset-router/IAssetRouterBase.sol b/l1-contracts/contracts/bridge/asset-router/IAssetRouterBase.sol index 8308ad8c9..2be571ddc 100644 --- a/l1-contracts/contracts/bridge/asset-router/IAssetRouterBase.sol +++ b/l1-contracts/contracts/bridge/asset-router/IAssetRouterBase.sol @@ -45,7 +45,7 @@ interface IAssetRouterBase { address assetDeploymentTracker ); - event AssetHandlerRegistered(bytes32 indexed assetId, address indexed _assetAddress); + event AssetHandlerRegistered(bytes32 indexed assetId, address indexed _assetHandlerAddress); event DepositFinalizedAssetRouter(uint256 indexed chainId, bytes32 indexed assetId, bytes assetData); diff --git a/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol index 8867c8856..1c9b0d1ea 100644 --- a/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol @@ -30,7 +30,7 @@ interface IL2AssetRouter is IAssetRouterBase { /// @dev Used to set the assetHandlerAddress for a given assetId. /// @dev Will be used by ZK Gateway - function setAssetHandlerAddress(uint256 _originChainId, bytes32 _assetId, address _assetAddress) external; + function setAssetHandlerAddress(uint256 _originChainId, bytes32 _assetId, address _assetHandlerAddress) external; /// @notice Function that allows native token vault to register itself as the asset handler for /// a legacy asset. From 235b42ea8f0dcfbe54a9d3b002322386481b01cf Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 15:04:44 +0100 Subject: [PATCH 45/57] i39 --- l1-contracts/contracts/bridge/L1Nullifier.sol | 8 ++++++-- l1-contracts/contracts/common/L1ContractErrors.sol | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/bridge/L1Nullifier.sol b/l1-contracts/contracts/bridge/L1Nullifier.sol index d623fd24e..89ee00277 100644 --- a/l1-contracts/contracts/bridge/L1Nullifier.sol +++ b/l1-contracts/contracts/bridge/L1Nullifier.sol @@ -28,7 +28,7 @@ import {DataEncoding} from "../common/libraries/DataEncoding.sol"; import {IBridgehub} from "../bridgehub/IBridgehub.sol"; import {L2_BASE_TOKEN_SYSTEM_CONTRACT_ADDR, L2_ASSET_ROUTER_ADDR} from "../common/L2ContractAddresses.sol"; import {DataEncoding} from "../common/libraries/DataEncoding.sol"; -import {Unauthorized, SharedBridgeKey, DepositExists, AddressAlreadySet, InvalidProof, DepositDoesNotExist, SharedBridgeValueNotSet, WithdrawalAlreadyFinalized, L2WithdrawalMessageWrongLength, InvalidSelector, SharedBridgeValueNotSet, ZeroAddress} from "../common/L1ContractErrors.sol"; +import {LegacyBridgeNotSet, Unauthorized, SharedBridgeKey, DepositExists, AddressAlreadySet, InvalidProof, DepositDoesNotExist, SharedBridgeValueNotSet, WithdrawalAlreadyFinalized, L2WithdrawalMessageWrongLength, InvalidSelector, SharedBridgeValueNotSet, ZeroAddress} from "../common/L1ContractErrors.sol"; import {WrongL2Sender, NativeTokenVaultAlreadySet, EthTransferFailed, WrongMsgLength} from "./L1BridgeContractErrors.sol"; /// @author Matter Labs @@ -754,11 +754,15 @@ contract L1Nullifier is IL1Nullifier, ReentrancyGuard, Ownable2StepUpgradeable, /// @dev We use a deprecated field to support L2->L1 legacy withdrawals, which were started /// by the legacy bridge. address legacyL2Bridge = __DEPRECATED_l2BridgeAddress[_chainId]; + if (legacyL2Bridge == address(0)) { + revert LegacyBridgeNotSet(); + } + FinalizeL1DepositParams memory finalizeWithdrawalParams = FinalizeL1DepositParams({ chainId: _chainId, l2BatchNumber: _l2BatchNumber, l2MessageIndex: _l2MessageIndex, - l2Sender: legacyL2Bridge == address(0) ? L2_ASSET_ROUTER_ADDR : legacyL2Bridge, + l2Sender: legacyL2Bridge, l2TxNumberInBatch: _l2TxNumberInBatch, message: _message, merkleProof: _merkleProof diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index f4f0289e4..7303618b0 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -385,6 +385,8 @@ error WrappedBaseTokenAlreadyRegistered(); error InvalidNTVBurnData(); // 0xbe7193d4 error InvalidSystemLogsLength(); +// 0x8efef97a +error LegacyBridgeNotSet(); enum SharedBridgeKey { PostUpgradeFirstBatch, From 35b17442efb979f74c24e8cbfe14b682cdca18c4 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 16:15:42 +0100 Subject: [PATCH 46/57] fix i40 --- .../contracts/bridge/asset-router/L2AssetRouter.sol | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol index 0339829e6..47651c07f 100644 --- a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol @@ -13,6 +13,7 @@ import {IL1ERC20Bridge} from "../interfaces/IL1ERC20Bridge.sol"; import {IBridgehub} from "../../bridgehub/IBridgehub.sol"; import {AddressAliasHelper} from "../../vendor/AddressAliasHelper.sol"; +import {ReentrancyGuard} from "../../common/ReentrancyGuard.sol"; import {L2_NATIVE_TOKEN_VAULT_ADDR, L2_BRIDGEHUB_ADDR} from "../../common/L2ContractAddresses.sol"; import {L2ContractHelper} from "../../common/libraries/L2ContractHelper.sol"; @@ -23,7 +24,7 @@ import {TokenNotLegacy, EmptyAddress, InvalidCaller, AmountMustBeGreaterThanZero /// @custom:security-contact security@matterlabs.dev /// @notice The "default" bridge implementation for the ERC20 tokens. Note, that it does not /// support any custom token logic, i.e. rebase tokens' functionality is not supported. -contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { +contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard { /// @dev The address of the L2 legacy shared bridge. address public immutable L2_LEGACY_SHARED_BRIDGE; @@ -84,7 +85,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { address _legacySharedBridge, bytes32 _baseTokenAssetId, address _aliasedOwner - ) AssetRouterBase(_l1ChainId, _eraChainId, IBridgehub(L2_BRIDGEHUB_ADDR)) { + ) AssetRouterBase(_l1ChainId, _eraChainId, IBridgehub(L2_BRIDGEHUB_ADDR)) reentrancyGuardInitializer { L2_LEGACY_SHARED_BRIDGE = _legacySharedBridge; if (_l1AssetRouter == address(0)) { revert EmptyAddress(); @@ -131,7 +132,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { uint256, bytes32 _assetId, bytes calldata _transferData - ) public payable override(AssetRouterBase, IAssetRouterBase) onlyAssetRouterCounterpartOrSelf(L1_CHAIN_ID) { + ) public payable override(AssetRouterBase, IAssetRouterBase) onlyAssetRouterCounterpartOrSelf(L1_CHAIN_ID) nonReentrant { if (_assetId == BASE_TOKEN_ASSET_ID) { revert AssetIdNotSupported(BASE_TOKEN_ASSET_ID); } @@ -146,7 +147,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { /// that rely on it must be upgradeable. /// @param _assetId The asset id of the withdrawn asset /// @param _assetData The data that is passed to the asset handler contract - function withdraw(bytes32 _assetId, bytes memory _assetData) public override returns (bytes32) { + function withdraw(bytes32 _assetId, bytes memory _assetData) nonReentrant public override returns (bytes32) { return _withdrawSender(_assetId, _assetData, msg.sender, true); } @@ -285,7 +286,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { /// @param _l1Receiver The account address that should receive funds on L1 /// @param _l2Token The L2 token address which is withdrawn /// @param _amount The total amount of tokens to be withdrawn - function withdraw(address _l1Receiver, address _l2Token, uint256 _amount) external { + function withdraw(address _l1Receiver, address _l2Token, uint256 _amount) external nonReentrant { if (_amount == 0) { revert AmountMustBeGreaterThanZero(); } @@ -303,7 +304,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { address _l2Token, uint256 _amount, address _sender - ) external onlyLegacyBridge { + ) external onlyLegacyBridge nonReentrant { _withdrawLegacy(_l1Receiver, _l2Token, _amount, _sender); } From 0c1905e7c73fa2ce2bc03a41945836e5e91b57cd Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 16:17:22 +0100 Subject: [PATCH 47/57] fix lint + compile --- .../contracts/bridge/asset-router/L2AssetRouter.sol | 10 ++++++++-- l1-contracts/contracts/bridgehub/Bridgehub.sol | 5 ++++- l1-contracts/contracts/governance/L2AdminFactory.sol | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol index 47651c07f..57bb67e9d 100644 --- a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol @@ -132,7 +132,13 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard { uint256, bytes32 _assetId, bytes calldata _transferData - ) public payable override(AssetRouterBase, IAssetRouterBase) onlyAssetRouterCounterpartOrSelf(L1_CHAIN_ID) nonReentrant { + ) + public + payable + override(AssetRouterBase, IAssetRouterBase) + onlyAssetRouterCounterpartOrSelf(L1_CHAIN_ID) + nonReentrant + { if (_assetId == BASE_TOKEN_ASSET_ID) { revert AssetIdNotSupported(BASE_TOKEN_ASSET_ID); } @@ -147,7 +153,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard { /// that rely on it must be upgradeable. /// @param _assetId The asset id of the withdrawn asset /// @param _assetData The data that is passed to the asset handler contract - function withdraw(bytes32 _assetId, bytes memory _assetData) nonReentrant public override returns (bytes32) { + function withdraw(bytes32 _assetId, bytes memory _assetData) public override nonReentrant returns (bytes32) { return _withdrawSender(_assetId, _assetData, msg.sender, true); } diff --git a/l1-contracts/contracts/bridgehub/Bridgehub.sol b/l1-contracts/contracts/bridgehub/Bridgehub.sol index f8f716268..69a5bc768 100644 --- a/l1-contracts/contracts/bridgehub/Bridgehub.sol +++ b/l1-contracts/contracts/bridgehub/Bridgehub.sol @@ -769,7 +769,10 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus bytes32 _assetId, bytes calldata _bridgehubMintData ) external payable override requireZeroValue(msg.value) onlyAssetRouter whenMigrationsNotPaused { - BridgehubMintCTMAssetData memory bridgehubMintData = abi.decode(_bridgehubMintData, (BridgehubMintCTMAssetData)); + BridgehubMintCTMAssetData memory bridgehubMintData = abi.decode( + _bridgehubMintData, + (BridgehubMintCTMAssetData) + ); address ctm = ctmAssetIdToAddress[_assetId]; if (ctm == address(0)) { diff --git a/l1-contracts/contracts/governance/L2AdminFactory.sol b/l1-contracts/contracts/governance/L2AdminFactory.sol index 2b38ac887..95a2a2312 100644 --- a/l1-contracts/contracts/governance/L2AdminFactory.sol +++ b/l1-contracts/contracts/governance/L2AdminFactory.sol @@ -53,7 +53,7 @@ contract L2AdminFactory { // this factory with `seed1` produces the same address as some other random factory with `seed2`, // allowing to deploy a malicious contract. admin = address(new ChainAdmin(restrictions)); - + emit AdminDeployed(address(admin)); } From 35f5eb840ac70f26e62fbdab523ae8dfb637728a Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 10 Dec 2024 17:05:32 +0100 Subject: [PATCH 48/57] fix tests --- .../_SharedL2ContractL1DeployerUtils.sol | 6 ++++++ .../L1SharedBridge/L1SharedBridgeBase.t.sol | 16 ++++++---------- .../L1SharedBridgeHyperEnabled.t.sol | 8 ++++---- .../L1SharedBridge/_L1SharedBridge_Shared.t.sol | 8 +++++++- 4 files changed, 23 insertions(+), 15 deletions(-) diff --git a/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/_SharedL2ContractL1DeployerUtils.sol b/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/_SharedL2ContractL1DeployerUtils.sol index c5076c3c1..904050282 100644 --- a/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/_SharedL2ContractL1DeployerUtils.sol +++ b/l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/_SharedL2ContractL1DeployerUtils.sol @@ -89,6 +89,12 @@ contract SharedL2ContractL1DeployerUtils is DeployUtils { ); vm.etch(L2_ASSET_ROUTER_ADDR, assetRouter.code); + // Initializing reentrancy guard + vm.store( + L2_ASSET_ROUTER_ADDR, + bytes32(0x8e94fed44239eb2314ab7a406345e6c5a8f0ccedf3b600de3d004e672c33abf4), + bytes32(uint256(1)) + ); stdstore .target(L2_ASSET_ROUTER_ADDR) diff --git a/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeBase.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeBase.t.sol index bd32d0430..e253d28ec 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeBase.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeBase.t.sol @@ -292,20 +292,16 @@ contract L1AssetRouterTestBase is L1AssetRouterTest { ); L2Message memory l2ToL1Message = L2Message({ txNumberInBatch: l2TxNumberInBatch, - sender: L2_ASSET_ROUTER_ADDR, + sender: l2LegacySharedBridgeAddr, data: message }); vm.mockCall( bridgehubAddress, // solhint-disable-next-line func-named-parameters - abi.encodeWithSelector( - IBridgehub.proveL2MessageInclusion.selector, - chainId, - l2BatchNumber, - l2MessageIndex, - l2ToL1Message, - merkleProof + abi.encodeCall( + IBridgehub.proveL2MessageInclusion, + (chainId, l2BatchNumber, l2MessageIndex, l2ToL1Message, merkleProof) ), abi.encode(true) ); @@ -338,7 +334,7 @@ contract L1AssetRouterTestBase is L1AssetRouterTest { ); L2Message memory l2ToL1Message = L2Message({ txNumberInBatch: l2TxNumberInBatch, - sender: L2_ASSET_ROUTER_ADDR, + sender: l2LegacySharedBridgeAddr, data: message }); @@ -427,7 +423,7 @@ contract L1AssetRouterTestBase is L1AssetRouterTest { //alt base token L2Message memory l2ToL1Message = L2Message({ txNumberInBatch: l2TxNumberInBatch, - sender: L2_ASSET_ROUTER_ADDR, + sender: l2LegacySharedBridgeAddr, data: message }); diff --git a/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeHyperEnabled.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeHyperEnabled.t.sol index da30b355f..232340082 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeHyperEnabled.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeHyperEnabled.t.sol @@ -9,7 +9,7 @@ import {L2Message, TxStatus} from "contracts/common/Messaging.sol"; import {IMailbox} from "contracts/state-transition/chain-interfaces/IMailbox.sol"; import {IL1AssetRouter} from "contracts/bridge/asset-router/IL1AssetRouter.sol"; import {IAssetRouterBase} from "contracts/bridge/asset-router/IAssetRouterBase.sol"; -import {L2_BASE_TOKEN_SYSTEM_CONTRACT_ADDR, L2_ASSET_ROUTER_ADDR} from "contracts/common/L2ContractAddresses.sol"; +import {L2_BASE_TOKEN_SYSTEM_CONTRACT_ADDR} from "contracts/common/L2ContractAddresses.sol"; import {DepositNotSet} from "test/foundry/L1TestsErrors.sol"; // note, this should be the same as where hyper is disabled @@ -226,7 +226,7 @@ contract L1AssetRouterHyperEnabledTest is L1AssetRouterTest { ); L2Message memory l2ToL1Message = L2Message({ txNumberInBatch: l2TxNumberInBatch, - sender: L2_ASSET_ROUTER_ADDR, + sender: l2LegacySharedBridgeAddr, data: message }); @@ -268,7 +268,7 @@ contract L1AssetRouterHyperEnabledTest is L1AssetRouterTest { ); L2Message memory l2ToL1Message = L2Message({ txNumberInBatch: l2TxNumberInBatch, - sender: L2_ASSET_ROUTER_ADDR, + sender: l2LegacySharedBridgeAddr, data: message }); @@ -351,7 +351,7 @@ contract L1AssetRouterHyperEnabledTest is L1AssetRouterTest { _setBaseTokenAssetId(bytes32(uint256(2))); //alt base token L2Message memory l2ToL1Message = L2Message({ txNumberInBatch: l2TxNumberInBatch, - sender: L2_ASSET_ROUTER_ADDR, + sender: l2LegacySharedBridgeAddr, data: message }); diff --git a/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/_L1SharedBridge_Shared.t.sol b/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/_L1SharedBridge_Shared.t.sol index 931b5dfc1..0f21e5f03 100644 --- a/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/_L1SharedBridge_Shared.t.sol +++ b/l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/_L1SharedBridge_Shared.t.sol @@ -14,6 +14,7 @@ import {IBridgehub} from "contracts/bridgehub/IBridgehub.sol"; import {TestnetERC20Token} from "contracts/dev-contracts/TestnetERC20Token.sol"; import {L1NativeTokenVault} from "contracts/bridge/ntv/L1NativeTokenVault.sol"; import {L1Nullifier} from "contracts/bridge/L1Nullifier.sol"; +import {L1NullifierDev} from "contracts/dev-contracts/L1NullifierDev.sol"; import {IL1NativeTokenVault} from "contracts/bridge/ntv/IL1NativeTokenVault.sol"; import {INativeTokenVault} from "contracts/bridge/ntv/INativeTokenVault.sol"; import {IL1AssetHandler} from "contracts/bridge/interfaces/IL1AssetHandler.sol"; @@ -91,6 +92,7 @@ contract L1AssetRouterTest is Test { uint256 randomChainId; address eraDiamondProxy; address eraErc20BridgeAddress; + address l2LegacySharedBridgeAddr; uint256 l2BatchNumber; uint256 l2MessageIndex; @@ -117,6 +119,7 @@ contract L1AssetRouterTest is Test { l2BatchNumber = 3; //uint256(uint160(makeAddr("l2BatchNumber"))); l2MessageIndex = uint256(uint160(makeAddr("l2MessageIndex"))); l2TxNumberInBatch = uint16(uint160(makeAddr("l2TxNumberInBatch"))); + l2LegacySharedBridgeAddr = makeAddr("l2LegacySharedBridge"); merkleProof = new bytes32[](1); eraPostUpgradeFirstBatch = 1; @@ -127,7 +130,7 @@ contract L1AssetRouterTest is Test { eraErc20BridgeAddress = makeAddr("eraErc20BridgeAddress"); token = new TestnetERC20Token("TestnetERC20Token", "TET", 18); - l1NullifierImpl = new L1Nullifier({ + l1NullifierImpl = new L1NullifierDev({ _bridgehub: IBridgehub(bridgehubAddress), _eraChainId: eraChainId, _eraDiamondProxy: eraDiamondProxy @@ -137,6 +140,9 @@ contract L1AssetRouterTest is Test { proxyAdmin, abi.encodeWithSelector(L1Nullifier.initialize.selector, owner, 1, 1, 1, 0) ); + L1NullifierDev(address(l1NullifierProxy)).setL2LegacySharedBridge(chainId, l2LegacySharedBridgeAddr); + L1NullifierDev(address(l1NullifierProxy)).setL2LegacySharedBridge(eraChainId, l2LegacySharedBridgeAddr); + l1Nullifier = L1Nullifier(payable(l1NullifierProxy)); sharedBridgeImpl = new L1AssetRouter({ _l1WethAddress: l1WethAddress, From fe16f5464e47f3357d830f4e5e0d2941b402e81e Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Wed, 11 Dec 2024 12:28:28 +0100 Subject: [PATCH 49/57] follow up for i32 --- l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol index 57bb67e9d..d2f711648 100644 --- a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol @@ -48,8 +48,8 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard { } /// @notice Checks that the message sender is the L1 Asset Router. - modifier onlyAssetRouterCounterpartOrSelf(uint256 _originChainId) { - if (_originChainId == L1_CHAIN_ID) { + modifier onlyAssetRouterCounterpartOrSelf(uint256 _chainId) { + if (_chainId == L1_CHAIN_ID) { // Only the L1 Asset Router counterpart can initiate and finalize the deposit. if ((AddressAliasHelper.undoL1ToL2Alias(msg.sender) != L1_ASSET_ROUTER) && (msg.sender != address(this))) { revert InvalidCaller(msg.sender); From c6c4cb109c43582d99ec8fa07584e75cc73a18e5 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Wed, 11 Dec 2024 14:54:20 +0100 Subject: [PATCH 50/57] maybe fix ci --- .github/workflows/l1-contracts-ci.yaml | 4 ++-- l1-contracts/foundry.toml | 2 +- l1-contracts/hardhat.config.ts | 2 +- l2-contracts/foundry.toml | 2 +- l2-contracts/hardhat.config.ts | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/l1-contracts-ci.yaml b/.github/workflows/l1-contracts-ci.yaml index ec546bb5c..46ff536b6 100644 --- a/.github/workflows/l1-contracts-ci.yaml +++ b/.github/workflows/l1-contracts-ci.yaml @@ -161,7 +161,7 @@ jobs: echo "$PWD/foundry-zksync" >> $GITHUB_PATH - name: Install dependencies - run: yarn + run: ci_run yarn - name: Build system contract artifacts run: yarn sc build:foundry @@ -175,7 +175,7 @@ jobs: da-contracts/out l1-contracts/cache-forge l1-contracts/out - l1-contracts/zkout + # TODO: cached `zkout` and the one for tests produce different hashes and so it causes the tests to fail l2-contracts/cache-forge l2-contracts/zkout system-contracts/zkout diff --git a/l1-contracts/foundry.toml b/l1-contracts/foundry.toml index 2dd32c36b..84614c12c 100644 --- a/l1-contracts/foundry.toml +++ b/l1-contracts/foundry.toml @@ -42,4 +42,4 @@ optimizer = true optimizer_runs = 200 [profile.default.zksync] enable_eravm_extensions = true -zksolc = "1.5.3" +zksolc = "1.5.7" diff --git a/l1-contracts/hardhat.config.ts b/l1-contracts/hardhat.config.ts index fc3ae18f1..931008a31 100644 --- a/l1-contracts/hardhat.config.ts +++ b/l1-contracts/hardhat.config.ts @@ -45,7 +45,7 @@ export default { }, zksolc: { compilerSource: "binary", - version: "1.5.3", + version: "1.5.7", settings: { isSystem: true, }, diff --git a/l2-contracts/foundry.toml b/l2-contracts/foundry.toml index b369e211f..cbc072ca8 100644 --- a/l2-contracts/foundry.toml +++ b/l2-contracts/foundry.toml @@ -24,4 +24,4 @@ fs_permissions = [ [profile.default.zksync] enable_eravm_extensions = true -zksolc = "1.5.3" +zksolc = "1.5.7" diff --git a/l2-contracts/hardhat.config.ts b/l2-contracts/hardhat.config.ts index 5aa44b527..073b0db2f 100644 --- a/l2-contracts/hardhat.config.ts +++ b/l2-contracts/hardhat.config.ts @@ -13,7 +13,7 @@ if (!process.env.CHAIN_ETH_NETWORK) { export default { zksolc: { - version: "1.5.3", + version: "1.5.7", compilerSource: "binary", settings: { isSystem: true, From b0536df8864fe425602b6667c089df0628217073 Mon Sep 17 00:00:00 2001 From: Zach Kolodny Date: Wed, 11 Dec 2024 14:16:46 -0500 Subject: [PATCH 51/57] Revert "maybe fix ci" This reverts commit c6c4cb109c43582d99ec8fa07584e75cc73a18e5. --- .github/workflows/l1-contracts-ci.yaml | 4 ++-- l1-contracts/foundry.toml | 2 +- l1-contracts/hardhat.config.ts | 2 +- l2-contracts/foundry.toml | 2 +- l2-contracts/hardhat.config.ts | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/l1-contracts-ci.yaml b/.github/workflows/l1-contracts-ci.yaml index 46ff536b6..ec546bb5c 100644 --- a/.github/workflows/l1-contracts-ci.yaml +++ b/.github/workflows/l1-contracts-ci.yaml @@ -161,7 +161,7 @@ jobs: echo "$PWD/foundry-zksync" >> $GITHUB_PATH - name: Install dependencies - run: ci_run yarn + run: yarn - name: Build system contract artifacts run: yarn sc build:foundry @@ -175,7 +175,7 @@ jobs: da-contracts/out l1-contracts/cache-forge l1-contracts/out - # TODO: cached `zkout` and the one for tests produce different hashes and so it causes the tests to fail + l1-contracts/zkout l2-contracts/cache-forge l2-contracts/zkout system-contracts/zkout diff --git a/l1-contracts/foundry.toml b/l1-contracts/foundry.toml index 84614c12c..2dd32c36b 100644 --- a/l1-contracts/foundry.toml +++ b/l1-contracts/foundry.toml @@ -42,4 +42,4 @@ optimizer = true optimizer_runs = 200 [profile.default.zksync] enable_eravm_extensions = true -zksolc = "1.5.7" +zksolc = "1.5.3" diff --git a/l1-contracts/hardhat.config.ts b/l1-contracts/hardhat.config.ts index 931008a31..fc3ae18f1 100644 --- a/l1-contracts/hardhat.config.ts +++ b/l1-contracts/hardhat.config.ts @@ -45,7 +45,7 @@ export default { }, zksolc: { compilerSource: "binary", - version: "1.5.7", + version: "1.5.3", settings: { isSystem: true, }, diff --git a/l2-contracts/foundry.toml b/l2-contracts/foundry.toml index cbc072ca8..b369e211f 100644 --- a/l2-contracts/foundry.toml +++ b/l2-contracts/foundry.toml @@ -24,4 +24,4 @@ fs_permissions = [ [profile.default.zksync] enable_eravm_extensions = true -zksolc = "1.5.7" +zksolc = "1.5.3" diff --git a/l2-contracts/hardhat.config.ts b/l2-contracts/hardhat.config.ts index 073b0db2f..5aa44b527 100644 --- a/l2-contracts/hardhat.config.ts +++ b/l2-contracts/hardhat.config.ts @@ -13,7 +13,7 @@ if (!process.env.CHAIN_ETH_NETWORK) { export default { zksolc: { - version: "1.5.7", + version: "1.5.3", compilerSource: "binary", settings: { isSystem: true, From 9cc1ccc34d71c57f6911d3b27b85217ed6cf8800 Mon Sep 17 00:00:00 2001 From: Zach Kolodny Date: Wed, 11 Dec 2024 14:23:09 -0500 Subject: [PATCH 52/57] Revert "Revert "maybe fix ci"" This reverts commit b0536df8864fe425602b6667c089df0628217073. --- .github/workflows/l1-contracts-ci.yaml | 4 ++-- l1-contracts/foundry.toml | 2 +- l1-contracts/hardhat.config.ts | 2 +- l2-contracts/foundry.toml | 2 +- l2-contracts/hardhat.config.ts | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/l1-contracts-ci.yaml b/.github/workflows/l1-contracts-ci.yaml index ec546bb5c..46ff536b6 100644 --- a/.github/workflows/l1-contracts-ci.yaml +++ b/.github/workflows/l1-contracts-ci.yaml @@ -161,7 +161,7 @@ jobs: echo "$PWD/foundry-zksync" >> $GITHUB_PATH - name: Install dependencies - run: yarn + run: ci_run yarn - name: Build system contract artifacts run: yarn sc build:foundry @@ -175,7 +175,7 @@ jobs: da-contracts/out l1-contracts/cache-forge l1-contracts/out - l1-contracts/zkout + # TODO: cached `zkout` and the one for tests produce different hashes and so it causes the tests to fail l2-contracts/cache-forge l2-contracts/zkout system-contracts/zkout diff --git a/l1-contracts/foundry.toml b/l1-contracts/foundry.toml index 2dd32c36b..84614c12c 100644 --- a/l1-contracts/foundry.toml +++ b/l1-contracts/foundry.toml @@ -42,4 +42,4 @@ optimizer = true optimizer_runs = 200 [profile.default.zksync] enable_eravm_extensions = true -zksolc = "1.5.3" +zksolc = "1.5.7" diff --git a/l1-contracts/hardhat.config.ts b/l1-contracts/hardhat.config.ts index fc3ae18f1..931008a31 100644 --- a/l1-contracts/hardhat.config.ts +++ b/l1-contracts/hardhat.config.ts @@ -45,7 +45,7 @@ export default { }, zksolc: { compilerSource: "binary", - version: "1.5.3", + version: "1.5.7", settings: { isSystem: true, }, diff --git a/l2-contracts/foundry.toml b/l2-contracts/foundry.toml index b369e211f..cbc072ca8 100644 --- a/l2-contracts/foundry.toml +++ b/l2-contracts/foundry.toml @@ -24,4 +24,4 @@ fs_permissions = [ [profile.default.zksync] enable_eravm_extensions = true -zksolc = "1.5.3" +zksolc = "1.5.7" diff --git a/l2-contracts/hardhat.config.ts b/l2-contracts/hardhat.config.ts index 5aa44b527..073b0db2f 100644 --- a/l2-contracts/hardhat.config.ts +++ b/l2-contracts/hardhat.config.ts @@ -13,7 +13,7 @@ if (!process.env.CHAIN_ETH_NETWORK) { export default { zksolc: { - version: "1.5.3", + version: "1.5.7", compilerSource: "binary", settings: { isSystem: true, From 00f8875a350aa65c83581831fd72c76a61da3c9a Mon Sep 17 00:00:00 2001 From: Zach Kolodny Date: Wed, 11 Dec 2024 14:23:47 -0500 Subject: [PATCH 53/57] undo revert and only update install deps --- .github/workflows/l1-contracts-ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/l1-contracts-ci.yaml b/.github/workflows/l1-contracts-ci.yaml index 46ff536b6..9b8b10369 100644 --- a/.github/workflows/l1-contracts-ci.yaml +++ b/.github/workflows/l1-contracts-ci.yaml @@ -161,7 +161,7 @@ jobs: echo "$PWD/foundry-zksync" >> $GITHUB_PATH - name: Install dependencies - run: ci_run yarn + run: yarn - name: Build system contract artifacts run: yarn sc build:foundry From 94c191c3077e41f4503f912b0a4d5ce342e4cc48 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Mon, 16 Dec 2024 13:56:48 +0100 Subject: [PATCH 54/57] Fix audittens comments (#71) --- .../bridge/asset-router/L2AssetRouter.sol | 2 +- .../bridge/ntv/L2NativeTokenVault.sol | 8 ++++---- .../bridgehub/CTMDeploymentTracker.sol | 2 +- .../contracts/bridgehub/MessageRoot.sol | 6 ++++-- .../contracts/governance/ChainAdmin.sol | 4 ++-- .../state-transition/ChainTypeManager.sol | 18 +++++++++++++++--- .../L1StateTransitionErrors.sol | 3 --- .../chain-deps/facets/Executor.sol | 18 +++++------------- 8 files changed, 32 insertions(+), 29 deletions(-) diff --git a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol index d2f711648..d415618e7 100644 --- a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol @@ -337,7 +337,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter, ReentrancyGuard { return address(0); } - return IBridgedStandardToken(_l2Token).l1Address(); + return IBridgedStandardToken(_l2Token).originToken(); } /// @notice Legacy function used for backward compatibility to return L2 wrapped token diff --git a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol index af4708169..135e9fb64 100644 --- a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol @@ -231,18 +231,18 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { //////////////////////////////////////////////////////////////*/ /// @notice Calculates L2 wrapped token address given the currently stored beacon proxy bytecode hash and beacon address. - /// @param _originChainId The chain id of the origin token. + /// @param _tokenOriginChainId The chain id of the origin token. /// @param _nonNativeToken The address of token on its origin chain.. /// @return Address of an L2 token counterpart. function calculateCreate2TokenAddress( - uint256 _originChainId, + uint256 _tokenOriginChainId, address _nonNativeToken ) public view virtual override(INativeTokenVault, NativeTokenVault) returns (address) { - if (address(L2_LEGACY_SHARED_BRIDGE) != address(0)) { + if (address(L2_LEGACY_SHARED_BRIDGE) != address(0) && _tokenOriginChainId == L1_CHAIN_ID) { return L2_LEGACY_SHARED_BRIDGE.l2TokenAddress(_nonNativeToken); } else { bytes32 constructorInputHash = keccak256(abi.encode(address(bridgedTokenBeacon), "")); - bytes32 salt = _getCreate2Salt(_originChainId, _nonNativeToken); + bytes32 salt = _getCreate2Salt(_tokenOriginChainId, _nonNativeToken); return L2ContractHelper.computeCreate2Address( address(this), diff --git a/l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol b/l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol index 945561f6b..d8ef1ba3a 100644 --- a/l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol +++ b/l1-contracts/contracts/bridgehub/CTMDeploymentTracker.sol @@ -52,7 +52,7 @@ contract CTMDeploymentTracker is ICTMDeploymentTracker, Ownable2StepUpgradeable /// @notice used to initialize the contract /// @param _owner the owner of the contract - function initialize(address _owner) external { + function initialize(address _owner) external initializer { _transferOwnership(_owner); } diff --git a/l1-contracts/contracts/bridgehub/MessageRoot.sol b/l1-contracts/contracts/bridgehub/MessageRoot.sol index 25ca5e3e5..14bf5c1b8 100644 --- a/l1-contracts/contracts/bridgehub/MessageRoot.sol +++ b/l1-contracts/contracts/bridgehub/MessageRoot.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.24; import {DynamicIncrementalMerkle} from "../common/libraries/DynamicIncrementalMerkle.sol"; +import {Initializable} from "@openzeppelin/contracts-v4/proxy/utils/Initializable.sol"; import {IBridgehub} from "./IBridgehub.sol"; import {IMessageRoot} from "./IMessageRoot.sol"; @@ -24,7 +25,7 @@ bytes32 constant SHARED_ROOT_TREE_EMPTY_HASH = bytes32( /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev /// @dev The MessageRoot contract is responsible for storing the cross message roots of the chains and the aggregated root of all chains. -contract MessageRoot is IMessageRoot { +contract MessageRoot is IMessageRoot, Initializable { using FullMerkle for FullMerkle.FullTree; using DynamicIncrementalMerkle for DynamicIncrementalMerkle.Bytes32PushTree; @@ -75,10 +76,11 @@ contract MessageRoot is IMessageRoot { constructor(IBridgehub _bridgehub) { BRIDGE_HUB = _bridgehub; _initialize(); + _disableInitializers(); } /// @dev Initializes a contract for later use. Expected to be used in the proxy on L1, on L2 it is a system contract without a proxy. - function initialize() external { + function initialize() external initializer { _initialize(); } diff --git a/l1-contracts/contracts/governance/ChainAdmin.sol b/l1-contracts/contracts/governance/ChainAdmin.sol index 25a30c498..d61c01d17 100644 --- a/l1-contracts/contracts/governance/ChainAdmin.sol +++ b/l1-contracts/contracts/governance/ChainAdmin.sol @@ -107,8 +107,8 @@ contract ChainAdmin is IChainAdmin, ReentrancyGuard { /// @dev Contract might receive/hold ETH as part of the maintenance process. receive() external payable {} - /// @notice Function that returns the current admin can perform the call. - /// @dev By default it always returns true, but can be overridden in derived contracts. + /// @notice Function that ensures that the current admin can perform the call. + /// @dev Reverts in case the call can not be performed. Successfully executes otherwise function _validateCall(Call calldata _call) private view { address[] memory restrictions = getRestrictions(); diff --git a/l1-contracts/contracts/state-transition/ChainTypeManager.sol b/l1-contracts/contracts/state-transition/ChainTypeManager.sol index e7dea6220..7b17dc7f0 100644 --- a/l1-contracts/contracts/state-transition/ChainTypeManager.sol +++ b/l1-contracts/contracts/state-transition/ChainTypeManager.sol @@ -20,10 +20,12 @@ import {ChainAlreadyLive, Unauthorized, ZeroAddress, HashMismatch, GenesisUpgrad import {SemVer} from "../common/libraries/SemVer.sol"; import {IBridgehub} from "../bridgehub/IBridgehub.sol"; +import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; + /// @title State Transition Manager contract /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev -contract ChainTypeManager is IChainTypeManager, Ownable2StepUpgradeable { +contract ChainTypeManager is IChainTypeManager, ReentrancyGuard, Ownable2StepUpgradeable { using EnumerableMap for EnumerableMap.UintToAddressMap; /// @notice Address of the bridgehub @@ -64,7 +66,12 @@ contract ChainTypeManager is IChainTypeManager, Ownable2StepUpgradeable { /// @dev Contract is expected to be used as proxy implementation. /// @dev Initialize the implementation to prevent Parity hack. - constructor(address _bridgehub) { + /// @dev Note, that while the contract does not use `nonReentrant` modifier, we still keep the `reentrancyGuardInitializer` + /// here for two reasons: + /// - It prevents the function from being called twice (including in the proxy impl). + /// - It makes the local version consistent with the one in production, which already had the reentrancy guard + /// initialized. + constructor(address _bridgehub) reentrancyGuardInitializer { BRIDGE_HUB = _bridgehub; // While this does not provide a protection in the production, it is needed for local testing @@ -116,7 +123,12 @@ contract ChainTypeManager is IChainTypeManager, Ownable2StepUpgradeable { } /// @dev initialize - function initialize(ChainTypeManagerInitializeData calldata _initializeData) external { + /// @dev Note, that while the contract does not use `nonReentrant` modifier, we still keep the `reentrancyGuardInitializer` + /// here for two reasons: + /// - It prevents the function from being called twice (including in the proxy impl). + /// - It makes the local version consistent with the one in production, which already had the reentrancy guard + /// initialized. + function initialize(ChainTypeManagerInitializeData calldata _initializeData) external reentrancyGuardInitializer { if (_initializeData.owner == address(0)) { revert ZeroAddress(); } diff --git a/l1-contracts/contracts/state-transition/L1StateTransitionErrors.sol b/l1-contracts/contracts/state-transition/L1StateTransitionErrors.sol index 767005991..a7fb2e589 100644 --- a/l1-contracts/contracts/state-transition/L1StateTransitionErrors.sol +++ b/l1-contracts/contracts/state-transition/L1StateTransitionErrors.sol @@ -110,9 +110,6 @@ error AdminZero(); // 0x681150be error OutdatedProtocolVersion(uint256 protocolVersion, uint256 currentProtocolVersion); -// 0x8e7df0b6 -error ChainWasMigrated(); - // 0x87470e36 error NotL1(uint256 blockChainId); diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol index 22b5cb70a..63b083878 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol @@ -16,7 +16,7 @@ import {IChainTypeManager} from "../../IChainTypeManager.sol"; import {PriorityTree, PriorityOpsBatchInfo} from "../../libraries/PriorityTree.sol"; import {IL1DAValidator, L1DAValidatorOutput} from "../../chain-interfaces/IL1DAValidator.sol"; import {InvalidSystemLogsLength, MissingSystemLogs, BatchNumberMismatch, TimeNotReached, ValueMismatch, HashMismatch, NonIncreasingTimestamp, TimestampError, InvalidLogSender, TxHashMismatch, UnexpectedSystemLog, LogAlreadyProcessed, InvalidProtocolVersion, CanOnlyProcessOneBatch, BatchHashMismatch, UpgradeBatchNumberIsNotZero, NonSequentialBatch, CantExecuteUnprovenBatches, SystemLogsSizeTooBig, InvalidNumberOfBlobs, VerifiedBatchesExceedsCommittedBatches, InvalidProof, RevertedBatchNotAfterNewLastBatch, CantRevertExecutedBatch, L2TimestampTooBig, PriorityOperationsRollingHashMismatch} from "../../../common/L1ContractErrors.sol"; -import {ChainWasMigrated, InvalidBatchesDataLength, MismatchL2DAValidator, MismatchNumberOfLayer1Txs, PriorityOpsDataLeftPathLengthIsNotZero, PriorityOpsDataRightPathLengthIsNotZero, PriorityOpsDataItemHashesLengthIsNotZero} from "../../L1StateTransitionErrors.sol"; +import {InvalidBatchesDataLength, MismatchL2DAValidator, MismatchNumberOfLayer1Txs, PriorityOpsDataLeftPathLengthIsNotZero, PriorityOpsDataRightPathLengthIsNotZero, PriorityOpsDataItemHashesLengthIsNotZero} from "../../L1StateTransitionErrors.sol"; // While formally the following import is not used, it is needed to inherit documentation from it import {IZKChainBase} from "../../chain-interfaces/IZKChainBase.sol"; @@ -36,14 +36,6 @@ contract ExecutorFacet is ZKChainBase, IExecutor { /// L1 that is at the most base layer. uint256 internal immutable L1_CHAIN_ID; - /// @dev Checks that the chain is connected to the current bridehub and not migrated away. - modifier chainOnCurrentBridgehub() { - if (s.settlementLayer != address(0)) { - revert ChainWasMigrated(); - } - _; - } - constructor(uint256 _l1ChainId) { L1_CHAIN_ID = _l1ChainId; } @@ -251,7 +243,7 @@ contract ExecutorFacet is ZKChainBase, IExecutor { uint256 _processFrom, uint256 _processTo, bytes calldata _commitData - ) external nonReentrant onlyValidator chainOnCurrentBridgehub { + ) external nonReentrant onlyValidator onlySettlementLayer { // check that we have the right protocol version // three comments: // 1. A chain has to keep their protocol version up to date, as processing a block requires the latest or previous protocol version @@ -457,7 +449,7 @@ contract ExecutorFacet is ZKChainBase, IExecutor { uint256 _processFrom, uint256 _processTo, bytes calldata _executeData - ) external nonReentrant onlyValidator chainOnCurrentBridgehub { + ) external nonReentrant onlyValidator onlySettlementLayer { (StoredBatchInfo[] memory batchesData, PriorityOpsBatchInfo[] memory priorityOpsData) = BatchDecoder .decodeAndCheckExecuteData(_executeData, _processFrom, _processTo); uint256 nBatches = batchesData.length; @@ -503,7 +495,7 @@ contract ExecutorFacet is ZKChainBase, IExecutor { uint256 _processBatchFrom, uint256 _processBatchTo, bytes calldata _proofData - ) external nonReentrant onlyValidator chainOnCurrentBridgehub { + ) external nonReentrant onlyValidator onlySettlementLayer { ( StoredBatchInfo memory prevBatch, StoredBatchInfo[] memory committedBatches, @@ -576,7 +568,7 @@ contract ExecutorFacet is ZKChainBase, IExecutor { _revertBatches(_newLastBatch); } - function _revertBatches(uint256 _newLastBatch) internal chainOnCurrentBridgehub { + function _revertBatches(uint256 _newLastBatch) internal onlySettlementLayer { if (s.totalBatchesCommitted <= _newLastBatch) { revert RevertedBatchNotAfterNewLastBatch(); } From 111173edb3f98da2a2548776d1dd018c26dea335 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 16 Dec 2024 21:02:39 +0100 Subject: [PATCH 55/57] comment out some of the migration hacks --- .../deploy-scripts/DeployL2Contracts.sol | 76 ++++++++-------- .../deploy-scripts/GatewayPreparation.s.sol | 86 +++++++++---------- .../deploy-scripts/RegisterZKChain.s.sol | 18 ++-- .../upgrade-envs/script-config/mainnet.toml | 4 +- system-contracts/SystemContractsHashes.json | 4 +- 5 files changed, 94 insertions(+), 94 deletions(-) diff --git a/l1-contracts/deploy-scripts/DeployL2Contracts.sol b/l1-contracts/deploy-scripts/DeployL2Contracts.sol index 22ac84447..ad8230f59 100644 --- a/l1-contracts/deploy-scripts/DeployL2Contracts.sol +++ b/l1-contracts/deploy-scripts/DeployL2Contracts.sol @@ -50,44 +50,44 @@ contract DeployL2Script is Script { function run() public { initializeConfig(); - string memory root = vm.projectRoot(); - string memory path = string.concat(root, "/script-out/gateway-deploy-governance-txs-1.json"); - - bool fileExists = vm.isFile(path); - if (fileExists) { - string memory json = vm.readFile(path); - for (uint i = 0; i < 10000; ++i) { - string memory tmp = string.concat(".transactions[", vm.toString(i)); - tmp = string.concat(tmp, "]"); - bool exists = vm.keyExistsJson(json, tmp); - if (!exists) { - break; - } else { - address from = vm.parseJsonAddress(json, string.concat(tmp, ".transaction.from")); - bool existsTo = vm.keyExistsJson(json, string.concat(tmp, ".transaction.to")); - uint256 value = vm.parseJsonUint(json, string.concat(tmp, ".transaction.value")); - if (existsTo) { - bytes memory input = vm.parseJsonBytes(json, string.concat(tmp, ".transaction.input")); - address to = vm.parseJsonAddress(json, string.concat(tmp, ".transaction.to")); - vm.startBroadcast(from); - to.call{value: value}(input); - vm.stopBroadcast(); - } else { - bytes memory input = vm.parseJsonBytes(json, string.concat(tmp, ".transaction.input")); - vm.startBroadcast(from); - address deployedAddress; - assembly { - deployedAddress := create(value, add(input, 0x20), mload(input)) - - if iszero(extcodesize(deployedAddress)) { - revert(0, 0) - } - } - vm.stopBroadcast(); - } - } - } - } + // string memory root = vm.projectRoot(); + // string memory path = string.concat(root, "/script-out/gateway-deploy-governance-txs-1.json"); + + // bool fileExists = vm.isFile(path); + // if (fileExists) { + // string memory json = vm.readFile(path); + // for (uint i = 0; i < 10000; ++i) { + // string memory tmp = string.concat(".transactions[", vm.toString(i)); + // tmp = string.concat(tmp, "]"); + // bool exists = vm.keyExistsJson(json, tmp); + // if (!exists) { + // break; + // } else { + // address from = vm.parseJsonAddress(json, string.concat(tmp, ".transaction.from")); + // bool existsTo = vm.keyExistsJson(json, string.concat(tmp, ".transaction.to")); + // uint256 value = vm.parseJsonUint(json, string.concat(tmp, ".transaction.value")); + // if (existsTo) { + // bytes memory input = vm.parseJsonBytes(json, string.concat(tmp, ".transaction.input")); + // address to = vm.parseJsonAddress(json, string.concat(tmp, ".transaction.to")); + // vm.startBroadcast(from); + // to.call{value: value}(input); + // vm.stopBroadcast(); + // } else { + // bytes memory input = vm.parseJsonBytes(json, string.concat(tmp, ".transaction.input")); + // vm.startBroadcast(from); + // address deployedAddress; + // assembly { + // deployedAddress := create(value, add(input, 0x20), mload(input)) + + // if iszero(extcodesize(deployedAddress)) { + // revert(0, 0) + // } + // } + // vm.stopBroadcast(); + // } + // } + // } + // } deploy(false); } diff --git a/l1-contracts/deploy-scripts/GatewayPreparation.s.sol b/l1-contracts/deploy-scripts/GatewayPreparation.s.sol index 74118ca8d..7ac906f7b 100644 --- a/l1-contracts/deploy-scripts/GatewayPreparation.s.sol +++ b/l1-contracts/deploy-scripts/GatewayPreparation.s.sol @@ -635,49 +635,49 @@ contract GatewayPreparation is Script { } function preRun(bool skip_stage12) internal { - string memory root = vm.projectRoot(); - string memory path = string.concat(root, "/script-out/gateway-deploy-governance-txs-1.json"); - - bool fileExists = vm.isFile(path); - if (fileExists) { - uint256 idxFrom = 0; - if (skip_stage12) { - vm.pauseGasMetering(); - idxFrom = 4; - } - string memory json = vm.readFile(path); - for (uint i = idxFrom; i < 10000; ++i) { - string memory tmp = string.concat(".transactions[", vm.toString(i)); - tmp = string.concat(tmp, "]"); - bool exists = vm.keyExistsJson(json, tmp); - if (!exists) { - break; - } else { - address from = vm.parseJsonAddress(json, string.concat(tmp, ".transaction.from")); - bool existsTo = vm.keyExistsJson(json, string.concat(tmp, ".transaction.to")); - uint256 value = vm.parseJsonUint(json, string.concat(tmp, ".transaction.value")); - if (existsTo) { - bytes memory input = vm.parseJsonBytes(json, string.concat(tmp, ".transaction.input")); - address to = vm.parseJsonAddress(json, string.concat(tmp, ".transaction.to")); - vm.startBroadcast(from); - to.call{value: value}(input); - vm.stopBroadcast(); - } else { - bytes memory input = vm.parseJsonBytes(json, string.concat(tmp, ".transaction.input")); - vm.startBroadcast(from); - address deployedAddress; - assembly { - deployedAddress := create(value, add(input, 0x20), mload(input)) - - if iszero(extcodesize(deployedAddress)) { - revert(0, 0) - } - } - vm.stopBroadcast(); - } - } - } - } + // string memory root = vm.projectRoot(); + // string memory path = string.concat(root, "/script-out/gateway-deploy-governance-txs-1.json"); + + // bool fileExists = vm.isFile(path); + // if (fileExists) { + // uint256 idxFrom = 0; + // if (skip_stage12) { + // vm.pauseGasMetering(); + // idxFrom = 4; + // } + // string memory json = vm.readFile(path); + // for (uint i = idxFrom; i < 10000; ++i) { + // string memory tmp = string.concat(".transactions[", vm.toString(i)); + // tmp = string.concat(tmp, "]"); + // bool exists = vm.keyExistsJson(json, tmp); + // if (!exists) { + // break; + // } else { + // address from = vm.parseJsonAddress(json, string.concat(tmp, ".transaction.from")); + // bool existsTo = vm.keyExistsJson(json, string.concat(tmp, ".transaction.to")); + // uint256 value = vm.parseJsonUint(json, string.concat(tmp, ".transaction.value")); + // if (existsTo) { + // bytes memory input = vm.parseJsonBytes(json, string.concat(tmp, ".transaction.input")); + // address to = vm.parseJsonAddress(json, string.concat(tmp, ".transaction.to")); + // vm.startBroadcast(from); + // to.call{value: value}(input); + // vm.stopBroadcast(); + // } else { + // bytes memory input = vm.parseJsonBytes(json, string.concat(tmp, ".transaction.input")); + // vm.startBroadcast(from); + // address deployedAddress; + // assembly { + // deployedAddress := create(value, add(input, 0x20), mload(input)) + + // if iszero(extcodesize(deployedAddress)) { + // revert(0, 0) + // } + // } + // vm.stopBroadcast(); + // } + // } + // } + // } } function governanceExecuteCalls(bytes memory callsToExecute, address governanceAddr) internal { diff --git a/l1-contracts/deploy-scripts/RegisterZKChain.s.sol b/l1-contracts/deploy-scripts/RegisterZKChain.s.sol index f73ebb23e..3285b2af5 100644 --- a/l1-contracts/deploy-scripts/RegisterZKChain.s.sol +++ b/l1-contracts/deploy-scripts/RegisterZKChain.s.sol @@ -110,15 +110,15 @@ contract RegisterZKChainScript is Script { string memory root = vm.projectRoot(); // kludge - string memory path = string.concat(root, "/script-out/gateway-upgrade-ecosystem.toml"); - bool fileExists = vm.isFile(path); - if (config.chainChainId == 505 && fileExists) { - string memory toml = vm.readFile(path); - bytes memory calls1 = toml.readBytes("$.governance_stage1_calls"); - bytes memory calls2 = toml.readBytes("$.governance_stage2_calls"); - governanceExecuteCalls(calls1, config.governance); - governanceExecuteCalls(calls2, config.governance); - } + // string memory path = string.concat(root, "/script-out/gateway-upgrade-ecosystem.toml"); + // bool fileExists = vm.isFile(path); + // if (config.chainChainId == 505 && fileExists) { + // string memory toml = vm.readFile(path); + // bytes memory calls1 = toml.readBytes("$.governance_stage1_calls"); + // bytes memory calls2 = toml.readBytes("$.governance_stage2_calls"); + // governanceExecuteCalls(calls1, config.governance); + // governanceExecuteCalls(calls2, config.governance); + // } outputPath = string.concat(root, outputPath); diff --git a/l1-contracts/test/foundry/l1/integration/upgrade-envs/script-config/mainnet.toml b/l1-contracts/test/foundry/l1/integration/upgrade-envs/script-config/mainnet.toml index 4089eb029..0c77be87e 100644 --- a/l1-contracts/test/foundry/l1/integration/upgrade-envs/script-config/mainnet.toml +++ b/l1-contracts/test/foundry/l1/integration/upgrade-envs/script-config/mainnet.toml @@ -20,8 +20,8 @@ diamond_init_max_pubdata_per_batch = 120000 diamond_init_max_l2_gas_per_batch = 80000000 diamond_init_priority_tx_max_pubdata = 99000 diamond_init_minimal_l2_gas_price = 250000000 -bootloader_hash = "0x010008e5c007f9cf449b3cb3cac0fd49806aa98540d0b123f4cc75501f0682a4" -default_aa_hash = "0x01000523dc43ef12a998843b5c95cefd77288333888bf3a5e7fc729b7e12b536" +bootloader_hash = "0x010008e5b883de8897598e83d383e332b87d09164363816c15f22353c3cd910d" +default_aa_hash = "0x0100052307b3b66ef67935255483d39b3c8dcdb47fdf94dddca11ebe8271afe6" bridgehub_proxy_address = "0x303a465B659cBB0ab36eE643eA362c509EEb5213" old_shared_bridge_proxy_address = "0xD7f9f54194C633F36CCD5F3da84ad4a1c38cB2cB" state_transition_manager_address = "0xc2eE6b6af7d616f6e27ce7F4A451Aedc2b0F5f5C" diff --git a/system-contracts/SystemContractsHashes.json b/system-contracts/SystemContractsHashes.json index 580fc0ea5..fc639d6b4 100644 --- a/system-contracts/SystemContractsHashes.json +++ b/system-contracts/SystemContractsHashes.json @@ -45,7 +45,7 @@ "contractName": "DefaultAccount", "bytecodePath": "artifacts-zk/contracts-preprocessed/DefaultAccount.sol/DefaultAccount.json", "sourceCodePath": "contracts-preprocessed/DefaultAccount.sol", - "bytecodeHash": "0x01000523dc43ef12a998843b5c95cefd77288333888bf3a5e7fc729b7e12b536", + "bytecodeHash": "0x0100052307b3b66ef67935255483d39b3c8dcdb47fdf94dddca11ebe8271afe6", "sourceCodeHash": "0xebffe840ebbd9329edb1ebff8ca50f6935e7dabcc67194a896fcc2e968d46dfb" }, { @@ -234,7 +234,7 @@ "contractName": "proved_batch", "bytecodePath": "bootloader/build/artifacts/proved_batch.yul.zbin", "sourceCodePath": "bootloader/build/proved_batch.yul", - "bytecodeHash": "0x010008e5c007f9cf449b3cb3cac0fd49806aa98540d0b123f4cc75501f0682a4", + "bytecodeHash": "0x010008e5b883de8897598e83d383e332b87d09164363816c15f22353c3cd910d", "sourceCodeHash": "0xf8f84fe15ef78e170f6649f12920266c9afe3d29b867d2f5fd28b672bd13e9a3" } ] From 68ddadf8106bde020368e6ab5613a522a88f58f6 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Mon, 16 Dec 2024 22:12:41 +0100 Subject: [PATCH 56/57] fix lint --- da-contracts/contracts/da-layers/avail/DummyAvailBridge.sol | 4 +++- da-contracts/contracts/da-layers/avail/DummyVectorX.sol | 4 +++- da-contracts/contracts/da-layers/avail/IAvailBridge.sol | 1 + l1-contracts/deploy-scripts/GatewayPreparation.s.sol | 2 -- .../contracts/data-availability/AvailL2DAValidator.sol | 2 +- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/da-contracts/contracts/da-layers/avail/DummyAvailBridge.sol b/da-contracts/contracts/da-layers/avail/DummyAvailBridge.sol index 374ef1d1d..8fe184439 100644 --- a/da-contracts/contracts/da-layers/avail/DummyAvailBridge.sol +++ b/da-contracts/contracts/da-layers/avail/DummyAvailBridge.sol @@ -1,5 +1,7 @@ // SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + import {IAvailBridge} from "./IAvailBridge.sol"; import {IVectorx} from "./IVectorx.sol"; import {DummyVectorX} from "./DummyVectorX.sol"; @@ -15,7 +17,7 @@ contract DummyAvailBridge is IAvailBridge { return vectorxContract; } - function verifyBlobLeaf(MerkleProofInput calldata input) external view returns (bool) { + function verifyBlobLeaf(MerkleProofInput calldata) external view returns (bool) { return true; } } diff --git a/da-contracts/contracts/da-layers/avail/DummyVectorX.sol b/da-contracts/contracts/da-layers/avail/DummyVectorX.sol index d04ed8673..eccee235b 100644 --- a/da-contracts/contracts/da-layers/avail/DummyVectorX.sol +++ b/da-contracts/contracts/da-layers/avail/DummyVectorX.sol @@ -1,9 +1,11 @@ // SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + import {IVectorx} from "./IVectorx.sol"; contract DummyVectorX is IVectorx { - function rangeStartBlocks(bytes32 rangeHash) external view returns (uint32 startBlock) { + function rangeStartBlocks(bytes32) external view returns (uint32 startBlock) { return 1; } } diff --git a/da-contracts/contracts/da-layers/avail/IAvailBridge.sol b/da-contracts/contracts/da-layers/avail/IAvailBridge.sol index 5759b8029..fb44b91b2 100644 --- a/da-contracts/contracts/da-layers/avail/IAvailBridge.sol +++ b/da-contracts/contracts/da-layers/avail/IAvailBridge.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.24; import {IVectorx} from "./IVectorx.sol"; interface IAvailBridge { + // solhint-disable-next-line gas-struct-packing struct Message { // single-byte prefix representing the message type bytes1 messageType; diff --git a/l1-contracts/deploy-scripts/GatewayPreparation.s.sol b/l1-contracts/deploy-scripts/GatewayPreparation.s.sol index 7ac906f7b..ec547a42a 100644 --- a/l1-contracts/deploy-scripts/GatewayPreparation.s.sol +++ b/l1-contracts/deploy-scripts/GatewayPreparation.s.sol @@ -637,7 +637,6 @@ contract GatewayPreparation is Script { function preRun(bool skip_stage12) internal { // string memory root = vm.projectRoot(); // string memory path = string.concat(root, "/script-out/gateway-deploy-governance-txs-1.json"); - // bool fileExists = vm.isFile(path); // if (fileExists) { // uint256 idxFrom = 0; @@ -668,7 +667,6 @@ contract GatewayPreparation is Script { // address deployedAddress; // assembly { // deployedAddress := create(value, add(input, 0x20), mload(input)) - // if iszero(extcodesize(deployedAddress)) { // revert(0, 0) // } diff --git a/l2-contracts/contracts/data-availability/AvailL2DAValidator.sol b/l2-contracts/contracts/data-availability/AvailL2DAValidator.sol index ba8c8582b..6462a5e42 100644 --- a/l2-contracts/contracts/data-availability/AvailL2DAValidator.sol +++ b/l2-contracts/contracts/data-availability/AvailL2DAValidator.sol @@ -19,7 +19,7 @@ contract AvailL2DAValidator is IL2DAValidator, StateDiffL2DAValidator { // Operator data, that is related to the DA itself bytes calldata _totalL2ToL1PubdataAndStateDiffs ) external returns (bytes32 outputHash) { - (bytes32 stateDiffHash, bytes calldata _totalPubdata, bytes calldata leftover) = _produceStateDiffPubdata( + (bytes32 stateDiffHash, bytes calldata _totalPubdata, ) = _produceStateDiffPubdata( _chainedMessagesHash, _chainedBytecodesHash, _totalL2ToL1PubdataAndStateDiffs From b3aa336bebbf66ee4744c7afd8c5abdd768e3724 Mon Sep 17 00:00:00 2001 From: Stanislav Breadless Date: Tue, 17 Dec 2024 11:55:35 +0100 Subject: [PATCH 57/57] remove kludges for gateway testing --- .../deploy-scripts/DeployL2Contracts.sol | 39 --------------- .../deploy-scripts/GatewayPreparation.s.sol | 49 ------------------- .../deploy-scripts/RegisterZKChain.s.sol | 11 ----- .../upgrade/BytecodePublisher.s.sol | 4 +- 4 files changed, 2 insertions(+), 101 deletions(-) diff --git a/l1-contracts/deploy-scripts/DeployL2Contracts.sol b/l1-contracts/deploy-scripts/DeployL2Contracts.sol index ad8230f59..1923672b4 100644 --- a/l1-contracts/deploy-scripts/DeployL2Contracts.sol +++ b/l1-contracts/deploy-scripts/DeployL2Contracts.sol @@ -50,45 +50,6 @@ contract DeployL2Script is Script { function run() public { initializeConfig(); - // string memory root = vm.projectRoot(); - // string memory path = string.concat(root, "/script-out/gateway-deploy-governance-txs-1.json"); - - // bool fileExists = vm.isFile(path); - // if (fileExists) { - // string memory json = vm.readFile(path); - // for (uint i = 0; i < 10000; ++i) { - // string memory tmp = string.concat(".transactions[", vm.toString(i)); - // tmp = string.concat(tmp, "]"); - // bool exists = vm.keyExistsJson(json, tmp); - // if (!exists) { - // break; - // } else { - // address from = vm.parseJsonAddress(json, string.concat(tmp, ".transaction.from")); - // bool existsTo = vm.keyExistsJson(json, string.concat(tmp, ".transaction.to")); - // uint256 value = vm.parseJsonUint(json, string.concat(tmp, ".transaction.value")); - // if (existsTo) { - // bytes memory input = vm.parseJsonBytes(json, string.concat(tmp, ".transaction.input")); - // address to = vm.parseJsonAddress(json, string.concat(tmp, ".transaction.to")); - // vm.startBroadcast(from); - // to.call{value: value}(input); - // vm.stopBroadcast(); - // } else { - // bytes memory input = vm.parseJsonBytes(json, string.concat(tmp, ".transaction.input")); - // vm.startBroadcast(from); - // address deployedAddress; - // assembly { - // deployedAddress := create(value, add(input, 0x20), mload(input)) - - // if iszero(extcodesize(deployedAddress)) { - // revert(0, 0) - // } - // } - // vm.stopBroadcast(); - // } - // } - // } - // } - deploy(false); } diff --git a/l1-contracts/deploy-scripts/GatewayPreparation.s.sol b/l1-contracts/deploy-scripts/GatewayPreparation.s.sol index ec547a42a..1f2c518d8 100644 --- a/l1-contracts/deploy-scripts/GatewayPreparation.s.sol +++ b/l1-contracts/deploy-scripts/GatewayPreparation.s.sol @@ -185,7 +185,6 @@ contract GatewayPreparation is Script { /// @dev Requires the sender to be the owner of the contract function governanceRegisterGateway() public { initializeConfig(); - preRun(false); IBridgehub bridgehub = IBridgehub(config.bridgehub); @@ -210,7 +209,6 @@ contract GatewayPreparation is Script { /// @dev Requires the sender to be the owner of the contract function governanceWhitelistGatewayCTM(address gatewayCTMAddress, bytes32 governanoceOperationSalt) public { initializeConfig(); - preRun(false); bytes memory data = abi.encodeCall(IBridgehub.addChainTypeManager, (gatewayCTMAddress)); @@ -232,7 +230,6 @@ contract GatewayPreparation is Script { function governanceSetCTMAssetHandler(bytes32 governanoceOperationSalt) public { initializeConfig(); - preRun(false); L1AssetRouter sharedBridge = L1AssetRouter(config.sharedBridgeProxy); bytes memory data = abi.encodeCall( @@ -289,7 +286,6 @@ contract GatewayPreparation is Script { function registerAssetIdInBridgehub(address gatewayCTMAddress, bytes32 governanoceOperationSalt) public { initializeConfig(); - preRun(false); bytes memory secondBridgeData = abi.encodePacked( bytes1(0x01), @@ -630,54 +626,9 @@ contract GatewayPreparation is Script { } function executeGovernanceTxs() public { - preRun(true); saveOutput(); } - function preRun(bool skip_stage12) internal { - // string memory root = vm.projectRoot(); - // string memory path = string.concat(root, "/script-out/gateway-deploy-governance-txs-1.json"); - // bool fileExists = vm.isFile(path); - // if (fileExists) { - // uint256 idxFrom = 0; - // if (skip_stage12) { - // vm.pauseGasMetering(); - // idxFrom = 4; - // } - // string memory json = vm.readFile(path); - // for (uint i = idxFrom; i < 10000; ++i) { - // string memory tmp = string.concat(".transactions[", vm.toString(i)); - // tmp = string.concat(tmp, "]"); - // bool exists = vm.keyExistsJson(json, tmp); - // if (!exists) { - // break; - // } else { - // address from = vm.parseJsonAddress(json, string.concat(tmp, ".transaction.from")); - // bool existsTo = vm.keyExistsJson(json, string.concat(tmp, ".transaction.to")); - // uint256 value = vm.parseJsonUint(json, string.concat(tmp, ".transaction.value")); - // if (existsTo) { - // bytes memory input = vm.parseJsonBytes(json, string.concat(tmp, ".transaction.input")); - // address to = vm.parseJsonAddress(json, string.concat(tmp, ".transaction.to")); - // vm.startBroadcast(from); - // to.call{value: value}(input); - // vm.stopBroadcast(); - // } else { - // bytes memory input = vm.parseJsonBytes(json, string.concat(tmp, ".transaction.input")); - // vm.startBroadcast(from); - // address deployedAddress; - // assembly { - // deployedAddress := create(value, add(input, 0x20), mload(input)) - // if iszero(extcodesize(deployedAddress)) { - // revert(0, 0) - // } - // } - // vm.stopBroadcast(); - // } - // } - // } - // } - } - function governanceExecuteCalls(bytes memory callsToExecute, address governanceAddr) internal { IGovernance governance = IGovernance(governanceAddr); Ownable2Step ownable = Ownable2Step(governanceAddr); diff --git a/l1-contracts/deploy-scripts/RegisterZKChain.s.sol b/l1-contracts/deploy-scripts/RegisterZKChain.s.sol index 3285b2af5..5dbf2b2b1 100644 --- a/l1-contracts/deploy-scripts/RegisterZKChain.s.sol +++ b/l1-contracts/deploy-scripts/RegisterZKChain.s.sol @@ -109,17 +109,6 @@ contract RegisterZKChainScript is Script { function runInner(string memory outputPath) internal { string memory root = vm.projectRoot(); - // kludge - // string memory path = string.concat(root, "/script-out/gateway-upgrade-ecosystem.toml"); - // bool fileExists = vm.isFile(path); - // if (config.chainChainId == 505 && fileExists) { - // string memory toml = vm.readFile(path); - // bytes memory calls1 = toml.readBytes("$.governance_stage1_calls"); - // bytes memory calls2 = toml.readBytes("$.governance_stage2_calls"); - // governanceExecuteCalls(calls1, config.governance); - // governanceExecuteCalls(calls2, config.governance); - // } - outputPath = string.concat(root, outputPath); if (config.initializeLegacyBridge) { diff --git a/l1-contracts/deploy-scripts/upgrade/BytecodePublisher.s.sol b/l1-contracts/deploy-scripts/upgrade/BytecodePublisher.s.sol index aca79e2e1..3981da95a 100644 --- a/l1-contracts/deploy-scripts/upgrade/BytecodePublisher.s.sol +++ b/l1-contracts/deploy-scripts/upgrade/BytecodePublisher.s.sol @@ -14,10 +14,10 @@ library BytecodePublisher { address internal constant VM_ADDRESS = address(uint160(uint256(keccak256("hevm cheat code")))); Vm internal constant vm = Vm(VM_ADDRESS); - // 100 kb + /// @notice Maximal size of bytecodes' batch to be published at once uint256 constant MAX_BATCH_SIZE = 126_000; - /// @notice Publishes bytecodes in batches, each not exceeding 100KB + /// @notice Publishes bytecodes in batches, each not exceeding `MAX_BATCH_SIZE` /// @param bytecodes The array of bytecodes to publish function publishBytecodesInBatches(BytecodesSupplier bytecodesSupplier, bytes[] memory bytecodes) internal { uint256 totalBytecodes = bytecodes.length;