Merge pull request #313 from consenlabs/chore/cleanup-and-upgrade-cod…

…ebase

Chore: cleanup and upgrade codebase

.gitmodules

@@ -1,7 +1,7 @@
 [submodule "lib/openzeppelin-contracts"]
 	path = lib/openzeppelin-contracts
-	url = https://github.com/openzeppelin/openzeppelin-contracts
-	branch = v4.9.2
+	url = https://github.com/OpenZeppelin/openzeppelin-contracts
+	branch = v5.0.2
 [submodule "lib/forge-std"]
 	path = lib/forge-std
 	url = https://github.com/foundry-rs/forge-std

contracts/AllowanceTarget.sol

@@ -1,35 +1,46 @@
 // SPDX-License-Identifier: MIT
-pragma solidity 0.8.17;
+pragma solidity 0.8.26;
 
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
-import { Pausable } from "@openzeppelin/contracts/security/Pausable.sol";
+import { Pausable } from "@openzeppelin/contracts/utils/Pausable.sol";
 
 import { Ownable } from "./abstracts/Ownable.sol";
 import { IAllowanceTarget } from "./interfaces/IAllowanceTarget.sol";
 
+/// @title AllowanceTarget Contract
+/// @author imToken Labs
+/// @notice This contract manages allowances and authorizes spenders to transfer tokens on behalf of users.
 contract AllowanceTarget is IAllowanceTarget, Pausable, Ownable {
     using SafeERC20 for IERC20;
 
-    mapping(address => bool) public authorized;
+    /// @notice Mapping of authorized addresses permitted to call spendFromUserTo.
+    mapping(address trustedCaller => bool isAuthorized) public authorized;
 
+    /// @notice Constructor to initialize the contract with the owner and trusted callers.
+    /// @param _owner The address of the contract owner.
+    /// @param trustedCaller An array of addresses that are initially authorized to call spendFromUserTo.
     constructor(address _owner, address[] memory trustedCaller) Ownable(_owner) {
         uint256 callerCount = trustedCaller.length;
         for (uint256 i = 0; i < callerCount; ++i) {
             authorized[trustedCaller[i]] = true;
         }
     }
 
+    /// @notice Pauses the contract, preventing the execution of spendFromUserTo.
+    /// @dev Only the owner can call this function.
     function pause() external onlyOwner {
         _pause();
     }
 
+    /// @notice Unpauses the contract, allowing the execution of spendFromUserTo.
+    /// @dev Only the owner can call this function.
     function unpause() external onlyOwner {
         _unpause();
     }
 
     /// @inheritdoc IAllowanceTarget
-    function spendFromUserTo(address from, address token, address to, uint256 amount) external override whenNotPaused {
+    function spendFromUserTo(address from, address token, address to, uint256 amount) external whenNotPaused {
         if (!authorized[msg.sender]) revert NotAuthorized();
         IERC20(token).safeTransferFrom(from, to, amount);
     }

contracts/CoordinatedTaker.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity 0.8.17;
+pragma solidity 0.8.26;
 
 import { TokenCollector } from "./abstracts/TokenCollector.sol";
 import { AdminManagement } from "./abstracts/AdminManagement.sol";
@@ -14,15 +14,26 @@ import { SignatureValidator } from "./libraries/SignatureValidator.sol";
 
 /// @title CoordinatedTaker Contract
 /// @author imToken Labs
+/// @notice This contract is a taker contract for the LimitOrderSwap.
+/// @dev It helps users avoid collisions when filling a limit order and provides an off-chain order canceling mechanism.
+/// For more details, check the reference: https://github.com/consenlabs/tokenlon-contracts/blob/v6.0.1/doc/CoordinatedTaker.md
 contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector, EIP712 {
     using Asset for address;
 
     IWETH public immutable weth;
     ILimitOrderSwap public immutable limitOrderSwap;
     address public coordinator;
 
-    mapping(bytes32 => bool) public allowFillUsed;
+    /// @notice Mapping to keep track of used allow fill hashes.
+    mapping(bytes32 allowFillHash => bool isUsed) public allowFillUsed;
 
+    /// @notice Constructor to initialize the contract with the owner, Uniswap permit2, allowance target, WETH, coordinator and LimitOrderSwap contract.
+    /// @param _owner The address of the contract owner.
+    /// @param _uniswapPermit2 The address for Uniswap permit2.
+    /// @param _allowanceTarget The address for the allowance target.
+    /// @param _weth The WETH contract instance.
+    /// @param _coordinator The initial coordinator address.
+    /// @param _limitOrderSwap The LimitOrderSwap contract address.
     constructor(
         address _owner,
         address _uniswapPermit2,
@@ -36,15 +47,20 @@ contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector,
         limitOrderSwap = _limitOrderSwap;
     }
 
+    /// @notice Receive function to receive ETH.
     receive() external payable {}
 
+    /// @notice Sets a new coordinator address.
+    /// @dev Only the owner can call this function.
+    /// @param _newCoordinator The address of the new coordinator.
     function setCoordinator(address _newCoordinator) external onlyOwner {
         if (_newCoordinator == address(0)) revert ZeroAddress();
         coordinator = _newCoordinator;
 
         emit SetCoordinator(_newCoordinator);
     }
 
+    /// @inheritdoc ICoordinatedTaker
     function submitLimitOrderFill(
         LimitOrder calldata order,
         bytes calldata makerSignature,
@@ -53,21 +69,21 @@ contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector,
         bytes calldata extraAction,
         bytes calldata userTokenPermit,
         CoordinatorParams calldata crdParams
-    ) external payable override {
+    ) external payable {
         // validate fill permission
         {
             if (crdParams.expiry < block.timestamp) revert ExpiredPermission();
 
             bytes32 orderHash = getLimitOrderHash(order);
-
             bytes32 allowFillHash = getEIP712Hash(
                 getAllowFillHash(
                     AllowFill({ orderHash: orderHash, taker: msg.sender, fillAmount: makerTokenAmount, salt: crdParams.salt, expiry: crdParams.expiry })
                 )
             );
-            if (!SignatureValidator.validateSignature(coordinator, allowFillHash, crdParams.sig)) revert InvalidSignature();
 
+            if (!SignatureValidator.validateSignature(coordinator, allowFillHash, crdParams.sig)) revert InvalidSignature();
             if (allowFillUsed[allowFillHash]) revert ReusedPermission();
+
             allowFillUsed[allowFillHash] = true;
 
             emit CoordinatorFill({ user: msg.sender, orderHash: orderHash, allowFillHash: allowFillHash });
@@ -80,7 +96,7 @@ contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector,
         }
 
         // send order to limit order contract
-        // use fullOrKill since coordinator should manage fill amount distribution
+        // use fillLimitOrderFullOrKill since coordinator should manage fill amount distribution
         limitOrderSwap.fillLimitOrderFullOrKill{ value: msg.value }(
             order,
             makerSignature,

contracts/GenericSwap.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity 0.8.17;
+pragma solidity 0.8.26;
 
 import { TokenCollector } from "./abstracts/TokenCollector.sol";
 import { EIP712 } from "./abstracts/EIP712.sol";
@@ -9,33 +9,38 @@ import { GenericSwapData, getGSDataHash } from "./libraries/GenericSwapData.sol"
 import { Asset } from "./libraries/Asset.sol";
 import { SignatureValidator } from "./libraries/SignatureValidator.sol";
 
+/// @title GenericSwap Contract
+/// @author imToken Labs
+/// @notice This contract facilitates token swaps using SmartOrderStrategy strategies.
 contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
     using Asset for address;
 
-    mapping(bytes32 => bool) private filledSwap;
+    /// @notice Mapping to keep track of filled swaps.
+    /// @dev Stores the status of swaps to ensure they are not filled more than once.
+    mapping(bytes32 swapHash => bool isFilled) public filledSwap;
 
+    /// @notice Constructor to initialize the contract with the permit2 and allowance target.
+    /// @param _uniswapPermit2 The address for Uniswap permit2.
+    /// @param _allowanceTarget The address for the allowance target.
     constructor(address _uniswapPermit2, address _allowanceTarget) TokenCollector(_uniswapPermit2, _allowanceTarget) {}
 
+    /// @notice Receive function to receive ETH.
     receive() external payable {}
 
-    /// @param swapData Swap data
-    /// @return returnAmount Output amount of the swap
-    function executeSwap(GenericSwapData calldata swapData, bytes calldata takerTokenPermit) external payable override returns (uint256 returnAmount) {
+    /// @inheritdoc IGenericSwap
+    function executeSwap(GenericSwapData calldata swapData, bytes calldata takerTokenPermit) external payable returns (uint256 returnAmount) {
         returnAmount = _executeSwap(swapData, msg.sender, takerTokenPermit);
 
         _emitGSExecuted(getGSDataHash(swapData), swapData, msg.sender, returnAmount);
     }
 
-    /// @param swapData Swap data
-    /// @param taker Claimed taker address
-    /// @param takerSig Taker signature
-    /// @return returnAmount Output amount of the swap
+    /// @inheritdoc IGenericSwap
     function executeSwapWithSig(
         GenericSwapData calldata swapData,
         bytes calldata takerTokenPermit,
         address taker,
         bytes calldata takerSig
-    ) external payable override returns (uint256 returnAmount) {
+    ) external payable returns (uint256 returnAmount) {
         bytes32 swapHash = getGSDataHash(swapData);
         bytes32 gs712Hash = getEIP712Hash(swapHash);
         if (filledSwap[swapHash]) revert AlreadyFilled();
@@ -47,6 +52,11 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
         _emitGSExecuted(swapHash, swapData, taker, returnAmount);
     }
 
+    /// @notice Executes a generic swap.
+    /// @param _swapData The swap data containing details of the swap.
+    /// @param _authorizedUser The address authorized to execute the swap.
+    /// @param _takerTokenPermit The permit for the taker token.
+    /// @return returnAmount The output amount of the swap.
     function _executeSwap(
         GenericSwapData calldata _swapData,
         address _authorizedUser,
@@ -78,6 +88,11 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
         _outputToken.transferTo(_swapData.recipient, returnAmount);
     }
 
+    /// @notice Emits the Swap event after executing a generic swap.
+    /// @param _gsOfferHash The hash of the generic swap offer.
+    /// @param _swapData The swap data containing details of the swap.
+    /// @param _taker The address of the taker.
+    /// @param returnAmount The output amount of the swap.
     function _emitGSExecuted(bytes32 _gsOfferHash, GenericSwapData calldata _swapData, address _taker, uint256 returnAmount) internal {
         emit Swap(
             _gsOfferHash,