From 8dce531c27a524e4b5e840a2cc2187fac5dbf734 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Tue, 1 Oct 2024 16:56:40 -0400 Subject: [PATCH 1/4] feat, test: round gas adjustments in favor of swapper --- src/reactors/V3DutchOrderReactor.sol | 12 ++++--- test/reactors/V3DutchOrderReactor.t.sol | 48 +++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/src/reactors/V3DutchOrderReactor.sol b/src/reactors/V3DutchOrderReactor.sol index dd184237..e6228404 100644 --- a/src/reactors/V3DutchOrderReactor.sol +++ b/src/reactors/V3DutchOrderReactor.sol @@ -112,15 +112,19 @@ contract V3DutchOrderReactor is BaseReactor { // positive means an increase in gas int256 gasDeltaGwei = block.basefee.sub(order.startingBaseFee); - // Gas increase should increase input - int256 inputDelta = int256(order.baseInput.adjustmentPerGweiBaseFee) * gasDeltaGwei / 1 gwei; + // Gas increase should increase input (round up when positive, down when negative) + int256 inputDelta = gasDeltaGwei >= 0 + ? (int256(order.baseInput.adjustmentPerGweiBaseFee) * gasDeltaGwei + 1 gwei - 1) / 1 gwei + : (int256(order.baseInput.adjustmentPerGweiBaseFee) * gasDeltaGwei) / 1 gwei; order.baseInput.startAmount = order.baseInput.startAmount.boundedAdd(inputDelta, 0, order.baseInput.maxAmount); - // Gas increase should decrease output + // Gas increase should decrease output (round down when positive, up when negative) uint256 outputsLength = order.baseOutputs.length; for (uint256 i = 0; i < outputsLength; i++) { V3DutchOutput memory output = order.baseOutputs[i]; - int256 outputDelta = int256(output.adjustmentPerGweiBaseFee) * gasDeltaGwei / 1 gwei; + int256 outputDelta = gasDeltaGwei >= 0 + ? (int256(output.adjustmentPerGweiBaseFee) * gasDeltaGwei) / 1 gwei + : (int256(output.adjustmentPerGweiBaseFee) * gasDeltaGwei - 1 gwei + 1) / 1 gwei; output.startAmount = output.startAmount.boundedSub(outputDelta, output.minAmount, type(uint256).max); } } diff --git a/test/reactors/V3DutchOrderReactor.t.sol b/test/reactors/V3DutchOrderReactor.t.sol index fe4420e4..53a54d5b 100644 --- a/test/reactors/V3DutchOrderReactor.t.sol +++ b/test/reactors/V3DutchOrderReactor.t.sol @@ -1401,6 +1401,54 @@ contract V3DutchOrderTest is PermitSignature, DeployPermit2, BaseReactorTest { assertEq(resolvedOrder.input.amount, 0); } + function testV3GasAdjustmentRounding() public { + uint256 currentBlock = 21212121; + vm.roll(currentBlock); + vm.fee(1 gwei); + + // Order with 0.8 gwei gas adjustments + SignedOrder memory order = generateOrder( + TestDutchOrderSpec({ + currentBlock: currentBlock, + startBlock: currentBlock, + deadline: currentBlock + 21, + input: V3DutchInput(tokenIn, 100 ether, CurveBuilder.emptyCurve(), 101 ether, 0.8 gwei), + outputs: OutputsBuilder.singleV3Dutch( + V3DutchOutput(address(tokenOut), 100 ether, CurveBuilder.emptyCurve(), address(0), 99 ether, 0.8 gwei) + ) + }) + ); + + // Test gas increase + vm.fee(2 gwei); + ResolvedOrder memory resolvedOrder = quoter.quote(order.order, order.sig); + assertEq(resolvedOrder.input.amount, 100 ether + 0.8 gwei, "Input should round up for positive gas change"); + assertEq(resolvedOrder.outputs[0].amount, 100 ether - 0.8 gwei, "Output should round down for positive gas change"); + + // Test gas decrease + vm.fee(0.5 gwei); + resolvedOrder = quoter.quote(order.order, order.sig); + assertEq(resolvedOrder.input.amount, 100 ether - 0.4 gwei, "Input should round down for negative gas change"); + assertEq(resolvedOrder.outputs[0].amount, 100 ether + 0.4 gwei, "Output should round up for negative gas change"); + + // Test gas half + vm.fee(0.5 gwei); + resolvedOrder = quoter.quote(order.order, order.sig); + assertEq(resolvedOrder.input.amount, 100 ether - 0.4 gwei, "Input should round down for gas halving change"); + assertEq(resolvedOrder.outputs[0].amount, 100 ether + 0.4 gwei, "Output should round up for exact gas halving change"); + + // Test smaller gas changes + vm.fee(1.1 gwei); + resolvedOrder = quoter.quote(order.order, order.sig); + assertEq(resolvedOrder.input.amount, 100 ether + 0.08 gwei, "Input should handle small positive gas changes"); + assertEq(resolvedOrder.outputs[0].amount, 100 ether - 0.08 gwei, "Output should handle small positive gas changes"); + + vm.fee(0.9 gwei); + resolvedOrder = quoter.quote(order.order, order.sig); + assertEq(resolvedOrder.input.amount, 100 ether - 0.08 gwei, "Input should handle small negative gas changes"); + assertEq(resolvedOrder.outputs[0].amount, 100 ether + 0.08 gwei, "Output should handle small negative gas changes"); + } + /* Test helpers */ struct TestDutchOrderSpec { From 5e8716f8bf91559856ee832568f2f5c6510d5665 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Thu, 3 Oct 2024 13:02:35 -0400 Subject: [PATCH 2/4] refactor: use solmate for rounding, test wei instead of gwei --- ...V3DutchOrder-BaseExecuteSingleWithFee.snap | 2 +- .../Base-V3DutchOrder-ExecuteBatch.snap | 2 +- ...utchOrder-ExecuteBatchMultipleOutputs.snap | 2 +- ...teBatchMultipleOutputsDifferentTokens.snap | 2 +- ...V3DutchOrder-ExecuteBatchNativeOutput.snap | 2 +- .../Base-V3DutchOrder-ExecuteSingle.snap | 2 +- ...3DutchOrder-ExecuteSingleNativeOutput.snap | 2 +- ...-V3DutchOrder-ExecuteSingleValidation.snap | 2 +- .../Base-V3DutchOrder-RevertInvalidNonce.snap | 2 +- .../Base-V3DutchOrder-V3-ExclusiveFiller.snap | 2 +- .../Base-V3DutchOrder-V3-InputOverride.snap | 2 +- .../Base-V3DutchOrder-V3-OutputOverride.snap | 2 +- src/reactors/V3DutchOrderReactor.sol | 27 ++++++++----- test/reactors/V3DutchOrderReactor.t.sol | 38 ++++++++++--------- 14 files changed, 49 insertions(+), 40 deletions(-) diff --git a/.forge-snapshots/Base-V3DutchOrder-BaseExecuteSingleWithFee.snap b/.forge-snapshots/Base-V3DutchOrder-BaseExecuteSingleWithFee.snap index 54347403..4189ab1c 100644 --- a/.forge-snapshots/Base-V3DutchOrder-BaseExecuteSingleWithFee.snap +++ b/.forge-snapshots/Base-V3DutchOrder-BaseExecuteSingleWithFee.snap @@ -1 +1 @@ -198780 \ No newline at end of file +198680 \ No newline at end of file diff --git a/.forge-snapshots/Base-V3DutchOrder-ExecuteBatch.snap b/.forge-snapshots/Base-V3DutchOrder-ExecuteBatch.snap index 038dfe2d..6d9b5398 100644 --- a/.forge-snapshots/Base-V3DutchOrder-ExecuteBatch.snap +++ b/.forge-snapshots/Base-V3DutchOrder-ExecuteBatch.snap @@ -1 +1 @@ -231116 \ No newline at end of file +230916 \ No newline at end of file diff --git a/.forge-snapshots/Base-V3DutchOrder-ExecuteBatchMultipleOutputs.snap b/.forge-snapshots/Base-V3DutchOrder-ExecuteBatchMultipleOutputs.snap index 04587131..db264c79 100644 --- a/.forge-snapshots/Base-V3DutchOrder-ExecuteBatchMultipleOutputs.snap +++ b/.forge-snapshots/Base-V3DutchOrder-ExecuteBatchMultipleOutputs.snap @@ -1 +1 @@ -244626 \ No newline at end of file +244376 \ No newline at end of file diff --git a/.forge-snapshots/Base-V3DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap b/.forge-snapshots/Base-V3DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap index 3f03abfb..5fa802f2 100644 --- a/.forge-snapshots/Base-V3DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap +++ b/.forge-snapshots/Base-V3DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap @@ -1 +1 @@ -302041 \ No newline at end of file +301741 \ No newline at end of file diff --git a/.forge-snapshots/Base-V3DutchOrder-ExecuteBatchNativeOutput.snap b/.forge-snapshots/Base-V3DutchOrder-ExecuteBatchNativeOutput.snap index 9e863299..18ceec9a 100644 --- a/.forge-snapshots/Base-V3DutchOrder-ExecuteBatchNativeOutput.snap +++ b/.forge-snapshots/Base-V3DutchOrder-ExecuteBatchNativeOutput.snap @@ -1 +1 @@ -224642 \ No newline at end of file +224442 \ No newline at end of file diff --git a/.forge-snapshots/Base-V3DutchOrder-ExecuteSingle.snap b/.forge-snapshots/Base-V3DutchOrder-ExecuteSingle.snap index f90f0f91..dac86684 100644 --- a/.forge-snapshots/Base-V3DutchOrder-ExecuteSingle.snap +++ b/.forge-snapshots/Base-V3DutchOrder-ExecuteSingle.snap @@ -1 +1 @@ -165165 \ No newline at end of file +165065 \ No newline at end of file diff --git a/.forge-snapshots/Base-V3DutchOrder-ExecuteSingleNativeOutput.snap b/.forge-snapshots/Base-V3DutchOrder-ExecuteSingleNativeOutput.snap index 9b6d35ad..462a84ec 100644 --- a/.forge-snapshots/Base-V3DutchOrder-ExecuteSingleNativeOutput.snap +++ b/.forge-snapshots/Base-V3DutchOrder-ExecuteSingleNativeOutput.snap @@ -1 +1 @@ -150727 \ No newline at end of file +150627 \ No newline at end of file diff --git a/.forge-snapshots/Base-V3DutchOrder-ExecuteSingleValidation.snap b/.forge-snapshots/Base-V3DutchOrder-ExecuteSingleValidation.snap index 692864e4..1bf89fbb 100644 --- a/.forge-snapshots/Base-V3DutchOrder-ExecuteSingleValidation.snap +++ b/.forge-snapshots/Base-V3DutchOrder-ExecuteSingleValidation.snap @@ -1 +1 @@ -174481 \ No newline at end of file +174381 \ No newline at end of file diff --git a/.forge-snapshots/Base-V3DutchOrder-RevertInvalidNonce.snap b/.forge-snapshots/Base-V3DutchOrder-RevertInvalidNonce.snap index 5abcbe4d..c7b7e542 100644 --- a/.forge-snapshots/Base-V3DutchOrder-RevertInvalidNonce.snap +++ b/.forge-snapshots/Base-V3DutchOrder-RevertInvalidNonce.snap @@ -1 +1 @@ -43785 \ No newline at end of file +43685 \ No newline at end of file diff --git a/.forge-snapshots/Base-V3DutchOrder-V3-ExclusiveFiller.snap b/.forge-snapshots/Base-V3DutchOrder-V3-ExclusiveFiller.snap index 2b02fc3b..751f1997 100644 --- a/.forge-snapshots/Base-V3DutchOrder-V3-ExclusiveFiller.snap +++ b/.forge-snapshots/Base-V3DutchOrder-V3-ExclusiveFiller.snap @@ -1 +1 @@ -169089 \ No newline at end of file +168989 \ No newline at end of file diff --git a/.forge-snapshots/Base-V3DutchOrder-V3-InputOverride.snap b/.forge-snapshots/Base-V3DutchOrder-V3-InputOverride.snap index 2d97ac44..1b58f6bb 100644 --- a/.forge-snapshots/Base-V3DutchOrder-V3-InputOverride.snap +++ b/.forge-snapshots/Base-V3DutchOrder-V3-InputOverride.snap @@ -1 +1 @@ -169170 \ No newline at end of file +169070 \ No newline at end of file diff --git a/.forge-snapshots/Base-V3DutchOrder-V3-OutputOverride.snap b/.forge-snapshots/Base-V3DutchOrder-V3-OutputOverride.snap index b4dc2dd4..8514e5c2 100644 --- a/.forge-snapshots/Base-V3DutchOrder-V3-OutputOverride.snap +++ b/.forge-snapshots/Base-V3DutchOrder-V3-OutputOverride.snap @@ -1 +1 @@ -169113 \ No newline at end of file +169013 \ No newline at end of file diff --git a/src/reactors/V3DutchOrderReactor.sol b/src/reactors/V3DutchOrderReactor.sol index e6228404..1f586c09 100644 --- a/src/reactors/V3DutchOrderReactor.sol +++ b/src/reactors/V3DutchOrderReactor.sol @@ -12,6 +12,7 @@ import {FixedPointMathLib} from "solmate/src/utils/FixedPointMathLib.sol"; import {MathExt} from "../lib/MathExt.sol"; import {Math} from "openzeppelin-contracts/utils/math/Math.sol"; import {CosignerLib} from "../lib/CosignerLib.sol"; +import {SafeCast} from "openzeppelin-contracts/utils/math/SafeCast.sol"; /// @notice Reactor for V3 dutch orders /// @dev V3 orders must be cosigned by the specified cosigner to override starting block and value @@ -109,26 +110,32 @@ contract V3DutchOrderReactor is BaseReactor { } function _updateWithGasAdjustment(V3DutchOrder memory order) internal view { - // positive means an increase in gas - int256 gasDeltaGwei = block.basefee.sub(order.startingBaseFee); + // Positive means an increase in gas + int256 gasDeltaWei = block.basefee.sub(order.startingBaseFee); - // Gas increase should increase input (round up when positive, down when negative) - int256 inputDelta = gasDeltaGwei >= 0 - ? (int256(order.baseInput.adjustmentPerGweiBaseFee) * gasDeltaGwei + 1 gwei - 1) / 1 gwei - : (int256(order.baseInput.adjustmentPerGweiBaseFee) * gasDeltaGwei) / 1 gwei; + // Round in favor of swapper + int256 inputDelta = _computeDelta(order.baseInput.adjustmentPerGweiBaseFee, gasDeltaWei); order.baseInput.startAmount = order.baseInput.startAmount.boundedAdd(inputDelta, 0, order.baseInput.maxAmount); - // Gas increase should decrease output (round down when positive, up when negative) uint256 outputsLength = order.baseOutputs.length; for (uint256 i = 0; i < outputsLength; i++) { V3DutchOutput memory output = order.baseOutputs[i]; - int256 outputDelta = gasDeltaGwei >= 0 - ? (int256(output.adjustmentPerGweiBaseFee) * gasDeltaGwei) / 1 gwei - : (int256(output.adjustmentPerGweiBaseFee) * gasDeltaGwei - 1 gwei + 1) / 1 gwei; + // Round in favor of swapper + int256 outputDelta = _computeDelta(output.adjustmentPerGweiBaseFee, gasDeltaWei); output.startAmount = output.startAmount.boundedSub(outputDelta, output.minAmount, type(uint256).max); } } + function _computeDelta(uint256 adjustmentPerGweiBaseFee, int256 gasDeltaWei) internal pure returns (int256) { + if (gasDeltaWei >= 0) { + // Gas increase: round adjustment down to decrease amount added to input or subtracted from output + return int256(adjustmentPerGweiBaseFee.mulDivDown(uint256(gasDeltaWei), 1 gwei)); + } else { + // Gas decrease: round adjustment up to increase amount added to output or subtracted from input + return -int256(adjustmentPerGweiBaseFee.mulDivUp(SafeCast.toUint256(-gasDeltaWei), 1 gwei)); + } + } + /// @notice validate the dutch order fields /// - deadline must have not passed /// - cosigner is valid if specified diff --git a/test/reactors/V3DutchOrderReactor.t.sol b/test/reactors/V3DutchOrderReactor.t.sol index 53a54d5b..fed2cf70 100644 --- a/test/reactors/V3DutchOrderReactor.t.sol +++ b/test/reactors/V3DutchOrderReactor.t.sol @@ -1406,47 +1406,49 @@ contract V3DutchOrderTest is PermitSignature, DeployPermit2, BaseReactorTest { vm.roll(currentBlock); vm.fee(1 gwei); - // Order with 0.8 gwei gas adjustments + // Order with 1 wei gas adjustments SignedOrder memory order = generateOrder( TestDutchOrderSpec({ currentBlock: currentBlock, startBlock: currentBlock, deadline: currentBlock + 21, - input: V3DutchInput(tokenIn, 100 ether, CurveBuilder.emptyCurve(), 101 ether, 0.8 gwei), + input: V3DutchInput(tokenIn, 1000, CurveBuilder.emptyCurve(), 1100, 1), outputs: OutputsBuilder.singleV3Dutch( - V3DutchOutput(address(tokenOut), 100 ether, CurveBuilder.emptyCurve(), address(0), 99 ether, 0.8 gwei) + V3DutchOutput(address(tokenOut), 1000, CurveBuilder.emptyCurve(), address(0), 900, 1) ) }) ); // Test gas increase - vm.fee(2 gwei); + vm.fee(1.5 gwei); ResolvedOrder memory resolvedOrder = quoter.quote(order.order, order.sig); - assertEq(resolvedOrder.input.amount, 100 ether + 0.8 gwei, "Input should round up for positive gas change"); - assertEq(resolvedOrder.outputs[0].amount, 100 ether - 0.8 gwei, "Output should round down for positive gas change"); + // The gas adjusted input would be 1000.5, which should round down to 1000 + assertEq(resolvedOrder.input.amount, 1000, "Input should round down"); + // The gas adjusted output would be 999.5, which should round up to 1000 + assertEq(resolvedOrder.outputs[0].amount, 1000, "Output should round up"); // Test gas decrease vm.fee(0.5 gwei); resolvedOrder = quoter.quote(order.order, order.sig); - assertEq(resolvedOrder.input.amount, 100 ether - 0.4 gwei, "Input should round down for negative gas change"); - assertEq(resolvedOrder.outputs[0].amount, 100 ether + 0.4 gwei, "Output should round up for negative gas change"); - - // Test gas half - vm.fee(0.5 gwei); - resolvedOrder = quoter.quote(order.order, order.sig); - assertEq(resolvedOrder.input.amount, 100 ether - 0.4 gwei, "Input should round down for gas halving change"); - assertEq(resolvedOrder.outputs[0].amount, 100 ether + 0.4 gwei, "Output should round up for exact gas halving change"); + // The gas adjusted input would be 999.5, which should round down to 999 + assertEq(resolvedOrder.input.amount, 999, "Input should round down"); + // The gas adjusted output would be 1000.5, which should round up to 1001 + assertEq(resolvedOrder.outputs[0].amount, 1001, "Output should round up"); // Test smaller gas changes vm.fee(1.1 gwei); resolvedOrder = quoter.quote(order.order, order.sig); - assertEq(resolvedOrder.input.amount, 100 ether + 0.08 gwei, "Input should handle small positive gas changes"); - assertEq(resolvedOrder.outputs[0].amount, 100 ether - 0.08 gwei, "Output should handle small positive gas changes"); + // The gas adjusted input would be 1000.1, which should round down to 1000 + assertEq(resolvedOrder.input.amount, 1000, "Input should round down"); + // The gas adjusted output would be 999.9, which should round up to 1000 + assertEq(resolvedOrder.outputs[0].amount, 1000, "Output should round up"); vm.fee(0.9 gwei); resolvedOrder = quoter.quote(order.order, order.sig); - assertEq(resolvedOrder.input.amount, 100 ether - 0.08 gwei, "Input should handle small negative gas changes"); - assertEq(resolvedOrder.outputs[0].amount, 100 ether + 0.08 gwei, "Output should handle small negative gas changes"); + // The gas adjusted input would be 999.9, which should round down to 999 + assertEq(resolvedOrder.input.amount, 999, "Input should round down"); + // The gas adjusted output would be 1000.1, which should round up to 1001 + assertEq(resolvedOrder.outputs[0].amount, 1001, "Output should round up"); } /* Test helpers */ From 14d77df1d039e19c0ed528a8d5122319b563d44f Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Fri, 4 Oct 2024 14:44:39 -0400 Subject: [PATCH 3/4] refactor: remove SafeCast --- src/reactors/V3DutchOrderReactor.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/reactors/V3DutchOrderReactor.sol b/src/reactors/V3DutchOrderReactor.sol index 1f586c09..ac20f2a6 100644 --- a/src/reactors/V3DutchOrderReactor.sol +++ b/src/reactors/V3DutchOrderReactor.sol @@ -12,7 +12,6 @@ import {FixedPointMathLib} from "solmate/src/utils/FixedPointMathLib.sol"; import {MathExt} from "../lib/MathExt.sol"; import {Math} from "openzeppelin-contracts/utils/math/Math.sol"; import {CosignerLib} from "../lib/CosignerLib.sol"; -import {SafeCast} from "openzeppelin-contracts/utils/math/SafeCast.sol"; /// @notice Reactor for V3 dutch orders /// @dev V3 orders must be cosigned by the specified cosigner to override starting block and value @@ -132,7 +131,7 @@ contract V3DutchOrderReactor is BaseReactor { return int256(adjustmentPerGweiBaseFee.mulDivDown(uint256(gasDeltaWei), 1 gwei)); } else { // Gas decrease: round adjustment up to increase amount added to output or subtracted from input - return -int256(adjustmentPerGweiBaseFee.mulDivUp(SafeCast.toUint256(-gasDeltaWei), 1 gwei)); + return -int256(adjustmentPerGweiBaseFee.mulDivUp(uint256(-gasDeltaWei), 1 gwei)); } } From cbf2a8dc944738421ad3bc69795f58b688385265 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 16 Oct 2024 12:00:00 -0400 Subject: [PATCH 4/4] style: forge fmt --- src/reactors/V3DutchOrderReactor.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/reactors/V3DutchOrderReactor.sol b/src/reactors/V3DutchOrderReactor.sol index afdd46cf..95faae6a 100644 --- a/src/reactors/V3DutchOrderReactor.sol +++ b/src/reactors/V3DutchOrderReactor.sol @@ -115,7 +115,8 @@ contract V3DutchOrderReactor is BaseReactor { if (order.baseInput.adjustmentPerGweiBaseFee != 0) { // Round in favor of swapper int256 inputDelta = _computeDelta(order.baseInput.adjustmentPerGweiBaseFee, gasDeltaWei); - order.baseInput.startAmount = order.baseInput.startAmount.boundedAdd(inputDelta, 0, order.baseInput.maxAmount); + order.baseInput.startAmount = + order.baseInput.startAmount.boundedAdd(inputDelta, 0, order.baseInput.maxAmount); } // Gas increase should decrease output uint256 outputsLength = order.baseOutputs.length;