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

Autoswap accounting fixes #NTRN-385 #685

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Autoswap accounting fixes #NTRN-385 #685

wants to merge 14 commits into from

Conversation

jcompagni10
Copy link
Contributor

@jcompagni10 jcompagni10 commented Aug 20, 2024

This PR fixes 3 issues to related to how autoswap shares are calculated. For broader context see the autoswap spec: https://www.notion.so/hadron/1f17de6d999f4ace9f322262dde11a93?v=5bca99093f134baf9688c9bf12f1aa1c&p=7aca1c1dd1c54ae38202792b79cfe54e&pm=s

Issue 1:
The price (price1To0Center) used for the calculation was incorrect.
Issue 2:
Autoswap shares were being issued at full value instead of being discounted by the normal share value calculation (existingShares / poolValue)
Issue 2.5:
The share value calculation must include the autoswap fee as part of the share value.

Because of these issues, it is possible that an incorrect number of shares were issued for pool deposits. As of this moment, there are no pools that are affected. But out of caution, there is an upgrade that will withdraw any potentially affected pools.

@jcompagni10 jcompagni10 marked this pull request as ready for review August 22, 2024 20:40
@pr0n00gler
Copy link
Collaborator

Is it ready for review? Cause the description says otherwise

@jcompagni10
Copy link
Contributor Author

Is it ready for review? Cause the description says otherwise

Yes, ready for review

@jcompagni10
Copy link
Contributor Author

x/dex/keeper/pool.go Show resolved Hide resolved
x/dex/migrations/v5/store.go Outdated Show resolved Hide resolved
@jcompagni10
Copy link
Contributor Author

@jcompagni10
Copy link
Contributor Author

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.

2 participants