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

fix: add fox benefits rfox asset selectors #8553

Merged
merged 3 commits into from
Jan 16, 2025

Conversation

NeOMakinG
Copy link
Collaborator

@NeOMakinG NeOMakinG commented Jan 13, 2025

Description

Added the Asset selector to the RFOX section of the fox benefits page

Fixed a bug in useLifetimeRewardsQuery where the select was throwing an error because detailsByStakingContract for the lp contract is not existing yet

Passed stakingAssetId everywhere to get the right data for the right contract

Issue (if applicable)

closes #8510

Risk

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

Testing

  • Go to the FOX Benefits page
  • Try to select the LP asset for the RFOX section
  • See all informations should be calculated again

Engineering

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)

Screenshots (if applicable)

image

Base automatically changed from rfox-lpoors to develop January 16, 2025 08:52
@NeOMakinG NeOMakinG force-pushed the fox-benefits-asset-selector branch from be5043a to 04ff09d Compare January 16, 2025 10:12
@NeOMakinG NeOMakinG removed the blocked label Jan 16, 2025
@NeOMakinG NeOMakinG marked this pull request as ready for review January 16, 2025 16:06
@NeOMakinG NeOMakinG requested a review from a team as a code owner January 16, 2025 16:06
@gomesalexandre gomesalexandre self-requested a review January 16, 2025 22:15
Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

Diff looking sane - runtime pass incoming!

Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

Tested locally, confirmed this does what it says on the box! Great one ser 🐐

https://jam.dev/c/42d6974f-09b4-403c-b477-249ba15bcd06

One thing I've spotted which isn't inherently blocking but would be nice-to-have is account number is carried over when clicking "Manage", but staking asset isn't. Not sure how much of a lift that is, so stamping regardless.

@gomesalexandre gomesalexandre enabled auto-merge (squash) January 16, 2025 23:09
@gomesalexandre gomesalexandre merged commit fda8993 into develop Jan 16, 2025
3 checks passed
@gomesalexandre gomesalexandre deleted the fox-benefits-asset-selector branch January 16, 2025 23: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.

Add asset selector for FOX Benefits to toggle FOX vs LP
2 participants