fix: remove duplicate hook for permit2 approvals (#6999)

* fix: remove duplicate hook for permit2 approvals

* fix: address comments

src/components/swap/ConfirmSwapModal.tsx

@@ -161,17 +161,7 @@ function useConfirmModalState({
         case ConfirmModalState.APPROVING_TOKEN:
           setConfirmModalState(ConfirmModalState.APPROVING_TOKEN)
           invariant(allowance.state === AllowanceState.REQUIRED, 'Allowance should be required')
-          allowance
-            .approve()
-            .then(() => {
-              sendAnalyticsEvent(InterfaceEventName.APPROVE_TOKEN_TXN_SUBMITTED, {
-                chain_id: chainId,
-                token_symbol: maximumAmountIn?.currency.symbol,
-                token_address: maximumAmountIn?.currency.address,
-                ...trace,
-              })
-            })
-            .catch((e) => catchUserReject(e, PendingModalError.TOKEN_APPROVAL_ERROR))
+          allowance.approve().catch((e) => catchUserReject(e, PendingModalError.TOKEN_APPROVAL_ERROR))
           break
         case ConfirmModalState.PERMITTING:
           setConfirmModalState(ConfirmModalState.PERMITTING)

src/graphql/data/nft/NftUniversalRouterAddress.ts

+import { ChainId } from '@uniswap/sdk-core'
+import { UNIVERSAL_ROUTER_ADDRESS } from '@uniswap/universal-router-sdk'
+import { isSupportedChain } from 'constants/chains'
 import gql from 'graphql-tag'
 
 import { useNftUniversalRouterAddressQuery } from '../__generated__/types-and-hooks'
@@ -10,6 +13,15 @@ gql`
   }
 `
 
+export function getURAddress(chainId?: number, nftURAddress?: string): string | undefined {
+  if (!chainId) return undefined
+  // if mainnet and on NFT flow, use the contract address returned by GQL
+  if (chainId === ChainId.MAINNET) {
+    return nftURAddress ?? UNIVERSAL_ROUTER_ADDRESS(chainId)
+  }
+  return isSupportedChain(chainId) ? UNIVERSAL_ROUTER_ADDRESS(chainId) : undefined
+}
+
 export function useNftUniversalRouterAddress() {
   const { data, loading } = useNftUniversalRouterAddressQuery({
     // no cache because a different version of nftRoute query is going to be called around the same time

src/hooks/useTokenAllowance.ts

 import { ContractTransaction } from '@ethersproject/contracts'
+import { InterfaceEventName } from '@uniswap/analytics-events'
 import { CurrencyAmount, MaxUint256, Token } from '@uniswap/sdk-core'
+import { sendAnalyticsEvent, useTrace } from 'analytics'
 import { useTokenContract } from 'hooks/useContract'
 import { useSingleCallResult } from 'lib/hooks/multicall'
 import { useCallback, useEffect, useMemo, useState } from 'react'
@@ -43,6 +45,7 @@ export function useUpdateTokenAllowance(
   spender: string
 ): () => Promise<{ response: ContractTransaction; info: ApproveTransactionInfo }> {
   const contract = useTokenContract(amount?.currency.address)
+  const trace = useTrace()
 
   return useCallback(async () => {
     try {
@@ -52,6 +55,12 @@ export function useUpdateTokenAllowance(
 
       const allowance = amount.equalTo(0) ? '0' : MAX_ALLOWANCE
       const response = await contract.approve(spender, allowance)
+      sendAnalyticsEvent(InterfaceEventName.APPROVE_TOKEN_TXN_SUBMITTED, {
+        chain_id: amount.currency.chainId,
+        token_symbol: amount.currency.symbol,
+        token_address: amount.currency.address,
+        ...trace,
+      })
       return {
         response,
         info: {
@@ -68,7 +77,7 @@ export function useUpdateTokenAllowance(
       }
       throw new Error(`${symbol} token allowance failed: ${e instanceof Error ? e.message : e}`)
     }
-  }, [amount, contract, spender])
+  }, [amount, contract, spender, trace])
 }
 
 export function useRevokeTokenAllowance(

src/nft/components/bag/BagFooter.test.tsx

 import { BigNumber } from '@ethersproject/bignumber'
 import { parseEther } from '@ethersproject/units'
 import { ChainId, CurrencyAmount, Percent } from '@uniswap/sdk-core'
+import { UNIVERSAL_ROUTER_ADDRESS } from '@uniswap/universal-router-sdk'
 import { useWeb3React } from '@web3-react/core'
 import { nativeOnChain } from 'constants/tokens'
-import { useNftUniversalRouterAddress } from 'graphql/data/nft/NftUniversalRouterAddress'
+import { getURAddress, useNftUniversalRouterAddress } from 'graphql/data/nft/NftUniversalRouterAddress'
 import { useCurrency } from 'hooks/Tokens'
-import { AllowanceState } from 'hooks/usePermit2Allowance'
+import usePermit2Allowance, { AllowanceState } from 'hooks/usePermit2Allowance'
 import { useTokenBalance } from 'lib/hooks/useCurrencyBalance'
 import { useBag, useWalletBalance } from 'nft/hooks'
 import { useBagTotalEthPrice } from 'nft/hooks/useBagTotalEthPrice'
 import useDerivedPayWithAnyTokenSwapInfo from 'nft/hooks/useDerivedPayWithAnyTokenSwapInfo'
 import usePayWithAnyTokenSwap from 'nft/hooks/usePayWithAnyTokenSwap'
-import usePermit2Approval from 'nft/hooks/usePermit2Approval'
 import { usePriceImpact } from 'nft/hooks/usePriceImpact'
 import { useTokenInput } from 'nft/hooks/useTokenInput'
 import { BagStatus } from 'nft/types'
@@ -30,8 +30,14 @@ jest.mock('lib/hooks/useCurrencyBalance')
 jest.mock('hooks/Tokens')
 jest.mock('nft/hooks/usePayWithAnyTokenSwap')
 jest.mock('nft/hooks/useDerivedPayWithAnyTokenSwapInfo')
-jest.mock('graphql/data/nft/NftUniversalRouterAddress')
-jest.mock('nft/hooks/usePermit2Approval')
+jest.mock('graphql/data/nft/NftUniversalRouterAddress', () => {
+  const originalModule = jest.requireActual('graphql/data/nft/NftUniversalRouterAddress')
+  return {
+    ...originalModule,
+    useNftUniversalRouterAddress: jest.fn(),
+  }
+})
+jest.mock('hooks/usePermit2Allowance')
 jest.mock('nft/hooks/usePriceImpact')
 
 const renderBagFooter = () => {
@@ -63,14 +69,9 @@ describe('BagFooter.tsx', () => {
       provider: undefined,
     })
 
-    mocked(usePermit2Approval).mockReturnValue({
-      allowance: {
-        state: AllowanceState.ALLOWED,
-        permitSignature: undefined,
-      },
-      isApprovalLoading: false,
-      isAllowancePending: false,
-      updateAllowance: () => Promise.resolve(),
+    mocked(usePermit2Allowance).mockReturnValue({
+      state: AllowanceState.ALLOWED,
+      permitSignature: undefined,
     })
     mocked(useNftUniversalRouterAddress).mockReturnValue({
       universalRouterAddress: undefined,
@@ -321,13 +322,8 @@ describe('BagFooter.tsx', () => {
   })
 
   it('loading allowance', () => {
-    mocked(usePermit2Approval).mockReturnValue({
-      allowance: {
-        state: AllowanceState.LOADING,
-      },
-      isAllowancePending: false,
-      isApprovalLoading: false,
-      updateAllowance: () => Promise.resolve(),
+    mocked(usePermit2Allowance).mockReturnValue({
+      state: AllowanceState.LOADING,
     })
 
     renderBagFooter()
@@ -338,24 +334,19 @@ describe('BagFooter.tsx', () => {
   })
 
   it('approval is loading', () => {
-    mocked(usePermit2Approval).mockReturnValue({
-      allowance: {
-        state: AllowanceState.REQUIRED,
-        isApprovalLoading: false,
-        isApprovalPending: false,
-        token: TEST_TOKEN_1,
-        approveAndPermit: () => Promise.resolve(),
-        approve: () => Promise.resolve(),
-        permit: () => Promise.resolve(),
-        revoke: () => Promise.resolve(),
-        needsSetupApproval: false,
-        needsPermitSignature: false,
-        isRevocationPending: false,
-        allowedAmount: CurrencyAmount.fromRawAmount(TEST_TOKEN_1, 0),
-      },
-      isAllowancePending: false,
+    mocked(usePermit2Allowance).mockReturnValue({
+      state: AllowanceState.REQUIRED,
       isApprovalLoading: true,
-      updateAllowance: () => Promise.resolve(),
+      isApprovalPending: false,
+      token: TEST_TOKEN_1,
+      approveAndPermit: () => Promise.resolve(),
+      approve: () => Promise.resolve(),
+      permit: () => Promise.resolve(),
+      revoke: () => Promise.resolve(),
+      needsSetupApproval: false,
+      needsPermitSignature: false,
+      isRevocationPending: false,
+      allowedAmount: CurrencyAmount.fromRawAmount(TEST_TOKEN_1, 0),
     })
 
     renderBagFooter()
@@ -366,24 +357,19 @@ describe('BagFooter.tsx', () => {
   })
 
   it('allowance to be confirmed in wallet', () => {
-    mocked(usePermit2Approval).mockReturnValue({
-      allowance: {
-        state: AllowanceState.REQUIRED,
-        isApprovalLoading: false,
-        isApprovalPending: false,
-        token: TEST_TOKEN_1,
-        approveAndPermit: () => Promise.resolve(),
-        approve: () => Promise.resolve(),
-        permit: () => Promise.resolve(),
-        revoke: () => Promise.resolve(),
-        needsSetupApproval: false,
-        needsPermitSignature: false,
-        isRevocationPending: false,
-        allowedAmount: CurrencyAmount.fromRawAmount(TEST_TOKEN_1, 0),
-      },
-      isAllowancePending: true,
+    mocked(usePermit2Allowance).mockReturnValue({
+      state: AllowanceState.REQUIRED,
       isApprovalLoading: false,
-      updateAllowance: () => Promise.resolve(),
+      isApprovalPending: true,
+      token: TEST_TOKEN_1,
+      approveAndPermit: () => Promise.resolve(),
+      approve: () => Promise.resolve(),
+      permit: () => Promise.resolve(),
+      revoke: () => Promise.resolve(),
+      needsSetupApproval: false,
+      needsPermitSignature: false,
+      isRevocationPending: false,
+      allowedAmount: CurrencyAmount.fromRawAmount(TEST_TOKEN_1, 0),
     })
 
     renderBagFooter()
@@ -394,24 +380,19 @@ describe('BagFooter.tsx', () => {
   })
 
   it('approve', () => {
-    mocked(usePermit2Approval).mockReturnValue({
-      allowance: {
-        state: AllowanceState.REQUIRED,
-        isApprovalLoading: false,
-        isApprovalPending: false,
-        token: TEST_TOKEN_1,
-        approveAndPermit: () => Promise.resolve(),
-        approve: () => Promise.resolve(),
-        permit: () => Promise.resolve(),
-        revoke: () => Promise.resolve(),
-        needsSetupApproval: false,
-        needsPermitSignature: false,
-        isRevocationPending: false,
-        allowedAmount: CurrencyAmount.fromRawAmount(TEST_TOKEN_1, 0),
-      },
-      isAllowancePending: false,
+    mocked(usePermit2Allowance).mockReturnValue({
+      state: AllowanceState.REQUIRED,
       isApprovalLoading: false,
-      updateAllowance: () => Promise.resolve(),
+      isApprovalPending: false,
+      token: TEST_TOKEN_1,
+      approveAndPermit: () => Promise.resolve(),
+      approve: () => Promise.resolve(),
+      permit: () => Promise.resolve(),
+      revoke: () => Promise.resolve(),
+      needsSetupApproval: false,
+      needsPermitSignature: false,
+      isRevocationPending: false,
+      allowedAmount: CurrencyAmount.fromRawAmount(TEST_TOKEN_1, 0),
     })
 
     renderBagFooter()
@@ -436,4 +417,12 @@ describe('BagFooter.tsx', () => {
     expect(buyButton.textContent).toBe('Pay Anyway')
     expect(buyButton).not.toBeDisabled()
   })
+
+  it('should use the correct UR address', () => {
+    expect(getURAddress(undefined)).toBe(undefined)
+    expect(getURAddress(ChainId.MAINNET)).toBe(UNIVERSAL_ROUTER_ADDRESS(ChainId.MAINNET))
+    expect(getURAddress(ChainId.MAINNET, 'test_nft_ur_address')).toBe('test_nft_ur_address')
+    expect(getURAddress(ChainId.OPTIMISM)).toBe(UNIVERSAL_ROUTER_ADDRESS(ChainId.OPTIMISM))
+    expect(getURAddress(10101010)).toBe(undefined)
+  })
 })

src/nft/components/bag/BagFooter.tsx

@@ -14,9 +14,9 @@ import CurrencySearchModal from 'components/SearchModal/CurrencySearchModal'
 import { LoadingBubble } from 'components/Tokens/loading'
 import { MouseoverTooltip } from 'components/Tooltip'
 import { isSupportedChain } from 'constants/chains'
-import { useNftUniversalRouterAddress } from 'graphql/data/nft/NftUniversalRouterAddress'
+import { getURAddress, useNftUniversalRouterAddress } from 'graphql/data/nft/NftUniversalRouterAddress'
 import { useCurrency } from 'hooks/Tokens'
-import { AllowanceState } from 'hooks/usePermit2Allowance'
+import usePermit2Allowance, { AllowanceState } from 'hooks/usePermit2Allowance'
 import { useStablecoinValue } from 'hooks/useStablecoinPrice'
 import { useSwitchChain } from 'hooks/useSwitchChain'
 import { useTokenBalance } from 'lib/hooks/useCurrencyBalance'
@@ -26,7 +26,6 @@ import { useBagTotalEthPrice } from 'nft/hooks/useBagTotalEthPrice'
 import useDerivedPayWithAnyTokenSwapInfo from 'nft/hooks/useDerivedPayWithAnyTokenSwapInfo'
 import { useFetchAssets } from 'nft/hooks/useFetchAssets'
 import usePayWithAnyTokenSwap from 'nft/hooks/usePayWithAnyTokenSwap'
-import usePermit2Approval from 'nft/hooks/usePermit2Approval'
 import { PriceImpact, usePriceImpact } from 'nft/hooks/usePriceImpact'
 import { useSubscribeTransactionState } from 'nft/hooks/useSubscribeTransactionState'
 import { useTokenInput } from 'nft/hooks/useTokenInput'
@@ -35,7 +34,7 @@ import { BagStatus } from 'nft/types'
 import { ethNumberStandardFormatter, formatWeiToDecimal } from 'nft/utils'
 import { PropsWithChildren, useEffect, useMemo, useState } from 'react'
 import { AlertTriangle, ChevronDown } from 'react-feather'
-import { InterfaceTrade, TradeState } from 'state/routing/types'
+import { InterfaceTrade, TradeFillType, TradeState } from 'state/routing/types'
 import styled, { useTheme } from 'styled-components/macro'
 import { ThemedText } from 'theme'
 import { shallow } from 'zustand/shallow'
@@ -312,10 +311,10 @@ export const BagFooter = ({ setModalIsOpen, eventProperties }: BagFooterProps) =
     maximumAmountIn,
     allowedSlippage,
   } = useDerivedPayWithAnyTokenSwapInfo(usingPayWithAnyToken ? inputCurrency : undefined, parsedOutputAmount)
-  const { allowance, isAllowancePending, isApprovalLoading, updateAllowance } = usePermit2Approval(
-    trade?.inputAmount.currency.isToken ? (trade?.inputAmount as CurrencyAmount<Token>) : undefined,
+  const allowance = usePermit2Allowance(
     maximumAmountIn,
-    universalRouterAddress
+    getURAddress(chainId, universalRouterAddress),
+    TradeFillType.Classic
   )
   const loadingAllowance = allowance.state === AllowanceState.LOADING || universalRouterAddressIsLoading
   usePayWithAnyTokenSwap(trade, allowance, allowedSlippage)
@@ -401,18 +400,21 @@ export const BagFooter = ({ setModalIsOpen, eventProperties }: BagFooterProps) =
       return getBuyButtonStateData(BuyButtonStates.FETCHING_TOKEN_ROUTE, theme)
     }
 
-    if (allowance.state === AllowanceState.REQUIRED || loadingAllowance) {
-      const handleClick = () => updateAllowance()
+    const allowanceRequired = allowance.state === AllowanceState.REQUIRED
+    const handleClick = () => allowanceRequired && allowance.approveAndPermit()
 
-      if (loadingAllowance) {
-        return getBuyButtonStateData(BuyButtonStates.LOADING_ALLOWANCE, theme, handleClick)
-      } else if (isAllowancePending) {
+    if (loadingAllowance) {
+      return getBuyButtonStateData(BuyButtonStates.LOADING_ALLOWANCE, theme, handleClick)
+    }
+
+    if (allowanceRequired) {
+      if (allowance.isApprovalPending) {
         return getBuyButtonStateData(BuyButtonStates.IN_WALLET_ALLOWANCE_APPROVAL, theme, handleClick)
-      } else if (isApprovalLoading) {
+      } else if (allowance.isApprovalLoading) {
         return getBuyButtonStateData(BuyButtonStates.PROCESSING_APPROVAL, theme, handleClick)
+      } else {
+        return getBuyButtonStateData(BuyButtonStates.REQUIRE_APPROVAL, theme, handleClick)
       }
-
-      return getBuyButtonStateData(BuyButtonStates.REQUIRE_APPROVAL, theme, handleClick)
     }
 
     if (bagStatus === BagStatus.CONFIRM_QUOTE) {
@@ -437,8 +439,8 @@ export const BagFooter = ({ setModalIsOpen, eventProperties }: BagFooterProps) =
     bagStatus,
     usingPayWithAnyToken,
     tradeState,
-    allowance.state,
     loadingAllowance,
+    allowance,
     priceImpact,
     theme,
     fetchAssets,
@@ -446,9 +448,6 @@ export const BagFooter = ({ setModalIsOpen, eventProperties }: BagFooterProps) =
     connector,
     toggleWalletDrawer,
     setBagExpanded,
-    isAllowancePending,
-    isApprovalLoading,
-    updateAllowance,
   ])
 
   const traceEventProperties = {

src/nft/hooks/usePermit2Approval.test.ts

-import { CurrencyAmount } from '@uniswap/sdk-core'
-import { USDC_MAINNET } from '@uniswap/smart-order-router'
-import { UNIVERSAL_ROUTER_ADDRESS } from '@uniswap/universal-router-sdk'
-import usePermit2Allowance, { AllowanceState } from 'hooks/usePermit2Allowance'
-import { renderHook } from 'test-utils/render'
-
-import usePermit2Approval from './usePermit2Approval'
-
-const USDCAmount = CurrencyAmount.fromRawAmount(USDC_MAINNET, '10000')
-
-jest.mock('hooks/usePermit2Allowance')
-
-const mockUsePermit2Allowance = usePermit2Allowance as jest.MockedFunction<typeof usePermit2Allowance>
-
-describe('usePermit2Approval', () => {
-  it('sets spender of the correct UR contract from NFT side', async () => {
-    mockUsePermit2Allowance.mockReturnValue({ state: AllowanceState.LOADING })
-    renderHook(() => usePermit2Approval(USDCAmount, undefined, UNIVERSAL_ROUTER_ADDRESS(1)))
-    expect(mockUsePermit2Allowance).toHaveBeenCalledWith(USDCAmount, UNIVERSAL_ROUTER_ADDRESS(1))
-  })
-})

src/nft/hooks/usePermit2Approval.ts

-import { InterfaceEventName } from '@uniswap/analytics-events'
-import { ChainId, CurrencyAmount, Token } from '@uniswap/sdk-core'
-import { UNIVERSAL_ROUTER_ADDRESS } from '@uniswap/universal-router-sdk'
-import { useWeb3React } from '@web3-react/core'
-import { sendAnalyticsEvent } from 'analytics'
-import { isSupportedChain } from 'constants/chains'
-import usePermit2Allowance, { AllowanceState } from 'hooks/usePermit2Allowance'
-import { useCallback, useMemo, useState } from 'react'
-import invariant from 'tiny-invariant'
-
-function getURAddress(chainId?: number, nftURAddress?: string) {
-  if (!chainId) return
-
-  // if mainnet and on NFT flow, use the contract address returned by GQL
-  if (chainId === ChainId.MAINNET) {
-    return nftURAddress ?? UNIVERSAL_ROUTER_ADDRESS(chainId)
-  }
-
-  return isSupportedChain(chainId) ? UNIVERSAL_ROUTER_ADDRESS(chainId) : undefined
-}
-
-export default function usePermit2Approval(
-  amount: CurrencyAmount<Token> | undefined,
-  maximumAmount: CurrencyAmount<Token> | undefined,
-  nftUniversalRouterContractAddress?: string
-) {
-  const { chainId } = useWeb3React()
-
-  const universalRouterAddress = getURAddress(chainId, nftUniversalRouterContractAddress)
-  const allowanceAmount = maximumAmount ?? (amount?.currency.isToken ? (amount as CurrencyAmount<Token>) : undefined)
-  const allowance = usePermit2Allowance(allowanceAmount, universalRouterAddress)
-  const isApprovalLoading = allowance.state === AllowanceState.REQUIRED && allowance.isApprovalLoading
-  const [isAllowancePending, setIsAllowancePending] = useState(false)
-  const updateAllowance = useCallback(async () => {
-    invariant(allowance.state === AllowanceState.REQUIRED)
-    setIsAllowancePending(true)
-    try {
-      await allowance.approveAndPermit()
-      sendAnalyticsEvent(InterfaceEventName.APPROVE_TOKEN_TXN_SUBMITTED, {
-        chain_id: chainId,
-        token_symbol: maximumAmount?.currency.symbol,
-        token_address: maximumAmount?.currency.address,
-      })
-    } catch (e) {
-      console.error(e)
-    } finally {
-      setIsAllowancePending(false)
-    }
-  }, [allowance, chainId, maximumAmount?.currency.address, maximumAmount?.currency.symbol])
-
-  return useMemo(() => {
-    return {
-      allowance,
-      isApprovalLoading,
-      isAllowancePending,
-      updateAllowance,
-    }
-  }, [allowance, isAllowancePending, isApprovalLoading, updateAllowance])
-}