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

feat: migrate univ3 badger/wbtc to badger/ebtc partially 50% #1546

Merged
merged 3 commits into from
Jun 20, 2024

Conversation

petrovska-petro
Copy link
Collaborator

tackles #1545

run:

brownie run scripts/issue/1545/bip_105_execution

@petrovska-petro
Copy link
Collaborator Author

petrovska-petro commented Jun 19, 2024

something i found tricky while coding the script is to not count for the fees collected while handling a mint afterwards. if while reviewing you find that optimisation to be more precise on the each mint, specially w/ focus on the active range. feel free to directly push the commit, since its relatively time sensitive for incoming signing session

discrepancy should be minimal in general terms, but ID 255188 has almost ~$20k of uncollected fees so may make a slightly difference

sajanrajdev
sajanrajdev previously approved these changes Jun 19, 2024
Copy link
Collaborator

@sajanrajdev sajanrajdev left a comment

Choose a reason for hiding this comment

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

Review

  • Univ3 function to withdraw from position was modified to allow for partial withdraw
  • The active range was properly identified and half of its BADGER and wBTC was withdrawn
  • The received wBTC is swapped for eBTC directly on UniV3 -LG. NOTE: Amount is wrong since received amount includes fees
  • New pool is deployed using the same fee as the current pool - LGTM
  • Pool is initialized to the current range - LGTM
  • BADGER and eBTC obtained from previous steps are used to mint a position mimicking the active range of the former pool - LG. NOTE: BADGER amount is not accurate as it includes the fees claimed from the active range
  • The script iterates through the upper positions of the former pool and:
    • Withdraws 50% of the BADGER (no wBTC)
    • Uses this BADGER amount to mint new positions on the new pool mimicking the former ranges
    • NOTE: Doesn't take into account claimed fees and there are some in existance

NOTES

  • @petrovska-petro, I noticed that we missed one of the upper positions IDs, I went ahead and added it. I sort them according to the ranges and added notes on the prices for reference.
  • Also took the liberty of adjusting the burn function so that it returns the actual amount withdrawn from the position (based on the emitted events). This amount is then used to exclude the fees from the deposits and makes the amounts match what is expected.
  • I also added more handy printouts for reference.
  • The printouts of the fees claimed and the amounts to deposit match those found on DeBank (50% for the amounts):
    image
  • I also confirmed that the ranges match those found on each position on their respective UniV3 NFT page
  • Note that the lefover wBTC matches the aggregation of the reported claimed fees but there is still a discrepancy, although smaller, for the BADGER lefover:
snapshot result for 0xD0A7A8B98957b9CD3cFB9c0425AbE44551158e9e:
                    balance_before               balance_after              balance_delta
symbol
BADGER  583,755.371348200412139922  604,593.805041828258704279  20,838.433693627846564357
WBTC         27.432375670000000000       28.408280710000000000       0.975905040000000000

wBTC fees: 0.17287903 + 0.80302601 = 0.97590504
BADGER fees: 3922.6086585426337+ 12464.148517053509 = 16386.7571756
BADGER discrepancy = 20,838.433693627846564357 - 16386.7571756 = 4451.67651803

By looking at the events, its seems like the leftover amount is due to the slippage upon minting of the new positions, but I am not entirely sure.

I don't think it should be a blocker, the positions are migrated to the proper ranges, with the proper amounts and fees are now claimed separately.

Will give it a stamp and keep an eye early in the morning in case you figure it out.

Nice job on the script!

@sajanrajdev
Copy link
Collaborator

something i found tricky while coding the script is to not count for the fees collected while handling a mint afterwards. if while reviewing you find that optimisation to be more precise on the each mint, specially w/ focus on the active range. feel free to directly push the commit, since its relatively time sensitive for incoming signing session

discrepancy should be minimal in general terms, but ID 255188 has almost ~$20k of uncollected fees so may make a slightly difference

Ah, @petrovska-petro, just realized that the fees issue was acknowledged. Hopefully my fix seems reasonable to you.

Copy link
Collaborator

@wtj2021 wtj2021 left a comment

Choose a reason for hiding this comment

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

looks good

@petrovska-petro
Copy link
Collaborator Author

appreciated the thoughtful review @sajanrajdev & @wtj2021 . your additions looks go to me to be more precise and not to account for the collected fees. good one on the events 🧠

the discrepancy for BADGER amount you highlighted ill attach to the slipp as well of some of the positions created and how some bits of liquidity did not managed to fit in properly, that's why was forced to add as well the approve(0) at some point

in the grant schemes of things is $14k compare to the amounts migrated is relatively nothing. lets merge it and post it ! 🚀

@petrovska-petro petrovska-petro merged commit dfdde7a into main Jun 20, 2024
3 checks passed
@petrovska-petro petrovska-petro deleted the feat/tcl-migration branch June 20, 2024 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants