Skip to content

Commit

Permalink
Fix allowance validation when staking in gauges (#3595)
Browse files Browse the repository at this point in the history
* Revert "Hotfix - remove post validation"

* Fix approving BPT spending with Gauges

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.

* Fake injectSpender function to avoid onchain calls during unit testing

* fix: Don't fetch allowances twice

---------

Co-authored-by: Alberto Gualis <[email protected]>
Co-authored-by: Gareth Fuller <[email protected]>
  • Loading branch information
3 people authored Jun 30, 2023
1 parent 5b0dd80 commit 36436a7
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 27 deletions.
29 changes: 14 additions & 15 deletions src/components/_global/BalActionSteps/BalActionSteps.vue
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,9 @@ function handleSignAction(state: TransactionActionState) {
async function handleTransaction(
tx: TransactionResponse,
state: TransactionActionState,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
actionInfo: TransactionActionInfo
): Promise<void> {
// const { postActionValidation, actionInvalidReason } = actionInfo;
const { postActionValidation, actionInvalidReason } = actionInfo;
await txListener(tx, {
onTxConfirmed: async (receipt: TransactionReceipt) => {
Expand All @@ -230,21 +229,21 @@ async function handleTransaction(
state.confirming = false;
// const isValid = await postActionValidation?.();
// if (isValid || !postActionValidation) {
const confirmedAt = await getTxConfirmedAt(receipt);
state.confirmedAt = dateTimeLabelFor(confirmedAt);
state.confirmed = true;
if (currentActionIndex.value >= actions.value.length - 1) {
emit('success', { receipt, confirmedAt: state.confirmedAt });
const isValid = await postActionValidation?.();
if (isValid || !postActionValidation) {
const confirmedAt = await getTxConfirmedAt(receipt);
state.confirmedAt = dateTimeLabelFor(confirmedAt);
state.confirmed = true;
if (currentActionIndex.value >= actions.value.length - 1) {
emit('success', { receipt, confirmedAt: state.confirmedAt });
} else {
currentActionIndex.value += 1;
}
} else {
currentActionIndex.value += 1;
// post action validation failed, display reason.
if (actionInvalidReason) state.error = actionInvalidReason;
state.init = false;
}
// } else {
// // post action validation failed, display reason.
// if (actionInvalidReason) state.error = actionInvalidReason;
// state.init = false;
// }
},
onTxFailed: () => {
state.confirming = false;
Expand Down
8 changes: 5 additions & 3 deletions src/composables/approvals/useTokenApprovalActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export default function useTokenApprovalActions() {
/**
* COMPOSABLES
*/
const { refetchAllowances, approvalsRequired, approvalRequired, getToken } =
const { approvalsRequired, approvalRequired, getToken, injectSpenders } =
useTokens();
const { t } = useI18n();
const { getSigner } = useWeb3();
Expand Down Expand Up @@ -89,15 +89,17 @@ export default function useTokenApprovalActions() {
amountsToApprove: AmountToApprove[],
spender: string
): Promise<AmountToApprove[]> {
await refetchAllowances();
await injectSpenders([spender]);

return approvalsRequired(amountsToApprove, spender);
}

async function isApprovalValid(
amountToApprove: AmountToApprove,
spender: string
): Promise<boolean> {
await refetchAllowances();
await injectSpenders([spender]);

return !approvalRequired(
amountToApprove.address,
amountToApprove.amount,
Expand Down
4 changes: 2 additions & 2 deletions src/composables/queries/useAllowancesQuery.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ test('Returns token allowances from balancer SDK', async () => {

initMulticall(generateMulticallMock(processCall));

const allowanceContracts = ref([
const spenders = ref([
'0xBA12222222228d8Ba445958a75a0704d566BF2C8',
'0x6320cD32aA674d2898A68ec82e869385Fc5f7E2f',
'0x33A99Dcc4C85C014cf12626959111D5898bbCAbF',
]);

const { result } = mountComposable(() =>
useAllowancesQuery(tokens, allowanceContracts)
useAllowancesQuery(tokens, spenders)
);

const data = await waitForQueryData(result);
Expand Down
8 changes: 7 additions & 1 deletion src/providers/__mocks__/tokens.provider.fake.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ silenceConsoleLog(vi, message => message.startsWith('Fetching'));
export interface TokensProviderState {
loading: boolean;
injectedTokens: TokenInfoMap;
allowanceContracts: string[];
spenders: string[];
injectedPrices: TokenPrices;
}

Expand Down Expand Up @@ -89,6 +89,11 @@ export const fakeTokensProvider = (
/**
* METHODS
*/
async function injectSpenders(addresses: string[]): Promise<void> {
//TODO: add logic test scenarios using spenders in a more realistic way
return Promise.resolve();
}

/**
* Fetch price for a token
*/
Expand Down Expand Up @@ -147,6 +152,7 @@ export const fakeTokensProvider = (
refetchBalances,
refetchAllowances,
hasBalance,
injectSpenders,
priceFor,
balanceFor,
getMaxBalanceFor,
Expand Down
30 changes: 24 additions & 6 deletions src/providers/tokens.provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const { uris: tokenListUris } = tokenListService;
export interface TokensProviderState {
loading: boolean;
injectedTokens: TokenInfoMap;
allowanceContracts: string[];
spenders: string[];
injectedPrices: TokenPrices;
}

Expand Down Expand Up @@ -87,7 +87,7 @@ export const tokensProvider = (
const state: TokensProviderState = reactive({
loading: true,
injectedTokens: {},
allowanceContracts: compact([
spenders: compact([
networkConfig.addresses.vault,
networkConfig.tokens.Addresses.wstETH,
configService.network.addresses.veBAL,
Expand Down Expand Up @@ -173,7 +173,7 @@ export const tokensProvider = (
isRefetching: allowanceQueryRefetching,
isError: allowancesQueryError,
refetch: refetchAllowances,
} = useAllowancesQuery(tokens, toRef(state, 'allowanceContracts'));
} = useAllowancesQuery(tokens, toRef(state, 'spenders'));

const prices = computed(
(): TokenPrices => (priceData.value ? priceData.value : {})
Expand Down Expand Up @@ -278,6 +278,21 @@ export const tokensProvider = (
await forChange(onchainDataLoading, false);
}

/**
* Injects contract addresses that could possibly spend the users tokens into
* the spenders map. E.g. This is used for injecting gauges into the map as they
* must be allowed to spend a users BPT in order to stake the BPT in the gauge.
*/
async function injectSpenders(addresses: string[]): Promise<void> {
addresses = addresses.filter(a => a).map(getAddress);

state.spenders = [...new Set(state.spenders.concat(addresses))];

// Wait for balances/allowances to be fetched for newly injected tokens.
await nextTick();
await forChange(onchainDataLoading, false);
}

/**
* Given query, filters tokens map by name, symbol or address.
* If address is provided, search for address in tokens or injectToken
Expand Down Expand Up @@ -344,14 +359,16 @@ export const tokensProvider = (
function approvalRequired(
tokenAddress: string,
amount: string,
contractAddress = networkConfig.addresses.vault
spenderAddress = networkConfig.addresses.vault
): boolean {
if (!amount || bnum(amount).eq(0)) return false;
if (!contractAddress) return false;
if (!spenderAddress) return false;
if (isSameAddress(tokenAddress, nativeAsset.address)) return false;

const allowance = bnum(
(allowances.value[contractAddress] || {})[getAddress(tokenAddress)]
(allowances.value[getAddress(spenderAddress)] || {})[
getAddress(tokenAddress)
]
);
return allowance.lt(amount);
}
Expand Down Expand Up @@ -510,6 +527,7 @@ export const tokensProvider = (
refetchBalances,
refetchAllowances,
injectTokens,
injectSpenders,
searchTokens,
hasBalance,
approvalRequired,
Expand Down

1 comment on commit 36436a7

@vercel
Copy link

@vercel vercel bot commented on 36436a7 Jun 30, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.