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

Fix protocol vs. pool fee calculations #3340

Closed
rowgraus opened this issue Jun 16, 2021 · 4 comments
Closed

Fix protocol vs. pool fee calculations #3340

rowgraus opened this issue Jun 16, 2021 · 4 comments
Assignees
Labels
AMM Core Economy OBSOLETE in favor of INTER-protocol

Comments

@rowgraus
Copy link

No description provided.

@rowgraus rowgraus added the AMM label Jun 16, 2021
@rowgraus rowgraus added this to the Mainnet: Phase 1 - Treasury Launch milestone Jun 16, 2021
@dckc
Copy link
Member

dckc commented Sep 29, 2021

I'm inclined to add to the description:

The rules that drive the design include

  • When the user names an input (or output) price, they shouldn't pay more
    (or receive less) than they said.
  • The pool fee is charged against the computed side of the price.
  • The protocol fee is always charged in RUN.
  • The fees should be calculated based on the pool balances before a transaction.
  • Computations are rounded in favor of the pool.

@dckc
Copy link
Member

dckc commented Oct 7, 2021

This part seems to belong as well:

The user can specify a maximum amount they want to pay or a minimum amount they
want to receive. Unlike Uniswap, this approach will charge less than the user
offered or pay more than they asked for when appropriate. By analogy, if a user
is willing to pay up to $20 when the price of soda is $3 per bottle, it would
give 6 bottles and only charge $18. Uniswap doesn't adjust the provided price,
so it charges $20. This matters whenever the values of the smallest unit of the
currencies are significantly different, which is common in DeFi. (We refer to
these as "improved" prices.)

-- https://github.com/Agoric/agoric-sdk/blob/master/packages/zoe/src/contracts/constantProduct/README.md

cc @katelynsills

@dckc
Copy link
Member

dckc commented Oct 7, 2021

Around Oct 1, I wrote in #3791:

I suspect there is a AMM design document that I missed where I could confirm that @rowgraus , @btulloh , @dtribble are content that we don't require finer resolution than a basis point for fee ratios.

but I learned that I didn't miss such a document after all.

cc @katelynsills

@dckc dckc added the Core Economy OBSOLETE in favor of INTER-protocol label Oct 7, 2021
@Chris-Hibbert
Copy link
Contributor

The only issue mentioned above that hasn't been addressed is whether we require a finer resolution for fee ratios than 1 BP.

I think we've "fixed protocol vs. pool fee calculations".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AMM Core Economy OBSOLETE in favor of INTER-protocol
Projects
None yet
Development

No branches or pull requests

3 participants