Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

feat: enable fx pools #972

Merged
merged 9 commits into from
Sep 17, 2024
Merged

Conversation

0xnook
Copy link
Contributor

@0xnook 0xnook commented Aug 15, 2024

  • displays Xave's fxpools on pool list
  • displays fx pool data and stats
  • links to Xave's interface to add/remove liquidity

Some data is still not correctly displayed, like the pool swap fee apr, pool owner, and pool attribute immutability. Using the swap interface also does not route through these pools.

However, these inaccuracies where also present in the frontend-v2 UI.

Copy link

vercel bot commented Aug 15, 2024

@0xnook is attempting to deploy a commit to the Balancer Team on Vercel.

A member of the Team first needs to authorize it.

@0xnook
Copy link
Contributor Author

0xnook commented Aug 15, 2024

resolves #973

Copy link

vercel bot commented Aug 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
frontend-v3 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 15, 2024 6:54pm
test-v3 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 15, 2024 6:54pm

@garethfuller
Copy link
Contributor

@0xnook what's the state here? Would be great to get this working. When I first checked I think something was required on the API side first?

* error display fixed in main, so changes not needed anymore
@0xnook
Copy link
Contributor Author

0xnook commented Sep 10, 2024

Hey @garethfuller, there are still some display inaccuracies.

  • The FX pool APRs are incorrect (regression from frontend v2) (needs to be fixed at backend?)
  • The swap fees are dynamic and always display < 0.01% (same as frontend v2, I believe this is the same for gyro pools)

Pool attributes:

  • It says it has no owner when it has one (same as frontend v2) (needs to be fixed at backend?)
  • swap fees again
  • attribute mutability shows immutable when it is indeed mutable (same as frontend v2) (needs fixed at backend?)

Pool risks:

  • Frontend v2 would put a warning on the fees tab of the pool of negative swap fee possibility. (unnecessary imo)
    • this is an incentive to rebalance pools when out of range)
  • The pools use an oracle which is not displayed anywhere (same as frontend v2)
    • not sure if it should be labeled as a rate provider and how the whole code-review rate provider linking works
    • rate providers also seem to be a different implementation
  • Pool mutability risks
  • I think this would show automatically if the pool mutability is fixed at the backend level

The trickiest part is the dynamic and possibly non symmetrical swap fee.

My opinion is that the PR is good enough to go (after a review) since most issues are fixed at the backend level, and they where already visible at frontend-v2 with the only regression being the APR. There is an open issue in balancer/backend#802

You can easily compare with current deployed versions:
https://app.balancer.fi/#/polygon/pool/0xd4cc1afe3430c6c227f8f640d022bf5747538f53000200000000000000000ebb
https://balancer.fi/pools/polygon/v2/0xd4cc1afe3430c6c227f8f640d022bf5747538f53000200000000000000000ebb

Only changes from this PR are redirecting to Xaves interface for adding/removing liquidity, and displaying the pools on the frontend.

Screencast of the PR:
fx-pr.webm

Also I didn't test staking in gauge, but I can test it shortly, and don't see why it would work any differently.

@0xnook 0xnook changed the title [WIP] feat: enable fx pools feat: enable fx pools Sep 10, 2024
@andreiashu
Copy link

hi @0xnook @garethfuller Xave team member here. If there's anything we can help with let me know.
Thanks for opening this PR!

Copy link
Contributor

@garethfuller garethfuller left a comment

Choose a reason for hiding this comment

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

Could you add a label for the filter here? https://github.com/balancer/frontend-v3/blob/main/lib/modules/pool/PoolList/usePoolListQueryState.tsx#L113-L128 I think it should be FX instead of Fx right?

@0xnook
Copy link
Contributor Author

0xnook commented Sep 17, 2024

I think it should be FX instead of Fx right?

I think so. Added the label

@garethfuller garethfuller merged commit 8438756 into balancer:main Sep 17, 2024
2 of 6 checks passed
@0xnook 0xnook mentioned this pull request Sep 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants