Fix allowance validation when staking in gauges #3595
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
When staking a user must allow a gauge to spend their token. However when we check for user allowances we only check for tokens the vault can spend, not the allowances for gauges. By allowing injecting a spender we can inject the gauge into the spenders list which then fetches allowances for it when then makes it able to correctly tell if the user has allowed the gauge to spend their BPT. This fixes the post approval check always failing when staking BPT.
timjrobinson
changed the title
timjrobinson
force-pushed
the
revert-3557-hotfix/remove-post-validation
branch
from
74042b5
to
9808a6e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds the post allowance validation from #3515 back into BalActionSteps.
It had a bug previously where no one was able to stake their BPT because we didn't think they had allowed it to be spent. This happened because the allowances query only checks spenders of the vault, wstETH and veBAL. It doesn't check if a gauge can spend a users BPT.
The bug was fixed by allowing injecting of new spenders, and injecting the gauge into the spenders list before the user stakes their BPT.
I renamed
allowanceContractstospendersas that's how the ERC20 standard refers to them, it made it less confusing to understand, and they might not be contracts (you can allow normal EOA's to be a spender of your tokens).This also fixes a minor bug we didn't notice before: Users had to re-allow their BPT every time they staked previously, now they don't have to do that.
This could be made faster by not re-fetching all allowances after the injection and only fetching the allowances of that gauge, but that could come in a future PR.
Testing