Skip to content

Commit 6c26dfc

Browse files
deluca-mikedeluca-mike
committed
fix: Slippage, Tick Ranges, Attacks
1 parent 83edcd1 commit 6c26dfc

File tree

4 files changed

+914
-260
lines changed

4 files changed

+914
-260
lines changed

‎src/MainnetController.sol‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,9 @@ contract MainnetController is ReentrancyGuard, AccessControlEnumerable {
9797
}
9898

9999
struct UniswapV4Limits {
100-
int24 tickLowerMin;
101-
int24 tickUpperMax;
100+
int24 tickLowerMin;
101+
int24 tickUpperMax;
102+
uint24 maxTickRange;
102103
}
103104

104105
/**********************************************************************************************/
@@ -135,7 +136,7 @@ contract MainnetController is ReentrancyGuard, AccessControlEnumerable {
135136
bool isWhitelisted
136137
);
137138
event RelayerRemoved(address indexed relayer);
138-
event UniswapV4TickLimitsSet(bytes32 indexed poolId, int24 tickLowerMin, int24 tickUpperMax);
139+
event UniswapV4TickLimitsSet(bytes32 indexed poolId, int24 tickLowerMin, int24 tickUpperMax, uint24 maxTickRange);
139140

140141
/**********************************************************************************************/
141142
/*** State variables ***/
@@ -322,7 +323,8 @@ contract MainnetController is ReentrancyGuard, AccessControlEnumerable {
322323
function setUniswapV4TickLimits(
323324
bytes32 poolId,
324325
int24 tickLowerMin,
325-
int24 tickUpperMax
326+
int24 tickUpperMax,
327+
uint24 maxTickRange
326328
)
327329
external nonReentrant
328330
{
@@ -332,10 +334,11 @@ contract MainnetController is ReentrancyGuard, AccessControlEnumerable {
332334

333335
uniswapV4Limits[poolId] = UniswapV4Limits({
334336
tickLowerMin : tickLowerMin,
335-
tickUpperMax : tickUpperMax
337+
tickUpperMax : tickUpperMax,
338+
maxTickRange : maxTickRange
336339
});
337340

338-
emit UniswapV4TickLimitsSet(poolId, tickLowerMin, tickUpperMax);
341+
emit UniswapV4TickLimitsSet(poolId, tickLowerMin, tickUpperMax, maxTickRange);
339342
}
340343

341344
/**********************************************************************************************/
@@ -650,14 +653,18 @@ contract MainnetController is ReentrancyGuard, AccessControlEnumerable {
650653
require(tickLower >= limits.tickLowerMin, "MC/tickLower-too-low");
651654
require(tickUpper <= limits.tickUpperMax, "MC/tickUpper-too-high");
652655

656+
require(
657+
uint256(int256(tickUpper) - int256(tickLower)) <= limits.maxTickRange,
658+
"MC/tickRange-too-wide"
659+
);
660+
653661
// NOTE: `maxSlippages` is a mapping from address to uint256, so we have to take the lower
654662
// 160 bits of the id. It is possible, but highly unlikely there is a collision.
655663
UniswapV4Lib.mintPosition({
656664
commonParams: UniswapV4Lib.CommonParams({
657665
proxy : address(proxy),
658666
rateLimits : address(rateLimits),
659667
rateLimitId : LIMIT_UNISWAP_V4_DEPOSIT,
660-
maxSlippage : maxSlippages[address(uint160(uint256(poolId)))],
661668
poolId : poolId
662669
}),
663670
tickLower : tickLower,
@@ -686,7 +693,6 @@ contract MainnetController is ReentrancyGuard, AccessControlEnumerable {
686693
proxy : address(proxy),
687694
rateLimits : address(rateLimits),
688695
rateLimitId : LIMIT_UNISWAP_V4_DEPOSIT,
689-
maxSlippage : maxSlippages[address(uint160(uint256(poolId)))],
690696
poolId : poolId
691697
}),
692698
tokenId : tokenId,
@@ -711,7 +717,6 @@ contract MainnetController is ReentrancyGuard, AccessControlEnumerable {
711717
proxy : address(proxy),
712718
rateLimits : address(rateLimits),
713719
rateLimitId : LIMIT_UNISWAP_V4_WITHDRAW,
714-
maxSlippage : maxSlippages[address(uint160(uint256(poolId)))],
715720
poolId : poolId
716721
}),
717722
tokenId : tokenId,
@@ -736,7 +741,6 @@ contract MainnetController is ReentrancyGuard, AccessControlEnumerable {
736741
proxy : address(proxy),
737742
rateLimits : address(rateLimits),
738743
rateLimitId : LIMIT_UNISWAP_V4_WITHDRAW,
739-
maxSlippage : maxSlippages[address(uint160(uint256(poolId)))],
740744
poolId : poolId
741745
}),
742746
tokenId : tokenId,

‎src/interfaces/UniswapV4.sol‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ interface IPositionManagerLike {
1212
uint256 tokenId
1313
) external view returns (PoolKey memory poolKey, PositionInfo info);
1414

15-
function poolKeys(bytes25 poolId) external view returns (PoolKey memory poolKeys);
15+
function poolKeys(bytes25 poolId) external view returns (PoolKey memory poolKey);
1616

1717
function modifyLiquidities(bytes calldata unlockData, uint256 deadline) external payable;
1818

‎src/libraries/UniswapV4Lib.sol‎

Lines changed: 6 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,12 @@ library UniswapV4Lib {
1919
address proxy;
2020
address rateLimits;
2121
bytes32 rateLimitId;
22-
uint256 maxSlippage;
2322
bytes32 poolId; // the PoolId of the Uniswap V4 pool
2423
}
2524

2625
// NOTE: From https://docs.uniswap.org/contracts/v4/deployments
2726
address internal constant _PERMIT2 = 0x000000000022D473030F116dDEE9F6B43aC78BA3;
2827
address internal constant _POSITION_MANAGER = 0xbD216513d74C8cf14cf4747E6AaA6420FF64ee9e;
29-
address internal constant _STATE_VIEW = 0x7fFE42C4a5DEeA5b0feC41C94C136Cf115597227;
3028

3129
/**********************************************************************************************/
3230
/*** Interactive Functions ***/
@@ -259,12 +257,10 @@ library UniswapV4Lib {
259257
bytes memory actions,
260258
bytes[] memory params
261259
) internal returns (uint256 rateLimitDecrease) {
262-
_requireNonZeroMaxSlippage(commonParams);
263-
264260
_approvePositionManager(commonParams.proxy, token0, amount0Max);
265261
_approvePositionManager(commonParams.proxy, token1, amount1Max);
266262

267-
// Get token balances before mint.
263+
// Get token balances before liquidity increase.
268264
uint256 startingBalance0 = _getBalance(token0, commonParams.proxy);
269265
uint256 startingBalance1 = _getBalance(token1, commonParams.proxy);
270266

@@ -277,23 +273,10 @@ library UniswapV4Lib {
277273
)
278274
);
279275

280-
// Get token balances after mint.
276+
// Get token balances after liquidity increase.
281277
uint256 endingBalance0 = _getBalance(token0, commonParams.proxy);
282278
uint256 endingBalance1 = _getBalance(token1, commonParams.proxy);
283279

284-
// Ensure the amountMax is below the allowed worst case scenario (amount / maxSlippage).
285-
require(
286-
amount0Max * commonParams.maxSlippage <=
287-
_clampedSub(startingBalance0, endingBalance0) * 1e18,
288-
"MC/amount0Max-too-high"
289-
);
290-
291-
require(
292-
amount1Max * commonParams.maxSlippage <=
293-
_clampedSub(startingBalance1, endingBalance1) * 1e18,
294-
"MC/amount1Max-too-high"
295-
);
296-
297280
// Account for the theoretical possibility of receiving tokens when adding liquidity by
298281
// using a clamped subtraction.
299282
// NOTE: The limitation of this integration is the assumption that the tokens are valued
@@ -328,9 +311,7 @@ library UniswapV4Lib {
328311
bytes memory actions,
329312
bytes[] memory params
330313
) internal returns (uint256 rateLimitDecrease) {
331-
_requireNonZeroMaxSlippage(commonParams);
332-
333-
// Get token balances before mint.
314+
// Get token balances before liquidity decrease.
334315
uint256 startingBalance0 = _getBalance(token0, commonParams.proxy);
335316
uint256 startingBalance1 = _getBalance(token1, commonParams.proxy);
336317

@@ -343,28 +324,15 @@ library UniswapV4Lib {
343324
)
344325
);
345326

346-
// Get token balances after mint.
327+
// Get token balances after liquidity decrease.
347328
uint256 endingBalance0 = _getBalance(token0, commonParams.proxy);
348329
uint256 endingBalance1 = _getBalance(token1, commonParams.proxy);
349330

350-
// Ensure the amountMin is above the allowed worst case scenario (amount * maxSlippage).
351-
require(
352-
amount0Min * 1e18 >= (endingBalance0 - startingBalance0) * commonParams.maxSlippage,
353-
"MC/amount0Min-too-small"
354-
);
355-
356-
require(
357-
amount1Min * 1e18 >= (endingBalance1 - startingBalance1) * commonParams.maxSlippage,
358-
"MC/amount1Min-too-small"
359-
);
360-
361331
// NOTE: The limitation of this integration is the assumption that the tokens are valued
362332
// equally (i.e. 1.00000 USDC = 1.000000000000000000 USDS).
363333
rateLimitDecrease =
364-
_getNormalizedBalance(token0, endingBalance0) +
365-
_getNormalizedBalance(token1, endingBalance1) -
366-
_getNormalizedBalance(token0, startingBalance0) -
367-
_getNormalizedBalance(token1, startingBalance1);
334+
_getNormalizedBalance(token0, endingBalance0 - startingBalance0) +
335+
_getNormalizedBalance(token1, endingBalance1 - startingBalance1);
368336

369337
// Perform rate limit decrease.
370338
IRateLimits(commonParams.rateLimits).triggerRateLimitDecrease(
@@ -396,10 +364,6 @@ library UniswapV4Lib {
396364
return IPositionManagerLike(_POSITION_MANAGER).poolKeys(bytes25(poolId));
397365
}
398366

399-
function _requireNonZeroMaxSlippage(CommonParams calldata commonParams) internal pure {
400-
require(commonParams.maxSlippage != 0, "MC/maxSlippage-not-set");
401-
}
402-
403367
function _requirePoolIdMatch(bytes32 poolId, uint256 tokenId) internal view {
404368
( PoolKey memory poolKey, ) = IPositionManagerLike(_POSITION_MANAGER).getPoolAndPositionInfo(tokenId);
405369

0 commit comments

Comments
 (0)