Refactoring and cleanup (#79)

README.md

@@ -30,12 +30,12 @@ On the destination chain, if 2 of the 3 AMB agree with each other, we would cons
 The flow of the message and how it is transformed and relayed is detailed below:
 
 1. Uniswap Ethereum governance structure, `src`, approves to execute a message `msg` on destination chain `dst`.
-2. Uniswap governance sends `msg` to `MultiMessageSender`.
-3. `MultiMessageSender` relays `msg` to different adapters `adapter`.
+2. Uniswap governance sends `msg` to `MultiBridgeMessageSender`.
+3. `MultiBridgeMessageSender` relays `msg` to different adapters `adapter`.
 4. `adapter` encodes `msg` into the corresponding formatted message, `formatted_msg`, and sends it to the hardcoded AMB contracts `AMB`.
 5. Each `AMB` independently carries `formatted_msg` to `dst`.
 6. On the destination chain, another set of `adapters` decodes `formatted_msgs` into the original `msg`.
-7. `msg` is collected inside the `MultiMessageReceiver` contract.
+7. `msg` is collected inside the `MultiBridgeMessageReceiver` contract.
 8. If 2 out of 3 `msg` is the same, the `msg` will be executed on `dst`.
 
 ![Illustration of ](https://files.gitbook.com/v0/b/gitbook-x-prod.appspot.com/o/spaces%2FyWOfgotvwuIBhzylK0ud%2Fuploads%2Fco073eKSrR7xUmhObi7v%2FMMA_Highlevel.png?alt=media&token=bff8ec55-c04f-4ab9-b362-caae601154db)
@@ -97,8 +97,8 @@ All code changes must be thoroughly tested. Please ensure that your tests cover
 ## Contracts
 ```
 contracts
-├── MultiMessageReceiver.sol
-├── MultiMessageSender.sol
+├── MultiBridgeMessageReceiver.sol
+├── MultiBridgeMessageSender.sol
 ├── adapters
 │   ├── BaseSenderAdapter.sol
 │   ├── axelar
@@ -126,7 +126,7 @@ contracts
 │   ├── IGovernanceTimelock.sol
 │   ├── IMessageReceiverAdapter.sol
 │   ├── IMessageSenderAdapter.sol
-│   └── IMultiMessageReceiver.sol
+│   └── IMultiBridgeMessageReceiver.sol
 └── libraries
     ├── Error.sol
     ├── Message.sol

src/MultiMessageReceiver.sol → src/MultiBridgeMessageReceiver.sol

@@ -5,24 +5,24 @@ pragma solidity >=0.8.9;
 import "@openzeppelin/contracts/proxy/utils/Initializable.sol";
 
 /// interfaces
-import "./interfaces/IMessageReceiverAdapter.sol";
-import "./interfaces/IMultiMessageReceiver.sol";
+import "./interfaces/adapters/IMessageReceiverAdapter.sol";
+import "./interfaces/IMultiBridgeMessageReceiver.sol";
 import "./interfaces/EIP5164/ExecutorAware.sol";
-import "./interfaces/IGovernanceTimelock.sol";
+import "./interfaces/controllers/IGovernanceTimelock.sol";
 
 /// libraries
 import "./libraries/Error.sol";
 import "./libraries/Message.sol";
 
 /// @title Multi-bridge message receiver.
-/// @notice This contract is deployed on each destination chain, and receives messages sent by the MultiMessageSender
+/// @notice This contract is deployed on each destination chain, and receives messages sent by the MultiBridgeMessageSender.sol
 /// contract on the source chain, through multiple bridge adapters. A message is considered valid and can be executed
 /// if it has been delivered by enough trusted bridge receiver adapters (i.e. has achieved a quorum threshold),
 /// before the message's expiration. If a message is successfully validated in time, it is queued for execution on the
 /// governance timelock contract.
 /// @dev The contract only accepts messages from trusted bridge receiver adapters, each of which implements the
 /// IMessageReceiverAdapter interface.
-contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializable {
+contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAware, Initializable {
     /*/////////////////////////////////////////////////////////////////
                             STATE VARIABLES
     ////////////////////////////////////////////////////////////////*/
@@ -116,7 +116,7 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
         }
 
         /// this msgId is totally different with each adapters' internal msgId(which is their internal nonce essentially)
-        /// although each adapters' internal msgId is attached at the end of calldata, it's not useful to MultiMessageReceiver.
+        /// although each adapters' internal msgId is attached at the end of calldata, it's not useful to MultiBridgeMessageReceiver.sol.
         bytes32 msgId = MessageLibrary.computeMsgId(_message);
 
         if (msgDeliveries[msgId][msg.sender]) {
@@ -143,13 +143,11 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
         }
 
         string memory bridgeName = IMessageReceiverAdapter(msg.sender).name();
-        emit SingleBridgeMsgReceived(msgId, bridgeName, _message.nonce, msg.sender);
+        emit BridgeMessageReceived(msgId, bridgeName, _message.nonce, msg.sender);
     }
 
-    /// @notice Execute the message (invoke external call according to the message content) if the message
-    /// @dev has reached the power threshold (the same message has been delivered by enough multiple bridges).
-    /// Param values can be found in the MultiMessageSent event from the source chain MultiMessageSender contract.
-    function executeMessage(bytes32 msgId) external {
+    /// @inheritdoc IMultiBridgeMessageReceiver
+    function executeMessage(bytes32 msgId) external override {
         ExecutionData memory _execData = msgExecData[msgId];
 
         /// @dev validates if msg execution is not beyond expiration
@@ -181,6 +179,7 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
     /// @dev called by admin to update receiver bridge adapters on all other chains
     function updateReceiverAdapters(address[] calldata _receiverAdapters, bool[] calldata _operations)
         external
+        override
         onlyGovernanceTimelock
     {
         _updateReceiverAdapters(_receiverAdapters, _operations);
@@ -192,7 +191,7 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
         uint64 _newQuorum,
         address[] calldata _receiverAdapters,
         bool[] calldata _operations
-    ) external onlyGovernanceTimelock {
+    ) external override onlyGovernanceTimelock {
         /// @dev updates quorum here
         _updateQuorum(_newQuorum);
 
@@ -201,7 +200,7 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
     }
 
     /// @notice Update power quorum threshold of message execution.
-    function updateQuorum(uint64 _quorum) external onlyGovernanceTimelock {
+    function updateQuorum(uint64 _quorum) external override onlyGovernanceTimelock {
         _updateQuorum(_quorum);
     }
 
@@ -281,6 +280,6 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
                 revert Error.INVALID_QUORUM_THRESHOLD();
             }
         }
-        emit ReceiverAdapterUpdated(_receiverAdapter, _add);
+        emit BridgeReceiverAdapterUpdated(_receiverAdapter, _add);
     }
 }

src/MultiMessageSender.sol → src/MultiBridgeMessageSender.sol

@@ -3,9 +3,9 @@
 pragma solidity >=0.8.9;
 
 /// interfaces
-import "./interfaces/IMessageSenderAdapter.sol";
-import "./interfaces/IMultiMessageReceiver.sol";
-import "./interfaces/IGAC.sol";
+import "./interfaces/adapters/IMessageSenderAdapter.sol";
+import "./interfaces/IMultiBridgeMessageReceiver.sol";
+import "./controllers/MessageSenderGAC.sol";
 
 /// libraries
 import "./libraries/Message.sol";
@@ -16,13 +16,13 @@ import "./libraries/Error.sol";
 /// The contract has only a single authorised caller that can send messages, and an owner that can change key parameters.
 /// Both of these are configured in the Global Access Control contract. In the case of Uniswap, both the authorised caller
 /// and owner should be set to the Uniswap V2 Timelock contract on Ethereum.
-contract MultiMessageSender {
+contract MultiBridgeMessageSender {
     /*///////////////////////////////////////////////////////////////
                              CONSTANTS
     //////////////////////////////////////////////////////////////*/
 
     /// @dev Global Access Controller (GAC) contract
-    IGAC public immutable gac;
+    MessageSenderGAC public immutable senderGAC;
 
     /// @dev the minimum and maximum duration that a message's expiration parameter can be set to
     uint256 public constant MIN_EXPIRATION = 2 days;
@@ -51,10 +51,10 @@ contract MultiMessageSender {
     /// @param target is the target execution address on the destination chain
     /// @param callData is the data to be sent to _target by low-level call(eg. address(_target).call(_callData))
     /// @param nativeValue is the value to be sent to _target by low-level call (eg. address(_target).call{value: _nativeValue}(_callData))
-    /// @param expiration refers to the number of days that a message remains valid before it is considered stale and can no longer be executed.
+    /// @param expiration refers to the number seconds that a message remains valid before it is considered stale and can no longer be executed.
     /// @param senderAdapters are the sender adapters that were used to send the message
     /// @param adapterSuccess are the message sending success status for each of the corresponding adapters listed in senderAdapters
-    event MultiMessageSent(
+    event MultiBridgeMessageSent(
         bytes32 indexed msgId,
         uint256 nonce,
         uint256 indexed dstChainId,
@@ -82,15 +82,15 @@ contract MultiMessageSender {
 
     /// @dev checks if msg.sender is the owner configured in GAC
     modifier onlyOwner() {
-        if (msg.sender != gac.getGlobalOwner()) {
+        if (msg.sender != senderGAC.getGlobalOwner()) {
             revert Error.CALLER_NOT_OWNER();
         }
         _;
     }
 
     /// @dev checks if msg.sender is the authorised caller configured in GAC
     modifier onlyCaller() {
-        if (msg.sender != gac.getMultiMessageCaller()) {
+        if (msg.sender != senderGAC.getAuthorisedCaller()) {
             revert Error.INVALID_PRIVILEGED_CALLER();
         }
         _;
@@ -109,13 +109,13 @@ contract MultiMessageSender {
                                 CONSTRUCTOR
     ////////////////////////////////////////////////////////////////*/
 
-    /// @param _gac is the controller/registry of MMA
-    constructor(address _gac) {
-        if (_gac == address(0)) {
+    /// @param _senderGAC is the controller/registry of MMA
+    constructor(address _senderGAC) {
+        if (_senderGAC == address(0)) {
             revert Error.ZERO_ADDRESS_INPUT();
         }
 
-        gac = IGAC(_gac);
+        senderGAC = MessageSenderGAC(_senderGAC);
     }
 
     /*/////////////////////////////////////////////////////////////////
@@ -131,7 +131,7 @@ contract MultiMessageSender {
     /// @param _target is the target execution point on the destination chain
     /// @param _callData is the data to be sent to _target by low-level call(eg. address(_target).call(_callData))
     /// @param _nativeValue is the value to be sent to _target by low-level call (eg. address(_target).call{value: _nativeValue}(_callData))
-    /// @param _expiration refers to the number of days that a message remains valid before it is considered stale and can no longer be executed.
+    /// @param _expiration refers to the number of seconds that a message remains valid before it is considered stale and can no longer be executed.
     /// @param _refundAddress refers to the refund address for any extra native tokens paid
     function remoteCall(
         uint256 _dstChainId,
@@ -149,7 +149,7 @@ contract MultiMessageSender {
     /// @param _target is the target execution point on the destination chain
     /// @param _callData is the data to be sent to _target by low-level call(eg. address(_target).call(_callData))
     /// @param _nativeValue is the value to be sent to _target by low-level call (eg. address(_target).call{value: _nativeValue}(_callData))
-    /// @param _expiration refers to the number of days that a message remains valid before it is considered stale and can no longer be executed.
+    /// @param _expiration refers to the number of seconds that a message remains valid before it is considered stale and can no longer be executed.
     /// @param _excludedAdapters are the sender adapters to be excluded from relaying the message
     /// @param _refundAddress refers to the refund address for any extra native tokens paid
     function remoteCall(
@@ -195,7 +195,7 @@ contract MultiMessageSender {
     /// @notice A helper function for estimating total required message fee by all available message bridges.
     function estimateTotalMessageFee(
         uint256 _dstChainId,
-        address _multiMessageReceiver,
+        address _multiBridgeMessageReceiver,
         address _target,
         bytes calldata _callData,
         uint256 _nativeValue
@@ -208,11 +208,12 @@ contract MultiMessageSender {
         address[] storage adapters = senderAdapters;
 
         /// @dev generates the dst chain function call
-        data = abi.encodeWithSelector(IMultiMessageReceiver.receiveMessage.selector, message);
+        data = abi.encodeWithSelector(IMultiBridgeMessageReceiver.receiveMessage.selector, message);
 
         for (uint256 i; i < adapters.length; ++i) {
-            uint256 fee =
-                IMessageSenderAdapter(adapters[i]).getMessageFee(uint256(_dstChainId), _multiMessageReceiver, data);
+            uint256 fee = IMessageSenderAdapter(adapters[i]).getMessageFee(
+                uint256(_dstChainId), _multiBridgeMessageReceiver, data
+            );
 
             totalFee += fee;
         }
@@ -247,7 +248,7 @@ contract MultiMessageSender {
             revert Error.INVALID_REFUND_ADDRESS();
         }
 
-        address mmaReceiver = gac.getMultiMessageReceiver(_dstChainId);
+        address mmaReceiver = senderGAC.getRemoteMultiBridgeMessageReceiver(_dstChainId);
 
         if (mmaReceiver == address(0)) {
             revert Error.ZERO_RECEIVER_ADAPTER();
@@ -267,7 +268,7 @@ contract MultiMessageSender {
         }
 
         bool[] memory adapterSuccesses = _dispatchMessages(adapters, mmaReceiver, _dstChainId, message);
-        emit MultiMessageSent(
+        emit MultiBridgeMessageSent(
             msgId, nonce, _dstChainId, _target, _callData, _nativeValue, _expiration, adapters, adapterSuccesses
         );
 

src/adapters/BaseReceiverAdapter.sol

@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-3.0-only
+pragma solidity >=0.8.9;
+
+import "../interfaces/adapters/IMessageReceiverAdapter.sol";
+import "../controllers/MessageReceiverGAC.sol";
+
+abstract contract BaseReceiverAdapter is IMessageReceiverAdapter {
+    MessageReceiverGAC public immutable receiverGAC;
+    address public senderAdapter;
+
+    modifier onlyGlobalOwner() {
+        if (!receiverGAC.isGlobalOwner(msg.sender)) {
+            revert Error.CALLER_NOT_OWNER();
+        }
+        _;
+    }
+
+    constructor(address _receiverGAC) {
+        if (_receiverGAC == address(0)) {
+            revert Error.ZERO_ADDRESS_INPUT();
+        }
+        receiverGAC = MessageReceiverGAC(_receiverGAC);
+    }
+
+    /// @inheritdoc IMessageReceiverAdapter
+    function updateSenderAdapter(address _newSender) external override onlyGlobalOwner {
+        if (_newSender == address(0)) {
+            revert Error.ZERO_ADDRESS_INPUT();
+        }
+
+        address oldSender = senderAdapter;
+        senderAdapter = _newSender;
+
+        emit SenderAdapterUpdated(oldSender, _newSender);
+    }
+}

src/adapters/BaseSenderAdapter.sol

@@ -1,12 +1,12 @@
 // SPDX-License-Identifier: GPL-3.0-only
 pragma solidity >=0.8.9;
 
-import "../interfaces/IGAC.sol";
 import "../libraries/Error.sol";
-import "../interfaces/IMessageSenderAdapter.sol";
+import "../interfaces/adapters/IMessageSenderAdapter.sol";
+import "../controllers/MessageSenderGAC.sol";
 
 abstract contract BaseSenderAdapter is IMessageSenderAdapter {
-    IGAC public immutable gac;
+    MessageSenderGAC public immutable senderGAC;
 
     /*/////////////////////////////////////////////////////////////////
                             STATE VARIABLES
@@ -18,15 +18,15 @@ abstract contract BaseSenderAdapter is IMessageSenderAdapter {
     /*/////////////////////////////////////////////////////////////////
                                  MODIFIER
     ////////////////////////////////////////////////////////////////*/
-    modifier onlyMultiMessageSender() {
-        if (msg.sender != gac.getMultiMessageSender()) {
+    modifier onlyMultiBridgeMessageSender() {
+        if (msg.sender != senderGAC.getMultiBridgeMessageSender()) {
             revert Error.CALLER_NOT_MULTI_MESSAGE_SENDER();
         }
         _;
     }
 
     modifier onlyGlobalOwner() {
-        if (!gac.isGlobalOwner(msg.sender)) {
+        if (!senderGAC.isGlobalOwner(msg.sender)) {
             revert Error.CALLER_NOT_OWNER();
         }
         _;
@@ -36,13 +36,13 @@ abstract contract BaseSenderAdapter is IMessageSenderAdapter {
                                  CONSTRUCTOR
     ////////////////////////////////////////////////////////////////*/
 
-    /// @param _gac is the global access control contract
-    constructor(address _gac) {
-        if (_gac == address(0)) {
+    /// @param _senderGAC is the global access control contract
+    constructor(address _senderGAC) {
+        if (_senderGAC == address(0)) {
             revert Error.ZERO_ADDRESS_INPUT();
         }
 
-        gac = IGAC(_gac);
+        senderGAC = MessageSenderGAC(_senderGAC);
     }
 
     /*/////////////////////////////////////////////////////////////////
@@ -62,15 +62,21 @@ abstract contract BaseSenderAdapter is IMessageSenderAdapter {
         }
 
         for (uint256 i; i < arrLength;) {
+            address oldReceiver = receiverAdapters[_dstChainIds[i]];
             receiverAdapters[_dstChainIds[i]] = _receiverAdapters[i];
-            emit ReceiverAdapterUpdated(_dstChainIds[i], _receiverAdapters[i]);
+            emit ReceiverAdapterUpdated(_dstChainIds[i], oldReceiver, _receiverAdapters[i]);
 
             unchecked {
                 ++i;
             }
         }
     }
 
+    /// @inheritdoc IMessageSenderAdapter
+    function getReceiverAdapter(uint256 _dstChainId) external view override returns (address) {
+        return receiverAdapters[_dstChainId];
+    }
+
     /*/////////////////////////////////////////////////////////////////
                             HELPER FUNCTIONS
     ////////////////////////////////////////////////////////////////*/