feat: rearrange core contracts addresses (#32)

* Fix missing custom errors

* Update tests

* fix: add renounce admin zero address check

* Rearrange core contracts addresses and functions

* Update tests after core addresses and functions rearrangement

* set getters as views in IBridge

* Set Bridge as single source of truth for Router and Executor addresses

---------

Co-authored-by: mpetrun5 <[email protected]>

Author: nmlinaric

package.json

@@ -25,7 +25,7 @@
     "@metamask/eth-sig-util": "^7.0.0",
     "@nomicfoundation/hardhat-chai-matchers": "^2.0.2",
     "@nomicfoundation/hardhat-ethers": "^3.0.4",
-    "@nomicfoundation/hardhat-network-helpers": "^1.0.9",
+    "@nomicfoundation/hardhat-network-helpers": "^1.0.10",
     "@nomicfoundation/hardhat-toolbox": "^3.0.0",
     "@nomicfoundation/hardhat-verify": "^1.0.0",
     "@openzeppelin/contracts": "4.9.3",
@@ -39,6 +39,7 @@
     "@typescript-eslint/parser": "^7.3.1",
     "chai": "^4.2.0",
     "chain": "^0.4.0",
+    "dotenv": "^16.4.1",
     "eslint": "^8.57.0",
     "eslint-config-prettier": "^9.0.0",
     "eslint-plugin-prettier": "^5.0.1",
@@ -53,8 +54,7 @@
     "solidity-coverage": "^0.8.0",
     "ts-node": ">=8.0.0",
     "tslint-config-prettier": "^1.18.0",
-    "typescript": "5.3.3",
-    "dotenv": "^16.4.1"
+    "typescript": "5.3.3"
   },
   "dependencies": {
     "hardhat": "^2.17.4",

src/contracts/Bridge.sol

@@ -20,8 +20,10 @@ contract Bridge is Pausable, Context {
     uint8 public immutable _domainID;
 
     IFeeHandler public _feeHandler;
-
     IAccessControlSegregator public _accessControl;
+    address public _executorAddress;
+    address public _routerAddress;
+
 
     // resourceID => handler address
     mapping(bytes32 => address) public _resourceIDToHandlerAddress;
@@ -140,4 +142,12 @@ contract Bridge is Pausable, Context {
         IERCHandler handler = IERCHandler(handlerAddress);
         handler.withdraw(data);
     }
+
+    function adminChangeRouterAddress(address routerAddress) external onlyAllowed {
+        _routerAddress = routerAddress;
+    }
+
+    function adminChangeExecutorAddress(address executorAddress) external onlyAllowed {
+        _executorAddress = executorAddress;
+    }
 }

src/contracts/TestContracts.sol

@@ -5,6 +5,7 @@ pragma solidity 0.8.11;
 import "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol";
 import "./handlers/ERCHandlerHelpers.sol";
 import "./interfaces/IERC20Plus.sol";
+import "./interfaces/IBridge.sol";
 
 contract NoArgument {
     event NoArgumentCalled();
@@ -58,13 +59,9 @@ abstract contract HandlerRevert is ERCHandlerHelpers {
     uint256 private _totalAmount;
 
     constructor(
-        address bridgeAddress,
-        address routerAddress,
-        address executorAddress
+        IBridge bridgeAddress
     ) ERCHandlerHelpers(
-        bridgeAddress,
-        routerAddress,
-        executorAddress
+        bridgeAddress
     ) {}
 
     function executeProposal(bytes32, bytes calldata) external view {

src/contracts/handlers/ERC20Handler.sol

@@ -13,18 +13,41 @@ import "../ERC20Safe.sol";
  */
 contract ERC20Handler is IHandler, ERCHandlerHelpers, ERC20Safe {
     /**
-        @param bridgeAddress Contract address of previously deployed Bridge.
+        @param bridge Contract address of previously deployed Bridge.
      */
     constructor(
-        address bridgeAddress,
-        address routerAddress,
-        address executorAddress
+        IBridge bridge
     ) ERCHandlerHelpers(
-        bridgeAddress,
-        routerAddress,
-        executorAddress
+        bridge
     ) {}
 
+    modifier onlyBridge() {
+        _onlyBridge();
+        _;
+    }
+
+    modifier onlyRouter() {
+        _onlyRouter();
+        _;
+    }
+
+    modifier onlyExecutor() {
+        _onlyExecutor();
+        _;
+    }
+
+    function _onlyBridge() private {
+        if (msg.sender != address(_bridge)) revert SenderNotBridgeContract();
+    }
+
+    function _onlyExecutor() private {
+        if (msg.sender != _bridge._executorAddress()) revert SenderNotExecutorContract();
+    }
+
+    function _onlyRouter() private {
+        if (msg.sender != _bridge._routerAddress()) revert SenderNotRouterContract();
+    }
+
     /**
         @notice A deposit is initiated by making a deposit in the Bridge contract.
         @param resourceID ResourceID used to find address of token to be used for deposit.
@@ -114,7 +137,7 @@ contract ERC20Handler is IHandler, ERCHandlerHelpers, ERC20Safe {
         recipient                              address     bytes  32 - 64
         amount                                 uint256     bytes  64 - 96
      */
-    function withdraw(bytes memory data) external override onlyBridge {
+    function withdraw(bytes memory data) external onlyBridge {
         address tokenAddress;
         address recipient;
         uint256 amount;
@@ -142,4 +165,13 @@ contract ERC20Handler is IHandler, ERCHandlerHelpers, ERC20Safe {
             _setDecimals(contractAddress, externalTokenDecimals);
         }
     }
+
+    /**
+        @notice First verifies {contractAddress} is whitelisted, then sets
+        {_tokenContractAddressToTokenProperties[contractAddress].isBurnable} to true.
+        @param contractAddress Address of contract to be used when making or executing deposits.
+     */
+    function setBurnable(address contractAddress) external onlyBridge {
+        _setBurnable(contractAddress);
+    }
 }

src/contracts/handlers/ERCHandlerHelpers.sol

@@ -3,16 +3,16 @@
 pragma solidity 0.8.11;
 
 import "../interfaces/IERCHandler.sol";
+import "../interfaces/IBridge.sol";
+
 
 /**
     @title Function used across handler contracts.
     @author ChainSafe Systems.
     @notice This contract is intended to be used with the Bridge contract.
  */
-abstract contract ERCHandlerHelpers is IERCHandler {
-    address public immutable _bridgeAddress;
-    address public immutable _routerAddress;
-    address public immutable _executorAddress;
+contract ERCHandlerHelpers {
+    IBridge public _bridge;
 
     uint8 public constant DEFAULT_DECIMALS = 18;
 
@@ -40,52 +40,11 @@ abstract contract ERCHandlerHelpers is IERCHandler {
     // token contract address => ERCTokenContractProperties
     mapping(address => ERCTokenContractProperties) public _tokenContractAddressToTokenProperties;
 
-    modifier onlyBridge() {
-        _onlyBridge();
-        _;
-    }
-
-    modifier onlyRouter() {
-        _onlyRouter();
-        _;
-    }
-
-    modifier onlyExecutor() {
-        _onlyExecutor();
-        _;
-    }
-
-    function _onlyBridge() private view {
-        if (msg.sender != _bridgeAddress) revert SenderNotBridgeContract();
-    }
-
-    function _onlyExecutor() private view {
-        if (msg.sender != _executorAddress) revert SenderNotExecutorContract();
-    }
-
-    function _onlyRouter() private view {
-        if (msg.sender != _routerAddress) revert SenderNotRouterContract();
-    }
-
-
     /**
-        @param bridgeAddress Contract address of previously deployed Bridge.
-        @param routerAddress Contract address of previously deployed Router.
-        @param executorAddress Contract address of previously deployed Executor.
-     */
-    constructor(address bridgeAddress, address routerAddress, address executorAddress) {
-        _bridgeAddress = bridgeAddress;
-        _routerAddress = routerAddress;
-        _executorAddress = executorAddress;
-    }
-
-    /**
-        @notice First verifies {contractAddress} is whitelisted, then sets
-        {_tokenContractAddressToTokenProperties[contractAddress].isBurnable} to true.
-        @param contractAddress Address of contract to be used when making or executing deposits.
+        @param bridge Contract address of previously deployed Bridge.
      */
-    function setBurnable(address contractAddress) external override onlyBridge {
-        _setBurnable(contractAddress);
+    constructor(IBridge bridge) {
+        _bridge = bridge;
     }
 
     function _setResource(bytes32 resourceID, address contractAddress) internal {

src/contracts/handlers/FeeHandlerRouter.sol

@@ -3,6 +3,7 @@
 pragma solidity 0.8.11;
 
 import "../interfaces/IFeeHandler.sol";
+import "../interfaces/IBridge.sol";
 import "@openzeppelin/contracts/access/AccessControl.sol";
 
 /**
@@ -11,7 +12,7 @@ import "@openzeppelin/contracts/access/AccessControl.sol";
     @notice This contract is intended to be used with the Bridge contract.
  */
 contract FeeHandlerRouter is IFeeHandler, AccessControl {
-    address public immutable _routerAddress;
+    IBridge public immutable _bridge;
 
     // domainID => resourceID => securityModel => feeHandlerAddress
     mapping(uint8 => mapping(bytes32 => mapping(uint8 => IFeeHandler))) public
@@ -32,8 +33,8 @@ contract FeeHandlerRouter is IFeeHandler, AccessControl {
         _;
     }
 
-    function _onlyRouter() private view {
-        if (msg.sender != _routerAddress) revert SenderNotRouterContract();
+    function _onlyRouter() private {
+        if (msg.sender != _bridge._routerAddress()) revert SenderNotRouterContract();
     }
 
     modifier onlyAdmin() {
@@ -46,10 +47,10 @@ contract FeeHandlerRouter is IFeeHandler, AccessControl {
     }
 
     /**
-        @param routerAddress Contract address of previously deployed Router.
+        @param bridge Contract address of previously deployed Bridge.
      */
-    constructor(address routerAddress) {
-        _routerAddress = routerAddress;
+    constructor(IBridge bridge) {
+        _bridge = bridge;
         _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
     }
 

src/contracts/handlers/PermissionlessGenericHandler.sol

@@ -3,6 +3,7 @@
 
 pragma solidity 0.8.11;
 import "../interfaces/IHandler.sol";
+import "../interfaces/IBridge.sol";
 
 /**
     @title Handles generic deposits and deposit executions.
@@ -12,8 +13,7 @@ import "../interfaces/IHandler.sol";
 contract PermissionlessGenericHandler is IHandler {
     uint256 public constant MAX_FEE = 1000000;
 
-    address public immutable _bridgeAddress;
-    address public immutable _executorAddress;
+    IBridge public immutable _bridge;
 
     modifier onlyBridge() {
         _onlyBridge();
@@ -27,25 +27,23 @@ contract PermissionlessGenericHandler is IHandler {
     error InvalidExecutioDataDepositor(address executioDataDepositor);
 
     function _onlyBridge() private view {
-        if (msg.sender != _bridgeAddress) revert SenderNotBridgeContract();
+        if (msg.sender != address(_bridge)) revert SenderNotBridgeContract();
     }
 
     modifier onlyExecutor() {
         _onlyExecutor();
         _;
     }
 
-    function _onlyExecutor() private view {
-        if (msg.sender != _executorAddress) revert SenderNotExecutorContract();
+    function _onlyExecutor() private {
+        if (msg.sender != _bridge._executorAddress()) revert SenderNotExecutorContract();
     }
 
     /**
-        @param bridgeAddress Contract address of previously deployed Bridge.
-        @param executorAddress Contract address of previously deployed Executor.
+        @param bridge Contract address of previously deployed Bridge.
      */
-    constructor(address bridgeAddress, address executorAddress) {
-        _bridgeAddress = bridgeAddress;
-        _executorAddress = executorAddress;
+    constructor(IBridge bridge) {
+        _bridge = bridge;
     }
 
     /**

src/contracts/handlers/fee/BasicFeeHandler.sol

@@ -3,6 +3,7 @@
 pragma solidity 0.8.11;
 
 import "../../interfaces/IFeeHandler.sol";
+import "../../interfaces/IBridge.sol";
 import "@openzeppelin/contracts/access/AccessControl.sol";
 
 import "../FeeHandlerRouter.sol";
@@ -13,9 +14,8 @@ import "../FeeHandlerRouter.sol";
     @notice This contract is intended to be used with the Bridge contract.
  */
 contract BasicFeeHandler is IFeeHandler, AccessControl {
-    address public immutable _bridgeAddress;
+    IBridge public immutable _bridge;
     address public immutable _feeHandlerRouterAddress;
-    address public immutable _routerAddress;
     // domainID => resourceID => securityModel => fee
     mapping (uint8 => mapping(bytes32 => mapping(uint8 => uint256))) public _domainResourceIDSecurityModelToFee;
 
@@ -44,25 +44,23 @@ contract BasicFeeHandler is IFeeHandler, AccessControl {
         _;
     }
 
-    function _onlyRouterOrFeeRouter() private view {
+    function _onlyRouterOrFeeRouter() private {
         if (msg.sender != _feeHandlerRouterAddress &&
-            msg.sender != _routerAddress
+            msg.sender != _bridge._routerAddress()
         ) revert SenderNotBridgeOrRouter();
     }
 
     /**
-        @param bridgeAddress Contract address of previously deployed Bridge.
+        @param bridge Contract address of previously deployed Bridge.
         @param feeHandlerRouterAddress Contract address of previously deployed FeeHandlerRouter.
      */
-    constructor(address bridgeAddress, address feeHandlerRouterAddress, address routerAddress) {
-        if (bridgeAddress == address(0) ||
-            feeHandlerRouterAddress == address(0) ||
-            routerAddress == address(0)
+    constructor(IBridge bridge, address feeHandlerRouterAddress) {
+        if (address(bridge) == address(0) ||
+            feeHandlerRouterAddress == address(0)
         ) revert ZeroAddressProvided();
 
-        _bridgeAddress = bridgeAddress;
+        _bridge = bridge;
         _feeHandlerRouterAddress = feeHandlerRouterAddress;
-        _routerAddress = routerAddress;
         _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
     }