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

N8 - Minted liquidity tokens can round to zero #23

Open
PhABC opened this issue Feb 10, 2020 · 4 comments
Open

N8 - Minted liquidity tokens can round to zero #23

PhABC opened this issue Feb 10, 2020 · 4 comments

Comments

@PhABC
Copy link
Contributor

PhABC commented Feb 10, 2020

image

@PhABC
Copy link
Contributor Author

PhABC commented Feb 12, 2020

How could it round to zero? I haven't found a scenario where this could happen.

@Agusx1211 (with the new divRound approach).

@Agusx1211
Copy link
Member

This is the ultimate edge case... but here it goes:

  • A pool exists with base = 1000, tokens = 2000, totalSupply = 500
  • An user adds 1 token, proportional base is 1 base
  • Minted liquidity tokens rounds to 0, because (1 * 500) / 1000 == 0

The new divRound makes the minting formula (0 * 500) / 1000 == 0, the example still works

@PhABC
Copy link
Contributor Author

PhABC commented Feb 17, 2020

Right, I see, though if we changed that, the owner of the 500 initial liquidity tokens would lose some, as per #19.

For instance, User A adds 1 token (and 1 base), and get 1 Liquidity Token.

Pools are now base == 1001, tokens == 2001, liquidity == 501.

User A burns 1 liquidity to get back
Base : 1 (1 == 1 * 1001 / 501)
Tokens: 3 (3 == 1 * 2001 / 501)

And made a 2 tokens profit at the cost of initial liquidity provider. If we round up, we have to take some tokens from the existing liquidity providers and if we round down, the new liquidity provider loses some tokens, favoring existing liquidity providers. I think not hurting existing liquidity providers is more important.

What do you think?

@Agusx1211
Copy link
Member

I think not hurting existing liquidity providers is more important.
I agree; all formulas that require rounding should favor them.

Maybe a simple way of fixing this issue is adding a require(minted != 0) or adding a minimumMinted as a parameter on the addLiquidity method.

@PhABC PhABC closed this as completed Mar 20, 2022
@PhABC PhABC reopened this Mar 20, 2022
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