disallow taker increaser

1inch · Oct 4, 2023 · fad5579 · fad5579
1 parent b3e38ba
commit fad5579
Show file tree

Hide file tree

Showing 7 changed files with 6 additions and 166 deletions.
diff --git a/contracts/OrderMixin.sol b/contracts/OrderMixin.sol
@@ -373,14 +373,11 @@ abstract contract OrderMixin is IOrderMixin, EIP712, OnlyWethReceiver, Predicate
             }
         }
 
-        if (interaction.length >= 20) {
+        if (interaction.length > 19) {
             // proceed only if interaction length is enough to store address
-            uint256 offeredTakingAmount = ITakerInteraction(address(bytes20(interaction))).takerInteraction(
+            ITakerInteraction(address(bytes20(interaction))).takerInteraction(
                 order, extension, orderHash, msg.sender, makingAmount, takingAmount, remainingMakingAmount, interaction[20:]
             );
-            if (offeredTakingAmount > takingAmount && order.makerTraits.allowImproveRateViaInteraction()) {
-                takingAmount = offeredTakingAmount;
-            }
         }
 
         // Taker => Maker

diff --git a/contracts/interfaces/ITakerInteraction.sol b/contracts/interfaces/ITakerInteraction.sol
@@ -30,5 +30,5 @@ interface ITakerInteraction {
         uint256 takingAmount,
         uint256 remainingMakingAmount,
         bytes calldata extraData
-    ) external returns(uint256 offeredTakingAmount);
+    ) external;
 }
diff --git a/contracts/mocks/InteractionMock.sol b/contracts/mocks/InteractionMock.sol
@@ -3,9 +3,8 @@ pragma solidity 0.8.19;
 
 import "../interfaces/IPreInteraction.sol";
 import "../interfaces/IPostInteraction.sol";
-import "../interfaces/ITakerInteraction.sol";
 
-contract InteractionMock is IPreInteraction, IPostInteraction, ITakerInteraction {
+contract InteractionMock is IPreInteraction, IPostInteraction {
     error InvalidExtraDataLength();
     error TakingAmountTooHigh();
     error IncorrectTakingAmount();
@@ -53,24 +52,4 @@ contract InteractionMock is IPreInteraction, IPostInteraction, ITakerInteraction
 
         if (takingAmount > threshold) revert TakingAmountTooHigh();
     }
-
-    function takerInteraction(
-        IOrderMixin.Order calldata /* order */,
-        bytes calldata /* extension */,
-        bytes32 /* orderHash */,
-        address /* taker */,
-        uint256 /* makingAmount */,
-        uint256 takingAmount,
-        uint256 /* remainingMakingAmount */,
-        bytes calldata extraData
-    ) external pure returns(uint256 offeredTakingAmount){
-        if (extraData.length < 32) return takingAmount;
-
-        uint256 increaseAmount;
-        assembly ("memory-safe") { // solhint-disable-line no-inline-assembly
-            increaseAmount := calldataload(extraData.offset)
-        }
-
-        return takingAmount + increaseAmount;
-    }
 }
diff --git a/contracts/mocks/RecursiveMatcher.sol b/contracts/mocks/RecursiveMatcher.sol
@@ -42,7 +42,7 @@ contract RecursiveMatcher is ITakerInteraction {
         uint256 /* takingAmount */,
         uint256 /* remainingMakingAmount */,
         bytes calldata extraData
-    ) external returns(uint256 offeredTakingAmount) {
+    ) external {
         if(extraData[0] & _FINALIZE_INTERACTION != 0x0) {
             (
                 address[] memory targets,
@@ -62,6 +62,5 @@ contract RecursiveMatcher is ITakerInteraction {
             );
             if (!success) revert FailedExternalCall(reason);
         }
-        return 0;
     }
 }
diff --git a/contracts/mocks/TakerIncreaser.sol b/contracts/mocks/TakerIncreaser.sol
diff --git a/test/Interactions.js b/test/Interactions.js
@@ -488,57 +488,4 @@ describe('Interactions', function () {
                 .to.be.revertedWithCustomError(orderIdInvalidator, 'InvalidOrderHash');
         });
     });
-
-    it('check the possibility of a dutch auction', async function () {
-        const { dai, weth, swap, chainId } = await initContracts();
-
-        const TakerIncreaser = await ethers.getContractFactory('TakerIncreaser');
-        const takerIncreaser = await TakerIncreaser.deploy();
-        await takerIncreaser.deployed();
-
-        const order = buildOrder({
-            makerAsset: weth.address,
-            takerAsset: dai.address,
-            makingAmount: ether('0.1'),
-            takingAmount: ether('50'),
-            maker: addr1.address,
-            makerTraits: buildMakerTraits({ expiry: 0xff00000000 }),
-        });
-
-        const signature = await signOrder(order, chainId, swap.address, addr1);
-        const { r, _vs: vs } = ethers.utils.splitSignature(signature);
-
-        const interaction = takerIncreaser.address + abiCoder.encode(
-            ['address[]', 'bytes[]'],
-            [
-                [
-                    dai.address,
-                    dai.address,
-                ],
-                [
-                    dai.interface.encodeFunctionData('transferFrom', [addr.address, takerIncreaser.address, ether('75')]),
-                    dai.interface.encodeFunctionData('approve', [swap.address, ether('75')]),
-                ],
-            ],
-        ).substring(2);
-        await dai.approve(takerIncreaser.address, ether('75'));
-
-        const addrweth = await weth.balanceOf(addr.address);
-        const addr1weth = await weth.balanceOf(addr1.address);
-        const addrdai = await dai.balanceOf(addr.address);
-        const addr1dai = await dai.balanceOf(addr1.address);
-
-        const takerTraits = buildTakerTraits({
-            makingAmount: true,
-            target: addr.address,
-            interaction,
-            minReturn: ether('50'),
-        });
-        await takerIncreaser.fillOrderArgs(swap.address, order, r, vs, ether('0.1'), takerTraits.traits, takerTraits.args);
-
-        expect(await weth.balanceOf(addr.address)).to.equal(addrweth.add(ether('0.1')));
-        expect(await weth.balanceOf(addr1.address)).to.equal(addr1weth.sub(ether('0.1')));
-        expect(await dai.balanceOf(addr.address)).to.equal(addrdai.sub(ether('75')));
-        expect(await dai.balanceOf(addr1.address)).to.equal(addr1dai.add(ether('75')));
-    });
 });
diff --git a/test/LimitOrderProtocol.js b/test/LimitOrderProtocol.js
@@ -301,9 +301,7 @@ describe('LimitOrderProtocol', function () {
         const deployContractsAndInit = async function () {
             const { dai, weth, swap, chainId } = await deploySwapTokens();
             await initContracts(dai, weth, swap);
-            const TakerIncreaser = await ethers.getContractFactory('TakerIncreaser');
-            const takerIncreaser = await TakerIncreaser.deploy();
-            return { dai, weth, swap, chainId, takerIncreaser };
+            return { dai, weth, swap, chainId };
         };
 
         it('disallow multiple fills', async function () {
@@ -388,30 +386,6 @@ describe('LimitOrderProtocol', function () {
             await expect(filltx).to.changeTokenBalance(weth, addr, -2);
             await expect(filltx).to.changeEtherBalance(addr1, 2);
         });
-
-        it('allow taker rate improvement', async function () {
-            const { dai, weth, swap, chainId, takerIncreaser } = await loadFixture(deployContractsAndInit);
-            // Order: 10 DAI => 2 WETH
-            // Swap:  10 DAI => 3 WETH
-
-            const order = buildOrder({
-                makerAsset: dai.address,
-                takerAsset: weth.address,
-                makingAmount: 10,
-                takingAmount: 2,
-                maker: addr1.address,
-                makerTraits: buildMakerTraits({ allowPriceImprovement: true }),
-            });
-
-            const { r, _vs: vs } = ethers.utils.splitSignature(await signOrder(order, chainId, swap.address, addr1));
-            const takerTraits = buildTakerTraits({
-                minRetrun: 2n,
-                interaction: takerIncreaser.address,
-            });
-            const filltx = swap.fillOrderArgs(order, r, vs, 10, takerTraits.traits, takerTraits.args);
-            await expect(filltx).to.changeTokenBalances(dai, [addr, addr1], [10, -10]);
-            await expect(filltx).to.changeTokenBalances(weth, [addr, addr1], [-3, 3]);
-        });
     });
 
     describe('Permit', function () {