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

Tokenomics implementation #54

Merged
merged 43 commits into from
Apr 14, 2022
Merged

Tokenomics implementation #54

merged 43 commits into from
Apr 14, 2022

Conversation

jepidoptera
Copy link
Contributor

Updated the implementation to comply with the new spec

@jepidoptera
Copy link
Contributor Author

Had to skip a few of the check in test_omnipool_amm. I feel like they are not important, but let me know. Also did not get the two-part token-lrna-token swap mechanic to work, and I think it needs more math. But overall I believe everything works that needs to.

@jepidoptera jepidoptera requested a review from poliwop April 6, 2022 03:02
hydradx/model/amm/omnipool_amm.py Outdated Show resolved Hide resolved
hydradx/model/amm/omnipool_amm.py Outdated Show resolved Hide resolved
hydradx/model/amm/omnipool_amm.py Outdated Show resolved Hide resolved
hydradx/tests/test_omnipool_amm.py Outdated Show resolved Hide resolved
hydradx/tests/test_omnipool_amm.py Show resolved Hide resolved
hydradx/tests/test_omnipool_amm.py Show resolved Hide resolved
hydradx/tests/test_omnipool_amm.py Outdated Show resolved Hide resolved
hydradx/tests/test_omnipool_amm.py Outdated Show resolved Hide resolved
hydradx/tests/test_omnipool_amm.py Outdated Show resolved Hide resolved
hydradx/tests/test_omnipool_amm.py Outdated Show resolved Hide resolved
@jepidoptera jepidoptera requested a review from poliwop April 11, 2022 17:13
@poliwop
Copy link
Collaborator

poliwop commented Apr 12, 2022

I think we also need changes in tests/test_amm.py? At least it appears to be broken for me due to missing 'L'... @jepidoptera

Copy link
Contributor Author

@jepidoptera jepidoptera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks right.

@poliwop
Copy link
Collaborator

poliwop commented Apr 12, 2022

@jepidoptera in addition to the changes in my comments in my review you will also want to make the fix for the spec issue you noticed in the actual implementation... I only fixed it in the spec.

@poliwop
Copy link
Collaborator

poliwop commented Apr 12, 2022

@jepidoptera in addition to the changes in my comments in my review you will also want to make the fix for the spec issue you noticed in the actual implementation... I only fixed it in the spec.

actually double-check this... but the implementation already looks correct to me?

Copy link
Collaborator

@poliwop poliwop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good overall, most of these changes are very small.

hydradx/tests/test_omnipool_amm.py Outdated Show resolved Hide resolved
hydradx/tests/test_omnipool_amm.py Outdated Show resolved Hide resolved
hydradx/tests/test_omnipool_amm.py Show resolved Hide resolved
hydradx/tests/test_omnipool_amm.py Outdated Show resolved Hide resolved
hydradx/model/amm/omnipool_amm.py Show resolved Hide resolved
hydradx/model/amm/omnipool_amm.py Show resolved Hide resolved
hydradx/model/amm/omnipool_amm.py Show resolved Hide resolved
hydradx/model/amm/omnipool_amm.py Show resolved Hide resolved
hydradx/model/amm/omnipool_amm.py Show resolved Hide resolved
hydradx/model/amm/omnipool_amm.py Show resolved Hide resolved
@jepidoptera jepidoptera requested a review from poliwop April 14, 2022 17:47
Copy link
Collaborator

@poliwop poliwop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@poliwop poliwop merged commit dde5754 into main Apr 14, 2022
@jepidoptera jepidoptera deleted the tokenomics_implementation branch June 20, 2022 18:11
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.

3 participants