Merge pull request #1095 from AmbireTech/optimization/make-defiPositi…

…onsCtrl-private

Optimization / Make DefiPositionsController State Private in MainController

src/controllers/defiPositions/defiPositions.test.ts

@@ -89,8 +89,8 @@ describe('DefiPositionsController', () => {
     const controller = await prepareTest()
 
     await controller.updatePositions()
-
-    expect(controller.state[ACCOUNT.addr].polygon.positionsByProvider.length).toBeGreaterThan(0)
+    const selectedAccountState = controller.getDefiPositionsState(ACCOUNT.addr)
+    expect(selectedAccountState.polygon.positionsByProvider.length).toBeGreaterThan(0)
   })
 
   it('should handle errors in update positions', async () => {
@@ -110,7 +110,8 @@ describe('DefiPositionsController', () => {
     const controller = await prepareTest()
     await controller.updatePositions()
 
-    expect(controller.state[ACCOUNT.addr].ethereum.providerErrors).toEqual([
+    const selectedAccountState = controller.getDefiPositionsState(ACCOUNT.addr)
+    expect(selectedAccountState.ethereum.providerErrors).toEqual([
       { providerName: 'AAVE v3', error: 'AAVE error' },
       { providerName: 'Uniswap V3', error: 'Uniswap error' }
     ])
@@ -122,7 +123,9 @@ describe('DefiPositionsController', () => {
     const controller = await prepareTest()
     await controller.updatePositions()
 
-    const positions = controller.state[ACCOUNT.addr].polygon.positionsByProvider
+    const selectedAccountState = controller.getDefiPositionsState(ACCOUNT.addr)
+
+    const positions = selectedAccountState.polygon.positionsByProvider
     expect(positions.length).toBeGreaterThan(0)
     positions.forEach((provider) => {
       provider.positions.forEach((position) => {
@@ -146,7 +149,8 @@ describe('DefiPositionsController', () => {
     const controller = await prepareTest()
     await controller.updatePositions()
 
-    const positions = controller.state[ACCOUNT.addr].polygon.positionsByProvider
+    const selectedAccountState = controller.getDefiPositionsState(ACCOUNT.addr)
+    const positions = selectedAccountState.polygon.positionsByProvider
     expect(positions.length).toBeGreaterThan(0)
     positions.forEach((provider) => {
       // AAVE positions get their prices from oracles

src/controllers/defiPositions/defiPositions.ts

@@ -1,7 +1,5 @@
 import { Fetch } from '../../interfaces/fetch'
 import { NetworkId } from '../../interfaces/network'
-// eslint-disable-next-line import/no-cycle
-import { getNetworksWithDeFiPositionsErrorBanners } from '../../libs/banners/banners'
 import { getAssetValue } from '../../libs/defiPositions/helpers'
 import { getAAVEPositions, getUniV3Positions } from '../../libs/defiPositions/providers'
 import {
@@ -26,7 +24,7 @@ export class DefiPositionsController extends EventEmitter {
 
   #minUpdateInterval: number = 60 * 1000 // 1 minute
 
-  state: DeFiPositionsState = {}
+  #state: DeFiPositionsState = {}
 
   constructor({
     fetch,
@@ -48,13 +46,13 @@ export class DefiPositionsController extends EventEmitter {
   }
 
   #initInitialAccountStateIfNeeded(accountAddr: string) {
-    if (!this.state[accountAddr]) {
-      this.state[accountAddr] = this.#networks.networks.reduce(
+    if (!this.#state[accountAddr]) {
+      this.#state[accountAddr] = this.#networks.networks.reduce(
         (acc, n) => ({
           ...acc,
           [n.id]: { isLoading: true, positionsByProvider: [] }
         }),
-        this.state[accountAddr]
+        this.#state[accountAddr]
       )
 
       this.emitUpdate()
@@ -67,18 +65,18 @@ export class DefiPositionsController extends EventEmitter {
     providerName: string,
     errorMessage: string
   ) {
-    if (!this.state[accountAddr][networkId].providerErrors) {
-      this.state[accountAddr][networkId].providerErrors = []
+    if (!this.#state[accountAddr][networkId].providerErrors) {
+      this.#state[accountAddr][networkId].providerErrors = []
     }
 
-    this.state[accountAddr][networkId].providerErrors!.push({
+    this.#state[accountAddr][networkId].providerErrors!.push({
       providerName,
       error: errorMessage
     })
   }
 
   #getCanSkipUpdate(accountAddr: string, networkId: string) {
-    const networkState = this.state[accountAddr][networkId]
+    const networkState = this.#state[accountAddr][networkId]
 
     if (networkState.isLoading) return false
     if (networkState.error) return false
@@ -106,7 +104,7 @@ export class DefiPositionsController extends EventEmitter {
           this.emitUpdate()
           return
         }
-        const networkState = this.state[selectedAccountAddr][n.id]
+        const networkState = this.#state[selectedAccountAddr][n.id]
 
         // Reset provider errors before updating
         networkState.providerErrors = []
@@ -143,7 +141,7 @@ export class DefiPositionsController extends EventEmitter {
             )
           ])
 
-          this.state[selectedAccountAddr][n.id] = {
+          this.#state[selectedAccountAddr][n.id] = {
             ...networkState,
             isLoading: false,
             positionsByProvider: [aavePositions, uniV3Positions].filter(
@@ -153,11 +151,11 @@ export class DefiPositionsController extends EventEmitter {
           }
           await this.#setAssetPrices(selectedAccountAddr, n.id).catch((e) => {
             console.error('#setAssetPrices error:', e)
-            this.state[selectedAccountAddr][n.id].error = DeFiPositionsError.AssetPriceError
+            this.#state[selectedAccountAddr][n.id].error = DeFiPositionsError.AssetPriceError
           })
         } catch (e: any) {
           const prevPositionsByProvider = networkState.positionsByProvider
-          this.state[selectedAccountAddr][n.id] = {
+          this.#state[selectedAccountAddr][n.id] = {
             isLoading: false,
             positionsByProvider: prevPositionsByProvider || [],
             error: DeFiPositionsError.CriticalError
@@ -173,7 +171,7 @@ export class DefiPositionsController extends EventEmitter {
   async #setAssetPrices(accountAddr: string, networkId: string) {
     const dedup = (x: any[]) => x.filter((y, i) => x.indexOf(y) === i)
 
-    const networkState = this.state[accountAddr][networkId]
+    const networkState = this.#state[accountAddr][networkId]
 
     const addresses: string[] = []
 
@@ -198,7 +196,7 @@ export class DefiPositionsController extends EventEmitter {
       // eslint-disable-next-line no-prototype-builtins
       if (body.hasOwnProperty('error')) throw body
 
-      const positionsByProviderWithPrices = this.state[accountAddr][
+      const positionsByProviderWithPrices = this.#state[accountAddr][
         networkId
       ].positionsByProvider.map((positionsByProvider) => {
         if (positionsByProvider.providerName.toLowerCase().includes('aave'))
@@ -246,29 +244,20 @@ export class DefiPositionsController extends EventEmitter {
         return { ...positionsByProvider, positions: updatedPositions, positionInUSD }
       })
 
-      this.state[accountAddr][networkId].positionsByProvider = positionsByProviderWithPrices
+      this.#state[accountAddr][networkId].positionsByProvider = positionsByProviderWithPrices
     } catch (error) {
       console.error('#setAssetPrices error in defiPositions:', error)
     }
   }
 
-  get banners() {
-    if (!this.#selectedAccount.account) return []
-
-    const errorBanners = getNetworksWithDeFiPositionsErrorBanners({
-      networks: this.#networks.networks,
-      currentAccountState: this.state[this.#selectedAccount.account.addr],
-      providers: this.#providers.providers
-    })
-
-    return errorBanners
+  getDefiPositionsState(accountAddr: string) {
+    return this.#state[accountAddr] || {}
   }
 
   toJSON() {
     return {
       ...this,
-      ...super.toJSON(),
-      banners: this.banners
+      ...super.toJSON()
     }
   }
 }

src/controllers/main/main.ts

@@ -314,7 +314,9 @@ export class MainController extends EventEmitter {
     this.selectedAccount.initControllers({
       portfolio: this.portfolio,
       defiPositions: this.defiPositions,
-      actions: this.actions
+      actions: this.actions,
+      networks: this.networks,
+      providers: this.providers
     })
     this.swapAndBridge = new SwapAndBridgeController({
       selectedAccount: this.selectedAccount,

src/controllers/selectedAccount/selectedAccount.test.ts

@@ -137,7 +137,9 @@ describe('SelectedAccount Controller', () => {
     await selectedAccountCtrl.initControllers({
       portfolio: portfolioCtrl,
       defiPositions: defiPositionsCtrl,
-      actions: actionsCtrl
+      actions: actionsCtrl,
+      networks: networksCtrl,
+      providers: providersCtrl
     })
     expect(selectedAccountCtrl.areControllersInitialized).toEqual(true)
   })

src/controllers/selectedAccount/selectedAccount.ts

@@ -1,6 +1,9 @@
 import { Account } from '../../interfaces/account'
+import { Banner } from '../../interfaces/banner'
 import { SelectedAccountPortfolio } from '../../interfaces/selectedAccount'
 import { Storage } from '../../interfaces/storage'
+// eslint-disable-next-line import/no-cycle
+import { getNetworksWithDeFiPositionsErrorBanners } from '../../libs/banners/banners'
 import { sortByValue } from '../../libs/defiPositions/helpers'
 import { PositionsByProvider } from '../../libs/defiPositions/types'
 import {
@@ -10,12 +13,14 @@ import {
 // eslint-disable-next-line import/no-cycle
 import { AccountsController } from '../accounts/accounts'
 // eslint-disable-next-line import/no-cycle
-import { Action, ActionsController } from '../actions/actions'
+import { ActionsController } from '../actions/actions'
 // eslint-disable-next-line import/no-cycle
 import { DefiPositionsController } from '../defiPositions/defiPositions'
 import EventEmitter from '../eventEmitter/eventEmitter'
+import { NetworksController } from '../networks/networks'
 // eslint-disable-next-line import/no-cycle
 import { PortfolioController } from '../portfolio/portfolio'
+import { ProvidersController } from '../providers/providers'
 
 export const DEFAULT_SELECTED_ACCOUNT_PORTFOLIO = {
   tokens: [],
@@ -39,6 +44,10 @@ export class SelectedAccountController extends EventEmitter {
 
   #actions: ActionsController | null = null
 
+  #networks: NetworksController | null = null
+
+  #providers: ProvidersController | null = null
+
   account: Account | null = null
 
   portfolio: SelectedAccountPortfolio = DEFAULT_SELECTED_ACCOUNT_PORTFOLIO
@@ -49,7 +58,7 @@ export class SelectedAccountController extends EventEmitter {
 
   defiPositions: PositionsByProvider[] = []
 
-  actions: Action[] = []
+  defiPositionsBanners: Banner[] = []
 
   isReady: boolean = false
 
@@ -83,19 +92,25 @@ export class SelectedAccountController extends EventEmitter {
   initControllers({
     portfolio,
     defiPositions,
-    actions
+    actions,
+    networks,
+    providers
   }: {
     portfolio: PortfolioController
     defiPositions: DefiPositionsController
     actions: ActionsController
+    networks: NetworksController
+    providers: ProvidersController
   }) {
     this.#portfolio = portfolio
     this.#defiPositions = defiPositions
     this.#actions = actions
+    this.#networks = networks
+    this.#providers = providers
 
     this.#updateSelectedAccountPortfolio(true)
     this.#updateSelectedAccountDefiPositions(true)
-    this.#updateSelectedAccountActions(true)
+    this.#updateDefiPositionsBanners(true)
 
     this.#portfolio.onUpdate(async () => {
       this.#debounceFunctionCallsOnSameTick('updateSelectedAccountPortfolio', () =>
@@ -110,10 +125,16 @@ export class SelectedAccountController extends EventEmitter {
       this.#debounceFunctionCallsOnSameTick('updateSelectedAccountDefiPositions', () =>
         this.#updateSelectedAccountDefiPositions()
       )
+      this.#debounceFunctionCallsOnSameTick('updateDefiPositionsBanners', () =>
+        this.#updateDefiPositionsBanners()
+      )
     })
 
-    this.#actions.onUpdate(() => {
-      this.#updateSelectedAccountActions()
+    this.#providers.onUpdate(() => {
+      this.#debounceFunctionCallsOnSameTick('updateDefiPositionsBanners', () =>
+        this.#updateDefiPositionsBanners()
+      )
+      // TODO: add portfolio banners and call updatePortfolioBanners here
     })
 
     this.areControllersInitialized = true
@@ -145,7 +166,7 @@ export class SelectedAccountController extends EventEmitter {
   #updateSelectedAccountPortfolio(skipUpdate?: boolean) {
     if (!this.#portfolio || !this.#defiPositions || !this.account) return
 
-    const defiPositionsAccountState = this.#defiPositions.state[this.account.addr]
+    const defiPositionsAccountState = this.#defiPositions.getDefiPositionsState(this.account.addr)
 
     const portfolioState = structuredClone({
       latest: this.#portfolio.latest,
@@ -157,7 +178,9 @@ export class SelectedAccountController extends EventEmitter {
       this.account
     )
 
-    const hasSignAccountOp = !!this.actions.filter((action) => action.type === 'accountOp')
+    const hasSignAccountOp = !!this.#actions?.visibleActionsQueue.filter(
+      (action) => action.type === 'accountOp'
+    )
 
     const newSelectedAccountPortfolio = calculateSelectedAccountPortfolio(
       this.account.addr,
@@ -189,17 +212,18 @@ export class SelectedAccountController extends EventEmitter {
   get areDefiPositionsLoading() {
     if (!this.account || !this.#defiPositions) return false
 
-    return Object.values(this.#defiPositions.state[this.account.addr] || {}).some(
-      (n) => n.isLoading
-    )
+    const defiPositionsAccountState = this.#defiPositions.getDefiPositionsState(this.account.addr)
+    return Object.values(defiPositionsAccountState).some((n) => n.isLoading)
   }
 
   #updateSelectedAccountDefiPositions(skipUpdate?: boolean) {
     if (!this.#defiPositions || !this.account) return
 
-    const positionsByProvider = Object.values(
-      this.#defiPositions.state[this.account.addr] || {}
-    ).flatMap((n) => n.positionsByProvider)
+    const defiPositionsAccountState = this.#defiPositions.getDefiPositionsState(this.account.addr)
+
+    const positionsByProvider = Object.values(defiPositionsAccountState).flatMap(
+      (n) => n.positionsByProvider
+    )
 
     const positionsByProviderWithSortedAssets = positionsByProvider.map((provider) => {
       const positions = provider.positions
@@ -224,28 +248,6 @@ export class SelectedAccountController extends EventEmitter {
     }
   }
 
-  #updateSelectedAccountActions(skipUpdate?: boolean) {
-    if (!this.#actions || !this.account) return
-
-    this.actions = this.#actions.actionsQueue.filter((a) => {
-      if (a.type === 'accountOp') {
-        return a.accountOp.accountAddr === this.account!.addr
-      }
-      if (a.type === 'signMessage') {
-        return a.userRequest.meta.accountAddr === this.account!.addr
-      }
-      if (a.type === 'benzin') {
-        return a.userRequest.meta.accountAddr === this.account!.addr
-      }
-
-      return true
-    })
-
-    if (!skipUpdate) {
-      this.emitUpdate()
-    }
-  }
-
   #debounceFunctionCallsOnSameTick(funcName: string, func: Function) {
     if (this.#shouldDebounceFlags[funcName]) return
     this.#shouldDebounceFlags[funcName] = true
@@ -257,6 +259,24 @@ export class SelectedAccountController extends EventEmitter {
     }, 0)
   }
 
+  #updateDefiPositionsBanners(skipUpdate?: boolean) {
+    if (!this.account || !this.#networks || !this.#providers || !this.#defiPositions) return
+
+    const defiPositionsAccountState = this.#defiPositions.getDefiPositionsState(this.account.addr)
+
+    const errorBanners = getNetworksWithDeFiPositionsErrorBanners({
+      networks: this.#networks.networks,
+      currentAccountState: defiPositionsAccountState,
+      providers: this.#providers.providers
+    })
+
+    this.defiPositionsBanners = errorBanners
+
+    if (!skipUpdate) {
+      this.emitUpdate()
+    }
+  }
+
   toJSON() {
     return {
       ...this,