Internal audit fixes + new coverage

contracts/interfaces/IBasePolygonZkEVMGlobalExitRoot.sol

@@ -9,9 +9,9 @@ interface IBasePolygonZkEVMGlobalExitRoot {
     error OnlyAllowedContracts();
 
     /**
-     * @dev Thrown when the caller is not the coinbase
+     * @dev Thrown when the caller is not the coinbase neither the globalExitRootUpdater
      */
-    error OnlyAggOracleOrCoinbase();
+    error OnlyGlobalExitRootUpdater();
 
     /**
      * @dev Thrown when trying to insert a global exit root that is already set

contracts/v2/PolygonZkEVMBridgeV2.sol

@@ -740,15 +740,15 @@ contract PolygonZkEVMBridgeV2 is
      * @notice Function to activate the emergency state
      " Only can be called by the Polygon ZK-EVM in extreme situations
      */
-    function activateEmergencyState() external onlyRollupManager {
+    function activateEmergencyState() external virtual onlyRollupManager {
         _activateEmergencyState();
     }
 
     /**
      * @notice Function to deactivate the emergency state
      " Only can be called by the Polygon ZK-EVM
      */
-    function deactivateEmergencyState() external onlyRollupManager {
+    function deactivateEmergencyState() external virtual onlyRollupManager {
         _deactivateEmergencyState();
     }
 

contracts/v2/interfaces/IBridgeL2SovereignChains.sol

@@ -46,6 +46,27 @@ interface IBridgeL2SovereignChains is IPolygonZkEVMBridgeV2 {
      */
     error InvalidInitializeFunction();
 
+    /**
+     * @dev Thrown when initializing calling a function with invalid arrays length
+     */
+    error InputArraysLengthMismatch();
+
+    /**
+     * @dev Thrown when trying to map a token that is already mapped
+     */
+    error TokenAlreadyMapped();
+
+    /**
+     * @dev Thrown when trying to remove a legacy mapped token that has nor previously been remapped
+     */
+    error TokenNotRemapped();
+
+    /**
+     * @dev Thrown when trying to set a custom wrapper for weth on a gas token network
+     */
+    error WETHRemappingNotSupportedOnGasTokenNetworks();
+
+
     function initialize(
         uint32 _networkID,
         address _gasTokenAddress,

contracts/v2/sovereignChains/BridgeL2SovereignChain.sol

@@ -52,7 +52,7 @@ contract BridgeL2SovereignChain is
     /**
      * @dev Emitted when a remapped token is removed from mapping
      */
-    event RemoveSovereignTokenAddress(address sovereignTokenAddress);
+    event RemoveLegacySovereignTokenAddress(address sovereignTokenAddress);
 
     /**
      * @dev Emitted when a WETH address is remapped by a sovereign WETH address
@@ -118,6 +118,7 @@ contract BridgeL2SovereignChain is
             gasTokenAddress = _gasTokenAddress;
             gasTokenNetwork = _gasTokenNetwork;
             gasTokenMetadata = _gasTokenMetadata;
+
             // Set sovereign weth token or create new if not provided
             if (_sovereignWETHAddress == address(0)) {
                 // Create a wrapped token for WETH, with salt == 0
@@ -185,12 +186,14 @@ contract BridgeL2SovereignChain is
         address[] memory sovereignTokenAddresses,
         bool[] memory isNotMintable
     ) external onlyBridgeManager {
-        require(
-            originNetworks.length == originTokenAddresses.length &&
-                originTokenAddresses.length == sovereignTokenAddresses.length &&
-                sovereignTokenAddresses.length == isNotMintable.length,
-            "Input array lengths mismatch"
-        );
+        if (
+            originNetworks.length != originTokenAddresses.length ||
+            originNetworks.length != sovereignTokenAddresses.length ||
+            originNetworks.length != isNotMintable.length
+        ) {
+            revert InputArraysLengthMismatch();
+        }
+
         // Make multiple calls to setSovereignTokenAddress
         for (uint256 i = 0; i < sovereignTokenAddresses.length; i++) {
             _setSovereignTokenAddress(
@@ -229,7 +232,16 @@ contract BridgeL2SovereignChain is
     }
 
     /**
-     * @notice Function to remap sovereign address
+     * @notice Remap a wrapped token to a new sovereign token address
+     * @dev This function is used to allow any existing token to be mapped with
+     *      origin token.
+     * @notice If this function is called multiple times for the same existingTokenAddress,
+     * this will override the previous calls and only keep the last sovereignTokenAddress.
+     * @notice The tokenInfoToWrappedToken mapping  value is replaced by the new sovereign address but it's not the case for the wrappedTokenToTokenInfo map where the value is added, this way user will always be able to withdraw their tokens
+     * @param originNetwork Origin network
+     * @param originTokenAddress Origin token address, 0 address is reserved for ether
+     * @param sovereignTokenAddress Address of the sovereign wrapped token
+     * @param isNotMintable Flag to indicate if the wrapped token is not mintable
      */
     function _setSovereignTokenAddress(
         uint32 originNetwork,
@@ -248,13 +260,22 @@ contract BridgeL2SovereignChain is
         if (originNetwork == networkID) {
             revert OriginNetworkInvalid();
         }
+        // Check if the token is already mapped
+        if (
+            wrappedTokenToTokenInfo[sovereignTokenAddress].originTokenAddress !=
+            address(0)
+        ) {
+            revert TokenAlreadyMapped();
+        }
+
         // Compute token info hash
         bytes32 tokenInfoHash = keccak256(
             abi.encodePacked(originNetwork, originTokenAddress)
         );
         // Set the address of the wrapper
         tokenInfoToWrappedToken[tokenInfoHash] = sovereignTokenAddress;
         // Set the token info mapping
+        // @note wrappedTokenToTokenInfo mapping is not overwritten while tokenInfoToWrappedToken it is
         wrappedTokenToTokenInfo[sovereignTokenAddress] = TokenInformation(
             originNetwork,
             originTokenAddress
@@ -274,10 +295,10 @@ contract BridgeL2SovereignChain is
      * @notice Although the token is removed from the mapping, the user will still be able to withdraw their tokens using tokenInfoToWrappedToken mapping
      * @param sovereignTokenAddress Address of the sovereign wrapped token
      */
-    function removeSovereignTokenAddress(
+    function removeLegacySovereignTokenAddress(
         address sovereignTokenAddress
     ) external onlyBridgeManager {
-        // Only allow to remove already mapped tokens
+        // Only allow to remove already remapped tokens
         TokenInformation memory tokenInfo = wrappedTokenToTokenInfo[
             sovereignTokenAddress
         ];
@@ -292,11 +313,11 @@ contract BridgeL2SovereignChain is
             tokenInfoToWrappedToken[tokenInfoHash] == address(0) ||
             tokenInfoToWrappedToken[tokenInfoHash] == sovereignTokenAddress
         ) {
-            revert TokenNotMapped();
+            revert TokenNotRemapped();
         }
         delete wrappedTokenToTokenInfo[sovereignTokenAddress];
         delete wrappedAddressIsNotMintable[sovereignTokenAddress];
-        emit RemoveSovereignTokenAddress(sovereignTokenAddress);
+        emit RemoveLegacySovereignTokenAddress(sovereignTokenAddress);
     }
 
     /**
@@ -309,6 +330,9 @@ contract BridgeL2SovereignChain is
         address sovereignWETHTokenAddress,
         bool isNotMintable
     ) external onlyBridgeManager {
+        if (gasTokenAddress == address(0)) {
+            revert WETHRemappingNotSupportedOnGasTokenNetworks();
+        }
         WETHToken = TokenWrapped(sovereignWETHTokenAddress);
         wrappedAddressIsNotMintable[sovereignWETHTokenAddress] = isNotMintable;
         emit SetSovereignWETHAddress(sovereignWETHTokenAddress, isNotMintable);
@@ -411,4 +435,19 @@ contract BridgeL2SovereignChain is
             tokenWrapped.mint(destinationAddress, amount);
         }
     }
+
+    // @note This function is not used in the current implementation. We overwrite it to improve deployed bytecode size
+    function activateEmergencyState()
+        external
+        override(IPolygonZkEVMBridgeV2, PolygonZkEVMBridgeV2)
+    {
+        revert NotValidBridgeManager();
+    }
+
+    function deactivateEmergencyState()
+        external
+        override(IPolygonZkEVMBridgeV2, PolygonZkEVMBridgeV2)
+    {
+        revert NotValidBridgeManager();
+    }
 }

contracts/v2/sovereignChains/GlobalExitRootManagerL2SovereignChain.sol

@@ -13,8 +13,8 @@ contract GlobalExitRootManagerL2SovereignChain is
     PolygonZkEVMGlobalExitRootL2,
     Initializable
 {
-    // aggOracleAddress address
-    address public aggOracleAddress;
+    // globalExitRootUpdater address
+    address public globalExitRootUpdater;
 
     // Inserted GER counter
     uint256 public insertedGERCount;
@@ -29,13 +29,16 @@ contract GlobalExitRootManagerL2SovereignChain is
      */
     event RemoveLastGlobalExitRoot(bytes32 indexed removedGlobalExitRoot);
 
-    modifier onlyAggOracleOrCoinbase() {
-        // Only allowed to be called by aggOracle or coinbase if aggOracle is zero
-        if (
-            (aggOracleAddress == address(0) && block.coinbase != msg.sender) ||
-            (aggOracleAddress != address(0) && aggOracleAddress != msg.sender)
-        ) {
-            revert OnlyAggOracleOrCoinbase();
+    modifier onlyGlobalExitRootUpdater() {
+        // Only allowed to be called by GlobalExitRootUpdater or coinbase if GlobalExitRootUpdater is zero
+        if (globalExitRootUpdater == address(0)) {
+            if (block.coinbase != msg.sender) {
+                revert OnlyGlobalExitRootUpdater();
+            }
+        } else {
+            if (globalExitRootUpdater != msg.sender) {
+                revert OnlyGlobalExitRootUpdater();
+            }
         }
         _;
     }
@@ -50,13 +53,13 @@ contract GlobalExitRootManagerL2SovereignChain is
     }
 
     /**
-     * @notice Initialize contract setting the aggOracleAddress
+     * @notice Initialize contract setting the globalExitRootUpdater
      */
     function initialize(
-        address _aggOracleAddress
+        address _globalExitRootUpdater
     ) external virtual initializer {
-        // set aggOracleAddress
-        aggOracleAddress = _aggOracleAddress;
+        // set globalExitRootUpdater
+        globalExitRootUpdater = _globalExitRootUpdater;
     }
 
     /**
@@ -65,7 +68,7 @@ contract GlobalExitRootManagerL2SovereignChain is
      */
     function insertGlobalExitRoot(
         bytes32 _newRoot
-    ) external onlyAggOracleOrCoinbase {
+    ) external onlyGlobalExitRootUpdater {
         // do not insert GER if already set
         if (globalExitRootMap[_newRoot] == 0) {
             globalExitRootMap[_newRoot] = ++insertedGERCount;
@@ -81,9 +84,10 @@ contract GlobalExitRootManagerL2SovereignChain is
      */
     function removeLastGlobalExitRoots(
         bytes32[] calldata gersToRemove
-    ) external onlyAggOracleOrCoinbase {
+    ) external onlyGlobalExitRootUpdater {
+        uint256 insertedGERCountCache = insertedGERCount;
         // Can't remove if not enough roots have been inserted
-        if (gersToRemove.length > insertedGERCount) {
+        if (gersToRemove.length > insertedGERCountCache) {
             revert NotEnoughGlobalExitRootsInserted();
         }
         // Iterate through the array of roots to remove them one by one
@@ -92,12 +96,12 @@ contract GlobalExitRootManagerL2SovereignChain is
 
             // Check that the root to remove is the last inserted
             uint256 lastInsertedIndex = globalExitRootMap[rootToRemove];
-            if (lastInsertedIndex != insertedGERCount) {
+            if (lastInsertedIndex != insertedGERCountCache) {
                 revert NotLastInsertedGlobalExitRoot();
             }
 
             // Remove from the mapping
-            delete globalExitRootMap[rootToRemove]; // Delete from the mapping
+            delete globalExitRootMap[rootToRemove];
             // Decrement the counter
             insertedGERCount--;
 

hardhat.config.ts

@@ -80,7 +80,7 @@ const config: HardhatUserConfig = {
                 settings: {
                     optimizer: {
                         enabled: true,
-                        runs: 0,
+                        runs: 20,
                     },
                     evmVersion: "shanghai",
                 }, // try yul optimizer

test/contractsv2/BridgeL2GasTokenMappedSovereignChains.test.ts

@@ -263,32 +263,6 @@ describe("SovereignChainBridge Gas tokens tests", () => {
         expect(await sovereignChainBridgeContract.gasTokenMetadata()).to.be.equal(gasTokenMetadata);
     });
 
-    it("should check the emergency state", async () => {
-        expect(await sovereignChainBridgeContract.isEmergencyState()).to.be.equal(false);
-
-        await expect(sovereignChainBridgeContract.activateEmergencyState()).to.be.revertedWithCustomError(
-            sovereignChainBridgeContract,
-            "OnlyRollupManager"
-        );
-        await expect(sovereignChainBridgeContract.connect(rollupManager).activateEmergencyState()).to.emit(
-            sovereignChainBridgeContract,
-            "EmergencyStateActivated"
-        );
-
-        expect(await sovereignChainBridgeContract.isEmergencyState()).to.be.equal(true);
-
-        await expect(
-            sovereignChainBridgeContract.connect(deployer).deactivateEmergencyState()
-        ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "OnlyRollupManager");
-
-        await expect(sovereignChainBridgeContract.connect(rollupManager).deactivateEmergencyState()).to.emit(
-            sovereignChainBridgeContract,
-            "EmergencyStateDeactivated"
-        );
-
-        expect(await sovereignChainBridgeContract.isEmergencyState()).to.be.equal(false);
-    });
-
     it("should SovereignChain bridge asset and verify merkle proof", async () => {
         const depositCount = await sovereignChainBridgeContract.depositCount();
         const originNetwork = networkIDRollup2;