Skip to content

Commit

Permalink
refine code base on PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@alex0207s
alex0207s committed Jul 15, 2024
1 parent d5fea53 commit 3c437d6
Show file tree
Hide file tree
Showing 10 changed files with 11 additions and 117 deletions.
14 changes: 3 additions & 11 deletions contracts/AllowanceTarget.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@ contract AllowanceTarget is IAllowanceTarget, Pausable, Ownable {
using SafeERC20 for IERC20;

/// @notice Mapping of authorized addresses permitted to call spendFromUserTo.
/// @dev Tracks the authorization status of each address to execute the spendFromUserTo function.
mapping(address => bool) public authorized;
mapping(address trustedCaller => bool isAuthorized) public authorized;

/// @notice Constructor to initialize the contract with the owner and trusted callers.
/// @dev Sets up the initial contract owner and authorizes a list of 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) {
Expand All @@ -30,23 +28,17 @@ contract AllowanceTarget is IAllowanceTarget, Pausable, Ownable {
}

/// @notice Pauses the contract, preventing the execution of spendFromUserTo.
/// @dev Pauses the Contract. Only the owner can call this function.
/// @dev Only the owner can call this function.
function pause() external onlyOwner {
_pause();
}

/// @notice Unpauses the contract, allowing the execution of spendFromUserTo.
/// @dev Unpauses the Contract. Only the owner can call this function.
/// @dev Only the owner can call this function.
function unpause() external onlyOwner {
_unpause();
}

/// @notice Transfers tokens from a user to a specified address.
/// @dev Transfers `amount` of `token` from `from` to `to`. The caller must be authorized.
/// @param from The address from which the tokens are transferred.
/// @param token The address of the ERC20 token contract.
/// @param to The address to which the tokens are transferred.
/// @param amount The amount of tokens to transfer.
/// @inheritdoc IAllowanceTarget
function spendFromUserTo(address from, address token, address to, uint256 amount) external whenNotPaused {
if (!authorized[msg.sender]) revert NotAuthorized();
Expand Down
16 changes: 2 additions & 14 deletions contracts/CoordinatedTaker.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { SignatureValidator } from "./libraries/SignatureValidator.sol";

/// @title CoordinatedTaker Contract
/// @author imToken Labs
/// @notice This contract allows for the coordination of limit order fills through a designated coordinator.
/// @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;

Expand All @@ -23,11 +23,9 @@ contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector,
address public coordinator;

/// @notice Mapping to keep track of used allow fill hashes.
/// @dev Tracks whether each allow fill hash has been used.
mapping(bytes32 => bool) public allowFillUsed;
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.
/// @dev Sets up the contract with the owner, Uniswap permit2 address, allowance target, WETH contract, 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.
Expand All @@ -48,7 +46,6 @@ contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector,
}

/// @notice Receive function to receive ETH.
/// @dev This function allows the contract to receive ETH payments.
receive() external payable {}

/// @notice Sets a new coordinator address.
Expand All @@ -61,15 +58,6 @@ contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector,
emit SetCoordinator(_newCoordinator);
}

/// @notice Submits a limit order fill request.
/// @dev Validates permissions and forwards the request to the LimitOrderSwap contract.
/// @param order The limit order details.
/// @param makerSignature The signature of the order maker.
/// @param takerTokenAmount The amount of taker tokens.
/// @param makerTokenAmount The amount of maker tokens.
/// @param extraAction Additional actions to perform.
/// @param userTokenPermit The permit for the user token.
/// @param crdParams The coordinator parameters.
/// @inheritdoc ICoordinatedTaker
function submitLimitOrderFill(
LimitOrder calldata order,
Expand Down
18 changes: 2 additions & 16 deletions contracts/GenericSwap.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,41 +11,29 @@ import { SignatureValidator } from "./libraries/SignatureValidator.sol";

/// @title GenericSwap Contract
/// @author imToken Labs
/// @notice This contract facilitates token swaps using predefined strategies.
/// @notice This contract facilitates token swaps using SmartOrderStrategy strategies.
contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
using Asset for address;

/// @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 => bool) public filledSwap;
mapping(bytes32 swapHash => bool isFilled) public filledSwap;

/// @notice Constructor to initialize the contract with the permit2 and allowance target.
/// @dev Sets up the contract with the Uniswap permit2 address and allowance target address.
/// @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.
/// @dev This function allows the contract to receive ETH payments.
receive() external payable {}

/// @notice Executes a generic swap using predefined strategies.
/// @param swapData The swap data containing details of the swap.
/// @param takerTokenPermit The permit for the taker token.
/// @return returnAmount The 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);
}

/// @notice Executes a generic swap using predefined strategies with a signature.
/// @param swapData The swap data containing details of the swap.
/// @param takerTokenPermit The permit for the taker token.
/// @param taker The address of the taker.
/// @param takerSig The signature of the taker.
/// @return returnAmount The output amount of the swap.
/// @inheritdoc IGenericSwap
function executeSwapWithSig(
GenericSwapData calldata swapData,
Expand All @@ -65,7 +53,6 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
}

/// @notice Executes a generic swap.
/// @dev Internal function to handle the swap execution.
/// @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.
Expand Down Expand Up @@ -102,7 +89,6 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
}

/// @notice Emits the Swap event after executing a generic swap.
/// @dev Internal function to emit the Swap event.
/// @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.
Expand Down
35 changes: 1 addition & 34 deletions contracts/LimitOrderSwap.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree
address payable public feeCollector;

/// @notice Mapping to track the filled amounts of maker tokens for each order hash.
/// @dev Keeps a record of the amount of maker tokens filled for each corresponding order hash.
mapping(bytes32 => uint256) public orderHashToMakerTokenFilledAmount;
mapping(bytes32 orderHash => uint256 orderFilledAmount) public orderHashToMakerTokenFilledAmount;

/// @notice Constructor to initialize the contract with the owner, Uniswap permit2, allowance target, WETH, and fee collector.
/// @dev Sets up the contract by initializing the owner, Uniswap permit2, allowance target, WETH token, and fee collector addresses.
/// @param _owner The address of the contract owner.
/// @param _uniswapPermit2 The address of the Uniswap permit2.
/// @param _allowanceTarget The address of the allowance target.
Expand All @@ -51,7 +49,6 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree
}

/// @notice Receive function to receive ETH.
/// @dev This function allows the contract to receive ETH payments.
receive() external payable {}

/// @notice Sets a new fee collector address.
Expand All @@ -64,23 +61,11 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree
emit SetFeeCollector(_newFeeCollector);
}

/// @notice Fills a single limit order, transferring tokens between maker and taker according to the order details.
/// @dev This function validates the order, calculates token amounts to transfer, collects tokens from makers, transfers tokens to takers,
/// collects fees, and handles any additional actions specified by takerParams.
/// @param order The limit order details to fill.
/// @param makerSignature The signature of the maker authorizing the order.
/// @param takerParams Additional parameters provided by the taker for this fill.
/// @inheritdoc ILimitOrderSwap
function fillLimitOrder(LimitOrder calldata order, bytes calldata makerSignature, TakerParams calldata takerParams) external payable nonReentrant {
_fillLimitOrder(order, makerSignature, takerParams, false);
}

/// @notice Fills a single limit order fully or not at all (FOK), ensuring either the entire order is filled or no tokens are transferred.
/// @dev This function validates the order, calculates token amounts to transfer, collects tokens from makers, transfers tokens to takers,
/// collects fees, and handles any additional actions specified by takerParams. If the full order cannot be filled, the transaction reverts.
/// @param order The limit order details to fill.
/// @param makerSignature The signature of the maker authorizing the order.
/// @param takerParams Additional parameters provided by the taker for this fill.
/// @inheritdoc ILimitOrderSwap
function fillLimitOrderFullOrKill(
LimitOrder calldata order,
Expand All @@ -90,14 +75,6 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree
_fillLimitOrder(order, makerSignature, takerParams, true);
}

/// @notice Fills multiple limit orders in a batch, transferring tokens between makers and takers according to the order details.
/// @dev This function validates each order in the batch, calculates token amounts to transfer for each order, collects tokens from makers,
/// transfers tokens to takers, collects fees, and handles any additional actions specified by takerParams for each order.
/// It also processes WETH withdrawals if needed to cover ETH payments and transfers any remaining tokens as profit to the taker.
/// @param orders Array of limit order details to fill.
/// @param makerSignatures Signatures of the makers authorizing each order.
/// @param makerTokenAmounts Amounts of maker tokens requested to be filled for each order.
/// @param profitTokens Array of tokens considered as profit for the taker.
/// @inheritdoc ILimitOrderSwap
function fillLimitOrderGroup(
LimitOrder[] calldata orders,
Expand Down Expand Up @@ -168,9 +145,6 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree
}
}

/// @notice Cancels an unfilled limit order, preventing it from being executed.
/// @dev Only the order maker can cancel an order, and the order must not be expired, filled, or already canceled.
/// @param order The limit order details to cancel.
/// @inheritdoc ILimitOrderSwap
function cancelOrder(LimitOrder calldata order) external nonReentrant {
if (order.expiry < uint64(block.timestamp)) revert ExpiredOrder();
Expand All @@ -185,17 +159,13 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree
emit OrderCanceled(orderHash, order.maker);
}

/// @notice Checks if an order is canceled.
/// @param orderHash The hash of the order to check.
/// @return True if the order is canceled, false otherwise.
/// @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.
/// @dev Internal function to fill a limit order with optional FOK(full-or-kill) logic.
/// @param order The limit order details.
/// @param makerSignature The maker's signature for the order.
/// @param takerParams The taker's parameters for the order.
Expand Down Expand Up @@ -240,7 +210,6 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree
}

/// @notice Validates an order and quotes the taker and maker spending amounts.
/// @dev Internal function to validate an order and calculate spending amounts.
/// @param _order The limit order details.
/// @param _makerSignature The maker's signature for the order.
/// @param _takerTokenAmount The amount of taker token.
Expand Down Expand Up @@ -295,7 +264,6 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree
}

/// @notice Validates an order and its signature.
/// @dev Internal function to validate an order and its maker's signature.
/// @param _order The limit order details.
/// @param _makerSignature The maker's signature for the order.
/// @return orderHash The hash of the validated order.
Expand Down Expand Up @@ -326,7 +294,6 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree
}

/// @notice Emits the LimitOrderFilled event after executing a limit order swap.
/// @dev Internal function to emit the LimitOrderFilled event.
/// @param _order The limit order details.
/// @param _orderHash The hash of the limit order.
/// @param _takerTokenSettleAmount The settled amount of taker token.
Expand Down
23 changes: 1 addition & 22 deletions contracts/RFQ.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,9 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 {
address payable public feeCollector;

/// @notice Mapping to track the filled status of each offer identified by its hash.
/// @dev Keeps track of whether each offer identified by its hash has been filled.
mapping(bytes32 => bool) public filledOffer;
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.
/// @dev Initializes the RFQ contract with the specified owner, sets the Uniswap permit2 address, allowance target address, WETH token instance, and fee collector address.
/// @param _owner The address of the contract owner.
/// @param _uniswapPermit2 The address of the Uniswap permit2.
/// @param _allowanceTarget The address of the allowance target.
Expand All @@ -56,7 +54,6 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 {
}

/// @notice Receive function to receive ETH.
/// @dev This function allows the contract to receive ETH payments.
receive() external payable {}

/// @notice Sets the fee collector address.
Expand All @@ -69,22 +66,11 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 {
emit SetFeeCollector(_newFeeCollector);
}

/// @notice Fills 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.
/// @inheritdoc IRFQ
function fillRFQ(RFQTx calldata rfqTx, bytes calldata makerSignature, bytes calldata makerTokenPermit, bytes calldata takerTokenPermit) external payable {
_fillRFQ(rfqTx, makerSignature, makerTokenPermit, takerTokenPermit, bytes(""));
}

/// @notice Fills an RFQ transaction with taker's signature, in addition to maker's 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.
/// @inheritdoc IRFQ
function fillRFQWithSig(
RFQTx calldata rfqTx,
Expand All @@ -96,10 +82,6 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 {
_fillRFQ(rfqTx, makerSignature, makerTokenPermit, takerTokenPermit, takerSignature);
}

/// @notice Cancels an RFQ offer created by the maker.
/// @dev This function cancels an RFQ offer if called by the maker, marking it as filled in the storage.
/// @param rfqOffer The RFQ offer data to be canceled.
/// Emits a `CancelRFQOffer` event upon successful cancellation.
/// @inheritdoc IRFQ
function cancelRFQOffer(RFQOffer calldata rfqOffer) external {
if (msg.sender != rfqOffer.maker) revert NotOfferMaker();
Expand Down Expand Up @@ -209,7 +191,6 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 {
}

/// @notice Emits the FilledRFQ event after executing an RFQ order swap.
/// @dev Internal function to emit FilledRFQ event.
/// @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.
Expand All @@ -229,7 +210,6 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 {
}

/// @notice Collects ETH and sends it to the specified address.
/// @dev Internal function to collect ETH and send 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.
Expand All @@ -245,7 +225,6 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 {
}

/// @notice Collects WETH and sends it to the specified address.
/// @dev Internal function to collect WETH and send 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.
Expand Down
Loading

0 comments on commit 3c437d6

Please sign in to comment.