diff --git a/contracts/AllowanceTarget.sol b/contracts/AllowanceTarget.sol index ce7d43c4..4760dd38 100644 --- a/contracts/AllowanceTarget.sol +++ b/contracts/AllowanceTarget.sol @@ -8,11 +8,18 @@ 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) { @@ -20,10 +27,14 @@ contract AllowanceTarget is IAllowanceTarget, Pausable, Ownable { } } + /// @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(); } diff --git a/contracts/CoordinatedTaker.sol b/contracts/CoordinatedTaker.sol index efaa1d8a..5c78a572 100644 --- a/contracts/CoordinatedTaker.sol +++ b/contracts/CoordinatedTaker.sol @@ -14,6 +14,7 @@ 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; @@ -21,8 +22,16 @@ contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector, 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,8 +45,12 @@ 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; @@ -45,6 +58,7 @@ contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector, 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, diff --git a/contracts/GenericSwap.sol b/contracts/GenericSwap.sol index 48dfdf23..e4c429f8 100644 --- a/contracts/GenericSwap.sol +++ b/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, diff --git a/contracts/LimitOrderSwap.sol b/contracts/LimitOrderSwap.sol index 3d52f072..15cbfb0a 100644 --- a/contracts/LimitOrderSwap.sol +++ b/contracts/LimitOrderSwap.sol @@ -16,17 +16,26 @@ import { SignatureValidator } from "./libraries/SignatureValidator.sol"; /// @title LimitOrderSwap Contract /// @author imToken Labs +/// @notice This contract allows users to execute limit orders for token swaps contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, ReentrancyGuard { using Asset for address; + /// @dev Mask used to mark order cancellation in `orderHashToMakerTokenFilledAmount`. + /// The left-most bit (bit 255) of `orderHashToMakerTokenFilledAmount[orderHash]` represents order cancellation. uint256 private constant ORDER_CANCEL_AMOUNT_MASK = 1 << 255; IWETH public immutable weth; address payable public feeCollector; - // how much maker token has been filled in an order - mapping(bytes32 => uint256) public orderHashToMakerTokenFilledAmount; + /// @notice Mapping to track the filled amounts of maker tokens for each order hash. + mapping(bytes32 orderHash => uint256 orderFilledAmount) public orderHashToMakerTokenFilledAmount; + /// @notice Constructor to initialize the contract with the owner, Uniswap permit2, allowance target, WETH, and fee collector. + /// @param _owner The address of the contract owner. + /// @param _uniswapPermit2 The address of the Uniswap permit2. + /// @param _allowanceTarget The address of the allowance target. + /// @param _weth The WETH token instance. + /// @param _feeCollector The initial address of the fee collector. constructor( address _owner, address _uniswapPermit2, @@ -39,10 +48,12 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree feeCollector = _feeCollector; } + /// @notice Receive function to receive ETH. receive() external payable {} - /// @notice Only owner can call - /// @param _newFeeCollector The new address of fee collector + /// @notice Sets a new fee collector address. + /// @dev Only the owner can call this function. + /// @param _newFeeCollector The new address of the fee collector. function setFeeCollector(address payable _newFeeCollector) external onlyOwner { if (_newFeeCollector == address(0)) revert ZeroAddress(); feeCollector = _newFeeCollector; @@ -80,17 +91,21 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree for (uint256 i = 0; i < orders.length; ++i) { LimitOrder calldata order = orders[i]; uint256 makingAmount = makerTokenAmounts[i]; + if (makingAmount == 0) revert ZeroMakerSpendingAmount(); (bytes32 orderHash, uint256 orderFilledAmount) = _validateOrder(order, makerSignatures[i]); { - uint256 orderAvailableAmount = order.makerTokenAmount - orderFilledAmount; + uint256 orderAvailableAmount; + unchecked { + // orderAvailableAmount must be greater than 0 here, or it will be reverted by the _validateOrder function + orderAvailableAmount = order.makerTokenAmount - orderFilledAmount; + } if (makingAmount > orderAvailableAmount) revert NotEnoughForFill(); takerTokenAmounts[i] = ((makingAmount * order.takerTokenAmount) / order.makerTokenAmount); + if (takerTokenAmounts[i] == 0) revert ZeroTakerTokenAmount(); - if (makingAmount == 0) { - if (takerTokenAmounts[i] == 0) revert ZeroTokenAmount(); - } - + // this if statement cannot be covered by tests due to the following issue + // https://github.com/foundry-rs/foundry/issues/3600 if (order.takerToken == address(weth)) { wethToPay += takerTokenAmounts[i]; } @@ -102,7 +117,7 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree // collect maker tokens _collect(order.makerToken, order.maker, address(this), makingAmount, order.makerTokenPermit); - // transfer fee if present + // Transfer fee if present uint256 fee = (makingAmount * order.feeFactor) / Constant.BPS_MAX; order.makerToken.transferTo(_feeCollector, fee); @@ -112,6 +127,7 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree // unwrap extra WETH in order to pay for ETH taker token and profit uint256 wethBalance = weth.balanceOf(address(this)); if (wethBalance > wethToPay) { + // this if statement cannot be fully covered because the WETH withdraw will always succeed as we have checked that wethBalance > wethToPay unchecked { weth.withdraw(wethBalance - wethToPay); } @@ -143,11 +159,17 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree emit OrderCanceled(orderHash, order.maker); } + /// @inheritdoc ILimitOrderSwap function isOrderCanceled(bytes32 orderHash) external view returns (bool) { uint256 orderFilledAmount = orderHashToMakerTokenFilledAmount[orderHash]; return (orderFilledAmount & ORDER_CANCEL_AMOUNT_MASK) != 0; } + /// @notice Fills a limit order. + /// @param order The limit order details. + /// @param makerSignature The maker's signature for the order. + /// @param takerParams The taker's parameters for the order. + /// @param fullOrKill Whether the order should be filled completely or not at all. function _fillLimitOrder(LimitOrder calldata order, bytes calldata makerSignature, TakerParams calldata takerParams, bool fullOrKill) private { (bytes32 orderHash, uint256 takerSpendingAmount, uint256 makerSpendingAmount) = _validateOrderAndQuote( order, @@ -167,6 +189,8 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree if (takerParams.extraAction.length != 0) { (address strategy, bytes memory strategyData) = abi.decode(takerParams.extraAction, (address, bytes)); + // the coverage report indicates that the following line causes the if statement to not be fully covered, + // even if the logic of the executeStrategy function is empty, this if statement is still not covered. IStrategy(strategy).executeStrategy(order.makerToken, order.takerToken, makerSpendingAmount - fee, strategyData); } @@ -185,6 +209,15 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree _emitLimitOrderFilled(order, orderHash, takerSpendingAmount, makerSpendingAmount - fee, fee, takerParams.recipient); } + /// @notice Validates an order and quotes the taker and maker spending amounts. + /// @param _order The limit order details. + /// @param _makerSignature The maker's signature for the order. + /// @param _takerTokenAmount The amount of taker token. + /// @param _makerTokenAmount The amount of maker token. + /// @param _fullOrKill Whether the order should be filled completely or not at all. + /// @return orderHash The hash of the validated order. + /// @return takerSpendingAmount The calculated taker spending amount. + /// @return makerSpendingAmount The calculated maker spending amount. function _validateOrderAndQuote( LimitOrder calldata _order, bytes calldata _makerSignature, @@ -195,8 +228,16 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree uint256 orderFilledAmount; (orderHash, orderFilledAmount) = _validateOrder(_order, _makerSignature); + if (_takerTokenAmount == 0) revert ZeroTakerSpendingAmount(); + if (_makerTokenAmount == 0) revert ZeroMakerSpendingAmount(); + // get the quote of the fill - uint256 orderAvailableAmount = _order.makerTokenAmount - orderFilledAmount; + uint256 orderAvailableAmount; + unchecked { + // orderAvailableAmount must be greater than 0 here, or it will be reverted by the _validateOrder function + orderAvailableAmount = _order.makerTokenAmount - orderFilledAmount; + } + if (_makerTokenAmount > orderAvailableAmount) { // the requested amount is larger than fillable amount if (_fullOrKill) revert NotEnoughForFill(); @@ -206,46 +247,59 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree // re-calculate the amount of taker willing to spend for this trade by the requested ratio _takerTokenAmount = ((_takerTokenAmount * makerSpendingAmount) / _makerTokenAmount); + // Check _takerTokenAmount again + // because there is a case where _takerTokenAmount == 0 after a division calculation + if (_takerTokenAmount == 0) revert ZeroTakerSpendingAmount(); } else { - // the requested amount can be statisfied + // the requested amount can be satisfied makerSpendingAmount = _makerTokenAmount; } uint256 minTakerTokenAmount = ((makerSpendingAmount * _order.takerTokenAmount) / _order.makerTokenAmount); - // check if taker provide enough amount for this fill (better price is allowed) + // check if taker provides enough amount for this fill (better price is allowed) if (_takerTokenAmount < minTakerTokenAmount) revert InvalidTakingAmount(); takerSpendingAmount = _takerTokenAmount; - if (takerSpendingAmount == 0) { - if (makerSpendingAmount == 0) revert ZeroTokenAmount(); - } - // record fill amount of this tx orderHashToMakerTokenFilledAmount[orderHash] = orderFilledAmount + makerSpendingAmount; } + /// @notice Validates an order and its signature. + /// @param _order The limit order details. + /// @param _makerSignature The maker's signature for the order. + /// @return orderHash The hash of the validated order. + /// @return orderFilledAmount The filled amount of the validated order. function _validateOrder(LimitOrder calldata _order, bytes calldata _makerSignature) private view returns (bytes32, uint256) { - // validate the constrain of the order + // validate the constraints of the order if (_order.expiry < block.timestamp) revert ExpiredOrder(); if (_order.taker != address(0)) { if (msg.sender != _order.taker) revert InvalidTaker(); } + if (_order.takerTokenAmount == 0) revert ZeroTakerTokenAmount(); + if (_order.makerTokenAmount == 0) revert ZeroMakerTokenAmount(); - // validate the status of the order bytes32 orderHash = getLimitOrderHash(_order); - - // check whether the order is fully filled or not uint256 orderFilledAmount = orderHashToMakerTokenFilledAmount[orderHash]; - // validate maker signature only once per order + if (orderFilledAmount == 0) { + // validate maker signature only once per order if (!SignatureValidator.validateSignature(_order.maker, getEIP712Hash(orderHash), _makerSignature)) revert InvalidSignature(); } + // validate the status of the order if ((orderFilledAmount & ORDER_CANCEL_AMOUNT_MASK) != 0) revert CanceledOrder(); + // check whether the order is fully filled or not if (orderFilledAmount >= _order.makerTokenAmount) revert FilledOrder(); return (orderHash, orderFilledAmount); } + /// @notice Emits the LimitOrderFilled event after executing a limit order swap. + /// @param _order The limit order details. + /// @param _orderHash The hash of the limit order. + /// @param _takerTokenSettleAmount The settled amount of taker token. + /// @param _makerTokenSettleAmount The settled amount of maker token. + /// @param _fee The fee amount. + /// @param _recipient The recipient of the order settlement. function _emitLimitOrderFilled( LimitOrder calldata _order, bytes32 _orderHash, diff --git a/contracts/RFQ.sol b/contracts/RFQ.sol index a795d492..95d97b11 100644 --- a/contracts/RFQ.sol +++ b/contracts/RFQ.sol @@ -14,22 +14,33 @@ import { RFQTx, getRFQTxHash } from "./libraries/RFQTx.sol"; import { Constant } from "./libraries/Constant.sol"; import { SignatureValidator } from "./libraries/SignatureValidator.sol"; +/// @title RFQ Contract +/// @author imToken Labs +/// @notice This contract allows users to execute an RFQ (Request For Quote) order. contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { using Asset for address; + /// @dev Flag indicating whether contract sender is allowed. uint256 private constant FLG_ALLOW_CONTRACT_SENDER = 1 << 255; + + /// @dev Flag indicating whether partial fill is allowed. uint256 private constant FLG_ALLOW_PARTIAL_FILL = 1 << 254; + + /// @dev Flag indicating whether market maker receives WETH. uint256 private constant FLG_MAKER_RECEIVES_WETH = 1 << 253; IWETH public immutable weth; address payable public feeCollector; - mapping(bytes32 => bool) private filledOffer; - - /// @notice Emitted when fee collector address is updated - /// @param newFeeCollector The address of the new fee collector - event SetFeeCollector(address newFeeCollector); + /// @notice Mapping to track the filled status of each offer identified by its hash. + mapping(bytes32 rfqOfferHash => bool isFilled) public filledOffer; + /// @notice Constructor to initialize the RFQ contract with the owner, Uniswap permit2, allowance target, WETH, and fee collector. + /// @param _owner The address of the contract owner. + /// @param _uniswapPermit2 The address of the Uniswap permit2. + /// @param _allowanceTarget The address of the allowance target. + /// @param _weth The WETH token instance. + /// @param _feeCollector The initial address of the fee collector. constructor( address _owner, address _uniswapPermit2, @@ -42,11 +53,12 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { feeCollector = _feeCollector; } + /// @notice Receive function to receive ETH. receive() external payable {} - /// @notice Set fee collector - /// @notice Only owner can call - /// @param _newFeeCollector The address of the new fee collector + /// @notice Sets the fee collector address. + /// @dev Only callable by the contract owner. + /// @param _newFeeCollector The new address of the fee collector. function setFeeCollector(address payable _newFeeCollector) external onlyOwner { if (_newFeeCollector == address(0)) revert ZeroAddress(); feeCollector = _newFeeCollector; @@ -54,10 +66,12 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { emit SetFeeCollector(_newFeeCollector); } + /// @inheritdoc IRFQ function fillRFQ(RFQTx calldata rfqTx, bytes calldata makerSignature, bytes calldata makerTokenPermit, bytes calldata takerTokenPermit) external payable { _fillRFQ(rfqTx, makerSignature, makerTokenPermit, takerTokenPermit, bytes("")); } + /// @inheritdoc IRFQ function fillRFQWithSig( RFQTx calldata rfqTx, bytes calldata makerSignature, @@ -68,6 +82,7 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { _fillRFQ(rfqTx, makerSignature, makerTokenPermit, takerTokenPermit, takerSignature); } + /// @inheritdoc IRFQ function cancelRFQOffer(RFQOffer calldata rfqOffer) external { if (msg.sender != rfqOffer.maker) revert NotOfferMaker(); bytes32 rfqOfferHash = getRFQOfferHash(rfqOffer); @@ -77,6 +92,12 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { emit CancelRFQOffer(rfqOfferHash, rfqOffer.maker); } + /// @dev Internal function to fill an RFQ transaction based on the provided parameters and signatures. + /// @param _rfqTx The RFQ transaction data. + /// @param _makerSignature The signature of the maker authorizing the transaction. + /// @param _makerTokenPermit The permit data for the maker's token transfer. + /// @param _takerTokenPermit The permit data for the taker's token transfer. + /// @param _takerSignature The signature of the taker authorizing the transaction. function _fillRFQ( RFQTx calldata _rfqTx, bytes calldata _makerSignature, @@ -85,7 +106,8 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { bytes memory _takerSignature ) private { RFQOffer memory _rfqOffer = _rfqTx.rfqOffer; - // check the offer deadline and fee factor + + // valid an RFQ offer if (_rfqOffer.expiry < block.timestamp) revert ExpiredRFQOffer(); if (_rfqOffer.flags & FLG_ALLOW_CONTRACT_SENDER == 0) { if (msg.sender != tx.origin) revert ForbidContract(); @@ -102,10 +124,10 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { if (filledOffer[rfqOfferHash]) revert FilledRFQOffer(); filledOffer[rfqOfferHash] = true; - // check maker signature + // validate maker signature if (!SignatureValidator.validateSignature(_rfqOffer.maker, getEIP712Hash(rfqOfferHash), _makerSignature)) revert InvalidSignature(); - // check taker signature if needed + // validate taker signature if required if (_rfqOffer.taker != msg.sender) { if (!SignatureValidator.validateSignature(_rfqOffer.taker, getEIP712Hash(rfqTxHash), _takerSignature)) revert InvalidSignature(); } @@ -134,15 +156,16 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { makerSettleAmount = (_rfqTx.takerRequestAmount * _rfqOffer.makerTokenAmount) / _rfqOffer.takerTokenAmount; } if (makerSettleAmount == 0) revert InvalidMakerAmount(); - // if the makerToken is ETH, we collect WETH from the maker to this contract - // if the makerToken is a ERC20 token (including WETH) , we collect that ERC20 token from maker to this contract + if (_rfqOffer.makerToken.isETH()) { + // if the makerToken is ETH, we collect WETH from the maker to this contract _collect(address(weth), _rfqOffer.maker, address(this), makerSettleAmount, _makerTokenPermit); } else { + // if the makerToken is a ERC20 token (including WETH) , we collect that ERC20 token from maker to this contract _collect(_rfqOffer.makerToken, _rfqOffer.maker, address(this), makerSettleAmount, _makerTokenPermit); } - // calculate maker token settlement amount (sub fee) + // calculate maker token settlement amount (minus fee) uint256 fee = (makerSettleAmount * _rfqOffer.feeFactor) / Constant.BPS_MAX; uint256 makerTokenToTaker; unchecked { @@ -153,9 +176,12 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { { // unwrap WETH and send out ETH if makerToken is ETH address makerToken = _rfqOffer.makerToken; + // after trying to withdraw more WETH than this contract has + // and replacing `makerToken.isETH()` with `makerToken == 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE` + // the if statement is still not fully covered by the test if (makerToken.isETH()) weth.withdraw(makerSettleAmount); - // collect fee + // transfer fee to fee collector makerToken.transferTo(feeCollector, fee); // transfer maker token to recipient makerToken.transferTo(_rfqTx.recipient, makerTokenToTaker); @@ -164,6 +190,11 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { _emitFilledRFQEvent(rfqOfferHash, _rfqTx, makerTokenToTaker, fee); } + /// @notice Emits the FilledRFQ event after executing an RFQ order swap. + /// @param _rfqOfferHash The hash of the RFQ offer. + /// @param _rfqTx The RFQ transaction data. + /// @param _makerTokenToTaker The amount of maker tokens transferred to the taker. + /// @param fee The fee amount collected. function _emitFilledRFQEvent(bytes32 _rfqOfferHash, RFQTx calldata _rfqTx, uint256 _makerTokenToTaker, uint256 fee) internal { emit FilledRFQ( _rfqOfferHash, @@ -178,17 +209,27 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { ); } - // Only used when taker token is ETH + /// @notice Collects ETH and sends it to the specified address. + /// @param to The address to send the collected ETH. + /// @param amount The amount of ETH to collect. + /// @param makerReceivesWETH Boolean flag to indicate if the maker receives WETH. function _collectETHAndSend(address payable to, uint256 amount, bool makerReceivesWETH) internal { if (makerReceivesWETH) { weth.deposit{ value: amount }(); weth.transfer(to, amount); } else { + // this branch cannot be covered because we cannot trigger the AddressInsufficientBalance error in sendValue, + // as this function is called only when msg.value == amount Address.sendValue(to, amount); } } - // Only used when taker token is WETH + /// @notice Collects WETH and sends it to the specified address. + /// @param from The address to collect WETH from. + /// @param to The address to send the collected WETH. + /// @param amount The amount of WETH to collect. + /// @param data Additional data for the collection. + /// @param makerReceivesWETH Boolean flag to indicate if the maker receives WETH. function _collectWETHAndSend(address from, address payable to, uint256 amount, bytes calldata data, bool makerReceivesWETH) internal { if (makerReceivesWETH) { _collect(address(weth), from, to, amount, data); diff --git a/contracts/SmartOrderStrategy.sol b/contracts/SmartOrderStrategy.sol index 15efd2e3..a9ecdf2e 100644 --- a/contracts/SmartOrderStrategy.sol +++ b/contracts/SmartOrderStrategy.sol @@ -9,17 +9,26 @@ import { IWETH } from "./interfaces/IWETH.sol"; import { ISmartOrderStrategy } from "./interfaces/ISmartOrderStrategy.sol"; import { IStrategy } from "./interfaces/IStrategy.sol"; +/// @title SmartOrderStrategy Contract +/// @author imToken Labs +/// @notice This contract allows users to execute complex token swap operations. contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement { address public immutable weth; address public immutable genericSwap; + /// @notice Receive function to receive ETH. receive() external payable {} + /// @notice Constructor to initialize the contract with the owner, generic swap contract address, and WETH contract address. + /// @param _owner The address of the contract owner. + /// @param _genericSwap The address of the generic swap contract that interacts with this strategy. + /// @param _weth The address of the WETH contract. constructor(address _owner, address _genericSwap, address _weth) AdminManagement(_owner) { genericSwap = _genericSwap; weth = _weth; } + /// @dev Modifier to restrict access to the function only to the generic swap contract. modifier onlyGenericSwap() { if (msg.sender != genericSwap) revert NotFromGS(); _; @@ -32,9 +41,11 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement { Operation[] memory ops = abi.decode(data, (Operation[])); if (ops.length == 0) revert EmptyOps(); - // wrap eth first + // wrap ETH to WETH if inputToken is ETH if (Asset.isETH(inputToken)) { if (msg.value != inputAmount) revert InvalidMsgValue(); + // the coverage report indicates that the following line causes this branch to not be covered by our tests + // even though we tried all possible success and revert scenarios IWETH(weth).deposit{ value: inputAmount }(); } else { if (msg.value != 0) revert InvalidMsgValue(); @@ -46,24 +57,37 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement { _call(op.dest, op.inputToken, op.ratioNumerator, op.ratioDenominator, op.dataOffset, op.value, op.data); } - // transfer output token back to GenericSwap - // ETH first so WETH is not considered as an option of outputToken + // unwrap WETH to ETH if outputToken is ETH if (Asset.isETH(outputToken)) { - // unwrap existing WETH if any + // the if statement is not fully covered by the tests even replacing `makerToken.isETH()` with `makerToken == 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE` + // and crafting some cases where outputToken is ETH and non-ETH uint256 wethBalance = IWETH(weth).balanceOf(address(this)); + if (wethBalance > 0) { + // this if statement is not be fully covered because WETH withdraw will always succeed as wethBalance > 0 IWETH(weth).withdraw(wethBalance); } } + uint256 selfBalance = Asset.getBalance(outputToken, address(this)); if (selfBalance > 1) { unchecked { --selfBalance; } } + + // transfer output tokens back to the generic swap contract Asset.transferTo(outputToken, payable(genericSwap), selfBalance); } + /// @dev This function adjusts the input amount based on a ratio if specified, then calls the destination contract with data. + /// @param _dest The destination address to call. + /// @param _inputToken The address of the input token for the call. + /// @param _ratioNumerator The numerator used for ratio calculation. + /// @param _ratioDenominator The denominator used for ratio calculation. + /// @param _dataOffset The offset in the data where the input amount is located. + /// @param _value The amount of ETH to send with the call. + /// @param _data Additional data to be passed with the call. function _call( address _dest, address _inputToken, @@ -73,7 +97,7 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement { uint256 _value, bytes memory _data ) internal { - // replace amount if ratio != 0 + // adjust amount if ratio != 0 if (_ratioNumerator != 0) { uint256 inputTokenBalance = IERC20(_inputToken).balanceOf(address(this)); // leaving one wei for gas optimization @@ -84,7 +108,6 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement { } // calculate input amount if ratio should be applied - // we provide a _ratioNumerator and a _ratioDenominator to decide a ratio if (_ratioNumerator != _ratioDenominator) { if (_ratioDenominator == 0) revert ZeroDenominator(); inputTokenBalance = (inputTokenBalance * _ratioNumerator) / _ratioDenominator; diff --git a/contracts/abstracts/AdminManagement.sol b/contracts/abstracts/AdminManagement.sol index 3491d2ae..051a63e4 100644 --- a/contracts/abstracts/AdminManagement.sol +++ b/contracts/abstracts/AdminManagement.sol @@ -9,11 +9,18 @@ import { Asset } from "../libraries/Asset.sol"; /// @title AdminManagement Contract /// @author imToken Labs +/// @notice This contract provides administrative functions for token management. abstract contract AdminManagement is Ownable { using SafeERC20 for IERC20; + /// @notice Sets the initial owner of the contract. + /// @param _owner The address of the owner who can execute administrative functions. constructor(address _owner) Ownable(_owner) {} + /// @notice Approves multiple tokens to multiple spenders with an unlimited allowance. + /// @dev Only the owner can call this function. + /// @param tokens The array of token addresses to approve. + /// @param spenders The array of spender addresses to approve for each token. function approveTokens(address[] calldata tokens, address[] calldata spenders) external onlyOwner { for (uint256 i = 0; i < tokens.length; ++i) { for (uint256 j = 0; j < spenders.length; ++j) { @@ -22,6 +29,10 @@ abstract contract AdminManagement is Ownable { } } + /// @notice Rescues multiple tokens held by this contract to the specified recipient. + /// @dev Only the owner can call this function. + /// @param tokens An array of token addresses to rescue. + /// @param recipient The address to which rescued tokens will be transferred. function rescueTokens(address[] calldata tokens, address recipient) external onlyOwner { for (uint256 i = 0; i < tokens.length; ++i) { uint256 selfBalance = Asset.getBalance(tokens[i], address(this)); diff --git a/contracts/abstracts/EIP712.sol b/contracts/abstracts/EIP712.sol index e1d450df..3e486530 100644 --- a/contracts/abstracts/EIP712.sol +++ b/contracts/abstracts/EIP712.sol @@ -1,6 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; +/// @title EIP712 Contract +/// @author imToken Labs +/// @notice This contract implements the EIP-712 standard for structured data hashing and signing. +/// @dev This contract provides functions to handle EIP-712 domain separator and hash calculation. abstract contract EIP712 { // EIP-191 Header string public constant EIP191_HEADER = "\x19\x01"; @@ -15,15 +19,20 @@ abstract contract EIP712 { uint256 public immutable originalChainId; bytes32 public immutable originalEIP712DomainSeparator; + /// @notice Initialize the original chain ID and domain separator. constructor() { originalChainId = block.chainid; originalEIP712DomainSeparator = _buildDomainSeparator(); } + /// @notice Internal function to build the EIP712 domain separator hash. + /// @return The EIP712 domain separator hash. function _buildDomainSeparator() private view returns (bytes32) { return keccak256(abi.encode(EIP712_TYPE_HASH, EIP712_HASHED_NAME, EIP712_HASHED_VERSION, block.chainid, address(this))); } + /// @notice Internal function to get the current EIP712 domain separator. + /// @return The current EIP712 domain separator. function _getDomainSeparator() private view returns (bytes32) { if (block.chainid == originalChainId) { return originalEIP712DomainSeparator; @@ -32,10 +41,15 @@ abstract contract EIP712 { } } + /// @notice Calculate the EIP712 hash of a structured data hash. + /// @param structHash The hash of the structured data. + /// @return The EIP712 hash of the structured data. function getEIP712Hash(bytes32 structHash) internal view returns (bytes32) { return keccak256(abi.encodePacked(EIP191_HEADER, _getDomainSeparator(), structHash)); } + /// @notice Get the current EIP712 domain separator. + /// @return The current EIP712 domain separator. function EIP712_DOMAIN_SEPARATOR() external view returns (bytes32) { return _getDomainSeparator(); } diff --git a/contracts/abstracts/Ownable.sol b/contracts/abstracts/Ownable.sol index d9fc0030..3422fc67 100644 --- a/contracts/abstracts/Ownable.sol +++ b/contracts/abstracts/Ownable.sol @@ -3,18 +3,41 @@ pragma solidity ^0.8.0; /// @title Ownable Contract /// @author imToken Labs +/// @notice This contract manages ownership and allows transfer and renouncement of ownership. +/// @dev This contract uses a nomination system for ownership transfer. abstract contract Ownable { address public owner; address public nominatedOwner; + /// @notice Error to be thrown when the caller is not the owner. + /// @dev This error is used to ensure that only the owner can call certain functions. error NotOwner(); + + /// @notice Error to be thrown when the caller is not the nominated owner. + /// @dev This error is used to ensure that only the nominated owner can accept ownership. error NotNominated(); + + /// @notice Error to be thrown when the provided owner address is zero. + /// @dev This error is used to ensure a valid address is provided for the owner. error ZeroOwner(); + + /// @notice Error to be thrown when there is already a nominated owner. + /// @dev This error is used to prevent nominating a new owner when one is already nominated. error NominationExists(); + /// @notice Event emitted when a new owner is nominated. + /// @dev This event is emitted when the current owner nominates a new owner. + /// @param newOwner The address of the new nominated owner. event OwnerNominated(address indexed newOwner); + + /// @notice Event emitted when ownership is transferred. + /// @dev This event is emitted when ownership is transferred to a new owner. + /// @param oldOwner The address of the previous owner. + /// @param newOwner The address of the new owner. event OwnerChanged(address indexed oldOwner, address indexed newOwner); + /// @notice Constructor to set the initial owner of the contract. + /// @param _owner The address of the initial owner. constructor(address _owner) { if (_owner == address(0)) revert ZeroOwner(); owner = _owner; @@ -25,8 +48,8 @@ abstract contract Ownable { _; } - /// @notice Activate new ownership - /// @notice Only nominated owner can call + /// @notice Accept the ownership transfer. + /// @dev Only the nominated owner can call this function to accept the ownership. function acceptOwnership() external { if (msg.sender != nominatedOwner) revert NotNominated(); emit OwnerChanged(owner, nominatedOwner); @@ -35,18 +58,17 @@ abstract contract Ownable { nominatedOwner = address(0); } - /// @notice Give up the ownership - /// @notice Only owner can call - /// @notice Ownership cannot be recovered + /// @notice Renounce ownership of the contract. + /// @dev Only the current owner can call this function to renounce ownership. Once renounced, ownership cannot be recovered. function renounceOwnership() external onlyOwner { if (nominatedOwner != address(0)) revert NominationExists(); emit OwnerChanged(owner, address(0)); owner = address(0); } - /// @notice Nominate new owner - /// @notice Only owner can call - /// @param newOwner The address of the new owner + /// @notice Nominate a new owner. + /// @dev Only the current owner can call this function to nominate a new owner. + /// @param newOwner The address of the new owner. function nominateNewOwner(address newOwner) external onlyOwner { nominatedOwner = newOwner; emit OwnerNominated(newOwner); diff --git a/contracts/abstracts/TokenCollector.sol b/contracts/abstracts/TokenCollector.sol index bdb345d5..078d7ad6 100644 --- a/contracts/abstracts/TokenCollector.sol +++ b/contracts/abstracts/TokenCollector.sol @@ -8,11 +8,20 @@ import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.s import { IUniswapPermit2 } from "../interfaces/IUniswapPermit2.sol"; import { IAllowanceTarget } from "../interfaces/IAllowanceTarget.sol"; +/// @title TokenCollector Contract +/// @author imToken Labs +/// @notice This contract handles the collection of tokens using various methods. +/// @dev This contract supports multiple token collection mechanisms including allowance targets, direct transfers, and permit transfers. abstract contract TokenCollector { using SafeERC20 for IERC20; + /// @notice Error to be thrown when Permit2 data is empty. + /// @dev This error is used to ensure Permit2 data is provided when required. error Permit2DataEmpty(); + /// @title Token Collection Sources + /// @notice Enumeration of possible token collection sources. + /// @dev Represents the various methods for collecting tokens. enum Source { TokenlonAllowanceTarget, Token, @@ -24,11 +33,21 @@ abstract contract TokenCollector { address public immutable permit2; address public immutable allowanceTarget; + /// @notice Constructor to set the Permit2 and allowance target addresses. + /// @param _permit2 The address of the Uniswap Permit2 contract. + /// @param _allowanceTarget The address of the allowance target contract. constructor(address _permit2, address _allowanceTarget) { permit2 = _permit2; allowanceTarget = _allowanceTarget; } + /// @notice Internal function to collect tokens using various methods. + /// @dev Handles token collection based on the specified source. + /// @param token The address of the token to be collected. + /// @param from The address from which the tokens will be collected. + /// @param to The address to which the tokens will be sent. + /// @param amount The amount of tokens to be collected. + /// @param data Additional data required for the token collection process. function _collect(address token, address from, address to, uint256 amount, bytes calldata data) internal { Source src = Source(uint8(data[0])); @@ -67,8 +86,5 @@ abstract contract TokenCollector { IUniswapPermit2.SignatureTransferDetails memory detail = IUniswapPermit2.SignatureTransferDetails({ to: to, requestedAmount: amount }); return IUniswapPermit2(permit2).permitTransferFrom(permit, detail, from, permitSig); } - - // won't be reached - revert(); } } diff --git a/contracts/interfaces/IAllowanceTarget.sol b/contracts/interfaces/IAllowanceTarget.sol index d8d83564..6ac2b55b 100644 --- a/contracts/interfaces/IAllowanceTarget.sol +++ b/contracts/interfaces/IAllowanceTarget.sol @@ -3,13 +3,18 @@ pragma solidity ^0.8.0; /// @title IAllowanceTarget Interface /// @author imToken Labs +/// @notice This interface defines the function for spending tokens on behalf of a user. +/// @dev Only authorized addresses can call the spend function. interface IAllowanceTarget { + /// @notice Error to be thrown when the caller is not authorized. + /// @dev This error is used to ensure that only authorized addresses can spend tokens on behalf of a user. error NotAuthorized(); - /// @dev Spend tokens on user's behalf. Only an authority can call this. - /// @param from The user to spend token from. - /// @param token The address of the token. - /// @param to The recipient of the trasnfer. - /// @param amount Amount to spend. + /// @notice Spend tokens on user's behalf. + /// @dev Only an authorized address can call this function to spend tokens on behalf of a user. + /// @param from The user to spend tokens from. + /// @param token The address of the token. + /// @param to The recipient of the transfer. + /// @param amount The amount to spend. function spendFromUserTo(address from, address token, address to, uint256 amount) external; } diff --git a/contracts/interfaces/ICoordinatedTaker.sol b/contracts/interfaces/ICoordinatedTaker.sol index 02e5b273..79a32ac0 100644 --- a/contracts/interfaces/ICoordinatedTaker.sol +++ b/contracts/interfaces/ICoordinatedTaker.sol @@ -5,25 +5,59 @@ import { LimitOrder } from "../libraries/LimitOrder.sol"; /// @title ICoordinatedTaker Interface /// @author imToken Labs +/// @notice This interface defines the functions and events for a coordinated taker contract. +/// @dev The contract handles limit order fills with additional coordination parameters. interface ICoordinatedTaker { + /// @notice Error to be thrown when a permission is reused. + /// @dev This error is used to prevent the reuse of permissions. error ReusedPermission(); + + /// @notice Error to be thrown when the msg.value is invalid. + /// @dev This error is used to ensure that the correct msg.value is sent with the transaction. error InvalidMsgValue(); + + /// @notice Error to be thrown when a signature is invalid. + /// @dev This error is used to ensure that the provided signature is valid. error InvalidSignature(); + + /// @notice Error to be thrown when a permission has expired. + /// @dev This error is used to ensure that the permission has not expired. error ExpiredPermission(); + + /// @notice Error to be thrown when an address is zero. + /// @dev This error is used to ensure that a valid address is provided. error ZeroAddress(); + /// @title Coordinator Parameters + /// @notice Struct for coordinator parameters. + /// @dev Contains the signature, salt, and expiry for coordinator authorization. struct CoordinatorParams { bytes sig; uint256 salt; uint256 expiry; } + /// @notice Emitted when a limit order is filled by the coordinator. + /// @dev This event is emitted when a limit order is successfully filled. + /// @param user The address of the user. + /// @param orderHash The hash of the order. + /// @param allowFillHash The hash of the allowed fill. event CoordinatorFill(address indexed user, bytes32 indexed orderHash, bytes32 indexed allowFillHash); - /// @notice Emitted when coordinator address is updated - /// @param newCoordinator The address of the new coordinator + /// @notice Emitted when the coordinator address is updated. + /// @dev This event is emitted when the coordinator address is updated. + /// @param newCoordinator The address of the new coordinator. event SetCoordinator(address newCoordinator); + /// @notice Submits a limit order fill. + /// @dev Allows a user to submit a limit order fill with additional coordination parameters. + /// @param order The limit order to be filled. + /// @param makerSignature The signature of the maker. + /// @param takerTokenAmount The amount of tokens to be taken by the taker. + /// @param makerTokenAmount The amount of tokens to be given by the maker. + /// @param extraAction Any extra action to be performed. + /// @param userTokenPermit The user's token permit. + /// @param crdParams The coordinator parameters. function submitLimitOrderFill( LimitOrder calldata order, bytes calldata makerSignature, diff --git a/contracts/interfaces/IERC1271Wallet.sol b/contracts/interfaces/IERC1271Wallet.sol index 0c79d2ac..81981ab7 100644 --- a/contracts/interfaces/IERC1271Wallet.sol +++ b/contracts/interfaces/IERC1271Wallet.sol @@ -1,6 +1,15 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; +/// @title IERC1271Wallet Interface +/// @author imToken Labs +/// @notice Interface for contracts that support ERC-1271 signature validation. +/// @dev This interface defines a function to check the validity of a signature for a given hash. interface IERC1271Wallet { + /// @notice Checks if a signature is valid for a given hash. + /// @dev Returns a magic value of 0x1626ba7e if the signature is valid, otherwise returns an error. + /// @param _hash The hash that was signed. + /// @param _signature The signature bytes. + /// @return magicValue The ERC-1271 magic value (0x1626ba7e) if the signature is valid, otherwise returns an error. function isValidSignature(bytes32 _hash, bytes calldata _signature) external view returns (bytes4 magicValue); } diff --git a/contracts/interfaces/IGenericSwap.sol b/contracts/interfaces/IGenericSwap.sol index c6c4875d..6bd48c66 100644 --- a/contracts/interfaces/IGenericSwap.sol +++ b/contracts/interfaces/IGenericSwap.sol @@ -3,14 +3,46 @@ pragma solidity ^0.8.0; import { GenericSwapData } from "../libraries/GenericSwapData.sol"; +/// @title IGenericSwap Interface +/// @author imToken Labs +/// @notice Interface for a generic swap contract. +/// @dev This interface defines functions and events related to executing swaps and handling swap errors. interface IGenericSwap { + /// @notice Error to be thrown when a swap is already filled. + /// @dev This error is used when attempting to fill a swap that has already been completed. error AlreadyFilled(); + + /// @notice Error to be thrown when the msg.value is invalid. + /// @dev This error is used to ensure that the correct msg.value is sent with the transaction. error InvalidMsgValue(); + + /// @notice Error to be thrown when the output amount is insufficient. + /// @dev This error is used when the output amount received from the swap is insufficient. error InsufficientOutput(); + + /// @notice Error to be thrown when a signature is invalid. + /// @dev This error is used to ensure that the provided signature is valid. error InvalidSignature(); + + /// @notice Error to be thrown when an order has expired. + /// @dev This error is used to ensure that the swap order has not expired. error ExpiredOrder(); + + /// @notice Error to be thrown when an address is zero. + /// @dev This error is used to ensure that a valid address is provided. error ZeroAddress(); + /// @notice Event emitted when a swap is executed. + /// @dev This event is emitted when a swap is successfully executed. + /// @param swapHash The hash of the swap data. + /// @param maker The address of the maker initiating the swap. + /// @param taker The address of the taker executing the swap. + /// @param recipient The address receiving the output tokens. + /// @param inputToken The address of the input token. + /// @param inputAmount The amount of input tokens. + /// @param outputToken The address of the output token. + /// @param outputAmount The amount of output tokens received. + /// @param salt The salt value used in the swap. event Swap( bytes32 indexed swapHash, address indexed maker, @@ -23,8 +55,20 @@ interface IGenericSwap { uint256 salt ); + /// @notice Executes a swap using provided swap data and taker token permit. + /// @dev Executes a swap based on the provided swap data and taker token permit. + /// @param swapData The swap data containing details of the swap. + /// @param takerTokenPermit The permit for spending taker's tokens. + /// @return returnAmount The amount of tokens returned from the swap. function executeSwap(GenericSwapData calldata swapData, bytes calldata takerTokenPermit) external payable returns (uint256 returnAmount); + /// @notice Executes a swap using provided swap data, taker token permit, taker address, and signature. + /// @dev Executes a swap with additional parameters including taker address and signature. + /// @param swapData The swap data containing details of the swap. + /// @param takerTokenPermit The permit for spending taker's tokens. + /// @param taker The address of the taker initiating the swap. + /// @param takerSig The signature of the taker authorizing the swap. + /// @return returnAmount The amount of tokens returned from the swap. function executeSwapWithSig( GenericSwapData calldata swapData, bytes calldata takerTokenPermit, diff --git a/contracts/interfaces/ILimitOrderSwap.sol b/contracts/interfaces/ILimitOrderSwap.sol index fe5e156e..c2bb4cc9 100644 --- a/contracts/interfaces/ILimitOrderSwap.sol +++ b/contracts/interfaces/ILimitOrderSwap.sol @@ -5,25 +5,85 @@ import { LimitOrder } from "../libraries/LimitOrder.sol"; /// @title ILimitOrderSwap Interface /// @author imToken Labs +/// @notice Interface for a limit order swap contract. +/// @dev This interface defines functions and events related to executing and managing limit orders. interface ILimitOrderSwap { + /// @notice Error to be thrown when an order has expired. + /// @dev Thrown when attempting to fill an order that has already expired. error ExpiredOrder(); + + /// @notice Error to be thrown when an order is canceled. + /// @dev Thrown when attempting to fill or interact with a canceled order. error CanceledOrder(); + + /// @notice Error to be thrown when an order is already filled. + /// @dev Thrown when attempting to fill an order that has already been fully filled. error FilledOrder(); + + /// @notice Error to be thrown when an address is zero. + /// @dev Thrown when an operation requires a non-zero address. error ZeroAddress(); - error ZeroTokenAmount(); + + /// @notice Error to be thrown when the taker token amount is zero. + /// @dev Thrown when filling an order with zero taker token amount. + error ZeroTakerTokenAmount(); + + /// @notice Error to be thrown when the maker token amount is zero. + /// @dev Thrown when filling an order with zero maker token amount. + error ZeroMakerTokenAmount(); + + /// @notice Error to be thrown when the taker spending amount is zero. + /// @dev Thrown when an action requires a non-zero taker spending amount. + error ZeroTakerSpendingAmount(); + + /// @notice Error to be thrown when the maker spending amount is zero. + /// @dev Thrown when an action requires a non-zero maker spending amount. + error ZeroMakerSpendingAmount(); + + /// @notice Error to be thrown when there are not enough tokens to fill the order. + /// @dev Thrown when attempting to fill an order with insufficient tokens available. error NotEnoughForFill(); + + /// @notice Error to be thrown when the msg.value is invalid. + /// @dev Thrown when an operation requires a specific msg.value that is not provided. error InvalidMsgValue(); + + /// @notice Error to be thrown when a signature is invalid. + /// @dev Thrown when an operation requires a valid cryptographic signature that is not provided or is invalid. error InvalidSignature(); + + /// @notice Error to be thrown when the taker address is invalid. + /// @dev Thrown when an operation requires a valid taker address that is not provided or is invalid. error InvalidTaker(); + + /// @notice Error to be thrown when the taking amount is invalid. + /// @dev Thrown when an operation requires a valid taking amount that is not provided or is invalid. error InvalidTakingAmount(); + + /// @notice Error to be thrown when the parameters provided are invalid. + /// @dev Thrown when an operation receives invalid parameters that prevent execution. error InvalidParams(); + + /// @notice Error to be thrown when the caller is not the maker of the order. + /// @dev Thrown when an operation is attempted by an unauthorized caller who is not the maker of the order. error NotOrderMaker(); - /// @notice Emitted when fee collector address is updated - /// @param newFeeCollector The address of the new fee collector + /// @notice Emitted when the fee collector address is updated. + /// @dev This event is emitted when the fee collector address is updated. + /// @param newFeeCollector The address of the new fee collector. event SetFeeCollector(address newFeeCollector); - /// @notice Emitted when an order is filled + /// @notice Emitted when a limit order is successfully filled. + /// @dev This event is emitted when a limit order is filled with all necessary details. + /// @param orderHash The hash of the limit order. + /// @param taker The address of the taker filling the order. + /// @param maker The address of the maker who created the order. + /// @param takerToken The address of the token taken by the taker. + /// @param takerTokenFilledAmount The amount of taker tokens filled. + /// @param makerToken The address of the token received by the maker. + /// @param makerTokenSettleAmount The amount of maker tokens settled. + /// @param fee The fee amount paid for the order. + /// @param recipient The address receiving the tokens. event LimitOrderFilled( bytes32 indexed orderHash, address indexed taker, @@ -36,23 +96,43 @@ interface ILimitOrderSwap { address recipient ); - /// @notice Emitted when order is canceled + /// @notice Emitted when an order is canceled. + /// @dev This event is emitted when a limit order is canceled. + /// @param orderHash The hash of the canceled order. + /// @param maker The address of the maker who canceled the order. event OrderCanceled(bytes32 orderHash, address maker); + /// @title Taker Parameters + /// @notice Struct containing parameters for the taker. + /// @dev This struct encapsulates the parameters necessary for a taker to fill a limit order. struct TakerParams { - uint256 takerTokenAmount; - uint256 makerTokenAmount; - address recipient; - bytes extraAction; - bytes takerTokenPermit; + uint256 takerTokenAmount; // Amount of tokens taken by the taker. + uint256 makerTokenAmount; // Amount of tokens provided by the maker. + address recipient; // Address to receive the tokens. + bytes extraAction; // Additional action to be performed. + bytes takerTokenPermit; // Permit for spending taker's tokens. } - /// @notice Fill an order + /// @notice Fills a limit order. + /// @dev This function allows a taker to fill a limit order based on the provided parameters. + /// @param order The limit order to be filled. + /// @param makerSignature The signature of the maker authorizing the fill. + /// @param takerParams The parameters specifying how the order should be filled by the taker. function fillLimitOrder(LimitOrder calldata order, bytes calldata makerSignature, TakerParams calldata takerParams) external payable; - /// @notice Fill an order + /// @notice Fills a limit order fully or cancels it. + /// @dev This function allows a taker to either fully fill a limit order or cancel it entirely. + /// @param order The limit order to be filled or canceled. + /// @param makerSignature The signature of the maker authorizing the fill or cancel. + /// @param takerParams The parameters specifying how the order should be filled by the taker. function fillLimitOrderFullOrKill(LimitOrder calldata order, bytes calldata makerSignature, TakerParams calldata takerParams) external payable; + /// @notice Fills a group of limit orders atomically. + /// @dev This function allows a taker to fill a group of limit orders atomically, ensuring all or none are executed. + /// @param orders The array of limit orders to be filled. + /// @param makerSignatures The array of signatures of the makers authorizing the fills. + /// @param makerTokenAmounts The array of amounts of tokens provided by the makers. + /// @param profitTokens The array of addresses of tokens used for profit sharing. function fillLimitOrderGroup( LimitOrder[] calldata orders, bytes[] calldata makerSignatures, @@ -60,8 +140,14 @@ interface ILimitOrderSwap { address[] calldata profitTokens ) external payable; - /// @notice Cancel an order + /// @notice Cancels a limit order. + /// @dev This function allows the maker of a limit order to cancel it. + /// @param order The limit order to be canceled. function cancelOrder(LimitOrder calldata order) external; + /// @notice Checks if an order is canceled. + /// @dev This function checks whether a specific order has been canceled. + /// @param orderHash The hash of the order to check. + /// @return True if the order is canceled, otherwise false. function isOrderCanceled(bytes32 orderHash) external view returns (bool); } diff --git a/contracts/interfaces/IRFQ.sol b/contracts/interfaces/IRFQ.sol index edb976f5..c9d31478 100644 --- a/contracts/interfaces/IRFQ.sol +++ b/contracts/interfaces/IRFQ.sol @@ -6,19 +6,69 @@ import { RFQTx } from "../libraries/RFQTx.sol"; /// @title IRFQ Interface /// @author imToken Labs +/// @notice Interface for an RFQ (Request for Quote) contract. +/// @dev This interface defines functions and events related to handling RFQ offers and transactions. interface IRFQ { + /// @notice Error to be thrown when an RFQ offer has expired. + /// @dev Thrown when attempting to fill an RFQ offer that has expired. error ExpiredRFQOffer(); + + /// @notice Error to be thrown when an RFQ offer is already filled. + /// @dev Thrown when attempting to fill an RFQ offer that has already been filled. error FilledRFQOffer(); + + /// @notice Error to be thrown when an address is zero. + /// @dev Thrown when an operation requires a non-zero address. error ZeroAddress(); + + /// @notice Error to be thrown when the fee factor is invalid. + /// @dev Thrown when an operation requires a valid fee factor that is not provided. error InvalidFeeFactor(); + + /// @notice Error to be thrown when the msg.value is invalid. + /// @dev Thrown when an operation requires a specific msg.value that is not provided. error InvalidMsgValue(); + + /// @notice Error to be thrown when a signature is invalid. + /// @dev Thrown when an operation requires a valid cryptographic signature that is not provided or is invalid. error InvalidSignature(); + + /// @notice Error to be thrown when the taker amount is invalid. + /// @dev Thrown when an operation requires a valid taker amount that is not provided or is invalid. error InvalidTakerAmount(); + + /// @notice Error to be thrown when the maker amount is invalid. + /// @dev Thrown when an operation requires a valid maker amount that is not provided or is invalid. error InvalidMakerAmount(); + + /// @notice Error to be thrown when interaction with contracts is forbidden. + /// @dev Thrown when an operation is attempted with a contract address where only EOA (Externally Owned Account) is allowed. error ForbidContract(); + + /// @notice Error to be thrown when partial fill is forbidden. + /// @dev Thrown when attempting to partially fill an RFQ offer that does not allow partial fills. error ForbidPartialFill(); + + /// @notice Error to be thrown when the caller is not the maker of the RFQ offer. + /// @dev Thrown when an operation is attempted by an unauthorized caller who is not the maker of the RFQ offer. error NotOfferMaker(); + /// @notice Emitted when the fee collector address is updated. + /// @dev This event is emitted when the fee collector address is updated. + /// @param newFeeCollector The address of the new fee collector. + event SetFeeCollector(address newFeeCollector); + + /// @notice Emitted when an RFQ offer is successfully filled. + /// @dev This event is emitted when an RFQ offer is filled with all necessary details. + /// @param rfqOfferHash The hash of the RFQ offer. + /// @param user The address of the user filling the RFQ offer. + /// @param maker The address of the maker who created the RFQ offer. + /// @param takerToken The address of the token taken by the taker. + /// @param takerTokenUserAmount The amount of taker tokens taken by the user. + /// @param makerToken The address of the token provided by the maker. + /// @param makerTokenUserAmount The amount of maker tokens received by the user. + /// @param recipient The address receiving the tokens. + /// @param fee The fee amount paid for the RFQ transaction. event FilledRFQ( bytes32 indexed rfqOfferHash, address indexed user, @@ -31,10 +81,27 @@ interface IRFQ { uint256 fee ); + /// @notice Emitted when an RFQ offer is canceled. + /// @dev This event is emitted when an RFQ offer is canceled by the maker. + /// @param rfqOfferHash The hash of the canceled RFQ offer. + /// @param maker The address of the maker who canceled the RFQ offer. event CancelRFQOffer(bytes32 indexed rfqOfferHash, address indexed maker); + /// @notice Fills an RFQ offer. + /// @dev This function allows a user to fill an RFQ offer based on the provided transaction details. + /// @param rfqTx The RFQ transaction details. + /// @param makerSignature The signature of the maker authorizing the fill. + /// @param makerTokenPermit The permit for spending maker's tokens. + /// @param takerTokenPermit The permit for spending taker's tokens. function fillRFQ(RFQTx calldata rfqTx, bytes calldata makerSignature, bytes calldata makerTokenPermit, bytes calldata takerTokenPermit) external payable; + /// @notice Fills an RFQ offer using a taker signature. + /// @dev This function allows a user to fill an RFQ offer using a taker's cryptographic signature. + /// @param rfqTx The RFQ transaction details. + /// @param makerSignature The signature of the maker authorizing the fill. + /// @param makerTokenPermit The permit for spending maker's tokens. + /// @param takerTokenPermit The permit for spending taker's tokens. + /// @param takerSignature The cryptographic signature of the taker authorizing the fill. function fillRFQWithSig( RFQTx calldata rfqTx, bytes calldata makerSignature, @@ -43,5 +110,8 @@ interface IRFQ { bytes calldata takerSignature ) external; + /// @notice Cancels an RFQ offer. + /// @dev This function allows the maker of an RFQ offer to cancel it. + /// @param rfqOffer The RFQ offer to be canceled. function cancelRFQOffer(RFQOffer calldata rfqOffer) external; } diff --git a/contracts/interfaces/ISmartOrderStrategy.sol b/contracts/interfaces/ISmartOrderStrategy.sol index f0055b79..39f0db59 100644 --- a/contracts/interfaces/ISmartOrderStrategy.sol +++ b/contracts/interfaces/ISmartOrderStrategy.sol @@ -5,14 +5,34 @@ import { IStrategy } from "./IStrategy.sol"; /// @title ISmartOrderStrategy Interface /// @author imToken Labs +/// @notice Interface for a smart order execution strategy contract. interface ISmartOrderStrategy is IStrategy { + /// @notice Error thrown when the input is zero. + /// @dev Thrown when an operation requires a non-zero input value that is not provided. error ZeroInput(); + + /// @notice Error thrown when the denominator is zero. + /// @dev Thrown when an operation requires a non-zero denominator that is not provided. error ZeroDenominator(); + + /// @notice Error thrown when the operation list is empty. + /// @dev Thrown when an operation list is required to be non-empty but is empty. error EmptyOps(); + + /// @notice Error thrown when the msg.value is invalid. + /// @dev Thrown when an operation requires a specific msg.value that is not provided. error InvalidMsgValue(); + + /// @notice Error thrown when the input ratio is invalid. + /// @dev Thrown when an operation requires a valid input ratio that is not provided or is invalid. error InvalidInputRatio(); + + /// @notice Error thrown when the operation is not from a Governance System (GS). + /// @dev Thrown when an operation is attempted by an unauthorized caller that is not from a Governance System (GS). error NotFromGS(); + /// @title Operation + /// @notice Struct containing parameters for the operation. /// @dev The encoded operation list should be passed as `data` when calling `IStrategy.executeStrategy` struct Operation { address dest; diff --git a/contracts/interfaces/IStrategy.sol b/contracts/interfaces/IStrategy.sol index 52c5b3de..416da964 100644 --- a/contracts/interfaces/IStrategy.sol +++ b/contracts/interfaces/IStrategy.sol @@ -3,6 +3,13 @@ pragma solidity ^0.8.0; /// @title IStrategy Interface /// @author imToken Labs +/// @notice Interface for a strategy contract that executes a specific trading strategy. interface IStrategy { + /// @notice Executes the strategy to trade `inputAmount` of `inputToken` for `outputToken`. + /// @dev Implementations should handle the logic to trade tokens based on the provided parameters. + /// @param inputToken The token to be traded from. + /// @param outputToken The token to be received after the trade. + /// @param inputAmount The amount of `inputToken` to be traded. + /// @param data Additional data needed for executing the strategy, encoded as bytes. function executeStrategy(address inputToken, address outputToken, uint256 inputAmount, bytes calldata data) external payable; } diff --git a/contracts/interfaces/IUniswapPermit2.sol b/contracts/interfaces/IUniswapPermit2.sol index 9f11b0ab..483a04ad 100644 --- a/contracts/interfaces/IUniswapPermit2.sol +++ b/contracts/interfaces/IUniswapPermit2.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; +/// @title IUniswapPermit2 Interface interface IUniswapPermit2 { /// @notice Thrown when an allowance on a token has expired. /// @param deadline The timestamp at which the allowed amount is no longer valid diff --git a/contracts/interfaces/IWETH.sol b/contracts/interfaces/IWETH.sol index f6a83e1f..77b03c0b 100644 --- a/contracts/interfaces/IWETH.sol +++ b/contracts/interfaces/IWETH.sol @@ -1,14 +1,34 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; +/// @title IWETH Interface interface IWETH { + /// @notice Returns the balance of `account`. + /// @param account The address for which to query the balance. + /// @return The balance of `account`. function balanceOf(address account) external view returns (uint256); + /// @notice Deposits ETH into the contract and wraps it into WETH. + /// @dev ETH is deposited and wrapped into WETH in the contract. function deposit() external payable; + /// @notice Withdraws a specified amount of WETH, unwraps it into ETH, and sends it to the caller. + /// @dev Withdraws `amount` of WETH from the caller's balance and unwraps it into ETH. + /// @param amount The amount of WETH to withdraw and unwrap. function withdraw(uint256 amount) external; + /// @notice Transfers a specified amount of WETH to a destination address. + /// @dev Transfers `wad` amount of WETH from the caller's balance to `dst`. + /// @param dst The recipient address to which WETH will be transferred. + /// @param wad The amount of WETH to transfer. + /// @return True if the transfer is successful, false otherwise. function transfer(address dst, uint256 wad) external returns (bool); + /// @notice Transfers a specified amount of WETH from a source address to a destination address. + /// @dev Transfers `wad` amount of WETH from `src` to `dst`, if the caller has been approved by `src`. + /// @param src The sender address from which WETH will be transferred. + /// @param dst The recipient address to which WETH will be transferred. + /// @param wad The amount of WETH to transfer. + /// @return True if the transfer is successful, false otherwise. function transferFrom(address src, address dst, uint256 wad) external returns (bool); } diff --git a/contracts/libraries/Asset.sol b/contracts/libraries/Asset.sol index bb0a3e9d..3e3101d0 100644 --- a/contracts/libraries/Asset.sol +++ b/contracts/libraries/Asset.sol @@ -6,15 +6,29 @@ import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.s import { Constant } from "./Constant.sol"; +/// @title Asset Library +/// @author imToken Labs +/// @notice Library for handling asset operations, including ETH and ERC20 tokens library Asset { using SafeERC20 for IERC20; + /// @notice Error thrown when there is insufficient balance for a transfer + /// @dev This error is thrown when attempting to transfer an amount that exceeds the available balance. error InsufficientBalance(); + /// @notice Checks if an address is ETH + /// @dev ETH is identified by comparing the address to Constant.ETH_ADDRESS or Constant.ZERO_ADDRESS + /// @param addr The address to check + /// @return true if the address is ETH, false otherwise function isETH(address addr) internal pure returns (bool) { return (addr == Constant.ETH_ADDRESS || addr == Constant.ZERO_ADDRESS); } + /// @notice Gets the balance of an asset for a specific owner + /// @dev If the asset is ETH, retrieves the ETH balance of the owner; otherwise, retrieves the ERC20 balance + /// @param asset The address of the asset (ETH or ERC20 token) + /// @param owner The address of the owner + /// @return The balance of the asset owned by the owner function getBalance(address asset, address owner) internal view returns (uint256) { if (isETH(asset)) { return owner.balance; @@ -23,11 +37,16 @@ library Asset { } } + /// @notice Transfers an amount of asset to a recipient address + /// @dev If the asset is ETH, transfers ETH using a low-level call; otherwise, uses SafeERC20 for ERC20 transfers + /// @param asset The address of the asset (ETH or ERC20 token) + /// @param to The address of the recipient + /// @param amount The amount to transfer function transferTo(address asset, address payable to, uint256 amount) internal { if (to == address(this) || amount == 0) return; if (isETH(asset)) { - // @dev forward all available gas and may cause reentrancy + // @dev Forward all available gas and may cause reentrancy if (address(this).balance < amount) revert InsufficientBalance(); (bool success, bytes memory result) = to.call{ value: amount }(""); if (!success) { diff --git a/contracts/libraries/Constant.sol b/contracts/libraries/Constant.sol index 5002aeee..42e0cdc7 100644 --- a/contracts/libraries/Constant.sol +++ b/contracts/libraries/Constant.sol @@ -1,7 +1,11 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; +/// @title Constant Library +/// @author imToken Labs +/// @notice Library for defining constant values used across contracts library Constant { + /// @dev Maximum value for basis points (BPS) uint16 internal constant BPS_MAX = 10000; address internal constant ETH_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; address internal constant ZERO_ADDRESS = address(0); diff --git a/contracts/libraries/GenericSwapData.sol b/contracts/libraries/GenericSwapData.sol index 04ea8654..fa73b681 100644 --- a/contracts/libraries/GenericSwapData.sol +++ b/contracts/libraries/GenericSwapData.sol @@ -21,7 +21,6 @@ struct GenericSwapData { } // solhint-disable-next-line func-visibility -// free functions cannot have function visibility function getGSDataHash(GenericSwapData memory gsData) pure returns (bytes32) { return keccak256( diff --git a/contracts/libraries/SignatureValidator.sol b/contracts/libraries/SignatureValidator.sol index f03972c6..cbc43377 100644 --- a/contracts/libraries/SignatureValidator.sol +++ b/contracts/libraries/SignatureValidator.sol @@ -6,19 +6,21 @@ import { Address } from "@openzeppelin/contracts/utils/Address.sol"; import { IERC1271Wallet } from "../interfaces/IERC1271Wallet.sol"; +/// @title Signature Validator Library +/// @author imToken Labs +/// @notice Library for validating signatures using ECDSA and ERC1271 standards library SignatureValidator { using Address for address; // bytes4(keccak256("isValidSignature(bytes32,bytes)")) bytes4 internal constant ERC1271_MAGICVALUE = 0x1626ba7e; - /** - * @dev Verifies that a hash has been signed by the given signer. - * @param _signerAddress Address that should have signed the given hash. - * @param _hash Hash of the EIP-712 encoded data - * @param _signature Proof that the hash has been signed by signer. - * @return True if the address recovered from the provided signature matches the input signer address. - */ + /// @notice Verifies that a hash has been signed by the given signer. + /// @dev This function verifies signatures either through ERC1271 wallets or direct ECDSA recovery. + /// @param _signerAddress Address that should have signed the given hash. + /// @param _hash Hash of the EIP-712 encoded data. + /// @param _signature Proof that the hash has been signed by signer. + /// @return True if the address recovered from the provided signature matches the input signer address. function validateSignature(address _signerAddress, bytes32 _hash, bytes memory _signature) internal view returns (bool) { if (_signerAddress.code.length > 0) { return ERC1271_MAGICVALUE == IERC1271Wallet(_signerAddress).isValidSignature(_hash, _signature); diff --git a/package.json b/package.json index ffdf8468..55303e50 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "compile": "forge build --force", "test-foundry-local": "DEPLOYED=false forge test --no-match-path 'test/forkMainnet/*.t.sol'", "test-foundry-fork": "DEPLOYED=false forge test --fork-url $MAINNET_NODE_RPC_URL --fork-block-number 17900000 --match-path 'test/forkMainnet/*.t.sol'", + "coverage": "DEPLOYED=false forge coverage --fork-url $MAINNET_NODE_RPC_URL --fork-block-number 17900000 --report summary", "gas-report-local": "yarn test-foundry-local --gas-report", "gas-report-fork": "yarn test-foundry-fork --gas-report" }, diff --git a/test/AllowanceTarget.t.sol b/test/AllowanceTarget.t.sol index 2df7f023..2539528b 100644 --- a/test/AllowanceTarget.t.sol +++ b/test/AllowanceTarget.t.sol @@ -103,6 +103,24 @@ contract AllowanceTargetTest is BalanceUtil { toBalance.assertChange(int256(amount)); } + function testSpendFromUserToAfterUnpause() public { + Snapshot memory fromBalance = BalanceSnapshot.take({ owner: user, token: address(mockERC20) }); + Snapshot memory toBalance = BalanceSnapshot.take({ owner: recipient, token: address(mockERC20) }); + + uint256 amount = 100; + + vm.startPrank(allowanceTargetOwner); + allowanceTarget.pause(); + allowanceTarget.unpause(); + vm.stopPrank(); + + vm.prank(authorized); + allowanceTarget.spendFromUserTo(user, address(mockERC20), recipient, amount); + + fromBalance.assertChange(-int256(amount)); + toBalance.assertChange(int256(amount)); + } + function testSpendFromUserToWithNoReturnValueToken() public { Snapshot memory fromBalance = BalanceSnapshot.take({ owner: user, token: address(noReturnERC20) }); Snapshot memory toBalance = BalanceSnapshot.take({ owner: recipient, token: address(noReturnERC20) }); diff --git a/test/abstracts/EIP712.t.sol b/test/abstracts/EIP712.t.sol new file mode 100644 index 00000000..423e18be --- /dev/null +++ b/test/abstracts/EIP712.t.sol @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import { Test } from "forge-std/Test.sol"; +import { EIP712 } from "contracts/abstracts/EIP712.sol"; + +contract EIP712Test is Test { + EIP712TestContract eip712; + + // Dummy struct hash for testing + bytes32 public constant DUMMY_STRUCT_HASH = keccak256("DummyStruct(string message)"); + + function setUp() public { + eip712 = new EIP712TestContract(); + } + + function testOriginalChainId() public { + uint256 chainId = block.chainid; + assertEq(eip712.originalChainId(), chainId); + } + + function testOriginalDomainSeparator() public { + bytes32 expectedDomainSeparator = eip712.calculateDomainSeparator(); + assertEq(eip712.originalEIP712DomainSeparator(), expectedDomainSeparator); + } + + function testGetEIP712Hash() public { + bytes32 structHash = DUMMY_STRUCT_HASH; + bytes32 domainSeparator = eip712.calculateDomainSeparator(); + bytes32 expectedEIP712Hash = keccak256(abi.encodePacked(eip712.EIP191_HEADER(), domainSeparator, structHash)); + + assertEq(eip712.getEIP712HashWrap(structHash), expectedEIP712Hash); + } + + function testDomainSeparatorOnDifferentChain() public { + uint256 chainId = block.chainid + 1234; + vm.chainId(chainId); + + bytes32 newDomainSeparator = eip712.calculateDomainSeparator(); + assertEq(eip712.EIP712_DOMAIN_SEPARATOR(), newDomainSeparator, "Domain separator should match the expected value on a different chain"); + } +} + +contract EIP712TestContract is EIP712 { + function calculateDomainSeparator() external view returns (bytes32) { + return keccak256(abi.encode(EIP712_TYPE_HASH, keccak256(bytes(EIP712_NAME)), keccak256(bytes(EIP712_VERSION)), block.chainid, address(this))); + } + + function getEIP712HashWrap(bytes32 structHash) public view returns (bytes32) { + return super.getEIP712Hash(structHash); + } +} diff --git a/test/abstracts/Ownable.t.sol b/test/abstracts/Ownable.t.sol new file mode 100644 index 00000000..6c4b5c2a --- /dev/null +++ b/test/abstracts/Ownable.t.sol @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import { Test } from "forge-std/Test.sol"; +import { Ownable } from "contracts/abstracts/Ownable.sol"; + +contract OwnableTest is Test { + OwnableTestContract ownable; + + address owner = makeAddr("owner"); + address newOwner = makeAddr("newOwner"); + address nominatedOwner = makeAddr("nominatedOwner"); + address otherAccount = makeAddr("otherAccount"); + + function setUp() public { + vm.prank(owner); + ownable = new OwnableTestContract(owner); + } + + function testOwnableInitialState() public { + assertEq(ownable.owner(), owner); + } + + function testCannotInitiateOwnerWithZeroAddress() public { + vm.expectRevert(Ownable.ZeroOwner.selector); + new OwnableTestContract(address(0)); + } + + function testCannotAcceptOwnershipWithOtherAccount() public { + vm.prank(owner); + ownable.nominateNewOwner(newOwner); + + vm.prank(otherAccount); + vm.expectRevert(Ownable.NotNominated.selector); + ownable.acceptOwnership(); + } + + function testCannotRenounceOwnershipWithNominatedOwner() public { + vm.prank(owner); + ownable.nominateNewOwner(newOwner); + + vm.prank(owner); + vm.expectRevert(Ownable.NominationExists.selector); + ownable.renounceOwnership(); + } + + function testCannotRenounceOwnershipWithOtherAccount() public { + vm.prank(otherAccount); + vm.expectRevert(Ownable.NotOwner.selector); + ownable.renounceOwnership(); + } + + function testCannotNominateNewOwnerWithOtherAccount() public { + vm.prank(otherAccount); + vm.expectRevert(Ownable.NotOwner.selector); + ownable.nominateNewOwner(newOwner); + } + + function testAcceptOwnership() public { + vm.prank(owner); + ownable.nominateNewOwner(newOwner); + + assertEq(ownable.nominatedOwner(), newOwner); + + vm.prank(newOwner); + vm.expectEmit(true, true, false, false); + emit Ownable.OwnerChanged(owner, newOwner); + ownable.acceptOwnership(); + + assertEq(ownable.owner(), newOwner); + assertEq(ownable.nominatedOwner(), address(0)); + } + + function testRenounceOwnership() public { + vm.prank(owner); + vm.expectEmit(true, true, false, false); + emit Ownable.OwnerChanged(owner, address(0)); + ownable.renounceOwnership(); + + assertEq(ownable.owner(), address(0)); + } + + function testNominateNewOwner() public { + vm.prank(owner); + vm.expectEmit(true, false, false, false); + emit Ownable.OwnerNominated(newOwner); + ownable.nominateNewOwner(newOwner); + + assertEq(ownable.nominatedOwner(), newOwner); + } +} + +contract OwnableTestContract is Ownable { + constructor(address _owner) Ownable(_owner) {} +} diff --git a/test/forkMainnet/GenericSwap.t.sol b/test/forkMainnet/GenericSwap.t.sol index c8d3c372..b71e4999 100644 --- a/test/forkMainnet/GenericSwap.t.sol +++ b/test/forkMainnet/GenericSwap.t.sol @@ -129,6 +129,13 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper defaultTakerPermit = getTokenlonPermit2Data(taker, takerPrivateKey, defaultGSData.takerToken, address(genericSwap)); } + function testGenericSwapInitialState() public { + genericSwap = new GenericSwap(UNISWAP_PERMIT2_ADDRESS, address(allowanceTarget)); + + assertEq(genericSwap.permit2(), UNISWAP_PERMIT2_ADDRESS); + assertEq(genericSwap.allowanceTarget(), address(allowanceTarget)); + } + function testGenericSwapWithUniswap() public { Snapshot memory takerTakerToken = BalanceSnapshot.take({ owner: taker, token: defaultGSData.takerToken }); Snapshot memory takerMakerToken = BalanceSnapshot.take({ owner: taker, token: defaultGSData.makerToken }); diff --git a/test/forkMainnet/LimitOrderSwap/CoordinatedTaker.t.sol b/test/forkMainnet/LimitOrderSwap/CoordinatedTaker.t.sol index 6005bb6b..636848d6 100644 --- a/test/forkMainnet/LimitOrderSwap/CoordinatedTaker.t.sol +++ b/test/forkMainnet/LimitOrderSwap/CoordinatedTaker.t.sol @@ -28,7 +28,7 @@ contract CoordinatedTakerTest is LimitOrderSwapTest { uint256 crdPrivateKey = uint256(2); address coordinator = vm.addr(crdPrivateKey); - bytes defaultUserPrmit; + bytes defaultUserPermit; LimitOrder defaultCrdOrder; AllowFill defaultAllowFill; ICoordinatedTaker.CoordinatorParams defaultCRDParams; @@ -58,7 +58,7 @@ contract CoordinatedTakerTest is LimitOrderSwapTest { defaultMakerSig = signLimitOrder(makerPrivateKey, defaultCrdOrder, address(limitOrderSwap)); - defaultUserPrmit = getTokenlonPermit2Data(user, userPrivateKey, defaultCrdOrder.takerToken, address(coordinatedTaker)); + defaultUserPermit = getTokenlonPermit2Data(user, userPrivateKey, defaultCrdOrder.takerToken, address(coordinatedTaker)); defaultAllowFill = AllowFill({ orderHash: getLimitOrderHash(defaultCrdOrder), @@ -75,6 +75,24 @@ contract CoordinatedTakerTest is LimitOrderSwapTest { }); } + function testCoordinatedTakerInitialState() public { + coordinatedTaker = new CoordinatedTaker( + crdTakerOwner, + UNISWAP_PERMIT2_ADDRESS, + address(allowanceTarget), + IWETH(WETH_ADDRESS), + coordinator, + ILimitOrderSwap(address(limitOrderSwap)) + ); + + assertEq(address(coordinatedTaker.owner()), crdTakerOwner); + assertEq(coordinatedTaker.permit2(), UNISWAP_PERMIT2_ADDRESS); + assertEq(coordinatedTaker.allowanceTarget(), address(allowanceTarget)); + assertEq(address(coordinatedTaker.weth()), WETH_ADDRESS); + assertEq(coordinatedTaker.coordinator(), coordinator); + assertEq(address(coordinatedTaker.limitOrderSwap()), address(limitOrderSwap)); + } + function testCannotSetCoordinatorByNotOwner() public { address newCoordinator = makeAddr("newCoordinator"); vm.prank(newCoordinator); @@ -152,7 +170,7 @@ contract CoordinatedTakerTest is LimitOrderSwapTest { takerTokenAmount: defaultCrdOrder.takerTokenAmount, makerTokenAmount: defaultCrdOrder.makerTokenAmount, extraAction: bytes(""), - userTokenPermit: defaultUserPrmit, + userTokenPermit: defaultUserPermit, crdParams: defaultCRDParams }); @@ -216,7 +234,7 @@ contract CoordinatedTakerTest is LimitOrderSwapTest { takerTokenAmount: order.takerTokenAmount, makerTokenAmount: order.makerTokenAmount, extraAction: bytes(""), - userTokenPermit: defaultUserPrmit, + userTokenPermit: defaultUserPermit, crdParams: crdParams }); @@ -238,7 +256,7 @@ contract CoordinatedTakerTest is LimitOrderSwapTest { takerTokenAmount: defaultCrdOrder.takerTokenAmount, makerTokenAmount: defaultCrdOrder.makerTokenAmount, extraAction: bytes(""), - userTokenPermit: defaultUserPrmit, + userTokenPermit: defaultUserPermit, crdParams: defaultCRDParams }); } @@ -258,7 +276,7 @@ contract CoordinatedTakerTest is LimitOrderSwapTest { takerTokenAmount: defaultCrdOrder.takerTokenAmount, makerTokenAmount: defaultCrdOrder.makerTokenAmount, extraAction: bytes(""), - userTokenPermit: defaultUserPrmit, + userTokenPermit: defaultUserPermit, crdParams: crdParams }); } @@ -271,7 +289,7 @@ contract CoordinatedTakerTest is LimitOrderSwapTest { takerTokenAmount: defaultCrdOrder.takerTokenAmount, makerTokenAmount: defaultCrdOrder.makerTokenAmount, extraAction: bytes(""), - userTokenPermit: defaultUserPrmit, + userTokenPermit: defaultUserPermit, crdParams: defaultCRDParams }); @@ -297,7 +315,7 @@ contract CoordinatedTakerTest is LimitOrderSwapTest { takerTokenAmount: defaultCrdOrder.takerTokenAmount, makerTokenAmount: defaultCrdOrder.makerTokenAmount, extraAction: bytes(""), - userTokenPermit: defaultUserPrmit, + userTokenPermit: defaultUserPermit, crdParams: defaultCRDParams }); } diff --git a/test/forkMainnet/LimitOrderSwap/Setup.t.sol b/test/forkMainnet/LimitOrderSwap/Setup.t.sol index ac995511..2cd9df9e 100644 --- a/test/forkMainnet/LimitOrderSwap/Setup.t.sol +++ b/test/forkMainnet/LimitOrderSwap/Setup.t.sol @@ -112,4 +112,14 @@ contract LimitOrderSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelp vm.label(taker, "taker"); vm.label(maker, "maker"); } + + function testLimitOrderSwapInitialState() public virtual { + limitOrderSwap = new LimitOrderSwap(limitOrderOwner, UNISWAP_PERMIT2_ADDRESS, address(allowanceTarget), IWETH(WETH_ADDRESS), feeCollector); + + assertEq(limitOrderSwap.owner(), limitOrderOwner); + assertEq(limitOrderSwap.permit2(), UNISWAP_PERMIT2_ADDRESS); + assertEq(limitOrderSwap.allowanceTarget(), address(allowanceTarget)); + assertEq(address(limitOrderSwap.weth()), WETH_ADDRESS); + assertEq(limitOrderSwap.feeCollector(), feeCollector); + } } diff --git a/test/forkMainnet/LimitOrderSwap/Validation.t.sol b/test/forkMainnet/LimitOrderSwap/Validation.t.sol new file mode 100644 index 00000000..bfa4e72d --- /dev/null +++ b/test/forkMainnet/LimitOrderSwap/Validation.t.sol @@ -0,0 +1,157 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import { LimitOrderSwapTest } from "./Setup.t.sol"; +import { ILimitOrderSwap } from "contracts/interfaces/ILimitOrderSwap.sol"; +import { LimitOrder } from "contracts/libraries/LimitOrder.sol"; + +contract ValidationTest is LimitOrderSwapTest { + function testCannotFillLimitOrderWithZeroTakerTokenAmount() public { + LimitOrder memory order = defaultOrder; + order.takerTokenAmount = 0; + + bytes memory makerSig = signLimitOrder(makerPrivateKey, order, address(limitOrderSwap)); + + vm.expectRevert(ILimitOrderSwap.ZeroTakerTokenAmount.selector); + limitOrderSwap.fillLimitOrder({ + order: order, + makerSignature: makerSig, + takerParams: ILimitOrderSwap.TakerParams({ + takerTokenAmount: order.takerTokenAmount, + makerTokenAmount: order.makerTokenAmount, + recipient: recipient, + extraAction: bytes(""), + takerTokenPermit: defaultTakerPermit + }) + }); + } + + function testCannotFillLimitOrderWithZeroMakerTokenAmount() public { + LimitOrder memory order = defaultOrder; + order.makerTokenAmount = 0; + + bytes memory makerSig = signLimitOrder(makerPrivateKey, order, address(limitOrderSwap)); + + vm.expectRevert(ILimitOrderSwap.ZeroMakerTokenAmount.selector); + limitOrderSwap.fillLimitOrder({ + order: order, + makerSignature: makerSig, + takerParams: ILimitOrderSwap.TakerParams({ + takerTokenAmount: order.takerTokenAmount, + makerTokenAmount: order.makerTokenAmount, + recipient: recipient, + extraAction: bytes(""), + takerTokenPermit: defaultTakerPermit + }) + }); + } + + function testCannotFillLimitOrderWithZeroTakerSpendingAmount() public { + vm.expectRevert(ILimitOrderSwap.ZeroTakerSpendingAmount.selector); + limitOrderSwap.fillLimitOrder({ + order: defaultOrder, + makerSignature: defaultMakerSig, + takerParams: ILimitOrderSwap.TakerParams({ + takerTokenAmount: 0, + makerTokenAmount: defaultOrder.makerTokenAmount, + recipient: recipient, + extraAction: bytes(""), + takerTokenPermit: defaultTakerPermit + }) + }); + } + + function testCannotFillLimitOrderWithZeroTakerSpendingAmountWhenRecalculation() public { + // this case tests if _takerTokenAmount is zero due to re-calculation. + vm.expectRevert(ILimitOrderSwap.ZeroTakerSpendingAmount.selector); + vm.prank(taker); + limitOrderSwap.fillLimitOrder({ + order: defaultOrder, + makerSignature: defaultMakerSig, + takerParams: ILimitOrderSwap.TakerParams({ + takerTokenAmount: 1, + makerTokenAmount: 1000 ether, + recipient: recipient, + extraAction: bytes(""), + takerTokenPermit: defaultTakerPermit + }) + }); + } + + function testCannotFillLimitOrderWithZeroMakerSpendingAmount() public { + vm.expectRevert(ILimitOrderSwap.ZeroMakerSpendingAmount.selector); + limitOrderSwap.fillLimitOrder({ + order: defaultOrder, + makerSignature: defaultMakerSig, + takerParams: ILimitOrderSwap.TakerParams({ + takerTokenAmount: defaultOrder.takerTokenAmount, + makerTokenAmount: 0, + recipient: recipient, + extraAction: bytes(""), + takerTokenPermit: defaultTakerPermit + }) + }); + } + + function testCannotFillLimitOrderGroupWithInvalidParams() public { + LimitOrder[] memory orders = new LimitOrder[](1); + bytes[] memory makerSigs = new bytes[](2); + uint256[] memory makerTokenAmounts = new uint256[](3); + address[] memory profitTokens = new address[](1); + + vm.expectRevert(ILimitOrderSwap.InvalidParams.selector); + limitOrderSwap.fillLimitOrderGroup({ orders: orders, makerSignatures: makerSigs, makerTokenAmounts: makerTokenAmounts, profitTokens: profitTokens }); + } + + function testCannotFillLimitOrderGroupWithNotEnoughForFill() public { + LimitOrder[] memory orders = new LimitOrder[](1); + bytes[] memory makerSigs = new bytes[](1); + uint256[] memory makerTokenAmounts = new uint256[](1); + address[] memory profitTokens = new address[](1); + + // order 10 DAI -> 10 USDT + orders[0] = LimitOrder({ + taker: address(0), + maker: maker, + takerToken: USDT_ADDRESS, + takerTokenAmount: 10 * 1e6, + makerToken: DAI_ADDRESS, + makerTokenAmount: 10 ether, + makerTokenPermit: defaultMakerPermit, + feeFactor: 0, + expiry: defaultExpiry, + salt: defaultSalt + }); + makerSigs[0] = signLimitOrder(makerPrivateKey, orders[0], address(limitOrderSwap)); + makerTokenAmounts[0] = orders[0].makerTokenAmount + 1; + + vm.expectRevert(ILimitOrderSwap.NotEnoughForFill.selector); + limitOrderSwap.fillLimitOrderGroup({ orders: orders, makerSignatures: makerSigs, makerTokenAmounts: makerTokenAmounts, profitTokens: profitTokens }); + } + + function testCannotFillLimitOrderGroupWithZeroMakerSpendingAmount() public { + LimitOrder[] memory orders = new LimitOrder[](1); + bytes[] memory makerSigs = new bytes[](1); + uint256[] memory makerTokenAmounts = new uint256[](1); + address[] memory profitTokens = new address[](1); + + // order 10 DAI -> 10 USDT + orders[0] = LimitOrder({ + taker: address(0), + maker: maker, + takerToken: USDT_ADDRESS, + takerTokenAmount: 10 * 10e6, + makerToken: DAI_ADDRESS, + makerTokenAmount: 10 ether, + makerTokenPermit: defaultMakerPermit, + feeFactor: 0, + expiry: defaultExpiry, + salt: defaultSalt + }); + makerSigs[0] = signLimitOrder(makerPrivateKey, orders[0], address(limitOrderSwap)); + makerTokenAmounts[0] = 0; + + vm.expectRevert(ILimitOrderSwap.ZeroMakerSpendingAmount.selector); + limitOrderSwap.fillLimitOrderGroup({ orders: orders, makerSignatures: makerSigs, makerTokenAmounts: makerTokenAmounts, profitTokens: profitTokens }); + } +} diff --git a/test/forkMainnet/RFQ.t.sol b/test/forkMainnet/RFQ.t.sol index 65e0198a..3d1f37ec 100644 --- a/test/forkMainnet/RFQ.t.sol +++ b/test/forkMainnet/RFQ.t.sol @@ -14,10 +14,9 @@ import { Ownable } from "contracts/abstracts/Ownable.sol"; import { AllowanceTarget } from "contracts/AllowanceTarget.sol"; import { IRFQ } from "contracts/interfaces/IRFQ.sol"; import { IWETH } from "contracts/interfaces/IWETH.sol"; -import { IUniswapPermit2 } from "contracts/interfaces/IUniswapPermit2.sol"; import { TokenCollector } from "contracts/abstracts/TokenCollector.sol"; import { RFQOffer, getRFQOfferHash } from "contracts/libraries/RFQOffer.sol"; -import { RFQTx, getRFQTxHash } from "contracts/libraries/RFQTx.sol"; +import { RFQTx } from "contracts/libraries/RFQTx.sol"; import { Constant } from "contracts/libraries/Constant.sol"; contract RFQTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper { @@ -103,6 +102,16 @@ contract RFQTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper { vm.label(address(rfq), "rfq"); } + function testRFQInitialState() public { + rfq = new RFQ(rfqOwner, UNISWAP_PERMIT2_ADDRESS, address(allowanceTarget), IWETH(WETH_ADDRESS), feeCollector); + + assertEq(rfq.owner(), rfqOwner); + assertEq(rfq.permit2(), UNISWAP_PERMIT2_ADDRESS); + assertEq(rfq.allowanceTarget(), address(allowanceTarget)); + assertEq(address(rfq.weth()), WETH_ADDRESS); + assertEq(rfq.feeCollector(), feeCollector); + } + function testCannotSetFeeCollectorByNotOwner() public { address newFeeCollector = makeAddr("newFeeCollector"); vm.prank(newFeeCollector); @@ -243,7 +252,7 @@ contract RFQTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper { fcMakerToken.assertChange(int256(fee)); } - function testFillRFQWithRawETHAndRecieveWETH() public { + function testFillRFQWithRawETHAndReceiveWETH() public { // case : taker token is ETH RFQOffer memory rfqOffer = defaultRFQOffer; rfqOffer.takerToken = Constant.ZERO_ADDRESS; @@ -282,9 +291,8 @@ contract RFQTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper { } function testFillRFQTakerGetRawETH() public { - // case : maker token is WETH RFQOffer memory rfqOffer = defaultRFQOffer; - rfqOffer.makerToken = WETH_ADDRESS; + rfqOffer.makerToken = Constant.ETH_ADDRESS; rfqOffer.makerTokenAmount = 1 ether; bytes memory makerSig = signRFQOffer(makerSignerPrivateKey, rfqOffer, address(rfq)); @@ -292,7 +300,43 @@ contract RFQTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper { Snapshot memory takerTakerToken = BalanceSnapshot.take({ owner: rfqOffer.taker, token: rfqOffer.takerToken }); Snapshot memory takerMakerToken = BalanceSnapshot.take({ owner: rfqOffer.taker, token: rfqOffer.makerToken }); Snapshot memory makerTakerToken = BalanceSnapshot.take({ owner: rfqOffer.maker, token: rfqOffer.takerToken }); - Snapshot memory makerMakerToken = BalanceSnapshot.take({ owner: rfqOffer.maker, token: rfqOffer.makerToken }); + // market maker only receives WETH + Snapshot memory makerMakerToken = BalanceSnapshot.take({ owner: rfqOffer.maker, token: WETH_ADDRESS }); + // recipient should receive raw ETH + Snapshot memory recTakerToken = BalanceSnapshot.take({ owner: recipient, token: rfqOffer.takerToken }); + Snapshot memory recMakerToken = BalanceSnapshot.take({ owner: recipient, token: rfqOffer.makerToken }); + Snapshot memory fcMakerToken = BalanceSnapshot.take({ owner: feeCollector, token: rfqOffer.makerToken }); + + uint256 fee = (rfqOffer.makerTokenAmount * defaultFeeFactor) / Constant.BPS_MAX; + uint256 amountAfterFee = rfqOffer.makerTokenAmount - fee; + + RFQTx memory rfqTx = defaultRFQTx; + rfqTx.rfqOffer = rfqOffer; + + vm.prank(rfqOffer.taker, rfqOffer.taker); + rfq.fillRFQ(rfqTx, makerSig, defaultMakerPermit, defaultTakerPermit); + + takerTakerToken.assertChange(-int256(rfqOffer.takerTokenAmount)); + takerMakerToken.assertChange(int256(0)); + makerTakerToken.assertChange(int256(rfqOffer.takerTokenAmount)); + makerMakerToken.assertChange(-int256(rfqOffer.makerTokenAmount)); + recTakerToken.assertChange(int256(0)); + // recipient gets less than original makerTokenAmount because of the fee + recMakerToken.assertChange(int256(amountAfterFee)); + fcMakerToken.assertChange(int256(fee)); + } + + function testFillRFQTakerGetRawETH2() public { + RFQOffer memory rfqOffer = defaultRFQOffer; + rfqOffer.makerToken = Constant.ZERO_ADDRESS; + rfqOffer.makerTokenAmount = 1 ether; + + bytes memory makerSig = signRFQOffer(makerSignerPrivateKey, rfqOffer, address(rfq)); + + Snapshot memory takerTakerToken = BalanceSnapshot.take({ owner: rfqOffer.taker, token: rfqOffer.takerToken }); + Snapshot memory takerMakerToken = BalanceSnapshot.take({ owner: rfqOffer.taker, token: rfqOffer.makerToken }); + Snapshot memory makerTakerToken = BalanceSnapshot.take({ owner: rfqOffer.maker, token: rfqOffer.takerToken }); + Snapshot memory makerMakerToken = BalanceSnapshot.take({ owner: rfqOffer.maker, token: WETH_ADDRESS }); // recipient should receive raw ETH Snapshot memory recTakerToken = BalanceSnapshot.take({ owner: recipient, token: rfqOffer.takerToken }); Snapshot memory recMakerToken = BalanceSnapshot.take({ owner: recipient, token: rfqOffer.makerToken }); @@ -355,7 +399,7 @@ contract RFQTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper { fcMakerToken.assertChange(int256(fee)); } - function testFillRFQWithWETHAndRecieveWETH() public { + function testFillRFQWithWETHAndReceiveWETH() public { // case : taker token is WETH RFQOffer memory rfqOffer = defaultRFQOffer; rfqOffer.takerToken = WETH_ADDRESS; @@ -480,6 +524,15 @@ contract RFQTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper { rfq.fillRFQ(rfqTx1, makerSig, defaultMakerPermit, defaultTakerPermit); } + function testCannotFillWithContractWhenNotAllowContractSender() public { + RFQTx memory rfqTx = defaultRFQTx; + address mockContract = makeAddr("mockContract"); + + vm.expectRevert(IRFQ.ForbidContract.selector); + vm.prank(mockContract, defaultRFQOffer.taker); + rfq.fillRFQ(rfqTx, defaultMakerSig, defaultMakerPermit, defaultTakerPermit); + } + function testCannotFillExpiredRFQTx() public { vm.warp(defaultRFQOffer.expiry + 1); diff --git a/test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol b/test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol index 6787006c..f767d273 100644 --- a/test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol +++ b/test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol @@ -12,6 +12,7 @@ import { IWETH } from "contracts/interfaces/IWETH.sol"; import { ISmartOrderStrategy } from "contracts/interfaces/ISmartOrderStrategy.sol"; import { ILimitOrderSwap } from "contracts/interfaces/ILimitOrderSwap.sol"; import { TokenCollector } from "contracts/abstracts/TokenCollector.sol"; +import { Constant } from "contracts/libraries/Constant.sol"; import { RFQOffer, getRFQOfferHash } from "contracts/libraries/RFQOffer.sol"; import { RFQTx } from "contracts/libraries/RFQTx.sol"; import { LimitOrder, getLimitOrderHash } from "contracts/libraries/LimitOrder.sol"; @@ -94,6 +95,85 @@ contract IntegrationV6Test is SmartOrderStrategyTest, SigHelper { gsOutputToken.assertChange(int256(realChangedInGS)); } + function testV6RFQIntegrationWhenTakerTokenIsETH() public { + RFQOffer memory rfqOffer = RFQOffer({ + taker: address(smartOrderStrategy), + maker: payable(maker), + takerToken: Constant.ETH_ADDRESS, + takerTokenAmount: 1 ether, + makerToken: LON_ADDRESS, + makerTokenAmount: 1000 ether, + feeFactor: 0, + flags: FLG_ALLOW_CONTRACT_SENDER, + expiry: defaultExpiry, + salt: defaultSalt + }); + + uint256 realChangedInGS = rfqOffer.makerTokenAmount - 1; // leaving 1 wei in GS + + RFQTx memory rfqTx = RFQTx({ rfqOffer: rfqOffer, takerRequestAmount: rfqOffer.takerTokenAmount, recipient: payable(address(smartOrderStrategy)) }); + bytes memory makerSig = signRFQOffer(makerPrivateKey, rfqOffer, address(rfq)); + bytes memory rfqData = abi.encodeWithSelector(RFQ_FILL_SELECTOR, rfqTx, makerSig, defaultPermit, defaultPermit); + + ISmartOrderStrategy.Operation[] memory operations = new ISmartOrderStrategy.Operation[](1); + operations[0] = ISmartOrderStrategy.Operation({ + dest: address(rfq), + inputToken: rfqOffer.takerToken, + ratioNumerator: 0, // zero ratio indicate no replacement + ratioDenominator: 0, + dataOffset: 0, + value: rfqOffer.takerTokenAmount, + data: rfqData + }); + bytes memory opsData = abi.encode(operations); + + vm.startPrank(genericSwap, genericSwap); + vm.deal(address(smartOrderStrategy), rfqOffer.takerTokenAmount); + Snapshot memory sosInputToken = BalanceSnapshot.take(address(smartOrderStrategy), rfqOffer.takerToken); + Snapshot memory gsOutputToken = BalanceSnapshot.take(genericSwap, rfqOffer.makerToken); + smartOrderStrategy.executeStrategy{ value: rfqOffer.takerTokenAmount }(rfqOffer.takerToken, rfqOffer.makerToken, rfqOffer.takerTokenAmount, opsData); + vm.stopPrank(); + + sosInputToken.assertChange(-int256(rfqOffer.takerTokenAmount)); + gsOutputToken.assertChange(int256(realChangedInGS)); + } + + function testV6RFQIntegrationWhenMakerTokenIsETH() public { + RFQOffer memory rfqOffer = RFQOffer({ + taker: address(smartOrderStrategy), + maker: payable(maker), + takerToken: USDT_ADDRESS, + takerTokenAmount: 10 * 1e6, + makerToken: Constant.ETH_ADDRESS, + makerTokenAmount: 1 ether, + feeFactor: 0, + flags: FLG_ALLOW_CONTRACT_SENDER, + expiry: defaultExpiry, + salt: defaultSalt + }); + + RFQTx memory rfqTx = RFQTx({ rfqOffer: rfqOffer, takerRequestAmount: rfqOffer.takerTokenAmount, recipient: payable(address(smartOrderStrategy)) }); + bytes memory makerSig = signRFQOffer(makerPrivateKey, rfqOffer, address(rfq)); + bytes memory rfqData = abi.encodeWithSelector(RFQ_FILL_SELECTOR, rfqTx, makerSig, defaultPermit, defaultPermit); + + ISmartOrderStrategy.Operation[] memory operations = new ISmartOrderStrategy.Operation[](1); + operations[0] = ISmartOrderStrategy.Operation({ + dest: address(rfq), + inputToken: rfqOffer.takerToken, + ratioNumerator: 0, // zero ratio indicate no replacement + ratioDenominator: 0, + dataOffset: 0, + value: 0, + data: rfqData + }); + bytes memory opsData = abi.encode(operations); + + vm.startPrank(genericSwap, genericSwap); + IERC20(rfqOffer.takerToken).safeTransfer(address(smartOrderStrategy), rfqOffer.takerTokenAmount); + smartOrderStrategy.executeStrategy(rfqOffer.takerToken, rfqOffer.makerToken, rfqOffer.takerTokenAmount, opsData); + vm.stopPrank(); + } + function testV6LOIntegration() public { LimitOrder memory order = LimitOrder({ taker: address(0), diff --git a/test/forkMainnet/SmartOrderStrategy/Validation.t.sol b/test/forkMainnet/SmartOrderStrategy/Validation.t.sol index 59c91d92..be810935 100644 --- a/test/forkMainnet/SmartOrderStrategy/Validation.t.sol +++ b/test/forkMainnet/SmartOrderStrategy/Validation.t.sol @@ -63,4 +63,23 @@ contract ValidationTest is SmartOrderStrategyTest { vm.prank(genericSwap, genericSwap); smartOrderStrategy.executeStrategy{ value: 1 }(defaultInputToken, defaultOutputToken, defaultInputAmount, defaultOpsData); } + + function testCannotExecuteAnOperationWillFail() public { + ISmartOrderStrategy.Operation[] memory operations = new ISmartOrderStrategy.Operation[](1); + operations[0] = ISmartOrderStrategy.Operation({ + dest: defaultInputToken, + inputToken: defaultInputToken, + ratioNumerator: 0, + ratioDenominator: 0, + dataOffset: 0, + value: 0, + data: abi.encode("invalid data") + }); + bytes memory opsData = abi.encode(operations); + + vm.startPrank(genericSwap, genericSwap); + vm.expectRevert(); + smartOrderStrategy.executeStrategy(defaultInputToken, defaultOutputToken, defaultInputAmount, opsData); + vm.stopPrank(); + } } diff --git a/test/forkMainnet/TokenCollector.t.sol b/test/forkMainnet/TokenCollector.t.sol index 5c1684bf..9c4dcf9c 100644 --- a/test/forkMainnet/TokenCollector.t.sol +++ b/test/forkMainnet/TokenCollector.t.sol @@ -53,6 +53,15 @@ contract TestTokenCollector is Addresses, Permit2Helper { vm.label(address(token), "TKN"); } + function testCannotCollectByInvalidSource() public { + uint8 invalidSource = 255; + bytes memory data = abi.encodePacked(invalidSource); + + // failed to convert value into enum type + vm.expectRevert(); + strategy.collect(address(token), user, address(this), 0, data); + } + /* Token Approval */ function testCannotCollectByTokenApprovalWhenAllowanceIsNotEnough() public { @@ -209,6 +218,12 @@ contract TestTokenCollector is Addresses, Permit2Helper { } /* Permit2 Allowance Transfer */ + function testCannotCollectByPermit2DataIsEmpty() public { + bytes memory data = abi.encodePacked(TokenCollector.Source.Permit2SignatureTransfer, ""); + + vm.expectRevert(TokenCollector.Permit2DataEmpty.selector); + strategy.collect(address(token), user, address(this), 0, data); + } function testCannotCollectByPermit2AllowanceTransferWhenPermitSigIsInvalid() public { IUniswapPermit2.PermitSingle memory permit = DEFAULT_PERMIT_SINGLE; diff --git a/test/libraries/Asset.t.sol b/test/libraries/Asset.t.sol new file mode 100644 index 00000000..be469116 --- /dev/null +++ b/test/libraries/Asset.t.sol @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.26; + +import { Test } from "forge-std/Test.sol"; +import { Asset } from "contracts/libraries/Asset.sol"; +import { Constant } from "contracts/libraries/Constant.sol"; +import { MockERC20 } from "test/mocks/MockERC20.sol"; + +contract AssetTest is Test { + using Asset for address; + + MockERC20 token; + + address payable recipient = payable(makeAddr("recipient")); + uint256 tokenBalance = 123; + uint256 ethBalance = 456; + + function setUp() public { + token = new MockERC20("TOKEN", "TKN", 18); + + // set balance + token.mint(address(this), tokenBalance); + vm.deal(address(this), ethBalance); + } + + function transferToWrap(address asset, address payable to, uint256 amount) public { + Asset.transferTo(asset, to, amount); + } + + function testIsETH() public { + assertTrue(Asset.isETH(Constant.ETH_ADDRESS)); + assertTrue(Asset.isETH(address(0))); + } + + function testGetBalance() public { + assertEq(Asset.getBalance(address(token), address(this)), tokenBalance); + assertEq(Asset.getBalance(Constant.ETH_ADDRESS, address(this)), ethBalance); + assertEq(Asset.getBalance(address(0), address(this)), ethBalance); + } + + function testDoNothingIfTransferWithZeroAmount() public { + Asset.transferTo(address(token), recipient, 0); + } + + function testDoNothingIfTransferToSelf() public { + Asset.transferTo(address(token), payable(address(token)), 0); + } + + function testTransferETHWithInsufficientBalance() public { + vm.expectRevert(Asset.InsufficientBalance.selector); + this.transferToWrap(Constant.ETH_ADDRESS, recipient, address(this).balance + 1); + } + + function testTransferETHToContractCannotReceiveETH() public { + vm.expectRevert(); + // mockERC20 cannot receive any ETH + this.transferToWrap(Constant.ETH_ADDRESS, payable(address(token)), 1); + } + + function testTransferETH() public { + uint256 amount = address(this).balance; + Asset.transferTo(Constant.ETH_ADDRESS, payable(recipient), amount); + + assertEq(address(recipient).balance, amount); + assertEq(address(this).balance, 0); + } + + function testTransferToken() public { + uint256 amount = token.balanceOf(address(this)); + Asset.transferTo(address(token), payable(recipient), amount); + + assertEq(token.balanceOf(recipient), amount); + assertEq(token.balanceOf(address(this)), 0); + } +} diff --git a/test/libraries/SignatureValidator.t.sol b/test/libraries/SignatureValidator.t.sol index 38ace49e..81218957 100644 --- a/test/libraries/SignatureValidator.t.sol +++ b/test/libraries/SignatureValidator.t.sol @@ -17,10 +17,10 @@ contract SignatureValidatorTest is Test { mockERC1271Wallet = new MockERC1271Wallet(vm.addr(walletAdminPrivateKey)); } - // this is a workaround for library contract tesets + // this is a workaround for library contract tests // assertion may not working for library internal functions // https://github.com/foundry-rs/foundry/issues/4405 - function _isValidSignatureWrap(address _signerAddress, bytes32 _hash, bytes memory _signature) public view returns (bool) { + function validateSignatureWrap(address _signerAddress, bytes32 _hash, bytes memory _signature) public view returns (bool) { return SignatureValidator.validateSignature(_signerAddress, _hash, _signature); } @@ -67,12 +67,31 @@ contract SignatureValidatorTest is Test { assertTrue(SignatureValidator.validateSignature(address(mockERC1271Wallet), digest, signature)); } + function testEIP1271WithWrongSignatureLength() public { + uint256 v = 1; + uint256 r = 2; + uint256 s = 3; + // should have 96 bytes signature + bytes memory signature = abi.encodePacked(r, s, v); + // will be reverted in OZ ECDSA lib + vm.expectRevert(abi.encodeWithSelector(ECDSA.ECDSAInvalidSignatureLength.selector, signature.length)); + SignatureValidator.validateSignature(address(mockERC1271Wallet), digest, signature); + } + function testEIP1271WithDifferentSigner() public { (uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, digest); bytes memory signature = abi.encodePacked(r, s, v); assertFalse(SignatureValidator.validateSignature(address(mockERC1271Wallet), digest, signature)); } + function testEIP1271WithInvalidSignatureS() public { + (uint8 v, bytes32 r, ) = vm.sign(userPrivateKey, digest); + bytes memory signature = abi.encodePacked(r, r, v); + + vm.expectRevert(abi.encodeWithSelector(ECDSA.ECDSAInvalidSignatureS.selector, r)); + SignatureValidator.validateSignature(address(mockERC1271Wallet), digest, signature); + } + function testEIP1271WithZeroAddressSigner() public { (, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, digest); // change the value of v so ecrecover will return address(0) @@ -80,7 +99,7 @@ contract SignatureValidatorTest is Test { // OZ ECDSA lib will handle the zero address case and throw error instead // so the zero address will never be matched vm.expectRevert(ECDSA.ECDSAInvalidSignature.selector); - this._isValidSignatureWrap(address(0), digest, signature); + this.validateSignatureWrap(address(mockERC1271Wallet), digest, signature); } function testEIP1271WithWrongReturnValue() public {