Add FeeHandlerRouter (#588)

* Make domainID immutable

* Add FeeHandlerRouter contract

* Update FeeHandlers with FeeRouter logic

* Update old test with FeeHandlerRouter logic

* Update old tests + add new tests for FeeHandlerRouter

* Change resourceID-handler mapping type

* Fix indentaton

* Change onlyRouter modifier to onlyBridgeOrRouter

* Remove redundant modifier

* Update old tests + add new tests for onlyBridgeOrRouter modifier

Author: nmlinaric

contracts/Bridge.sol

@@ -18,7 +18,6 @@ import "./interfaces/IAccessControlSegregator.sol";
 contract Bridge is Pausable, Context {
     using ECDSA for bytes32;
 
-
     uint8   public immutable _domainID;
     address public _MPCAddress;
 

contracts/handlers/FeeHandlerRouter.sol

@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+pragma solidity 0.8.11;
+pragma experimental ABIEncoderV2;
+
+import "../interfaces/IFeeHandler.sol";
+import "../utils/AccessControl.sol";
+
+/**
+    @title Handles FeeHandler routing for resources.
+    @author ChainSafe Systems.
+    @notice This contract is intended to be used with the Bridge contract.
+ */
+contract FeeHandlerRouter is IFeeHandler, AccessControl {
+    address public immutable _bridgeAddress;
+
+    // destination domainID => resourceID => feeHandlerAddress
+    mapping (uint8 => mapping(bytes32 => IFeeHandler)) public _domainResourceIDToFeeHandlerAddress;
+
+    event FeeChanged(
+        uint256 newFee
+    );
+
+    modifier onlyBridge() {
+        _onlyBridge();
+        _;
+    }
+
+    function _onlyBridge() private view {
+        require(msg.sender == _bridgeAddress, "sender must be bridge contract");
+    }
+
+    modifier onlyAdmin() {
+        _onlyAdmin();
+        _;
+    }
+
+    function _onlyAdmin() private view {
+        require(hasRole(DEFAULT_ADMIN_ROLE, _msgSender()), "sender doesn't have admin role");
+    }
+
+    /**
+        @param bridgeAddress Contract address of previously deployed Bridge.
+     */
+    constructor(address bridgeAddress) public {
+        _bridgeAddress = bridgeAddress;
+        _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
+    }
+    /**
+        @notice Maps the {handlerAddress} to {resourceID} to {destinantionDomainID} in {_domainResourceIDToFeeHandlerAddress}.
+        @param destinationDomainID ID of chain FeeHandler contracts will be called.
+        @param resourceID ResourceID for which the corresponding FeeHandler will collect/calcualte fee.
+        @param handlerAddress Address of FeeHandler which will be called for specified resourceID.
+     */
+    function adminSetResourceHandler(uint8 destinationDomainID, bytes32 resourceID, IFeeHandler handlerAddress) external onlyAdmin {
+        _domainResourceIDToFeeHandlerAddress[destinationDomainID][resourceID] = handlerAddress;
+    }
+
+
+    /**
+        @notice Initiates collecting fee with corresponding fee handler contract using IFeeHandler interface.
+        @param sender Sender of the deposit.
+        @param destinationDomainID ID of chain deposit will be bridged to.
+        @param resourceID ResourceID to be used when making deposits.
+        @param depositData Additional data to be passed to specified handler.
+        @param feeData Additional data to be passed to the fee handler.
+     */
+    function collectFee(address sender, uint8 fromDomainID, uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) payable external onlyBridge {
+        IFeeHandler feeHandler = _domainResourceIDToFeeHandlerAddress[destinationDomainID][resourceID];
+        feeHandler.collectFee{value: msg.value}(sender, fromDomainID, destinationDomainID, resourceID, depositData, feeData);
+    }
+
+     /**
+        @notice Initiates calculating fee with corresponding fee handler contract using IFeeHandler interface.
+        @param sender Sender of the deposit.
+        @param destinationDomainID ID of chain deposit will be bridged to.
+        @param resourceID ResourceID to be used when making deposits.
+        @param depositData Additional data to be passed to specified handler.
+        @param feeData Additional data to be passed to the fee handler.
+        @return fee Returns the fee amount.
+        @return tokenAddress Returns the address of the token to be used for fee.
+     */
+    function calculateFee(address sender, uint8 fromDomainID, uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) external view returns(uint256 fee, address tokenAddress) {
+        IFeeHandler feeHandler = _domainResourceIDToFeeHandlerAddress[destinationDomainID][resourceID];
+        return feeHandler.calculateFee(sender, fromDomainID, destinationDomainID, resourceID, depositData, feeData);
+    }
+}

contracts/handlers/fee/BasicFeeHandler.sol

@@ -12,27 +12,37 @@ import "../../utils/AccessControl.sol";
  */
 contract BasicFeeHandler is IFeeHandler, AccessControl {
     address public immutable _bridgeAddress;
+    address public immutable _feeHandlerRouterAddress;
 
     uint256 public _fee;
 
     event FeeChanged(
         uint256 newFee
     );
 
-    modifier onlyBridge() {
-        _onlyBridge();
+    modifier onlyAdmin() {
+        require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "sender doesn't have admin role");
+        _;
+    }
+
+    modifier onlyBridgeOrRouter() {
+        _onlyBridgeOrRouter();
         _;
     }
 
-    function _onlyBridge() private view {
-        require(msg.sender == _bridgeAddress, "sender must be bridge contract");
+    function _onlyBridgeOrRouter() private view {
+        require(
+            msg.sender == _bridgeAddress || msg.sender == _feeHandlerRouterAddress,
+            "sender must be bridge or fee router contract"
+        );
     }
 
     /**
-        @param bridgeAddress Contract address of previously deployed Bridge.
+        @param feeHandlerRouterAddress Contract address of previously deployed FeeHandlerRouter.
      */
-    constructor(address bridgeAddress) public {
+    constructor(address bridgeAddress, address feeHandlerRouterAddress) public {
         _bridgeAddress = bridgeAddress;
+        _feeHandlerRouterAddress = feeHandlerRouterAddress;
         _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
     }
 
@@ -41,7 +51,7 @@ contract BasicFeeHandler is IFeeHandler, AccessControl {
         @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 {
+    function renounceAdmin(address newAdmin) external {
         address sender = _msgSender();
         require(sender != newAdmin, 'Cannot renounce oneself');
         grantRole(DEFAULT_ADMIN_ROLE, newAdmin);
@@ -56,7 +66,7 @@ contract BasicFeeHandler is IFeeHandler, AccessControl {
         @param depositData Additional data to be passed to specified handler.
         @param feeData Additional data to be passed to the fee handler.
      */
-    function collectFee(address sender, uint8 fromDomainID, uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) payable external onlyBridge {
+    function collectFee(address sender, uint8 fromDomainID, uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) payable external onlyBridgeOrRouter {
         require(msg.value == _fee, "Incorrect fee supplied");
         emit FeeCollected(sender, fromDomainID, destinationDomainID, resourceID, _fee, address(0));
     }
@@ -100,8 +110,5 @@ contract BasicFeeHandler is IFeeHandler, AccessControl {
         }
     }
 
-    modifier onlyAdmin() {
-        require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "sender doesn't have admin role");
-        _;
-    }
+
 }

contracts/handlers/fee/FeeHandlerWithOracle.sol

@@ -17,6 +17,7 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
  */
 contract FeeHandlerWithOracle is IFeeHandler, AccessControl, ERC20Safe {
     address public immutable _bridgeAddress;
+    address public immutable _feeHandlerRouterAddress;
 
     address public _oracleAddress;
 
@@ -41,11 +42,30 @@ contract FeeHandlerWithOracle is IFeeHandler, AccessControl, ERC20Safe {
         uint256 amount;
     }
 
+    modifier onlyAdmin() {
+        require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "sender doesn't have admin role");
+        _;
+    }
+
+    modifier onlyBridgeOrRouter() {
+        _onlyBridgeOrRouter();
+        _;
+    }
+
+    function _onlyBridgeOrRouter() private view {
+        require(
+            msg.sender == _bridgeAddress || msg.sender == _feeHandlerRouterAddress,
+            "sender must be bridge or fee router contract"
+        );
+    }
+
     /**
         @param bridgeAddress Contract address of previously deployed Bridge.
+        @param feeHandlerRouterAddress Contract address of previously deployed FeeHandlerRouter.
      */
-    constructor(address bridgeAddress) public {
+    constructor(address bridgeAddress, address feeHandlerRouterAddress) public {
         _bridgeAddress = bridgeAddress;
+        _feeHandlerRouterAddress = feeHandlerRouterAddress;
         _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
     }
 
@@ -56,7 +76,7 @@ contract FeeHandlerWithOracle is IFeeHandler, AccessControl, ERC20Safe {
         @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 {
+    function renounceAdmin(address newAdmin) external {
         address sender = _msgSender();
         require(sender != newAdmin, 'Cannot renounce oneself');
         grantRole(DEFAULT_ADMIN_ROLE, newAdmin);
@@ -90,7 +110,7 @@ contract FeeHandlerWithOracle is IFeeHandler, AccessControl, ERC20Safe {
         @param depositData Additional data to be passed to specified handler.
         @param feeData Additional data to be passed to the fee handler.
      */
-    function collectFee(address sender, uint8 fromDomainID, uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) payable external onlyBridge {
+    function collectFee(address sender, uint8 fromDomainID, uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) payable external onlyBridgeOrRouter {
         require(msg.value == 0, "collectFee: msg.value != 0");
         (uint256 fee, address tokenAddress) = _calculateFee(sender, fromDomainID, destinationDomainID, resourceID, depositData, feeData);
         lockERC20(tokenAddress, sender, address(this), fee);
@@ -189,18 +209,4 @@ contract FeeHandlerWithOracle is IFeeHandler, AccessControl, ERC20Safe {
         address signerAddressRecovered = ECDSA.recover(message, signature);
         require(signerAddressRecovered == signerAddress, 'Invalid signature');
     }
-
-    modifier onlyAdmin() {
-        require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "sender doesn't have admin role");
-        _;
-    }
-
-    modifier onlyBridge() {
-        _onlyBridge();
-        _;
-    }
-
-    function _onlyBridge() private view {
-        require(msg.sender == _bridgeAddress, "sender must be bridge contract");
-    }
 }

test/feeRouter/feeRouter.js

@@ -8,6 +8,7 @@
  const Helpers = require("../../../helpers");
 
  const BasicFeeHandlerContract = artifacts.require("BasicFeeHandler");
+ const FeeHandlerRouterContract = artifacts.require("FeeHandlerRouter");
 
  contract("BasicFeeHandler - [admin]", async accounts => {
     const domainID = 1;
@@ -24,7 +25,8 @@
 
     beforeEach(async () => {
         BridgeInstance = awaitBridgeInstance = await Helpers.deployBridge(domainID, accounts[0]);
-        BasicFeeHandlerInstance = await BasicFeeHandlerContract.new(BridgeInstance.address);
+        FeeHandlerRouterInstance = await FeeHandlerRouterContract.new(BridgeInstance.address);
+        BasicFeeHandlerInstance = await BasicFeeHandlerContract.new(BridgeInstance.address, FeeHandlerRouterInstance.address);
 
         ADMIN_ROLE = await BasicFeeHandlerInstance.DEFAULT_ADMIN_ROLE();
     });

test/handlers/fee/basic/admin.js

@@ -8,10 +8,10 @@ const Ethers = require("ethers");
 
 const Helpers = require("../../../helpers");
 
-const BridgeContract = artifacts.require("Bridge");
 const ERC20MintableContract = artifacts.require("ERC20PresetMinterPauser");
 const ERC20HandlerContract = artifacts.require("ERC20Handler");
 const BasicFeeHandlerContract = artifacts.require("BasicFeeHandler");
+const FeeHandlerRouterContract = artifacts.require("FeeHandlerRouter");
 
 contract("BasicFeeHandler - [calculateFee]", async (accounts) => {
     const originDomainID = 1;
@@ -27,42 +27,48 @@ contract("BasicFeeHandler - [calculateFee]", async (accounts) => {
     let initialResourceIDs;
     let initialContractAddresses;
     let ERC20MintableInstance;
+    let FeeHandlerRouterInstance;
 
     beforeEach(async () => {
         await Promise.all([
+            BridgeInstance = await Helpers.deployBridge(destinationDomainID, accounts[0]),
             ERC20MintableContract.new("token", "TOK").then(instance => ERC20MintableInstance = instance),
-            BridgeInstance = BridgeInstance = await Helpers.deployBridge(destinationDomainID, accounts[0])
         ]);
 
+        ERC20HandlerInstance = await ERC20HandlerContract.new(BridgeInstance.address);
+        FeeHandlerRouterInstance = await FeeHandlerRouterContract.new(BridgeInstance.address);
+        BasicFeeHandlerInstance = await BasicFeeHandlerContract.new(BridgeInstance.address, FeeHandlerRouterInstance.address);
+
         resourceID = Helpers.createResourceID(ERC20MintableInstance.address, originDomainID);
         initialResourceIDs = [resourceID];
         initialContractAddresses = [ERC20MintableInstance.address];
-        burnableContractAddresses = [];
-
-        BasicFeeHandlerInstance = await BasicFeeHandlerContract.new(BridgeInstance.address);
-
-        ERC20HandlerInstance = await ERC20HandlerContract.new(BridgeInstance.address);
 
-        await BridgeInstance.adminSetResource(ERC20HandlerInstance.address, resourceID, ERC20MintableInstance.address)
+        burnableContractAddresses = [];
 
         depositData = Helpers.createERCDepositData(100, 20, recipientAddress);
+
+        await Promise.all([
+            BridgeInstance.adminSetResource(ERC20HandlerInstance.address, resourceID, ERC20MintableInstance.address),
+            BridgeInstance.adminChangeFeeHandler(FeeHandlerRouterInstance.address),
+            FeeHandlerRouterInstance.adminSetResourceHandler(destinationDomainID, resourceID, BasicFeeHandlerInstance.address),
+        ]);
     });
 
     it("should return amount of fee", async () => {
-        await BridgeInstance.adminChangeFeeHandler(BasicFeeHandlerInstance.address);
         // current fee is set to 0
-        let res = await BasicFeeHandlerInstance.calculateFee.call(
+        let res = await FeeHandlerRouterInstance.calculateFee.call(
             relayer,
             originDomainID,
             destinationDomainID,
             resourceID,
             depositData,
             feeData
         );
+
         assert.equal(web3.utils.fromWei(res[0], "ether"), "0");
         // Change fee to 0.5 ether
         await BasicFeeHandlerInstance.changeFee(Ethers.utils.parseEther("0.5"));
-        res = await BasicFeeHandlerInstance.calculateFee.call(
+        res = await FeeHandlerRouterInstance.calculateFee.call(
             relayer,
             originDomainID,
             destinationDomainID,

test/handlers/fee/basic/calculateFee.js

@@ -9,6 +9,7 @@ const Ethers = require("ethers");
 const Helpers = require('../../../helpers');
 
 const BasicFeeHandlerContract = artifacts.require("BasicFeeHandler");
+const FeeHandlerRouterContract = artifacts.require("FeeHandlerRouter");
 
 contract("BasicFeeHandler - [changeFee]", async accounts => {
     const domainID = 1;
@@ -21,16 +22,17 @@ contract("BasicFeeHandler - [changeFee]", async accounts => {
     let BridgeInstance;
 
     beforeEach(async () => {
-        BridgeInstance = awaitBridgeInstance = await Helpers.deployBridge(domainID, accounts[0]);
+        BridgeInstance = await Helpers.deployBridge(domainID, accounts[0]);
+        FeeHandlerRouterInstance = await FeeHandlerRouterContract.new(BridgeInstance.address);
     });
 
     it("[sanity] contract should be deployed successfully", async () => {
         TruffleAssert.passes(
-            await BasicFeeHandlerContract.new(BridgeInstance.address));
+            await BasicFeeHandlerContract.new(BridgeInstance.address, FeeHandlerRouterInstance.address));
     });
 
     it("should set fee", async () => {
-        const BasicFeeHandlerInstance = await BasicFeeHandlerContract.new(BridgeInstance.address);
+        const BasicFeeHandlerInstance = await BasicFeeHandlerContract.new(BridgeInstance.address, FeeHandlerRouterInstance.address);
         const fee = Ethers.utils.parseEther("0.05");
         const tx = await BasicFeeHandlerInstance.changeFee(fee);
         TruffleAssert.eventEmitted(tx, "FeeChanged", (event) =>
@@ -41,12 +43,12 @@ contract("BasicFeeHandler - [changeFee]", async accounts => {
     });
 
     it("should not set the same fee", async () => {
-        const BasicFeeHandlerInstance = await BasicFeeHandlerContract.new(BridgeInstance.address);
+        const BasicFeeHandlerInstance = await BasicFeeHandlerContract.new(BridgeInstance.address, FeeHandlerRouterInstance.address);
         await TruffleAssert.reverts(BasicFeeHandlerInstance.changeFee(0), "Current fee is equal to new fee");
     });
 
     it("should require admin role to change fee", async () => {
-        const BasicFeeHandlerInstance = await BasicFeeHandlerContract.new(BridgeInstance.address);
+        const BasicFeeHandlerInstance = await BasicFeeHandlerContract.new(BridgeInstance.address, FeeHandlerRouterInstance.address);
         await assertOnlyAdmin(BasicFeeHandlerInstance.changeFee, 1);
     });
 });