Fix #7945: NFTs displayed in Portfolio when grouped by accounts (#8300)

Make sure NFTs filtered out when grouping by accounts; resolves NFTs displaying in account groups & possible graph issues (NFT dependent)
brave · Oct 23, 2023 · 0ceb0b4 · 0ceb0b4
1 parent dd8db90
commit 0ceb0b4
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 4 deletions.
diff --git a/Sources/BraveWallet/Crypto/Stores/PortfolioStore.swift b/Sources/BraveWallet/Crypto/Stores/PortfolioStore.swift
@@ -407,7 +407,16 @@ public class PortfolioStore: ObservableObject, WalletObserverStore {
       let filters = self.filters
       let selectedAccounts = filters.accounts.filter(\.isSelected).map(\.model)
       let selectedNetworks = filters.networks.filter(\.isSelected).map(\.model)
-      let allVisibleUserAssets = assetManager.getAllUserAssetsInNetworkAssetsByVisibility(networks: selectedNetworks, visible: true)
+      let allVisibleUserAssets: [NetworkAssets] = assetManager.getAllUserAssetsInNetworkAssetsByVisibility(
+        networks: selectedNetworks,
+        visible: true
+      ).map { networkAssets in // filter out NFTs from Portfolio
+        NetworkAssets(
+          network: networkAssets.network,
+          tokens: networkAssets.tokens.filter { !($0.isNft || $0.isErc721) },
+          sortOrder: networkAssets.sortOrder
+        )
+      }
       // update assets on display immediately with empty values. Issue #5567
       self.assetGroups = buildAssetGroupViewModels(
         groupBy: filters.groupBy,
@@ -597,7 +606,7 @@ public class PortfolioStore: ObservableObject, WalletObserverStore {
     switch groupType {
     case .none:
       return allVisibleUserAssets.flatMap { networkAssets in
-        networkAssets.tokens.filter { (!$0.isErc721 && !$0.isNft) }.map { token in
+        networkAssets.tokens.map { token in
           AssetViewModel(
             groupType: groupType,
             token: token,
@@ -629,7 +638,6 @@ public class PortfolioStore: ObservableObject, WalletObserverStore {
         return []
       }
       return networkAssets.tokens
-        .filter { (!$0.isErc721 && !$0.isNft) }
         .map { token in
           AssetViewModel(
             groupType: groupType,

diff --git a/Tests/BraveWalletTests/PortfolioStoreTests.swift b/Tests/BraveWalletTests/PortfolioStoreTests.swift
@@ -146,7 +146,8 @@ import Preferences
     let mockEthUserAssets: [BraveWallet.BlockchainToken] = [
       .previewToken.copy(asVisibleAsset: true),
       .previewDaiToken, // Verify non-visible assets not displayed #6386
-      .mockUSDCToken.copy(asVisibleAsset: true)
+      .mockUSDCToken.copy(asVisibleAsset: true),
+      .mockERC721NFTToken.copy(asVisibleAsset: true) // Verify NFTs not used in Portfolio #7945
     ]
     let ethBalanceWei = formatter.weiString(
       from: mockETHBalanceAccount1,
@@ -260,6 +261,10 @@ import Preferences
         completion(usdcAccount2BalanceWei, .success, "")
       }
     }
+    rpcService._erc721TokenBalance = { _, _, _, _, completion in
+      // should not be fetching NFT balance in Portfolio
+      completion("", .internalError, "Error Message")
+    }
     rpcService._solanaBalance = { accountAddress, chainId, completion in
       // sol balance
       if accountAddress == self.solAccount1.address {
@@ -662,6 +667,12 @@ import Preferences
         XCTAssertEqual(group.assets[safe: 4]?.quantity, String(format: "%.04f", 0))
         // SOL (value = $0, SOL networks hidden)
         XCTAssertNil(group.assets[safe: 5])
+
+        // Verify NFTs not used in Portfolio #7945
+        let noAssetsAreNFTs = lastUpdatedAssetGroups.flatMap(\.assets).allSatisfy({
+          !($0.token.isNft || $0.token.isErc721)
+        })
+        XCTAssertTrue(noAssetsAreNFTs)
       }.store(in: &cancellables)
     store.saveFilters(.init(
       groupBy: store.filters.groupBy,
@@ -778,6 +789,12 @@ import Preferences
         XCTAssertEqual(filAccount2Group.assets[safe: 0]?.token.symbol,
                        BraveWallet.BlockchainToken.mockFilToken.symbol)
         XCTAssertEqual(filAccount2Group.assets[safe: 0]?.quantity, String(format: "%.04f", 0))
+
+        // Verify NFTs not used in Portfolio #7945
+        let noAssetsAreNFTs = lastUpdatedAssetGroups.flatMap(\.assets).allSatisfy({
+          !($0.token.isNft || $0.token.isErc721)
+        })
+        XCTAssertTrue(noAssetsAreNFTs)
       }
       .store(in: &cancellables)
     store.saveFilters(.init(
@@ -942,6 +959,12 @@ import Preferences
         XCTAssertEqual(ethGoerliGroup.assets[safe: 0]?.token.symbol,
                        BraveWallet.BlockchainToken.previewToken.symbol)
         XCTAssertEqual(ethGoerliGroup.assets[safe: 0]?.quantity, String(format: "%.04f", 0))
+
+        // Verify NFTs not used in Portfolio #7945
+        let noAssetsAreNFTs = lastUpdatedAssetGroups.flatMap(\.assets).allSatisfy({
+          !($0.token.isNft || $0.token.isErc721)
+        })
+        XCTAssertTrue(noAssetsAreNFTs)
       }
       .store(in: &cancellables)
     store.saveFilters(.init(