Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove sqrtpricelimit from v4 router #365

Merged
merged 4 commits into from
Oct 18, 2024
Merged

Remove sqrtpricelimit from v4 router #365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
627bea7
remove sqrtpricelimit from v4 router
hensha256 Oct 15, 2024
68ddc9c
Merge branch 'main' into remove-price-limit
hensha256 Oct 17, 2024
a4cd944
Merge branch 'main' into remove-price-limit
snreynolds Oct 17, 2024
2983234
add comment about sqrtpricelimit
hensha256 Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .forge-snapshots/Payments_swap_settleFromCaller_takeAllToMsgSender.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129854
129601
2 changes: 1 addition & 1 deletion .forge-snapshots/Payments_swap_settleFromCaller_takeAllToSpecifiedAddress.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
131905
131664
2 changes: 1 addition & 1 deletion .forge-snapshots/Payments_swap_settleWithBalance_takeAllToMsgSender.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
124110
123869
2 changes: 1 addition & 1 deletion .forge-snapshots/Payments_swap_settleWithBalance_takeAllToSpecifiedAddress.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
124252
124011
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_Bytecode.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7148
7037
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn1Hop_nativeIn.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
115722
115708
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn1Hop_nativeOut.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
116043
116029
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn1Hop_oneForZero.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
124861
124847
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn1Hop_zeroForOne.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
130584
130570
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn2Hops.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
185439
185411
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn2Hops_nativeIn.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
170577
170549
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn3Hops.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
240297
240255
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn3Hops_nativeIn.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
225435
225393
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactInputSingle.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129854
129601
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactInputSingle_nativeIn.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
114992
114739
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactInputSingle_nativeOut.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
115282
115028
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut1Hop_nativeIn_sweepETH.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
121985
121971
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut1Hop_nativeOut.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
117107
117093
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut1Hop_oneForZero.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
125925
125911
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut1Hop_zeroForOne.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129870
129856
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut2Hops.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
183787
183759
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut2Hops_nativeIn.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
175902
175874
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut3Hops.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
237735
237693
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut3Hops_nativeIn.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
229850
229808
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut3Hops_nativeOut.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
217090
217048
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOutputSingle.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129140
128884
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOutputSingle_nativeIn_sweepETH.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
121255
120999
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOutputSingle_nativeOut.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
116452
116195
42 changes: 12 additions & 30 deletions src/V4Router.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,8 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver {
amountIn =
_getFullCredit(params.zeroForOne ? params.poolKey.currency0 : params.poolKey.currency1).toUint128();
}
uint128 amountOut = _swap(
params.poolKey, params.zeroForOne, -int256(uint256(amountIn)), params.sqrtPriceLimitX96, params.hookData
).toUint128();
uint128 amountOut =
_swap(params.poolKey, params.zeroForOne, -int256(uint256(amountIn)), params.hookData).toUint128();
if (amountOut < params.amountOutMinimum) revert V4TooLittleReceived(params.amountOutMinimum, amountOut);
}

Expand All @@ -110,7 +109,7 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver {
pathKey = params.path[i];
(PoolKey memory poolKey, bool zeroForOne) = pathKey.getPoolAndSwapDirection(currencyIn);
// The output delta will always be positive, except for when interacting with certain hook pools
amountOut = _swap(poolKey, zeroForOne, -int256(uint256(amountIn)), 0, pathKey.hookData).toUint128();
amountOut = _swap(poolKey, zeroForOne, -int256(uint256(amountIn)), pathKey.hookData).toUint128();

amountIn = amountOut;
currencyIn = pathKey.intermediateCurrency;
Expand All @@ -127,17 +126,7 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver {
_getFullDebt(params.zeroForOne ? params.poolKey.currency1 : params.poolKey.currency0).toUint128();
}
uint128 amountIn = (
uint256(
-int256(
_swap(
params.poolKey,
params.zeroForOne,
int256(uint256(amountOut)),
params.sqrtPriceLimitX96,
params.hookData
)
)
)
uint256(-int256(_swap(params.poolKey, params.zeroForOne, int256(uint256(amountOut)), params.hookData)))
).toUint128();
if (amountIn > params.amountInMaximum) revert V4TooMuchRequested(params.amountInMaximum, amountIn);
}
Expand All @@ -159,9 +148,8 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver {
pathKey = params.path[i - 1];
(PoolKey memory poolKey, bool oneForZero) = pathKey.getPoolAndSwapDirection(currencyOut);
// The output delta will always be negative, except for when interacting with certain hook pools
amountIn = (
uint256(-int256(_swap(poolKey, !oneForZero, int256(uint256(amountOut)), 0, pathKey.hookData)))
).toUint128();
amountIn = (uint256(-int256(_swap(poolKey, !oneForZero, int256(uint256(amountOut)), pathKey.hookData))))
.toUint128();

amountOut = amountIn;
currencyOut = pathKey.intermediateCurrency;
Expand All @@ -170,22 +158,16 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver {
}
}

function _swap(
PoolKey memory poolKey,
bool zeroForOne,
int256 amountSpecified,
uint160 sqrtPriceLimitX96,
bytes calldata hookData
) private returns (int128 reciprocalAmount) {
function _swap(PoolKey memory poolKey, bool zeroForOne, int256 amountSpecified, bytes calldata hookData)
private
returns (int128 reciprocalAmount)
{
// for protection of exactOut swaps, sqrtPriceLimit is not exposed as a feature in this contract
unchecked {
BalanceDelta delta = poolManager.swap(
poolKey,
IPoolManager.SwapParams(
zeroForOne,
amountSpecified,
sqrtPriceLimitX96 == 0
? (zeroForOne ? TickMath.MIN_SQRT_PRICE + 1 : TickMath.MAX_SQRT_PRICE - 1)
: sqrtPriceLimitX96
zeroForOne, amountSpecified, zeroForOne ? TickMath.MIN_SQRT_PRICE + 1 : TickMath.MAX_SQRT_PRICE - 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we leave a comment somewhere explaining motivation for this? Its an edge case for sure, but maybe like "for protection of end users, this contract automatically sets a maximum value for the sqrtPriceLimit, and slippage checks are relied on to enforce proper amount out checks" or something..

),
hookData
);
Expand Down
2 changes: 0 additions & 2 deletions src/interfaces/IV4Router.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ interface IV4Router is IImmutableState {
bool zeroForOne;
uint128 amountIn;
uint128 amountOutMinimum;
uint160 sqrtPriceLimitX96;
bytes hookData;
}

Expand All @@ -38,7 +37,6 @@ interface IV4Router is IImmutableState {
bool zeroForOne;
uint128 amountOut;
uint128 amountInMaximum;
uint160 sqrtPriceLimitX96;
bytes hookData;
}

Expand Down
2 changes: 0 additions & 2 deletions test/libraries/CalldataDecoder.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ contract CalldataDecoderTest is Test {
assertEq(swapParams.zeroForOne, _swapParams.zeroForOne);
assertEq(swapParams.amountIn, _swapParams.amountIn);
assertEq(swapParams.amountOutMinimum, _swapParams.amountOutMinimum);
assertEq(swapParams.sqrtPriceLimitX96, _swapParams.sqrtPriceLimitX96);
assertEq(swapParams.hookData, _swapParams.hookData);
_assertEq(swapParams.poolKey, _swapParams.poolKey);
}
Expand All @@ -128,7 +127,6 @@ contract CalldataDecoderTest is Test {
assertEq(swapParams.zeroForOne, _swapParams.zeroForOne);
assertEq(swapParams.amountOut, _swapParams.amountOut);
assertEq(swapParams.amountInMaximum, _swapParams.amountInMaximum);
assertEq(swapParams.sqrtPriceLimitX96, _swapParams.sqrtPriceLimitX96);
assertEq(swapParams.hookData, _swapParams.hookData);
_assertEq(swapParams.poolKey, _swapParams.poolKey);
}
Expand Down
8 changes: 4 additions & 4 deletions test/router/Payments.gas.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
function test_gas_swap_settleFromCaller_takeAllToSpecifiedAddress() public {
uint256 amountIn = 1 ether;
IV4Router.ExactInputSingleParams memory params =
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, 0, bytes(""));
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, bytes(""));

plan = plan.add(Actions.SWAP_EXACT_IN_SINGLE, abi.encode(params));
plan = plan.add(Actions.SETTLE_ALL, abi.encode(key0.currency0, MAX_SETTLE_AMOUNT));
Expand All @@ -36,7 +36,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
function test_gas_swap_settleFromCaller_takeAllToMsgSender() public {
uint256 amountIn = 1 ether;
IV4Router.ExactInputSingleParams memory params =
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, 0, bytes(""));
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, bytes(""));

plan = plan.add(Actions.SWAP_EXACT_IN_SINGLE, abi.encode(params));
plan = plan.add(Actions.SETTLE_TAKE_PAIR, abi.encode(key0.currency0, key0.currency1));
Expand All @@ -49,7 +49,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
function test_gas_swap_settleWithBalance_takeAllToSpecifiedAddress() public {
uint256 amountIn = 1 ether;
IV4Router.ExactInputSingleParams memory params =
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, 0, bytes(""));
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, bytes(""));

// seed the router with tokens
key0.currency0.transfer(address(router), amountIn);
Expand All @@ -66,7 +66,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
function test_gas_swap_settleWithBalance_takeAllToMsgSender() public {
uint256 amountIn = 1 ether;
IV4Router.ExactInputSingleParams memory params =
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, 0, bytes(""));
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, bytes(""));

// seed the router with tokens
key0.currency0.transfer(address(router), amountIn);
Expand Down
18 changes: 9 additions & 9 deletions test/router/Payments.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
uint256 amountIn = 1 ether;
uint256 expectedAmountOut = 992054607780215625;
IV4Router.ExactInputSingleParams memory params =
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, 0, bytes(""));
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, bytes(""));

plan = plan.add(Actions.SWAP_EXACT_IN_SINGLE, abi.encode(params));
plan = plan.add(Actions.SETTLE_TAKE_PAIR, abi.encode(key0.currency0, key0.currency1));
Expand Down Expand Up @@ -54,7 +54,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
function test_exactIn_settleAll_revertsSlippage() public {
uint256 amountIn = 1 ether;
IV4Router.ExactInputSingleParams memory params =
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, 0, bytes(""));
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, bytes(""));

plan = plan.add(Actions.SWAP_EXACT_IN_SINGLE, abi.encode(params));
plan = plan.add(Actions.SETTLE_ALL, abi.encode(key0.currency0, amountIn - 1));
Expand All @@ -69,7 +69,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
uint256 amountIn = 1 ether;
uint256 expectedAmountOut = 992054607780215625;
IV4Router.ExactInputSingleParams memory params =
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, 0, bytes(""));
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, bytes(""));

plan = plan.add(Actions.SWAP_EXACT_IN_SINGLE, abi.encode(params));
plan = plan.add(Actions.SETTLE_ALL, abi.encode(key0.currency0, MAX_SETTLE_AMOUNT));
Expand All @@ -87,7 +87,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
uint256 expectedAmountIn = 1008049273448486163;

IV4Router.ExactOutputSingleParams memory params =
IV4Router.ExactOutputSingleParams(key0, true, uint128(amountOut), uint128(expectedAmountIn), 0, bytes(""));
IV4Router.ExactOutputSingleParams(key0, true, uint128(amountOut), uint128(expectedAmountIn), bytes(""));

plan = plan.add(Actions.SWAP_EXACT_OUT_SINGLE, abi.encode(params));
plan = plan.add(Actions.SETTLE_ALL, abi.encode(key0.currency0, expectedAmountIn - 1));
Expand All @@ -105,7 +105,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
uint256 expectedAmountIn = 1008049273448486163;

IV4Router.ExactOutputSingleParams memory params =
IV4Router.ExactOutputSingleParams(key0, true, uint128(amountOut), uint128(expectedAmountIn), 0, bytes(""));
IV4Router.ExactOutputSingleParams(key0, true, uint128(amountOut), uint128(expectedAmountIn), bytes(""));

plan = plan.add(Actions.SWAP_EXACT_OUT_SINGLE, abi.encode(params));
plan = plan.add(Actions.SETTLE_ALL, abi.encode(key0.currency0, MAX_SETTLE_AMOUNT));
Expand All @@ -121,7 +121,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
uint256 expectedAmountIn = 1008049273448486163;

IV4Router.ExactOutputSingleParams memory params =
IV4Router.ExactOutputSingleParams(key0, true, uint128(amountOut), uint128(expectedAmountIn), 0, bytes(""));
IV4Router.ExactOutputSingleParams(key0, true, uint128(amountOut), uint128(expectedAmountIn), bytes(""));

plan = plan.add(Actions.SWAP_EXACT_OUT_SINGLE, abi.encode(params));
plan = plan.add(Actions.SETTLE_ALL, abi.encode(key0.currency0, expectedAmountIn));
Expand All @@ -135,7 +135,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
uint256 amountIn = 1 ether;
uint256 expectedAmountOut = 992054607780215625;
IV4Router.ExactInputSingleParams memory params =
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, 0, bytes(""));
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, bytes(""));

// seed the router with tokens
key0.currency0.transfer(address(router), amountIn);
Expand Down Expand Up @@ -169,7 +169,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
uint256 amountIn = 1 ether;
uint256 expectedAmountOut = 992054607780215625;
IV4Router.ExactInputSingleParams memory params =
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, 0, bytes(""));
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, bytes(""));

plan = plan.add(Actions.SWAP_EXACT_IN_SINGLE, abi.encode(params));
// take 15 bips to Bob
Expand Down Expand Up @@ -205,7 +205,7 @@ contract PaymentsTests is RoutingTestHelpers, GasSnapshot {
function test_settle_takePortion_reverts() public {
uint256 amountIn = 1 ether;
IV4Router.ExactInputSingleParams memory params =
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, 0, bytes(""));
IV4Router.ExactInputSingleParams(key0, true, uint128(amountIn), 0, bytes(""));

plan = plan.add(Actions.SWAP_EXACT_IN_SINGLE, abi.encode(params));
// bips is larger than maximum bips
Expand Down
Loading
Loading