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..ab57a146 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,7 +79,8 @@ contract AMMsTest is SmartOrderStrategyTest { operations[0] = ISmartOrderStrategy.Operation({ dest: UNISWAP_SWAP_ROUTER_02_ADDRESS, inputToken: defaultInputToken, - inputRatio: defaultInputRatio, + ratioNumerator: defaultInputRatio, + ratioDenominator: 10000, dataOffset: uint128(4 + 32 + 128), // add 32 bytes of length prefix value: 0, data: uniswapData @@ -117,7 +119,8 @@ 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 + ratioNumerator: 1, // same numerator and ratioDenominator indicate the input amount will be replaced by the actual balance + ratioDenominator: 1, dataOffset: uint128(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/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);