Review fixes III

0xPolygonHermez · Sep 19, 2024 · fda25e4 · fda25e4
1 parent d922a88
commit fda25e4
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 26 deletions.
diff --git a/contracts/v2/sovereignChains/BridgeL2SovereignChain.sol b/contracts/v2/sovereignChains/BridgeL2SovereignChain.sol
@@ -16,7 +16,6 @@ contract BridgeL2SovereignChain is
     PolygonZkEVMBridgeV2,
     IBridgeL2SovereignChains
 {
-
     using SafeERC20Upgradeable for IERC20Upgradeable;
 
     // Map to store wrappedAddresses that are not mintable
@@ -31,6 +30,29 @@ contract BridgeL2SovereignChain is
      */
     event BridgeManagerUpdated(address bridgeManager);
 
+    /**
+     * @dev Emitted when a token address is remapped by a sovereign token address
+     */
+    event SetSovereignTokenAddress(
+        uint32 originNetwork,
+        address originTokenAddress,
+        address sovereignTokenAddress,
+        bool isNotMintable
+    );
+
+    /**
+     * @dev Emitted when a remapped token is removed from mapping
+     */
+    event RemoveSovereignTokenAddress(address sovereignTokenAddress);
+
+    /**
+     * @dev Emitted when a WETH address is remapped by a sovereign WETH address
+     */
+    event SetSovereignWETHAddress(
+        address sovereignWETHTokenAddress,
+        bool isNotMintable
+    );
+
     /**
      * Disable initalizers on the implementation following the best practices
      */
@@ -129,6 +151,12 @@ contract BridgeL2SovereignChain is
             originTokenAddress
         );
         wrappedAddressIsNotMintable[sovereignTokenAddress] = isNotMintable;
+        emit SetSovereignTokenAddress(
+            originNetwork,
+            originTokenAddress,
+            sovereignTokenAddress,
+            isNotMintable
+        );
     }
 
     /**
@@ -141,16 +169,22 @@ contract BridgeL2SovereignChain is
         address sovereignTokenAddress
     ) external onlyBridgeManager {
         // Only allow to remove already mapped tokens
-        TokenInformation memory tokenInfo = wrappedTokenToTokenInfo[sovereignTokenAddress];
+        TokenInformation memory tokenInfo = wrappedTokenToTokenInfo[
+            sovereignTokenAddress
+        ];
         bytes32 tokenInfoHash = keccak256(
-                        abi.encodePacked(tokenInfo.originNetwork, tokenInfo.originTokenAddress)
-                    );
+            abi.encodePacked(
+                tokenInfo.originNetwork,
+                tokenInfo.originTokenAddress
+            )
+        );
 
-        if(tokenInfoToWrappedToken[tokenInfoHash] == address(0)) {
+        if (tokenInfoToWrappedToken[tokenInfoHash] == address(0)) {
             revert TokenNotMapped();
         }
         delete wrappedTokenToTokenInfo[sovereignTokenAddress];
         delete wrappedAddressIsNotMintable[sovereignTokenAddress];
+        emit RemoveSovereignTokenAddress(sovereignTokenAddress);
     }
 
     /**
@@ -165,9 +199,9 @@ contract BridgeL2SovereignChain is
     ) external onlyBridgeManager {
         WETHToken = TokenWrapped(sovereignWETHTokenAddress);
         wrappedAddressIsNotMintable[sovereignWETHTokenAddress] = isNotMintable;
+        emit SetSovereignWETHAddress(sovereignWETHTokenAddress, isNotMintable);
     }
 
-
     /**
      * @notice Burn tokens from wrapped token to execute the bridge
      * note This  function has been extracted to be able to override it by other contracts like Bridge2SovereignChain
@@ -182,7 +216,11 @@ contract BridgeL2SovereignChain is
         // or (2) wrapped with custom contract from origin network
         if (wrappedAddressIsNotMintable[address(tokenWrapped)]) {
             // Don't use burn but transfer to bridge
-            IERC20Upgradeable(address(tokenWrapped)).safeTransferFrom(msg.sender, address(this), amount);
+            IERC20Upgradeable(address(tokenWrapped)).safeTransferFrom(
+                msg.sender,
+                address(this),
+                amount
+            );
         } else {
             // Burn tokens
             tokenWrapped.burn(msg.sender, amount);
@@ -204,7 +242,10 @@ contract BridgeL2SovereignChain is
         // If is not mintable transfer instead of mint
         if (wrappedAddressIsNotMintable[address(tokenWrapped)]) {
             // Transfer wETH
-            IERC20Upgradeable(address(tokenWrapped)).safeTransfer(destinationAddress, amount);
+            IERC20Upgradeable(address(tokenWrapped)).safeTransfer(
+                destinationAddress,
+                amount
+            );
         } else {
             // Claim wETH
             tokenWrapped.mint(destinationAddress, amount);

diff --git a/contracts/v2/sovereignChains/GlobalExitRootManagerL2SovereignChain.sol b/contracts/v2/sovereignChains/GlobalExitRootManagerL2SovereignChain.sol
@@ -12,16 +12,6 @@ contract GlobalExitRootManagerL2SovereignChain is PolygonZkEVMGlobalExitRootL2 {
      */
     event InsertGlobalExitRoot(bytes32 indexed newGlobalExitRoot);
 
-    /**
-     * @notice Only allows a function to be callable if its called by coinbase (trusted sequencer in sovereign chains)
-     */
-    modifier onlyCoinbase() {
-        if (block.coinbase != msg.sender) {
-            revert OnlyCoinbase();
-        }
-        _;
-    }
-
     /**
      * @param _bridgeAddress PolygonZkEVMBridge contract address
      */
@@ -35,7 +25,11 @@ contract GlobalExitRootManagerL2SovereignChain is PolygonZkEVMGlobalExitRootL2 {
      */
     function insertGlobalExitRoot(
         bytes32 _newRoot
-    ) external onlyCoinbase {
+    ) external {
+        // Only allowed to be called by coinbase
+         if (block.coinbase != msg.sender) {
+            revert OnlyCoinbase();
+        }
         // do not update timestamp if already set
         if (globalExitRootMap[_newRoot] == 0) {
             globalExitRootMap[_newRoot] = block.timestamp;

diff --git a/test/contractsv2/BridgeL2SovereignChain.test.ts b/test/contractsv2/BridgeL2SovereignChain.test.ts
@@ -114,9 +114,13 @@ describe("BridgeL2SovereignChain Contract", () => {
         const balanceBridge = await sovereignToken.balanceOf(sovereignChainBridgeContract.target);
         // Remap asset
         // Remap not mintable token
-        await sovereignChainBridgeContract
-            .connect(bridgeManager)
-            .setSovereignTokenAddress(networkIDRollup, tokenAddress, sovereignToken.target, true);
+        await expect(
+            sovereignChainBridgeContract
+                .connect(bridgeManager)
+                .setSovereignTokenAddress(networkIDRollup, tokenAddress, sovereignToken.target, true)
+        )
+            .to.emit(sovereignChainBridgeContract, "SetSovereignTokenAddress")
+            .withArgs(networkIDRollup, tokenAddress, sovereignToken.target, true);
         // pre compute root merkle tree in Js
         const height = 32;
         const merkleTree = new MerkleTreeBridge(height);
@@ -198,7 +202,11 @@ describe("BridgeL2SovereignChain Contract", () => {
         expect(await sovereignChainGlobalExitRoot.globalExitRootMap(computedGlobalExitRoot)).to.not.be.eq(0);
 
         // Remove sovereign token address
-        await sovereignChainBridgeContract.connect(bridgeManager).removeSovereignTokenAddress(sovereignToken.target);
+        await expect(
+            sovereignChainBridgeContract.connect(bridgeManager).removeSovereignTokenAddress(sovereignToken.target)
+        )
+            .to.emit(sovereignChainBridgeContract, "RemoveSovereignTokenAddress")
+            .withArgs(sovereignToken.target);
     });
 
     it("should Sovereign Chain bridge a remapped asset mintable and verify merkle proof", async () => {
@@ -250,9 +258,13 @@ describe("BridgeL2SovereignChain Contract", () => {
                 .connect(bridgeManager)
                 .setSovereignTokenAddress(networkIDRollup2, tokenAddress, sovereignToken.target, false)
         ).to.be.revertedWithCustomError(sovereignChainBridgeContract, "OriginNetworkInvalid");
-        await sovereignChainBridgeContract
-            .connect(bridgeManager)
-            .setSovereignTokenAddress(networkIDRollup, tokenAddress, sovereignToken.target, false);
+        await expect(
+            sovereignChainBridgeContract
+                .connect(bridgeManager)
+                .setSovereignTokenAddress(networkIDRollup, tokenAddress, sovereignToken.target, false)
+        )
+            .to.emit(sovereignChainBridgeContract, "SetSovereignTokenAddress")
+            .withArgs(networkIDRollup, tokenAddress, sovereignToken.target, false);
         // pre compute root merkle tree in Js
         const height = 32;
         const merkleTree = new MerkleTreeBridge(height);