From f1d50e62f880d03f2f71fae8c63b3fb109ebce50 Mon Sep 17 00:00:00 2001 From: Tim Robinson Date: Fri, 30 Jun 2023 13:40:09 +1000 Subject: [PATCH 1/4] Revert "Hotfix - remove post validation" --- .../_global/BalActionSteps/BalActionSteps.vue | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/components/_global/BalActionSteps/BalActionSteps.vue b/src/components/_global/BalActionSteps/BalActionSteps.vue index a4a9778403..04ebb407c5 100644 --- a/src/components/_global/BalActionSteps/BalActionSteps.vue +++ b/src/components/_global/BalActionSteps/BalActionSteps.vue @@ -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 { - // const { postActionValidation, actionInvalidReason } = actionInfo; + const { postActionValidation, actionInvalidReason } = actionInfo; await txListener(tx, { onTxConfirmed: async (receipt: TransactionReceipt) => { @@ -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; From 9808a6ef6e00be0ab7e77654fc6c98ca5e353582 Mon Sep 17 00:00:00 2001 From: Tim Robinson Date: Fri, 30 Jun 2023 15:48:44 +1000 Subject: [PATCH 2/4] 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. --- .../approvals/useTokenApprovalActions.ts | 11 +++++-- .../queries/useAllowancesQuery.spec.ts | 4 +-- .../__mocks__/tokens.provider.fake.ts | 2 +- src/providers/tokens.provider.ts | 30 +++++++++++++++---- 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/composables/approvals/useTokenApprovalActions.ts b/src/composables/approvals/useTokenApprovalActions.ts index 076635b639..768ca8b865 100644 --- a/src/composables/approvals/useTokenApprovalActions.ts +++ b/src/composables/approvals/useTokenApprovalActions.ts @@ -37,8 +37,13 @@ export default function useTokenApprovalActions() { /** * COMPOSABLES */ - const { refetchAllowances, approvalsRequired, approvalRequired, getToken } = - useTokens(); + const { + refetchAllowances, + approvalsRequired, + approvalRequired, + getToken, + injectSpenders, + } = useTokens(); const { t } = useI18n(); const { getSigner } = useWeb3(); const { addTransaction } = useTransactions(); @@ -89,6 +94,7 @@ export default function useTokenApprovalActions() { amountsToApprove: AmountToApprove[], spender: string ): Promise { + await injectSpenders([spender]); await refetchAllowances(); return approvalsRequired(amountsToApprove, spender); } @@ -97,6 +103,7 @@ export default function useTokenApprovalActions() { amountToApprove: AmountToApprove, spender: string ): Promise { + await injectSpenders([spender]); await refetchAllowances(); return !approvalRequired( amountToApprove.address, diff --git a/src/composables/queries/useAllowancesQuery.spec.ts b/src/composables/queries/useAllowancesQuery.spec.ts index b4fb334f82..1fbd0e67b2 100644 --- a/src/composables/queries/useAllowancesQuery.spec.ts +++ b/src/composables/queries/useAllowancesQuery.spec.ts @@ -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); diff --git a/src/providers/__mocks__/tokens.provider.fake.ts b/src/providers/__mocks__/tokens.provider.fake.ts index 7befb06514..ed1e37db35 100644 --- a/src/providers/__mocks__/tokens.provider.fake.ts +++ b/src/providers/__mocks__/tokens.provider.fake.ts @@ -39,7 +39,7 @@ silenceConsoleLog(vi, message => message.startsWith('Fetching')); export interface TokensProviderState { loading: boolean; injectedTokens: TokenInfoMap; - allowanceContracts: string[]; + spenders: string[]; injectedPrices: TokenPrices; } diff --git a/src/providers/tokens.provider.ts b/src/providers/tokens.provider.ts index d0d3dcd903..43c394f86a 100644 --- a/src/providers/tokens.provider.ts +++ b/src/providers/tokens.provider.ts @@ -53,7 +53,7 @@ const { uris: tokenListUris } = tokenListService; export interface TokensProviderState { loading: boolean; injectedTokens: TokenInfoMap; - allowanceContracts: string[]; + spenders: string[]; injectedPrices: TokenPrices; } @@ -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, @@ -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 : {}) @@ -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 { + 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 @@ -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); } @@ -510,6 +527,7 @@ export const tokensProvider = ( refetchBalances, refetchAllowances, injectTokens, + injectSpenders, searchTokens, hasBalance, approvalRequired, From fa8c219681bdbea029ad440d174f205f1d1af753 Mon Sep 17 00:00:00 2001 From: Alberto Gualis Date: Fri, 30 Jun 2023 09:16:03 +0200 Subject: [PATCH 3/4] Fake injectSpender function to avoid onchain calls during unit testing --- src/providers/__mocks__/tokens.provider.fake.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/providers/__mocks__/tokens.provider.fake.ts b/src/providers/__mocks__/tokens.provider.fake.ts index ed1e37db35..5dccf14708 100644 --- a/src/providers/__mocks__/tokens.provider.fake.ts +++ b/src/providers/__mocks__/tokens.provider.fake.ts @@ -89,6 +89,11 @@ export const fakeTokensProvider = ( /** * METHODS */ + async function injectSpenders(addresses: string[]): Promise { + //TODO: add logic test scenarios using spenders in a more realistic way + return Promise.resolve(); + } + /** * Fetch price for a token */ @@ -147,6 +152,7 @@ export const fakeTokensProvider = ( refetchBalances, refetchAllowances, hasBalance, + injectSpenders, priceFor, balanceFor, getMaxBalanceFor, From 0bac4fdc19c6f62441abf9213ae5567a103bf9d4 Mon Sep 17 00:00:00 2001 From: Gareth Fuller Date: Fri, 30 Jun 2023 14:23:28 +0100 Subject: [PATCH 4/4] fix: Don't fetch allowances twice --- .../approvals/useTokenApprovalActions.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/composables/approvals/useTokenApprovalActions.ts b/src/composables/approvals/useTokenApprovalActions.ts index 768ca8b865..b9a781223c 100644 --- a/src/composables/approvals/useTokenApprovalActions.ts +++ b/src/composables/approvals/useTokenApprovalActions.ts @@ -37,13 +37,8 @@ export default function useTokenApprovalActions() { /** * COMPOSABLES */ - const { - refetchAllowances, - approvalsRequired, - approvalRequired, - getToken, - injectSpenders, - } = useTokens(); + const { approvalsRequired, approvalRequired, getToken, injectSpenders } = + useTokens(); const { t } = useI18n(); const { getSigner } = useWeb3(); const { addTransaction } = useTransactions(); @@ -95,7 +90,7 @@ export default function useTokenApprovalActions() { spender: string ): Promise { await injectSpenders([spender]); - await refetchAllowances(); + return approvalsRequired(amountsToApprove, spender); } @@ -104,7 +99,7 @@ export default function useTokenApprovalActions() { spender: string ): Promise { await injectSpenders([spender]); - await refetchAllowances(); + return !approvalRequired( amountToApprove.address, amountToApprove.amount,