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

Swap alt fee tokens into the native token prior to fee burn #4328

Closed
Tracked by #4326
hdevalence opened this issue May 6, 2024 · 5 comments
Closed
Tracked by #4326

Swap alt fee tokens into the native token prior to fee burn #4328

hdevalence opened this issue May 6, 2024 · 5 comments
Labels
A-fees Area: Relates to transaction fees _P-medium Medium priority _P-V1 Priority: slated for V1 release

Comments

@hdevalence
Copy link
Member

As described in #4320, the initial implementation of multi-asset fee support burns the alt fee tokens. This is incorrect, since the chain should only be minting/burning native assets. The correct behavior is that the fees paid in alt fee tokens should be converted to the native token. However, since the DEX is batched, we need some plumbing to collect fee amounts, include them in batch swaps, and then handle the resulting fee payments. This overlaps with work we'd thought about (but haven't done, and deferred) as part of #3666 and we should think about them at the same time.

@github-project-automation github-project-automation bot moved this to Backlog in Penumbra May 6, 2024
@github-actions github-actions bot added the needs-refinement unclear, incomplete, or stub issue that needs work label May 6, 2024
@aubrika aubrika added _P-V1 Priority: slated for V1 release _P-medium Medium priority and removed needs-refinement unclear, incomplete, or stub issue that needs work labels May 8, 2024
@zbuc
Copy link
Contributor

zbuc commented May 22, 2024

However, since the DEX is batched, we need some plumbing to collect fee amounts, include them in batch swaps, and then handle the resulting fee payments.

Should we add an additional fee multiplier to fees paid in the non-native asset to account for the additional work of swapping?

@hdevalence
Copy link
Member Author

I don't think so, since that cost isn't really attributable to a psecific transaction.

I'm also not sure it's necessary. In this V0 design, there's no connection between the different prices, all of which are fixed -- so the values of the different fee tokens could float around and cause pricing disparities. To address this, I think that the fixed fees for alternative gas tokens should be significantly higher (10x). This way, users can come into the chain and use it, but they're incentivized to use the native fee token which will protect their privacy.

@hdevalence
Copy link
Member Author

The issue here is primarily that we don't currently have any plumbing set up to move the values of the fees around and then swap them, and doing that will require some design work. Maybe we don't get to doing this pre-mainnet.

@aubrika aubrika added the needs-refinement unclear, incomplete, or stub issue that needs work label May 30, 2024
@aubrika
Copy link
Contributor

aubrika commented May 30, 2024

Sounds like we need to discuss & clarify which part of this ticket is in scope for V1 - @hdevalence when you say "maybe we don't get to doing this pre-mainnet", do you mean this ticket generally? Will burning the alt fee tokens cause issues for users?

@aubrika aubrika added this to the Sprint 8 milestone Jun 3, 2024
@aubrika aubrika moved this from Backlog to Todo in Penumbra Jun 3, 2024
@aubrika aubrika removed the needs-refinement unclear, incomplete, or stub issue that needs work label Jun 5, 2024
@aubrika aubrika moved this from Todo to Backlog in Penumbra Jun 12, 2024
@aubrika aubrika removed this from the Sprint 8 milestone Jun 14, 2024
@TalDerei
Copy link
Collaborator

TalDerei commented Jun 15, 2024

@aubrika what does it mean if this ticket is tagged as v1 but removed from the current sprint, specifically in the context of the upcoming code freeze? does that mean this issue should slip into v2?

@TalDerei TalDerei added the A-fees Area: Relates to transaction fees label Jun 15, 2024
erwanor added a commit that referenced this issue Jun 25, 2024
## Describe your changes

Moves fee handling out of the Transaction's ActionHandler and into
the component, which accumulates fees (in any token), and adds hooks to
the DEX
thatadd chain-submitted swaps for any non-native fee tokens into the
native
token. These are then accumulated back into the fee component.

The base fee and tip are tracked separately through this whole process,
in
order to support the intended behaviour (the base fee is burned to
account for
the _chain_'s resource use, the tip is sent to the proposer to encourage
them
to include transactions in fee priority order). However, tip handling is
not
currently implemented, so both are burned. Later, the
funding/distribution
components should extract the tip and send it to the proposer's funding
streams. However, until a robust fee market develops, this form of
proposer
incentivization can be deferred. The accounting will be there for it
when it
arises.

## Issue ticket number and link

#4328 

## Checklist before requesting a review

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:

  > consensus-breaking but not state-breaking, no migrations necessary

---------

Co-authored-by: Erwan Or <erwanor@penumbralabs.xyz>
@TalDerei TalDerei closed this as completed Jul 2, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in Penumbra Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-fees Area: Relates to transaction fees _P-medium Medium priority _P-V1 Priority: slated for V1 release
Projects
Archived in project
Development

No branches or pull requests

4 participants