-
Notifications
You must be signed in to change notification settings - Fork 44
refactor: remove user balance store dependency in deposit gauge reward feature #1765
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
base: fix/pool-user-balances-not-loading-correctly
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| const tokenBalance = userBalancesMapper[rewardTokenId] | ||
|
|
||
| if (!tokenBalance) return | ||
| enforce(userBalance).condition((userBalance) => ({ |
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.
I tried something like userBalance.greaterThan(0) but it didn't show the message. This is a bit more verbose but at least it works
| enforce(amount).condition((amount) => ({ | ||
| pass: +amount < +tokenBalance, | ||
| message: t`Amount ${formatNumber(amount, { decimals: 5 })} > wallet balance ${formatNumber(tokenBalance, { decimals: 5 })}`, | ||
| pass: +amount <= +userBalance!, |
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.
This fixes a bug where we used < instead of <=, so if you pressed the 'Max' button it would incorrectly give an error
|
|
||
| // Sync userBalance from query into form for validation | ||
| useEffect(() => { | ||
| methods.setValue('userBalance', userBalance, { shouldValidate: true }) |
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.
Initially I looked at vest contexts together with react form hook, but that turned out to be a disaster as contexts aren't reactive. So I applied the same trick as we did for the new borrow form, just make the user balance part of the form values.
| type="number" | ||
| labelProps={ | ||
| haveSigner && { | ||
| signerAddress && { |
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.
We ought to replace InputDebounced with an LTI, but that's out of scope
Depends on #1763
In an effort to removing the user balances store slice and imperative
userBalancesMapperusage, I found that aside from quickswap, the biggest user is the gauge reward feature. This PR aims to remove that dependency to rely solely on the newuseTokenBalancefromtoken-balance.query.If you want to test, you can do so by using the
0xE13aDC278c252e04DCBdca8eDced2C973Db994fAaddress with this pool(The feature is a bit crap in that sometimes it might detect you as a gauge manager, just refresh with a connected wallet sometimes; out of scope of this PR)