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

PriceAuthority doesn't allow AMM to modify amountIn when submitting prices #2789

Open
Chris-Hibbert opened this issue Apr 1, 2021 · 3 comments
Assignees
Labels
bug Something isn't working Core Economy OBSOLETE in favor of INTER-protocol enhancement New feature or request Zoe Contract Contracts within Zoe

Comments

@Chris-Hibbert
Copy link
Contributor

Describe the bug

The PriceAuthority presumes that prices provided by calcAmountIn and calcAmountOut will only provide one amount. Autoswap's getOutputForGivenInput() will sometimes update the amountIn as well as providing the amountOut, and conversely for getInputForGivenOutput().

Expected behavior

The PriceAuthority should transmit the best price information available.

Additional context

In priceAuthority.js, trigger expects that calcAmountOut will only produce amountOut. This forces the priceAuthority in the AMM to discard its amountIn before responding. The current version of getOutputForGivenInput() in autoswap returns the amountOut along with a possibly updated amountIn.

quoteGiven and quoteAtTime should take both values from calcAmountOut, and quoteWanted should take both values from calcAmountIn.

@Chris-Hibbert Chris-Hibbert added bug Something isn't working Small Zoe Contract Contracts within Zoe Core Economy OBSOLETE in favor of INTER-protocol labels Apr 1, 2021
@Chris-Hibbert Chris-Hibbert added this to the Beta: Governance milestone Apr 1, 2021
@Chris-Hibbert Chris-Hibbert self-assigned this Apr 1, 2021
@Tartuffo Tartuffo added MN-1 restival to be done before RUN Protocol Purple Team festival and removed restival to be done before RUN Protocol Purple Team festival MN-1 labels Jan 21, 2022
@Tartuffo
Copy link
Contributor

Tartuffo commented Feb 2, 2022

@samsiegart to make sure we are displaying the answer consistently.

@Tartuffo Tartuffo removed the MN-1 label Feb 7, 2022
@Tartuffo Tartuffo removed this from the Beta Phase 4: Governance milestone Feb 8, 2022
@Tartuffo Tartuffo added the enhancement New feature or request label Feb 24, 2022
@Tartuffo
Copy link
Contributor

@dtribble Is this a "nice to have" for MN-1?

@Tartuffo
Copy link
Contributor

This can be done as part of the major work on PricingAuthority, #2688. Implementing this issue later should not break anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Core Economy OBSOLETE in favor of INTER-protocol enhancement New feature or request Zoe Contract Contracts within Zoe
Projects
None yet
Development

No branches or pull requests

4 participants