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

Reinstate claim_admin_fees (Cryptoswap) #231

Closed
chanhosuh opened this issue Sep 6, 2023 · 2 comments · Fixed by #275
Closed

Reinstate claim_admin_fees (Cryptoswap) #231

chanhosuh opened this issue Sep 6, 2023 · 2 comments · Fixed by #275
Assignees
Labels
discussion More ideation is required

Comments

@chanhosuh
Copy link
Member

Right now this is disabled since it isn't needed for basic sims. Activating this requires some care as this is done in different operations for the 2-coin factory cryptopool and the tricrypto-ng pools.

@chanhosuh chanhosuh added the discussion More ideation is required label Sep 6, 2023
@allt0ld
Copy link
Collaborator

allt0ld commented Sep 22, 2023

The tricrypto-ng and 2-coin cryptopool implementations of _claim_admin_fees have one main difference. Tricrypto-ng will end execution early (without updating state) if there aren't enough profits to claim. Cryptopool, will, at the very least, still update D and virtual_price even if there aren't enough profits to claim.

@allt0ld
Copy link
Collaborator

allt0ld commented Oct 5, 2023

Another important thing to note: cryptopool calls _claim_admin_fees only in tweak_price, but tricrypto does not. Instead, tricrypto calls _claim_admin_fees in add_liquidity and remove_liquidity_one_coin, both of which also call tweak_price either before or after, and in remove_liquidity, which doesn't need to call tweak_price.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion More ideation is required
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants