Skip to content

Commit 99e2106

Browse files
alex0207salex0207s
committed
feature: decouple strategyData from GenericSwapData
1 parent 57f3a2a commit 99e2106

File tree

6 files changed

+40
-32
lines changed

6 files changed

+40
-32
lines changed

‎contracts/GenericSwap.sol

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,20 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
2828
receive() external payable {}
2929

3030
/// @inheritdoc IGenericSwap
31-
function executeSwap(GenericSwapData calldata swapData, bytes calldata takerTokenPermit) external payable returns (uint256 returnAmount) {
32-
returnAmount = _executeSwap(swapData, msg.sender, takerTokenPermit);
31+
function executeSwap(
32+
GenericSwapData calldata swapData,
33+
bytes calldata strategyData,
34+
bytes calldata takerTokenPermit
35+
) external payable returns (uint256 returnAmount) {
36+
returnAmount = _executeSwap(swapData, strategyData, msg.sender, takerTokenPermit);
3337

3438
_emitGSExecuted(getGSDataHash(swapData), swapData, msg.sender, returnAmount);
3539
}
3640

3741
/// @inheritdoc IGenericSwap
3842
function executeSwapWithSig(
3943
GenericSwapData calldata swapData,
44+
bytes calldata strategyData,
4045
bytes calldata takerTokenPermit,
4146
address taker,
4247
bytes calldata takerSig
@@ -47,7 +52,7 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
4752
filledSwap[swapHash] = true;
4853
if (!SignatureValidator.validateSignature(taker, gs712Hash, takerSig)) revert InvalidSignature();
4954

50-
returnAmount = _executeSwap(swapData, taker, takerTokenPermit);
55+
returnAmount = _executeSwap(swapData, strategyData, taker, takerTokenPermit);
5156

5257
_emitGSExecuted(swapHash, swapData, taker, returnAmount);
5358
}
@@ -59,6 +64,7 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
5964
/// @return returnAmount The output amount of the swap.
6065
function _executeSwap(
6166
GenericSwapData calldata _swapData,
67+
bytes calldata strategyData,
6268
address _authorizedUser,
6369
bytes calldata _takerTokenPermit
6470
) private returns (uint256 returnAmount) {
@@ -75,7 +81,7 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
7581
_collect(_inputToken, _authorizedUser, _swapData.maker, _swapData.takerTokenAmount, _takerTokenPermit);
7682
}
7783

78-
IStrategy(_swapData.maker).executeStrategy{ value: msg.value }(_inputToken, _outputToken, _swapData.takerTokenAmount, _swapData.strategyData);
84+
IStrategy(_swapData.maker).executeStrategy{ value: msg.value }(_inputToken, _outputToken, _swapData.takerTokenAmount, strategyData);
7985

8086
returnAmount = _outputToken.getBalance(address(this));
8187
if (returnAmount > 1) {

‎contracts/interfaces/IGenericSwap.sol

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,18 +56,25 @@ interface IGenericSwap {
5656

5757
/// @notice Executes a swap using provided swap data and taker token permit.
5858
/// @param swapData The swap data containing details of the swap.
59+
/// @param strategyData The strategy data contains the details on how to perform the swap.
5960
/// @param takerTokenPermit The permit for spending taker's tokens.
6061
/// @return returnAmount The amount of tokens returned from the swap.
61-
function executeSwap(GenericSwapData calldata swapData, bytes calldata takerTokenPermit) external payable returns (uint256 returnAmount);
62+
function executeSwap(
63+
GenericSwapData calldata swapData,
64+
bytes calldata strategyData,
65+
bytes calldata takerTokenPermit
66+
) external payable returns (uint256 returnAmount);
6267

6368
/// @notice Executes a swap using provided swap data, taker token permit, taker address, and signature.
6469
/// @param swapData The swap data containing details of the swap.
70+
/// @param strategyData The strategy data contains the details on how to perform the swap.
6571
/// @param takerTokenPermit The permit for spending taker's tokens.
6672
/// @param taker The address of the taker initiating the swap.
6773
/// @param takerSig The signature of the taker authorizing the swap.
6874
/// @return returnAmount The amount of tokens returned from the swap.
6975
function executeSwapWithSig(
7076
GenericSwapData calldata swapData,
77+
bytes calldata strategyData,
7178
bytes calldata takerTokenPermit,
7279
address taker,
7380
bytes calldata takerSig

‎contracts/libraries/GenericSwapData.sol

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
pragma solidity ^0.8.0;
33

44
string constant GS_DATA_TYPESTRING = string(
5-
"GenericSwapData(address maker,address takerToken,uint256 takerTokenAmount,address makerToken,uint256 makerTokenAmount,uint256 minMakerTokenAmount,uint256 expiry,uint256 salt,address recipient,bytes strategyData)"
5+
"GenericSwapData(address maker,address takerToken,uint256 takerTokenAmount,address makerToken,uint256 makerTokenAmount,uint256 minMakerTokenAmount,uint256 expiry,uint256 salt,address recipient)"
66
);
77

88
bytes32 constant GS_DATA_TYPEHASH = keccak256(bytes(GS_DATA_TYPESTRING));
@@ -17,7 +17,6 @@ struct GenericSwapData {
1717
uint256 expiry;
1818
uint256 salt;
1919
address payable recipient;
20-
bytes strategyData;
2120
}
2221

2322
// solhint-disable-next-line func-visibility
@@ -34,8 +33,7 @@ function getGSDataHash(GenericSwapData memory gsData) pure returns (bytes32) {
3433
gsData.minMakerTokenAmount,
3534
gsData.expiry,
3635
gsData.salt,
37-
gsData.recipient,
38-
keccak256(gsData.strategyData)
36+
gsData.recipient
3937
)
4038
);
4139
}

‎test/Signing.t.sol

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ contract testEIP712Signing is SigHelper {
4848
minMakerTokenAmount: abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.minMakerTokenAmount"), (uint256)),
4949
expiry: abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.expiry"), (uint256)),
5050
salt: abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.salt"), (uint256)),
51-
recipient: abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.recipient"), (address)),
52-
strategyData: abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.strategyData"), (bytes))
51+
recipient: abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.recipient"), (address))
5352
});
5453
uint256 signingKey = abi.decode(vm.parseJson(genericSwapDataPayloadJson, "$.signingKey"), (uint256));
5554
bytes memory sig = signGenericSwap(signingKey, genericSwapData, chainId, verifyingContract);

‎test/forkMainnet/GenericSwap.t.sol

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { IUniswapSwapRouter02 } from "test/utils/IUniswapSwapRouter02.sol";
1414
import { MockStrategy } from "test/mocks/MockStrategy.sol";
1515
import { GenericSwap } from "contracts/GenericSwap.sol";
1616
import { AllowanceTarget } from "contracts/AllowanceTarget.sol";
17-
import { TokenCollector } from "contracts/abstracts/TokenCollector.sol";
1817
import { SmartOrderStrategy } from "contracts/SmartOrderStrategy.sol";
1918
import { Constant } from "contracts/libraries/Constant.sol";
2019
import { GenericSwapData, getGSDataHash } from "contracts/libraries/GenericSwapData.sol";
@@ -50,6 +49,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
5049
uint24[] defaultV3Fees = [3000];
5150
bytes defaultTakerPermit;
5251
bytes alicePermit;
52+
bytes swapData;
5353
SmartOrderStrategy smartStrategy;
5454
GenericSwap genericSwap;
5555
GenericSwapData defaultGSData;
@@ -104,7 +104,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
104104
value: 0,
105105
data: routerPayload
106106
});
107-
bytes memory swapData = abi.encode(operations);
107+
swapData = abi.encode(operations);
108108

109109
deal(taker, 100 ether);
110110
setTokenBalanceAndApprove(taker, UNISWAP_PERMIT2_ADDRESS, tokens, 100000);
@@ -122,8 +122,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
122122
minMakerTokenAmount: minOutputAmount,
123123
expiry: defaultExpiry,
124124
salt: 5678,
125-
recipient: payable(taker),
126-
strategyData: swapData
125+
recipient: payable(taker)
127126
});
128127

129128
defaultTakerPermit = getTokenlonPermit2Data(taker, takerPrivateKey, defaultGSData.takerToken, address(genericSwap));
@@ -154,7 +153,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
154153
);
155154

156155
vm.prank(taker);
157-
genericSwap.executeSwap(defaultGSData, defaultTakerPermit);
156+
genericSwap.executeSwap(defaultGSData, swapData, defaultTakerPermit);
158157

159158
takerTakerToken.assertChange(-int256(defaultGSData.takerTokenAmount));
160159
// the makerTokenAmount in the defaultGSData is the exact quote from strategy
@@ -190,7 +189,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
190189
gsData.salt
191190
);
192191
vm.prank(taker);
193-
genericSwap.executeSwap(gsData, defaultTakerPermit);
192+
genericSwap.executeSwap(gsData, swapData, defaultTakerPermit);
194193

195194
takerTakerToken.assertChange(-int256(gsData.takerTokenAmount));
196195
takerMakerToken.assertChange(int256(realChangedInGS));
@@ -225,7 +224,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
225224
gsData.salt
226225
);
227226
vm.prank(taker);
228-
genericSwap.executeSwap{ value: gsData.takerTokenAmount }(gsData, defaultTakerPermit);
227+
genericSwap.executeSwap{ value: gsData.takerTokenAmount }(gsData, swapData, defaultTakerPermit);
229228

230229
takerTakerToken.assertChange(-int256(gsData.takerTokenAmount));
231230
takerMakerToken.assertChange(int256(realChangedInGS));
@@ -261,7 +260,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
261260
gsData.salt
262261
);
263262
vm.prank(taker);
264-
genericSwap.executeSwap(gsData, defaultTakerPermit);
263+
genericSwap.executeSwap(gsData, swapData, defaultTakerPermit);
265264

266265
takerTakerToken.assertChange(-int256(gsData.takerTokenAmount));
267266
takerMakerToken.assertChange(int256(realChangedInGS));
@@ -274,13 +273,13 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
274273

275274
vm.prank(taker);
276275
vm.expectRevert(IGenericSwap.ExpiredOrder.selector);
277-
genericSwap.executeSwap(defaultGSData, defaultTakerPermit);
276+
genericSwap.executeSwap(defaultGSData, swapData, defaultTakerPermit);
278277
}
279278

280279
function testCannotSwapWithInvalidETHInput() public {
281280
// case1 : msg.value != 0 when takerToken is not ETH
282281
vm.expectRevert(IGenericSwap.InvalidMsgValue.selector);
283-
genericSwap.executeSwap{ value: 1 }(defaultGSData, defaultTakerPermit);
282+
genericSwap.executeSwap{ value: 1 }(defaultGSData, swapData, defaultTakerPermit);
284283

285284
// change input token as ETH and update amount
286285
GenericSwapData memory gsData = defaultGSData;
@@ -290,12 +289,12 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
290289
// case2 : msg.value > takerTokenAmount
291290
vm.prank(taker);
292291
vm.expectRevert(IGenericSwap.InvalidMsgValue.selector);
293-
genericSwap.executeSwap{ value: gsData.takerTokenAmount + 1 }(gsData, defaultTakerPermit);
292+
genericSwap.executeSwap{ value: gsData.takerTokenAmount + 1 }(gsData, swapData, defaultTakerPermit);
294293

295294
// case3 : msg.value < takerTokenAmount
296295
vm.prank(taker);
297296
vm.expectRevert(IGenericSwap.InvalidMsgValue.selector);
298-
genericSwap.executeSwap{ value: gsData.takerTokenAmount - 1 }(gsData, defaultTakerPermit);
297+
genericSwap.executeSwap{ value: gsData.takerTokenAmount - 1 }(gsData, swapData, defaultTakerPermit);
299298
}
300299

301300
function testCannotSwapWithInsufficientOutput() public {
@@ -306,7 +305,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
306305
mockStrategy.setOutputAmountAndRecipient(gsData.minMakerTokenAmount - 1, payable(address(genericSwap)));
307306
vm.prank(taker);
308307
vm.expectRevert(IGenericSwap.InsufficientOutput.selector);
309-
genericSwap.executeSwap(gsData, defaultTakerPermit);
308+
genericSwap.executeSwap(gsData, swapData, defaultTakerPermit);
310309
}
311310

312311
function testCannotSwapWithZeroRecipient() public {
@@ -315,7 +314,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
315314

316315
vm.prank(taker);
317316
vm.expectRevert(IGenericSwap.ZeroAddress.selector);
318-
genericSwap.executeSwap(gsData, defaultTakerPermit);
317+
genericSwap.executeSwap(gsData, swapData, defaultTakerPermit);
319318
}
320319

321320
function testGenericSwapRelayed() public {
@@ -336,7 +335,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
336335
);
337336

338337
bytes memory takerSig = signGenericSwap(takerPrivateKey, defaultGSData, address(genericSwap));
339-
genericSwap.executeSwapWithSig(defaultGSData, defaultTakerPermit, taker, takerSig);
338+
genericSwap.executeSwapWithSig(defaultGSData, swapData, defaultTakerPermit, taker, takerSig);
340339

341340
takerTakerToken.assertChange(-int256(defaultGSData.takerTokenAmount));
342341
// the makerTokenAmount in the defaultGSData is the exact quote from strategy
@@ -349,15 +348,15 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
349348

350349
vm.expectRevert(IGenericSwap.InvalidSignature.selector);
351350
// submit with user address as expected signer
352-
genericSwap.executeSwapWithSig(defaultGSData, defaultTakerPermit, taker, randomSig);
351+
genericSwap.executeSwapWithSig(defaultGSData, swapData, defaultTakerPermit, taker, randomSig);
353352
}
354353

355354
function testCannotReplayGenericSwapSig() public {
356355
bytes memory takerSig = signGenericSwap(takerPrivateKey, defaultGSData, address(genericSwap));
357-
genericSwap.executeSwapWithSig(defaultGSData, defaultTakerPermit, taker, takerSig);
356+
genericSwap.executeSwapWithSig(defaultGSData, swapData, defaultTakerPermit, taker, takerSig);
358357

359358
vm.expectRevert(IGenericSwap.AlreadyFilled.selector);
360-
genericSwap.executeSwapWithSig(defaultGSData, defaultTakerPermit, taker, takerSig);
359+
genericSwap.executeSwapWithSig(defaultGSData, swapData, defaultTakerPermit, taker, takerSig);
361360
}
362361

363362
function testLeaveOneWeiWithMultipleUsers() public {
@@ -385,7 +384,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
385384
);
386385

387386
vm.prank(taker);
388-
genericSwap.executeSwap(defaultGSData, defaultTakerPermit);
387+
genericSwap.executeSwap(defaultGSData, swapData, defaultTakerPermit);
389388

390389
// the second user: Alice
391390
// his makerTokenAmount is recalculate by `quoteExactInput() function base on the current state`
@@ -418,7 +417,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
418417
);
419418

420419
vm.startPrank(alice);
421-
genericSwap.executeSwap(aliceGSData, alicePermit);
420+
genericSwap.executeSwap(aliceGSData, swapData, alicePermit);
422421
vm.stopPrank();
423422

424423
takerTakerToken.assertChange(-int256(defaultGSData.takerTokenAmount));

‎test/utils/payload/genericSwapData.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
"expiry": 3376656000,
99
"salt": 123,
1010
"recipient": "0xdf5621f59CcD4A58cF487E0C4E3fCDd6Db67E4dA",
11-
"strategyData": "0xcbe324dfbf8d48a7ad9ef3cb0c2bdd411129db5238371b8609272f14921736fb0ab28dcede4f2e1e41393d575ffec60002e565bfbf5eaf896bc490a965c8452d08d9b71b",
1211
"expectedSig": "0xdfe4e0414baa9b0ac4edcefc1d8ceb18dc21230f6b93a83c91188a105d1d7ece7809ca3c67db499fd497e35ba271a0a6689f7683d41b5d4602a5bb7895537a2d1c",
1312
"signingKey": "0x1dc4f07f92fa1783042350ffe685439f857a8fdd837589c00c5299e287c4afcb",
1413
"chainId": 5,

0 commit comments

Comments
 (0)