Merge branch 'v2' into feature/keystore-extra-entropy

src/controllers/defiPositions/defiPositions.ts

@@ -74,12 +74,16 @@ export class DefiPositionsController extends EventEmitter {
     })
   }
 
-  #getCanSkipUpdate(accountAddr: string, networkId: string) {
+  #getCanSkipUpdate(
+    accountAddr: string,
+    networkId: string,
+    maxDataAgeMs = this.#minUpdateInterval
+  ) {
     const networkState = this.#state[accountAddr][networkId]
 
     if (networkState.error || networkState.providerErrors?.length) return false
     const isWithinMinUpdateInterval =
-      networkState.updatedAt && Date.now() - networkState.updatedAt < this.#minUpdateInterval
+      networkState.updatedAt && Date.now() - networkState.updatedAt < maxDataAgeMs
 
     return isWithinMinUpdateInterval || networkState.isLoading
   }
@@ -101,7 +105,8 @@ export class DefiPositionsController extends EventEmitter {
     )
   }
 
-  async updatePositions(networkId?: NetworkId) {
+  async updatePositions(opts?: { networkId?: NetworkId; maxDataAgeMs?: number }) {
+    const { networkId, maxDataAgeMs } = opts || {}
     if (!this.#selectedAccount.account) return
 
     const selectedAccountAddr = this.#selectedAccount.account.addr
@@ -123,7 +128,7 @@ export class DefiPositionsController extends EventEmitter {
           }
         }
 
-        if (this.#getCanSkipUpdate(selectedAccountAddr, n.id)) {
+        if (this.#getCanSkipUpdate(selectedAccountAddr, n.id, maxDataAgeMs)) {
           // Emit an update so that the current account data getter is updated
           this.emitUpdate()
           return

src/controllers/main/main.ts

@@ -391,9 +391,9 @@ export class MainController extends EventEmitter {
     const selectedAccountAddr = this.selectedAccount.account?.addr
     this.domains.batchReverseLookup(this.accounts.accounts.map((a) => a.addr))
     if (!this.activity.broadcastedButNotConfirmed.length) {
-      this.updateSelectedAccountPortfolio(undefined, undefined, FIVE_MINUTES)
       // Update defi positions together with the portfolio for simplicity
-      this.defiPositions.updatePositions()
+      this.defiPositions.updatePositions({ maxDataAgeMs: FIVE_MINUTES })
+      this.updateSelectedAccountPortfolio(undefined, undefined, FIVE_MINUTES)
     }
 
     if (selectedAccountAddr && !this.accounts.areAccountStatesLoading)
@@ -412,8 +412,8 @@ export class MainController extends EventEmitter {
     await this.accounts.initialLoadPromise
     await this.selectedAccount.initialLoadPromise
 
-    this.updateSelectedAccountPortfolio()
     this.defiPositions.updatePositions()
+    this.updateSelectedAccountPortfolio()
     this.domains.batchReverseLookup(this.accounts.accounts.map((a) => a.addr))
     /**
      * Listener that gets triggered as a finalization step of adding new
@@ -1028,7 +1028,7 @@ export class MainController extends EventEmitter {
       // Additionally, if we trigger the portfolio update twice (i.e., running a long-living interval + force update from the Dashboard),
       // there won't be any error thrown, as all portfolio updates are queued and they don't use the `withStatus` helper.
       this.updateSelectedAccountPortfolio(forceUpdate, networkToUpdate),
-      this.defiPositions.updatePositions(networkId)
+      this.defiPositions.updatePositions({ networkId })
     ])
   }
 

src/controllers/selectedAccount/selectedAccount.ts

@@ -138,7 +138,7 @@ export class SelectedAccountController extends EventEmitter {
       this.#debounceFunctionCallsOnSameTick('updateSelectedAccountDefiPositions', () => {
         this.#updateSelectedAccountDefiPositions()
 
-        if (!this.areDefiPositionsLoading && this.portfolio.isReadyToVisualize) {
+        if (!this.areDefiPositionsLoading) {
           this.#updateSelectedAccountPortfolio(true)
           this.#updateDefiPositionsErrors()
         }
@@ -246,6 +246,7 @@ export class SelectedAccountController extends EventEmitter {
       pendingStateSelectedAccountWithDefiPositions,
       this.portfolio,
       this.portfolioStartedLoadingAtTimestamp,
+      defiPositionsAccountState,
       hasSignAccountOp
     )
 

src/libs/errorHumanizer/helpers.test.ts

@@ -62,4 +62,28 @@ describe('Generic error fallbacks work', () => {
 
     restore()
   })
+  it('Innercall failure error with reason', () => {
+    const message = getGenericMessageFromType(
+      ErrorType.InnerCallFailureError,
+      'The contract reverted',
+      MESSAGE_PREFIX,
+      ''
+    )
+
+    expect(message).toBe(
+      `${MESSAGE_PREFIX} it will revert onchain. Error code: The contract reverted\n`
+    )
+  })
+  it('Innercall failure error with no reason', () => {
+    const message = getGenericMessageFromType(
+      ErrorType.InnerCallFailureError,
+      '0x',
+      MESSAGE_PREFIX,
+      ''
+    )
+
+    expect(message).toBe(
+      `${MESSAGE_PREFIX} it will revert onchain with reason unknown.\nPlease try again or contact Ambire support for assistance.`
+    )
+  })
 })

src/libs/errorHumanizer/helpers.ts

@@ -25,9 +25,9 @@ function getGenericMessageFromType(
     case ErrorType.UnknownError:
       return `${messagePrefix} of an unknown error.${messageSuffix}`
     case ErrorType.InnerCallFailureError:
-      return (
-        reason ?? `${messagePrefix} it will revert onchain with reason unknown.${messageSuffix}`
-      )
+      return reasonString
+        ? `${messagePrefix} it will revert onchain.${messageSuffixNoSupport}`
+        : `${messagePrefix} it will revert onchain with reason unknown.${messageSuffix}`
     // I don't think we should say anything else for this case
     case ErrorType.UserRejectionError:
       return 'Transaction rejected.'

src/libs/selectedAccount/selectedAccount.ts

@@ -1,5 +1,3 @@
-import { getAddress } from 'ethers'
-
 import {
   SelectedAccountPortfolio,
   SelectedAccountPortfolioState,
@@ -32,7 +30,7 @@ export const updatePortfolioStateWithDefiPositions = (
 
     if (!networkState?.result || defiPositionsAccountState[networkId]?.isLoading) return
 
-    let tokens = networkState.result.tokens || []
+    const tokens = networkState.result.tokens || []
     let networkBalance = networkState.result.total?.usd || 0
 
     const positions = defiPositionsAccountState[networkId] || {}
@@ -47,80 +45,77 @@ export const updatePortfolioStateWithDefiPositions = (
         pos.assets
           .filter((a) => a.type !== AssetType.Liquidity && a.protocolAsset)
           .forEach((a) => {
-            const tokenInPortfolioIndex = tokens.findIndex((t) => {
+            const tokenInPortfolio = tokens.find((t) => {
               return (
-                getAddress(t.address) === getAddress(a.protocolAsset!.address) &&
-                t.networkId === networkId
+                t.address.toLowerCase() === (a.protocolAsset?.address || '').toLowerCase() &&
+                t.networkId === networkId &&
+                !t.flags.rewardsType &&
+                !t.flags.onGasTank
               )
             })
-            let amountPostSimulation
-            let simulationAmount
 
-            if (tokenInPortfolioIndex !== -1) {
-              const tokenInPortfolio = tokens[tokenInPortfolioIndex]
-              const priceUSD = tokenInPortfolio.priceIn.find(
+            if (tokenInPortfolio?.flags.isHidden) return
+
+            // Add only the balance of the collateral tokens to the network balance
+            if (a.type === AssetType.Collateral) {
+              const protocolPriceUSD = a.priceIn.find(
                 ({ baseCurrency }: { baseCurrency: string }) => baseCurrency.toLowerCase() === 'usd'
               )?.price
 
-              amountPostSimulation = tokenInPortfolio.amountPostSimulation
-              simulationAmount = tokenInPortfolio.simulationAmount
-              const tokenBalanceUSD = priceUSD
+              const protocolTokenBalanceUSD = protocolPriceUSD
                 ? Number(
                     safeTokenAmountAndNumberMultiplication(
-                      BigInt(amountPostSimulation || tokenInPortfolio.amount),
-                      tokenInPortfolio.decimals,
-                      priceUSD
+                      BigInt(tokenInPortfolio?.amountPostSimulation || a.amount),
+                      Number(a.protocolAsset!.decimals),
+                      protocolPriceUSD
                     )
                   )
                 : undefined
 
-              networkBalance -= tokenBalanceUSD || 0 // deduct portfolio token balance
-              tokens = tokens.filter((_, index) => index !== tokenInPortfolioIndex)
+              networkBalance += protocolTokenBalanceUSD || 0
             }
 
-            // Add only the balance of the collateral tokens to the network balance
-            if (a.type === AssetType.Collateral) {
-              const protocolPriceUSD = a.priceIn.find(
+            if (tokenInPortfolio) {
+              const priceUSD = tokenInPortfolio.priceIn.find(
                 ({ baseCurrency }: { baseCurrency: string }) => baseCurrency.toLowerCase() === 'usd'
               )?.price
 
-              const protocolTokenBalanceUSD = protocolPriceUSD
+              const tokenBalanceUSD = priceUSD
                 ? Number(
                     safeTokenAmountAndNumberMultiplication(
-                      BigInt(amountPostSimulation || a.amount),
-                      Number(a.protocolAsset!.decimals),
-                      protocolPriceUSD
+                      BigInt(tokenInPortfolio.amountPostSimulation || tokenInPortfolio.amount),
+                      tokenInPortfolio.decimals,
+                      priceUSD
                     )
                   )
                 : undefined
 
-              networkBalance += protocolTokenBalanceUSD || 0
-            }
-
-            const positionAsset = {
-              amount: a.amount,
-              amountPostSimulation: a.amountPostSimulation,
-              simulationAmount: a.simulationAmount,
-              // Only list the borrowed asset with no price
-              priceIn: a.type === AssetType.Collateral ? a.priceIn : [],
-              decimals: Number(a.protocolAsset!.decimals),
-              address: a.protocolAsset!.address,
-              symbol: a.protocolAsset!.symbol,
-              networkId,
-              flags: {
-                canTopUpGasTank: false,
-                isFeeToken: false,
-                onGasTank: false,
-                rewardsType: null
+              networkBalance -= tokenBalanceUSD || 0 // deduct portfolio token balance
+              // Get the price from defiPositions
+              tokenInPortfolio.priceIn = a.type === AssetType.Collateral ? a.priceIn : []
+            } else {
+              const positionAsset: TokenResult = {
+                amount: a.amount,
+                // Only list the borrowed asset with no price
+                priceIn: a.type === AssetType.Collateral ? a.priceIn : [],
+                decimals: Number(a.protocolAsset!.decimals),
+                address: a.protocolAsset!.address,
+                symbol: a.protocolAsset!.symbol,
+                networkId,
+                flags: {
+                  canTopUpGasTank: false,
+                  isFeeToken: false,
+                  onGasTank: false,
+                  rewardsType: null,
+                  isDefiToken: true
+                  // @BUG: defi positions tokens can't be hidden and can be added as custom
+                  // because processTokens is called in the portfolio
+                  // Issue: https://github.com/AmbireTech/ambire-app/issues/3971
+                }
               }
-            }
 
-            if (simulationAmount) {
-              positionAsset.simulationAmount = simulationAmount
-              positionAsset.amountPostSimulation = amountPostSimulation
+              tokens.push(positionAsset)
             }
-
-            tokens.push(positionAsset)
           })
       })
     })
@@ -204,6 +199,7 @@ export function calculateSelectedAccountPortfolio(
   pendingStateSelectedAccount: AccountState,
   accountPortfolio: SelectedAccountPortfolio | null,
   portfolioStartedLoadingAtTimestamp: number | null,
+  defiPositionsAccountState: DefiPositionsAccountState,
   hasSignAccountOp?: boolean
 ) {
   const now = Date.now()
@@ -293,7 +289,8 @@ export function calculateSelectedAccountPortfolio(
       collections.push(...networkCollections)
     }
 
-    if (!isNetworkReady(networkData)) {
+    // The total balance and token list are affected by the defi positions
+    if (!isNetworkReady(networkData) || defiPositionsAccountState[network]?.isLoading) {
       isAllReady = false
     }
   })