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

updated rebate to use native denom instead of reward denom #1162

Merged
merged 10 commits into from
Mar 27, 2024

Conversation

sampocs
Copy link
Collaborator

@sampocs sampocs commented Mar 25, 2024

Context

The rebate must now be issued using the native denom (DYDX) instead of the reward denom (USDC).

Brief Changelog

  • Removed CalculateRewardsSplitBeforeRebate and CalculateRewardsSplitAfterRebate in favor of a simpler function CalculateRewardsSplit that returns the portion of rebate, stride fee, and reinvestment
  • Updated withdrawal-rewards callback (the callback on the USDC query) to trade the full queried amount instead of checking for a rebate
  • Updated withdrawal-host callback (the callback on the DYDX query) to optionally fund the community pool if a rebate was specified
  • Updated unit tests

Copy link
Contributor

@riley-stride riley-stride left a comment

Choose a reason for hiding this comment

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

Looking solid to me - great work!

Copy link
Contributor

@asalzmann asalzmann left a comment

Choose a reason for hiding this comment

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

lgtm - although I'm slightly less familiar with this code, should get a review from @shellvish and @ethan-stride too!

Copy link
Contributor

@ethan-stride ethan-stride left a comment

Choose a reason for hiding this comment

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

LGTM

One weird thing is that StrideCommission and RebateRate are not stored in the same format. We might also want to rename StrideCommission if it is really the percent of total fees not just the stride fees.

@github-actions github-actions bot removed the C:CLI label Mar 27, 2024
Copy link
Contributor

@shellvish shellvish left a comment

Choose a reason for hiding this comment

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

This looks great, thanks a lot for doing this! Seems like this greatly simplifies the rebate logic!

@sampocs sampocs merged commit a30c164 into main Mar 27, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants