Merge branch 'main' into rm-estimate-fee

Address review comments

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
 /// 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),
+/// if it has been delivered by enough trusted bridge receiver adapters (i.e. has achieved a configured 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
     ////////////////////////////////////////////////////////////////*/
@@ -102,12 +102,7 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
 
     /// @notice receive messages from allowed bridge receiver adapters
     /// @param _message is the crosschain message sent by the mma sender
-    /// @param _bridgeName is the name of the bridge that relays the message
-    function receiveMessage(MessageLibrary.Message calldata _message, string memory _bridgeName)
-        external
-        override
-        onlyReceiverAdapter
-    {
+    function receiveMessage(MessageLibrary.Message calldata _message) external override onlyReceiverAdapter {
         if (_message.dstChainId != block.chainid) {
             revert Error.INVALID_DST_CHAIN();
         }
@@ -121,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]) {
@@ -147,13 +142,12 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
             );
         }
 
-        emit SingleBridgeMsgReceived(msgId, _bridgeName, _message.nonce, msg.sender);
+        string memory bridgeName = IMessageReceiverAdapter(msg.sender).name();
+        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
@@ -185,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);
@@ -196,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);
 
@@ -205,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);
     }
 
@@ -285,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,7 +16,7 @@ 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 {
     /*/////////////////////////////////////////////////////////////////
                                     STRUCTS
     ////////////////////////////////////////////////////////////////*/
@@ -36,11 +36,11 @@ contract MultiMessageSender {
     //////////////////////////////////////////////////////////////*/
 
     /// @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;
-    uint256 public constant MAX_EXPIRATION = 30 days;
+    uint256 public constant MIN_MESSAGE_EXPIRATION = 2 days;
+    uint256 public constant MAX_MESSAGE_EXPIRATION = 30 days;
 
     /*/////////////////////////////////////////////////////////////////
                             STATE VARIABLES
@@ -66,10 +66,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,
@@ -97,23 +97,23 @@ 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();
         }
         _;
     }
 
     /// @dev validates the expiration provided by the user
     modifier validateExpiration(uint256 _expiration) {
-        if (_expiration < MIN_EXPIRATION || _expiration > MAX_EXPIRATION) {
+        if (_expiration < MIN_MESSAGE_EXPIRATION || _expiration > MAX_MESSAGE_EXPIRATION) {
             revert Error.INVALID_EXPIRATION_DURATION();
         }
 
@@ -124,13 +124,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);
     }
 
     /*/////////////////////////////////////////////////////////////////
@@ -141,12 +141,12 @@ contract MultiMessageSender {
     /// via all available bridges. This function can only be called by the authorised called configured in the GAC.
     /// @dev a fee in native token may be required by each message bridge to send messages. Any native token fee remained
     /// will be refunded back to a refund address defined in the _refundAddress parameter.
-    /// Caller can use estimateTotalMessageFee() to get total message fees before calling this function.
+    /// Caller needs to specify fees for each adapter when calling this function.
     /// @param _dstChainId is the destination chainId
     /// @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
     /// @param _fees refers to the fees to pay to each adapter
     function remoteCall(
@@ -169,7 +169,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
     /// @param _fees refers to the fees to pay to each adapter
     /// @param _excludedAdapters are the sender adapters to be excluded from relaying the message, in ascending order by address
@@ -263,7 +263,7 @@ contract MultiMessageSender {
     ////////////////////////////////////////////////////////////////*/
 
     function _remoteCall(RemoteCallArgs memory _args) private {
-        (address mmaReceiver, address[] memory adapters) = _checkRemoteCallArgs(
+        (address mmaReceiver, address[] memory adapters) = _checkAndProcessArgs(
             _args.dstChainId, _args.target, _args.refundAddress, _args.fees, _args.excludedAdapters
         );
 
@@ -282,7 +282,7 @@ contract MultiMessageSender {
         bytes32 msgId = MessageLibrary.computeMsgId(message);
         bool[] memory adapterSuccess = _dispatchMessages(adapters, mmaReceiver, _args.dstChainId, message, _args.fees);
 
-        emit MultiMessageSent(
+        emit MultiBridgeMessageSent(
             msgId,
             nonce,
             _args.dstChainId,
@@ -300,7 +300,7 @@ contract MultiMessageSender {
         }
     }
 
-    function _checkRemoteCallArgs(
+    function _checkAndProcessArgs(
         uint256 _dstChainId,
         address _target,
         address _refundAddress,
@@ -323,7 +323,7 @@ contract MultiMessageSender {
             revert Error.INVALID_REFUND_ADDRESS();
         }
 
-        mmaReceiver = gac.getMultiMessageReceiver(_dstChainId);
+        mmaReceiver = senderGAC.getRemoteMultiBridgeMessageReceiver(_dstChainId);
 
         if (mmaReceiver == address(0)) {
             revert Error.ZERO_RECEIVER_ADAPTER();
@@ -337,6 +337,16 @@ contract MultiMessageSender {
         if (adapters.length != _fees.length) {
             revert Error.INVALID_SENDER_ADAPTER_FEES();
         }
+        uint256 totalFees;
+        for (uint256 i; i < _fees.length;) {
+            totalFees += _fees[i];
+            unchecked {
+                ++i;
+            }
+        }
+        if (totalFees > msg.value) {
+            revert Error.INVALID_MSG_VALUE();
+        }
     }
 
     function _dispatchMessages(

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);
+    }
+}