Merge pull request #724 from matter-labs/kl/message-root-update

fix: chainRegistered

.github/workflows/l1-contracts-ci.yaml

@@ -155,7 +155,7 @@ jobs:
       - name: Run tests
         run: yarn l1 test --no-compile
 
-  check-verifier-generator:
+  check-verifier-generator-l1:
     runs-on: ubuntu-latest
 
     steps:

.github/workflows/l2-contracts-ci.yaml

@@ -26,6 +26,9 @@ jobs:
       - name: Build L2 artifacts
         run: yarn l2 build
 
+      - name: Build system contract artifacts
+        run: yarn sc build
+
       - name: Create cache
         uses: actions/cache/save@v3
         with:
@@ -37,6 +40,9 @@ jobs:
             l2-contracts/artifacts-zk
             l2-contracts/cache-zk
             l2-contracts/typechain
+            system-contracts/artifacts-zk
+            system-contracts/cache-zk
+            system-contracts/typechain
 
   lint:
     runs-on: ubuntu-latest
@@ -57,6 +63,23 @@ jobs:
       - name: Lint
         run: yarn lint:check
 
+  check-verifier-generator-l2:
+    needs: [build]
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout the repository
+        uses: actions/checkout@v4
+        with:
+          submodules: recursive
+
+      - name: Generate Verifier.sol
+        working-directory: tools
+        run: cargo run --bin zksync_verifier_contract_generator --release -- --input_path data/scheduler_key.json --l2_mode
+
+      - name: Compare
+        run: diff tools/data/Verifier.sol l2-contracts/contracts/verifier/Verifier.sol
+
   test:
     needs: [build, lint]
     runs-on: ubuntu-latest
@@ -88,12 +111,19 @@ jobs:
             l2-contracts/artifacts-zk
             l2-contracts/cache-zk
             l2-contracts/typechain
-
-      - name: Run Era test node
-        uses: dutterbutter/[email protected]
-
-      - name: Copy typechain from System Contracts
-        run: yarn sc build && yarn sc copy:typechain
+            system-contracts/artifacts-zk
+            system-contracts/cache-zk
+            system-contracts/typechain
+
+      - name: Install foundry zksync
+        run: |
+          wget https://github.com/matter-labs/foundry-zksync/releases/download/nightly-f908ce43834bc1ffb4de6576ea5600eaab49dddb/foundry_nightly_linux_amd64.tar.gz -O foundry-zksync.tar.gz
+          tar -xzf foundry-zksync.tar.gz
+          sudo mv forge /usr/local/bin/forge
+          sudo mv cast /usr/local/bin/cast
+          sudo chmod +x /usr/local/bin/forge
+          sudo chmod +x /usr/local/bin/cast
+          forge --version
 
       - name: Run tests
-        run: yarn l2 test
+        run: yarn l2 test:foundry

.gitmodules

@@ -12,3 +12,6 @@
 [submodule "lib/forge-std"]
 	path = lib/forge-std
 	url = https://github.com/foundry-rs/forge-std
+[submodule "lib/@matterlabs/zksync-contracts"]
+	path = lib/@matterlabs/zksync-contracts
+	url = https://github.com/matter-labs/v2-testnet-contracts

.solhintignore

@@ -16,6 +16,7 @@ l1-contracts-foundry/lib
 # l2-contracts
 l2-contracts/cache-zk
 l2-contracts/node_modules
+l2-contracts/test
 
 # system-contracts
 system-contracts/contracts/openzeppelin

l1-contracts/contracts/bridge/L1AssetRouter.sol

@@ -25,7 +25,7 @@ import {ReentrancyGuard} from "../common/ReentrancyGuard.sol";
 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 {L2_NATIVE_TOKEN_VAULT_ADDRESS} from "../common/L2ContractAddresses.sol";
+import {L2_NATIVE_TOKEN_VAULT_ADDR} from "../common/L2ContractAddresses.sol";
 
 import {IBridgehub, L2TransactionRequestTwoBridgesInner, L2TransactionRequestDirect} from "../bridgehub/IBridgehub.sol";
 import {L2_BASE_TOKEN_SYSTEM_CONTRACT_ADDR, L2_ASSET_ROUTER_ADDR} from "../common/L2ContractAddresses.sol";
@@ -280,7 +280,7 @@ contract L1AssetRouter is
     /// @param _assetHandlerAddress The address of the asset handler to be set for the provided asset.
     function setAssetHandlerAddressThisChain(bytes32 _assetRegistrationData, address _assetHandlerAddress) external {
         bool senderIsNTV = msg.sender == address(nativeTokenVault);
-        address sender = senderIsNTV ? L2_NATIVE_TOKEN_VAULT_ADDRESS : msg.sender;
+        address sender = senderIsNTV ? L2_NATIVE_TOKEN_VAULT_ADDR : msg.sender;
         bytes32 assetId = DataEncoding.encodeAssetId(block.chainid, _assetRegistrationData, sender);
         require(senderIsNTV || msg.sender == assetDeploymentTracker[assetId], "ShB: not NTV or ADT");
         assetHandlerAddress[assetId] = _assetHandlerAddress;
@@ -542,7 +542,7 @@ contract L1AssetRouter is
             _assetData
         );
 
-        emit ClaimedFailedDepositSharedBridge(_chainId, _depositSender, _assetId, _assetData);
+        emit ClaimedFailedDepositAssetRouter(_chainId, _depositSender, _assetId, _assetData);
     }
 
     /// @dev Receives and parses (name, symbol, decimals) from the token contract
@@ -624,9 +624,10 @@ contract L1AssetRouter is
         }
         address l1AssetHandler = assetHandlerAddress[assetId];
         IL1AssetHandler(l1AssetHandler).bridgeMint(_chainId, assetId, transferData);
-        (amount, l1Receiver) = abi.decode(transferData, (uint256, address));
-
-        emit WithdrawalFinalizedSharedBridge(_chainId, l1Receiver, assetId, amount);
+        if (l1AssetHandler == address(nativeTokenVault)) {
+            (amount, l1Receiver) = abi.decode(transferData, (uint256, address));
+        }
+        emit WithdrawalFinalizedAssetRouter(_chainId, assetId, transferData);
     }
 
     /// @notice Decodes the transfer input for legacy data and transfers allowance to NTV

l1-contracts/contracts/bridge/L1NativeTokenVault.sol

@@ -126,9 +126,7 @@ contract L1NativeTokenVault is IL1NativeTokenVault, Ownable2StepUpgradeable, Pau
     ) external payable override onlyBridge whenNotPaused {
         // here we are minting the tokens after the bridgeBurn has happened on an L2, so we can assume the l1Token is not zero
         address l1Token = tokenAddress[_assetId];
-        uint256 amount;
-        address l1Receiver;
-        (amount, l1Receiver) = abi.decode(_data, (uint256, address));
+        (uint256 amount, address l1Receiver) = abi.decode(_data, (uint256, address));
         // Check that the chain has sufficient balance
         if (chainBalance[_chainId][l1Token] < amount) {
             revert InsufficientChainBalance();

l1-contracts/contracts/bridge/interfaces/IL1AssetRouter.sol

@@ -43,14 +43,9 @@ interface IL1AssetRouter {
         bytes32 indexed l2DepositTxHash
     );
 
-    event WithdrawalFinalizedSharedBridge(
-        uint256 indexed chainId,
-        address indexed to,
-        bytes32 indexed assetId,
-        uint256 amount
-    );
+    event WithdrawalFinalizedAssetRouter(uint256 indexed chainId, bytes32 indexed assetId, bytes assetData);
 
-    event ClaimedFailedDepositSharedBridge(
+    event ClaimedFailedDepositAssetRouter(
         uint256 indexed chainId,
         address indexed to,
         bytes32 indexed assetId,

l1-contracts/contracts/bridgehub/Bridgehub.sol

@@ -96,6 +96,9 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus
     /// @notice we store registered assetIds (for arbitrary base token)
     mapping(bytes32 baseTokenAssetId => bool) public assetIdIsRegistered;
 
+    /// @notice used to pause the migrations of chains. Used for upgrades.
+    bool public migrationPaused;
+
     modifier onlyOwnerOrAdmin() {
         if (msg.sender != admin && msg.sender != owner()) {
             revert Unauthorized(msg.sender);
@@ -124,6 +127,11 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus
         _;
     }
 
+    modifier whenMigrationsNotPaused() {
+        require(!migrationPaused, "BH: migrations paused");
+        _;
+    }
+
     /// @notice to avoid parity hack
     constructor(uint256 _l1ChainId, address _owner, uint256 _maxNumberOfHyperchains) reentrancyGuardInitializer {
         _disableInitializers();
@@ -134,13 +142,16 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus
         // This is indeed true, since the only methods where this immutable is used are the ones with `onlyL1` modifier.
         ETH_TOKEN_ASSET_ID = DataEncoding.encodeNTVAssetId(block.chainid, ETH_TOKEN_ADDRESS);
         _transferOwnership(_owner);
+        whitelistedSettlementLayers[_l1ChainId] = true;
     }
 
     /// @notice used to initialize the contract
     /// @notice this contract is also deployed on L2 as a system contract there the owner and the related functions will not be used
     /// @param _owner the owner of the contract
     function initialize(address _owner) external reentrancyGuardInitializer {
         _transferOwnership(_owner);
+
+        whitelistedSettlementLayers[L1_CHAIN_ID] = true;
     }
 
     //// Initialization and registration
@@ -670,7 +681,7 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus
         bytes32 _assetId,
         address _prevMsgSender,
         bytes calldata _data
-    ) external payable override onlyAssetRouter onlyL1 returns (bytes memory bridgehubMintData) {
+    ) external payable override onlyAssetRouter whenMigrationsNotPaused returns (bytes memory bridgehubMintData) {
         require(whitelistedSettlementLayers[_settlementChainId], "BH: SL not whitelisted");
 
         BridgehubBurnSTMAssetData memory bridgeData = abi.decode(_data, (BridgehubBurnSTMAssetData));
@@ -703,7 +714,7 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus
         uint256, // originChainId
         bytes32 _assetId,
         bytes calldata _bridgehubMintData
-    ) external payable override onlyAssetRouter {
+    ) external payable override onlyAssetRouter whenMigrationsNotPaused {
         BridgehubMintSTMAssetData memory bridgeData = abi.decode(_bridgehubMintData, (BridgehubMintSTMAssetData));
 
         address stm = stmAssetIdToAddress[_assetId];
@@ -712,16 +723,17 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus
 
         settlementLayer[bridgeData.chainId] = block.chainid;
         stateTransitionManager[bridgeData.chainId] = stm;
-        address hyperchain;
-        if (hyperchainMap.contains(bridgeData.chainId)) {
-            hyperchain = hyperchainMap.get(bridgeData.chainId);
-        } else {
+
+        address hyperchain = getHyperchain(bridgeData.chainId);
+        bool contractAlreadyDeployed = hyperchain != address(0);
+        if (!contractAlreadyDeployed) {
             hyperchain = IStateTransitionManager(stm).forwardedBridgeMint(bridgeData.chainId, bridgeData.stmData);
+            require(hyperchain != address(0), "BH: chain not registered");
+            _registerNewHyperchain(bridgeData.chainId, hyperchain);
+            messageRoot.addNewChain(bridgeData.chainId);
         }
 
-        messageRoot.addNewChainIfNeeded(bridgeData.chainId);
-        _registerNewHyperchain(bridgeData.chainId, hyperchain);
-        IZkSyncHyperchain(hyperchain).forwardedBridgeMint(bridgeData.chainData);
+        IZkSyncHyperchain(hyperchain).forwardedBridgeMint(bridgeData.chainData, contractAlreadyDeployed);
 
         emit MigrationFinalized(bridgeData.chainId, _assetId, hyperchain);
     }
@@ -768,4 +780,14 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus
     function unpause() external onlyOwner {
         _unpause();
     }
+
+    /// @notice Pauses migration functions.
+    function pauseMigration() external onlyOwner {
+        migrationPaused = true;
+    }
+
+    /// @notice Unpauses migration functions.
+    function unpauseMigration() external onlyOwner {
+        migrationPaused = false;
+    }
 }

l1-contracts/contracts/bridgehub/IBridgehub.sol

@@ -113,6 +113,8 @@ interface IBridgehub is IL1AssetHandler {
 
     function getAllHyperchainChainIDs() external view returns (uint256[] memory);
 
+    function migrationPaused() external view returns (bool);
+
     /// Mailbox forwarder
 
     function proveL2MessageInclusion(

l1-contracts/contracts/bridgehub/IMessageRoot.sol

@@ -12,6 +12,4 @@ interface IMessageRoot {
     function addNewChain(uint256 _chainId) external;
 
     function addChainBatchRoot(uint256 _chainId, uint256 _batchNumber, bytes32 _chainBatchRoot) external;
-
-    function addNewChainIfNeeded(uint256 _chainId) external;
 }

l1-contracts/contracts/bridgehub/MessageRoot.sol

@@ -45,19 +45,12 @@ contract MessageRoot is IMessageRoot, ReentrancyGuard {
     /// @notice The number of chains that are registered.
     uint256 public chainCount;
 
-    /// @notice The mapping from chainId to chainIndex.
+    /// @notice The mapping from chainId to chainIndex. Note index 0 is maintained for the chain the contract is on.
     mapping(uint256 chainId => uint256 chainIndex) public chainIndex;
 
     /// @notice The mapping from chainIndex to chainId.
     mapping(uint256 chainIndex => uint256 chainId) public chainIndexToId;
 
-    // There are two ways to distinguish chains:
-    // - Either by reserving the index 0 as a special value which denotes an unregistered chain
-    // - Use a separate mapping
-    // The second approach is used due to explicitness.
-    /// @notice The mapping from chainId to whether the chain is registered. Used because the chainIndex can be 0.
-    mapping(uint256 chainId => bool isRegistered) public chainRegistered;
-
     /// @notice The shared full merkle tree storing the aggregate hash.
     FullMerkle.FullTree public sharedTree;
 
@@ -91,16 +84,12 @@ contract MessageRoot is IMessageRoot, ReentrancyGuard {
     }
 
     function addNewChain(uint256 _chainId) external onlyBridgehub {
-        require(!chainRegistered[_chainId], "MR: chain exists");
+        require(!chainRegistered(_chainId), "MR: chain exists");
         _addNewChain(_chainId);
     }
 
-    /// @dev Adds a new chain to the message root if it has not been added yet.
-    /// @param _chainId the chainId of the chain
-    function addNewChainIfNeeded(uint256 _chainId) external onlyBridgehub {
-        if (!chainRegistered[_chainId]) {
-            _addNewChain(_chainId);
-        }
+    function chainRegistered(uint256 _chainId) public view returns (bool) {
+        return (_chainId == block.chainid || chainIndex[_chainId] != 0);
     }
 
     /// @dev add a new chainBatchRoot to the chainTree
@@ -109,7 +98,7 @@ contract MessageRoot is IMessageRoot, ReentrancyGuard {
         uint256 _batchNumber,
         bytes32 _chainBatchRoot
     ) external onlyChain(_chainId) {
-        require(chainRegistered[_chainId], "MR: not registered");
+        require(chainRegistered(_chainId), "MR: not registered");
         bytes32 chainRoot;
         // slither-disable-next-line unused-return
         (, chainRoot) = chainTree[_chainId].push(MessageHashing.batchLeafHash(_chainBatchRoot, _batchNumber));
@@ -124,6 +113,9 @@ contract MessageRoot is IMessageRoot, ReentrancyGuard {
 
     /// @dev Gets the aggregated root of all chains.
     function getAggregatedRoot() external view returns (bytes32) {
+        if (chainCount == 0) {
+            return SHARED_ROOT_TREE_EMPTY_HASH;
+        }
         return sharedTree.root();
     }
 
@@ -146,18 +138,12 @@ contract MessageRoot is IMessageRoot, ReentrancyGuard {
     function _initialize() internal {
         // slither-disable-next-line unused-return
         sharedTree.setup(SHARED_ROOT_TREE_EMPTY_HASH);
+        _addNewChain(block.chainid);
     }
 
     /// @dev Adds a single chain to the message root.
     /// @param _chainId the chainId of the chain
     function _addNewChain(uint256 _chainId) internal {
-        // The chain itself can not be the part of the message root.
-        // The message root will only aggregate chains that settle on it.
-        require(_chainId != block.chainid, "MR: chainId is this chain");
-
-        chainRegistered[_chainId] = true;
-
-        // We firstly increment `chainCount` and then apply it to ensure that `0` is reserved for chains that are not present.
         uint256 cachedChainCount = chainCount;
         require(cachedChainCount < MAX_NUMBER_OF_HYPERCHAINS, "MR: too many chains");
 
@@ -167,6 +153,7 @@ contract MessageRoot is IMessageRoot, ReentrancyGuard {
 
         // slither-disable-next-line unused-return
         bytes32 initialHash = chainTree[_chainId].setup(CHAIN_TREE_EMPTY_ENTRY_HASH);
+
         // slither-disable-next-line unused-return
         sharedTree.pushNewLeaf(MessageHashing.chainIdLeafHash(initialHash, _chainId));
 

l1-contracts/contracts/bridgehub/STMDeploymentTracker.sol

@@ -25,6 +25,9 @@ contract STMDeploymentTracker is ISTMDeploymentTracker, ReentrancyGuard, Ownable
     /// @dev Bridgehub smart contract that is used to operate with L2 via asynchronous L2 <-> L1 communication.
     IL1AssetRouter public immutable override L1_ASSET_ROUTER;
 
+    /// @dev The encoding version of the data.
+    bytes1 internal constant ENCODING_VERSION = 0x01;
+
     /// @notice Checks that the message sender is the bridgehub.
     modifier onlyBridgehub() {
         // solhint-disable-next-line gas-custom-errors
@@ -89,7 +92,9 @@ contract STMDeploymentTracker is ISTMDeploymentTracker, ReentrancyGuard, Ownable
         // solhint-disable-next-line gas-custom-errors
 
         require(_prevMsgSender == owner(), "STMDT: not owner");
-        (address _stmL1Address, address _stmL2Address) = abi.decode(_data, (address, address));
+        bytes1 encodingVersion = _data[0];
+        require(encodingVersion == ENCODING_VERSION, "STMDT: wrong encoding version");
+        (address _stmL1Address, address _stmL2Address) = abi.decode(_data[1:], (address, address));
 
         request = _registerSTMAssetOnL2Bridgehub(_chainId, _stmL1Address, _stmL2Address);
     }

l1-contracts/contracts/common/L2ContractAddresses.sol

@@ -47,4 +47,7 @@ address constant L2_ASSET_ROUTER_ADDR = address(0x10003);
 /// @dev An l2 system contract address, used in the assetId calculation for native assets.
 /// This is needed for automatic bridging, i.e. without deploying the AssetHandler contract,
 /// if the assetId can be calculated with this address then it is in fact an NTV asset
-address constant L2_NATIVE_TOKEN_VAULT_ADDRESS = address(0x10004);
+address constant L2_NATIVE_TOKEN_VAULT_ADDR = address(0x10004);
+
+/// @dev the address of the l2 asse3t router.
+address constant L2_MESSAGE_ROOT_ADDR = address(0x10005);