c4: gas optimization fixes

contracts/gateway/GraphTokenGateway.sol

@@ -21,7 +21,7 @@ abstract contract GraphTokenGateway is GraphUpgradeable, Pausable, Managed, ITok
     modifier onlyGovernorOrGuardian() {
         require(
             msg.sender == controller.getGovernor() || msg.sender == pauseGuardian,
-            "Only Governor or Guardian can call"
+            "Only Governor or Guardian"
         );
         _;
     }

contracts/gateway/L1GraphTokenGateway.sol

@@ -143,7 +143,8 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess
      * @param _escrow Address of the BridgeEscrow
      */
     function setEscrowAddress(address _escrow) external onlyGovernor {
-        require(_escrow != address(0) && Address.isContract(_escrow), "INVALID_ESCROW");
+        require(_escrow != address(0), "INVALID_ESCROW");
+        require(Address.isContract(_escrow), "MUST_BE_CONTRACT");
         escrow = _escrow;
         emit EscrowAddressSet(_escrow);
     }
@@ -202,8 +203,8 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess
         bytes calldata _data
     ) external payable override notPaused returns (bytes memory) {
         IGraphToken token = graphToken();
+        require(_amount != 0, "INVALID_ZERO_AMOUNT");
         require(_l1Token == address(token), "TOKEN_NOT_GRT");
-        require(_amount > 0, "INVALID_ZERO_AMOUNT");
         require(_to != address(0), "INVALID_DESTINATION");
 
         // nested scopes to avoid stack too deep errors

contracts/governance/Governed.sol

@@ -51,15 +51,16 @@ contract Governed {
      * This function must called by the pending governor.
      */
     function acceptOwnership() external {
+        address _pendingGovernor = pendingGovernor;
         require(
-            pendingGovernor != address(0) && msg.sender == pendingGovernor,
+            _pendingGovernor != address(0) && msg.sender == _pendingGovernor,
             "Caller must be pending governor"
         );
 
         address oldGovernor = governor;
-        address oldPendingGovernor = pendingGovernor;
+        address oldPendingGovernor = _pendingGovernor;
 
-        governor = pendingGovernor;
+        governor = _pendingGovernor;
         pendingGovernor = address(0);
 
         emit NewOwnership(oldGovernor, governor);

contracts/governance/Managed.sol

@@ -28,6 +28,14 @@ contract Managed {
     mapping(bytes32 => address) private addressCache;
     uint256[10] private __gap;
 
+    // Immutables
+    bytes32 immutable CURATION = keccak256("Curation");
+    bytes32 immutable EPOCH_MANAGER = keccak256("EpochManager");
+    bytes32 immutable REWARDS_MANAGER = keccak256("RewardsManager");
+    bytes32 immutable STAKING = keccak256("Staking");
+    bytes32 immutable GRAPH_TOKEN = keccak256("GraphToken");
+    bytes32 immutable GRAPH_TOKEN_GATEWAY = keccak256("GraphTokenGateway");
+
     // -- Events --
 
     event ParameterUpdated(string param);
@@ -50,7 +58,7 @@ contract Managed {
     }
 
     function _onlyGovernor() internal view {
-        require(msg.sender == controller.getGovernor(), "Caller must be Controller governor");
+        require(msg.sender == controller.getGovernor(), "Only Controller governor");
     }
 
     function _onlyController() internal view {
@@ -111,47 +119,47 @@ contract Managed {
      * @return Curation contract registered with Controller
      */
     function curation() internal view returns (ICuration) {
-        return ICuration(_resolveContract(keccak256("Curation")));
+        return ICuration(_resolveContract(CURATION));
     }
 
     /**
      * @dev Return EpochManager interface.
      * @return Epoch manager contract registered with Controller
      */
     function epochManager() internal view returns (IEpochManager) {
-        return IEpochManager(_resolveContract(keccak256("EpochManager")));
+        return IEpochManager(_resolveContract(EPOCH_MANAGER));
     }
 
     /**
      * @dev Return RewardsManager interface.
      * @return Rewards manager contract registered with Controller
      */
     function rewardsManager() internal view returns (IRewardsManager) {
-        return IRewardsManager(_resolveContract(keccak256("RewardsManager")));
+        return IRewardsManager(_resolveContract(REWARDS_MANAGER));
     }
 
     /**
      * @dev Return Staking interface.
      * @return Staking contract registered with Controller
      */
     function staking() internal view returns (IStaking) {
-        return IStaking(_resolveContract(keccak256("Staking")));
+        return IStaking(_resolveContract(STAKING));
     }
 
     /**
      * @dev Return GraphToken interface.
      * @return Graph token contract registered with Controller
      */
     function graphToken() internal view returns (IGraphToken) {
-        return IGraphToken(_resolveContract(keccak256("GraphToken")));
+        return IGraphToken(_resolveContract(GRAPH_TOKEN));
     }
 
     /**
      * @dev Return GraphTokenGateway (L1 or L2) interface.
      * @return Graph token gateway contract registered with Controller
      */
     function graphTokenGateway() internal view returns (ITokenGateway) {
-        return ITokenGateway(_resolveContract(keccak256("GraphTokenGateway")));
+        return ITokenGateway(_resolveContract(GRAPH_TOKEN_GATEWAY));
     }
 
     /**

contracts/l2/gateway/L2GraphTokenGateway.sol

@@ -143,7 +143,7 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran
         bytes calldata _data
     ) public payable override notPaused nonReentrant returns (bytes memory) {
         require(_l1Token == l1GRT, "TOKEN_NOT_GRT");
-        require(_amount > 0, "INVALID_ZERO_AMOUNT");
+        require(_amount != 0, "INVALID_ZERO_AMOUNT");
         require(msg.value == 0, "INVALID_NONZERO_VALUE");
         require(_to != address(0), "INVALID_DESTINATION");
 

contracts/upgrades/GraphProxy.sol

@@ -102,7 +102,7 @@ contract GraphProxy is GraphProxyStorage {
      * NOTE: Only the admin can call this function.
      */
     function setAdmin(address _newAdmin) external ifAdmin {
-        require(_newAdmin != address(0), "Cannot change the admin of a proxy to the zero address");
+        require(_newAdmin != address(0), "Admin cant be the zero address");
         _setAdmin(_newAdmin);
     }
 
@@ -138,11 +138,9 @@ contract GraphProxy is GraphProxyStorage {
      */
     function _acceptUpgrade() internal {
         address _pendingImplementation = _pendingImplementation();
-        require(Address.isContract(_pendingImplementation), "Implementation must be a contract");
-        require(
-            _pendingImplementation != address(0) && msg.sender == _pendingImplementation,
-            "Caller must be the pending implementation"
-        );
+        require(Address.isContract(_pendingImplementation), "Impl must be a contract");
+        require(_pendingImplementation != address(0), "Impl cannot be zero address");
+        require(msg.sender == _pendingImplementation, "Only pending implementation");
 
         _setImplementation(_pendingImplementation);
         _setPendingImplementation(address(0));

contracts/upgrades/GraphUpgradeable.sol

@@ -29,7 +29,7 @@ contract GraphUpgradeable {
      * @dev Check if the caller is the implementation.
      */
     modifier onlyImpl() {
-        require(msg.sender == _implementation(), "Caller must be the implementation");
+        require(msg.sender == _implementation(), "Only implementation");
         _;
     }
 

e2e/deployment/config/l1/l1GraphTokenGateway.test.ts

@@ -34,38 +34,38 @@ describe('[L1] L1GraphTokenGateway configuration', function () {
         unauthorized.address,
         unauthorized.address,
       )
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
 
     it('setL2TokenAddress should revert', async function () {
       const tx = L1GraphTokenGateway.connect(unauthorized).setL2TokenAddress(unauthorized.address)
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
 
     it('setL2CounterpartAddress should revert', async function () {
       const tx = L1GraphTokenGateway.connect(unauthorized).setL2CounterpartAddress(
         unauthorized.address,
       )
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
 
     it('setEscrowAddress should revert', async function () {
       const tx = L1GraphTokenGateway.connect(unauthorized).setEscrowAddress(unauthorized.address)
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
 
     it('addToCallhookWhitelist should revert', async function () {
       const tx = L1GraphTokenGateway.connect(unauthorized).addToCallhookWhitelist(
         unauthorized.address,
       )
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
 
     it('removeFromCallhookWhitelist should revert', async function () {
       const tx = L1GraphTokenGateway.connect(unauthorized).removeFromCallhookWhitelist(
         unauthorized.address,
       )
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
 
     it('finalizeInboundTransfer should revert', async function () {

e2e/deployment/config/l2/l2GraphTokenGateway.test.ts

@@ -31,19 +31,19 @@ describe('[L2] L2GraphTokenGateway configuration', function () {
 
     it('setL2Router should revert', async function () {
       const tx = L2GraphTokenGateway.connect(unauthorized).setL2Router(unauthorized.address)
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
 
     it('setL1TokenAddress should revert', async function () {
       const tx = L2GraphTokenGateway.connect(unauthorized).setL1TokenAddress(unauthorized.address)
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
 
     it('setL1CounterpartAddress should revert', async function () {
       const tx = L2GraphTokenGateway.connect(unauthorized).setL1CounterpartAddress(
         unauthorized.address,
       )
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
 
     it('finalizeInboundTransfer should revert', async function () {

test/curation/configuration.test.ts

@@ -55,7 +55,7 @@ describe('Curation:Config', () => {
 
     it('reject set `defaultReserveRatio` if not allowed', async function () {
       const tx = curation.connect(me.signer).setDefaultReserveRatio(defaults.curation.reserveRatio)
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
   })
 
@@ -79,7 +79,7 @@ describe('Curation:Config', () => {
       const tx = curation
         .connect(me.signer)
         .setMinimumCurationDeposit(defaults.curation.minimumCurationDeposit)
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
   })
 
@@ -99,7 +99,7 @@ describe('Curation:Config', () => {
 
     it('reject set `curationTaxPercentage` if not allowed', async function () {
       const tx = curation.connect(me.signer).setCurationTaxPercentage(0)
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
   })
 
@@ -124,7 +124,7 @@ describe('Curation:Config', () => {
     it('reject set `curationTokenMaster` if not allowed', async function () {
       const newCurationTokenMaster = curation.address
       const tx = curation.connect(me.signer).setCurationTokenMaster(newCurationTokenMaster)
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
   })
 })

test/disputes/configuration.test.ts

@@ -49,7 +49,7 @@ describe('DisputeManager:Config', () => {
 
       it('reject set `arbitrator` if not allowed', async function () {
         const tx = disputeManager.connect(me.signer).setArbitrator(arbitrator.address)
-        await expect(tx).revertedWith('Caller must be Controller governor')
+        await expect(tx).revertedWith('Only Controller governor')
       })
 
       it('reject set `arbitrator` to address zero', async function () {
@@ -74,7 +74,7 @@ describe('DisputeManager:Config', () => {
       it('reject set `minimumDeposit` if not allowed', async function () {
         const newValue = toBN('1')
         const tx = disputeManager.connect(me.signer).setMinimumDeposit(newValue)
-        await expect(tx).revertedWith('Caller must be Controller governor')
+        await expect(tx).revertedWith('Only Controller governor')
       })
     })
 
@@ -97,7 +97,7 @@ describe('DisputeManager:Config', () => {
 
       it('reject set `fishermanRewardPercentage` if not allowed', async function () {
         const tx = disputeManager.connect(me.signer).setFishermanRewardPercentage(50)
-        await expect(tx).revertedWith('Caller must be Controller governor')
+        await expect(tx).revertedWith('Only Controller governor')
       })
     })
 
@@ -127,7 +127,7 @@ describe('DisputeManager:Config', () => {
 
       it('reject set `slashingPercentage` if not allowed', async function () {
         const tx = disputeManager.connect(me.signer).setSlashingPercentage(50, 50)
-        await expect(tx).revertedWith('Caller must be Controller governor')
+        await expect(tx).revertedWith('Only Controller governor')
       })
     })
   })

test/gateway/bridgeEscrow.test.ts

@@ -41,7 +41,7 @@ describe('BridgeEscrow', () => {
   describe('approveAll', function () {
     it('cannot be called by someone other than the governor', async function () {
       const tx = bridgeEscrow.connect(tokenReceiver.signer).approveAll(spender.address)
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
     it('allows a spender to transfer GRT held by the contract', async function () {
       expect(await grt.allowance(bridgeEscrow.address, spender.address)).eq(0)
@@ -62,7 +62,7 @@ describe('BridgeEscrow', () => {
   describe('revokeAll', function () {
     it('cannot be called by someone other than the governor', async function () {
       const tx = bridgeEscrow.connect(tokenReceiver.signer).revokeAll(spender.address)
-      await expect(tx).revertedWith('Caller must be Controller governor')
+      await expect(tx).revertedWith('Only Controller governor')
     })
     it("revokes a spender's permission to transfer GRT held by the contract", async function () {
       await bridgeEscrow.connect(governor.signer).approveAll(spender.address)