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

Replace AMM calculations with simpler approach in estimate.js #5842

Closed
Chris-Hibbert opened this issue Jul 27, 2022 · 2 comments
Closed

Replace AMM calculations with simpler approach in estimate.js #5842

Chris-Hibbert opened this issue Jul 27, 2022 · 2 comments
Assignees
Labels
AMM Inter-protocol Overarching Inter Protocol performance Performance related issues

Comments

@Chris-Hibbert
Copy link
Contributor

What is the Problem Being Solved?

The current code to calculate prices for swaps on the AMM is spread across several files: calcFees, calcSwapPrices,getXY, core, invariants, and swap. Some have found it hard to follow. estimate.js was written to allow the UI to predict prices without requiring a round trip to the chain. It has a test that shows it behaves the same as the more complex solution. If estimate is easier to explain, it might be a good idea to use it for the AMM's own calculations.

Description of the Design

Replace the code. Add more thorough documentation in the code to show how it carries out the design in README.md.

Security Considerations

This shouldn't effect security.

Test Plan

Consider whether any test cases need to be added. Consider substituting estimate.js in the existing AMM tests.

@Chris-Hibbert Chris-Hibbert added this to the Mainnet 1 milestone Jul 27, 2022
@Chris-Hibbert Chris-Hibbert self-assigned this Jul 27, 2022
@Tartuffo Tartuffo removed this from the Mainnet 1 RC0 milestone Aug 10, 2022
@Chris-Hibbert
Copy link
Contributor Author

It might also have much better performance. It's just math, and not a lot of sending messages or packing and unpacking parameters.

@Chris-Hibbert Chris-Hibbert added AMM Inter-protocol Overarching Inter Protocol performance Performance related issues labels Sep 29, 2022
@Chris-Hibbert
Copy link
Contributor Author

The AMM was removed in #7074

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AMM Inter-protocol Overarching Inter Protocol performance Performance related issues
Projects
None yet
Development

No branches or pull requests

2 participants