From 514686a3bd6adebb0776b70d98a642e91bb29c3d Mon Sep 17 00:00:00 2001 From: alex0207s Date: Tue, 30 Apr 2024 17:14:40 +0800 Subject: [PATCH 01/11] add expiry and salt to Swap event in GS contract --- contracts/GenericSwap.sol | 29 +++++++++------ contracts/interfaces/IGenericSwap.sol | 3 +- test/forkMainnet/GenericSwap.t.sol | 51 ++++++++++++++++++++++----- 3 files changed, 63 insertions(+), 20 deletions(-) diff --git a/contracts/GenericSwap.sol b/contracts/GenericSwap.sol index a361d28a..f0e77aa9 100644 --- a/contracts/GenericSwap.sol +++ b/contracts/GenericSwap.sol @@ -22,16 +22,8 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 { /// @return returnAmount Output amount of the swap function executeSwap(GenericSwapData calldata swapData, bytes calldata takerTokenPermit) external payable override returns (uint256 returnAmount) { returnAmount = _executeSwap(swapData, msg.sender, takerTokenPermit); - emit Swap( - getGSDataHash(swapData), - swapData.maker, - msg.sender, // taker - swapData.recipient, - swapData.takerToken, - swapData.takerTokenAmount, - swapData.makerToken, - returnAmount - ); + + _emitGSExecuted(getGSDataHash(swapData), swapData, msg.sender, returnAmount); } /// @param swapData Swap data @@ -51,7 +43,8 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 { if (!SignatureValidator.validateSignature(taker, gs712Hash, takerSig)) revert InvalidSignature(); returnAmount = _executeSwap(swapData, taker, takerTokenPermit); - emit Swap(swapHash, swapData.maker, taker, swapData.recipient, swapData.takerToken, swapData.takerTokenAmount, swapData.makerToken, returnAmount); + + _emitGSExecuted(swapHash, swapData, taker, returnAmount); } function _executeSwap( @@ -84,4 +77,18 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 { _outputToken.transferTo(_swapData.recipient, returnAmount); } + + function _emitGSExecuted(bytes32 _gsOfferHash, GenericSwapData calldata _swapData, address _taker, uint256 returnAmount) internal { + emit Swap( + _gsOfferHash, + _swapData.maker, + _taker, + _swapData.recipient, + _swapData.takerToken, + _swapData.takerTokenAmount, + _swapData.makerToken, + returnAmount, + _swapData.salt + ); + } } diff --git a/contracts/interfaces/IGenericSwap.sol b/contracts/interfaces/IGenericSwap.sol index e764884c..0a0b3bc3 100644 --- a/contracts/interfaces/IGenericSwap.sol +++ b/contracts/interfaces/IGenericSwap.sol @@ -19,7 +19,8 @@ interface IGenericSwap { address inputToken, uint256 inputAmount, address outputToken, - uint256 outputAmount + uint256 outputAmount, + uint256 salt ); function executeSwap(GenericSwapData calldata swapData, bytes calldata takerTokenPermit) external payable returns (uint256 returnAmount); diff --git a/test/forkMainnet/GenericSwap.t.sol b/test/forkMainnet/GenericSwap.t.sol index c2d5b510..47890adb 100644 --- a/test/forkMainnet/GenericSwap.t.sol +++ b/test/forkMainnet/GenericSwap.t.sol @@ -32,7 +32,8 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper address inputToken, uint256 inputAmount, address outputToken, - uint256 outputAmount + uint256 outputAmount, + uint256 salt ); address strategyAdmin = makeAddr("strategyAdmin"); @@ -140,7 +141,8 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper defaultGSData.takerToken, defaultGSData.takerTokenAmount, defaultGSData.makerToken, - defaultGSData.makerTokenAmount + defaultGSData.makerTokenAmount, + defaultGSData.salt ); vm.prank(taker); @@ -168,7 +170,17 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper // 800 < 900 < 1000 mockStrategy.setOutputAmountAndRecipient(actualOutput, payable(address(genericSwap))); vm.expectEmit(true, true, true, true); - emit Swap(getGSDataHash(gsData), gsData.maker, taker, taker, gsData.takerToken, gsData.takerTokenAmount, gsData.makerToken, realChangedInGS); + emit Swap( + getGSDataHash(gsData), + gsData.maker, + taker, + taker, + gsData.takerToken, + gsData.takerTokenAmount, + gsData.makerToken, + realChangedInGS, + gsData.salt + ); vm.prank(taker); genericSwap.executeSwap(gsData, defaultTakerPermit); @@ -193,7 +205,17 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper mockStrategy.setOutputAmountAndRecipient(gsData.makerTokenAmount, payable(address(genericSwap))); vm.expectEmit(true, true, true, true); - emit Swap(getGSDataHash(gsData), gsData.maker, taker, taker, gsData.takerToken, gsData.takerTokenAmount, gsData.makerToken, realChangedInGS); + emit Swap( + getGSDataHash(gsData), + gsData.maker, + taker, + taker, + gsData.takerToken, + gsData.takerTokenAmount, + gsData.makerToken, + realChangedInGS, + gsData.salt + ); vm.prank(taker); genericSwap.executeSwap{ value: gsData.takerTokenAmount }(gsData, defaultTakerPermit); @@ -219,7 +241,17 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper mockStrategy.setOutputAmountAndRecipient(gsData.makerTokenAmount, payable(address(genericSwap))); vm.expectEmit(true, true, true, true); - emit Swap(getGSDataHash(gsData), gsData.maker, taker, taker, gsData.takerToken, gsData.takerTokenAmount, gsData.makerToken, realChangedInGS); + emit Swap( + getGSDataHash(gsData), + gsData.maker, + taker, + taker, + gsData.takerToken, + gsData.takerTokenAmount, + gsData.makerToken, + realChangedInGS, + gsData.salt + ); vm.prank(taker); genericSwap.executeSwap(gsData, defaultTakerPermit); @@ -291,7 +323,8 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper defaultGSData.takerToken, defaultGSData.takerTokenAmount, defaultGSData.makerToken, - defaultGSData.makerTokenAmount + defaultGSData.makerTokenAmount, + defaultGSData.salt ); bytes memory takerSig = signGenericSwap(takerPrivateKey, defaultGSData, address(genericSwap)); @@ -339,7 +372,8 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper defaultGSData.takerToken, defaultGSData.takerTokenAmount, defaultGSData.makerToken, - defaultGSData.makerTokenAmount + defaultGSData.makerTokenAmount, + defaultGSData.salt ); vm.prank(taker); @@ -371,7 +405,8 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper aliceGSData.takerToken, aliceGSData.takerTokenAmount, aliceGSData.makerToken, - aliceGSData.makerTokenAmount + aliceGSData.makerTokenAmount, + aliceGSData.salt ); vm.startPrank(alice); From 34e3df5ba5b547d758fbf38f3a3efeec4d82db20 Mon Sep 17 00:00:00 2001 From: alex0207s Date: Tue, 30 Apr 2024 17:25:09 +0800 Subject: [PATCH 02/11] correct handling logic for makerToken as ETH and fix typo --- contracts/RFQ.sol | 25 ++++++++++++++----------- test/forkMainnet/RFQ.t.sol | 4 ++-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/contracts/RFQ.sol b/contracts/RFQ.sol index 971f7957..655af4ae 100644 --- a/contracts/RFQ.sol +++ b/contracts/RFQ.sol @@ -118,10 +118,10 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { // transfer takerToken to maker if (_rfqOffer.takerToken.isETH()) { if (msg.value != _rfqTx.takerRequestAmount) revert InvalidMsgValue(); - _collecETHAndSend(_rfqOffer.maker, _rfqTx.takerRequestAmount, ((_rfqOffer.flags & FLG_MAKER_RECEIVES_WETH) != 0)); + _collectETHAndSend(_rfqOffer.maker, _rfqTx.takerRequestAmount, ((_rfqOffer.flags & FLG_MAKER_RECEIVES_WETH) != 0)); } else if (_rfqOffer.takerToken == address(weth)) { if (msg.value != 0) revert InvalidMsgValue(); - _collecWETHAndSend( + _collectWETHAndSend( _rfqOffer.taker, _rfqOffer.maker, _rfqTx.takerRequestAmount, @@ -133,13 +133,19 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { _collect(_rfqOffer.takerToken, _rfqOffer.taker, _rfqOffer.maker, _rfqTx.takerRequestAmount, _takerTokenPermit); } - // collect makerToken from maker to this + // collect makerToken from maker to this contract uint256 makerSettleAmount = _rfqOffer.makerTokenAmount; if (_rfqTx.takerRequestAmount != _rfqOffer.takerTokenAmount) { makerSettleAmount = (_rfqTx.takerRequestAmount * _rfqOffer.makerTokenAmount) / _rfqOffer.takerTokenAmount; } if (makerSettleAmount == 0) revert InvalidMakerAmount(); - _collect(_rfqOffer.makerToken, _rfqOffer.maker, address(this), makerSettleAmount, _makerTokenPermit); + // 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()) { + _collect(address(weth), _rfqOffer.maker, address(this), makerSettleAmount, _makerTokenPermit); + } else { + _collect(_rfqOffer.makerToken, _rfqOffer.maker, address(this), makerSettleAmount, _makerTokenPermit); + } // calculate maker token settlement amount (sub fee) uint256 fee = (makerSettleAmount * _rfqOffer.feeFactor) / Constant.BPS_MAX; @@ -150,12 +156,9 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { } { - // determine if WETH unwrap is needed, send out ETH if makerToken is WETH + // unwrap WETH and send out ETH if makerToken is ETH address makerToken = _rfqOffer.makerToken; - if (makerToken == address(weth)) { - weth.withdraw(makerSettleAmount); - makerToken = Constant.ETH_ADDRESS; - } + if (makerToken.isETH()) weth.withdraw(makerSettleAmount); // collect fee makerToken.transferTo(feeCollector, fee); @@ -181,7 +184,7 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { } // Only used when taker token is ETH - function _collecETHAndSend(address payable to, uint256 amount, bool makerReceivesWETH) internal { + function _collectETHAndSend(address payable to, uint256 amount, bool makerReceivesWETH) internal { if (makerReceivesWETH) { weth.deposit{ value: amount }(); weth.transfer(to, amount); @@ -191,7 +194,7 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { } // Only used when taker token is WETH - function _collecWETHAndSend(address from, address payable to, uint256 amount, bytes calldata data, bool makerReceivesWETH) internal { + function _collectWETHAndSend(address from, address payable to, uint256 amount, bytes calldata data, bool makerReceivesWETH) internal { if (makerReceivesWETH) { _collect(address(weth), from, to, amount, data); } else { diff --git a/test/forkMainnet/RFQ.t.sol b/test/forkMainnet/RFQ.t.sol index 8b8b4d73..ad5ad714 100644 --- a/test/forkMainnet/RFQ.t.sol +++ b/test/forkMainnet/RFQ.t.sol @@ -295,8 +295,8 @@ contract RFQTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper { Snapshot memory makerMakerToken = BalanceSnapshot.take({ owner: rfqOffer.maker, token: rfqOffer.makerToken }); // recipient should receive raw ETH Snapshot memory recTakerToken = BalanceSnapshot.take({ owner: recipient, token: rfqOffer.takerToken }); - Snapshot memory recMakerToken = BalanceSnapshot.take({ owner: recipient, token: Constant.ZERO_ADDRESS }); - Snapshot memory fcMakerToken = BalanceSnapshot.take({ owner: feeCollector, token: Constant.ZERO_ADDRESS }); + 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; From 52dad8038b93c228d2246b2b63f76065682b1e2f Mon Sep 17 00:00:00 2001 From: alex0207s Date: Tue, 21 May 2024 14:13:34 +0800 Subject: [PATCH 03/11] adjust input token ratio calculation method --- contracts/SmartOrderStrategy.sol | 23 ++++++++++++------- contracts/interfaces/ISmartOrderStrategy.sol | 6 +++-- test/forkMainnet/GenericSwap.t.sol | 3 ++- .../forkMainnet/SmartOrderStrategy/AMMs.t.sol | 22 +++++++++++------- .../SmartOrderStrategy/IntegrationV6.t.sol | 6 +++-- .../SmartOrderStrategy/Setup.t.sol | 2 +- .../SmartOrderStrategy/Validation.t.sol | 12 ++++++++++ 7 files changed, 52 insertions(+), 22 deletions(-) diff --git a/contracts/SmartOrderStrategy.sol b/contracts/SmartOrderStrategy.sol index 25a51ee7..c0019b2b 100644 --- a/contracts/SmartOrderStrategy.sol +++ b/contracts/SmartOrderStrategy.sol @@ -5,7 +5,6 @@ import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { AdminManagement } from "./abstracts/AdminManagement.sol"; import { Asset } from "./libraries/Asset.sol"; -import { Constant } from "./libraries/Constant.sol"; import { IWETH } from "./interfaces/IWETH.sol"; import { ISmartOrderStrategy } from "./interfaces/ISmartOrderStrategy.sol"; import { IStrategy } from "./interfaces/IStrategy.sol"; @@ -44,7 +43,7 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement { uint256 opsCount = ops.length; for (uint256 i = 0; i < opsCount; ++i) { Operation memory op = ops[i]; - _call(op.dest, op.inputToken, op.inputRatio, op.dataOffset, op.value, op.data); + _call(op.dest, op.inputToken, op.ratioNumerator, op.ratioDenominator, op.dataOffset, op.value, op.data); } // transfer output token back to GenericSwap @@ -65,11 +64,17 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement { Asset.transferTo(outputToken, payable(genericSwap), selfBalance); } - function _call(address _dest, address _inputToken, uint128 _inputRatio, uint128 _dataOffset, uint256 _value, bytes memory _data) internal { - if (_inputRatio > Constant.BPS_MAX) revert InvalidInputRatio(); - + function _call( + address _dest, + address _inputToken, + uint256 _ratioNumerator, + uint256 _ratioDenominator, + uint256 _dataOffset, + uint256 _value, + bytes memory _data + ) internal { // replace amount if ratio != 0 - if (_inputRatio != 0) { + if (_ratioNumerator != 0) { uint256 inputTokenBalance = IERC20(_inputToken).balanceOf(address(this)); // leaving one wei for gas optimization if (inputTokenBalance > 1) { @@ -79,8 +84,10 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement { } // calculate input amount if ratio should be applied - if (_inputRatio != Constant.BPS_MAX) { - inputTokenBalance = (inputTokenBalance * _inputRatio) / Constant.BPS_MAX; + // we provide a _ratioNumerator and a _ratioDenominator to decide a ratio + if (_ratioNumerator != _ratioDenominator) { + if (_ratioDenominator == 0) revert ZeroDenominator(); + inputTokenBalance = (inputTokenBalance * _ratioNumerator) / _ratioDenominator; } assembly { mstore(add(_data, _dataOffset), inputTokenBalance) diff --git a/contracts/interfaces/ISmartOrderStrategy.sol b/contracts/interfaces/ISmartOrderStrategy.sol index 0354e1ad..397d4493 100644 --- a/contracts/interfaces/ISmartOrderStrategy.sol +++ b/contracts/interfaces/ISmartOrderStrategy.sol @@ -7,6 +7,7 @@ import { IStrategy } from "./IStrategy.sol"; /// @author imToken Labs interface ISmartOrderStrategy is IStrategy { error ZeroInput(); + error ZeroDenominator(); error EmptyOps(); error InvalidMsgValue(); error InvalidInputRatio(); @@ -16,8 +17,9 @@ interface ISmartOrderStrategy is IStrategy { struct Operation { address dest; address inputToken; - uint128 inputRatio; - uint128 dataOffset; + uint256 ratioNumerator; + uint256 ratioDenominator; + uint256 dataOffset; uint256 value; bytes data; } diff --git a/test/forkMainnet/GenericSwap.t.sol b/test/forkMainnet/GenericSwap.t.sol index 47890adb..47be301e 100644 --- a/test/forkMainnet/GenericSwap.t.sol +++ b/test/forkMainnet/GenericSwap.t.sol @@ -98,7 +98,8 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper operations[0] = ISmartOrderStrategy.Operation({ dest: UNISWAP_SWAP_ROUTER_02_ADDRESS, inputToken: defaultInputToken, - inputRatio: 0, // zero ratio indicate no replacement + ratioNumerator: 0, // zero ratio indicate no replacement + ratioDenominator: 0, dataOffset: 0, value: 0, data: routerPayload diff --git a/test/forkMainnet/SmartOrderStrategy/AMMs.t.sol b/test/forkMainnet/SmartOrderStrategy/AMMs.t.sol index 2f95d5f0..3358d6f5 100644 --- a/test/forkMainnet/SmartOrderStrategy/AMMs.t.sol +++ b/test/forkMainnet/SmartOrderStrategy/AMMs.t.sol @@ -37,7 +37,8 @@ contract AMMsTest is SmartOrderStrategyTest { operations[0] = ISmartOrderStrategy.Operation({ dest: UNISWAP_SWAP_ROUTER_02_ADDRESS, inputToken: defaultInputToken, - inputRatio: 0, // zero ratio indicate no replacement + ratioNumerator: 0, // zero ratio indicate no replacement + ratioDenominator: 0, dataOffset: 0, value: 0, data: uniswapData @@ -78,8 +79,9 @@ contract AMMsTest is SmartOrderStrategyTest { operations[0] = ISmartOrderStrategy.Operation({ dest: UNISWAP_SWAP_ROUTER_02_ADDRESS, inputToken: defaultInputToken, - inputRatio: defaultInputRatio, - dataOffset: uint128(4 + 32 + 128), // add 32 bytes of length prefix + ratioNumerator: defaultInputRatio, + ratioDenominator: 10000, + dataOffset: 4 + 32 + 128, // add 32 bytes of length prefix value: 0, data: uniswapData }); @@ -117,8 +119,9 @@ contract AMMsTest is SmartOrderStrategyTest { operations[0] = ISmartOrderStrategy.Operation({ dest: UNISWAP_SWAP_ROUTER_02_ADDRESS, inputToken: defaultInputToken, - inputRatio: Constant.BPS_MAX, // BPS_MAX indicate the input amount will be replaced by the actual balance - dataOffset: uint128(4 + 32 + 128), // add 32 bytes of length prefix + ratioNumerator: 1, // same numerator and ratioDenominator indicate the input amount will be replaced by the actual balance + ratioDenominator: 1, + dataOffset: 4 + 32 + 128, // add 32 bytes of length prefix value: 0, data: uniswapData }); @@ -161,7 +164,8 @@ contract AMMsTest is SmartOrderStrategyTest { operations[0] = ISmartOrderStrategy.Operation({ dest: UNISWAP_SWAP_ROUTER_02_ADDRESS, inputToken: defaultInputToken, - inputRatio: 0, // zero ratio indicate no replacement + ratioNumerator: 0, // zero ratio indicate no replacement + ratioDenominator: 0, dataOffset: 0, value: 0, data: uniswapData @@ -218,7 +222,8 @@ contract AMMsTest is SmartOrderStrategyTest { operations[0] = ISmartOrderStrategy.Operation({ dest: UNISWAP_SWAP_ROUTER_02_ADDRESS, inputToken: USDC_ADDRESS, - inputRatio: 0, // zero ratio indicate no replacement + ratioNumerator: 0, // zero ratio indicate no replacement + ratioDenominator: 0, dataOffset: 0, value: 0, data: uniswapData @@ -226,7 +231,8 @@ contract AMMsTest is SmartOrderStrategyTest { operations[1] = ISmartOrderStrategy.Operation({ dest: CURVE_TRICRYPTO2_POOL_ADDRESS, inputToken: WETH_ADDRESS, - inputRatio: 0, // zero ratio indicate no replacement + ratioNumerator: 0, // zero ratio indicate no replacement + ratioDenominator: 0, dataOffset: 0, value: 0, data: curveData diff --git a/test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol b/test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol index 37b1d61d..907f060b 100644 --- a/test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol +++ b/test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol @@ -75,7 +75,8 @@ contract IntegrationV6Test is SmartOrderStrategyTest, SigHelper { operations[0] = ISmartOrderStrategy.Operation({ dest: address(rfq), inputToken: rfqOffer.takerToken, - inputRatio: 0, // zero ratio indicate no replacement + ratioNumerator: 0, // zero ratio indicate no replacement + ratioDenominator: 0, dataOffset: 0, value: 0, data: rfqData @@ -123,7 +124,8 @@ contract IntegrationV6Test is SmartOrderStrategyTest, SigHelper { operations[0] = ISmartOrderStrategy.Operation({ dest: address(limitOrderSwap), inputToken: order.takerToken, - inputRatio: 0, // zero ratio indicate no replacement + ratioNumerator: 0, // zero ratio indicate no replacement + ratioDenominator: 0, dataOffset: 0, value: 0, data: loData diff --git a/test/forkMainnet/SmartOrderStrategy/Setup.t.sol b/test/forkMainnet/SmartOrderStrategy/Setup.t.sol index d507f46a..16d60bef 100644 --- a/test/forkMainnet/SmartOrderStrategy/Setup.t.sol +++ b/test/forkMainnet/SmartOrderStrategy/Setup.t.sol @@ -14,7 +14,7 @@ contract SmartOrderStrategyTest is Test, Tokens, BalanceUtil { address defaultInputToken = USDC_ADDRESS; address defaultOutputToken = WETH_ADDRESS; uint256 defaultInputAmount = 1000; - uint128 defaultInputRatio = 5000; + uint256 defaultInputRatio = 5000; uint256 defaultExpiry = block.timestamp + 100; bytes defaultOpsData; bytes encodedUniv3Path; diff --git a/test/forkMainnet/SmartOrderStrategy/Validation.t.sol b/test/forkMainnet/SmartOrderStrategy/Validation.t.sol index 664f71ca..e65837d5 100644 --- a/test/forkMainnet/SmartOrderStrategy/Validation.t.sol +++ b/test/forkMainnet/SmartOrderStrategy/Validation.t.sol @@ -17,6 +17,18 @@ contract ValidationTest is SmartOrderStrategyTest { smartOrderStrategy.executeStrategy(defaultInputToken, defaultOutputToken, 0, defaultOpsData); } + function testCannotExecuteWithZeroRatioDenominatorWhenRatioNumeratorIsNonZero() public { + ISmartOrderStrategy.Operation[] memory operations = new ISmartOrderStrategy.Operation[](1); + operations[0].inputToken = USDC_ADDRESS; + operations[0].ratioNumerator = 1; + operations[0].ratioDenominator = 0; + bytes memory opsData = abi.encode(operations); + + vm.expectRevert(ISmartOrderStrategy.ZeroDenominator.selector); + vm.prank(genericSwap, genericSwap); + smartOrderStrategy.executeStrategy(defaultInputToken, defaultOutputToken, defaultInputAmount, opsData); + } + function testCannotExecuteWithFailDecodedData() public { vm.expectRevert(); vm.prank(genericSwap, genericSwap); From 32951ad4448c48bf7a98ab71c5d3cb3055152bc2 Mon Sep 17 00:00:00 2001 From: alex0207s Date: Tue, 2 Jul 2024 15:50:00 +0800 Subject: [PATCH 04/11] remove uniAgent contract --- contracts/UniAgent.sol | 115 -------------- contracts/interfaces/IUniAgent.sol | 39 ----- doc/UniAgent.md | 3 - test/forkMainnet/UniAgent/Setup.t.sol | 49 ------ test/forkMainnet/UniAgent/SwapRouter02.t.sol | 72 --------- test/forkMainnet/UniAgent/Universal.t.sol | 81 ---------- test/forkMainnet/UniAgent/V2.t.sol | 158 ------------------- test/forkMainnet/UniAgent/V3.t.sol | 92 ----------- 8 files changed, 609 deletions(-) delete mode 100644 contracts/UniAgent.sol delete mode 100644 contracts/interfaces/IUniAgent.sol delete mode 100644 doc/UniAgent.md delete mode 100644 test/forkMainnet/UniAgent/Setup.t.sol delete mode 100644 test/forkMainnet/UniAgent/SwapRouter02.t.sol delete mode 100644 test/forkMainnet/UniAgent/Universal.t.sol delete mode 100644 test/forkMainnet/UniAgent/V2.t.sol delete mode 100644 test/forkMainnet/UniAgent/V3.t.sol diff --git a/contracts/UniAgent.sol b/contracts/UniAgent.sol deleted file mode 100644 index 5c769bec..00000000 --- a/contracts/UniAgent.sol +++ /dev/null @@ -1,115 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.17; - -import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; - -import { TokenCollector } from "./abstracts/TokenCollector.sol"; -import { Ownable } from "./abstracts/Ownable.sol"; -import { IUniAgent } from "./interfaces/IUniAgent.sol"; -import { Asset } from "./libraries/Asset.sol"; - -contract UniAgent is IUniAgent, Ownable, TokenCollector { - using Asset for address; - - address private constant v2Router = 0x7a250d5630B4cF539739dF2C5dAcb4c659F2488D; - address private constant v3Router = 0xE592427A0AEce92De3Edee1F18E0157C05861564; - address private constant swapRouter02 = 0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45; - address payable private constant universalRouter = payable(0x3fC91A3afd70395Cd496C647d5a6CC9D4B2b7FAD); - - constructor(address _owner, address _uniswapPermit2, address _allowanceTarget) Ownable(_owner) TokenCollector(_uniswapPermit2, _allowanceTarget) {} - - receive() external payable {} - - 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)); - if (selfBalance > 0) { - Asset.transferTo(tokens[i], payable(recipient), selfBalance); - } - } - } - - function approveTokensToRouters(address[] calldata tokens) external { - for (uint256 i = 0; i < tokens.length; ++i) { - // use low level call to avoid return size check - // ignore return value and proceed anyway since three calls are independent - tokens[i].call(abi.encodeCall(IERC20.approve, (v2Router, type(uint256).max))); - tokens[i].call(abi.encodeCall(IERC20.approve, (v3Router, type(uint256).max))); - tokens[i].call(abi.encodeCall(IERC20.approve, (swapRouter02, type(uint256).max))); - } - } - - /// @inheritdoc IUniAgent - function approveAndSwap( - RouterType routerType, - address inputToken, - uint256 inputAmount, - bytes calldata payload, - bytes calldata userPermit - ) external payable override { - _swap(routerType, true, inputToken, inputAmount, payload, userPermit); - } - - /// @inheritdoc IUniAgent - function swap(RouterType routerType, address inputToken, uint256 inputAmount, bytes calldata payload, bytes calldata userPermit) external payable override { - _swap(routerType, false, inputToken, inputAmount, payload, userPermit); - } - - function _swap( - RouterType routerType, - bool needApprove, - address inputToken, - uint256 inputAmount, - bytes calldata payload, - bytes calldata userPermit - ) private { - address routerAddr = _getRouterAddress(routerType); - if (needApprove) { - // use low level call to avoid return size check - (bool apvSuccess, bytes memory apvResult) = inputToken.call(abi.encodeCall(IERC20.approve, (routerAddr, type(uint256).max))); - if (!apvSuccess) { - assembly { - revert(add(apvResult, 32), mload(apvResult)) - } - } - } - - if (inputToken.isETH()) { - if (msg.value != inputAmount) revert InvalidMsgValue(); - } - if (!inputToken.isETH()) { - if (msg.value != 0) revert InvalidMsgValue(); - - if (routerType == RouterType.UniversalRouter) { - // deposit directly into router if it's universal router - _collect(inputToken, msg.sender, universalRouter, inputAmount, userPermit); - } else { - // v2, v3, swapRouter02 use transferFrom - _collect(inputToken, msg.sender, address(this), inputAmount, userPermit); - } - } - (bool success, bytes memory result) = routerAddr.call{ value: msg.value }(payload); - if (!success) { - assembly { - revert(add(result, 32), mload(result)) - } - } - - emit Swap({ user: msg.sender, router: routerAddr, inputToken: inputToken, inputAmount: inputAmount }); - } - - function _getRouterAddress(RouterType routerType) private pure returns (address) { - if (routerType == RouterType.V2Router) { - return v2Router; - } else if (routerType == RouterType.V3Router) { - return v3Router; - } else if (routerType == RouterType.SwapRouter02) { - return swapRouter02; - } else if (routerType == RouterType.UniversalRouter) { - return universalRouter; - } - - // won't be reached - revert(); - } -} diff --git a/contracts/interfaces/IUniAgent.sol b/contracts/interfaces/IUniAgent.sol deleted file mode 100644 index fdfb49da..00000000 --- a/contracts/interfaces/IUniAgent.sol +++ /dev/null @@ -1,39 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; - -/// @title IUniAgent Interface -/// @author imToken Labs -interface IUniAgent { - error InvalidMsgValue(); - - /// @notice Emitted when a swap is executed - /// @param user The user address of the swap. - /// @param router The uniswap router address of the swap. - /// @param inputToken The input token address of the swap. - /// @param inputAmount The input amount of the swap. - event Swap(address indexed user, address indexed router, address indexed inputToken, uint256 inputAmount); - - /// @notice The enum of which uniswap router should be called. - enum RouterType { - V2Router, - V3Router, - SwapRouter02, - UniversalRouter - } - - /// @notice Approve token to router and execute a swap - /// @param routerType The type of uniswap router should be used. - /// @param inputToken The input token address of the swap. - /// @param inputAmount The input amount of the swap. - /// @param payload The execution payload for uniswap router. - /// @param userPermit The permit of user for token transfering. - function approveAndSwap(RouterType routerType, address inputToken, uint256 inputAmount, bytes calldata payload, bytes calldata userPermit) external payable; - - /// @notice Execute a swap - /// @param routerType The type of uniswap router should be used. - /// @param inputToken The input token address of the swap. - /// @param inputAmount The input amount of the swap. - /// @param payload The execution payload for uniswap router. - /// @param userPermit The permit of user for token transfering. - function swap(RouterType routerType, address inputToken, uint256 inputAmount, bytes calldata payload, bytes calldata userPermit) external payable; -} diff --git a/doc/UniAgent.md b/doc/UniAgent.md deleted file mode 100644 index 2f7bbbb2..00000000 --- a/doc/UniAgent.md +++ /dev/null @@ -1,3 +0,0 @@ -# UniAgent - -UniAgent is a shortcut for integrating Uniswap procotol. The first step of executing a swap is collecting input token from a user. Then the contract calls one of Uniswap routers with user specified payload. Although it's funtionalidty can be covered by GenericSwap contract but UniAgent is more friendly to simple or small swap because of lower gas consumption. diff --git a/test/forkMainnet/UniAgent/Setup.t.sol b/test/forkMainnet/UniAgent/Setup.t.sol deleted file mode 100644 index 00861e00..00000000 --- a/test/forkMainnet/UniAgent/Setup.t.sol +++ /dev/null @@ -1,49 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.17; - -import { Test } from "forge-std/Test.sol"; -import { Tokens } from "test/utils/Tokens.sol"; -import { BalanceUtil } from "test/utils/BalanceUtil.sol"; -import { getEIP712Hash } from "test/utils/Sig.sol"; -import { BalanceSnapshot, Snapshot } from "test/utils/BalanceSnapshot.sol"; -import { computeContractAddress } from "test/utils/Addresses.sol"; -import { Permit2Helper } from "test/utils/Permit2Helper.sol"; -import { UniAgent } from "contracts/UniAgent.sol"; -import { AllowanceTarget } from "contracts/AllowanceTarget.sol"; -import { TokenCollector } from "contracts/abstracts/TokenCollector.sol"; -import { Constant } from "contracts/libraries/Constant.sol"; -import { IUniAgent } from "contracts/interfaces/IUniAgent.sol"; -import { IWETH } from "contracts/interfaces/IWETH.sol"; - -contract UniAgentTest is Test, Tokens, BalanceUtil, Permit2Helper { - uint256 userPrivateKey = uint256(1); - address user = vm.addr(userPrivateKey); - address uniAgentOwner = makeAddr("uniAgentOwner"); - address allowanceTargetOwner = makeAddr("allowanceTargetOwner"); - address payable recipient = payable(makeAddr("recipient")); - uint256 defaultExpiry = block.timestamp + 1; - uint256 defaultInputAmount = 10 * 1e6; - address defaultInputToken = USDT_ADDRESS; - address defaultOutputToken = CRV_ADDRESS; - address[] defaultPath = [defaultInputToken, defaultOutputToken]; - bytes defaultUserPermit; - UniAgent uniAgent; - AllowanceTarget allowanceTarget; - - function setUp() public virtual { - // deploy allowance target - address[] memory trusted = new address[](1); - // pre-compute UniAgent address since the whitelist of allowance target is immutable - // NOTE: this assumes UniAgent is deployed right next to Allowance Target - trusted[0] = computeContractAddress(address(this), uint8(vm.getNonce(address(this)) + 1)); - allowanceTarget = new AllowanceTarget(allowanceTargetOwner, trusted); - - uniAgent = new UniAgent(uniAgentOwner, UNISWAP_PERMIT2_ADDRESS, address(allowanceTarget)); - uniAgent.approveTokensToRouters(defaultPath); - - deal(user, 100 ether); - setTokenBalanceAndApprove(user, UNISWAP_PERMIT2_ADDRESS, tokens, 100000); - - defaultUserPermit = getTokenlonPermit2Data(user, userPrivateKey, defaultInputToken, address(uniAgent)); - } -} diff --git a/test/forkMainnet/UniAgent/SwapRouter02.t.sol b/test/forkMainnet/UniAgent/SwapRouter02.t.sol deleted file mode 100644 index 7bbd35fd..00000000 --- a/test/forkMainnet/UniAgent/SwapRouter02.t.sol +++ /dev/null @@ -1,72 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.17; - -import { IUniswapSwapRouter02 } from "test/utils/IUniswapSwapRouter02.sol"; -import { IUniswapV3Quoter } from "test/utils/IUniswapV3Quoter.sol"; -import { IUniAgent } from "contracts/interfaces/IUniAgent.sol"; -import { UniswapV3 } from "test/utils/UniswapV3.sol"; -import { BalanceSnapshot, Snapshot } from "test/utils/BalanceSnapshot.sol"; -import { UniswapV2Library } from "test/utils/UniswapV2Library.sol"; -import { UniAgentTest } from "test/forkMainnet/UniAgent/Setup.t.sol"; - -contract SwapRouter02Test is UniAgentTest { - using BalanceSnapshot for Snapshot; - - IUniswapV3Quoter v3Quoter = IUniswapV3Quoter(UNISWAP_V3_QUOTER_ADDRESS); - uint256 defaultOutputAmount; - uint24 defaultFee = 3000; - uint24[] v3Fees = [defaultFee]; - - function setUp() public override { - super.setUp(); - } - - function testV2SwapExactTokensForTokens() public { - // USDT -> CRV - Snapshot memory userInputToken = BalanceSnapshot.take({ owner: user, token: defaultInputToken }); - Snapshot memory recvOutputToken = BalanceSnapshot.take({ owner: recipient, token: defaultOutputToken }); - - uint256[] memory amounts = UniswapV2Library.getAmountsOut(defaultInputAmount, defaultPath); - uint256 outputAmount = amounts[amounts.length - 1]; - uint256 minOutputAmount = (defaultOutputAmount * 95) / 100; // default 5% slippage tolerance - bytes memory payload = abi.encodeCall(IUniswapSwapRouter02.swapExactTokensForTokens, (defaultInputAmount, minOutputAmount, defaultPath, recipient)); - - vm.prank(user); - uniAgent.swap(IUniAgent.RouterType.SwapRouter02, defaultInputToken, defaultInputAmount, payload, defaultUserPermit); - - userInputToken.assertChange(-int256(defaultInputAmount)); - // recipient should receive exact amount of quote from Uniswap - recvOutputToken.assertChange(int256(outputAmount)); - } - - function testV3ExactInputSingle() public { - // USDT -> CRV - Snapshot memory userInputToken = BalanceSnapshot.take({ owner: user, token: defaultInputToken }); - Snapshot memory recvOutputToken = BalanceSnapshot.take({ owner: recipient, token: defaultOutputToken }); - - bytes memory encodedPath = UniswapV3.encodePath(defaultPath, v3Fees); - defaultOutputAmount = v3Quoter.quoteExactInput(encodedPath, defaultInputAmount); - uint256 minOutputAmount = (defaultOutputAmount * 95) / 100; // default 5% slippage tolerance - bytes memory payload = abi.encodeCall( - IUniswapSwapRouter02.exactInputSingle, - ( - IUniswapSwapRouter02.ExactInputSingleParams({ - tokenIn: defaultInputToken, - tokenOut: defaultOutputToken, - fee: defaultFee, - recipient: recipient, - amountIn: defaultInputAmount, - amountOutMinimum: minOutputAmount, - sqrtPriceLimitX96: 0 - }) - ) - ); - - vm.prank(user); - uniAgent.swap(IUniAgent.RouterType.SwapRouter02, defaultInputToken, defaultInputAmount, payload, defaultUserPermit); - - userInputToken.assertChange(-int256(defaultInputAmount)); - // recipient should receive exact amount of quote from Uniswap - recvOutputToken.assertChange(int256(defaultOutputAmount)); - } -} diff --git a/test/forkMainnet/UniAgent/Universal.t.sol b/test/forkMainnet/UniAgent/Universal.t.sol deleted file mode 100644 index b96ee268..00000000 --- a/test/forkMainnet/UniAgent/Universal.t.sol +++ /dev/null @@ -1,81 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.17; - -import { IUniswapV3Quoter } from "test/utils/IUniswapV3Quoter.sol"; -import { IUniversalRouter } from "test/utils/IUniswapUniversalRouter.sol"; -import { IUniAgent } from "contracts/interfaces/IUniAgent.sol"; -import { UniswapV3 } from "test/utils/UniswapV3.sol"; -import { UniswapCommands } from "test/utils/UniswapCommands.sol"; -import { UniswapV2Library } from "test/utils/UniswapV2Library.sol"; -import { BalanceSnapshot, Snapshot } from "test/utils/BalanceSnapshot.sol"; -import { UniAgentTest } from "test/forkMainnet/UniAgent/Setup.t.sol"; - -contract UniversalTest is UniAgentTest { - using BalanceSnapshot for Snapshot; - - IUniswapV3Quoter v3Quoter; - - function setUp() public override { - super.setUp(); - - v3Quoter = IUniswapV3Quoter(UNISWAP_V3_QUOTER_ADDRESS); - } - - function testURV2SwapExactIn() public { - // USDT -> CRV - Snapshot memory userInputToken = BalanceSnapshot.take({ owner: user, token: defaultInputToken }); - Snapshot memory recvOutputToken = BalanceSnapshot.take({ owner: recipient, token: defaultOutputToken }); - - uint256[] memory amounts = UniswapV2Library.getAmountsOut(defaultInputAmount, defaultPath); - uint256 outputAmount = amounts[amounts.length - 1]; - uint256 minOutputAmount = (outputAmount * 95) / 100; // default 5% slippage tolerance - - bytes memory cmds = abi.encodePacked(bytes1(uint8(UniswapCommands.V2_SWAP_EXACT_IN))); - bytes[] memory inputs = new bytes[](1); - inputs[0] = abi.encode(recipient, defaultInputAmount, minOutputAmount, defaultPath, false); - bytes memory payload = abi.encodeCall(IUniversalRouter.execute, (cmds, inputs, defaultExpiry)); - - vm.prank(user); - uniAgent.swap(IUniAgent.RouterType.UniversalRouter, defaultInputToken, defaultInputAmount, payload, defaultUserPermit); - - userInputToken.assertChange(-int256(defaultInputAmount)); - // recipient should receive exact amount of quote from Uniswap - recvOutputToken.assertChange(int256(outputAmount)); - } - - function testURV3SwapExactIn() public { - // USDT -> CRV - Snapshot memory userInputToken = BalanceSnapshot.take({ owner: user, token: defaultInputToken }); - Snapshot memory recvOutputToken = BalanceSnapshot.take({ owner: recipient, token: defaultOutputToken }); - - uint24[] memory v3Fees = new uint24[](1); - v3Fees[0] = 3000; - bytes memory encodedPath = UniswapV3.encodePath(defaultPath, v3Fees); - uint256 outputAmount = v3Quoter.quoteExactInput(encodedPath, defaultInputAmount); - - bytes memory cmds = abi.encodePacked(bytes1(uint8(UniswapCommands.V3_SWAP_EXACT_IN))); - bytes[] memory inputs = new bytes[](1); - inputs[0] = abi.encode(recipient, defaultInputAmount, outputAmount, encodedPath, false); - bytes memory payload = abi.encodeCall(IUniversalRouter.execute, (cmds, inputs, defaultExpiry)); - - vm.prank(user); - uniAgent.swap(IUniAgent.RouterType.UniversalRouter, defaultInputToken, defaultInputAmount, payload, defaultUserPermit); - - userInputToken.assertChange(-int256(defaultInputAmount)); - // recipient should receive exact amount of quote from Uniswap - recvOutputToken.assertChange(int256(outputAmount)); - } - - function testURHandleRouterError() public { - vm.warp(defaultExpiry + 1); - - bytes memory cmds = abi.encodePacked(bytes1(uint8(UniswapCommands.V2_SWAP_EXACT_IN))); - bytes[] memory inputs = new bytes[](1); - inputs[0] = abi.encode(recipient, defaultInputAmount, 0, defaultPath, false); - bytes memory payload = abi.encodeCall(IUniversalRouter.execute, (cmds, inputs, defaultExpiry)); - - vm.expectRevert(IUniversalRouter.TransactionDeadlinePassed.selector); - vm.prank(user); - uniAgent.swap(IUniAgent.RouterType.UniversalRouter, defaultInputToken, defaultInputAmount, payload, defaultUserPermit); - } -} diff --git a/test/forkMainnet/UniAgent/V2.t.sol b/test/forkMainnet/UniAgent/V2.t.sol deleted file mode 100644 index a1d82b22..00000000 --- a/test/forkMainnet/UniAgent/V2.t.sol +++ /dev/null @@ -1,158 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.17; - -import { IUniAgent } from "contracts/interfaces/IUniAgent.sol"; -import { IUniswapV2Router } from "test/utils/IUniswapV2Router.sol"; -import { Constant } from "contracts/libraries/Constant.sol"; -import { BalanceSnapshot, Snapshot } from "test/utils/BalanceSnapshot.sol"; -import { UniswapV2Library } from "test/utils/UniswapV2Library.sol"; -import { UniAgentTest } from "test/forkMainnet/UniAgent/Setup.t.sol"; - -contract V2Test is UniAgentTest { - using BalanceSnapshot for Snapshot; - - uint256 defaultOutputAmount; - bytes defaultRouterPayload; - - function setUp() public override { - super.setUp(); - - uint256[] memory amounts = UniswapV2Library.getAmountsOut(defaultInputAmount, defaultPath); - defaultOutputAmount = amounts[amounts.length - 1]; - uint256 minOutputAmount = (defaultOutputAmount * 95) / 100; // default 5% slippage tolerance - - defaultRouterPayload = abi.encodeCall( - IUniswapV2Router.swapExactTokensForTokens, - (defaultInputAmount, minOutputAmount, defaultPath, recipient, defaultExpiry) - ); - } - - function testV2SwapExactTokensForTokens() public { - // USDT -> CRV - Snapshot memory userInputToken = BalanceSnapshot.take({ owner: user, token: defaultInputToken }); - Snapshot memory recvOutputToken = BalanceSnapshot.take({ owner: recipient, token: defaultOutputToken }); - - vm.prank(user); - uniAgent.swap(IUniAgent.RouterType.V2Router, defaultInputToken, defaultInputAmount, defaultRouterPayload, defaultUserPermit); - - userInputToken.assertChange(-int256(defaultInputAmount)); - // recipient should receive exact amount of quote from Uniswap - recvOutputToken.assertChange(int256(defaultOutputAmount)); - } - - function testV2SwapExactETHForTokens() public { - // ETH -> CRV - address inputToken = Constant.ETH_ADDRESS; - address[] memory path = new address[](2); - // uniswap always use WETH in path - path[0] = WETH_ADDRESS; - path[1] = CRV_ADDRESS; - - Snapshot memory userInputToken = BalanceSnapshot.take({ owner: user, token: inputToken }); - Snapshot memory recvOutputToken = BalanceSnapshot.take({ owner: recipient, token: path[1] }); - - uint256[] memory amounts = UniswapV2Library.getAmountsOut(defaultInputAmount, path); - uint256 outputAmount = amounts[amounts.length - 1]; - - bytes memory payload = abi.encodeCall(IUniswapV2Router.swapExactETHForTokens, (outputAmount, path, recipient, defaultExpiry)); - - vm.prank(user); - uniAgent.swap{ value: defaultInputAmount }(IUniAgent.RouterType.V2Router, inputToken, defaultInputAmount, payload, defaultUserPermit); - - userInputToken.assertChange(-int256(defaultInputAmount)); - // recipient should receive exact amount of quote from Uniswap - recvOutputToken.assertChange(int256(outputAmount)); - } - - function testV2SwapExactTokensForETH() public { - // USDT -> ETH - address outputToken = Constant.ETH_ADDRESS; - address[] memory path = new address[](2); - // uniswap always use WETH in path - path[0] = USDT_ADDRESS; - path[1] = WETH_ADDRESS; - - Snapshot memory userInputToken = BalanceSnapshot.take({ owner: user, token: path[0] }); - Snapshot memory recvOutputToken = BalanceSnapshot.take({ owner: recipient, token: outputToken }); - - uint256[] memory amounts = UniswapV2Library.getAmountsOut(defaultInputAmount, path); - uint256 outputAmount = amounts[amounts.length - 1]; - - bytes memory payload = abi.encodeCall(IUniswapV2Router.swapExactTokensForETH, (defaultInputAmount, outputAmount, path, recipient, defaultExpiry)); - - vm.prank(user); - uniAgent.swap(IUniAgent.RouterType.V2Router, path[0], defaultInputAmount, payload, defaultUserPermit); - - userInputToken.assertChange(-int256(defaultInputAmount)); - // recipient should receive exact amount of quote from Uniswap - recvOutputToken.assertChange(int256(outputAmount)); - } - - function testV2ApproveAndSwap() public { - // USDC -> CRV - address inputToken = USDC_ADDRESS; - Snapshot memory userInputToken = BalanceSnapshot.take({ owner: user, token: inputToken }); - Snapshot memory recvOutputToken = BalanceSnapshot.take({ owner: recipient, token: defaultOutputToken }); - - address[] memory path = new address[](2); - path[0] = inputToken; - path[1] = defaultOutputToken; - - uint256[] memory amounts = UniswapV2Library.getAmountsOut(defaultInputAmount, path); - uint256 outputAmount = amounts[amounts.length - 1]; - - bytes memory payload = abi.encodeCall(IUniswapV2Router.swapExactTokensForTokens, (defaultInputAmount, outputAmount, path, recipient, defaultExpiry)); - bytes memory userPermit = getTokenlonPermit2Data(user, userPrivateKey, inputToken, address(uniAgent)); - - vm.prank(user); - uniAgent.approveAndSwap(IUniAgent.RouterType.V2Router, inputToken, defaultInputAmount, payload, userPermit); - - userInputToken.assertChange(-int256(defaultInputAmount)); - // recipient should receive exact amount of quote from Uniswap - recvOutputToken.assertChange(int256(outputAmount)); - } - - function testV2WithDupplicatedApprove() public { - // case1 : input token is USDT - // USDT will revert if approve to a spender but current allownace != 0 - vm.prank(user); - vm.expectRevert(); - uniAgent.approveAndSwap(IUniAgent.RouterType.V2Router, defaultInputToken, defaultInputAmount, defaultRouterPayload, defaultUserPermit); - - // case2 : input token is WETH - // WETH will overwrite allowance without any check - address inputToken = WETH_ADDRESS; - Snapshot memory userInputToken = BalanceSnapshot.take({ owner: user, token: inputToken }); - Snapshot memory recvOutputToken = BalanceSnapshot.take({ owner: recipient, token: defaultOutputToken }); - - address[] memory path = new address[](2); - path[0] = inputToken; - path[1] = defaultOutputToken; - - uint256[] memory amounts = UniswapV2Library.getAmountsOut(defaultInputAmount, path); - uint256 outputAmount = amounts[amounts.length - 1]; - - bytes memory payload = abi.encodeCall(IUniswapV2Router.swapExactTokensForTokens, (defaultInputAmount, outputAmount, path, recipient, defaultExpiry)); - bytes memory userPermit = getTokenlonPermit2Data(user, userPrivateKey, inputToken, address(uniAgent)); - - // should still succeed even re-approve the token - vm.startPrank(user); - address[] memory approveList = new address[](1); - approveList[0] = inputToken; - uniAgent.approveTokensToRouters(approveList); - uniAgent.approveAndSwap(IUniAgent.RouterType.V2Router, inputToken, defaultInputAmount, payload, userPermit); - vm.stopPrank(); - - userInputToken.assertChange(-int256(defaultInputAmount)); - // recipient should receive exact amount of quote from Uniswap - recvOutputToken.assertChange(int256(outputAmount)); - } - - function testV2HandleRouterError() public { - vm.warp(defaultExpiry + 1); - - vm.expectRevert("UniswapV2Router: EXPIRED"); - vm.prank(user); - uniAgent.swap(IUniAgent.RouterType.V2Router, defaultInputToken, defaultInputAmount, defaultRouterPayload, defaultUserPermit); - } -} diff --git a/test/forkMainnet/UniAgent/V3.t.sol b/test/forkMainnet/UniAgent/V3.t.sol deleted file mode 100644 index d1e97f94..00000000 --- a/test/forkMainnet/UniAgent/V3.t.sol +++ /dev/null @@ -1,92 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.17; - -import { IUniswapV3Quoter } from "test/utils/IUniswapV3Quoter.sol"; -import { IUniswapV3SwapRouter } from "test/utils/IUniswapV3SwapRouter.sol"; -import { IUniAgent } from "contracts/interfaces/IUniAgent.sol"; -import { UniswapV3 } from "test/utils/UniswapV3.sol"; -import { BalanceSnapshot, Snapshot } from "test/utils/BalanceSnapshot.sol"; -import { UniAgentTest } from "test/forkMainnet/UniAgent/Setup.t.sol"; - -contract V3Test is UniAgentTest { - using BalanceSnapshot for Snapshot; - - IUniswapV3Quoter v3Quoter; - uint24 defaultFee = 3000; - uint24[] v3Fees = [defaultFee]; - uint256 defaultOutputAmount; - bytes defaultRouterPayload; - - function setUp() public override { - super.setUp(); - - v3Quoter = IUniswapV3Quoter(UNISWAP_V3_QUOTER_ADDRESS); - bytes memory encodedPath = UniswapV3.encodePath(defaultPath, v3Fees); - defaultOutputAmount = v3Quoter.quoteExactInput(encodedPath, defaultInputAmount); - uint256 minOutputAmount = (defaultOutputAmount * 95) / 100; // default 5% slippage tolerance - - defaultRouterPayload = abi.encodeCall( - IUniswapV3SwapRouter.exactInputSingle, - ( - IUniswapV3SwapRouter.ExactInputSingleParams({ - tokenIn: defaultInputToken, - tokenOut: defaultOutputToken, - fee: defaultFee, - recipient: recipient, - deadline: defaultExpiry, - amountIn: defaultInputAmount, - amountOutMinimum: minOutputAmount, - sqrtPriceLimitX96: 0 - }) - ) - ); - } - - function testV3ExactInputSingle() public { - // USDT -> CRV - Snapshot memory userInputToken = BalanceSnapshot.take({ owner: user, token: defaultInputToken }); - Snapshot memory recvOutputToken = BalanceSnapshot.take({ owner: recipient, token: defaultOutputToken }); - - vm.prank(user); - uniAgent.swap(IUniAgent.RouterType.V3Router, defaultInputToken, defaultInputAmount, defaultRouterPayload, defaultUserPermit); - - userInputToken.assertChange(-int256(defaultInputAmount)); - // recipient should receive exact amount of quote from Uniswap - recvOutputToken.assertChange(int256(defaultOutputAmount)); - } - - function testV3ExactInput() public { - // USDT -> CRV - Snapshot memory userInputToken = BalanceSnapshot.take({ owner: user, token: defaultInputToken }); - Snapshot memory recvOutputToken = BalanceSnapshot.take({ owner: recipient, token: defaultOutputToken }); - - bytes memory encodedPath = UniswapV3.encodePath(defaultPath, v3Fees); - bytes memory payload = abi.encodeCall( - IUniswapV3SwapRouter.exactInput, - ( - IUniswapV3SwapRouter.ExactInputParams({ - path: encodedPath, - recipient: recipient, - deadline: defaultExpiry, - amountIn: defaultInputAmount, - amountOutMinimum: defaultOutputAmount - }) - ) - ); - - vm.prank(user); - uniAgent.swap(IUniAgent.RouterType.V3Router, defaultInputToken, defaultInputAmount, payload, defaultUserPermit); - - userInputToken.assertChange(-int256(defaultInputAmount)); - // recipient should receive exact amount of quote from Uniswap - recvOutputToken.assertChange(int256(defaultOutputAmount)); - } - - function testV3HandleRouterError() public { - vm.warp(defaultExpiry + 1); - - vm.expectRevert("Transaction too old"); - vm.prank(user); - uniAgent.swap(IUniAgent.RouterType.V3Router, defaultInputToken, defaultInputAmount, defaultRouterPayload, defaultUserPermit); - } -} From 19a33e74577d896da70bdddfd346d34d5c5c4a19 Mon Sep 17 00:00:00 2001 From: alex0207s Date: Tue, 28 May 2024 15:43:58 +0800 Subject: [PATCH 05/11] chore: remove unused contracts --- contracts/abstracts/Multicall.sol | 35 ------------------ contracts/interfaces/IAMMStrategy.sol | 44 ----------------------- contracts/interfaces/IBalancerV2Vault.sol | 42 ---------------------- contracts/interfaces/IMulticall.sol | 8 ----- contracts/libraries/MerkleProof.sol | 32 ----------------- 5 files changed, 161 deletions(-) delete mode 100644 contracts/abstracts/Multicall.sol delete mode 100644 contracts/interfaces/IAMMStrategy.sol delete mode 100644 contracts/interfaces/IBalancerV2Vault.sol delete mode 100644 contracts/interfaces/IMulticall.sol delete mode 100644 contracts/libraries/MerkleProof.sol diff --git a/contracts/abstracts/Multicall.sol b/contracts/abstracts/Multicall.sol deleted file mode 100644 index be28e52d..00000000 --- a/contracts/abstracts/Multicall.sol +++ /dev/null @@ -1,35 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; - -import { IMulticall } from "../interfaces/IMulticall.sol"; - -// Modified from https://github.com/Uniswap/uniswap-v3-periphery/blob/v1.1.1/contracts/base/Multicall.sol -abstract contract Multicall is IMulticall { - function multicall(bytes[] calldata data, bool revertOnFail) external override returns (bool[] memory successes, bytes[] memory results) { - successes = new bool[](data.length); - results = new bytes[](data.length); - for (uint256 i = 0; i < data.length; ++i) { - (bool success, bytes memory result) = address(this).delegatecall(data[i]); - successes[i] = success; - results[i] = result; - - if (!success) { - // Get failed reason - string memory revertReason; - if (result.length < 68) { - revertReason = "Delegatecall failed"; - } else { - assembly { - result := add(result, 0x04) - } - revertReason = abi.decode(result, (string)); - } - - if (revertOnFail) { - revert(revertReason); - } - emit MulticallFailure(i, revertReason); - } - } - } -} diff --git a/contracts/interfaces/IAMMStrategy.sol b/contracts/interfaces/IAMMStrategy.sol deleted file mode 100644 index eb501e1f..00000000 --- a/contracts/interfaces/IAMMStrategy.sol +++ /dev/null @@ -1,44 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; - -import { IStrategy } from "./IStrategy.sol"; - -/// @title IAMMStrategy Interface -/// @author imToken Labs -interface IAMMStrategy is IStrategy { - /// @notice Emitted when allowed amm is updated - /// @param ammAddr The address of the amm - /// @param enable The status of amm - event SetAMM(address ammAddr, bool enable); - - /// @notice Event emitted for each executed operation. - /// @param dest The target address of the operation - /// @param value The eth value carried when calling `dest` - /// @param selector The selector when calling `dest` - event Action(address indexed dest, uint256 value, bytes4 selector); - - /** @dev The encoded operation list should be passed as `data` when calling `IStrategy.executeStrategy` */ - struct Operation { - address dest; - uint256 value; - bytes data; - } - - /// @notice Only owner can call - /// @param _ammAddrs The amm addresses allowed to use in `executeStrategy` if according `enable` equals `true` - /// @param _enables The status of accouring amm addresses - function setAMMs(address[] calldata _ammAddrs, bool[] calldata _enables) external; - - /// @notice Only owner can call - /// @param tokens The address list of assets - /// @param spenders The address list of approved amms - /// @param usePermit2InSpenders Indicate whether spender uses Permit2 - /// @param amount The approved asset amount - function approveTokens(address[] calldata tokens, address[] calldata spenders, bool[] calldata usePermit2InSpenders, uint256 amount) external; - - /// @notice Only owner can call - /// There may be some tokens left after swap while the order has been filled - /// @param tokens The address list of legacy tokens - /// @param receiver The receiver address - function withdrawLegacyTokensTo(address[] calldata tokens, address receiver) external; -} diff --git a/contracts/interfaces/IBalancerV2Vault.sol b/contracts/interfaces/IBalancerV2Vault.sol deleted file mode 100644 index c49a1171..00000000 --- a/contracts/interfaces/IBalancerV2Vault.sol +++ /dev/null @@ -1,42 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; - -/// @dev Minimal Balancer V2 Vault interface -/// for documentation refer to https://github.com/balancer-labs/balancer-core-v2/blob/master/contracts/vault/interfaces/IVault.sol -interface IBalancerV2Vault { - enum SwapKind { - GIVEN_IN, - GIVEN_OUT - } - - struct BatchSwapStep { - bytes32 poolId; - uint256 assetInIndex; - uint256 assetOutIndex; - uint256 amount; - bytes userData; - } - - struct FundManagement { - address sender; - bool fromInternalBalance; - address payable recipient; - bool toInternalBalance; - } - - function queryBatchSwap( - SwapKind kind, - BatchSwapStep[] memory swaps, - address[] memory assets, - FundManagement memory funds - ) external returns (int256[] memory assetDeltas); - - function batchSwap( - SwapKind kind, - BatchSwapStep[] memory swaps, - address[] memory assets, - FundManagement memory funds, - int256[] memory limits, - uint256 deadline - ) external payable returns (int256[] memory); -} diff --git a/contracts/interfaces/IMulticall.sol b/contracts/interfaces/IMulticall.sol deleted file mode 100644 index 25cdd68c..00000000 --- a/contracts/interfaces/IMulticall.sol +++ /dev/null @@ -1,8 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; - -interface IMulticall { - event MulticallFailure(uint256 index, string reason); - - function multicall(bytes[] calldata data, bool revertOnFail) external returns (bool[] memory successes, bytes[] memory results); -} diff --git a/contracts/libraries/MerkleProof.sol b/contracts/libraries/MerkleProof.sol deleted file mode 100644 index 071e7cd4..00000000 --- a/contracts/libraries/MerkleProof.sol +++ /dev/null @@ -1,32 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0 -pragma solidity ^0.8.0; - -/** - * @dev These functions deal with verification of Merkle trees (hash trees), - */ -library MerkleProof { - /** - * @dev Returns true if a `leaf` can be proved to be a part of a Merkle tree - * defined by `root`. For this, a `proof` must be provided, containing - * sibling hashes on the branch from the leaf to the root of the tree. Each - * pair of leaves and each pair of pre-images are assumed to be sorted. - */ - function verify(bytes32[] memory proof, bytes32 root, bytes32 leaf) internal pure returns (bool) { - bytes32 computedHash = leaf; - - for (uint256 i = 0; i < proof.length; ++i) { - bytes32 proofElement = proof[i]; - - if (computedHash <= proofElement) { - // Hash(current computed hash + current element of the proof) - computedHash = keccak256(abi.encodePacked(computedHash, proofElement)); - } else { - // Hash(current element of the proof + current computed hash) - computedHash = keccak256(abi.encodePacked(proofElement, computedHash)); - } - } - - // Check if the computed hash (root) is equal to the provided root - return computedHash == root; - } -} From 51b4f8f74082ee8039512659f4d47d5d0cecb70f Mon Sep 17 00:00:00 2001 From: alex0207s Date: Tue, 28 May 2024 15:46:08 +0800 Subject: [PATCH 06/11] chore: remove unused token --- envrc | 1 - test/utils/Tokens.sol | 3 --- 2 files changed, 4 deletions(-) diff --git a/envrc b/envrc index 08b43e37..35497ee5 100644 --- a/envrc +++ b/envrc @@ -11,7 +11,6 @@ export USDC_ADDRESS="0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48" export DAI_ADDRESS="0x6B175474E89094C44Da98b954EedeAC495271d0F" export WBTC_ADDRESS="0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599" export LON_ADDRESS="0x0000000000095413afC295d19EDeb1Ad7B71c952" -export ANKRETH_ADDRESS="0xE95A203B1a91a908F9B9CE46459d101078c2c3cb" export ARBITRUM_L1_GATEWAY_ROUTER_ADDRESS="0x72Ce9c846789fdB6fC1f34aC4AD25Dd9ef7031ef" export ARBITRUM_L1_BRIDGE_ADDRESS="0x8315177aB297bA92A06054cE80a67Ed4DBd7ed3a" diff --git a/test/utils/Tokens.sol b/test/utils/Tokens.sol index 3384d8a0..41f3d122 100644 --- a/test/utils/Tokens.sol +++ b/test/utils/Tokens.sol @@ -13,7 +13,6 @@ contract Tokens is Addresses { IERC20 public dai; IERC20 public wbtc; IERC20 public lon; - IERC20 public ankreth; IERC20[] public tokens; constructor() { @@ -38,13 +37,11 @@ contract Tokens is Addresses { } tokens = [weth, usdt, usdc, dai, wbtc, lon]; - vm.label(address(weth), "WETH"); vm.label(address(usdt), "USDT"); vm.label(address(usdc), "USDC"); vm.label(address(dai), "DAI"); vm.label(address(wbtc), "WBTC"); vm.label(address(lon), "LON"); - vm.label(address(ankreth), "ANKRETH"); } } From 703d6b92f81843766b2526d84c81fbb624764e63 Mon Sep 17 00:00:00 2001 From: alex0207s Date: Tue, 28 May 2024 17:16:17 +0800 Subject: [PATCH 07/11] chore: upgrade solidity version to 0.8.26 and modify version for library --- contracts/AllowanceTarget.sol | 2 +- contracts/CoordinatedTaker.sol | 2 +- contracts/GenericSwap.sol | 2 +- contracts/LimitOrderSwap.sol | 2 +- contracts/RFQ.sol | 2 +- contracts/SmartOrderStrategy.sol | 2 +- contracts/abstracts/TokenCollector.sol | 2 +- contracts/interfaces/IAllowanceTarget.sol | 2 +- contracts/interfaces/ICoordinatedTaker.sol | 2 +- contracts/interfaces/IERC1271Wallet.sol | 2 +- contracts/interfaces/IGenericSwap.sol | 2 +- contracts/interfaces/ILimitOrderSwap.sol | 2 +- contracts/interfaces/IRFQ.sol | 2 +- contracts/interfaces/ISmartOrderStrategy.sol | 2 +- contracts/interfaces/IStrategy.sol | 2 +- contracts/interfaces/IUniswapPermit2.sol | 2 +- contracts/interfaces/IWETH.sol | 2 +- foundry.toml | 2 +- test/AdminManagement.t.sol | 2 +- test/AllowanceTarget.t.sol | 2 +- test/Signing.t.sol | 2 +- test/forkMainnet/GenericSwap.t.sol | 2 +- test/forkMainnet/LimitOrderSwap/CancelOrder.t.sol | 2 +- test/forkMainnet/LimitOrderSwap/CoordinatedTaker.t.sol | 2 +- test/forkMainnet/LimitOrderSwap/Fill.t.sol | 2 +- test/forkMainnet/LimitOrderSwap/FullOrKill.t.sol | 2 +- test/forkMainnet/LimitOrderSwap/GroupFill.t.sol | 2 +- test/forkMainnet/LimitOrderSwap/Management.t.sol | 2 +- test/forkMainnet/LimitOrderSwap/Setup.t.sol | 2 +- test/forkMainnet/RFQ.t.sol | 2 +- test/forkMainnet/SmartOrderStrategy/AMMs.t.sol | 2 +- test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol | 2 +- test/forkMainnet/SmartOrderStrategy/Setup.t.sol | 2 +- test/forkMainnet/SmartOrderStrategy/Validation.t.sol | 2 +- test/forkMainnet/TokenCollector.t.sol | 2 +- test/libraries/SignatureValidator.t.sol | 2 +- test/utils/ICurveFi.sol | 2 +- test/utils/ICurveFiV2.sol | 2 +- test/utils/IUniswapSwapRouter02.sol | 2 +- test/utils/IUniswapUniversalRouter.sol | 2 +- test/utils/IUniswapV2Router.sol | 2 +- test/utils/IUniswapV3Quoter.sol | 2 +- test/utils/IUniswapV3SwapRouter.sol | 2 +- test/utils/Permit2Helper.sol | 2 +- test/utils/Sig.sol | 2 +- test/utils/SigHelper.sol | 2 +- test/utils/UniswapCommands.sol | 2 +- test/utils/UniswapV2Library.sol | 2 +- 48 files changed, 48 insertions(+), 48 deletions(-) diff --git a/contracts/AllowanceTarget.sol b/contracts/AllowanceTarget.sol index c31c70fd..2879c9ae 100644 --- a/contracts/AllowanceTarget.sol +++ b/contracts/AllowanceTarget.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; diff --git a/contracts/CoordinatedTaker.sol b/contracts/CoordinatedTaker.sol index c94ffec1..ac335000 100644 --- a/contracts/CoordinatedTaker.sol +++ b/contracts/CoordinatedTaker.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { TokenCollector } from "./abstracts/TokenCollector.sol"; import { AdminManagement } from "./abstracts/AdminManagement.sol"; diff --git a/contracts/GenericSwap.sol b/contracts/GenericSwap.sol index f0e77aa9..1e7ba299 100644 --- a/contracts/GenericSwap.sol +++ b/contracts/GenericSwap.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { TokenCollector } from "./abstracts/TokenCollector.sol"; import { EIP712 } from "./abstracts/EIP712.sol"; diff --git a/contracts/LimitOrderSwap.sol b/contracts/LimitOrderSwap.sol index 939b4944..ed0fa948 100644 --- a/contracts/LimitOrderSwap.sol +++ b/contracts/LimitOrderSwap.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { ReentrancyGuard } from "@openzeppelin/contracts/security/ReentrancyGuard.sol"; diff --git a/contracts/RFQ.sol b/contracts/RFQ.sol index 655af4ae..48c7f7ca 100644 --- a/contracts/RFQ.sol +++ b/contracts/RFQ.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { Address } from "@openzeppelin/contracts/utils/Address.sol"; diff --git a/contracts/SmartOrderStrategy.sol b/contracts/SmartOrderStrategy.sol index c0019b2b..c8df5563 100644 --- a/contracts/SmartOrderStrategy.sol +++ b/contracts/SmartOrderStrategy.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; diff --git a/contracts/abstracts/TokenCollector.sol b/contracts/abstracts/TokenCollector.sol index 75408a27..bdb345d5 100644 --- a/contracts/abstracts/TokenCollector.sol +++ b/contracts/abstracts/TokenCollector.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { IERC20Permit } from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Permit.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; diff --git a/contracts/interfaces/IAllowanceTarget.sol b/contracts/interfaces/IAllowanceTarget.sol index e72d27e3..d8d83564 100644 --- a/contracts/interfaces/IAllowanceTarget.sol +++ b/contracts/interfaces/IAllowanceTarget.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; /// @title IAllowanceTarget Interface /// @author imToken Labs diff --git a/contracts/interfaces/ICoordinatedTaker.sol b/contracts/interfaces/ICoordinatedTaker.sol index 94a16f84..02e5b273 100644 --- a/contracts/interfaces/ICoordinatedTaker.sol +++ b/contracts/interfaces/ICoordinatedTaker.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; import { LimitOrder } from "../libraries/LimitOrder.sol"; diff --git a/contracts/interfaces/IERC1271Wallet.sol b/contracts/interfaces/IERC1271Wallet.sol index 27efc2e8..0c79d2ac 100644 --- a/contracts/interfaces/IERC1271Wallet.sol +++ b/contracts/interfaces/IERC1271Wallet.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; interface IERC1271Wallet { 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 0a0b3bc3..c6c4875d 100644 --- a/contracts/interfaces/IGenericSwap.sol +++ b/contracts/interfaces/IGenericSwap.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; import { GenericSwapData } from "../libraries/GenericSwapData.sol"; diff --git a/contracts/interfaces/ILimitOrderSwap.sol b/contracts/interfaces/ILimitOrderSwap.sol index dce5affb..fe5e156e 100644 --- a/contracts/interfaces/ILimitOrderSwap.sol +++ b/contracts/interfaces/ILimitOrderSwap.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; import { LimitOrder } from "../libraries/LimitOrder.sol"; diff --git a/contracts/interfaces/IRFQ.sol b/contracts/interfaces/IRFQ.sol index 1045a49c..edb976f5 100644 --- a/contracts/interfaces/IRFQ.sol +++ b/contracts/interfaces/IRFQ.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; import { RFQOffer } from "../libraries/RFQOffer.sol"; import { RFQTx } from "../libraries/RFQTx.sol"; diff --git a/contracts/interfaces/ISmartOrderStrategy.sol b/contracts/interfaces/ISmartOrderStrategy.sol index 397d4493..f0055b79 100644 --- a/contracts/interfaces/ISmartOrderStrategy.sol +++ b/contracts/interfaces/ISmartOrderStrategy.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; import { IStrategy } from "./IStrategy.sol"; diff --git a/contracts/interfaces/IStrategy.sol b/contracts/interfaces/IStrategy.sol index 629ca65a..52c5b3de 100644 --- a/contracts/interfaces/IStrategy.sol +++ b/contracts/interfaces/IStrategy.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; /// @title IStrategy Interface /// @author imToken Labs diff --git a/contracts/interfaces/IUniswapPermit2.sol b/contracts/interfaces/IUniswapPermit2.sol index 9d7c1b64..9f11b0ab 100644 --- a/contracts/interfaces/IUniswapPermit2.sol +++ b/contracts/interfaces/IUniswapPermit2.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; interface IUniswapPermit2 { /// @notice Thrown when an allowance on a token has expired. diff --git a/contracts/interfaces/IWETH.sol b/contracts/interfaces/IWETH.sol index 07fc311f..f6a83e1f 100644 --- a/contracts/interfaces/IWETH.sol +++ b/contracts/interfaces/IWETH.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; interface IWETH { function balanceOf(address account) external view returns (uint256); diff --git a/foundry.toml b/foundry.toml index 8432def2..3f9128b7 100644 --- a/foundry.toml +++ b/foundry.toml @@ -2,7 +2,7 @@ src = 'contracts' # the source directory cache = true # whether to cache builds or not force = false # whether to ignore the cache (clean build) -solc_version = "0.8.17" +solc_version = "0.8.26" optimizer = true # enable or disable the solc optimizer optimizer_runs = 1000 # the number of optimizer runs verbosity = 3 # The verbosity of tests diff --git a/test/AdminManagement.t.sol b/test/AdminManagement.t.sol index 621d0e57..4b4ccdc1 100644 --- a/test/AdminManagement.t.sol +++ b/test/AdminManagement.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; diff --git a/test/AllowanceTarget.t.sol b/test/AllowanceTarget.t.sol index 4db720bf..48840a27 100644 --- a/test/AllowanceTarget.t.sol +++ b/test/AllowanceTarget.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; diff --git a/test/Signing.t.sol b/test/Signing.t.sol index 079a8dc9..d571a9ee 100644 --- a/test/Signing.t.sol +++ b/test/Signing.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { AllowFill, ALLOWFILL_DATA_TYPEHASH } from "contracts/libraries/AllowFill.sol"; import { GenericSwapData, GS_DATA_TYPEHASH } from "contracts/libraries/GenericSwapData.sol"; diff --git a/test/forkMainnet/GenericSwap.t.sol b/test/forkMainnet/GenericSwap.t.sol index 47be301e..c8d3c372 100644 --- a/test/forkMainnet/GenericSwap.t.sol +++ b/test/forkMainnet/GenericSwap.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { Test } from "forge-std/Test.sol"; import { Tokens } from "test/utils/Tokens.sol"; diff --git a/test/forkMainnet/LimitOrderSwap/CancelOrder.t.sol b/test/forkMainnet/LimitOrderSwap/CancelOrder.t.sol index 3bdf0292..07db6c52 100644 --- a/test/forkMainnet/LimitOrderSwap/CancelOrder.t.sol +++ b/test/forkMainnet/LimitOrderSwap/CancelOrder.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { getLimitOrderHash } from "contracts/libraries/LimitOrder.sol"; import { ILimitOrderSwap } from "contracts/interfaces/ILimitOrderSwap.sol"; diff --git a/test/forkMainnet/LimitOrderSwap/CoordinatedTaker.t.sol b/test/forkMainnet/LimitOrderSwap/CoordinatedTaker.t.sol index 078005e0..6005bb6b 100644 --- a/test/forkMainnet/LimitOrderSwap/CoordinatedTaker.t.sol +++ b/test/forkMainnet/LimitOrderSwap/CoordinatedTaker.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { ILimitOrderSwap } from "contracts/interfaces/ILimitOrderSwap.sol"; import { ICoordinatedTaker } from "contracts/interfaces/ICoordinatedTaker.sol"; diff --git a/test/forkMainnet/LimitOrderSwap/Fill.t.sol b/test/forkMainnet/LimitOrderSwap/Fill.t.sol index 88f62088..b7585902 100644 --- a/test/forkMainnet/LimitOrderSwap/Fill.t.sol +++ b/test/forkMainnet/LimitOrderSwap/Fill.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; diff --git a/test/forkMainnet/LimitOrderSwap/FullOrKill.t.sol b/test/forkMainnet/LimitOrderSwap/FullOrKill.t.sol index 16dc2e59..4c48e10b 100644 --- a/test/forkMainnet/LimitOrderSwap/FullOrKill.t.sol +++ b/test/forkMainnet/LimitOrderSwap/FullOrKill.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { ILimitOrderSwap } from "contracts/interfaces/ILimitOrderSwap.sol"; import { Constant } from "contracts/libraries/Constant.sol"; diff --git a/test/forkMainnet/LimitOrderSwap/GroupFill.t.sol b/test/forkMainnet/LimitOrderSwap/GroupFill.t.sol index da323e6e..81d86c46 100644 --- a/test/forkMainnet/LimitOrderSwap/GroupFill.t.sol +++ b/test/forkMainnet/LimitOrderSwap/GroupFill.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { ILimitOrderSwap } from "contracts/interfaces/ILimitOrderSwap.sol"; import { IUniswapPermit2 } from "contracts/interfaces/IUniswapPermit2.sol"; diff --git a/test/forkMainnet/LimitOrderSwap/Management.t.sol b/test/forkMainnet/LimitOrderSwap/Management.t.sol index 6deeacad..639f30b8 100644 --- a/test/forkMainnet/LimitOrderSwap/Management.t.sol +++ b/test/forkMainnet/LimitOrderSwap/Management.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { ILimitOrderSwap } from "contracts/interfaces/ILimitOrderSwap.sol"; import { Ownable } from "contracts/abstracts/Ownable.sol"; diff --git a/test/forkMainnet/LimitOrderSwap/Setup.t.sol b/test/forkMainnet/LimitOrderSwap/Setup.t.sol index 116f2a1e..ac995511 100644 --- a/test/forkMainnet/LimitOrderSwap/Setup.t.sol +++ b/test/forkMainnet/LimitOrderSwap/Setup.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { Test } from "forge-std/Test.sol"; import { Tokens } from "test/utils/Tokens.sol"; diff --git a/test/forkMainnet/RFQ.t.sol b/test/forkMainnet/RFQ.t.sol index ad5ad714..65e0198a 100644 --- a/test/forkMainnet/RFQ.t.sol +++ b/test/forkMainnet/RFQ.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { Test } from "forge-std/Test.sol"; import { Tokens } from "test/utils/Tokens.sol"; diff --git a/test/forkMainnet/SmartOrderStrategy/AMMs.t.sol b/test/forkMainnet/SmartOrderStrategy/AMMs.t.sol index 3358d6f5..d0952bfd 100644 --- a/test/forkMainnet/SmartOrderStrategy/AMMs.t.sol +++ b/test/forkMainnet/SmartOrderStrategy/AMMs.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; diff --git a/test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol b/test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol index 907f060b..6787006c 100644 --- a/test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol +++ b/test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; diff --git a/test/forkMainnet/SmartOrderStrategy/Setup.t.sol b/test/forkMainnet/SmartOrderStrategy/Setup.t.sol index 16d60bef..17e069f1 100644 --- a/test/forkMainnet/SmartOrderStrategy/Setup.t.sol +++ b/test/forkMainnet/SmartOrderStrategy/Setup.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { Test } from "forge-std/Test.sol"; import { SmartOrderStrategy } from "contracts/SmartOrderStrategy.sol"; diff --git a/test/forkMainnet/SmartOrderStrategy/Validation.t.sol b/test/forkMainnet/SmartOrderStrategy/Validation.t.sol index e65837d5..59c91d92 100644 --- a/test/forkMainnet/SmartOrderStrategy/Validation.t.sol +++ b/test/forkMainnet/SmartOrderStrategy/Validation.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { SmartOrderStrategyTest } from "./Setup.t.sol"; import { ISmartOrderStrategy } from "contracts/interfaces/ISmartOrderStrategy.sol"; diff --git a/test/forkMainnet/TokenCollector.t.sol b/test/forkMainnet/TokenCollector.t.sol index 2f18a941..d2ae823b 100644 --- a/test/forkMainnet/TokenCollector.t.sol +++ b/test/forkMainnet/TokenCollector.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { AllowanceTarget } from "contracts/AllowanceTarget.sol"; import { TokenCollector } from "contracts/abstracts/TokenCollector.sol"; diff --git a/test/libraries/SignatureValidator.t.sol b/test/libraries/SignatureValidator.t.sol index e027f6bc..c32cd2ae 100644 --- a/test/libraries/SignatureValidator.t.sol +++ b/test/libraries/SignatureValidator.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { Test } from "forge-std/Test.sol"; import { IERC1271Wallet } from "contracts/interfaces/IERC1271Wallet.sol"; diff --git a/test/utils/ICurveFi.sol b/test/utils/ICurveFi.sol index b62ab1ce..e58397fb 100644 --- a/test/utils/ICurveFi.sol +++ b/test/utils/ICurveFi.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; interface ICurveFi { function get_virtual_price() external returns (uint256 out); diff --git a/test/utils/ICurveFiV2.sol b/test/utils/ICurveFiV2.sol index f6ed63f6..3cad8012 100644 --- a/test/utils/ICurveFiV2.sol +++ b/test/utils/ICurveFiV2.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; interface ICurveFiV2 { function get_dy(uint256 i, uint256 j, uint256 dx) external view returns (uint256 out); diff --git a/test/utils/IUniswapSwapRouter02.sol b/test/utils/IUniswapSwapRouter02.sol index 336ace70..3b098401 100644 --- a/test/utils/IUniswapSwapRouter02.sol +++ b/test/utils/IUniswapSwapRouter02.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; interface IUniswapSwapRouter02 { function swapExactTokensForTokens(uint256 amountIn, uint256 amountOutMin, address[] calldata path, address to) external payable returns (uint256 amountOut); diff --git a/test/utils/IUniswapUniversalRouter.sol b/test/utils/IUniswapUniversalRouter.sol index 2157b884..a1b637c8 100644 --- a/test/utils/IUniswapUniversalRouter.sol +++ b/test/utils/IUniswapUniversalRouter.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; interface IUniversalRouter { error TransactionDeadlinePassed(); diff --git a/test/utils/IUniswapV2Router.sol b/test/utils/IUniswapV2Router.sol index fffc34cd..669dc272 100644 --- a/test/utils/IUniswapV2Router.sol +++ b/test/utils/IUniswapV2Router.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; interface IUniswapV2Router { function swapExactTokensForTokens( diff --git a/test/utils/IUniswapV3Quoter.sol b/test/utils/IUniswapV3Quoter.sol index c94c8498..f3eb5188 100644 --- a/test/utils/IUniswapV3Quoter.sol +++ b/test/utils/IUniswapV3Quoter.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; /// @title Quoter Interface /// @notice Supports quoting the calculated amounts from exact input or exact output swaps diff --git a/test/utils/IUniswapV3SwapRouter.sol b/test/utils/IUniswapV3SwapRouter.sol index d0ebdaa1..885ab082 100644 --- a/test/utils/IUniswapV3SwapRouter.sol +++ b/test/utils/IUniswapV3SwapRouter.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity >=0.8.0; +pragma solidity ^0.8.0; /// @title Router token swapping functionality /// @notice Functions for swapping tokens via Uniswap V3 diff --git a/test/utils/Permit2Helper.sol b/test/utils/Permit2Helper.sol index 8614ecd5..92c7e946 100644 --- a/test/utils/Permit2Helper.sol +++ b/test/utils/Permit2Helper.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { IUniswapPermit2 } from "contracts/interfaces/IUniswapPermit2.sol"; import { TokenCollector } from "contracts/abstracts/TokenCollector.sol"; diff --git a/test/utils/Sig.sol b/test/utils/Sig.sol index c1352770..d9770bb7 100644 --- a/test/utils/Sig.sol +++ b/test/utils/Sig.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; function getEIP712Hash(bytes32 domainSeparator, bytes32 structHash) pure returns (bytes32) { string memory EIP191_HEADER = "\x19\x01"; diff --git a/test/utils/SigHelper.sol b/test/utils/SigHelper.sol index d606f6e5..701fa762 100644 --- a/test/utils/SigHelper.sol +++ b/test/utils/SigHelper.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity 0.8.26; import { AllowFill, getAllowFillHash } from "contracts/libraries/AllowFill.sol"; import { GenericSwapData, getGSDataHash } from "contracts/libraries/GenericSwapData.sol"; diff --git a/test/utils/UniswapCommands.sol b/test/utils/UniswapCommands.sol index 3fec8768..a66a6754 100644 --- a/test/utils/UniswapCommands.sol +++ b/test/utils/UniswapCommands.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.17; +pragma solidity ^0.8.0; library UniswapCommands { bytes1 internal constant FLAG_ALLOW_REVERT = 0x80; diff --git a/test/utils/UniswapV2Library.sol b/test/utils/UniswapV2Library.sol index e546616c..b5847309 100644 --- a/test/utils/UniswapV2Library.sol +++ b/test/utils/UniswapV2Library.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity 0.8.17; +pragma solidity ^0.8.0; interface IUniswapV2Pair { event Approval(address indexed owner, address indexed spender, uint256 value); From 6b0dd6dfecdd304b32c539f6ec96c01b95df48e5 Mon Sep 17 00:00:00 2001 From: alex0207s Date: Fri, 31 May 2024 17:31:05 +0800 Subject: [PATCH 08/11] chore: upgrade openzeppelin lib to v5.0.2 --- .gitmodules | 4 +-- contracts/AllowanceTarget.sol | 2 +- contracts/LimitOrderSwap.sol | 2 +- contracts/abstracts/AdminManagement.sol | 2 +- contracts/libraries/SignatureValidator.sol | 2 +- lib/openzeppelin-contracts | 2 +- test/AllowanceTarget.t.sol | 5 ++-- test/forkMainnet/LimitOrderSwap/Fill.t.sol | 5 ++-- test/forkMainnet/TokenCollector.t.sol | 31 +++++++++++++--------- test/libraries/SignatureValidator.t.sol | 6 ++--- test/mocks/MockERC1271Wallet.sol | 4 +-- test/utils/BalanceUtil.sol | 2 +- 12 files changed, 38 insertions(+), 29 deletions(-) diff --git a/.gitmodules b/.gitmodules index 55a27aab..9c687ea0 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,7 +1,7 @@ [submodule "lib/openzeppelin-contracts"] path = lib/openzeppelin-contracts - url = https://github.com/openzeppelin/openzeppelin-contracts - branch = v4.9.2 + url = https://github.com/OpenZeppelin/openzeppelin-contracts + branch = v5.0.2 [submodule "lib/forge-std"] path = lib/forge-std url = https://github.com/foundry-rs/forge-std diff --git a/contracts/AllowanceTarget.sol b/contracts/AllowanceTarget.sol index 2879c9ae..ca1db2c2 100644 --- a/contracts/AllowanceTarget.sol +++ b/contracts/AllowanceTarget.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.26; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -import { Pausable } from "@openzeppelin/contracts/security/Pausable.sol"; +import { Pausable } from "@openzeppelin/contracts/utils/Pausable.sol"; import { Ownable } from "./abstracts/Ownable.sol"; import { IAllowanceTarget } from "./interfaces/IAllowanceTarget.sol"; diff --git a/contracts/LimitOrderSwap.sol b/contracts/LimitOrderSwap.sol index ed0fa948..3b956a71 100644 --- a/contracts/LimitOrderSwap.sol +++ b/contracts/LimitOrderSwap.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; -import { ReentrancyGuard } from "@openzeppelin/contracts/security/ReentrancyGuard.sol"; +import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; import { TokenCollector } from "./abstracts/TokenCollector.sol"; import { Ownable } from "./abstracts/Ownable.sol"; diff --git a/contracts/abstracts/AdminManagement.sol b/contracts/abstracts/AdminManagement.sol index 1112b42a..6e605b49 100644 --- a/contracts/abstracts/AdminManagement.sol +++ b/contracts/abstracts/AdminManagement.sol @@ -17,7 +17,7 @@ abstract contract AdminManagement is Ownable { 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) { - IERC20(tokens[i]).safeApprove(spenders[j], type(uint256).max); + IERC20(tokens[i]).forceApprove(spenders[j], type(uint256).max); } } } diff --git a/contracts/libraries/SignatureValidator.sol b/contracts/libraries/SignatureValidator.sol index 4afb5f47..f03972c6 100644 --- a/contracts/libraries/SignatureValidator.sol +++ b/contracts/libraries/SignatureValidator.sol @@ -20,7 +20,7 @@ library SignatureValidator { * @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.isContract()) { + if (_signerAddress.code.length > 0) { return ERC1271_MAGICVALUE == IERC1271Wallet(_signerAddress).isValidSignature(_hash, _signature); } else { return _signerAddress == ECDSA.recover(_hash, _signature); diff --git a/lib/openzeppelin-contracts b/lib/openzeppelin-contracts index e50c24f5..dbb6104c 160000 --- a/lib/openzeppelin-contracts +++ b/lib/openzeppelin-contracts @@ -1 +1 @@ -Subproject commit e50c24f5839db17f46991478384bfda14acfb830 +Subproject commit dbb6104ce834628e473d2173bbc9d47f81a9eec3 diff --git a/test/AllowanceTarget.t.sol b/test/AllowanceTarget.t.sol index 48840a27..2df7f023 100644 --- a/test/AllowanceTarget.t.sol +++ b/test/AllowanceTarget.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.26; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import { Pausable } from "@openzeppelin/contracts/utils/Pausable.sol"; import { IAllowanceTarget } from "contracts/interfaces/IAllowanceTarget.sol"; import { AllowanceTarget } from "contracts/AllowanceTarget.sol"; @@ -64,7 +65,7 @@ contract AllowanceTargetTest is BalanceUtil { function testCannotSpendFromUserInsufficientBalanceWithReturnFalseToken() public { uint256 userBalance = noRevertERC20.balanceOf(user); - vm.expectRevert("SafeERC20: ERC20 operation did not succeed"); + vm.expectRevert(abi.encodeWithSelector(SafeERC20.SafeERC20FailedOperation.selector, address(noRevertERC20))); vm.prank(authorized); allowanceTarget.spendFromUserTo(user, address(noRevertERC20), recipient, userBalance + 1); } @@ -86,7 +87,7 @@ contract AllowanceTargetTest is BalanceUtil { vm.prank(allowanceTargetOwner, allowanceTargetOwner); allowanceTarget.pause(); - vm.expectRevert("Pausable: paused"); + vm.expectRevert(Pausable.EnforcedPause.selector); allowanceTarget.spendFromUserTo(user, address(mockERC20), recipient, 1234); } diff --git a/test/forkMainnet/LimitOrderSwap/Fill.t.sol b/test/forkMainnet/LimitOrderSwap/Fill.t.sol index b7585902..fb1fc69b 100644 --- a/test/forkMainnet/LimitOrderSwap/Fill.t.sol +++ b/test/forkMainnet/LimitOrderSwap/Fill.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.26; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import { Address } from "@openzeppelin/contracts/utils/Address.sol"; import { ILimitOrderSwap } from "contracts/interfaces/ILimitOrderSwap.sol"; import { Constant } from "contracts/libraries/Constant.sol"; @@ -473,10 +474,10 @@ contract FillTest is LimitOrderSwapTest { mockStrategy.setOutputAmountAndRecipient(defaultOrder.takerTokenAmount - 1, payable(randomTaker)); vm.startPrank(randomTaker); - IERC20(defaultOrder.takerToken).safeApprove(address(limitOrderSwap), type(uint256).max); + IERC20(defaultOrder.takerToken).forceApprove(address(limitOrderSwap), type(uint256).max); // the final step transferFrom will fail since taker doesn't have enough balance to fill - vm.expectRevert("SafeERC20: low-level call failed"); + vm.expectRevert(Address.FailedInnerCall.selector); limitOrderSwap.fillLimitOrder({ order: defaultOrder, makerSignature: defaultMakerSig, diff --git a/test/forkMainnet/TokenCollector.t.sol b/test/forkMainnet/TokenCollector.t.sol index d2ae823b..5c1684bf 100644 --- a/test/forkMainnet/TokenCollector.t.sol +++ b/test/forkMainnet/TokenCollector.t.sol @@ -1,6 +1,9 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; +import { ERC20Permit } from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol"; +import { IERC20Errors } from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol"; + import { AllowanceTarget } from "contracts/AllowanceTarget.sol"; import { TokenCollector } from "contracts/abstracts/TokenCollector.sol"; import { IUniswapPermit2 } from "contracts/interfaces/IUniswapPermit2.sol"; @@ -19,6 +22,7 @@ contract Strategy is TokenCollector { contract TestTokenCollector is Addresses, Permit2Helper { uint256 otherPrivateKey = uint256(123); uint256 userPrivateKey = uint256(1); + address other = vm.addr(otherPrivateKey); address user = vm.addr(userPrivateKey); address allowanceTargetOwner = makeAddr("allowanceTargetOwner"); @@ -34,7 +38,7 @@ contract TestTokenCollector is Addresses, Permit2Helper { Strategy strategy = new Strategy(address(permit2), address(allowanceTarget)); function setUp() public { - token.mint(user, 10000 * 1e18); + token.mint(user, 10000 ether); // get permit2 nonce and compose PermitSingle for AllowanceTransfer uint256 expiration = block.timestamp + 1 days; @@ -54,12 +58,12 @@ contract TestTokenCollector is Addresses, Permit2Helper { function testCannotCollectByTokenApprovalWhenAllowanceIsNotEnough() public { bytes memory data = abi.encodePacked(TokenCollector.Source.Token); - vm.expectRevert("ERC20: insufficient allowance"); + vm.expectRevert(abi.encodeWithSelector(IERC20Errors.ERC20InsufficientAllowance.selector, address(strategy), 0, 1)); strategy.collect(address(token), user, address(this), 1, data); } function testCollectByTokenApproval() public { - uint256 amount = 100 * 1e18; + uint256 amount = 100 ether; vm.prank(user); token.approve(address(strategy), amount); @@ -74,12 +78,14 @@ contract TestTokenCollector is Addresses, Permit2Helper { function testCannotCollectByAllowanceTargetIfNoPriorApprove() public { bytes memory data = abi.encodePacked(TokenCollector.Source.TokenlonAllowanceTarget); - vm.expectRevert("ERC20: insufficient allowance"); + vm.expectRevert(abi.encodeWithSelector(IERC20Errors.ERC20InsufficientAllowance.selector, address(allowanceTarget), 0, 1)); + vm.startPrank(user); strategy.collect(address(token), user, address(this), 1, data); + vm.stopPrank(); } function testCollectByAllowanceTarget() public { - uint256 amount = 100 * 1e18; + uint256 amount = 100 ether; vm.prank(user); token.approve(address(allowanceTarget), amount); @@ -98,7 +104,7 @@ contract TestTokenCollector is Addresses, Permit2Helper { token: address(token), owner: user, spender: address(strategy), - amount: 100 * 1e18, + amount: 100 ether, nonce: token.nonces(user), deadline: block.timestamp + 1 days }); @@ -132,7 +138,7 @@ contract TestTokenCollector is Addresses, Permit2Helper { (uint8 v, bytes32 r, bytes32 s) = vm.sign(otherPrivateKey, permitHash); bytes memory data = encodeTokenPermitData(permit, v, r, s); - vm.expectRevert("ERC20Permit: invalid signature"); + vm.expectRevert(abi.encodeWithSelector(ERC20Permit.ERC2612InvalidSigner.selector, other, permit.owner)); strategy.collect(address(token), permit.owner, address(this), permit.amount, data); } @@ -145,7 +151,7 @@ contract TestTokenCollector is Addresses, Permit2Helper { (uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, permitHash); bytes memory data = encodeTokenPermitData(permit, v, r, s); - vm.expectRevert("ERC20: insufficient allowance"); + vm.expectRevert(abi.encodeWithSelector(IERC20Errors.ERC20InsufficientAllowance.selector, address(strategy), 0, permit.amount)); strategy.collect(address(token), permit.owner, address(this), permit.amount, data); } @@ -158,7 +164,7 @@ contract TestTokenCollector is Addresses, Permit2Helper { (uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, permitHash); bytes memory data = encodeTokenPermitData(permit, v, r, s); - vm.expectRevert("ERC20: insufficient allowance"); + vm.expectRevert(abi.encodeWithSelector(IERC20Errors.ERC20InsufficientAllowance.selector, address(strategy), permit.amount, invalidAmount)); strategy.collect(address(token), permit.owner, address(this), invalidAmount, data); } @@ -169,9 +175,10 @@ contract TestTokenCollector is Addresses, Permit2Helper { bytes32 permitHash = getTokenPermitHash(permit); (uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, permitHash); + address recoveredAddress = 0x5d6b650146e111D930C9F97570876A12F568D2B5; bytes memory data = encodeTokenPermitData(permit, v, r, s); - vm.expectRevert("ERC20Permit: invalid signature"); + vm.expectRevert(abi.encodeWithSelector(ERC20Permit.ERC2612InvalidSigner.selector, recoveredAddress, permit.owner)); strategy.collect(address(token), permit.owner, address(this), permit.amount, data); } @@ -184,7 +191,7 @@ contract TestTokenCollector is Addresses, Permit2Helper { (uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, permitHash); bytes memory data = encodeTokenPermitData(permit, v, r, s); - vm.expectRevert("ERC20Permit: expired deadline"); + vm.expectRevert(abi.encodeWithSelector(ERC20Permit.ERC2612ExpiredSignature.selector, permit.deadline)); strategy.collect(address(token), permit.owner, address(this), permit.amount, data); } @@ -289,7 +296,7 @@ contract TestTokenCollector is Addresses, Permit2Helper { IUniswapPermit2.PermitTransferFrom DEFAULT_PERMIT_TRANSFER = IUniswapPermit2.PermitTransferFrom({ - permitted: IUniswapPermit2.TokenPermissions({ token: address(token), amount: 100 * 1e18 }), + permitted: IUniswapPermit2.TokenPermissions({ token: address(token), amount: 100 ether }), nonce: 0, deadline: block.timestamp + 1 days }); diff --git a/test/libraries/SignatureValidator.t.sol b/test/libraries/SignatureValidator.t.sol index c32cd2ae..38ace49e 100644 --- a/test/libraries/SignatureValidator.t.sol +++ b/test/libraries/SignatureValidator.t.sol @@ -50,14 +50,14 @@ contract SignatureValidatorTest is Test { // should have 96 bytes signature bytes memory signature = abi.encodePacked(r, s, v); // will be reverted in OZ ECDSA lib - vm.expectRevert("ECDSA: invalid signature length"); + vm.expectRevert(abi.encodeWithSelector(ECDSA.ECDSAInvalidSignatureLength.selector, signature.length)); SignatureValidator.validateSignature(vm.addr(userPrivateKey), digest, signature); } function testEIP712WithEmptySignature() public { bytes memory signature; // will be reverted in OZ ECDSA lib - vm.expectRevert("ECDSA: invalid signature length"); + vm.expectRevert(abi.encodeWithSelector(ECDSA.ECDSAInvalidSignatureLength.selector, signature.length)); SignatureValidator.validateSignature(vm.addr(userPrivateKey), digest, signature); } @@ -79,7 +79,7 @@ contract SignatureValidatorTest is Test { bytes memory signature = abi.encodePacked(r, s, uint8(10)); // OZ ECDSA lib will handle the zero address case and throw error instead // so the zero address will never be matched - vm.expectRevert("ECDSA: invalid signature"); + vm.expectRevert(ECDSA.ECDSAInvalidSignature.selector); this._isValidSignatureWrap(address(0), digest, signature); } diff --git a/test/mocks/MockERC1271Wallet.sol b/test/mocks/MockERC1271Wallet.sol index a7f30de4..31acff9e 100644 --- a/test/mocks/MockERC1271Wallet.sol +++ b/test/mocks/MockERC1271Wallet.sol @@ -29,13 +29,13 @@ contract MockERC1271Wallet is IERC1271Wallet { function setAllowance(address[] memory _tokenList, address _spender) external onlyOperator { for (uint256 i = 0; i < _tokenList.length; i++) { - IERC20(_tokenList[i]).safeApprove(_spender, MAX_UINT); + IERC20(_tokenList[i]).forceApprove(_spender, MAX_UINT); } } function closeAllowance(address[] memory _tokenList, address _spender) external onlyOperator { for (uint256 i = 0; i < _tokenList.length; i++) { - IERC20(_tokenList[i]).safeApprove(_spender, 0); + IERC20(_tokenList[i]).forceApprove(_spender, 0); } } diff --git a/test/utils/BalanceUtil.sol b/test/utils/BalanceUtil.sol index cec5db91..4ffc7d2c 100644 --- a/test/utils/BalanceUtil.sol +++ b/test/utils/BalanceUtil.sol @@ -34,7 +34,7 @@ contract BalanceUtil is Test { function approveERC20(address token, address user, address spender) internal { vm.startPrank(user); - IERC20(token).safeApprove(spender, type(uint256).max); + IERC20(token).forceApprove(spender, type(uint256).max); vm.stopPrank(); } } From 019650b2eaf9c86d0e3fce3c04acfe001f5f4cd9 Mon Sep 17 00:00:00 2001 From: alex0207s Date: Thu, 6 Jun 2024 11:19:40 +0800 Subject: [PATCH 09/11] chore: remove redundant logic and modifier --- contracts/AllowanceTarget.sol | 2 +- contracts/CoordinatedTaker.sol | 2 +- contracts/GenericSwap.sol | 4 ++-- contracts/LimitOrderSwap.sol | 14 ++++++-------- contracts/RFQ.sol | 11 +++-------- contracts/SmartOrderStrategy.sol | 2 +- contracts/abstracts/AdminManagement.sol | 4 +--- contracts/libraries/Asset.sol | 5 ++--- 8 files changed, 17 insertions(+), 27 deletions(-) diff --git a/contracts/AllowanceTarget.sol b/contracts/AllowanceTarget.sol index ca1db2c2..ce7d43c4 100644 --- a/contracts/AllowanceTarget.sol +++ b/contracts/AllowanceTarget.sol @@ -29,7 +29,7 @@ contract AllowanceTarget is IAllowanceTarget, Pausable, Ownable { } /// @inheritdoc IAllowanceTarget - function spendFromUserTo(address from, address token, address to, uint256 amount) external override whenNotPaused { + function spendFromUserTo(address from, address token, address to, uint256 amount) external whenNotPaused { if (!authorized[msg.sender]) revert NotAuthorized(); IERC20(token).safeTransferFrom(from, to, amount); } diff --git a/contracts/CoordinatedTaker.sol b/contracts/CoordinatedTaker.sol index ac335000..efaa1d8a 100644 --- a/contracts/CoordinatedTaker.sol +++ b/contracts/CoordinatedTaker.sol @@ -53,7 +53,7 @@ contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector, bytes calldata extraAction, bytes calldata userTokenPermit, CoordinatorParams calldata crdParams - ) external payable override { + ) external payable { // validate fill permission { if (crdParams.expiry < block.timestamp) revert ExpiredPermission(); diff --git a/contracts/GenericSwap.sol b/contracts/GenericSwap.sol index 1e7ba299..48dfdf23 100644 --- a/contracts/GenericSwap.sol +++ b/contracts/GenericSwap.sol @@ -20,7 +20,7 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 { /// @param swapData Swap data /// @return returnAmount Output amount of the swap - function executeSwap(GenericSwapData calldata swapData, bytes calldata takerTokenPermit) external payable override returns (uint256 returnAmount) { + 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); @@ -35,7 +35,7 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 { bytes calldata takerTokenPermit, address taker, bytes calldata takerSig - ) external payable override returns (uint256 returnAmount) { + ) external payable returns (uint256 returnAmount) { bytes32 swapHash = getGSDataHash(swapData); bytes32 gs712Hash = getEIP712Hash(swapHash); if (filledSwap[swapHash]) revert AlreadyFilled(); diff --git a/contracts/LimitOrderSwap.sol b/contracts/LimitOrderSwap.sol index 3b956a71..3d52f072 100644 --- a/contracts/LimitOrderSwap.sol +++ b/contracts/LimitOrderSwap.sol @@ -51,7 +51,7 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree } /// @inheritdoc ILimitOrderSwap - function fillLimitOrder(LimitOrder calldata order, bytes calldata makerSignature, TakerParams calldata takerParams) external payable override nonReentrant { + function fillLimitOrder(LimitOrder calldata order, bytes calldata makerSignature, TakerParams calldata takerParams) external payable nonReentrant { _fillLimitOrder(order, makerSignature, takerParams, false); } @@ -60,7 +60,7 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree LimitOrder calldata order, bytes calldata makerSignature, TakerParams calldata takerParams - ) external payable override nonReentrant { + ) external payable nonReentrant { _fillLimitOrder(order, makerSignature, takerParams, true); } @@ -70,7 +70,7 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree bytes[] calldata makerSignatures, uint256[] calldata makerTokenAmounts, address[] calldata profitTokens - ) external payable override nonReentrant { + ) external payable nonReentrant { if (orders.length != makerSignatures.length || orders.length != makerTokenAmounts.length) revert InvalidParams(); // validate orders and calculate takingAmounts @@ -130,7 +130,7 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree } /// @inheritdoc ILimitOrderSwap - function cancelOrder(LimitOrder calldata order) external override nonReentrant { + function cancelOrder(LimitOrder calldata order) external nonReentrant { if (order.expiry < uint64(block.timestamp)) revert ExpiredOrder(); if (msg.sender != order.maker) revert NotOrderMaker(); bytes32 orderHash = getLimitOrderHash(order); @@ -143,7 +143,7 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree emit OrderCanceled(orderHash, order.maker); } - function isOrderCanceled(bytes32 orderHash) external view override returns (bool) { + function isOrderCanceled(bytes32 orderHash) external view returns (bool) { uint256 orderFilledAmount = orderHashToMakerTokenFilledAmount[orderHash]; return (orderFilledAmount & ORDER_CANCEL_AMOUNT_MASK) != 0; } @@ -175,9 +175,7 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree if (msg.value != takerParams.takerTokenAmount) revert InvalidMsgValue(); Asset.transferTo(Constant.ETH_ADDRESS, order.maker, takerSpendingAmount); uint256 ethRefund = takerParams.takerTokenAmount - takerSpendingAmount; - if (ethRefund > 0) { - Asset.transferTo(Constant.ETH_ADDRESS, payable(msg.sender), ethRefund); - } + Asset.transferTo(Constant.ETH_ADDRESS, payable(msg.sender), ethRefund); } else { if (msg.value != 0) revert InvalidMsgValue(); _collect(order.takerToken, msg.sender, order.maker, takerSpendingAmount, takerParams.takerTokenPermit); diff --git a/contracts/RFQ.sol b/contracts/RFQ.sol index 48c7f7ca..a795d492 100644 --- a/contracts/RFQ.sol +++ b/contracts/RFQ.sol @@ -54,12 +54,7 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { emit SetFeeCollector(_newFeeCollector); } - function fillRFQ( - RFQTx calldata rfqTx, - bytes calldata makerSignature, - bytes calldata makerTokenPermit, - bytes calldata takerTokenPermit - ) external payable override { + function fillRFQ(RFQTx calldata rfqTx, bytes calldata makerSignature, bytes calldata makerTokenPermit, bytes calldata takerTokenPermit) external payable { _fillRFQ(rfqTx, makerSignature, makerTokenPermit, takerTokenPermit, bytes("")); } @@ -69,11 +64,11 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { bytes calldata makerTokenPermit, bytes calldata takerTokenPermit, bytes calldata takerSignature - ) external override { + ) external { _fillRFQ(rfqTx, makerSignature, makerTokenPermit, takerTokenPermit, takerSignature); } - function cancelRFQOffer(RFQOffer calldata rfqOffer) external override { + function cancelRFQOffer(RFQOffer calldata rfqOffer) external { if (msg.sender != rfqOffer.maker) revert NotOfferMaker(); bytes32 rfqOfferHash = getRFQOfferHash(rfqOffer); if (filledOffer[rfqOfferHash]) revert FilledRFQOffer(); diff --git a/contracts/SmartOrderStrategy.sol b/contracts/SmartOrderStrategy.sol index c8df5563..15efd2e3 100644 --- a/contracts/SmartOrderStrategy.sol +++ b/contracts/SmartOrderStrategy.sol @@ -26,7 +26,7 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement { } /// @inheritdoc IStrategy - function executeStrategy(address inputToken, address outputToken, uint256 inputAmount, bytes calldata data) external payable override onlyGenericSwap { + function executeStrategy(address inputToken, address outputToken, uint256 inputAmount, bytes calldata data) external payable onlyGenericSwap { if (inputAmount == 0) revert ZeroInput(); Operation[] memory ops = abi.decode(data, (Operation[])); diff --git a/contracts/abstracts/AdminManagement.sol b/contracts/abstracts/AdminManagement.sol index 6e605b49..3491d2ae 100644 --- a/contracts/abstracts/AdminManagement.sol +++ b/contracts/abstracts/AdminManagement.sol @@ -25,9 +25,7 @@ abstract contract AdminManagement is Ownable { 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)); - if (selfBalance > 0) { - Asset.transferTo(tokens[i], payable(recipient), selfBalance); - } + Asset.transferTo(tokens[i], payable(recipient), selfBalance); } } } diff --git a/contracts/libraries/Asset.sol b/contracts/libraries/Asset.sol index 2331fb49..bb0a3e9d 100644 --- a/contracts/libraries/Asset.sol +++ b/contracts/libraries/Asset.sol @@ -24,9 +24,8 @@ library Asset { } function transferTo(address asset, address payable to, uint256 amount) internal { - if (to == address(this) || amount == 0) { - return; - } + if (to == address(this) || amount == 0) return; + if (isETH(asset)) { // @dev forward all available gas and may cause reentrancy if (address(this).balance < amount) revert InsufficientBalance(); From b90e10cb830f327a73e824a54663cba1c003e9a3 Mon Sep 17 00:00:00 2001 From: alex0207s Date: Tue, 2 Jul 2024 15:31:16 +0800 Subject: [PATCH 10/11] chore: increase test coverage to near 100% --- contracts/LimitOrderSwap.sol | 38 +++-- contracts/RFQ.sol | 5 + contracts/SmartOrderStrategy.sol | 8 + contracts/abstracts/TokenCollector.sol | 3 - contracts/interfaces/ILimitOrderSwap.sol | 5 +- package.json | 1 + test/AllowanceTarget.t.sol | 18 ++ test/abstracts/EIP712.t.sol | 52 ++++++ test/abstracts/Ownable.t.sol | 95 +++++++++++ test/forkMainnet/GenericSwap.t.sol | 7 + .../LimitOrderSwap/CoordinatedTaker.t.sol | 34 +++- test/forkMainnet/LimitOrderSwap/Setup.t.sol | 10 ++ .../LimitOrderSwap/Validation.t.sol | 157 ++++++++++++++++++ test/forkMainnet/RFQ.t.sol | 67 +++++++- .../SmartOrderStrategy/IntegrationV6.t.sol | 80 +++++++++ .../SmartOrderStrategy/Validation.t.sol | 19 +++ test/forkMainnet/TokenCollector.t.sol | 15 ++ test/libraries/Asset.t.sol | 75 +++++++++ test/libraries/SignatureValidator.t.sol | 25 ++- 19 files changed, 681 insertions(+), 33 deletions(-) create mode 100644 test/abstracts/EIP712.t.sol create mode 100644 test/abstracts/Ownable.t.sol create mode 100644 test/forkMainnet/LimitOrderSwap/Validation.t.sol create mode 100644 test/libraries/Asset.t.sol diff --git a/contracts/LimitOrderSwap.sol b/contracts/LimitOrderSwap.sol index 3d52f072..55a61668 100644 --- a/contracts/LimitOrderSwap.sol +++ b/contracts/LimitOrderSwap.sol @@ -80,17 +80,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(); - } - + // the 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]; } @@ -112,6 +116,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); } @@ -167,6 +172,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); } @@ -195,8 +202,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,8 +221,11 @@ 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); @@ -215,10 +233,6 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree 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; } @@ -229,6 +243,8 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree 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); diff --git a/contracts/RFQ.sol b/contracts/RFQ.sol index a795d492..c0217ebe 100644 --- a/contracts/RFQ.sol +++ b/contracts/RFQ.sol @@ -153,6 +153,9 @@ 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 @@ -184,6 +187,8 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { 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); } } diff --git a/contracts/SmartOrderStrategy.sol b/contracts/SmartOrderStrategy.sol index 15efd2e3..5e7bd284 100644 --- a/contracts/SmartOrderStrategy.sol +++ b/contracts/SmartOrderStrategy.sol @@ -35,6 +35,8 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement { // wrap eth first 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(); @@ -48,9 +50,15 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement { // transfer output token back to GenericSwap // ETH first so WETH is not considered as an option of outputToken + + // after replacing `makerToken.isETH()` with `makerToken == 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE` + // and crafting some cases where outputToken is ETH and non-ETH + // the if statement is still not fully covered by the test if (Asset.isETH(outputToken)) { // unwrap existing WETH if any uint256 wethBalance = IWETH(weth).balanceOf(address(this)); + // after trying to craft a test case where wethBalance == 0 + // the if statement is still not fully covered by the test if (wethBalance > 0) { IWETH(weth).withdraw(wethBalance); } diff --git a/contracts/abstracts/TokenCollector.sol b/contracts/abstracts/TokenCollector.sol index bdb345d5..da2057fb 100644 --- a/contracts/abstracts/TokenCollector.sol +++ b/contracts/abstracts/TokenCollector.sol @@ -67,8 +67,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/ILimitOrderSwap.sol b/contracts/interfaces/ILimitOrderSwap.sol index fe5e156e..34af0428 100644 --- a/contracts/interfaces/ILimitOrderSwap.sol +++ b/contracts/interfaces/ILimitOrderSwap.sol @@ -10,7 +10,10 @@ interface ILimitOrderSwap { error CanceledOrder(); error FilledOrder(); error ZeroAddress(); - error ZeroTokenAmount(); + error ZeroTakerTokenAmount(); + error ZeroMakerTokenAmount(); + error ZeroTakerSpendingAmount(); + error ZeroMakerSpendingAmount(); error NotEnoughForFill(); error InvalidMsgValue(); error InvalidSignature(); 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 { From 3c55d03e596df046aeeb58c3551385105f5eb459 Mon Sep 17 00:00:00 2001 From: alex0207s Date: Thu, 11 Jul 2024 00:42:31 +0800 Subject: [PATCH 11/11] include NatSpec comments in contracts, libraries and interfaces --- contracts/AllowanceTarget.sol | 13 ++- contracts/CoordinatedTaker.sol | 24 ++++- contracts/GenericSwap.sol | 29 ++++-- contracts/LimitOrderSwap.sol | 64 ++++++++++--- contracts/RFQ.sol | 73 +++++++++++---- contracts/SmartOrderStrategy.sol | 39 +++++--- contracts/abstracts/AdminManagement.sol | 11 +++ contracts/abstracts/EIP712.sol | 14 +++ contracts/abstracts/Ownable.sol | 36 +++++-- contracts/abstracts/TokenCollector.sol | 19 ++++ contracts/interfaces/IAllowanceTarget.sol | 15 ++- contracts/interfaces/ICoordinatedTaker.sol | 32 ++++++- contracts/interfaces/IERC1271Wallet.sol | 4 + contracts/interfaces/IGenericSwap.sol | 41 ++++++++ contracts/interfaces/ILimitOrderSwap.sol | 98 +++++++++++++++++--- contracts/interfaces/IRFQ.sol | 64 +++++++++++++ contracts/interfaces/ISmartOrderStrategy.sol | 19 ++++ contracts/interfaces/IStrategy.sol | 7 ++ contracts/interfaces/IUniswapPermit2.sol | 1 + contracts/interfaces/IWETH.sol | 16 ++++ contracts/libraries/Asset.sol | 20 +++- contracts/libraries/Constant.sol | 4 + contracts/libraries/GenericSwapData.sol | 1 - contracts/libraries/SignatureValidator.sol | 16 ++-- 24 files changed, 570 insertions(+), 90 deletions(-) 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..48d2407e 100644 --- a/contracts/CoordinatedTaker.sol +++ b/contracts/CoordinatedTaker.sol @@ -14,6 +14,9 @@ import { SignatureValidator } from "./libraries/SignatureValidator.sol"; /// @title CoordinatedTaker Contract /// @author imToken Labs +/// @notice This contract is a taker contract for the LimitOrderSwap. +/// @dev It helps users avoid collisions when filling a limit order and provides an off-chain order canceling mechanism. +/// For more details, check the reference: https://github.com/consenlabs/tokenlon-contracts/blob/v6.0.1/doc/CoordinatedTaker.md contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector, EIP712 { using Asset for address; @@ -21,8 +24,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 +47,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 +60,7 @@ contract CoordinatedTaker is ICoordinatedTaker, AdminManagement, TokenCollector, emit SetCoordinator(_newCoordinator); } + /// @inheritdoc ICoordinatedTaker function submitLimitOrderFill( LimitOrder calldata order, bytes calldata makerSignature, @@ -59,15 +75,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 +96,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 55a61668..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; @@ -93,7 +104,7 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree takerTokenAmounts[i] = ((makingAmount * order.takerTokenAmount) / order.makerTokenAmount); if (takerTokenAmounts[i] == 0) revert ZeroTakerTokenAmount(); - // the if statement cannot be covered by tests, due to the following issue + // 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]; @@ -106,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); @@ -148,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, @@ -172,7 +189,7 @@ 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. + // 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); } @@ -192,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, @@ -229,7 +255,7 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree 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; @@ -237,8 +263,13 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree 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(); @@ -246,22 +277,29 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree 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 c0217ebe..a4001017 100644 --- a/contracts/RFQ.sol +++ b/contracts/RFQ.sol @@ -14,22 +14,36 @@ 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 used to mark contract allowance in `RFQOffer.flag`. + /// The left-most bit (bit 255) of the RFQOffer.flags represents whether the contract sender is allowed. uint256 private constant FLG_ALLOW_CONTRACT_SENDER = 1 << 255; + + /// @dev Flag used to mark partial fill allowance in `RFQOffer.flag`. + /// The second left-most bit (bit 254) of the RFQOffer.flags represents whether partial fill is allowed. uint256 private constant FLG_ALLOW_PARTIAL_FILL = 1 << 254; + + /// @dev Flag used to mark market maker receives WETH in `RFQOffer.flag`. + /// The third left-most bit (bit 253) of the RFQOffer.flags represents whether the 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 +56,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 +69,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 +85,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 +95,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 +109,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 +127,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 +159,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 { @@ -158,7 +184,7 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { // 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); @@ -167,6 +193,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, @@ -181,7 +212,10 @@ 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 }(); @@ -193,7 +227,12 @@ contract RFQ is IRFQ, Ownable, TokenCollector, EIP712 { } } - // 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 5e7bd284..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,7 +41,7 @@ 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 @@ -48,30 +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 - - // after replacing `makerToken.isETH()` with `makerToken == 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE` - // and crafting some cases where outputToken is ETH and non-ETH - // the if statement is still not fully covered by the test + // 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)); - // after trying to craft a test case where wethBalance == 0 - // the if statement is still not fully covered by the test + 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, @@ -81,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 @@ -92,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..615b326c 100644 --- a/contracts/abstracts/Ownable.sol +++ b/contracts/abstracts/Ownable.sol @@ -3,18 +3,39 @@ 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. + /// @param newOwner The address of the new nominated owner. event OwnerNominated(address indexed newOwner); + + /// @notice Event emitted when ownership is transferred. + /// @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 +46,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 +56,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 da2057fb..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])); 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..ac5147bb 100644 --- a/contracts/interfaces/ICoordinatedTaker.sol +++ b/contracts/interfaces/ICoordinatedTaker.sol @@ -6,24 +6,52 @@ import { LimitOrder } from "../libraries/LimitOrder.sol"; /// @title ICoordinatedTaker Interface /// @author imToken Labs 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 + /// @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. + /// @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. + /// @param newCoordinator The address of the new coordinator. event SetCoordinator(address newCoordinator); + /// @notice Submits 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..8d2e7515 100644 --- a/contracts/interfaces/IERC1271Wallet.sol +++ b/contracts/interfaces/IERC1271Wallet.sol @@ -2,5 +2,9 @@ pragma solidity ^0.8.0; interface IERC1271Wallet { + /// @notice Checks if a signature is valid for a given hash. + /// @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..4bc5693e 100644 --- a/contracts/interfaces/IGenericSwap.sol +++ b/contracts/interfaces/IGenericSwap.sol @@ -3,14 +3,45 @@ 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. + /// @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 +54,18 @@ interface IGenericSwap { uint256 salt ); + /// @notice Executes a swap using 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. + /// @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 34af0428..e16cef0d 100644 --- a/contracts/interfaces/ILimitOrderSwap.sol +++ b/contracts/interfaces/ILimitOrderSwap.sol @@ -5,28 +5,83 @@ 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(); + + /// @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. + /// @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. + /// @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, @@ -39,23 +94,38 @@ interface ILimitOrderSwap { address recipient ); - /// @notice Emitted when order is canceled + /// @notice Emitted when an 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); + /// @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. + /// @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. + /// @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. + /// @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, @@ -63,8 +133,12 @@ interface ILimitOrderSwap { address[] calldata profitTokens ) external payable; - /// @notice Cancel an order + /// @notice Cancels a limit order. + /// @param order The limit order to be canceled. function cancelOrder(LimitOrder calldata order) external; + /// @notice Checks if an order is 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..78373b52 100644 --- a/contracts/interfaces/IRFQ.sol +++ b/contracts/interfaces/IRFQ.sol @@ -6,19 +6,67 @@ 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. + /// @param newFeeCollector The address of the new fee collector. + event SetFeeCollector(address newFeeCollector); + + /// @notice Emitted when an RFQ offer is successfully filled. + /// @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 +79,24 @@ interface IRFQ { uint256 fee ); + /// @notice Emitted when an RFQ offer is canceled. + /// @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. + /// @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. + /// @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 +105,7 @@ interface IRFQ { bytes calldata takerSignature ) external; + /// @notice Cancels an RFQ offer. + /// @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..d79b1723 100644 --- a/contracts/interfaces/ISmartOrderStrategy.sol +++ b/contracts/interfaces/ISmartOrderStrategy.sol @@ -6,13 +6,32 @@ import { IStrategy } from "./IStrategy.sol"; /// @title ISmartOrderStrategy Interface /// @author imToken Labs 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..d99777ed 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 contract that implements 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..5f7f6f35 100644 --- a/contracts/interfaces/IWETH.sol +++ b/contracts/interfaces/IWETH.sol @@ -1,14 +1,30 @@ // 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. function deposit() external payable; + /// @notice Withdraws a specified amount of WETH, unwraps it into ETH, and sends it to the caller. + /// @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. + /// @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. + /// @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..ca7adcd5 100644 --- a/contracts/libraries/Asset.sol +++ b/contracts/libraries/Asset.sol @@ -6,15 +6,28 @@ 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 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 +36,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);