feat: portfolio refresh on new tx (#1179)

* feat: portfolio refresh on new tx * fix: force refetch and get the most recent tx, not the oldest * wip: comments * fix: wip - fixing doubling asset balance * fix: assets balances on tx * fix: using accounts to calculate total balance instead of assets. * refactor: bn changed to bnorzero * fix: wrong logic fixed * fix: fixing ui flickers * fix: selector reverted * fix: using tx related account * fix: logic fixed * fix: requested changes applied * fix: lowercase account specifier * chore: comment outstanding work * chore: tidy and flatten logic * feat: refetch portfolio on new txid, handles all chains and scenarios. change unique tx id delimiters * chore: comments and stricter type defs * fix: lowercase ETH public key intercept URL * fix: use expected button text in test * Update src/context/PortfolioProvider/PortfolioContext.tsx Co-authored-by: Apotheosis <[email protected]> * Update src/state/slices/portfolioSlice/portfolioSlice.ts Co-authored-by: Apotheosis <[email protected]> * Update src/state/slices/txHistorySlice/utils.ts Co-authored-by: Apotheosis <[email protected]> * Update src/context/PortfolioProvider/PortfolioContext.tsx Co-authored-by: Apotheosis <[email protected]> * Update src/state/slices/txHistorySlice/utils.ts Co-authored-by: Apotheosis <[email protected]> * fix: effect dependencies based on PR review Co-authored-by: stackedQ <[email protected]> Co-authored-by: Apotheosis <[email protected]>
shapeshift · Apr 10, 2022 · 5003516 · 5003516
1 parent 60ffe3f
commit 5003516
Show file tree

Hide file tree

Showing 8 changed files with 169 additions and 46 deletions.
diff --git a/cypress/integration/dashboard_spec.ts b/cypress/integration/dashboard_spec.ts
@@ -44,7 +44,7 @@ describe('The Dashboard', () => {
     cy.getBySel('trade-preview-button').should('be.disabled')
     cy.getBySel('token-row-sell-max-button').click()
     // TODO@0xApotheosis - this timeout won't be necessary once external request bounty complete
-    cy.getBySel('trade-preview-button').should('have.text', 'Not enough ETH to cover gas', {
+    cy.getBySel('trade-preview-button').should('have.text', 'Insufficient Funds', {
       timeout: 30000,
     })
     // TODO - We are now at the approval screen - test the rest of the flow

diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts
@@ -112,9 +112,11 @@ Cypress.Commands.add('mockExternalRequests', () => {
 
 // @ts-ignore
 Cypress.Commands.add('mockInternalRequests', () => {
-  cy.intercept('GET', `${ethereumApi}/api/v1/account/${publicKey}`, makeEthAccount()).as(
-    'getEthAccount',
-  )
+  cy.intercept(
+    'GET',
+    `${ethereumApi}/api/v1/account/${publicKey.toLowerCase()}`,
+    makeEthAccount(),
+  ).as('getEthAccount')
   cy.intercept('GET', `${bitcoinApi}/api/v1/account/${publicKey}`, makeBtcAccount()).as(
     'getBtcAccount',
   )

diff --git a/src/context/PortfolioProvider/PortfolioContext.tsx b/src/context/PortfolioProvider/PortfolioContext.tsx
@@ -12,8 +12,10 @@ import {
   supportsOsmosis,
 } from '@shapeshiftoss/hdwallet-core'
 import { ChainTypes, NetworkTypes } from '@shapeshiftoss/types'
+import difference from 'lodash/difference'
+import head from 'lodash/head'
 import isEmpty from 'lodash/isEmpty'
-import React, { useEffect, useState } from 'react'
+import React, { useCallback, useEffect, useState } from 'react'
 import { useDispatch, useSelector } from 'react-redux'
 import { useChainAdapters } from 'context/PluginProvider/PluginProvider'
 import { useWallet } from 'hooks/useWallet/useWallet'
@@ -25,12 +27,18 @@ import { useGetAssetsQuery } from 'state/slices/assetsSlice/assetsSlice'
 import { marketApi, useFindAllQuery } from 'state/slices/marketDataSlice/marketDataSlice'
 import { portfolio, portfolioApi } from 'state/slices/portfolioSlice/portfolioSlice'
 import { supportedAccountTypes } from 'state/slices/portfolioSlice/portfolioSliceCommon'
+import { cosmosChainId } from 'state/slices/portfolioSlice/utils'
 import {
   selectAccountSpecifiers,
   selectAssetIds,
   selectAssets,
   selectPortfolioAssetIds,
+  selectTxHistoryStatus,
+  selectTxIds,
+  selectTxs,
 } from 'state/slices/selectors'
+import { TxId } from 'state/slices/txHistorySlice/txHistorySlice'
+import { deserializeUniqueTxId } from 'state/slices/txHistorySlice/utils'
 
 /**
  * note - be super careful playing with this component, as it's responsible for asset,
@@ -51,6 +59,9 @@ export const PortfolioProvider = ({ children }: { children: React.ReactNode }) =
   const assetsById = useSelector(selectAssets)
   const assetIds = useSelector(selectAssetIds)
 
+  // keep track of pending tx ids, so we can refetch the portfolio when they confirm
+  const [pendingTxIds, setPendingTxIds] = useState<Set<TxId>>(new Set<TxId>())
+
   // immediately load all assets, before the wallet is even connected,
   // so the app is functional and ready
   useGetAssetsQuery()
@@ -107,7 +118,7 @@ export const PortfolioProvider = ({ children }: { children: React.ReactNode }) =
               const pubkey = await adapter.getAddress({ wallet })
               if (!pubkey) continue
               const CAIP2 = caip2.toCAIP2({ chain, network: NetworkTypes.MAINNET })
-              acc.push({ [CAIP2]: pubkey })
+              acc.push({ [CAIP2]: pubkey.toLowerCase() })
               break
             }
             case ChainTypes.Bitcoin: {
@@ -171,6 +182,80 @@ export const PortfolioProvider = ({ children }: { children: React.ReactNode }) =
     })()
   }, [assetsById, chainAdapter, dispatch, wallet])
 
+  const txIds = useSelector(selectTxIds)
+  const txsById = useSelector(selectTxs)
+  const txHistoryStatus = useSelector(selectTxHistoryStatus)
+
+  /**
+   * refetch an account given a newly confirmed txid
+   */
+  const refetchAccountByTxId = useCallback(
+    (txId: TxId) => {
+      // the accountSpecifier the tx came from
+      const { txAccountSpecifier } = deserializeUniqueTxId(txId)
+      // only refetch the specific account for this tx
+      const accountSpecifierMap = accountSpecifiersList.reduce((acc, cur) => {
+        const [chainId, accountSpecifier] = Object.entries(cur)[0]
+        const accountId = `${chainId}:${accountSpecifier}`
+        if (accountId === txAccountSpecifier) acc[chainId] = accountSpecifier
+        return acc
+      }, {})
+      const { getAccount } = portfolioApi.endpoints
+      dispatch(getAccount.initiate({ accountSpecifierMap }, { forceRefetch: true }))
+    },
+    [accountSpecifiersList, dispatch],
+  )
+
+  /**
+   * monitor for new pending txs, add them to a set, so we can monitor when they're confirmed
+   */
+  useEffect(() => {
+    // we only want to refetch portfolio if a new tx comes in after we're finished loading
+    if (txHistoryStatus !== 'loaded') return
+    // don't fire with nothing connected
+    if (isEmpty(accountSpecifiers)) return
+    // grab the most recent txId
+    const txId = head(txIds)!
+    // grab the actual tx
+    const tx = txsById[txId]
+    // always wear protection, or don't it's your choice really
+    if (!tx) return
+
+    if (tx.caip2 === cosmosChainId) {
+      // cosmos txs only come in when they're confirmed, so refetch that account immediately
+      return refetchAccountByTxId(txId)
+    } else {
+      /**
+       * the unchained getAccount call does not include pending txs in the portfolio
+       * add them to a set, and the two effects below monitor the set of pending txs
+       */
+      if (tx.status === 'pending') setPendingTxIds(new Set([...pendingTxIds, txId]))
+    }
+
+    // txsById changes on each tx - as txs have more confirmations
+    // pendingTxIds is changed by this effect, so don't create an infinite loop
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [txIds, txHistoryStatus, refetchAccountByTxId])
+
+  /**
+   * monitor the pending tx ids for when they change to confirmed.
+   * when they do change to confirmed, refetch the portfolio for that chain
+   * (unchained does not include assets for pending txs)
+   */
+  useEffect(() => {
+    // don't monitor any of this stuff if we're still loading - txsByIds will be thrashing
+    if (txHistoryStatus !== 'loaded') return
+    if (!pendingTxIds.size) return
+    // can't map a set, spread it first
+    const confirmedTxIds = [...pendingTxIds].filter(txId => txsById[txId]?.status === 'confirmed')
+    // txsById will change often, but we only care that if they've gone from pending -> confirmed
+    if (!confirmedTxIds.length) return
+    // refetch the account for each newly confirmed tx
+    confirmedTxIds.forEach(txId => refetchAccountByTxId(txId))
+    // stop monitoring the pending tx ids that have now been confirmed
+    setPendingTxIds(new Set([...difference([...pendingTxIds], confirmedTxIds)]))
+  }, [pendingTxIds, refetchAccountByTxId, txsById, txHistoryStatus])
+
   // we only prefetch market data for the top 1000 assets
   // once the portfolio has loaded, check we have market data
   // for more obscure assets, if we don't have it, fetch it

diff --git a/src/state/slices/portfolioSlice/portfolioSlice.ts b/src/state/slices/portfolioSlice/portfolioSlice.ts
@@ -1,7 +1,6 @@
 import { createSlice } from '@reduxjs/toolkit'
 import { createApi, fetchBaseQuery } from '@reduxjs/toolkit/query/react'
 import { CAIP2, caip2 } from '@shapeshiftoss/caip'
-import { mergeWith } from 'lodash'
 import cloneDeep from 'lodash/cloneDeep'
 import isEmpty from 'lodash/isEmpty'
 import { getChainAdapters } from 'context/PluginProvider/PluginProvider'
@@ -12,15 +11,6 @@ import { AccountSpecifierMap } from '../accountSpecifiersSlice/accountSpecifiers
 import { initialState, Portfolio } from './portfolioSliceCommon'
 import { accountToPortfolio } from './utils'
 
-// for assetBalances, they're aggregated across all accounts, so we need to
-// upsert and sum the balances by id
-// https://lodash.com/docs/4.17.15#mergeWith
-const upsertAndSum = (dest: string, src: string) => {
-  // if we have an existing balance for this asset, add to it
-  if (dest) return bnOrZero(dest).plus(bnOrZero(src)).toString()
-  // returning undefined uses default merge function, i.e. upsert
-}
-
 export const portfolio = createSlice({
   name: 'portfolio',
   initialState,
@@ -32,11 +22,31 @@ export const portfolio = createSlice({
       const accountIds = Array.from(new Set([...state.accounts.ids, ...payload.accounts.ids]))
       state.accounts.ids = accountIds
 
-      state.assetBalances.byId = mergeWith(
-        state.assetBalances.byId,
-        payload.assetBalances.byId,
-        upsertAndSum,
-      )
+      /**
+       * when fetching an account we got to calculate the difference between the
+       * state accountBalance and the payload accountBalance for each of account assets
+       * and then add [or subtract] the diff to the state.assetBalances related item.
+       */
+      payload.accountBalances.ids.forEach(accountSpecifier => {
+        // iterate over the account assets balances and calculate the diff
+        Object.entries(payload.accountBalances.byId[accountSpecifier]).forEach(
+          ([caip19, newAccountAssetBalance]) => {
+            // in case if getting accounts for the first time
+            const currentAccountBalance = bnOrZero(
+              state.accountBalances.byId[accountSpecifier]?.[caip19],
+            )
+            // diff could be both positive [tx type -> receive] and negative [tx type -> send]
+            const differenceBetweenCurrentAndNew =
+              bnOrZero(newAccountAssetBalance).minus(currentAccountBalance)
+            // get current asset balance from the state
+            const currentAssetBalance = bnOrZero(state.assetBalances.byId?.[caip19])
+            // update state.assetBalances with calculated diff
+            state.assetBalances.byId[caip19] = currentAssetBalance
+              .plus(differenceBetweenCurrentAndNew)
+              .toString()
+          },
+        )
+      })
 
       state.accountBalances.byId = {
         ...state.accountBalances.byId,

diff --git a/src/state/slices/txHistorySlice/txHistorySlice.test.ts b/src/state/slices/txHistorySlice/txHistorySlice.test.ts
@@ -5,7 +5,8 @@ import { BtcSend, ethereumTransactions, EthReceive, EthSend } from 'test/mocks/t
 import { store } from 'state/store'
 
 import { selectLastNTxIds } from './selectors'
-import { makeUniqueTxId, RebasesState, TxHistory, txHistory, TxsState } from './txHistorySlice'
+import { RebasesState, TxHistory, txHistory, TxsState } from './txHistorySlice'
+import { makeUniqueTxId } from './utils'
 
 describe('txHistorySlice', () => {
   beforeAll(() => {

diff --git a/src/state/slices/txHistorySlice/txHistorySlice.ts b/src/state/slices/txHistorySlice/txHistorySlice.ts
@@ -12,7 +12,7 @@ import {
   AccountSpecifierMap,
 } from 'state/slices/accountSpecifiersSlice/accountSpecifiersSlice'
 
-import { addToIndex, getRelatedAssetIds } from './utils'
+import { addToIndex, getRelatedAssetIds, makeUniqueTxId, UNIQUE_TX_ID_DELIMITER } from './utils'
 
 export type TxId = string
 export type Tx = chainAdapters.Transaction<ChainTypes> & { accountType?: UtxoAccountType }
@@ -120,26 +120,6 @@ const initialState: TxHistory = {
  * If transaction already exists, update the value, otherwise add the new transaction
  */
 
-/**
- * now we support accounts, we have a new problem
- * the same tx id can have multiple representations, depending on the
- * account's persective, especially utxos.
- *
- * i.e. a bitcoin send will have a send component, and a receive component for
- * the change, to a new address, but the same tx id.
- * this means we can't uniquely index tx's simply by their id.
- *
- * we'll probably need to go back to some composite index that can be built from
- * the txid and address, or account id, that can be deterministically generated,
- * from the tx data and the account id - note, not the address.
- *
- * the correct solution is to not rely on the parsed representation of the tx
- * as a "send" or "receive" from chain adapters, just index the tx related to the
- * asset or account, and parse the tx closer to the view layer.
- */
-export const makeUniqueTxId = (tx: Tx, accountId: AccountSpecifier): string =>
-  `${accountId}-${tx.txid}-${tx.address}`
-
 const updateOrInsertTx = (txHistory: TxHistory, tx: Tx, accountSpecifier: AccountSpecifier) => {
   const { txs } = txHistory
   const txid = makeUniqueTxId(tx, accountSpecifier)
@@ -225,7 +205,7 @@ type MakeRebaseIdArgs = {
 type MakeRebaseId = (args: MakeRebaseIdArgs) => string
 
 const makeRebaseId: MakeRebaseId = ({ accountId, assetId, rebase }) =>
-  `${accountId}-${assetId}-${rebase.blockTime}`
+  [accountId, assetId, rebase.blockTime].join(UNIQUE_TX_ID_DELIMITER)
 
 type TxHistoryStatusPayload = { payload: TxHistoryStatus }
 type RebaseHistoryPayload = {

diff --git a/src/state/slices/txHistorySlice/utils.test.ts b/src/state/slices/txHistorySlice/utils.test.ts
@@ -1,6 +1,6 @@
 import { BtcSend, EthReceive, EthSend, FOXSend, yearnVaultDeposit } from 'test/mocks/txs'
 
-import { addToIndex, getRelatedAssetIds } from './utils'
+import { addToIndex, deserializeUniqueTxId, getRelatedAssetIds, makeUniqueTxId } from './utils'
 
 describe('txHistorySlice:utils', () => {
   describe('getRelatedAssetIds', () => {
@@ -61,4 +61,18 @@ describe('txHistorySlice:utils', () => {
       expect(addToIndex([2, 1, 3], [3], 1)).toStrictEqual([1, 3])
     })
   })
+
+  describe('deserializeUniqueTxId', () => {
+    it('can deserialize a txId', () => {
+      const ethCAIP2 = EthSend.caip2
+      const accountSpecifier = `${ethCAIP2}:0xdef1cafe`
+      const txId = makeUniqueTxId(EthSend, accountSpecifier)
+
+      const { txAccountSpecifier, txid, txAddress } = deserializeUniqueTxId(txId)
+
+      expect(txAccountSpecifier).toEqual(accountSpecifier)
+      expect(txid).toEqual(EthSend.txid)
+      expect(txAddress).toEqual(EthSend.address)
+    })
+  })
 })
diff --git a/src/state/slices/txHistorySlice/utils.ts b/src/state/slices/txHistorySlice/utils.ts
@@ -2,7 +2,8 @@ import { CAIP19 } from '@shapeshiftoss/caip'
 import intersection from 'lodash/intersection'
 import union from 'lodash/union'
 
-import { Tx } from './txHistorySlice'
+import { AccountSpecifier } from '../portfolioSlice/portfolioSliceCommon'
+import { Tx, TxId } from './txHistorySlice'
 
 export const getRelatedAssetIds = (tx: Tx): CAIP19[] => {
   // we only want unique ids
@@ -23,3 +24,33 @@ export const getRelatedAssetIds = (tx: Tx): CAIP19[] => {
  */
 export const addToIndex = <T>(parentIndex: T[], childIndex: T[], newItem: T): T[] =>
   intersection(parentIndex, union(childIndex, [newItem]))
+
+/**
+ * now we support accounts, we have a new problem
+ * the same tx id can have multiple representations, depending on the
+ * account's perspective, especially utxos.
+ *
+ * i.e. a bitcoin send will have a send component, and a receive component for
+ * the change, to a new address, but the same tx id.
+ * this means we can't uniquely index tx's simply by their id.
+ *
+ * we'll probably need to go back to some composite index that can be built from
+ * the txid and address, or account id, that can be deterministically generated,
+ * from the tx data and the account id - note, not the address.
+ *
+ * the correct solution is to not rely on the parsed representation of the tx
+ * as a "send" or "receive" from chain adapters, just index the tx related to the
+ * asset or account, and parse the tx closer to the view layer.
+ */
+
+// we can't use a hyphen as a delimiter, as it appears in the chain reference for cosmos
+export const UNIQUE_TX_ID_DELIMITER = '*'
+export const makeUniqueTxId = (tx: Tx, accountId: AccountSpecifier): string =>
+  [accountId, tx.txid, tx.address].join(UNIQUE_TX_ID_DELIMITER)
+
+type DeserializeUniqueTxId = { txAccountSpecifier: string; txid: string; txAddress: string }
+
+export const deserializeUniqueTxId = (txId: TxId): DeserializeUniqueTxId => {
+  const [txAccountSpecifier, txid, txAddress] = txId.split(UNIQUE_TX_ID_DELIMITER)
+  return { txAccountSpecifier, txid, txAddress }
+}