First round of revert string removals (#139)

* First round of revert string removements

* add revert errors in forkable bridge

contracts/ForkableBridge.sol

@@ -56,8 +56,12 @@ contract ForkableBridge is
         uint256 amount,
         address to
     ) external onlyAfterForking {
-        require(_hardAssetManager == msg.sender, "Not authorized");
-        require(to == children[0] || to == children[1], "Invalid to");
+        if (_hardAssetManager != msg.sender) {
+            revert NotAuthorized();
+        }
+        if (to != children[0] && to != children[1]) {
+            revert InvalidDestinationForHardAsset();
+        }
         IERC20(token).transfer(to, amount);
     }
 

contracts/interfaces/IForkableBridge.sol

@@ -7,6 +7,11 @@ import {IBasePolygonZkEVMGlobalExitRoot} from "@RealityETH/zkevm-contracts/contr
 import {IForkableStructure} from "./IForkableStructure.sol";
 
 interface IForkableBridge is IForkableStructure, IPolygonZkEVMBridge {
+    /// @dev Error thrown when activity is started by a non-authorized actor
+    error NotAuthorized();
+    /// @dev Error thrown when trying to send bridged tokens to a child contract
+    error InvalidDestinationForHardAsset();
+
     /**
      * @dev Function to initialize the contract
      * @param _forkmanager: address of the forkmanager contract

contracts/interfaces/IForkableStructure.sol

@@ -3,9 +3,21 @@
 pragma solidity ^0.8.20;
 
 interface IForkableStructure {
+    /// @dev Error thrown when trying to call a function that can only be called after forking
+    error NoChangesAfterForking();
+    /// @dev Error thrown when trying to call a function that can only be called before forking
+    error OnlyAfterForking();
+    /// @dev Error thrown when trying to call a function that can only be called by the parent contract
+    error OnlyParentIsAllowed();
+    /// @dev Error thrown when trying to call a function that can only be called by the forkmanager
+    error OnlyForkManagerIsAllowed();
+
+    /// @dev Returns the address of the forkmanager
     function forkmanager() external view returns (address);
 
+    /// @dev Returns the address of the parent contract
     function parentContract() external view returns (address);
 
+    /// @dev Returns the addresses of the two children
     function getChildren() external view returns (address, address);
 }

contracts/mixin/ForkableStructure.sol

@@ -18,28 +18,33 @@ contract ForkableStructure is IForkableStructure, Initializable {
     mapping(uint256 => address) public children;
 
     modifier onlyBeforeForking() {
-        require(children[0] == address(0x0), "No changes after forking");
-        _;
-    }
-
-    modifier onlyBeforeCreatingChild2() {
-        require(children[2] == address(0x0), "No changes after forking");
+        if (children[0] != address(0x0)) {
+            revert NoChangesAfterForking();
+        }
         _;
     }
 
     modifier onlyAfterForking() {
-        require(children[0] != address(0x0), "onlyAfterForking");
-        require(children[1] != address(0x0), "onlyAfterForking");
+        if (children[0] == address(0x0)) {
+            revert OnlyAfterForking();
+        }
+        if (children[1] == address(0x0)) {
+            revert OnlyAfterForking();
+        }
         _;
     }
 
     modifier onlyParent() {
-        require(msg.sender == parentContract, "Not parent");
+        if (msg.sender != parentContract) {
+            revert OnlyParentIsAllowed();
+        }
         _;
     }
 
     modifier onlyForkManger() {
-        require(msg.sender == forkmanager, "Not forkManager");
+        if (msg.sender != forkmanager) {
+            revert OnlyForkManagerIsAllowed();
+        }
         _;
     }
 

test/ForkableBridge.t.sol

@@ -4,6 +4,8 @@ import {Test} from "forge-std/Test.sol";
 import {ForkableBridge} from "../contracts/ForkableBridge.sol";
 import {ForkableBridgeWrapper} from "./testcontract/ForkableBridgeWrapper.sol";
 import {ForkonomicToken} from "../contracts/ForkonomicToken.sol";
+import {IForkableBridge} from "../contracts/interfaces/IForkableBridge.sol";
+import {IForkableStructure} from "../contracts/interfaces/IForkableStructure.sol";
 import {IForkonomicToken} from "../contracts/interfaces/IForkonomicToken.sol";
 import {ForkableGlobalExitRoot} from "../contracts/ForkableGlobalExitRoot.sol";
 import {IERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/interfaces/IERC20Upgradeable.sol";
@@ -69,7 +71,7 @@ contract ForkableBridgeTest is Test {
     }
 
     function testCreateChildren() public {
-        vm.expectRevert(bytes("Not forkManager"));
+        vm.expectRevert(IForkableStructure.OnlyForkManagerIsAllowed.selector);
         forkableBridge.createChildren();
         ForkableGlobalExitRoot exitRoot = new ForkableGlobalExitRoot();
         vm.mockCall(
@@ -96,7 +98,7 @@ contract ForkableBridgeTest is Test {
             abi.encodePacked(originNetwork, token)
         );
 
-        vm.expectRevert(bytes("Not parent"));
+        vm.expectRevert(IForkableStructure.OnlyParentIsAllowed.selector);
         forkableBridge.mintForkableToken(
             address(token),
             originNetwork,
@@ -173,7 +175,7 @@ contract ForkableBridgeTest is Test {
             amount
         );
 
-        vm.expectRevert(bytes("Not parent"));
+        vm.expectRevert(IForkableStructure.OnlyParentIsAllowed.selector);
         forkableBridge.burnForkableTokens(
             destinationAddress,
             originTokenAddress,
@@ -213,7 +215,8 @@ contract ForkableBridgeTest is Test {
         );
 
         // Testing revert if children are not yet created
-        vm.expectRevert(bytes("onlyAfterForking"));
+        vm.expectRevert(IForkableStructure.OnlyAfterForking.selector);
+
         forkableBridge.splitTokenIntoChildToken(address(token), amount, true);
 
         vm.prank(forkableBridge.forkmanager());
@@ -345,7 +348,8 @@ contract ForkableBridgeTest is Test {
         forkableBridge.splitTokenIntoChildToken(forkableToken, amount, true);
 
         // Only parent can merge
-        vm.expectRevert(bytes("onlyAfterForking"));
+        vm.expectRevert(IForkableStructure.OnlyAfterForking.selector);
+
         ForkableBridge(child1).mergeChildTokens(forkableToken, amount + 1);
 
         // Merge the token
@@ -387,7 +391,7 @@ contract ForkableBridgeTest is Test {
         vm.prank(forkableBridge.forkmanager());
         forkableBridge.createChildren();
 
-        vm.expectRevert(bytes("No changes after forking"));
+        vm.expectRevert(IForkableStructure.NoChangesAfterForking.selector);
         forkableBridge.bridgeAsset(
             12,
             address(0x3),
@@ -397,11 +401,11 @@ contract ForkableBridgeTest is Test {
             bytes("0x3453")
         );
 
-        vm.expectRevert(bytes("No changes after forking"));
+        vm.expectRevert(IForkableStructure.NoChangesAfterForking.selector);
         forkableBridge.bridgeMessage(2, address(0x45), false, bytes("0x3453"));
 
         bytes32[32] memory smtProof;
-        vm.expectRevert(bytes("No changes after forking"));
+        vm.expectRevert(IForkableStructure.NoChangesAfterForking.selector);
         forkableBridge.claimMessage(
             smtProof,
             2,
@@ -415,7 +419,7 @@ contract ForkableBridgeTest is Test {
             bytes("0x")
         );
 
-        vm.expectRevert(bytes("No changes after forking"));
+        vm.expectRevert(IForkableStructure.NoChangesAfterForking.selector);
         forkableBridge.claimAsset(
             smtProof,
             32,
@@ -447,7 +451,7 @@ contract ForkableBridgeTest is Test {
             false,
             ""
         );
-        vm.expectRevert("onlyAfterForking");
+        vm.expectRevert(IForkableStructure.OnlyAfterForking.selector);
         vm.prank(hardAssetManger);
         forkableBridge.transferHardAssetsToChild(
             address(erc20Token),
@@ -493,14 +497,16 @@ contract ForkableBridgeTest is Test {
             depositTreeHashes
         );
 
-        vm.expectRevert("Not authorized");
+        vm.expectRevert(IForkableBridge.NotAuthorized.selector);
         forkableBridge.transferHardAssetsToChild(
             address(erc20Token),
             amount,
             to
         );
 
-        vm.expectRevert("Invalid to");
+        vm.expectRevert(
+            IForkableBridge.InvalidDestinationForHardAsset.selector
+        );
         vm.prank(hardAssetManger);
         forkableBridge.transferHardAssetsToChild(
             address(erc20Token),
@@ -561,10 +567,12 @@ contract ForkableBridgeTest is Test {
         erc20GasToken.mint(address(forkableBridge2), mintAmount);
 
         // Now, call the function as the forkmanager
-        vm.expectRevert(bytes("onlyAfterForking"));
+        vm.expectRevert(IForkableStructure.OnlyAfterForking.selector);
+
         vm.prank(forkmanager);
         forkableBridge2.sendForkonomicTokensToChild(10, true, false);
-        vm.expectRevert(bytes("onlyAfterForking"));
+        vm.expectRevert(IForkableStructure.OnlyAfterForking.selector);
+
         vm.prank(forkmanager);
         forkableBridge2.sendForkonomicTokensToChild(10, true, false);
 

test/ForkableGlobalExitRoot.t.sol

@@ -1,6 +1,7 @@
 pragma solidity ^0.8.20;
 
 import {Test} from "forge-std/Test.sol";
+import {IForkableStructure} from "../contracts/interfaces/IForkableStructure.sol";
 import {ForkableGlobalExitRoot} from "../contracts/ForkableGlobalExitRoot.sol";
 import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
 import {Util} from "./utils/Util.sol";
@@ -69,7 +70,7 @@ contract ForkableGlobalExitRootTest is Test {
     }
 
     function testCreateChildrenOnlyByForkManager() public {
-        vm.expectRevert("Not forkManager");
+        vm.expectRevert(IForkableStructure.OnlyForkManagerIsAllowed.selector);
         forkableGlobalExitRoot.createChildren();
         vm.prank(forkableGlobalExitRoot.forkmanager());
         forkableGlobalExitRoot.createChildren();

test/ForkableZkEVM.t.sol

@@ -3,6 +3,7 @@ pragma solidity ^0.8.20;
 import {Test} from "forge-std/Test.sol";
 import {PolygonZkEVM} from "@RealityETH/zkevm-contracts/contracts/inheritedMainContracts/PolygonZkEVM.sol";
 import {ForkableZkEVM} from "../contracts/ForkableZkEVM.sol";
+import {IForkableStructure} from "../contracts/interfaces/IForkableStructure.sol";
 import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
 import {IPolygonZkEVMGlobalExitRoot} from "@RealityETH/zkevm-contracts/contracts/interfaces/IPolygonZkEVMGlobalExitRoot.sol";
 import {IVerifierRollup} from "@RealityETH/zkevm-contracts/contracts/interfaces/IVerifierRollup.sol";
@@ -93,7 +94,7 @@ contract ForkableZkEVMTest is Test {
     }
 
     function testCreateChildren() public {
-        vm.expectRevert("Not forkManager");
+        vm.expectRevert(IForkableStructure.OnlyForkManagerIsAllowed.selector);
         forkableZkEVM.createChildren();
 
         vm.prank(forkableZkEVM.forkmanager());
@@ -130,7 +131,7 @@ contract ForkableZkEVMTest is Test {
         vm.prank(forkableZkEVM.forkmanager());
         forkableZkEVM.createChildren();
 
-        vm.expectRevert("No changes after forking");
+        vm.expectRevert(IForkableStructure.NoChangesAfterForking.selector);
         forkableZkEVM.verifyBatches(
             pendingStateNum,
             initNumBatch,
@@ -145,15 +146,15 @@ contract ForkableZkEVMTest is Test {
         vm.prank(forkableZkEVM.forkmanager());
         forkableZkEVM.createChildren();
 
-        vm.expectRevert("No changes after forking");
+        vm.expectRevert(IForkableStructure.NoChangesAfterForking.selector);
         forkableZkEVM.consolidatePendingState(10);
 
         bytes32[24] memory proof;
         for (uint i = 0; i < 24; i++) {
             proof[i] = bytes32("0x"); // Whatever initialization value you want
         }
 
-        vm.expectRevert("No changes after forking");
+        vm.expectRevert(IForkableStructure.NoChangesAfterForking.selector);
         forkableZkEVM.overridePendingState(
             10,
             10,
@@ -164,7 +165,7 @@ contract ForkableZkEVMTest is Test {
             proof
         );
 
-        vm.expectRevert("No changes after forking");
+        vm.expectRevert(IForkableStructure.NoChangesAfterForking.selector);
         PolygonZkEVM.BatchData[] memory batches = new PolygonZkEVM.BatchData[](
             1
         );

test/ForkingManager.t.sol

@@ -9,6 +9,7 @@ import {ForkableBridge} from "../contracts/ForkableBridge.sol";
 import {ForkableZkEVM} from "../contracts/ForkableZkEVM.sol";
 import {ForkonomicToken} from "../contracts/ForkonomicToken.sol";
 import {ForkableGlobalExitRoot} from "../contracts/ForkableGlobalExitRoot.sol";
+import {IForkableStructure} from "../contracts/interfaces/IForkableStructure.sol";
 import {IPolygonZkEVMGlobalExitRoot} from "@RealityETH/zkevm-contracts/contracts/interfaces/IPolygonZkEVMGlobalExitRoot.sol";
 import {IForkingManager} from "../contracts/interfaces/IForkingManager.sol";
 import {IVerifierRollup} from "@RealityETH/zkevm-contracts/contracts/interfaces/IVerifierRollup.sol";
@@ -641,7 +642,7 @@ contract ForkingManagerTest is Test {
         forkmanager.initiateFork(disputeData);
         skip(forkmanager.forkPreparationTime() + 1);
         forkmanager.executeFork();
-        vm.expectRevert("No changes after forking");
+        vm.expectRevert(IForkableStructure.NoChangesAfterForking.selector);
         forkmanager.executeFork();
     }
 

test/L1GlobalChainInfoPublisher.t.sol

@@ -19,6 +19,7 @@ import {ForkableRealityETH_ERC20} from "../contracts/ForkableRealityETH_ERC20.so
 import {RealityETH_v3_0} from "../contracts/lib/reality-eth/RealityETH-3.0.sol";
 import {AdjudicationFramework} from "../contracts/AdjudicationFramework.sol";
 
+import {IForkableStructure} from "../contracts/interfaces/IForkableStructure.sol";
 import {L2ForkArbitrator} from "../contracts/L2ForkArbitrator.sol";
 import {L1GlobalChainInfoPublisher} from "../contracts/L1GlobalChainInfoPublisher.sol";
 import {L1GlobalForkRequester} from "../contracts/L1GlobalForkRequester.sol";
@@ -290,7 +291,7 @@ contract L1GlobalChainInfoPublisherTest is Test {
         forkmanager.executeFork();
 
         // The current bridge should no longer work
-        vm.expectRevert("No changes after forking");
+        vm.expectRevert(IForkableStructure.NoChangesAfterForking.selector);
         l1GlobalChainInfoPublisher.updateL2ChainInfo(
             address(bridge),
             address(l2ChainInfo),
@@ -352,7 +353,7 @@ contract L1GlobalChainInfoPublisherTest is Test {
         skip(forkmanager.forkPreparationTime() + 1);
         forkmanager2.executeFork();
 
-        vm.expectRevert("No changes after forking");
+        vm.expectRevert(IForkableStructure.NoChangesAfterForking.selector);
         l1GlobalChainInfoPublisher.updateL2ChainInfo(
             bridge2,
             address(l2ChainInfo),