Merge pull request #314 from consenlabs/chore/increase-test-coverage-…

…and-add-natspec

Chore/increase test coverage and add natspec

contracts/AllowanceTarget.sol

@@ -8,22 +8,33 @@ 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();
     }

contracts/CoordinatedTaker.sol

@@ -14,15 +14,24 @@ import { SignatureValidator } from "./libraries/SignatureValidator.sol";
 
 /// @title CoordinatedTaker Contract
 /// @author imToken Labs
+/// @notice This contract allows for the coordinator of limit order fills through a designated coordinator.
 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 +45,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,
@@ -59,15 +73,15 @@ contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector,
             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 +94,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

@@ -9,27 +9,32 @@ 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
+    /// @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,
@@ -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,