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

Auto-swap non-osmo tx fees to osmo #1121

Closed
AlpinYukseloglu opened this issue Mar 21, 2022 · 6 comments · Fixed by #1145
Closed

Auto-swap non-osmo tx fees to osmo #1121

AlpinYukseloglu opened this issue Mar 21, 2022 · 6 comments · Fixed by #1145
Assignees

Comments

@AlpinYukseloglu
Copy link
Contributor

Currently when a transaction fee is paid in a valid non-OSMO token, the token is collected and distributed as-is to stakers. While this doesn't affect the core user flow since gas fees are zero for the time being anyway, it causes confusing UX around claiming staking rewards (since epochs with too many additional denoms used as tx fees breach the auto set gas limit on Keplr) and dust for random denoms showing up in wallets. The goal of this change would be to implement the following logic:

  • For osmo, do whatever it currently does (send to distribution module)
  • For not osmo, send to a module account $FOO in txfees module
  • On epoch, auto-swap the funds in module account $FOO and distribute that osmo to stakers.

More specifically, one relatively clean way of implementing this would be to take the following steps:

  1. Create a second module account in the TxFees module to send non-OSMO fee tokens to
  2. Add a check in the DeductFees function (sends tokens from fee payer to module account) that checks if the paid fee denom is OSMO and if not, make sure they are supported on the DEX (with enough liquidity) and send the tokens to the second module account
  3. At the end of each epoch, iterate through all the denoms in the second module account, swapping each into OSMO and then transferring the total amount to the first module account to be distributed like normal
@ValarDragon
Copy link
Member

ValarDragon commented Mar 21, 2022

  • Move DeductFeesAnteHandler code into txfeeskeeper
  • Make new module account in txfees
  • Send all non-native fee coins to that module account
  • On epoch code, swap all non-native tx fees in that module to osmo, move that osmo to distribution for staking rewards

Later

  • Upstream the deduct fees keeper (no swap code) to mainline SDK to fix their gas problems

@daniel-farina daniel-farina moved this to 🔍 Needs Review in Osmosis Chain Development Mar 30, 2022
Repository owner moved this from 🔍 Needs Review to ✅ Done in Osmosis Chain Development Apr 22, 2022
@danwt
Copy link

danwt commented Oct 17, 2024

Hi,

I was just wondering why the epoch part of this is important? Would it still be important even if the fees were not distributed to stakers?

Thanks a lot

@czarcas7ic
Copy link
Member

Its run at epoch in order for the fees that got transformed into osmo to the be distributed. It could happen at faster or slower intervals, but just makes sense to run at epoch prior to distribution.

@danwt
Copy link

danwt commented Oct 21, 2024

Thanks @czarcas7ic I understand it needs to be transformed into osmo, but I was just curious why the need to do this at epoch and not on the fly? thanks!

@AlpinYukseloglu
Copy link
Contributor Author

Thanks @czarcas7ic I understand it needs to be transformed into osmo, but I was just curious why the need to do this at epoch and not on the fly? thanks!

The main reason is gas efficiency for users. If every transaction included an additional swap to settle fees, users would have to pay much more in gas to compensate. This approach amortizes the cost across many users.

Keep in mind that it would be fully secure even if we never swapped to OSMO, so there's no security risk with this parameter. Doing this at epoch just happened to be both very effective and very convenient.

@danwt
Copy link

danwt commented Oct 21, 2024

Ok thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants