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

Vault.sellUSDG() incorrect feeBasisPoints #28

Open
frontier159 opened this issue Aug 31, 2022 · 1 comment
Open

Vault.sellUSDG() incorrect feeBasisPoints #28

frontier159 opened this issue Aug 31, 2022 · 1 comment

Comments

@frontier159
Copy link

frontier159 commented Aug 31, 2022

gm,

The fee calc when selling GLP (the vault.sellUSDG() function) may have a bug.

When selling GLP, the fee for the token to sell into is calculated here:
https://github.com/gmx-io/gmx-contracts/blob/master/contracts/core/Vault.sol#L508

This underlying fee calc depends on vault.usdgAmounts(usg) for the initialAmount:
https://github.com/gmx-io/gmx-contracts/blob/master/contracts/core/VaultUtils.sol#L146

However this usdgAmount has already been updated (decreased) prior to this being called
https://github.com/gmx-io/gmx-contracts/blob/master/contracts/core/Vault.sol#L497

So it looks like the initialAmount var isn't the initial amount, it's already had the usdgDelta applied to it.

Also as far as I can tell, the UI quote is inconsistent with this contract calc - since the usdgAmount being used is the amount prior to the transaction.
https://github.com/gmx-io/gmx-interface/blob/master/src/Helpers.js#L655

Apart from potentially taking more fees than necessary, I guess if the UI quote is different from the actual calc, it may have slippage issues for users.

@frontier159 frontier159 changed the title Vault.sellUSDG() feeBasisPoints Vault.sellUSDG() incorrect feeBasisPoints Aug 31, 2022
@frontier159
Copy link
Author

To illustrate how to match the UI code with what the contract is actually currently doing:
https://github.com/gmx-io/gmx-interface/blob/master/src/Helpers.js#L579

Using increment=false as the signal for a sell in getFeeBasisPoints()

  const initialAmount = token.usdgAmount;
  if (!increment) {
    // The contract code already decrements the initial amount by the amount to sell.
    // So to have a matching quote, the same logic needs to be added here.
    initialAmount = initialAmount.lte(usdgDelta) ? bigNumberify(0) : initialAmount.sub(usdgDelta);
  }
  let nextAmount = ...

But I feel like this is a bug in the contract code - so raising here.

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

No branches or pull requests

1 participant