[VEN-2240]: Certik audit fixes on token bridge

contracts/Bridge/BaseXVSProxyOFT.sol

@@ -21,8 +21,8 @@
 
 abstract contract BaseXVSProxyOFT is Pausable, ExponentialNoError, BaseOFTV2 {
     using SafeERC20 for IERC20;
     IERC20 internal immutable innerToken;
     uint256 internal immutable ld2sdRate;
 
     /**
      * @notice The address of ResilientOracle contract wrapped in its interface.
@@ -61,12 +61,12 @@
      */
     mapping(uint16 => uint256) public chainIdToLast24HourReceiveWindowStart;
     /**
-     * @notice Address on which cap check and bound limit is not appicable.
+     * @notice Address on which cap check and bound limit is not applicable.
      */
     mapping(address => bool) public whitelist;
 
     /**
-     * @notice Emmited when address is added to whitelist.
+     * @notice Emitted when address is added to whitelist.
      */
     event SetWhitelist(address indexed addr, bool isWhitelist);
     /**
@@ -98,14 +98,26 @@
      */
     event InnerTokenAdded(address indexed innerToken);
 
+    /**
+     *@notice Emitted on sweep token success
+     */
+    event SweepToken(address indexed token, address indexed to, uint256 sweepAmount);
+
+    /**
+     *@notice Error thrown when this contract balance is less than sweep amount
+     */
+    error InsufficientBalance(uint256 sweepAmount, uint256 balance);
+
     /**
      * @param tokenAddress_ Address of the inner token.
-     * @param sharedDecimals_ No of shared decimals.
+     * @param sharedDecimals_ Number of shared decimals.
      * @param lzEndpoint_ Address of the layer zero endpoint contract.
      * @param oracle_ Address of the price oracle.
      * @custom:error ZeroAddressNotAllowed is thrown when token contract address is zero.
      * @custom:error ZeroAddressNotAllowed is thrown when lzEndpoint contract address is zero.
      * @custom:error ZeroAddressNotAllowed is thrown when oracle contract address is zero.
+     * @custom:event Emits InnerTokenAdded with token address.
+     * @custom:event Emits OracleChanged with zero address and oracle address.
      */
     constructor(
         address tokenAddress_,
@@ -115,18 +127,17 @@
     ) BaseOFTV2(sharedDecimals_, lzEndpoint_) {
         ensureNonzeroAddress(tokenAddress_);
         ensureNonzeroAddress(lzEndpoint_);
+        ensureNonzeroAddress(oracle_);
 
         innerToken = IERC20(tokenAddress_);
 
         (bool success, bytes memory data) = tokenAddress_.staticcall(abi.encodeWithSignature("decimals()"));
         require(success, "ProxyOFT: failed to get token decimals");
         uint8 decimals = abi.decode(data, (uint8));
 
         require(sharedDecimals_ <= decimals, "ProxyOFT: sharedDecimals must be <= decimals");
         ld2sdRate = 10 ** (decimals - sharedDecimals_);
 
-        ensureNonzeroAddress(oracle_);
-
         emit InnerTokenAdded(tokenAddress_);
         emit OracleChanged(address(0), oracle_);
 
@@ -154,7 +165,7 @@
      * @custom:event Emits SetMaxSingleTransactionLimit with old and new limit associated with chain id.
      */
     function setMaxSingleTransactionLimit(uint16 chainId_, uint256 limit_) external onlyOwner {
         require(limit_ <= chainIdToMaxDailyLimit[chainId_], "Single transaction limit > Daily limit");
         emit SetMaxSingleTransactionLimit(chainId_, chainIdToMaxSingleTransactionLimit[chainId_], limit_);
         chainIdToMaxSingleTransactionLimit[chainId_] = limit_;
     }
@@ -167,7 +178,7 @@
      * @custom:event Emits setMaxDailyLimit with old and new limit associated with chain id.
      */
     function setMaxDailyLimit(uint16 chainId_, uint256 limit_) external onlyOwner {
         require(limit_ >= chainIdToMaxSingleTransactionLimit[chainId_], "Daily limit < single transaction limit");
         emit SetMaxDailyLimit(chainId_, chainIdToMaxDailyLimit[chainId_], limit_);
         chainIdToMaxDailyLimit[chainId_] = limit_;
     }
@@ -180,7 +191,7 @@
      * @custom:event Emits setMaxSingleReceiveTransactionLimit with old and new limit associated with chain id.
      */
     function setMaxSingleReceiveTransactionLimit(uint16 chainId_, uint256 limit_) external onlyOwner {
         require(limit_ <= chainIdToMaxDailyReceiveLimit[chainId_], "single receive transaction limit > Daily limit");
         emit SetMaxSingleReceiveTransactionLimit(chainId_, chainIdToMaxSingleReceiveTransactionLimit[chainId_], limit_);
         chainIdToMaxSingleReceiveTransactionLimit[chainId_] = limit_;
     }
@@ -193,7 +204,7 @@
      * @custom:event Emits setMaxDailyReceiveLimit with old and new limit associated with chain id.
      */
     function setMaxDailyReceiveLimit(uint16 chainId_, uint256 limit_) external onlyOwner {
         require(
             limit_ >= chainIdToMaxSingleReceiveTransactionLimit[chainId_],
             "Daily limit < single receive transaction limit"
         );
@@ -203,7 +214,7 @@
 
     /**
      * @notice Sets the whitelist address to skip checks on transaction limit.
-     * @param user_ Adress to be add in whitelist.
+     * @param user_ Address to be add in whitelist.
      * @param val_ Boolean to be set (true for user_ address is whitelisted).
      * @custom:access Only owner.
      * @custom:event Emits setWhitelist.
@@ -229,27 +240,73 @@
         _unpause();
     }
 
+    /**
+     * @notice A public function to sweep accidental ERC-20 transfers to this contract. Tokens are sent to user
+     * @param token_ The address of the ERC-20 token to sweep
+     * @param to_ The address of the recipient
+     * @param amount_ The amount of tokens needs to transfer
+     * @custom:event Emits SweepToken event
+     * @custom:error Throw InsufficientBalance if amount_ is greater than the available balance of the token in the contract
+     * @custom:access Only Owner
+     */
+    function sweepToken(IERC20 token_, address to_, uint256 amount_) external onlyOwner {
+        uint256 balance = token_.balanceOf(address(this));
+        if (amount_ > balance) {
+            revert InsufficientBalance(amount_, balance);
+        }
+
+        emit SweepToken(address(token_), to_, amount_);
+
+        token_.safeTransfer(to_, amount_);
+    }
+
     /**
      * @notice Remove trusted remote from storage.
      * @param remoteChainId_ The chain's id corresponds to setting the trusted remote to empty.
+     * @custom:access Only owner.
+     * @custom:event Emits TrustedRemoteRemoved once chain id is removed from trusted remote.
      */
     function removeTrustedRemote(uint16 remoteChainId_) external onlyOwner {
         delete trustedRemoteLookup[remoteChainId_];
         emit TrustedRemoteRemoved(remoteChainId_);
     }
 
+    function retryMessage(
+        uint16 _srcChainId,
+        bytes calldata _srcAddress,
+        uint64 _nonce,
+        bytes calldata _payload
+    ) public payable override {
+        bytes memory trustedRemote = trustedRemoteLookup[_srcChainId];
+        // it will still block the message pathway from (srcChainId, srcAddress). should not receive message from untrusted remote.
+        require(
+            _srcAddress.length == trustedRemote.length &&
+                trustedRemote.length > 0 &&
+                keccak256(_srcAddress) == keccak256(trustedRemote),
+            "LzApp: invalid source sending contract"
+        );
+        super.retryMessage(_srcChainId, _srcAddress, _nonce, _payload);
+    }
+
     /**
      * @notice Empty implementation of renounce ownership to avoid any mishappening.
      */
     function renounceOwnership() public override {}
 
     /**
      * @notice Return's the address of the inner token of this bridge.
+     * @return Address of the inner token of this bridge.
      */
     function token() public view override returns (address) {
         return address(innerToken);
     }
 
+    /**
+     * @notice Checks if the sender is eligible to send tokens
+     * @param from_ Sender's address sending tokens
+     * @param dstChainId_ Chain id on which tokens should be sent
+     * @param amount_ Amount of tokens to be sent
+     */
     function _isEligibleToSend(address from_, uint16 dstChainId_, uint256 amount_) internal {
         // Check if the sender's address is whitelisted
         bool isWhiteListedUser = whitelist[from_];
@@ -288,6 +345,12 @@
         chainIdToLast24HourTransferred[dstChainId_] = transferredInWindow;
     }
 
+    /**
+     * @notice Checks if receiver is able to receive tokens
+     * @param toAddress_ Receiver address
+     * @param srcChainId_ Source chain id from which token is send
+     * @param receivedAmount_ Amount of tokens received
+     */
     function _isEligibleToReceive(address toAddress_, uint16 srcChainId_, uint256 receivedAmount_) internal {
         // Check if the recipient's address is whitelisted
         bool isWhiteListedUser = whitelist[toAddress_];
@@ -327,6 +390,13 @@
         chainIdToLast24HourReceived[srcChainId_] = receivedInWindow;
     }
 
+    /**
+     * @notice Transfer tokens from sender to receiver account.
+     * @param from_ Address from which token has to be transferred(Sender).
+     * @param to_ Address on which token will be tranferred(Receiver).
+     * @param amount_ Amount of token to be transferred.
+     * @return Actual balance difference.
+     */
     function _transferFrom(
         address from_,
         address to_,
@@ -341,23 +411,10 @@
         return innerToken.balanceOf(to_) - before;
     }
 
-    function _nonblockingLzReceive(
-        uint16 _srcChainId,
-        bytes memory _srcAddress,
-        uint64 _nonce,
-        bytes memory _payload
-    ) internal override {
-        bytes memory trustedRemote = trustedRemoteLookup[_srcChainId];
-        // if will still block the message pathway from (srcChainId, srcAddress). should not receive message from untrusted remote.
-        require(
-            _srcAddress.length == trustedRemote.length &&
-                trustedRemote.length > 0 &&
-                keccak256(_srcAddress) == keccak256(trustedRemote),
-            "LzApp: invalid source sending contract"
-        );
-        super._nonblockingLzReceive(_srcChainId, _srcAddress, _nonce, _payload);
-    }
-
+    /**
+     * @notice Returns Conversion rate factor from large decimals to shared decimals.
+     * @return Conversion rate factor.
+     */
     function _ld2sdRate() internal view override returns (uint256) {
         return ld2sdRate;
     }

contracts/Bridge/XVSBridgeAdmin.sol

@@ -21,7 +21,9 @@ contract XVSBridgeAdmin is AccessControlledV8 {
      */
     mapping(bytes4 => string) public functionRegistry;
 
-    // Event emitted when function registry updated
+    /**
+     * @notice emitted when function registry updated
+     */
     event FunctionRegistryChanged(string signature, bool active);
 
     /// @custom:oz-upgrades-unsafe-allow constructor
@@ -33,15 +35,14 @@ contract XVSBridgeAdmin is AccessControlledV8 {
 
     /**
      * @param accessControlManager_ Address of access control manager contract.
-     * @custom:error ZeroAddressNotAllowed is thrown when accessControlManager contract address is zero.
      */
     function initialize(address accessControlManager_) external initializer {
-        ensureNonzeroAddress(accessControlManager_);
         __AccessControlled_init(accessControlManager_);
     }
 
     /**
-     * @notice Invoked when called function does not exist in the contract
+     * @notice Invoked when called function does not exist in the contract.
+     * @return Response of low level call.
      * @custom:access Controlled by AccessControlManager.
      */
     fallback(bytes calldata data) external returns (bytes memory) {
@@ -54,8 +55,10 @@ contract XVSBridgeAdmin is AccessControlledV8 {
     }
 
     /**
+     * @notice Sets trusted remote on particular chain.
      * @param remoteChainId_ Chain Id of the destination chain.
      * @param remoteAddress_ Address of the destination bridge.
+     * @custom:access Controlled by AccessControlManager.
      * @custom:error ZeroAddressNotAllowed is thrown when remoteAddress_ contract address is zero.
      */
     function setTrustedRemoteAddress(uint16 remoteChainId_, bytes calldata remoteAddress_) external {
@@ -66,14 +69,16 @@ contract XVSBridgeAdmin is AccessControlledV8 {
     }
 
     /**
-     * @notice A registry of functions that are allowed to be executed from proposals
+     * @notice A setter for the registry of functions that are allowed to be executed from proposals.
      * @param signatures_  Function signature to be added or removed.
      * @param active_ bool value, should be true to add function.
+     * @custom:access Only owner.
+     * @custom:event Emits FunctionRegistryChanged if bool value of function changes.
      */
     function upsertSignature(string[] calldata signatures_, bool[] calldata active_) external onlyOwner {
         uint256 signatureLength = signatures_.length;
         require(signatureLength == active_.length, "Input arrays must have the same length");
-        for (uint256 i; i < signatureLength; i++) {
+        for (uint256 i; i < signatureLength; ) {
             bytes4 sigHash = bytes4(keccak256(bytes(signatures_[i])));
             bytes memory signature = bytes(functionRegistry[sigHash]);
             if (active_[i] && signature.length == 0) {
@@ -83,25 +88,28 @@ contract XVSBridgeAdmin is AccessControlledV8 {
                 delete functionRegistry[sigHash];
                 emit FunctionRegistryChanged(signatures_[i], false);
             }
+            unchecked {
+                ++i;
+            }
         }
     }
 
     /**
-     * @notice This function transfer the ownership of the bridge from this contract to new owner.
+     * @notice This function transfers the ownership of the bridge from this contract to new owner.
      * @param newOwner_ New owner of the XVS Bridge.
      * @custom:access Controlled by AccessControlManager.
      */
     function transferBridgeOwnership(address newOwner_) external {
         _checkAccessAllowed("transferBridgeOwnership(address)");
-        ensureNonzeroAddress(newOwner_);
         XVSBridge.transferOwnership(newOwner_);
     }
 
     /**
-     * @notice Returns bool = true if srcAddress_ is trustedRemote corresponds to chainId_.
+     * @notice Returns true if remote address is trustedRemote corresponds to chainId_.
      * @param remoteChainId_ Chain Id of the destination chain.
      * @param remoteAddress_ Address of the destination bridge.
      * @custom:error ZeroAddressNotAllowed is thrown when remoteAddress_ contract address is zero.
+     * @return Bool indicating whether the remote chain is trusted or not.
      */
     function isTrustedRemote(uint16 remoteChainId_, bytes calldata remoteAddress_) external returns (bool) {
         require(remoteChainId_ != 0, "ChainId must not be zero");
@@ -116,11 +124,18 @@ contract XVSBridgeAdmin is AccessControlledV8 {
 
     /**
      * @dev Returns function name string associated with function signature.
+     * @param signature_ Four bytes of function signature.
+     * @return Function signature corresponding to its hash.
      */
     function _getFunctionName(bytes4 signature_) internal view returns (string memory) {
         return functionRegistry[signature_];
     }
 
+    /**
+     * @notice Converts given bytes into address.
+     * @param b Bytes to be converted into address.
+     * @return Converted address of given bytes.
+     */
     function bytesToAddress(bytes calldata b) private pure returns (address) {
         return address(uint160(bytes20(b)));
     }

contracts/Bridge/XVSProxyOFTDest.sol

@@ -14,7 +14,7 @@ import { BaseXVSProxyOFT } from "./BaseXVSProxyOFT.sol";
 
 contract XVSProxyOFTDest is BaseXVSProxyOFT {
     /**
-     * @notice Emits when stored message dropped without successfull retrying.
+     * @notice Emits when stored message dropped without successful retrying.
      */
     event DropFailedMessage(uint16 srcChainId, bytes indexed srcAddress, uint64 nonce);
 
@@ -25,10 +25,13 @@ contract XVSProxyOFTDest is BaseXVSProxyOFT {
         address oracle_
     ) BaseXVSProxyOFT(tokenAddress_, sharedDecimals_, lzEndpoint_, oracle_) {}
 
-    /** @notice Clear failed messages from the storage.
+    /**
+     * @notice Clear failed messages from the storage.
      * @param srcChainId_ Chain id of source
      * @param srcAddress_ Address of source followed by current bridge address
      * @param nonce_ Nonce_ of the transaction
+     * @custom:access Only owner
+     * @custom:event Emits DropFailedMessage on clearance of failed message.
      */
     function dropFailedMessage(uint16 srcChainId_, bytes memory srcAddress_, uint64 nonce_) external onlyOwner {
         failedMessages[srcChainId_][srcAddress_][nonce_] = bytes32(0);
@@ -37,11 +40,19 @@ contract XVSProxyOFTDest is BaseXVSProxyOFT {
 
     /**
      * @notice Returns the total circulating supply of the token on the destination chain i.e (total supply).
+     * @return total circulating supply of the token on the destination chain.
      */
     function circulatingSupply() public view override returns (uint256) {
         return innerToken.totalSupply();
     }
 
+    /**
+     * @notice Debit tokens from the given address
+     * @param from_  Address from which tokens to be debited
+     * @param dstChainId_ Destination chain id
+     * @param amount_ Amount of tokens to be debited
+     * @return Actual amount debited
+     */
     function _debitFrom(
         address from_,
         uint16 dstChainId_,
@@ -54,6 +65,13 @@ contract XVSProxyOFTDest is BaseXVSProxyOFT {
         return amount_;
     }
 
+    /**
+     * @notice Credit tokens in the given account
+     * @param srcChainId_  Source chain id
+     * @param toAddress_ Address on which token will be credited
+     * @param amount_ Amount of tokens to be credited
+     * @return Actual amount credited
+     */
     function _creditTo(
         uint16 srcChainId_,
         address toAddress_,