feat: remove input and output decay validation (#245)

* feat: remove input and output decay validation Unnecessary restriction causing extra gas via looping through all outputs and checking if decaing * feat: more tests
Uniswap · Apr 18, 2024 · 856a66f · 856a66f
1 parent f867c13
commit 856a66f
Show file tree

Hide file tree

Showing 15 changed files with 64 additions and 47 deletions.
diff --git a/.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap b/.forge-snapshots/Base-V2DutchOrder-BaseExecuteSingleWithFee.snap
@@ -1 +1 @@
-188514
+188455
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExclusiveFiller.snap b/.forge-snapshots/Base-V2DutchOrder-ExclusiveFiller.snap
@@ -1 +1 @@
-158695
+158636
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatch.snap
@@ -1 +1 @@
-210452
+210334
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputs.snap
@@ -1 +1 @@
-220677
+220559
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap
@@ -1 +1 @@
-274804
+274686
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteBatchNativeOutput.snap
@@ -1 +1 @@
-203978
+203860
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingle.snap
@@ -1 +1 @@
-154931
+154872
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleNativeOutput.snap
@@ -1 +1 @@
-140493
+140434
diff --git a/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap b/.forge-snapshots/Base-V2DutchOrder-ExecuteSingleValidation.snap
@@ -1 +1 @@
-164242
+164183
diff --git a/.forge-snapshots/Base-V2DutchOrder-InputOverride.snap b/.forge-snapshots/Base-V2DutchOrder-InputOverride.snap
@@ -1 +1 @@
-158984
+158714
diff --git a/.forge-snapshots/Base-V2DutchOrder-OutputOverride.snap b/.forge-snapshots/Base-V2DutchOrder-OutputOverride.snap
@@ -1 +1 @@
-158722
+158663
diff --git a/src/reactors/V2DutchOrderReactor.sol b/src/reactors/V2DutchOrderReactor.sol
@@ -28,9 +28,6 @@ contract V2DutchOrderReactor is BaseReactor {
     /// @notice thrown when an order's deadline is before its end time
     error DeadlineBeforeEndTime();
 
-    /// @notice thrown when an order's inputs and outputs both decay
-    error InputAndOutputDecay();
-
     /// @notice thrown when an order's cosignature does not match the expected cosigner
     error InvalidCosignature();
 
@@ -123,14 +120,5 @@ contract V2DutchOrderReactor is BaseReactor {
         if (order.cosigner != signer || signer == address(0)) {
             revert InvalidCosignature();
         }
-
-        if (order.baseInput.startAmount != order.baseInput.endAmount) {
-            for (uint256 i = 0; i < order.baseOutputs.length; i++) {
-                DutchOutput memory output = order.baseOutputs[i];
-                if (output.startAmount != output.endAmount) {
-                    revert InputAndOutputDecay();
-                }
-            }
-        }
     }
 }
diff --git a/test/reactors/BaseDutchOrderReactor.t.sol b/test/reactors/BaseDutchOrderReactor.t.sol
@@ -60,7 +60,7 @@ abstract contract BaseDutchOrderReactorTest is PermitSignature, DeployPermit2, B
         return (SignedOrder(abi.encode(request), signOrder(swapperPrivateKey, address(permit2), request)), orderHash);
     }
 
-    function generateOrder(TestDutchOrderSpec memory spec) private returns (SignedOrder memory order) {
+    function generateOrder(TestDutchOrderSpec memory spec) internal returns (SignedOrder memory order) {
         vm.warp(spec.currentTime);
         tokenIn.mint(address(swapper), uint256(spec.input.endAmount));
         tokenIn.forceApprove(swapper, address(permit2), spec.input.endAmount);
@@ -349,27 +349,6 @@ abstract contract BaseDutchOrderReactorTest is PermitSignature, DeployPermit2, B
         quoter.quote(order.order, order.sig);
     }
 
-    // The input decays, which means the outputs must not decay. In this test, the
-    // 2nd output decays, so revert with error InputAndOutputDecay().
-    function test_dutch_validateInputAndOutputDecay() public {
-        uint256 currentTime = 100;
-
-        SignedOrder memory order = generateOrder(
-            TestDutchOrderSpec({
-                currentTime: currentTime,
-                startTime: currentTime,
-                endTime: currentTime + 100,
-                deadline: currentTime + 100,
-                input: DutchInput(tokenIn, 100, 110),
-                outputs: OutputsBuilder.multipleDutch(
-                    tokenOut, Solarray.uint256s(1000, 1000), Solarray.uint256s(1000, 900), address(0)
-                )
-            })
-        );
-        vm.expectRevert(DutchOrderReactor.InputAndOutputDecay.selector);
-        quoter.quote(order.order, order.sig);
-    }
-
     function test_dutch_validateInputDecayIncorrectAmounts() public {
         uint256 currentTime = 100;
 

diff --git a/test/reactors/DutchOrderReactor.t.sol b/test/reactors/DutchOrderReactor.t.sol
@@ -25,7 +25,7 @@ import {MockFillContract} from "../util/mock/MockFillContract.sol";
 import {MockFillContractWithOutputOverride} from "../util/mock/MockFillContractWithOutputOverride.sol";
 import {PermitSignature} from "../util/PermitSignature.sol";
 import {ReactorEvents} from "../../src/base/ReactorEvents.sol";
-import {BaseDutchOrderReactorTest} from "./BaseDutchOrderReactor.t.sol";
+import {BaseDutchOrderReactorTest, TestDutchOrderSpec} from "./BaseDutchOrderReactor.t.sol";
 
 // This suite of tests test execution with a mock fill contract.
 contract DutchOrderReactorTest is PermitSignature, DeployPermit2, BaseDutchOrderReactorTest {
@@ -245,6 +245,24 @@ contract DutchOrderReactorTest is PermitSignature, DeployPermit2, BaseDutchOrder
         fill.executeBatch(generateSignedOrders(orders));
     }
 
+    // 2nd output decays, so revert with error InputAndOutputDecay().
+    function testValidateInputAndOutputDecay() public {
+        uint256 currentTime = 100;
+
+        SignedOrder memory order = generateOrder(
+            TestDutchOrderSpec({
+                currentTime: currentTime,
+                startTime: currentTime,
+                endTime: currentTime + 100,
+                deadline: currentTime + 100,
+                input: DutchInput(tokenIn, 100, 110),
+                outputs: OutputsBuilder.singleDutch(tokenOut, 1000, 900, address(0))
+            })
+        );
+        vm.expectRevert(DutchOrderReactor.InputAndOutputDecay.selector);
+        quoter.quote(order.order, order.sig);
+    }
+
     function generateSignedOrders(DutchOrder[] memory orders) private view returns (SignedOrder[] memory result) {
         result = new SignedOrder[](orders.length);
         for (uint256 i = 0; i < orders.length; i++) {

diff --git a/test/reactors/V2DutchOrderReactor.t.sol b/test/reactors/V2DutchOrderReactor.t.sol
@@ -25,7 +25,7 @@ import {MockFillContract} from "../util/mock/MockFillContract.sol";
 import {MockFillContractWithOutputOverride} from "../util/mock/MockFillContractWithOutputOverride.sol";
 import {PermitSignature} from "../util/PermitSignature.sol";
 import {ReactorEvents} from "../../src/base/ReactorEvents.sol";
-import {DutchOrder, BaseDutchOrderReactorTest} from "./BaseDutchOrderReactor.t.sol";
+import {DutchOrder, BaseDutchOrderReactorTest, TestDutchOrderSpec} from "./BaseDutchOrderReactor.t.sol";
 
 contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReactorTest {
     using OrderInfoBuilder for OrderInfo;
@@ -453,6 +453,38 @@ contract V2DutchOrderTest is PermitSignature, DeployPermit2, BaseDutchOrderReact
         assertEq(tokenIn.balanceOf(address(fillContract)), inputAmount);
     }
 
+    function testExecuteInputAndOutputDecay() public {
+        uint256 inputAmount = 1 ether;
+        uint256 outputAmount = 1 ether;
+        uint256 startTime = block.timestamp;
+        uint256 deadline = startTime + 1000;
+        // Seed both swapper and fillContract with enough tokens (important for dutch order)
+        tokenIn.mint(address(swapper), uint256(inputAmount) * 100);
+        tokenOut.mint(address(fillContract), uint256(outputAmount) * 100);
+        tokenIn.forceApprove(swapper, address(permit2), inputAmount);
+
+        SignedOrder memory order = generateOrder(
+            TestDutchOrderSpec({
+                currentTime: startTime,
+                startTime: startTime,
+                endTime: deadline,
+                deadline: deadline,
+                input: DutchInput(tokenIn, inputAmount, inputAmount * 110 / 100),
+                outputs: OutputsBuilder.singleDutch(tokenOut, outputAmount, outputAmount * 90 / 100, address(swapper))
+            })
+        );
+        uint256 swapperInputBalanceStart = tokenIn.balanceOf(address(swapper));
+        uint256 swapperOutputBalanceStart = tokenOut.balanceOf(address(swapper));
+        vm.expectEmit(false, true, true, false, address(reactor));
+        emit Fill(keccak256("not checked"), address(fillContract), swapper, 0);
+        vm.warp(startTime + 500);
+        fillContract.execute(order);
+        uint256 swapperInputBalanceEnd = tokenIn.balanceOf(address(swapper));
+        uint256 swapperOutputBalanceEnd = tokenOut.balanceOf(address(swapper));
+        assertEq(swapperInputBalanceStart - swapperInputBalanceEnd, inputAmount * 105 / 100);
+        assertEq(swapperOutputBalanceEnd - swapperOutputBalanceStart, outputAmount * 95 / 100);
+    }
+
     function cosignOrder(bytes32 orderHash, CosignerData memory cosignerData) private pure returns (bytes memory sig) {
         bytes32 msgHash = keccak256(abi.encodePacked(orderHash, abi.encode(cosignerData)));
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(cosignerPrivateKey, msgHash);