Improve code comments

src/MultiBridgeMessageReceiver.sol

@@ -15,9 +15,9 @@ 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 MultiBridgeMessageSender.sol
+/// @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

src/MultiBridgeMessageSender.sol

@@ -25,8 +25,8 @@ contract MultiBridgeMessageSender {
     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
@@ -98,7 +98,7 @@ contract MultiBridgeMessageSender {
 
     /// @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();
         }
 

src/controllers/GovernanceTimelock.sol

@@ -21,7 +21,7 @@ contract GovernanceTimelock is IGovernanceTimelock {
                             STATE VARIABLES
     ////////////////////////////////////////////////////////////////*/
     uint256 public txCounter;
-    uint256 public delay = MINIMUM_DELAY;
+    uint256 public delay;
 
     /// @notice the admin is the one allowed to schedule events
     address public admin;

src/interfaces/IMultiBridgeMessageReceiver.sol

@@ -3,9 +3,9 @@ pragma solidity >=0.8.9;
 
 import "../libraries/Message.sol";
 
-/// @dev interface for the multi-bridge message receiver
+/// @notice interface for the multi-bridge message receiver
 interface IMultiBridgeMessageReceiver {
-    /// @notice encapsulates data that is relevant to a message's intended transaction execution
+    /// @notice encapsulates data that is relevant to a message's intended transaction execution.
     struct ExecutionData {
         // target contract address on the destination chain
         address target;
@@ -19,7 +19,7 @@ interface IMultiBridgeMessageReceiver {
         uint256 expiration;
     }
 
-    /// @notice emitted when a message has been received from a single bridge
+    /// @notice emitted when a message has been received from a single bridge.
     /// @param msgId is the unique identifier of the message
     /// @param bridgeName is the name of the bridge from which the message was received
     /// @param nonce is the nonce of the message
@@ -28,7 +28,7 @@ interface IMultiBridgeMessageReceiver {
         bytes32 indexed msgId, string indexed bridgeName, uint256 nonce, address receiverAdapter
     );
 
-    /// @notice emitted when a message has been queued for execution in the destination timelock contract
+    /// @notice emitted when a message has been queued for execution in the destination timelock contract.
     /// @param msgId is the unique identifier of the message
     /// @param target is the address of the final target address that will be called once the timelock matures
     /// @param nativeValue is the value that will be passed to the target address through low-level call
@@ -38,12 +38,12 @@ interface IMultiBridgeMessageReceiver {
         bytes32 indexed msgId, address indexed target, uint256 nativeValue, uint256 nonce, bytes callData
     );
 
-    /// @notice emitted when receiver adapter of a specific bridge is updated
+    /// @notice emitted when receiver adapter of a specific bridge is updated.
     /// @param receiverAdapter is the new receiver adapter address
     /// @param add is true if the receiver adapter was added, false if removed
     event BridgeReceiverAdapterUpdated(address indexed receiverAdapter, bool add);
 
-    /// @notice emitted when the quorum for message validity is updated
+    /// @notice emitted when the quorum for message validity is updated.
     /// @param oldQuorum is the old quorum value
     /// @param newQuorum is the new quorum value
     event QuorumUpdated(uint64 oldQuorum, uint64 newQuorum);
@@ -53,23 +53,23 @@ interface IMultiBridgeMessageReceiver {
     /// @param _message is the message to be received
     function receiveMessage(MessageLibrary.Message calldata _message) external;
 
-    /// @notice Sends a message, that has achieved quorum and has not yet expired, to the governance timelock for eventual execution
+    /// @notice Sends a message, that has achieved quorum and has not yet expired, to the governance timelock for eventual execution.
     /// @param _msgId is the unique identifier of the message
     function executeMessage(bytes32 _msgId) external;
 
-    /// @notice adds or removes bridge receiver adapters
+    /// @notice adds or removes bridge receiver adapters.
     /// @param _receiverAdapters the list of receiver adapters to add or remove
-    /// @param _operations the list of operations to perform for corresponding receiver adapters. true for add, false for remove
+    /// @param _operations the list of operations to perform for corresponding receiver adapters, true for add, false for remove
     function updateReceiverAdapters(address[] calldata _receiverAdapters, bool[] calldata _operations) external;
 
     /// @notice updates the quorum for message validity, which is the number of bridges that must deliver the message for it to be considered valid.
     /// @param _quorum is the new quorum value
     function updateQuorum(uint64 _quorum) external;
 
-    /// @notice updates the the list of receiver adapters and the quorum for message validity
+    /// @notice updates the the list of receiver adapters and the quorum for message validity.
     /// @param _newQuorum is the new quorum value
     /// @param _receiverAdapters the list of receiver adapters to add or remove
-    /// @param _operations the list of operations to perform for corresponding receiver adapters. true for add, false for remove
+    /// @param _operations the list of operations to perform for corresponding receiver adapters, true for add, false for remove
     function updateQuorumAndReceiverAdapter(
         uint64 _newQuorum,
         address[] calldata _receiverAdapters,

src/interfaces/adapters/IMessageReceiverAdapter.sol

@@ -3,21 +3,21 @@ pragma solidity >=0.8.9;
 
 import "../EIP5164/MessageExecutor.sol";
 
-//// @dev interface for message receiver adapters
+/**
+ * @notice A common interface for AMB message receiver adapters, that builds on EIP-5164 MessageExecutor.
+ * A message receiver adapter receives messages through the AMB that are sent by a specific sender adapter on the source chain.
+ * It validates the message and forwards it to the IMultiBridgeMessageReceiver contract on the destination chain.
+ */
 interface IMessageReceiverAdapter is MessageExecutor {
-    /*/////////////////////////////////////////////////////////////////
-                                EVENTS
-    ////////////////////////////////////////////////////////////////*/
+    /// @notice emitted when the sender adapter address, which resides on the source chain, is updated.
+    /// @param oldSenderAdapter is the old sender adapter address
+    /// @param newSenderAdapter is the new sender adapter address
     event SenderAdapterUpdated(address indexed oldSenderAdapter, address indexed newSenderAdapter);
 
-    /*/////////////////////////////////////////////////////////////////
-                            EXTERNAL FUNCTIONS
-    ////////////////////////////////////////////////////////////////*/
-
-    /// @dev returns name of the message bridge wrapped by the adapter
+    /// @notice returns name of the message bridge wrapped by the adapter
     function name() external view returns (string memory);
 
-    /// @dev allows the admin to update the sender adapter
+    /// @dev Changes the address of the sender adapter on the source chain, that is authorised to send this receiver messages.
     /// @param _senderAdapter is the bridge's sender adapter deployed on the source chain (i.e. Ethereum)
     /// note: access controlled to be called by the global admin contract
     function updateSenderAdapter(address _senderAdapter) external;

src/interfaces/adapters/IMessageSenderAdapter.sol

@@ -4,17 +4,22 @@ pragma solidity >=0.8.9;
 
 import "../EIP5164/SingleMessageDispatcher.sol";
 
-/// @dev common interface for all message sender adapters of messaging bridges
+/**
+ * @notice A common interface for AMB message sender adapters, that builds on EIP-5164 SingleMessageDispatcher.
+ * A message sender adapter for a specific AMB is responsible for receiving messages from an MultiBridgeMessageSender
+ * on the source chain and sending the message to corresponding receiver adapters on the intended destination chain.
+ * The sender adapter keeps a list of the remote receiver adapters on each destination chain that it forwards messages to.
+ */
 interface IMessageSenderAdapter is SingleMessageDispatcher {
-    /// @notice emitted when a the sender's corresponding receiver adapter on a remote chain is changed
+    /// @notice emitted when a the sender's corresponding receiver adapter on a destination chain is changed
     /// @param  dstChainId is the destination chain for which the receiver adapter is updated
     /// @param  oldReceiver is the old receiver adapter address
     /// @param  newReceiver is the new receiver adapter address
     event ReceiverAdapterUpdated(uint256 indexed dstChainId, address indexed oldReceiver, address indexed newReceiver);
 
-    /// @notice allows owner to update the receiver adapters on different destination chains
+    /// @notice Updates the corresponding message receiver adapters for different destination chains
     /// @param _dstChainIds are the destination chain IDs for which the receiver adapters are to be updated
-    /// @param _receiverAdapters receiver adapter addresses for the corresponding destination chain ids in _dstChainIds
+    /// @param _receiverAdapters new receiver adapter addresses for the corresponding destination chain ids in _dstChainIds
     function updateReceiverAdapter(uint256[] calldata _dstChainIds, address[] calldata _receiverAdapters) external;
 
     /// @notice returns name of the message bridge wrapped by the adapter

src/interfaces/controllers/IGAC.sol

@@ -4,12 +4,12 @@ pragma solidity >=0.8.9;
 
 /// @notice interface for GAC (Global Access Controller)
 interface IGAC {
-    /// @dev returns `true` if the caller is global access controller
-    /// @param _caller is the msg.sender to validate
-    /// @return boolean indicating the validity of the caller
-    function isGlobalOwner(address _caller) external view returns (bool);
+    /// @notice Checks whether a given address is the global owner
+    /// @param _addr is the address to compare with the global owner
+    /// @return boolean true if _caller is the global owner, false otherwise
+    function isGlobalOwner(address _addr) external view returns (bool);
 
-    /// @dev returns the global owner address
+    /// @notice returns the global owner address.
     /// @return _owner is the global owner address
     function getGlobalOwner() external view returns (address _owner);
 }

src/interfaces/controllers/IGovernanceTimelock.sol

@@ -1,44 +1,65 @@
 // SPDX-License-Identifier: GPL-3.0-only
 pragma solidity >=0.8.9;
 
-/// @dev interface for governance timelock before execution events on dst chain
+/**
+ * @notice interface for governance timelock on the destination chain.
+ * @dev this contract is responsible for executing all cross-chain messages on the destination chain
+ * after they have been queued for a configurable period of time.
+ * This contract is also typically, the GAC (Global Access Controller) on the destination chain, ensuring that all
+ * admin actions on the destination chain go through the same validation process and time delay as other governance actions.
+ */
 interface IGovernanceTimelock {
-    /*/////////////////////////////////////////////////////////////////
-                                EVENTS
-    ////////////////////////////////////////////////////////////////*/
+    /// @notice emitted when a transaction is scheduled for execution.
+    /// @param txId is the generated unique identifier of the scheduled transaction
+    /// @param target is the address to call by low-level call
+    /// @param value is the value to pass to target by low-level call
+    /// @param data is the abieencoded function selector and arguments data, to execute on target
+    /// @param eta is the timestamp after which the transaction should be executed
     event TransactionScheduled(uint256 indexed txId, address indexed target, uint256 value, bytes data, uint256 eta);
+
+    /// @notice emitted when a transaction is executed.
+    /// @param txId is the unique identifier of the executed transaction, generated when the transaction was scheduled
+    /// @param target is the address called as part of the execution
+    /// @param value is the value passed to target by low-level call
+    /// @param data is the abieencoded function selector and arguments data, executed on target
+    /// @param eta is the timestamp after which the transaction should be executed
     event TransactionExecuted(uint256 indexed txId, address indexed target, uint256 value, bytes data, uint256 eta);
 
-    event ExecutionPeriodUpdated(uint256 oldPeriod, uint256 newPeriod);
+    /// @notice emitted when the time delay parameter is changed.
+    /// @param oldDelay is the previous value of the time delay
+    /// @param newDelay is the new value of the time delay
     event DelayUpdated(uint256 oldDelay, uint256 newDelay);
-    event AdminUpdated(address oldAdmin, address newAdmin);
 
-    /*/////////////////////////////////////////////////////////////////
-                            EXTERNAL FUNCTIONS
-    ////////////////////////////////////////////////////////////////*/
+    /// @notice emitted when the admin of the time lock contract is changed
+    /// @param oldAdmin is the previous admin of the time lock contract
+    /// @param newAdmin is the new admin of the time lock contract
+    event AdminUpdated(address oldAdmin, address newAdmin);
 
-    /// @notice Schedules the provided transaction for execution after a specified ETA.
-    /// @dev this function can only be called by an authorised bridge adapter on the remote chain
-    /// @param _target the contract to call
-    /// @param _value the amount to pass when calling target
-    /// @param _data the abieencoded function selector and arguments data, to execute on target
+    /// @notice Schedules the provided transaction for execution after a pre-configured delay period.
+    /// @dev this function can only be called by the admin of the timelock contract.
+    /// @param _target is the address to call by low-level call
+    /// @param _value is the value to pass to target by low-level call
+    /// @param _data is the abieencoded function selector and arguments data, to execute on target
     function scheduleTransaction(address _target, uint256 _value, bytes memory _data) external;
 
-    /// @notice Executes a previously scheduled transaction if it has reached its ETA.
-    /// @param _txId is the unqiue identifier of the scheduled transaction
-    /// @param _target the contract to call
-    /// @param _value the amount to pass when calling target
-    /// @param _data the abieencoded function selector and arguments data, to execute on target
-    /// @param _eta the time after which the transaction should be executed
+    /// @notice Executes a previously scheduled transaction if it has reached its ETA, but has not exceeded a grace period beyond that.
+    /// @param _txId is the unique identifier of the executed transaction, generated when the transaction was scheduled
+    /// @param _target is the address called as part of the execution
+    /// @param _value is the value passed to target by low-level call
+    /// @param _data is the abieencoded function selector and arguments data, executed on target
+    /// @param _eta is the timestamp after which the transaction should be executed
     function executeTransaction(uint256 _txId, address _target, uint256 _value, bytes memory _data, uint256 _eta)
         external
         payable;
 
-    /// @notice Updates the minimum delay for a transaction before it can be executed.
-    /// @dev This can only be invoked by the timelock contract itself, thus requiring that this change go through the same time delay as other governance actions.
+    /// @notice Changes the time period that transactions must be queued for before they can be executed.
+    /// The new delay must still be within an allowed range.
+    /// This function can only be invoked by the timelock contract itself, thus requiring that this change go
+    /// through the process and time delays as other governance actions.
     function setDelay(uint256 _delay) external;
 
-    /// @notice Updates the admin.
-    /// @dev This can only be invoked by the timelock contract itself, thus requiring that this change go through the same time delay as other governance actions.
+    /// @notice Changes the admin.
+    /// This function can only be invoked by the timelock contract itself, thus requiring that this change go
+    /// through the process and time delays as other governance actions.
     function setAdmin(address newAdmin) external;
 }

test/unit-tests/GovernanceTimelock.t.sol

@@ -13,7 +13,6 @@ contract GovernanceTimelockTest is Setup {
     event TransactionScheduled(uint256 indexed txId, address indexed target, uint256 value, bytes data, uint256 eta);
     event TransactionExecuted(uint256 indexed txId, address indexed target, uint256 value, bytes data, uint256 eta);
 
-    event ExecutionPeriodUpdated(uint256 oldPeriod, uint256 newPeriod);
     event DelayUpdated(uint256 oldDelay, uint256 newDelay);
     event AdminUpdated(address oldAdmin, address newAdmin);
 

test/unit-tests/MultiBridgeMessageSender.t.sol

@@ -183,8 +183,8 @@ contract MultiBridgeMessageSenderTest is Setup {
 
     /// @dev message expiration has to be within allowed range
     function test_remote_call_invalid_expiration() public {
-        uint256 invalidExpMin = sender.MIN_EXPIRATION() - 1 days;
-        uint256 invalidExpMax = sender.MAX_EXPIRATION() + 1 days;
+        uint256 invalidExpMin = sender.MIN_MESSAGE_EXPIRATION() - 1 days;
+        uint256 invalidExpMax = sender.MAX_MESSAGE_EXPIRATION() + 1 days;
 
         // test expiration validation in remoteCall() which does not accept excluded adapters
         vm.startPrank(caller);