V4 BinMigrator #59
Merged
V4 BinMigrator #59
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chefburger
force-pushed
the
feat/binPool-migrator
branch
from
4bb0e82 to
7971c3b
Compare
chefburger
force-pushed
the
feat/binPool-migrator
branch
from
f1fb45e to
c48480b
Compare
chefburger
force-pushed
the
feat/binPool-migrator
branch
from
c48480b to
3c0c42d
Compare
ChefSnoopy
reviewed
Jul 12, 2024
ChefMist
reviewed
Jul 13, 2024
ChefMist
reviewed
Jul 13, 2024
ChefMist
reviewed
Jul 13, 2024
| binFungiblePositionManager = IBinFungiblePositionManager(_binFungiblePositionManager); | ||
| } | ||
|
|
||
| function migrateFromV2( |
There was a problem hiding this comment.
likewise can we add more natspec to all these function as well to make it easier for other devs
chefburger
force-pushed
the
feat/binPool-migrator
branch
from
e700e77 to
2208034
Compare
ChefSnoopy
reviewed
Jul 15, 2024
| /// @notice compare if tokens from v3 pool are the same as token0/token1. Revert with | ||
| /// `TOKEN_NOT_MATCH` if tokens does not match | ||
| function checkTokenMatchFromV3(address nfp, uint256 tokenId, Currency token0, Currency token1) internal view { | ||
| (,, address token0V3, address token1V3,,,,,,,,) = IV3NonfungiblePositionManager(nfp).positions(tokenId); |
There was a problem hiding this comment.
Because the NFT position storage is too big , now we will query twice in checkTokenMatchFromV3 and withdrawLiquidityFromV3.
we should only query the NFT position once , can save almost 4k Gas.
Did a POC commit.
pancakeswap/pancake-v4-periphery@3f5afe6#diff-1a378b56bc0f605ffd85c5c0d02bae3dd8e2b735213e76b74f84719322dbd454
There was a problem hiding this comment.
Oh yea, agree. Thx for reminding. let me draft an update for this
ChefSnoopy
reviewed
Jul 15, 2024
| /// `TOKEN_NOT_MATCH` if tokens does not match | ||
| function checkTokenMatchFromV2(address v2Pair, Currency token0, Currency token1) internal view { | ||
| address token0V2 = IPancakePair(v2Pair).token0(); | ||
| address token1V2 = IPancakePair(v2Pair).token1(); |
There was a problem hiding this comment.
Same for the IPancakePair(v2Pair).token1(); did not test how muck gas will cost.
But it is better to query once.
ChefSnoopy
approved these changes
Jul 15, 2024
ChefMist
approved these changes
Jul 16, 2024
* feat: support selfPermitForERC721 * docs: added comments suggesting users to use selfPermitERC721IfNecessary * test: added tests to prevent ppl from removing payable keyword from external functions
chefburger
added a commit
that referenced
this pull request
Jul 18, 2024
* feat: implement v4Migrator for cl-pool * feat: added same price slippage check for migrateFromV2 * feat: using actual receive amount when adding liquidity to v4 * test: added test for clMigrator * fix: take into consideration of extra funds when calc refund * optimization: avoid unnecessary approve in old token cases * optimization: avoid unnecessary refund call after adding liquidity * optimization: avoid unnecessary WETH unwrap * fix: extra check so that non-owner can not steal funds from lpV3 token * typo * refactor: restructure baseMigrator a bit so that more can be reused across pool type * chore: typo and renaming * V4 BinMigrator (#59) * feat: impl binPool migrator * refactor: restructure & renaming accordingly * test: add tests cases for binPool migrator * chore: renaming as well to align with clMigrator * fix: add check to prevent token mismatch between source and target pool * feat: added refundETH function and necessary comments * optimization: avoid duplicate external call when query v2/v3 pool info * feat: support selfPermitForERC721 (#62) * feat: support selfPermitForERC721 * docs: added comments suggesting users to use selfPermitERC721IfNecessary * test: added tests to prevent ppl from removing payable keyword from external functions * docs: add explanation about the case where extra token0 is ETH
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Same as pancakeswap/pancake-v4-periphery#51 but for bin-pool