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

feat: paloma bridge exit tax #1196

Merged

Conversation

maharifu
Copy link
Contributor

@maharifu maharifu commented Jun 18, 2024

Related Github tickets

Background

We're going to charge a tax fee for every transfer out of paloma. The tax rate is defined by governance vote and we can set exceptions to tokens and addresses.

  • Add new governance vote to gravity module
  • The bridge tax information is added to genesis
  • Before adding transfers to the batched pool in gravity, we set the tax amount, if applicable. This amount is removed from the total amount to transfer. Compass only sees the decreased amount.
  • On receiving the transfer complete event we burn the total amount of tokens (transferred + taxed)
  • On cancelling a transfer, we return the total amount including tax

Testing completed

  • test coverage exists or has been added/updated
  • tested in a private testnet

Breaking changes

  • I have checked my code for breaking changes
  • If there are breaking changes, there is a supporting migration.

@maharifu maharifu force-pushed the lcarvalho/1709-bridge-tax-governance branch from dc55880 to 9ec464d Compare June 18, 2024 15:51
@maharifu maharifu changed the title Paloma bridge exit tax feat: paloma bridge exit tax Jun 18, 2024
@maharifu maharifu marked this pull request as ready for review June 18, 2024 16:30
@taariq
Copy link
Contributor

taariq commented Jun 18, 2024

- On receiving the transfer complete event we burn the total amount of tokens (transferred + taxed) @maharifu to be clear, we're not BURNING THE TOTAL AMOUNT OF TOKENS! RIGHT? RIGHT?!?!?

Copy link
Contributor

@taariq taariq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please let us not BURN ALL the tokens!

x/gravity/keeper/batch.go Show resolved Hide resolved
Copy link
Contributor

@byte-bandit byte-bandit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work. Some small remarks.

proto/palomachain/paloma/gravity/query.proto Outdated Show resolved Hide resolved
proto/palomachain/paloma/gravity/query.proto Outdated Show resolved Hide resolved
x/gravity/keeper/batch.go Show resolved Hide resolved
x/gravity/keeper/keeper.go Outdated Show resolved Hide resolved
x/gravity/keeper/keeper.go Show resolved Hide resolved
x/gravity/keeper/keeper.go Outdated Show resolved Hide resolved
@maharifu maharifu force-pushed the lcarvalho/1709-bridge-tax-governance branch 2 times, most recently from 1b41622 to 927fde9 Compare June 19, 2024 11:55
@maharifu maharifu force-pushed the lcarvalho/1709-bridge-tax-governance branch from 927fde9 to 2cbd798 Compare June 19, 2024 12:41
Instead of subtracting from amount to transfer, we transfer the full
amount and add the tax as a transfer cost to the user.
Copy link
Contributor

@byte-bandit byte-bandit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤘

x/gravity/keeper/keeper.go Show resolved Hide resolved
@byte-bandit byte-bandit requested a review from taariq June 19, 2024 15:46
Copy link
Contributor

@taariq taariq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LFG!

@taariq taariq merged commit d3bc02e into palomachain:master Jun 19, 2024
4 checks passed
@taariq taariq deleted the lcarvalho/1709-bridge-tax-governance branch June 19, 2024 16:09
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

Successfully merging this pull request may close these issues.

3 participants