c4: gas optimization fixes

contracts/gateway/BridgeEscrow.sol

@@ -36,7 +36,6 @@ contract BridgeEscrow is Initializable, GraphUpgradeable, Managed {
      * @param _spender Address of the spender that will be revoked
      */
     function revokeAll(address _spender) external onlyGovernor {
-        IGraphToken grt = graphToken();
-        grt.decreaseAllowance(_spender, grt.allowance(address(this), _spender));
+        graphToken().approve(_spender, 0);
     }
 }

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
@@ -284,7 +285,7 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess
      * @return Base ether value required to keep retryable ticket alive
      * @return Additional data sent to L2
      */
-    function parseOutboundData(bytes memory _data)
+    function parseOutboundData(bytes calldata _data)
         private
         view
         returns (

contracts/governance/Governed.sol

@@ -51,15 +51,16 @@ contract Governed {
      * This function must called by the pending governor.
      */
     function acceptOwnership() external {
+        address oldPendingGovernor = pendingGovernor;
+
         require(
-            pendingGovernor != address(0) && msg.sender == pendingGovernor,
+            oldPendingGovernor != address(0) && msg.sender == oldPendingGovernor,
             "Caller must be pending governor"
         );
 
         address oldGovernor = governor;
-        address oldPendingGovernor = pendingGovernor;
 
-        governor = pendingGovernor;
+        governor = oldPendingGovernor;
         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 private immutable CURATION = keccak256("Curation");
+    bytes32 private immutable EPOCH_MANAGER = keccak256("EpochManager");
+    bytes32 private immutable REWARDS_MANAGER = keccak256("RewardsManager");
+    bytes32 private immutable STAKING = keccak256("Staking");
+    bytes32 private immutable GRAPH_TOKEN = keccak256("GraphToken");
+    bytes32 private 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");
 
@@ -238,8 +238,7 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran
         if (_data.length > 0) {
             bytes memory callhookData;
             {
-                bytes memory gatewayData;
-                (gatewayData, callhookData) = abi.decode(_data, (bytes, bytes));
+                (, callhookData) = abi.decode(_data, (bytes, bytes));
             }
             ICallhookReceiver(_to).onTokenTransfer(_from, _amount, callhookData);
         }
@@ -283,7 +282,7 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran
      * @return Sender of the tx
      * @return Any other data sent to L1
      */
-    function parseOutboundData(bytes memory _data) private view returns (address, bytes memory) {
+    function parseOutboundData(bytes calldata _data) private view returns (address, bytes memory) {
         address from;
         bytes memory extraData;
         if (msg.sender == l2Router) {

contracts/l2/token/GraphTokenUpgradeable.sol

@@ -4,7 +4,6 @@ pragma solidity ^0.7.6;
 
 import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20BurnableUpgradeable.sol";
 import "@openzeppelin/contracts/cryptography/ECDSA.sol";
-import "@openzeppelin/contracts/math/SafeMath.sol";
 
 import "../../upgrades/GraphUpgradeable.sol";
 import "../../governance/Governed.sol";
@@ -26,20 +25,18 @@ import "../../governance/Governed.sol";
  * the original's constructor + non-upgradeable approach.
  */
 contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgradeable {
-    using SafeMath for uint256;
-
     // -- EIP712 --
     // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-712.md#definition-of-domainseparator
 
-    bytes32 private constant DOMAIN_TYPE_HASH =
+    bytes32 private immutable DOMAIN_TYPE_HASH =
         keccak256(
             "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract,bytes32 salt)"
         );
-    bytes32 private constant DOMAIN_NAME_HASH = keccak256("Graph Token");
-    bytes32 private constant DOMAIN_VERSION_HASH = keccak256("0");
-    bytes32 private constant DOMAIN_SALT =
+    bytes32 private immutable DOMAIN_NAME_HASH = keccak256("Graph Token");
+    bytes32 private immutable DOMAIN_VERSION_HASH = keccak256("0");
+    bytes32 private immutable DOMAIN_SALT =
         0xe33842a7acd1d5a1d28f25a931703e5605152dc48d64dc4716efdae1f5659591; // Randomly generated salt
-    bytes32 private constant PERMIT_TYPEHASH =
+    bytes32 private immutable PERMIT_TYPEHASH =
         keccak256(
             "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"
         );
@@ -96,7 +93,7 @@ contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgra
         require(_owner == recoveredAddress, "GRT: invalid permit");
         require(_deadline == 0 || block.timestamp <= _deadline, "GRT: expired permit");
 
-        nonces[_owner] = nonces[_owner].add(1);
+        nonces[_owner] = nonces[_owner] + 1;
         _approve(_owner, _spender, _value);
     }
 
@@ -114,15 +111,15 @@ contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgra
      * @param _account Address of the minter
      */
     function removeMinter(address _account) external onlyGovernor {
-        require(_minters[_account], "NOT_A_MINTER");
+        require(isMinter(_account), "NOT_A_MINTER");
         _removeMinter(_account);
     }
 
     /**
      * @dev Renounce to be a minter.
      */
     function renounceMinter() external {
-        require(_minters[msg.sender], "NOT_A_MINTER");
+        require(isMinter(msg.sender), "NOT_A_MINTER");
         _removeMinter(msg.sender);
     }
 

contracts/l2/token/L2GraphToken.sol

@@ -59,7 +59,7 @@ contract L2GraphToken is GraphTokenUpgradeable, IArbToken {
     function setGateway(address _gw) external onlyGovernor {
         require(_gw != address(0), "INVALID_GATEWAY");
         gateway = _gw;
-        emit GatewaySet(gateway);
+        emit GatewaySet(_gw);
     }
 
     /**

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

@@ -26,46 +26,46 @@ describe('[L1] L1GraphTokenGateway configuration', function () {
   describe('calls with unauthorized user', () => {
     it('initialize should revert', async function () {
       const tx = L1GraphTokenGateway.connect(unauthorized).initialize(unauthorized.address)
-      await expect(tx).revertedWith('Caller must be the implementation')
+      await expect(tx).revertedWith('Only implementation')
     })
 
     it('setArbitrumAddresses should revert', async function () {
       const tx = L1GraphTokenGateway.connect(unauthorized).setArbitrumAddresses(
         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

@@ -26,24 +26,24 @@ describe('[L2] L2GraphTokenGateway configuration', function () {
   describe('calls with unauthorized user', () => {
     it('initialize should revert', async function () {
       const tx = L2GraphTokenGateway.connect(unauthorized).initialize(unauthorized.address)
-      await expect(tx).revertedWith('Caller must be the implementation')
+      await expect(tx).revertedWith('Only implementation')
     })
 
     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')
     })
   })
 })