Skip to content

Commit

Permalink
[s24 | s25 | s29] Internal audit part 1 (#55)
Browse files Browse the repository at this point in the history 
* Internal-S24: remove settle_take_pair

* internal-s25: set sync before native settle

* internal-s29 do not revert for initializePool

* test: fix test
  • Loading branch information
@ChefMist
ChefMist authored Nov 27, 2024
1 parent 2816ff4 commit 74529e4
Show file tree
Hide file tree
Showing 68 changed files with 140 additions and 85 deletions.
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMigratorFromPancakeswapV2Test#testMigrateFromV2IncludingInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1184970
1186286
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMigratorFromPancakeswapV2Test#testMigrateFromV2WithoutInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1051689
1053024
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMigratorFromPancakeswapV2Test#testMigrateFromV2WithoutNativeToken.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1093853
1093821
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMigratorFromPancakeswapV3Test#testMigrateFromV3IncludingInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1257324
1258640
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMigratorFromPancakeswapV3Test#testMigrateFromV3WithoutInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1124132
1125467
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMigratorFromPancakeswapV3Test#testMigrateFromV3WithoutNativeToken.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1160290
1160258
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMigratorFromUniswapV2Test#testMigrateFromV2IncludingInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1184970
1186286
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMigratorFromUniswapV2Test#testMigrateFromV2WithoutInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1051689
1053024
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMigratorFromUniswapV2Test#testMigrateFromV2WithoutNativeToken.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1093849
1093817
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMigratorFromUniswapV3Test#testMigrateFromV3IncludingInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1255306
1256622
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMigratorFromUniswapV3Test#testMigrateFromV3WithoutInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1122114
1123449
2 changes: 1 addition & 1 deletion .forge-snapshots/BinMigratorFromUniswapV3Test#testMigrateFromV3WithoutNativeToken.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1158268
1158236
2 changes: 1 addition & 1 deletion .../BinPositionManager_ModifyLiquidityTest#test_addLiquidity_OutsideActiveId_ExistingId.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
297801
297785
2 changes: 1 addition & 1 deletion ...shots/BinPositionManager_ModifyLiquidityTest#test_addLiquidity_OutsideActiveId_NewId.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1136586
1136570
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPositionManager_ModifyLiquidityTest#test_addLiquidity_SingleBin.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
541580
541548
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPositionManager_ModifyLiquidityTest#test_addLiquidity_ThreeBins.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
916664
916632
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPositionManager_NativeTokenTest#test_addLiquidity.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
866787
868122
2 changes: 1 addition & 1 deletion .forge-snapshots/BinSwapRouterTest#testExactInputSingle_DifferentRecipient.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
142818
142488
2 changes: 1 addition & 1 deletion .forge-snapshots/BinSwapRouterTest#testExactInputSingle_EthPool_SwapEthForToken.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
136470
137514
2 changes: 1 addition & 1 deletion .forge-snapshots/BinSwapRouterTest#testExactInputSingle_EthPool_SwapTokenForEth.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
141892
141562
2 changes: 1 addition & 1 deletion .forge-snapshots/BinSwapRouterTest#testExactInputSingle_SwapForY_1.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
142814
142484
2 changes: 1 addition & 1 deletion .forge-snapshots/BinSwapRouterTest#testExactInputSingle_SwapForY_2.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
142874
142544
2 changes: 1 addition & 1 deletion .forge-snapshots/BinSwapRouterTest#testExactInput_MultiHopDifferentRecipient.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
174705
174111
2 changes: 1 addition & 1 deletion .forge-snapshots/BinSwapRouterTest#testExactOutputSingle_DifferentRecipient.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
147096
146766
2 changes: 1 addition & 1 deletion .forge-snapshots/BinSwapRouterTest#testExactOutputSingle_SwapForY_1.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
147092
146762
2 changes: 1 addition & 1 deletion .forge-snapshots/BinSwapRouterTest#testExactOutputSingle_SwapForY_2.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
147149
146819
2 changes: 1 addition & 1 deletion .forge-snapshots/BinSwapRouterTest#testExactOutput_MultiHopDifferentRecipient.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
178333
177739
2 changes: 1 addition & 1 deletion .forge-snapshots/BinSwapRouterTest#testExactOutput_SingleHop.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
148864
148450
2 changes: 1 addition & 1 deletion .forge-snapshots/CLMigratorFromPancakeswapV2Test#testCLMigrateFromV2IncludingInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
793533
794828
2 changes: 1 addition & 1 deletion .forge-snapshots/CLMigratorFromPancakeswapV2Test#testCLMigrateFromV2WithoutInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
658918
660253
2 changes: 1 addition & 1 deletion .forge-snapshots/CLMigratorFromPancakeswapV2Test#testCLMigrateFromV2WithoutNativeToken.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
730567
730535
2 changes: 1 addition & 1 deletion .forge-snapshots/CLMigratorFromPancakeswapV3Test#testCLMigrateFromV3IncludingInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
843440
844735
2 changes: 1 addition & 1 deletion .forge-snapshots/CLMigratorFromPancakeswapV3Test#testCLMigrateFromV3WithoutInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
711354
712689
2 changes: 1 addition & 1 deletion .forge-snapshots/CLMigratorFromPancakeswapV3Test#testCLMigrateFromV3WithoutNativeToken.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
753562
753530
2 changes: 1 addition & 1 deletion .forge-snapshots/CLMigratorFromUniswapV2Test#testCLMigrateFromV2IncludingInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
793533
794828
2 changes: 1 addition & 1 deletion .forge-snapshots/CLMigratorFromUniswapV2Test#testCLMigrateFromV2WithoutInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
658918
660253
2 changes: 1 addition & 1 deletion .forge-snapshots/CLMigratorFromUniswapV2Test#testCLMigrateFromV2WithoutNativeToken.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
730567
730535
2 changes: 1 addition & 1 deletion .forge-snapshots/CLMigratorFromUniswapV3Test#testCLMigrateFromV3IncludingInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
841422
842717
2 changes: 1 addition & 1 deletion .forge-snapshots/CLMigratorFromUniswapV3Test#testCLMigrateFromV3WithoutInit.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
709336
710671
2 changes: 1 addition & 1 deletion .forge-snapshots/CLMigratorFromUniswapV3Test#testCLMigrateFromV3WithoutNativeToken.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
751544
751512
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_increaseLiquidity_erc20_withClose.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
220629
220597
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_increaseLiquidity_erc20_withSettlePair.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
219608
219576
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_increaseLiquidity_native.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
202610
203945
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_mint_native.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
538815
540150
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_mint_nativeWithSweep_withClose.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
547331
548666
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_mint_nativeWithSweep_withSettlePair.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
546594
547929
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_mint_onSameTickLower.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
393618
393586
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_mint_onSameTickUpper.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
394055
394023
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_mint_sameRange.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
302843
302811
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_mint_settleWithBalance_sweep.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
592910
592878
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_mint_warmedPool_differentRange.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
399330
399298
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_mint_withClose.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
594074
594042
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_mint_withSettlePair.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
593195
593163
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPositionManager_multicall_initialize_mint.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
652097
652025
2 changes: 1 addition & 1 deletion .forge-snapshots/CLSwapRouterTest#ExactInput.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
240154
239536
2 changes: 1 addition & 1 deletion .forge-snapshots/CLSwapRouterTest#ExactInputSingle.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
175400
175058
2 changes: 1 addition & 1 deletion .forge-snapshots/CLSwapRouterTest#ExactOutput.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
239054
238436
2 changes: 1 addition & 1 deletion .forge-snapshots/CLSwapRouterTest#ExactOutputSingle.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
175115
174773
7 changes: 1 addition & 6 deletions src/V4Router.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,7 @@ abstract contract V4Router is IV4Router, CLRouterBase, BinRouterBase, BaseAction
return;
}
} else {
if (action == Actions.SETTLE_TAKE_PAIR) {
(Currency settleCurrency, Currency takeCurrency) = params.decodeCurrencyPair();
_settle(settleCurrency, msgSender(), _getFullDebt(settleCurrency));
_take(takeCurrency, msgSender(), _getFullCredit(takeCurrency));
return;
} else if (action == Actions.SETTLE_ALL) {
if (action == Actions.SETTLE_ALL) {
(Currency currency, uint256 maxAmount) = params.decodeCurrencyAndUint256();
uint256 amount = _getFullDebt(currency);
if (amount > maxAmount) revert V4TooMuchRequested(maxAmount, amount);
Expand Down
3 changes: 2 additions & 1 deletion src/base/DeltaResolver.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ abstract contract DeltaResolver is ImmutableState {
/// @dev Returns early if the amount is 0
function _settle(Currency currency, address payer, uint256 amount) internal {
if (amount == 0) return;

vault.sync(currency);
if (currency.isNative()) {
vault.settle{value: amount}();
} else {
vault.sync(currency);
_pay(currency, payer, amount);
vault.settle();
}
Expand Down
1 change: 0 additions & 1 deletion src/libraries/Actions.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ library Actions {
uint256 constant TAKE_PORTION = 0x14;
uint256 constant TAKE_PAIR = 0x15;

uint256 constant SETTLE_TAKE_PAIR = 0x16;
uint256 constant CLOSE_CURRENCY = 0x17;
uint256 constant CLEAR_OR_TAKE = 0x18;
uint256 constant SWEEP = 0x19;
Expand Down
4 changes: 3 additions & 1 deletion src/libraries/Planner.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ library Planner {
returns (bytes memory)
{
if (takeRecipient == ActionConstants.MSG_SENDER) {
plan = plan.add(Actions.SETTLE_TAKE_PAIR, abi.encode(inputCurrency, outputCurrency));
// blindly settling and taking all, without slippage checks, isnt recommended in prod
plan = plan.add(Actions.SETTLE_ALL, abi.encode(inputCurrency, type(uint256).max));
plan = plan.add(Actions.TAKE_ALL, abi.encode(outputCurrency, 0));
} else {
plan = plan.add(Actions.SETTLE, abi.encode(inputCurrency, ActionConstants.OPEN_DELTA, true));
plan = plan.add(Actions.TAKE, abi.encode(outputCurrency, takeRecipient, ActionConstants.OPEN_DELTA));
Expand Down
3 changes: 2 additions & 1 deletion src/pool-bin/BinPositionManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ contract BinPositionManager is

/// @inheritdoc IBinPositionManager
function initializePool(PoolKey memory key, uint24 activeId) external payable {
binPoolManager.initialize(key, activeId);
/// @dev if the pool revert due to other error (currencyOutOfOrder etc..), then the follow-up action to the pool will still revert accordingly
try binPoolManager.initialize(key, activeId) {} catch {}
}

function msgSender() public view override returns (address) {
Expand Down
1 change: 1 addition & 0 deletions src/pool-bin/interfaces/IBinPositionManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ interface IBinPositionManager is IPositionManager {
function binPoolManager() external view returns (IBinPoolManager);

/// @notice Initialize a v4 PCS bin pool
/// @dev If the pool is already initialized, this function will not revert
/// @param key the PoolKey of the pool to initialize
/// @param activeId the active bin id of the pool
function initializePool(PoolKey memory key, uint24 activeId) external payable;
Expand Down
8 changes: 7 additions & 1 deletion src/pool-cl/CLPositionManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,13 @@ contract CLPositionManager is

/// @inheritdoc ICLPositionManager
function initializePool(PoolKey calldata key, uint160 sqrtPriceX96) external payable override returns (int24) {
return clPoolManager.initialize(key, sqrtPriceX96);
/// @dev Swallow any error. If the pool revert due to other errors eg. currencyOutOfOrder etc..,
/// then follow-up action to the pool will still revert accordingly
try clPoolManager.initialize(key, sqrtPriceX96) returns (int24 tick) {
return tick;
} catch {
return type(int24).max;
}
}

/// @inheritdoc IPositionManager
Expand Down
2 changes: 2 additions & 0 deletions src/pool-cl/interfaces/ICLPositionManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ interface ICLPositionManager is IPositionManager {
function clPoolManager() external view returns (ICLPoolManager);

/// @notice Initialize a v4 PCS cl pool
/// @dev If the pool is already initialized, this function will not revert and just return type(int24).max
/// @param key the PoolKey of the pool to initialize
/// @param sqrtPriceX96 the initial sqrtPriceX96 of the pool
/// @return tick The current tick of the pool
function initializePool(PoolKey calldata key, uint160 sqrtPriceX96) external payable returns (int24);

/// @notice Used to get the ID that will be used for the next minted liquidity position
Expand Down
Loading

0 comments on commit 74529e4

Please sign in to comment.