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.

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

Spearbit 89 Unsafe casting and overflow of negation #341

Merged
merged 4 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129459
129465
Original file line number Diff line number Diff line change
@@ -1 +1 @@
131377
131383
Original file line number Diff line number Diff line change
@@ -1 +1 @@
123612
123588
Original file line number Diff line number Diff line change
@@ -1 +1 @@
123754
123730
Original file line number Diff line number Diff line change
@@ -1 +1 @@
154735
154710
Original file line number Diff line number Diff line change
@@ -1 +1 @@
153793
153768
Original file line number Diff line number Diff line change
@@ -1 +1 @@
136186
136161
Original file line number Diff line number Diff line change
@@ -1 +1 @@
132042
132017
Original file line number Diff line number Diff line change
@@ -1 +1 @@
172746
172721
Original file line number Diff line number Diff line change
@@ -1 +1 @@
143428
143403
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
339884
339859
Original file line number Diff line number Diff line change
@@ -1 +1 @@
348264
348239
Original file line number Diff line number Diff line change
@@ -1 +1 @@
347630
347605
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_onSameTickLower.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
318202
318177
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_onSameTickUpper.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
318872
318847
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_sameRange.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
244441
244416
Original file line number Diff line number Diff line change
@@ -1 +1 @@
374127
374102
Original file line number Diff line number Diff line change
@@ -1 +1 @@
324233
324208
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_withClose.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
375533
375508
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_mint_withSettlePair.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
374733
374708
Original file line number Diff line number Diff line change
@@ -1 +1 @@
419680
419655
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_Bytecode.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7107
7063
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn1Hop_nativeIn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
115155
115185
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn1Hop_nativeOut.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
115632
115662
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn1Hop_oneForZero.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
124447
124477
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn1Hop_zeroForOne.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
130165
130195
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn2Hops.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
179173
179203
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn2Hops_nativeIn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
169873
169903
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn3Hops.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
228160
228190
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactIn3Hops_nativeIn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
218884
218914
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactInputSingle.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129459
129465
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactInputSingle_nativeIn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
114449
114455
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactInputSingle_nativeOut.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
114895
114901
Original file line number Diff line number Diff line change
@@ -1 +1 @@
121387
121413
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut1Hop_nativeOut.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
116688
116714
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut1Hop_oneForZero.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
125503
125529
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut1Hop_zeroForOne.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129420
129446
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut2Hops.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
179252
179274
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut2Hops_nativeIn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
175136
175158
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut3Hops.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
229091
229109
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut3Hops_nativeIn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
224999
225017
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOut3Hops_nativeOut.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
220300
220318
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOutputSingle.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
128698
128716
Original file line number Diff line number Diff line change
@@ -1 +1 @@
120665
120683
2 changes: 1 addition & 1 deletion .forge-snapshots/V4Router_ExactOutputSingle_nativeOut.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
116041
116059
2 changes: 2 additions & 0 deletions src/PositionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ contract PositionManager is
// the locker is the payer or receiver
address caller = msgSender();
if (currencyDelta < 0) {
// Casting is safe due to limits on the total supply of a pool
_settle(currency, caller, uint256(-currencyDelta));
} else if (currencyDelta > 0) {
_take(currency, caller, uint256(currencyDelta));
Expand Down Expand Up @@ -417,6 +418,7 @@ contract PositionManager is
if (payer == address(this)) {
currency.transfer(address(poolManager), amount);
} else {
// Casting from uint256 to uint160 is safe due to limits on the total supply of a pool
permit2.transferFrom(payer, address(poolManager), uint160(amount), Currency.unwrap(currency));
}
}
Expand Down
18 changes: 14 additions & 4 deletions src/V4Router.sol
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver {
_getFullCredit(params.zeroForOne ? params.poolKey.currency0 : params.poolKey.currency1).toUint128();
}
uint128 amountOut = _swap(
params.poolKey, params.zeroForOne, int256(-int128(amountIn)), params.sqrtPriceLimitX96, params.hookData
params.poolKey, params.zeroForOne, -int256(uint256(amountIn)), params.sqrtPriceLimitX96, params.hookData
).toUint128();
if (amountOut < params.amountOutMinimum) revert V4TooLittleReceived(params.amountOutMinimum, amountOut);
}
Expand Down Expand Up @@ -127,8 +127,16 @@ abstract contract V4Router is IV4Router, BaseActionsRouter, DeltaResolver {
_getFullDebt(params.zeroForOne ? params.poolKey.currency1 : params.poolKey.currency0).toUint128();
}
uint128 amountIn = (
-_swap(
params.poolKey, params.zeroForOne, int256(int128(amountOut)), params.sqrtPriceLimitX96, params.hookData
uint256(
-int256(
_swap(
params.poolKey,
params.zeroForOne,
int256(uint256(amountOut)),
params.sqrtPriceLimitX96,
params.hookData
)
)
)
).toUint128();
if (amountIn > params.amountInMaximum) revert V4TooMuchRequested(params.amountInMaximum, amountIn);
Expand All @@ -151,7 +159,9 @@ 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 = (-_swap(poolKey, !oneForZero, int256(uint256(amountOut)), 0, pathKey.hookData)).toUint128();
amountIn = (
uint256(-int256(_swap(poolKey, !oneForZero, int256(uint256(amountOut)), 0, pathKey.hookData)))
).toUint128();

amountOut = amountIn;
currencyOut = pathKey.intermediateCurrency;
Expand Down
1 change: 1 addition & 0 deletions src/base/DeltaResolver.sol
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ abstract contract DeltaResolver is ImmutableState {
int256 _amount = poolManager.currencyDelta(address(this), currency);
// If the amount is positive, it should be taken not settled.
if (_amount > 0) revert DeltaNotNegative(currency);
// Casting is safe due to limits on the total supply of a pool
amount = uint256(-_amount);
}

Expand Down
12 changes: 8 additions & 4 deletions src/libraries/SlippageCheck.sol
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,13 @@ library SlippageCheck {
// Thus, we only cast the delta if it is guaranteed to be negative.
// And we do NOT revert in the positive delta case. Since a positive delta means the hook is crediting tokens to the user for minting/increasing liquidity, we do not check slippage.
// This means this contract will NOT support _positive_ slippage checks (minAmountOut checks) on pools where the hook returns a positive delta on mint/increase.
int128 amount0 = delta.amount0();
int128 amount1 = delta.amount1();
if (amount0 < 0 && amount0Max < uint128(-amount0)) revert MaximumAmountExceeded(amount0Max, uint128(-amount0));
if (amount1 < 0 && amount1Max < uint128(-amount1)) revert MaximumAmountExceeded(amount1Max, uint128(-amount1));
int256 amount0 = delta.amount0();
int256 amount1 = delta.amount1();
if (amount0 < 0 && amount0Max < uint128(uint256(-amount0))) {
revert MaximumAmountExceeded(amount0Max, uint128(uint256(-amount0)));
}
if (amount1 < 0 && amount1Max < uint128(uint256(-amount1))) {
revert MaximumAmountExceeded(amount1Max, uint128(uint256(-amount1)));
}
}
}
Loading