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

Problem caused by unregister LP when removing all liquidity from a pool #36

Open
marco-sundsk opened this issue Aug 9, 2021 · 1 comment

Comments

@marco-sundsk
Copy link
Contributor

marco-sundsk commented Aug 9, 2021

It's a true story from some user, who wants to withdraw lp token from farming but failed.

Scenario

First try :

  1. Alice adds liquidity to some pool, let's say pool#1429, and got 1.0 lp token,
  2. Alice stakes 0.5 lp token to farming,
  3. Alice remove all remain liquidity (0.5) from pool#1429,
  4. Alice withdraw lp token from farming.
    Then, bom.... ERR13_LP_NOT_REGISTERED

What happened:
In step 3, as Alice removed all her liquidity, the pool unregister her from its LP to save storage;
So, in step 4, you can not transfer lp token to someone not registered on that token contract.

Resolution:
There is only a mft_balance_of can tell us something about user in the LP token, Maybe we just re-register Alice before she withdraws lp token from farming?

Second try :

  1. Alice adds liquidity to some pool, let say pool#1429, and got 1.0 lp token,
  2. Alice stakes 1.0 lp token to farming,
  3. Alice starts withdraw lp token from farming:
  4. See her lp token remaining using mft_balance_of, and got 0,
  5. Register her into the lp token before withdraw;
    Then, bom.... ERR14_LP_ALREADY_REGISTERED

What's wrong:
For transfer of lp tokens, there is no unregsiter logic in it. So Alice is still hang in there even if she has no lp toke left.

Conclusion and Suggestion

Transferring and remove_liqudity are different at register management logic. It's hard to tell a user whether or not he is registered in a lp token, we may need to add a storage_balance_of interface to lp token here.

@marco-sundsk marco-sundsk changed the title Problem caused by unregsiter LP when removing all liquidity from a pool Problem caused by unregister LP when removing all liquidity from a pool Aug 9, 2021
@referencedev
Copy link
Contributor

Quick fix would be not to remove any LP positions even if all LP tokens are removed via remove_liquidity.

Full fix requires to actually migrate Account to new version that keeps track of LP positions as well in storage properly. Robert was trying to do that but it is a bit complex as need to maintain old Account model going forward and need to pass through Account information into the Pools as well.

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

No branches or pull requests

2 participants