fix: bug where user cannot burn lp position if fetching fee values fails. (#3633)

* fix: can burn position even if fetching fees fails.

* Revert "fix: can burn position even if fetching fees fails."

This reverts commit a96f7178e55a264c32a3cf8c1b6562785cbd50d4.

* recover more gracefully from failed fee fetch
Co-authored-by: Noah Zinsmeister <noahwz@gmail.com>

src/hooks/useV3PositionFees.ts

@@ -23,13 +23,11 @@ export function useV3PositionFees(
   const tokenIdHexString = tokenId?.toHexString()
   const latestBlockNumber = useBlockNumber()
 
-  // TODO find a way to get this into multicall
+  // we can't use multicall for this because we need to simulate the call from a specific address
   // latestBlockNumber is included to ensure data stays up-to-date every block
-  const [amounts, setAmounts] = useState<[BigNumber, BigNumber]>()
+  const [amounts, setAmounts] = useState<[BigNumber, BigNumber] | undefined>()
   useEffect(() => {
-    let stale = false
-
-    if (positionManager && tokenIdHexString && owner && typeof latestBlockNumber === 'number') {
+    if (positionManager && tokenIdHexString && owner) {
       positionManager.callStatic
         .collect(
           {
@@ -41,19 +39,15 @@ export function useV3PositionFees(
           { from: owner } // need to simulate the call as the owner
         )
         .then((results) => {
-          if (!stale) setAmounts([results.amount0, results.amount1])
+          setAmounts([results.amount0, results.amount1])
         })
     }
-
-    return () => {
-      stale = true
-    }
   }, [positionManager, tokenIdHexString, owner, latestBlockNumber])
 
   if (pool && amounts) {
     return [
-      CurrencyAmount.fromRawAmount(!asWETH ? unwrappedToken(pool.token0) : pool.token0, amounts[0].toString()),
-      CurrencyAmount.fromRawAmount(!asWETH ? unwrappedToken(pool.token1) : pool.token1, amounts[1].toString()),
+      CurrencyAmount.fromRawAmount(asWETH ? pool.token0 : unwrappedToken(pool.token0), amounts[0].toString()),
+      CurrencyAmount.fromRawAmount(asWETH ? pool.token1 : unwrappedToken(pool.token1), amounts[1].toString()),
     ]
   } else {
     return [undefined, undefined]

src/pages/Pool/PositionPage.tsx

@@ -336,11 +336,11 @@ export function PositionPage({
 
   const removed = liquidity?.eq(0)
 
+  const metadata = usePositionTokenURI(parsedTokenId)
+
   const token0 = useToken(token0Address)
   const token1 = useToken(token1Address)
 
-  const metadata = usePositionTokenURI(parsedTokenId)
-
   const currency0 = token0 ? unwrappedToken(token0) : undefined
   const currency1 = token1 ? unwrappedToken(token1) : undefined
 
@@ -389,6 +389,10 @@ export function PositionPage({
   // fees
   const [feeValue0, feeValue1] = useV3PositionFees(pool ?? undefined, positionDetails?.tokenId, receiveWETH)
 
+  // these currencies will match the feeValue{0,1} currencies for the purposes of fee collection
+  const currency0ForFeeCollectionPurposes = pool ? (receiveWETH ? pool.token0 : unwrappedToken(pool.token0)) : undefined
+  const currency1ForFeeCollectionPurposes = pool ? (receiveWETH ? pool.token1 : unwrappedToken(pool.token1)) : undefined
+
   const [collecting, setCollecting] = useState<boolean>(false)
   const [collectMigrationHash, setCollectMigrationHash] = useState<string | null>(null)
   const isCollectPending = useIsTransactionPending(collectMigrationHash ?? undefined)
@@ -422,14 +426,25 @@ export function PositionPage({
   const addTransaction = useTransactionAdder()
   const positionManager = useV3NFTPositionManagerContract()
   const collect = useCallback(() => {
-    if (!chainId || !feeValue0 || !feeValue1 || !positionManager || !account || !tokenId || !library) return
+    if (
+      !currency0ForFeeCollectionPurposes ||
+      !currency1ForFeeCollectionPurposes ||
+      !chainId ||
+      !positionManager ||
+      !account ||
+      !tokenId ||
+      !library
+    )
+      return
 
     setCollecting(true)
 
+    // we fall back to expecting 0 fees in case the fetch fails, which is safe in the
+    // vast majority of cases
     const { calldata, value } = NonfungiblePositionManager.collectCallParameters({
       tokenId: tokenId.toString(),
-      expectedCurrencyOwed0: feeValue0,
-      expectedCurrencyOwed1: feeValue1,
+      expectedCurrencyOwed0: feeValue0 ?? CurrencyAmount.fromRawAmount(currency0ForFeeCollectionPurposes, 0),
+      expectedCurrencyOwed1: feeValue1 ?? CurrencyAmount.fromRawAmount(currency1ForFeeCollectionPurposes, 0),
       recipient: account,
     })
 
@@ -458,13 +473,13 @@ export function PositionPage({
             ReactGA.event({
               category: 'Liquidity',
               action: 'CollectV3',
-              label: [feeValue0.currency.symbol, feeValue1.currency.symbol].join('/'),
+              label: [currency0ForFeeCollectionPurposes.symbol, currency1ForFeeCollectionPurposes.symbol].join('/'),
             })
 
             addTransaction(response, {
               type: TransactionType.COLLECT_FEES,
-              currencyId0: currencyId(feeValue0.currency),
-              currencyId1: currencyId(feeValue1.currency),
+              currencyId0: currencyId(currency0ForFeeCollectionPurposes),
+              currencyId1: currencyId(currency1ForFeeCollectionPurposes),
             })
           })
       })
@@ -472,7 +487,18 @@ export function PositionPage({
         setCollecting(false)
         console.error(error)
       })
-  }, [chainId, feeValue0, feeValue1, positionManager, account, tokenId, addTransaction, library])
+  }, [
+    chainId,
+    feeValue0,
+    feeValue1,
+    currency0ForFeeCollectionPurposes,
+    currency1ForFeeCollectionPurposes,
+    positionManager,
+    account,
+    tokenId,
+    addTransaction,
+    library,
+  ])
 
   const owner = useSingleCallResult(!!tokenId ? positionManager : null, 'ownerOf', [tokenId]).result?.[0]
   const ownsNFT = owner === account || positionDetails?.operator === account

src/pages/RemoveLiquidity/V3.tsx

 import { BigNumber } from '@ethersproject/bignumber'
 import { TransactionResponse } from '@ethersproject/providers'
 import { Trans } from '@lingui/macro'
-import { Percent } from '@uniswap/sdk-core'
+import { CurrencyAmount, Percent } from '@uniswap/sdk-core'
 import { NonfungiblePositionManager } from '@uniswap/v3-sdk'
 import RangeBadge from 'components/Badge/RangeBadge'
 import { ButtonConfirmed, ButtonPrimary } from 'components/Button'
@@ -109,8 +109,6 @@ function Remove({ tokenId }: { tokenId: BigNumber }) {
       !deadline ||
       !account ||
       !chainId ||
-      !feeValue0 ||
-      !feeValue1 ||
       !positionSDK ||
       !liquidityPercentage ||
       !library
@@ -118,14 +116,16 @@ function Remove({ tokenId }: { tokenId: BigNumber }) {
       return
     }
 
+    // we fall back to expecting 0 fees in case the fetch fails, which is safe in the
+    // vast majority of cases
     const { calldata, value } = NonfungiblePositionManager.removeCallParameters(positionSDK, {
       tokenId: tokenId.toString(),
       liquidityPercentage,
       slippageTolerance: allowedSlippage,
       deadline: deadline.toString(),
       collectOptions: {
-        expectedCurrencyOwed0: feeValue0,
-        expectedCurrencyOwed1: feeValue1,
+        expectedCurrencyOwed0: feeValue0 ?? CurrencyAmount.fromRawAmount(liquidityValue0.currency, 0),
+        expectedCurrencyOwed1: feeValue1 ?? CurrencyAmount.fromRawAmount(liquidityValue1.currency, 0),
         recipient: account,
       },
     })