-
Notifications
You must be signed in to change notification settings - Fork 195
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: make lp opportunities great #8888
Conversation
src/components/EarnDashboard/components/PositionDetails/LpPositionsByProvider.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Ensure that you can see your LP read-only opportunities in the DeFi section, vs. empty when expanding previously
✅ No LP opportunities are displayed as Eligible in the dashboard
✅ There is no dead link button in opportunities (View/Manage)
Noting that we still have the wagmi issue (I think due to missing assetReference for some assets), but that is not a regression in this PR.

Also 2 nitpicks I'd prefer to see fixed.
src/components/EarnDashboard/components/PositionDetails/StakingPositionsByProvider.tsx
Outdated
Show resolved
Hide resolved
src/components/EarnDashboard/components/PositionDetails/LpPositionsByProvider.tsx
Outdated
Show resolved
Hide resolved
0f835cb
to
fa75173
Compare
fa75173
to
f35cde1
Compare
Description
Many birds, one stone:
underlyingAssetId
(the top-level asset, basically an LP token) vs. theopportunityId
(a composite AssetId of sorts).Using said composite ID means we were trying to introspect
assets[compositeId]
, failing, early returning, and effectively not having the opportunity as part of LP ones. The reason it "worked" previously was accidental, as a direct consequence of the above.First of all, it did not work before. It only worked in "By Provider", not in "By Asset" but we'd never noticed. The reason it worked there is because "By Provider" is more dumb and doesn't use staking/id indexes for rendering but simply rendered all opportunitie indexed by provider, with no regard for their DefiType.
In comparison, the 'By Asset' view has two "Staking" and "LP" tables, which uses each index and then introspects things, which is where this fix comes in
Issue (if applicable)
Risk
Low
Testing
Engineering
Operations
Screenshots (if applicable)
develop (right) vs. this diff (left)