Add renouceAdmin function (#592)

* Add renounceAdmin function to FeeHandlers

* Add tests for renounceAdmin functions

* Fix indentation

Author: nmlinaric

contracts/handlers/fee/BasicFeeHandler.sol

@@ -36,6 +36,18 @@ contract BasicFeeHandler is IFeeHandler, AccessControl {
         _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
     }
 
+    /**
+        @notice Removes admin role from {_msgSender()} and grants it to {newAdmin}.
+        @notice Only callable by an address that currently has the admin role.
+        @param newAdmin Address that admin role will be granted to.
+     */
+    function renounceAdmin(address newAdmin) external onlyAdmin {
+        address sender = _msgSender();
+        require(sender != newAdmin, 'Cannot renounce oneself');
+        grantRole(DEFAULT_ADMIN_ROLE, newAdmin);
+        renounceRole(DEFAULT_ADMIN_ROLE, sender);
+    }
+
     /**
         @notice Collects fee for deposit.
         @param sender Sender of the deposit.

contracts/handlers/fee/FeeHandlerWithOracle.sol

@@ -25,7 +25,7 @@ contract FeeHandlerWithOracle is IFeeHandler, AccessControl, ERC20Safe {
 
     struct OracleMessageType {
         // Base Effective Rate - effective rate between base currencies of source and dest networks (eg. MATIC/ETH)
-        uint256 ber; 
+        uint256 ber;
         // Token Effective Rate - rate between base currency of destination network and token that is being trasferred (eg. MATIC/USDT)
         uint256 ter;
         uint256 dstGasPrice;
@@ -51,6 +51,18 @@ contract FeeHandlerWithOracle is IFeeHandler, AccessControl, ERC20Safe {
 
     // Admin functions
 
+    /**
+        @notice Removes admin role from {_msgSender()} and grants it to {newAdmin}.
+        @notice Only callable by an address that currently has the admin role.
+        @param newAdmin Address that admin role will be granted to.
+     */
+    function renounceAdmin(address newAdmin) external onlyAdmin {
+        address sender = _msgSender();
+        require(sender != newAdmin, 'Cannot renounce oneself');
+        grantRole(DEFAULT_ADMIN_ROLE, newAdmin);
+        renounceRole(DEFAULT_ADMIN_ROLE, sender);
+    }
+
     /**
         @notice Sets the fee oracle address for signature verification.
         @param oracleAddress Fee oracle address.
@@ -101,7 +113,7 @@ contract FeeHandlerWithOracle is IFeeHandler, AccessControl, ERC20Safe {
     }
 
     function _calculateFee(address sender, uint8 fromDomainID, uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) internal view returns(uint256 fee, address tokenAddress) {
-        /** 
+        /**
             Message:
             ber * 10^18:  uint256
             ter * 10^18:  uint256
@@ -120,7 +132,7 @@ contract FeeHandlerWithOracle is IFeeHandler, AccessControl, ERC20Safe {
 
             amount: uint256
             total: 321
-        */  
+        */
 
         require(feeData.length == 321, "Incorrect feeData length");
 
@@ -132,9 +144,9 @@ contract FeeHandlerWithOracle is IFeeHandler, AccessControl, ERC20Safe {
 
         OracleMessageType memory oracleMessage = abi.decode(feeDataDecoded.message, (OracleMessageType));
         require(block.timestamp <= oracleMessage.expiresAt, "Obsolete oracle data");
-        require((oracleMessage.fromDomainID == fromDomainID) 
-            && (oracleMessage.toDomainID == destinationDomainID) 
-            && (oracleMessage.resourceID == resourceID), 
+        require((oracleMessage.fromDomainID == fromDomainID)
+            && (oracleMessage.toDomainID == destinationDomainID)
+            && (oracleMessage.resourceID == resourceID),
             "Incorrect deposit params"
         );
 
@@ -144,12 +156,12 @@ contract FeeHandlerWithOracle is IFeeHandler, AccessControl, ERC20Safe {
 
         address tokenHandler = IBridge(_bridgeAddress)._resourceIDToHandlerAddress(resourceID);
         address tokenAddress = IERCHandler(tokenHandler)._resourceIDToTokenContractAddress(resourceID);
-        
+
         // txCost = dstGasPrice * _gasUsed * Token Effective Rate (rate of dest base currency to token)
         uint256 txCost = oracleMessage.dstGasPrice * _gasUsed * oracleMessage.ter / 1e18;
 
         fee = feeDataDecoded.amount * _feePercent / 1e4; // 100 for percent and 100 to avoid precision loss
-        
+
         if (fee < txCost) {
             fee = txCost;
         }

test/handlers/fee/basic/admin.js

@@ -0,0 +1,56 @@
+/**
+ * Copyright 2022 ChainSafe Systems
+ * SPDX-License-Identifier: LGPL-3.0-only
+ */
+
+ const TruffleAssert = require("truffle-assertions");
+
+ const Helpers = require("../../../helpers");
+
+ const BasicFeeHandlerContract = artifacts.require("BasicFeeHandler");
+
+ contract("BasicFeeHandler - [admin]", async accounts => {
+    const domainID = 1;
+    const initialRelayers = accounts.slice(0, 3);
+    const currentFeeHandlerAdmin = accounts[0];
+
+    const assertOnlyAdmin = (method, ...params) => {
+        return TruffleAssert.reverts(method(...params, {from: initialRelayers[1]}), "sender doesn't have admin role");
+    };
+
+    let BridgeInstance;
+    let BasicFeeHandlerInstance;
+    let ADMIN_ROLE;
+
+    beforeEach(async () => {
+        BridgeInstance = awaitBridgeInstance = await Helpers.deployBridge(domainID, accounts[0]);
+        BasicFeeHandlerInstance = await BasicFeeHandlerContract.new(BridgeInstance.address);
+
+        ADMIN_ROLE = await BasicFeeHandlerInstance.DEFAULT_ADMIN_ROLE();
+    });
+
+    it("should set fee property", async () => {
+        const fee = 3;
+        assert.equal(await BasicFeeHandlerInstance._fee.call(), "0");
+        await BasicFeeHandlerInstance.changeFee(fee);
+        assert.equal(await BasicFeeHandlerInstance._fee.call(), fee);
+    });
+
+    it("should require admin role to change fee property", async () => {
+        const fee = 3;
+        await assertOnlyAdmin(BasicFeeHandlerInstance.changeFee, fee);
+    });
+
+    it('FeeHandlerWithOracle admin should be changed to expectedFeeHandlerWithOracleAdmin', async () => {
+        const expectedFeeHandlerWithOracleAdmin = accounts[1];
+
+        // check current admin
+        assert.isTrue(await BasicFeeHandlerInstance.hasRole(ADMIN_ROLE, currentFeeHandlerAdmin));
+
+        await TruffleAssert.passes(BasicFeeHandlerInstance.renounceAdmin(expectedFeeHandlerWithOracleAdmin))
+        assert.isTrue(await BasicFeeHandlerInstance.hasRole(ADMIN_ROLE, expectedFeeHandlerWithOracleAdmin));
+
+        // check that former admin is no longer admin
+        assert.isFalse(await BasicFeeHandlerInstance.hasRole(ADMIN_ROLE, currentFeeHandlerAdmin));
+    });
+});

test/handlers/fee/withOracle/admin.js

@@ -12,17 +12,21 @@
  contract("FeeHandlerWithOracle - [admin]", async accounts => {
     const domainID = 1;
     const initialRelayers = accounts.slice(0, 3);
+    const currentFeeHandlerAdmin = accounts[0];
 
     const assertOnlyAdmin = (method, ...params) => {
         return TruffleAssert.reverts(method(...params, {from: initialRelayers[1]}), "sender doesn't have admin role");
     };
 
     let BridgeInstance;
     let FeeHandlerWithOracleInstance;
+    let ADMIN_ROLE;
 
     beforeEach(async () => {
         BridgeInstance = awaitBridgeInstance = await Helpers.deployBridge(domainID, accounts[0]);
         FeeHandlerWithOracleInstance = await FeeHandlerWithOracleContract.new(BridgeInstance.address);
+
+        ADMIN_ROLE = await FeeHandlerWithOracleInstance.DEFAULT_ADMIN_ROLE();
     });
 
     it("should set fee oracle", async () => {
@@ -53,4 +57,17 @@
         const feePercent = 5;
         await assertOnlyAdmin(FeeHandlerWithOracleInstance.setFeeProperties, gasUsed, feePercent);
     });
+
+    it('FeeHandlerWithOracle admin should be changed to expectedFeeHandlerWithOracleAdmin', async () => {
+        const expectedFeeHandlerWithOracleAdmin = accounts[1];
+
+        // check current admin
+        assert.isTrue(await FeeHandlerWithOracleInstance.hasRole(ADMIN_ROLE, currentFeeHandlerAdmin));
+
+        await TruffleAssert.passes(FeeHandlerWithOracleInstance.renounceAdmin(expectedFeeHandlerWithOracleAdmin))
+        assert.isTrue(await FeeHandlerWithOracleInstance.hasRole(ADMIN_ROLE, expectedFeeHandlerWithOracleAdmin));
+
+        // check that former admin is no longer admin
+        assert.isFalse(await FeeHandlerWithOracleInstance.hasRole(ADMIN_ROLE, currentFeeHandlerAdmin));
+    });
 });