feature: decouple strategyData from GenericSwapData

contracts/GenericSwap.sol

@@ -3,6 +3,7 @@ pragma solidity 0.8.26;
 
 import { EIP712 } from "./abstracts/EIP712.sol";
 import { TokenCollector } from "./abstracts/TokenCollector.sol";
+import { Ownable } from "./abstracts/Ownable.sol";
 
 import { IGenericSwap } from "./interfaces/IGenericSwap.sol";
 import { IStrategy } from "./interfaces/IStrategy.sol";
@@ -14,42 +15,84 @@ 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 {
+contract GenericSwap is IGenericSwap, TokenCollector, EIP712, Ownable {
     using Asset for address;
 
+    /// @notice Mapping to track addresses authorized as solvers.
+    /// @dev Maps each address to a boolean indicating whether it is an authorized solver.
+    mapping(address solver => bool isSolver) public solvers;
+
     /// @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;
 
+    modifier onlySolver() {
+        if (!solvers[msg.sender]) revert InvalidSolver();
+        _;
+    }
+
     /// @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) {}
+    constructor(
+        address _uniswapPermit2,
+        address _allowanceTarget,
+        address _owner,
+        address[] memory _trustedSolvers
+    ) TokenCollector(_uniswapPermit2, _allowanceTarget) Ownable(_owner) {
+        uint256 length = _trustedSolvers.length;
+        for (uint256 i; i < length; ++i) {
+            solvers[_trustedSolvers[i]] = true;
+        }
+    }
 
     /// @notice Receive function to receive ETH.
     receive() external payable {}
 
+    function addSolver(address solver) external onlyOwner {
+        if (solver == address(0)) revert ZeroAddress();
+
+        if (!solvers[solver]) {
+            solvers[solver] = true;
+
+            emit AddSolver(solver);
+        }
+    }
+
+    function removeSolver(address solver) external onlyOwner {
+        if (solvers[solver]) {
+            solvers[solver] = false;
+
+            emit RemoveSolver(solver);
+        }
+    }
+
     /// @inheritdoc IGenericSwap
-    function executeSwap(GenericSwapData calldata swapData, bytes calldata takerTokenPermit) external payable returns (uint256 returnAmount) {
-        returnAmount = _executeSwap(swapData, msg.sender, takerTokenPermit);
+    function executeSwap(
+        GenericSwapData calldata swapData,
+        bytes calldata strategyData,
+        bytes calldata takerTokenPermit
+    ) external payable returns (uint256 returnAmount) {
+        returnAmount = _executeSwap(swapData, strategyData, msg.sender, takerTokenPermit);
 
         _emitGSExecuted(getGSDataHash(swapData), swapData, msg.sender, returnAmount);
     }
 
     /// @inheritdoc IGenericSwap
     function executeSwapWithSig(
         GenericSwapData calldata swapData,
+        bytes calldata strategyData,
         bytes calldata takerTokenPermit,
         address taker,
         bytes calldata takerSig
-    ) external payable returns (uint256 returnAmount) {
+    ) external payable onlySolver returns (uint256 returnAmount) {
         bytes32 swapHash = getGSDataHash(swapData);
         bytes32 gs712Hash = getEIP712Hash(swapHash);
         if (filledSwap[swapHash]) revert AlreadyFilled();
         filledSwap[swapHash] = true;
         if (!SignatureValidator.validateSignature(taker, gs712Hash, takerSig)) revert InvalidSignature();
 
-        returnAmount = _executeSwap(swapData, taker, takerTokenPermit);
+        returnAmount = _executeSwap(swapData, strategyData, taker, takerTokenPermit);
 
         _emitGSExecuted(swapHash, swapData, taker, returnAmount);
     }
@@ -61,6 +104,7 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
     /// @return returnAmount The output amount of the swap.
     function _executeSwap(
         GenericSwapData calldata _swapData,
+        bytes calldata strategyData,
         address _authorizedUser,
         bytes calldata _takerTokenPermit
     ) private returns (uint256 returnAmount) {
@@ -78,7 +122,7 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
             _collect(_inputToken, _authorizedUser, _swapData.maker, _swapData.takerTokenAmount, _takerTokenPermit);
         }
 
-        IStrategy(_swapData.maker).executeStrategy{ value: msg.value }(_outputToken, _swapData.strategyData);
+        IStrategy(_swapData.maker).executeStrategy{ value: msg.value }(_outputToken, strategyData);
 
         returnAmount = _outputToken.getBalance(address(this));
         if (returnAmount > 1) {

contracts/interfaces/IGenericSwap.sol

@@ -8,6 +8,14 @@ import { GenericSwapData } from "../libraries/GenericSwapData.sol";
 /// @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 Emitted when a new solver is added to the contract.
+    /// @param solver The address of the solver being added.
+    event AddSolver(address indexed solver);
+
+    /// @notice Emitted when a solver is removed from the contract.
+    /// @param solver The address of the solver being removed.
+    event RemoveSolver(address indexed solver);
+
     /// @notice Event emitted when a swap is executed.
     /// @param swapHash The hash of the swap data.
     /// @param maker The address of the maker initiating the swap.
@@ -30,6 +38,10 @@ interface IGenericSwap {
         uint256 salt
     );
 
+    /// @notice Error to be thrown when an unauthorized address attempts to execute a swap.
+    /// @dev This error is used to restrict access to certain functions to authorized solvers only
+    error InvalidSolver();
+
     /// @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();
@@ -60,18 +72,25 @@ interface IGenericSwap {
 
     /// @notice Executes a swap using provided swap data and taker token permit.
     /// @param swapData The swap data containing details of the swap.
+    /// @param strategyData The strategy data contains the details on how to perform 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);
+    function executeSwap(
+        GenericSwapData calldata swapData,
+        bytes calldata strategyData,
+        bytes calldata takerTokenPermit
+    ) external payable returns (uint256 returnAmount);
 
     /// @notice Executes a swap using provided swap data, taker token permit, taker address, and signature.
     /// @param swapData The swap data containing details of the swap.
+    /// @param strategyData The strategy data contains the details on how to perform 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 strategyData,
         bytes calldata takerTokenPermit,
         address taker,
         bytes calldata takerSig

contracts/libraries/GenericSwapData.sol

@@ -2,7 +2,7 @@
 pragma solidity ^0.8.0;
 
 string constant GS_DATA_TYPESTRING = string(
-    "GenericSwapData(address maker,address takerToken,uint256 takerTokenAmount,address makerToken,uint256 makerTokenAmount,uint256 minMakerTokenAmount,uint256 expiry,uint256 salt,address recipient,bytes strategyData)"
+    "GenericSwapData(address maker,address takerToken,uint256 takerTokenAmount,address makerToken,uint256 makerTokenAmount,uint256 minMakerTokenAmount,uint256 expiry,uint256 salt,address recipient)"
 );
 
 bytes32 constant GS_DATA_TYPEHASH = keccak256(bytes(GS_DATA_TYPESTRING));
@@ -17,7 +17,6 @@ struct GenericSwapData {
     uint256 expiry;
     uint256 salt;
     address payable recipient;
-    bytes strategyData;
 }
 
 // solhint-disable-next-line func-visibility
@@ -34,8 +33,7 @@ function getGSDataHash(GenericSwapData memory gsData) pure returns (bytes32) {
                 gsData.minMakerTokenAmount,
                 gsData.expiry,
                 gsData.salt,
-                gsData.recipient,
-                keccak256(gsData.strategyData)
+                gsData.recipient
             )
         );
 }

snapshots/AdminManagement.json

@@ -1,4 +1,4 @@
 {
-  "approveTokens(): testApproveTokens": "133041",
-  "rescueTokens(): testRescueTokens": "84308"
+  "approveTokens(): testApproveTokens": "106201",
+  "rescueTokens(): testRescueTokens": "59723"
 }

snapshots/AllowanceTarget.json

@@ -1,8 +1,8 @@
 {
-  "pause(): testSpendFromUserToAfterUnpause": "27565",
-  "spendFromUserTo(): testSpendFromUserTo": "61960",
-  "spendFromUserTo(): testSpendFromUserToAfterUnpause": "61960",
-  "spendFromUserTo(): testSpendFromUserToWithDeflationaryToken": "89862",
-  "spendFromUserTo(): testSpendFromUserToWithNoReturnValueToken": "67278",
-  "unpause(): testSpendFromUserToAfterUnpause": "27326"
+  "pause(): testSpendFromUserToAfterUnpause": "6978",
+  "spendFromUserTo(): testSpendFromUserTo": "36872",
+  "spendFromUserTo(): testSpendFromUserToAfterUnpause": "34872",
+  "spendFromUserTo(): testSpendFromUserToWithDeflationaryToken": "65947",
+  "spendFromUserTo(): testSpendFromUserToWithNoReturnValueToken": "42418",
+  "unpause(): testSpendFromUserToAfterUnpause": "2175"
 }

snapshots/Asset.json

@@ -1,11 +1,11 @@
 {
-  "getBalance(): testGetBalance": "5799",
-  "getBalance(): testGetBalance(ETH_ADDRESS)": "510",
-  "getBalance(): testGetBalance(ZERO_ADDRESS)": "517",
-  "isETH(): testIsETH(ETH_ADDRESS)": "306",
-  "isETH(): testIsETH(ZERO_ADDRESS)": "313",
-  "transferTo(): testDoNothingIfTransferToSelf": "22182",
-  "transferTo(): testDoNothingIfTransferWithZeroAmount": "22170",
-  "transferTo(): testTransferETH": "56716",
-  "transferTo(): testTransferToken": "50378"
+  "getBalance(): testGetBalance": "7087",
+  "getBalance(): testGetBalance(ETH_ADDRESS)": "1143",
+  "getBalance(): testGetBalance(ZERO_ADDRESS)": "1165",
+  "isETH(): testIsETH(ETH_ADDRESS)": "724",
+  "isETH(): testIsETH(ZERO_ADDRESS)": "746",
+  "transferTo(): testDoNothingIfTransferToSelf": "877",
+  "transferTo(): testDoNothingIfTransferWithZeroAmount": "877",
+  "transferTo(): testTransferETH": "35651",
+  "transferTo(): testTransferToken": "31566"
 }

snapshots/CoordinatedTaker.json

@@ -1,6 +1,6 @@
 {
-  "approveTokens(): testApproveTokens": "53171",
-  "setCoordinator(): testSetCoordinator": "30166",
-  "submitLimitOrderFill(): testFillWithETH": "189977",
-  "submitLimitOrderFill(): testFillWithPermission": "252405"
+  "approveTokens(): testApproveTokens": "28815",
+  "setCoordinator(): testSetCoordinator": "8995",
+  "submitLimitOrderFill(): testFillWithETH": "193271",
+  "submitLimitOrderFill(): testFillWithPermission": "272753"
 }

snapshots/EIP712.json

@@ -1,5 +1,5 @@
 {
-  "EIP712_DOMAIN_SEPARATOR(): testDomainSeparatorOnChain": "337",
-  "EIP712_DOMAIN_SEPARATOR(): testDomainSeparatorOnDifferentChain": "1074",
-  "getEIP712Hash(): testGetEIP712Hash": "315"
+  "EIP712_DOMAIN_SEPARATOR(): testDomainSeparatorOnChain": "443",
+  "EIP712_DOMAIN_SEPARATOR(): testDomainSeparatorOnDifferentChain": "1404",
+  "getEIP712Hash(): testGetEIP712Hash": "817"
 }

snapshots/GenericSwap.json

@@ -1,9 +1,11 @@
 {
-  "executeSwap(): testGenericSwapWithUniswap": "248141",
-  "executeSwap(): testLeaveOneWeiWithMultipleUsers(the first deposit)": "248141",
-  "executeSwap(): testLeaveOneWeiWithMultipleUsers(the second deposit)": "204495",
-  "executeSwap(): testSwapWithETHInput": "97813",
-  "executeSwap(): testSwapWithETHOutput": "127973",
-  "executeSwap(): testSwapWithLessOutputButWithinTolerance": "157429",
-  "executeSwapWithSig(): testGenericSwapRelayed": "279472"
+  "addSolver(): testAddSolver": "26392",
+  "executeSwap(): testGenericSwapWithUniswap": "240706",
+  "executeSwap(): testLeaveOneWeiWithMultipleUsers(the first deposit)": "234706",
+  "executeSwap(): testLeaveOneWeiWithMultipleUsers(the second deposit)": "143860",
+  "executeSwap(): testSwapWithETHInput": "61310",
+  "executeSwap(): testSwapWithETHOutput": "89483",
+  "executeSwap(): testSwapWithLessOutputButWithinTolerance": "117249",
+  "executeSwapWithSig(): testGenericSwapRelayed": "270770",
+  "removeSolver(): testRemoveSolver": "9261"
 }

snapshots/LimitOrderSwap.json

@@ -1,20 +1,20 @@
 {
-  "cancelOrder(): testCancelOrder": "52957",
-  "fillLimitOrder(): testFillLimitOrderWithETH": "155159",
-  "fillLimitOrder(): testFillWithBetterTakingAmount": "212150",
-  "fillLimitOrder(): testFillWithBetterTakingAmountButGetAdjusted": "212318",
-  "fillLimitOrder(): testFillWithETHRefund": "162238",
-  "fillLimitOrder(): testFillWithLargerVolumeAndSettleAsManyAsPossible": "212306",
-  "fillLimitOrder(): testFillWithoutMakerSigForVerifiedOrder": "212150",
-  "fillLimitOrder(): testFillWithoutMakerSigForVerifiedOrder(without makerSig)": "119214",
-  "fillLimitOrder(): testFullyFillLimitOrder": "212150",
-  "fillLimitOrder(): testFullyFillLimitOrderUsingAMM": "231466",
-  "fillLimitOrder(): testPartiallyFillLimitOrder": "212150",
-  "fillLimitOrderFullOrKill(): testFillWithFOK": "212327",
-  "fillLimitOrderGroup(): testGroupFillRingTrade": "287118",
-  "fillLimitOrderGroup(): testGroupFillWithPartialWETHUnwrap": "273557",
-  "fillLimitOrderGroup(): testGroupFillWithTakerPrefundETH": "195696",
-  "fillLimitOrderGroup(): testGroupFillWithWETHUnwrap": "195696",
-  "fillLimitOrderGroup(): testPartialFillLargeOrderWithSmallOrders": "261176",
-  "testGroupFillWithProfit: fillLimitOrderGroup()": "221442"
+  "cancelOrder(): testCancelOrder": "35188",
+  "fillLimitOrder(): testFillLimitOrderWithETH": "147904",
+  "fillLimitOrder(): testFillWithBetterTakingAmount": "203163",
+  "fillLimitOrder(): testFillWithBetterTakingAmountButGetAdjusted": "203649",
+  "fillLimitOrder(): testFillWithETHRefund": "155664",
+  "fillLimitOrder(): testFillWithLargerVolumeAndSettleAsManyAsPossible": "203649",
+  "fillLimitOrder(): testFillWithoutMakerSigForVerifiedOrder": "220163",
+  "fillLimitOrder(): testFillWithoutMakerSigForVerifiedOrder(without makerSig)": "66600",
+  "fillLimitOrder(): testFullyFillLimitOrder": "203163",
+  "fillLimitOrder(): testFullyFillLimitOrderUsingAMM": "230361",
+  "fillLimitOrder(): testPartiallyFillLimitOrder": "203163",
+  "fillLimitOrderFullOrKill(): testFillWithFOK": "203208",
+  "fillLimitOrderGroup(): testGroupFillRingTrade": "312414",
+  "fillLimitOrderGroup(): testGroupFillWithPartialWETHUnwrap": "286911",
+  "fillLimitOrderGroup(): testGroupFillWithTakerPrefundETH": "208427",
+  "fillLimitOrderGroup(): testGroupFillWithWETHUnwrap": "208427",
+  "fillLimitOrderGroup(): testPartialFillLargeOrderWithSmallOrders": "275390",
+  "testGroupFillWithProfit: fillLimitOrderGroup()": "235042"
 }

snapshots/Ownable.json

@@ -1,5 +1,5 @@
 {
-  "acceptOwnership(): testAcceptOwnership": "28029",
-  "nominateNewOwner(): testNominateNewOwner": "46965",
-  "renounceOwnership(): testRenounceOwnership": "25105"
+  "acceptOwnership(): testAcceptOwnership": "5634",
+  "nominateNewOwner(): testNominateNewOwner": "25920",
+  "renounceOwnership(): testRenounceOwnership": "9241"
 }

snapshots/RFQ.json

@@ -1,15 +1,15 @@
 {
-  "cancelRFQOffer(): testCancelRFQOffer": "49775",
-  "fillRFQ(): testFillRFQ": "216172",
-  "fillRFQ(): testFillRFQTakerGetRawETH": "236417",
-  "fillRFQ(): testFillRFQWithRawETH": "156621",
-  "fillRFQ(): testFillRFQWithRawETHAndReceiveWETH": "171907",
-  "fillRFQ(): testFillRFQWithTakerApproveAllowanceTarget": "183936",
-  "fillRFQ(): testFillRFQWithWETH": "218668",
-  "fillRFQ(): testFillRFQWithWETHAndReceiveWETH": "204826",
-  "fillRFQ(): testFillRFQWithZeroFee": "189744",
-  "fillRFQ(): testFillWithContract": "219582",
-  "fillRFQ(): testPartialFill": "216312",
-  "fillRFQWithSig(): testFillRFQByTakerSig": "224772",
-  "setFeeCollector(): testSetFeeCollector": "30134"
+  "cancelRFQOffer(): testCancelRFQOffer": "29205",
+  "fillRFQ(): testFillRFQ": "206779",
+  "fillRFQ(): testFillRFQTakerGetRawETH": "221657",
+  "fillRFQ(): testFillRFQWithRawETH": "149209",
+  "fillRFQ(): testFillRFQWithRawETHAndReceiveWETH": "182544",
+  "fillRFQ(): testFillRFQWithTakerApproveAllowanceTarget": "182120",
+  "fillRFQ(): testFillRFQWithWETH": "226368",
+  "fillRFQ(): testFillRFQWithWETHAndReceiveWETH": "192883",
+  "fillRFQ(): testFillRFQWithZeroFee": "181238",
+  "fillRFQ(): testFillWithContract": "223799",
+  "fillRFQ(): testPartialFill": "207199",
+  "fillRFQWithSig(): testFillRFQByTakerSig": "211868",
+  "setFeeCollector(): testSetFeeCollector": "9236"
 }

snapshots/SmartOrderStrategy.json

@@ -1,11 +1,11 @@
 {
-  "executeStrategy(): testMultipleAMMs": "607558",
-  "executeStrategy(): testUniswapV2WithWETHUnwrap": "140873",
-  "executeStrategy(): testUniswapV3WithAmountReplace": "159842",
-  "executeStrategy(): testUniswapV3WithMaxAmountReplace": "154811",
-  "executeStrategy(): testUniswapV3WithoutAmountReplace": "152972",
-  "executeStrategy(): testV6LOIntegration": "173538",
-  "executeStrategy(): testV6RFQIntegration": "177215",
-  "executeStrategy(): testV6RFQIntegrationWhenMakerTokenIsETH": "144660",
-  "executeStrategy(): testV6RFQIntegrationWhenTakerTokenIsETH": "165683"
+  "executeStrategy(): testMultipleAMMs": "578554",
+  "executeStrategy(): testUniswapV2WithWETHUnwrap": "123073",
+  "executeStrategy(): testUniswapV3WithAmountReplace": "114182",
+  "executeStrategy(): testUniswapV3WithMaxAmountReplace": "113726",
+  "executeStrategy(): testUniswapV3WithoutAmountReplace": "111675",
+  "executeStrategy(): testV6LOIntegration": "169549",
+  "executeStrategy(): testV6RFQIntegration": "170773",
+  "executeStrategy(): testV6RFQIntegrationWhenMakerTokenIsETH": "143277",
+  "executeStrategy(): testV6RFQIntegrationWhenTakerTokenIsETH": "193658"
 }

snapshots/TokenCollector.json

@@ -1,7 +1,7 @@
 {
-  "collect(): testCollectByAllowanceTarget": "64565",
-  "collect(): testCollectByPermit2AllowanceTransfer": "100762",
-  "collect(): testCollectByPermit2SignatureTransfer": "94003",
-  "collect(): testCollectByTokenApproval": "57170",
-  "collect(): testCollectByTokenPermit": "91054"
+  "collect(): testCollectByAllowanceTarget": "44298",
+  "collect(): testCollectByPermit2AllowanceTransfer": "71796",
+  "collect(): testCollectByPermit2SignatureTransfer": "68053",
+  "collect(): testCollectByTokenApproval": "35424",
+  "collect(): testCollectByTokenPermit": "90883"
 }

test/Signing.t.sol

@@ -49,8 +49,7 @@ contract testEIP712Signing is SigHelper {
             minMakerTokenAmount: abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.minMakerTokenAmount"), (uint256)),
             expiry: abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.expiry"), (uint256)),
             salt: abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.salt"), (uint256)),
-            recipient: abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.recipient"), (address)),
-            strategyData: abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.strategyData"), (bytes))
+            recipient: abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.recipient"), (address))
         });
         uint256 signingKey = abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.signingKey"), (uint256));
         bytes memory sig = signGenericSwap(signingKey, genericSwapData, chainId, verifyingContract);