feat: ZKChain Upgrades and Libraries Diff Audit

l1-contracts/contracts/bridge/L2SharedBridgeLegacy.sol

@@ -7,7 +7,7 @@ import {UpgradeableBeacon} from "@openzeppelin/contracts-v4/proxy/beacon/Upgrade
 
 import {BridgedStandardERC20} from "./BridgedStandardERC20.sol";
 
-import {DEPLOYER_SYSTEM_CONTRACT, L2_ASSET_ROUTER_ADDR, L2_NATIVE_TOKEN_VAULT_ADDR} from "../common/L2ContractAddresses.sol";
+import {L2_DEPLOYER_SYSTEM_CONTRACT_ADDR, L2_ASSET_ROUTER_ADDR, L2_NATIVE_TOKEN_VAULT_ADDR} from "../common/L2ContractAddresses.sol";
 import {SystemContractsCaller} from "../common/libraries/SystemContractsCaller.sol";
 import {L2ContractHelper, IContractDeployer} from "../common/libraries/L2ContractHelper.sol";
 import {AddressAliasHelper} from "../vendor/AddressAliasHelper.sol";
@@ -175,7 +175,7 @@ contract L2SharedBridgeLegacy is IL2SharedBridgeLegacy, Initializable {
     function deployBeaconProxy(bytes32 salt) external onlyNTV returns (address proxy) {
         (bool success, bytes memory returndata) = SystemContractsCaller.systemCallWithReturndata(
             uint32(gasleft()),
-            DEPLOYER_SYSTEM_CONTRACT,
+            L2_DEPLOYER_SYSTEM_CONTRACT_ADDR,
             0,
             abi.encodeCall(
                 IContractDeployer.create2,

l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol

@@ -16,7 +16,7 @@ import {NativeTokenVault} from "./NativeTokenVault.sol";
 import {IL2SharedBridgeLegacy} from "../interfaces/IL2SharedBridgeLegacy.sol";
 import {BridgedStandardERC20} from "../BridgedStandardERC20.sol";
 
-import {DEPLOYER_SYSTEM_CONTRACT, L2_ASSET_ROUTER_ADDR} from "../../common/L2ContractAddresses.sol";
+import {L2_DEPLOYER_SYSTEM_CONTRACT_ADDR, L2_ASSET_ROUTER_ADDR} from "../../common/L2ContractAddresses.sol";
 import {L2ContractHelper, IContractDeployer} from "../../common/libraries/L2ContractHelper.sol";
 
 import {SystemContractsCaller} from "../../common/libraries/SystemContractsCaller.sol";
@@ -163,7 +163,7 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault {
 
             (bool success, bytes memory returndata) = SystemContractsCaller.systemCallWithReturndata(
                 uint32(gasleft()),
-                DEPLOYER_SYSTEM_CONTRACT,
+                L2_DEPLOYER_SYSTEM_CONTRACT_ADDR,
                 0,
                 abi.encodeCall(
                     IContractDeployer.create2,

l1-contracts/contracts/bridgehub/IMessageRoot.sol

@@ -4,8 +4,11 @@ pragma solidity 0.8.24;
 
 import {IBridgehub} from "./IBridgehub.sol";
 
-/// @author Matter Labs
-/// @custom:security-contact [email protected]
+/**
+ * @author Matter Labs
+ * @notice MessageRoot contract is responsible for storing and aggregating the roots of the batches from different chains into the MessageRoot.
+ * @custom:security-contact [email protected]
+ */
 interface IMessageRoot {
     function BRIDGE_HUB() external view returns (IBridgehub);
 

l1-contracts/contracts/common/Config.sol

@@ -18,7 +18,6 @@ uint256 constant MAX_L2_TO_L1_LOGS_COMMITMENT_BYTES = 4 + L2_TO_L1_LOG_SERIALIZE
 /// @dev Actually equal to the `keccak256(new bytes(L2_TO_L1_LOG_SERIALIZE_SIZE))`
 bytes32 constant L2_L1_LOGS_TREE_DEFAULT_LEAF_HASH = 0x72abee45b59e344af8a6e520241c4744aff26ed411f4c4b00f8af09adada43ba;
 
-// TODO: change constant to the real root hash of empty Merkle tree (SMA-184)
 bytes32 constant DEFAULT_L2_LOGS_TREE_ROOT_HASH = bytes32(0);
 
 /// @dev Denotes the type of the ZKsync transaction that came from L1.

l1-contracts/contracts/common/L1ContractErrors.sol

@@ -314,6 +314,8 @@ error IncorrectBatchBounds(
 );
 // 0x64107968
 error AssetHandlerNotRegistered(bytes32 assetId);
+// 0xff4bbdf1
+error NotAHyperchain(address chainAddress);
 // 0x64846fe4
 error NotARestriction(address addr);
 // 0xfa5cd00f

l1-contracts/contracts/common/L2ContractAddresses.sol

@@ -69,15 +69,12 @@ interface IL2Messenger {
 /// if the assetId can be calculated with this address then it is in fact an NTV asset
 address constant L2_NATIVE_TOKEN_VAULT_ADDR = address(0x10004);
 
-/// @dev the address of the l2 asse3t router.
+/// @dev the address of the l2 asset router.
 address constant L2_MESSAGE_ROOT_ADDR = address(0x10005);
 
 /// @dev the offset for the system contracts
 uint160 constant SYSTEM_CONTRACTS_OFFSET = 0x8000; // 2^15
 
-/// @dev the address of the deployer system contract
-address constant DEPLOYER_SYSTEM_CONTRACT = address(SYSTEM_CONTRACTS_OFFSET + 0x06);
-
 /// @dev the address of the l2 messenger system contract
 IL2Messenger constant L2_MESSENGER = IL2Messenger(address(SYSTEM_CONTRACTS_OFFSET + 0x08));
 

l1-contracts/contracts/common/libraries/DataEncoding.sol

@@ -148,7 +148,8 @@ library DataEncoding {
         }
     }
 
-    /// @notice Encodes the token data by combining chain id, asset deployment tracker and asset data.
+    /// @notice Encodes the token data by combining chain id, and its metadata.
+    /// @dev Note that all the metadata of the token is expected to be ABI encoded.
     /// @param _chainId The id of the chain token is native to.
     /// @param _name The name of the token.
     /// @param _symbol The symbol of the token.

l1-contracts/contracts/common/libraries/SystemContractsCaller.sol

@@ -48,7 +48,7 @@ library SystemContractsCaller {
         assembly {
             dataStart := add(data, 0x20)
         }
-        uint32 dataLength = uint32(Utils.safeCastToU32(data.length));
+        uint32 dataLength = Utils.safeCastToU32(data.length);
 
         uint256 farCallAbi = getFarCallABI({
             dataOffset: 0,

l1-contracts/contracts/common/libraries/UnsafeBytes.sol

@@ -30,13 +30,6 @@ library UnsafeBytes {
         }
     }
 
-    function readUint128(bytes memory _bytes, uint256 _start) internal pure returns (uint128 result, uint256 offset) {
-        assembly {
-            offset := add(_start, 16)
-            result := mload(add(_bytes, offset))
-        }
-    }
-
     function readUint256(bytes memory _bytes, uint256 _start) internal pure returns (uint256 result, uint256 offset) {
         assembly {
             offset := add(_start, 32)

l1-contracts/contracts/dev-contracts/test/CustomUpgradeTest.sol

@@ -25,7 +25,7 @@ contract CustomUpgradeTest is BaseZkSyncUpgrade {
     /// upgrade.
     function _postUpgrade(bytes calldata _customCallDataForUpgrade) internal override {}
 
-    /// @notice The main function that will be called by the upgrade proxy.
+    /// @notice The main function that will be delegate-called by the chain.
     /// @param _proposedUpgrade The upgrade to be executed.
     function upgrade(ProposedUpgrade calldata _proposedUpgrade) public override returns (bytes32) {
         (uint32 newMinorVersion, bool isPatchOnly) = _setNewProtocolVersion(_proposedUpgrade.newProtocolVersion);

l1-contracts/contracts/dev-contracts/test/DummySharedBridge.sol

@@ -10,7 +10,6 @@ import {TWO_BRIDGES_MAGIC_VALUE, ETH_TOKEN_ADDRESS} from "../../common/Config.so
 import {IL1NativeTokenVault} from "../../bridge/ntv/L1NativeTokenVault.sol";
 import {L2_NATIVE_TOKEN_VAULT_ADDR} from "../../common/L2ContractAddresses.sol";
 import {SafeERC20} from "@openzeppelin/contracts-v4/token/ERC20/utils/SafeERC20.sol";
-import {IL2Bridge} from "../../bridge/interfaces/IL2Bridge.sol";
 import {IL2SharedBridgeLegacy} from "../../bridge/interfaces/IL2SharedBridgeLegacy.sol";
 import {IL2SharedBridgeLegacyFunctions} from "../../bridge/interfaces/IL2SharedBridgeLegacyFunctions.sol";
 

l1-contracts/contracts/governance/PermanentRestriction.sol

@@ -2,7 +2,7 @@
 
 pragma solidity 0.8.24;
 
-import {CallNotAllowed, RemovingPermanentRestriction, ZeroAddress, UnallowedImplementation, AlreadyWhitelisted, NotAllowed} from "../common/L1ContractErrors.sol";
+import {CallNotAllowed, RemovingPermanentRestriction, ZeroAddress, UnallowedImplementation, AlreadyWhitelisted, NotAHyperchain, NotAllowed} from "../common/L1ContractErrors.sol";
 
 import {L2TransactionRequestTwoBridgesOuter, BridgehubBurnCTMAssetData} from "../bridgehub/IBridgehub.sol";
 import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable-v4/access/Ownable2StepUpgradeable.sol";
@@ -219,6 +219,13 @@ contract PermanentRestriction is Restriction, IPermanentRestriction, Ownable2Ste
         // - Query the Bridgehub for the Hyperchain with the given `chainId`.
         // - We compare the corresponding addresses
 
+        // Note, that we do not use an explicit call here to ensure that the function does not panic in case of
+        // incorrect `_chain` address.
+        (bool success, bytes memory data) = _chain.staticcall(abi.encodeWithSelector(IGetters.getChainId.selector));
+        if (!success || data.length < 32) {
+            revert NotAHyperchain(_chain);
+        }
+
         // Note, that we do use assembly here to ensure that the function does not panic in case of
         // either incorrect `_chain` address or in case the returndata is too large
 

l1-contracts/contracts/transactionFilterer/GatewayTransactionFilterer.sol

@@ -8,7 +8,6 @@ import {ReentrancyGuard} from "../common/ReentrancyGuard.sol";
 import {AlreadyWhitelisted, InvalidSelector, NotWhitelisted, ZeroAddress} from "../common/L1ContractErrors.sol";
 import {ITransactionFilterer} from "../state-transition/chain-interfaces/ITransactionFilterer.sol";
 import {IBridgehub} from "../bridgehub/IBridgehub.sol";
-import {IL2Bridge} from "../bridge/interfaces/IL2Bridge.sol";
 import {IAssetRouterBase} from "../bridge/asset-router/IAssetRouterBase.sol";
 import {IL2AssetRouter} from "../bridge/asset-router/IL2AssetRouter.sol";
 
@@ -86,25 +85,22 @@ contract GatewayTransactionFilterer is ITransactionFilterer, ReentrancyGuard, Ow
 
             if (IL2AssetRouter.setAssetHandlerAddress.selector == l2TxSelector) {
                 (, bytes32 decodedAssetId, ) = abi.decode(l2Calldata[4:], (uint256, bytes32, address));
-                return _checkSTMAssetId(decodedAssetId);
+                return _checkCTMAssetId(decodedAssetId);
             }
 
-            if (
-                IAssetRouterBase.finalizeDeposit.selector != l2TxSelector &&
-                IL2Bridge.finalizeDeposit.selector != l2TxSelector
-            ) {
+            if (IAssetRouterBase.finalizeDeposit.selector != l2TxSelector) {
                 revert InvalidSelector(l2TxSelector);
             }
 
             (, bytes32 decodedAssetId, ) = abi.decode(l2Calldata[4:], (uint256, bytes32, bytes));
-            return _checkSTMAssetId(decodedAssetId);
+            return _checkCTMAssetId(decodedAssetId);
         }
 
         return whitelistedSenders[sender];
     }
 
-    function _checkSTMAssetId(bytes32 assetId) internal view returns (bool) {
-        address stmAddress = BRIDGE_HUB.ctmAssetIdToAddress(assetId);
-        return stmAddress != address(0);
+    function _checkCTMAssetId(bytes32 assetId) internal view returns (bool) {
+        address ctmAddress = BRIDGE_HUB.ctmAssetIdToAddress(assetId);
+        return ctmAddress != address(0);
     }
 }

l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol

@@ -61,7 +61,7 @@ abstract contract BaseZkSyncUpgrade is ZKChainBase {
     /// @notice Notifies about complete upgrade
     event UpgradeComplete(uint256 indexed newProtocolVersion, bytes32 indexed l2UpgradeTxHash, ProposedUpgrade upgrade);
 
-    /// @notice The main function that will be provided by the upgrade proxy
+    /// @notice The main function that will be delegate-called by the chain.
     /// @dev This is a virtual function and should be overridden by custom upgrade implementations.
     /// @param _proposedUpgrade The upgrade to be executed.
     /// @return txHash The hash of the L2 system contract upgrade transaction.

l1-contracts/contracts/upgrades/DefaultUpgrade.sol

@@ -8,7 +8,7 @@ import {BaseZkSyncUpgrade, ProposedUpgrade} from "./BaseZkSyncUpgrade.sol";
 /// @author Matter Labs
 /// @custom:security-contact [email protected]
 contract DefaultUpgrade is BaseZkSyncUpgrade {
-    /// @notice The main function that will be called by the upgrade proxy.
+    /// @notice The main function that will be delegate-called by the chain.
     /// @param _proposedUpgrade The upgrade to be executed.
     function upgrade(ProposedUpgrade calldata _proposedUpgrade) public override returns (bytes32) {
         super.upgrade(_proposedUpgrade);

l1-contracts/contracts/upgrades/GatewayUpgrade.sol

@@ -33,14 +33,17 @@ contract GatewayUpgrade is BaseZkSyncUpgrade {
     using PriorityQueue for PriorityQueue.Queue;
     using PriorityTree for PriorityTree.Tree;
 
+    /// @notice The address of this contract.
+    /// @dev needed as this address is delegateCalled, and we delegateCall it again.
     address public immutable THIS_ADDRESS;
 
     constructor() {
         THIS_ADDRESS = address(this);
     }
 
-    /// @notice The main function that will be called by the upgrade proxy.
+    /// @notice The main function that will be delegate-called by the chain.
     /// @param _proposedUpgrade The upgrade to be executed.
+    /// @dev Doesn't require any access-control restrictions as the contract is used in the delegate call.
     function upgrade(ProposedUpgrade calldata _proposedUpgrade) public override returns (bytes32) {
         GatewayUpgradeEncodedInput memory encodedInput = abi.decode(
             _proposedUpgrade.postUpgradeCalldata,
@@ -72,16 +75,15 @@ contract GatewayUpgrade is BaseZkSyncUpgrade {
         );
 
         // slither-disable-next-line controlled-delegatecall
-        (bool success, ) = THIS_ADDRESS.delegatecall(
-            abi.encodeWithSelector(IGatewayUpgrade.upgradeExternal.selector, proposedUpgrade)
-        );
+        (bool success, ) = THIS_ADDRESS.delegatecall(abi.encodeCall(IGatewayUpgrade.upgradeExternal, proposedUpgrade));
         if (!success) {
             revert GatewayUpgradeFailed();
         }
         return Diamond.DIAMOND_INIT_SUCCESS_RETURN_VALUE;
     }
 
     /// @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 {
         super.upgrade(_proposedUpgrade);
     }

l1-contracts/contracts/upgrades/IGatewayUpgrade.sol

@@ -4,6 +4,14 @@ pragma solidity 0.8.24;
 
 import {ProposedUpgrade} from "./BaseZkSyncUpgrade.sol";
 
+/**
+ * @author Matter Labs
+ * @custom:security-contact [email protected]
+ * @notice Gateway upgrade interface. Used for the protocol upgrade that introduces the Gateway.
+ */
 interface IGatewayUpgrade {
+    /// @notice The upgrade function called from within this same contract
+    /// @dev This is needed for memory -> calldata conversion of the _upgrade arg.
+    /// @param _upgrade The upgrade to be executed.
     function upgradeExternal(ProposedUpgrade calldata _upgrade) external returns (bytes32);
 }

l1-contracts/contracts/upgrades/IL1GenesisUpgrade.sol

@@ -4,15 +4,32 @@ pragma solidity 0.8.24;
 
 import {L2CanonicalTransaction} from "../common/Messaging.sol";
 
+/**
+ * @author Matter Labs
+ * @custom:security-contact [email protected]
+ * @notice L1 genesis upgrade interface. Every chain has to process an upgrade txs at its genesis.
+ * @notice This is needed to set system params like the chainId and to deploy some system contracts.
+ */
 interface IL1GenesisUpgrade {
     /// @dev emitted when a chain registers and a GenesisUpgrade happens
+    /// @param _zkChain the address of the zk chain
+    /// @param _l2Transaction the l2 genesis upgrade transaction
+    /// @param _protocolVersion the current protocol version
+    /// @param _factoryDeps the factory dependencies needed for the upgrade
     event GenesisUpgrade(
         address indexed _zkChain,
         L2CanonicalTransaction _l2Transaction,
         uint256 indexed _protocolVersion,
         bytes[] _factoryDeps
     );
 
+    /// @notice The main function that will be called by the Admin facet at genesis.
+    /// @param _l1GenesisUpgrade the address of the l1 genesis upgrade
+    /// @param _chainId the chain id
+    /// @param _protocolVersion the current protocol version
+    /// @param _l1CtmDeployerAddress the address of the l1 ctm deployer
+    /// @param _forceDeployments the force deployments
+    /// @param _factoryDeps the factory dependencies
     function genesisUpgrade(
         address _l1GenesisUpgrade,
         uint256 _chainId,

l1-contracts/contracts/upgrades/L1GenesisUpgrade.sol

@@ -24,7 +24,13 @@ import {L1GatewayHelper} from "./L1GatewayHelper.sol";
 /// @author Matter Labs
 /// @custom:security-contact [email protected]
 contract L1GenesisUpgrade is IL1GenesisUpgrade, BaseZkSyncUpgradeGenesis {
-    /// @notice The main function that will be called by the upgrade proxy.
+    /// @notice The main function that will be called by the Admin facet.
+    /// @param _l1GenesisUpgrade the address of the l1 genesis upgrade
+    /// @param _chainId the chain id
+    /// @param _protocolVersion the current protocol version
+    /// @param _l1CtmDeployerAddress the address of the l1 ctm deployer
+    /// @param _fixedForceDeploymentsData the force deployments data
+    /// @param _factoryDeps the factory dependencies
     function genesisUpgrade(
         address _l1GenesisUpgrade,
         uint256 _chainId,

l1-contracts/deploy-scripts/DeployL1.s.sol

@@ -44,7 +44,6 @@ import {BridgedStandardERC20} from "contracts/bridge/BridgedStandardERC20.sol";
 import {AddressHasNoCode} from "./ZkSyncScriptErrors.sol";
 import {ICTMDeploymentTracker} from "contracts/bridgehub/ICTMDeploymentTracker.sol";
 import {IMessageRoot} from "contracts/bridgehub/IMessageRoot.sol";
-import {IL2ContractDeployer} from "contracts/common/interfaces/IL2ContractDeployer.sol";
 import {L2ContractHelper} from "contracts/common/libraries/L2ContractHelper.sol";
 import {AddressAliasHelper} from "contracts/vendor/AddressAliasHelper.sol";
 import {IL1Nullifier} from "contracts/bridge/L1Nullifier.sol";

l1-contracts/test/foundry/l1/integration/l2-tests-in-l1-context/L2WethTestAbstract.t.sol

@@ -11,9 +11,7 @@ import {BridgedStandardERC20} from "contracts/bridge/BridgedStandardERC20.sol";
 import {L2AssetRouter} from "contracts/bridge/asset-router/L2AssetRouter.sol";
 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 {Unauthorized, BridgeMintNotImplemented} from "contracts/common/L1ContractErrors.sol";
 import {L2_ASSET_ROUTER_ADDR, L2_NATIVE_TOKEN_VAULT_ADDR, L2_BRIDGEHUB_ADDR} from "contracts/common/L2ContractAddresses.sol";
 import {ETH_TOKEN_ADDRESS, SETTLEMENT_LAYER_RELAY_SENDER} from "contracts/common/Config.sol";
 
@@ -31,7 +29,6 @@ import {IZKChain} from "contracts/state-transition/chain-interfaces/IZKChain.sol
 import {SystemContractsArgs} from "./_SharedL2ContractL1DeployerUtils.sol";
 
 import {DeployUtils} from "deploy-scripts/DeployUtils.s.sol";
-import {Unauthorized, BridgeMintNotImplemented} from "contracts/common/L1ContractErrors.sol";
 
 abstract contract L2WethTestAbstract is Test, SharedL2ContractDeployer {
     function test_shouldDepositWethByCallingDeposit() public {