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

[s24 | s25 | s29] Internal audit part 1 #55

Merged
merged 4 commits into from
Nov 27, 2024
Merged

[s24 | s25 | s29] Internal audit part 1 #55

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
a57edd7
Internal-S24: remove settle_take_pair
ChefMist Nov 26, 2024
49b42fa
internal-s25: set sync before native settle
ChefMist Nov 26, 2024
cc6ac3c
internal-s29 do not revert for initializePool
ChefMist Nov 27, 2024
8d72c0e
test: fix test
ChefMist Nov 27, 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/BinMigratorFromPancakeswapV2Test#testMigrateFromV2IncludingInit.snap
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually this means the pool is already initialized, should we just return slot0.tick ?

Copy link
Contributor

Choose a reason for hiding this comment

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

considered contract size , i agree with the current max logic.
if not , it is better to query latest tick.

}
}

/// @inheritdoc IPositionManager
Expand Down
2 changes: 2 additions & 0 deletions src/pool-cl/interfaces/ICLPositionManager.sol
View file
Open in desktop
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