fix: enforce dutch decay actually decays

Previously there was ambiguous undefined behavior for if startTime and endTime were the same - This commit removes that ambiguity by enforcing startTime !== endTime
Uniswap · Aug 22, 2023 · 068076f · 068076f
1 parent 0d81086
commit 068076f
Show file tree

Hide file tree

Showing 37 changed files with 55 additions and 40 deletions.
diff --git a/.forge-snapshots/Base-DutchOrder-BaseExecuteSingleWithFee.snap b/.forge-snapshots/Base-DutchOrder-BaseExecuteSingleWithFee.snap
@@ -1 +1 @@
-181978
+181972
diff --git a/.forge-snapshots/Base-DutchOrder-ExecuteBatch.snap b/.forge-snapshots/Base-DutchOrder-ExecuteBatch.snap
@@ -1 +1 @@
-196879
+196867
diff --git a/.forge-snapshots/Base-DutchOrder-ExecuteBatchMultipleOutputs.snap b/.forge-snapshots/Base-DutchOrder-ExecuteBatchMultipleOutputs.snap
@@ -1 +1 @@
-206650
+206635
diff --git a/.forge-snapshots/Base-DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap b/.forge-snapshots/Base-DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap
@@ -1 +1 @@
-260307
+260289
diff --git a/.forge-snapshots/Base-DutchOrder-ExecuteBatchNativeOutput.snap b/.forge-snapshots/Base-DutchOrder-ExecuteBatchNativeOutput.snap
@@ -1 +1 @@
-190411
+190399
diff --git a/.forge-snapshots/Base-DutchOrder-ExecuteSingle.snap b/.forge-snapshots/Base-DutchOrder-ExecuteSingle.snap
@@ -1 +1 @@
-148194
+148188
diff --git a/.forge-snapshots/Base-DutchOrder-ExecuteSingleNativeOutput.snap b/.forge-snapshots/Base-DutchOrder-ExecuteSingleNativeOutput.snap
@@ -1 +1 @@
-133756
+133750
diff --git a/.forge-snapshots/Base-DutchOrder-ExecuteSingleValidation.snap b/.forge-snapshots/Base-DutchOrder-ExecuteSingleValidation.snap
@@ -1 +1 @@
-137605
+157499
diff --git a/.forge-snapshots/Base-ExclusiveDutchOrder-BaseExecuteSingleWithFee.snap b/.forge-snapshots/Base-ExclusiveDutchOrder-BaseExecuteSingleWithFee.snap
@@ -1 +1 @@
-150633
+182180
diff --git a/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteBatch.snap b/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteBatch.snap
@@ -1 +1 @@
-197283
+197271
diff --git a/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteBatchMultipleOutputs.snap b/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteBatchMultipleOutputs.snap
@@ -1 +1 @@
-207049
+207034
diff --git a/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap b/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap
@@ -1 +1 @@
-260720
+260702
diff --git a/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteBatchNativeOutput.snap b/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteBatchNativeOutput.snap
@@ -1 +1 @@
-190809
+190797
diff --git a/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteSingle.snap b/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteSingle.snap
@@ -1 +1 @@
-148401
+148395
diff --git a/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteSingleNativeOutput.snap b/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteSingleNativeOutput.snap
@@ -1 +1 @@
-95568
+133962
diff --git a/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteSingleValidation.snap b/.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteSingleValidation.snap
@@ -1 +1 @@
-157716
+157710
diff --git a/.forge-snapshots/Base-LimitOrderReactor-ExecuteSingleValidation.snap b/.forge-snapshots/Base-LimitOrderReactor-ExecuteSingleValidation.snap
@@ -1 +1 @@
-133743
+153643
diff --git a/.forge-snapshots/DirectFillerFillMacroSingleOrder.snap b/.forge-snapshots/DirectFillerFillMacroSingleOrder.snap
@@ -1 +1 @@
-136213
+136207
diff --git a/.forge-snapshots/DirectFillerFillMacroSingleOrderWithFee.snap b/.forge-snapshots/DirectFillerFillMacroSingleOrderWithFee.snap
@@ -1 +1 @@
-175064
+175055
diff --git a/.forge-snapshots/DirectFillerFillMacroTestEth1Output.snap b/.forge-snapshots/DirectFillerFillMacroTestEth1Output.snap
@@ -1 +1 @@
-147390
+147384
diff --git a/.forge-snapshots/DirectFillerFillMacroTestEth2Outputs.snap b/.forge-snapshots/DirectFillerFillMacroTestEth2Outputs.snap
@@ -1 +1 @@
-170604
+170592
diff --git a/.forge-snapshots/DirectFillerFillMacroThreeOrdersWithFees.snap b/.forge-snapshots/DirectFillerFillMacroThreeOrdersWithFees.snap
@@ -1 +1 @@
-435187
+435160
diff --git a/.forge-snapshots/DirectFillerFillMacroTwoOrders.snap b/.forge-snapshots/DirectFillerFillMacroTwoOrders.snap
@@ -1 +1 @@
-255972
+255957
diff --git a/.forge-snapshots/EthOutputTest3OrdersWithEthAndERC20Outputs.snap b/.forge-snapshots/EthOutputTest3OrdersWithEthAndERC20Outputs.snap
@@ -1 +1 @@
-363478
+363457
diff --git a/.forge-snapshots/EthOutputTestEthOutput.snap b/.forge-snapshots/EthOutputTestEthOutput.snap
@@ -1 +1 @@
-156223
+156217
diff --git a/.forge-snapshots/ProtocolFeesGasComparisonTest-InterfaceAndProtocolFee.snap b/.forge-snapshots/ProtocolFeesGasComparisonTest-InterfaceAndProtocolFee.snap
@@ -1 +1 @@
-176915
+176906
diff --git a/.forge-snapshots/ProtocolFeesGasComparisonTest-InterfaceAndProtocolFeeEthOutput.snap b/.forge-snapshots/ProtocolFeesGasComparisonTest-InterfaceAndProtocolFeeEthOutput.snap
@@ -1 +1 @@
-162907
+162898
diff --git a/.forge-snapshots/ProtocolFeesGasComparisonTest-InterfaceFee.snap b/.forge-snapshots/ProtocolFeesGasComparisonTest-InterfaceFee.snap
@@ -1 +1 @@
-166046
+166037
diff --git a/.forge-snapshots/ProtocolFeesGasComparisonTest-InterfaceFeeEthOutput.snap b/.forge-snapshots/ProtocolFeesGasComparisonTest-InterfaceFeeEthOutput.snap
@@ -1 +1 @@
-146873
+146864
diff --git a/.forge-snapshots/ProtocolFeesGasComparisonTest-NoFees.snap b/.forge-snapshots/ProtocolFeesGasComparisonTest-NoFees.snap
@@ -1 +1 @@
-149267
+149261
diff --git a/.forge-snapshots/ProtocolFeesGasComparisonTest-NoFeesEthOutput.snap b/.forge-snapshots/ProtocolFeesGasComparisonTest-NoFeesEthOutput.snap
@@ -1 +1 @@
-124929
+124923
diff --git a/.forge-snapshots/SwapRouter02ExecutorExecute.snap b/.forge-snapshots/SwapRouter02ExecutorExecute.snap
@@ -1 +1 @@
-262973
+262967
diff --git a/.forge-snapshots/SwapRouter02ExecutorExecuteAlreadyApproved.snap b/.forge-snapshots/SwapRouter02ExecutorExecuteAlreadyApproved.snap
@@ -1 +1 @@
-118092
+118086
diff --git a/.forge-snapshots/testExclusiveFillerSucceeds.snap b/.forge-snapshots/testExclusiveFillerSucceeds.snap
@@ -1 +1 @@
-174066
+174060
diff --git a/src/lib/DutchDecayLib.sol b/src/lib/DutchDecayLib.sol
@@ -28,7 +28,7 @@ library DutchDecayLib {
         view
         returns (uint256 decayedAmount)
     {
-        if (decayEndTime < decayStartTime) {
+        if (decayEndTime <= decayStartTime) {
             revert EndTimeBeforeStartTime();
         } else if (decayEndTime <= block.timestamp) {
             decayedAmount = endAmount;

diff --git a/test/lib/DutchDecayLib.t.sol b/test/lib/DutchDecayLib.t.sol
@@ -6,7 +6,7 @@ import {DutchDecayLib} from "../../src/lib/DutchDecayLib.sol";
 
 contract DutchDecayLibTest is Test {
     function testDutchDecayNoDecay(uint256 amount, uint256 decayStartTime, uint256 decayEndTime) public {
-        vm.assume(decayEndTime >= decayStartTime);
+        vm.assume(decayEndTime > decayStartTime);
         assertEq(DutchDecayLib.decay(amount, amount, decayStartTime, decayEndTime), amount);
     }
 
@@ -78,7 +78,7 @@ contract DutchDecayLibTest is Test {
         public
     {
         vm.assume(endAmount > startAmount);
-        vm.assume(decayEndTime >= decayStartTime);
+        vm.assume(decayEndTime > decayStartTime);
         uint256 decayed = DutchDecayLib.decay(startAmount, endAmount, decayStartTime, decayEndTime);
         assertGe(decayed, startAmount);
         assertLe(decayed, endAmount);
@@ -91,7 +91,7 @@ contract DutchDecayLibTest is Test {
         uint256 decayEndTime
     ) public {
         vm.assume(endAmount < startAmount);
-        vm.assume(decayEndTime >= decayStartTime);
+        vm.assume(decayEndTime > decayStartTime);
         uint256 decayed = DutchDecayLib.decay(startAmount, endAmount, decayStartTime, decayEndTime);
         assertLe(decayed, startAmount);
         assertGe(decayed, endAmount);

diff --git a/test/reactors/DutchOrderReactor.t.sol b/test/reactors/DutchOrderReactor.t.sol
@@ -160,13 +160,28 @@ contract DutchOrderReactorValidationTest is Test, DeployPermit2 {
         reactor.resolveOrder(SignedOrder(abi.encode(dlo), sig));
     }
 
-    function testValidateDutchEndTimeAfterStart() public view {
+    function testValidateDutchEndTimeEqualStart() public {
+        vm.expectRevert(DutchDecayLib.EndTimeBeforeStartTime.selector);
         DutchOutput[] memory dutchOutputs = new DutchOutput[](1);
         dutchOutputs[0] = DutchOutput(address(0), 1000, 900, address(0));
         DutchOrder memory dlo = DutchOrder(
             OrderInfoBuilder.init(address(reactor)).withDeadline(1659130540),
-            1659120540,
             1659130540,
+            1659130540,
+            DutchInput(MockERC20(address(0)), 0, 0),
+            dutchOutputs
+        );
+        bytes memory sig = hex"1234";
+        reactor.resolveOrder(SignedOrder(abi.encode(dlo), sig));
+    }
+
+    function testValidateDutchEndTimeAfterStart() public view {
+        DutchOutput[] memory dutchOutputs = new DutchOutput[](1);
+        dutchOutputs[0] = DutchOutput(address(0), 1000, 900, address(0));
+        DutchOrder memory dlo = DutchOrder(
+            OrderInfoBuilder.init(address(reactor)).withDeadline(1659130541),
+            1659120540,
+            1659130541,
             DutchInput(MockERC20(address(0)), 0, 0),
             dutchOutputs
         );