-
Notifications
You must be signed in to change notification settings - Fork 155
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
Withdraw liquidity #2959
Withdraw liquidity #2959
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly good. I think the ratioMax stuff is not right, but otherwise LGTM.
toId: auth.uid, | ||
category: 'REMOVE_SUBSIDY', | ||
amount: totalAmount, | ||
token: contract.token === 'CASH' ? 'CASH' : 'M$', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just contract.token
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
contract.token is type 'CASH' | 'MANA'
common/src/calculate-cpmm.ts
Outdated
let error = '' | ||
|
||
if (newPool.YES < MINIMUM_LIQUIDITY || newPool.NO < MINIMUM_LIQUIDITY) { | ||
error = 'below minimum' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't really use this style of error handling (versus throwing an exception) anywhere else on the backend to my knowledge...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function is also used on the frontend and I want it to still return what the pool values would be even if it is below 100
common/src/calculate-cpmm.ts
Outdated
|
||
const z = FRACTION_OF_POOL_MIN | ||
const { YES: y, NO: n } = pool | ||
const ratioMax = (n * z + y * z - Math.min(y, n)) / (2 * z - 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this right? if x=y=100, then this value is negative, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't really understand why we need a ratio limit if we already have a limit on the number of shares in the liquidity pool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, after playing around with the maniswap calculator a bit more, I'm convinced that extreme ratios are fine so I removed the restriction
No description provided.