update: forceUpdate portfolio only when needed

src/controllers/main/main.ts

@@ -233,7 +233,7 @@ export class MainController extends EventEmitter {
       async (network: Network) => {
         this.providers.setProvider(network)
         await this.accounts.updateAccountStates('latest', [network.id])
-        await this.updateSelectedAccountPortfolio(true)
+        await this.updateSelectedAccountPortfolio()
         await this.defiPositions.updatePositions(network.id)
       },
       (networkId: NetworkId) => {
@@ -354,9 +354,7 @@ export class MainController extends EventEmitter {
     await this.accounts.initialLoadPromise
     await this.selectedAccount.initialLoadPromise
 
-    // TODO: We agreed to always fetch the latest and pending states.
-    // To achieve this, we need to refactor how we use forceUpdate to obtain pending state updates.
-    this.updateSelectedAccountPortfolio(true)
+    this.updateSelectedAccountPortfolio()
     this.defiPositions.updatePositions()
     /**
      * Listener that gets triggered as a finalization step of adding new
@@ -425,9 +423,7 @@ export class MainController extends EventEmitter {
     }
     this.selectedAccount.setAccount(accountToSelect)
     this.activity.init()
-    // TODO: We agreed to always fetch the latest and pending states.
-    // To achieve this, we need to refactor how we use forceUpdate to obtain pending state updates.
-    await this.updateSelectedAccountPortfolio(true)
+    await this.updateSelectedAccountPortfolio()
     await this.defiPositions.updatePositions()
     // forceEmitUpdate to update the getters in the FE state of the ctrl
     await this.forceEmitUpdate()
@@ -925,7 +921,7 @@ export class MainController extends EventEmitter {
   }
 
   // eslint-disable-next-line default-param-last
-  async updateSelectedAccountPortfolio(forceUpdate: boolean = true, network?: Network) {
+  async updateSelectedAccountPortfolio(forceUpdate: boolean = false, network?: Network) {
     await this.#initialLoadPromise
     if (!this.selectedAccount.account) return
 
@@ -1541,7 +1537,7 @@ export class MainController extends EventEmitter {
 
   async addNetwork(network: AddNetworkRequestParams) {
     await this.networks.addNetwork(network)
-    await this.updateSelectedAccountPortfolio(true)
+    await this.updateSelectedAccountPortfolio()
   }
 
   async removeNetwork(id: NetworkId) {

src/controllers/portfolio/portfolio.test.ts

@@ -262,26 +262,20 @@ describe('Portfolio Controller ', () => {
   })
 
   describe('Latest tokens', () => {
-    test('Latest tokens are fetched and kept in the controller, while the pending should not be fetched (no AccountOp passed)', (done) => {
+    test('Latest tokens are fetched and kept in the controller', async () => {
       const { controller } = prepareTest()
 
-      controller.onUpdate(() => {
-        const latestState =
-          controller.latest['0xB674F3fd5F43464dB0448a57529eAF37F04cceA5']?.ethereum!
-        const pendingState =
-          controller.pending['0xB674F3fd5F43464dB0448a57529eAF37F04cceA5']?.ethereum
-        if (latestState && latestState.isReady) {
-          expect(latestState.isReady).toEqual(true)
-          expect(latestState.result?.tokens.length).toBeGreaterThan(0)
-          expect(latestState.result?.collections?.length).toBeGreaterThan(0)
-          expect(latestState.result?.hintsFromExternalAPI).toBeTruthy()
-          expect(latestState.result?.total.usd).toBeGreaterThan(1000)
-          expect(pendingState).toBeFalsy()
-          done()
-        }
-      })
+      await controller.updateSelectedAccount(account.addr)
 
-      controller.updateSelectedAccount(account.addr)
+      const latestState = controller.latest['0xB674F3fd5F43464dB0448a57529eAF37F04cceA5']?.ethereum!
+      const pendingState =
+        controller.pending['0xB674F3fd5F43464dB0448a57529eAF37F04cceA5']?.ethereum!
+      expect(latestState.isReady).toEqual(true)
+      expect(latestState.result?.tokens.length).toBeGreaterThan(0)
+      expect(latestState.result?.collections?.length).toBeGreaterThan(0)
+      expect(latestState.result?.hintsFromExternalAPI).toBeTruthy()
+      expect(latestState.result?.total.usd).toBeGreaterThan(1000)
+      expect(pendingState).toBeDefined()
     })
 
     // @TODO redo this test

src/controllers/portfolio/portfolio.ts

@@ -537,20 +537,6 @@ export class PortfolioController extends EventEmitter {
             currentAccountOps && simulatedAccountOps
               ? !isAccountOpsIntentEqual(currentAccountOps, simulatedAccountOps)
               : currentAccountOps !== simulatedAccountOps
-
-          // TODO: The `updateSelectedAccount` function is currently always invoked with `forceUpdate: true` from the application code.
-          // TODO: We decided to implement this improvement once we first address https://github.com/AmbireTech/ambire-app/issues/2335#issue-2345433944
-          // We should rewrite this, and state updates should work in the following manner:
-          // 1. On the Dashboard screen, there is an interval that updates both the latest and pending states.
-          //    Why do we update both states? Even without AccountOps to simulate, fetching the pending state
-          //    is beneficial because if someone sends you a token, you will see it on the Dashboard as a pending token balance.
-          // 2. On the Dashboard, if the user manually triggers a Portfolio update, we should pass `forceUpdate: true` and update both states,
-          //    regardless of any internal portfolio controller limits or caching.
-          // 3. When signing a transaction and we have AccountOps, we have the following two scenarios:
-          //    3.1. The Dashboard interval should continue updating both the latest and pending states,
-          //    and also simulate AccountOps (without further optimizations, such as checking `areAccountOpsChanged` and updating the state only if necessary).
-          //    3.2. The SignAccountOp screen also has a simulation interval that updates both states and simulates the AccountOps.
-          //    Here, we should apply the `areAccountOpsChanged` optimization and update both states only if the AccountOps have changed or have not been simulated yet.
           const forceUpdate = opts?.forceUpdate || areAccountOpsChanged
 
           const fallbackHints = (this.#previousHints?.fromExternalAPI &&
@@ -582,29 +568,25 @@ export class PortfolioController extends EventEmitter {
               },
               forceUpdate
             ),
-            // Pending state update
-            // We are updating the pending state, only if AccountOps are changed or the application logic requests a force update
-            forceUpdate
-              ? this.updatePortfolioState(
-                  accountId,
-                  pendingState,
-                  network,
-                  portfolioLib,
-                  {
-                    blockTag: 'pending',
-                    previousHints: fallbackHints,
-                    ...(currentAccountOps && {
-                      simulation: {
-                        account: selectedAccount,
-                        accountOps: currentAccountOps
-                      }
-                    }),
-                    isEOA: !isSmartAccount(selectedAccount),
-                    additionalHints
-                  },
-                  forceUpdate
-                )
-              : Promise.resolve(false)
+            this.updatePortfolioState(
+              accountId,
+              pendingState,
+              network,
+              portfolioLib,
+              {
+                blockTag: 'pending',
+                previousHints: fallbackHints,
+                ...(currentAccountOps && {
+                  simulation: {
+                    account: selectedAccount,
+                    accountOps: currentAccountOps
+                  }
+                }),
+                isEOA: !isSmartAccount(selectedAccount),
+                additionalHints
+              },
+              forceUpdate
+            )
           ])
 
           // Persist latest state in previousHints in the disk storage for further requests