diff --git a/Sources/BraveWallet/Crypto/NFT/NFTView.swift b/Sources/BraveWallet/Crypto/NFT/NFTView.swift index a37c19246a02..f3626754f777 100644 --- a/Sources/BraveWallet/Crypto/NFT/NFTView.swift +++ b/Sources/BraveWallet/Crypto/NFT/NFTView.swift @@ -114,12 +114,12 @@ struct NFTView: View { } } .pickerStyle(.inline) - .disabled(nftStore.isShowingNFTLoadingState) + .disabled(nftStore.isLoadingJunkNFTs) } label: { HStack(spacing: 12) { Text(nftStore.displayType.dropdownTitle) .font(.subheadline.weight(.semibold)) - if !nftStore.isShowingNFTLoadingState { + if !nftStore.isLoadingJunkNFTs { Text("\(nftStore.totalDisplayedNFTCount)") .padding(.horizontal, 8) .padding(.vertical, 4) @@ -141,9 +141,9 @@ struct NFTView: View { Spacer() addCustomAssetButton .padding(.trailing, 10) - .disabled(nftStore.isShowingNFTLoadingState) + .disabled(nftStore.isLoadingJunkNFTs) filtersButton - .disabled(nftStore.isShowingNFTLoadingState) + .disabled(nftStore.isLoadingJunkNFTs) } .padding(.horizontal) .frame(maxWidth: .infinity, alignment: .leading) @@ -274,7 +274,7 @@ struct NFTView: View { var body: some View { LazyVStack(spacing: 16) { nftHeaderView - if nftStore.isShowingNFTLoadingState { + if nftStore.isLoadingJunkNFTs { SkeletonLoadingNFTView() } else if nftStore.isShowingNFTEmptyState { emptyView diff --git a/Sources/BraveWallet/Crypto/Stores/CryptoStore.swift b/Sources/BraveWallet/Crypto/Stores/CryptoStore.swift index d9925d519dc1..9745499d8e0c 100644 --- a/Sources/BraveWallet/Crypto/Stores/CryptoStore.swift +++ b/Sources/BraveWallet/Crypto/Stores/CryptoStore.swift @@ -259,11 +259,17 @@ public class CryptoStore: ObservableObject, WalletObserverStore { // CoreData are added after. guard let self else { return } if !self.isUpdatingUserAssets { + let dispatchGroup = DispatchGroup() for asset in discoveredAssets { - self.userAssetManager.addUserAsset(asset, completion: nil) + dispatchGroup.enter() + self.userAssetManager.addUserAsset(asset) { + dispatchGroup.leave() + } } - if !discoveredAssets.isEmpty { - self.updateAssets() + dispatchGroup.notify(queue: .main) { + if !discoveredAssets.isEmpty { + self.updateAutoDiscoveredAssets() + } } } else { self.autoDiscoveredAssets.append(contentsOf: discoveredAssets) @@ -576,6 +582,14 @@ public class CryptoStore: ObservableObject, WalletObserverStore { nftStore.update() } + func updateAutoDiscoveredAssets() { + // at this point, all auto-discovered assets have been added to CD + // update `Portfolio/Assets` + portfolioStore.update() + // fetch junk NFTs from SimpleHash which will also update `Portfolio/NFTs` + nftStore.fetchJunkNFTs() + } + func prepare(isInitialOpen: Bool = false) { Task { @MainActor in if isInitialOpen { diff --git a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift index 19cdce2feede..eb99963adbfc 100644 --- a/Sources/BraveWallet/Crypto/Stores/NFTStore.swift +++ b/Sources/BraveWallet/Crypto/Stores/NFTStore.swift @@ -133,8 +133,8 @@ public class NFTStore: ObservableObject, WalletObserverStore { @Published var displayType: NFTDisplayType = .visible /// View model for all NFT include visible, hidden and spams @Published private(set) var userNFTGroups: [NFTGroupViewModel] = [] - /// showing shimmering loading state when the view finishes loading NFT information - @Published var isShowingNFTLoadingState: Bool = false + /// showing shimmering loading state when the view is fetching non-fungible tokens without fetching its metadata or balance + @Published var isLoadingJunkNFTs: Bool = false private let keyringService: BraveWalletKeyringService private let rpcService: BraveWalletJsonRpcService @@ -152,7 +152,11 @@ public class NFTStore: ObservableObject, WalletObserverStore { /// Cache of metadata for NFTs. The key is the token's `id`. private var metadataCache: [String: NFTMetadata] = [:] /// Spam from SimpleHash in form of `NetworkAssets` - private var simpleHashSpamNFTs: [NetworkAssets] = [] + private var simpleHashSpamNFTs: [NetworkAssets] = [] { + didSet { + update() + } + } var isObserving: Bool { rpcServiceObserver != nil && keyringServiceObserver != nil && walletServiveObserber != nil @@ -249,7 +253,6 @@ public class NFTStore: ObservableObject, WalletObserverStore { private var nftBalancesCache: [String: [String: Int]] = [:] func update() { - self.isShowingNFTLoadingState = true self.updateTask?.cancel() self.updateTask = Task { @MainActor in self.allAccounts = await keyringService.allAccounts().accounts @@ -261,14 +264,13 @@ public class NFTStore: ObservableObject, WalletObserverStore { let selectedAccounts = filters.accounts.filter(\.isSelected).map(\.model) let selectedNetworks = filters.networks.filter(\.isSelected).map(\.model) - // all spam NFTs marked by SimpleHash - simpleHashSpamNFTs = await walletService.simpleHashSpamNFTs(for: selectedAccounts, on: selectedNetworks) + // First display grids with placeholder since we haven't fetched balance + // or metadata let unionedSpamNFTs = computeSpamNFTs( selectedNetworks: selectedNetworks, selectedAccounts: selectedAccounts, simpleHashSpamNFTs: simpleHashSpamNFTs ) - let (userNFTGroups, allUserNFTs) = buildNFTGroupModels( groupBy: filters.groupBy, spams: unionedSpamNFTs, @@ -277,13 +279,14 @@ public class NFTStore: ObservableObject, WalletObserverStore { ) self.userNFTGroups = userNFTGroups + // Then, we fetch balance and update the UI // if we're not hiding unowned or grouping by account, balance isn't needed if filters.isHidingUnownedNFTs || filters.groupBy == .accounts { let allAccounts = filters.accounts.map(\.model) nftBalancesCache = await withTaskGroup( of: [String: [String: Int]].self, body: { @MainActor [nftBalancesCache, rpcService] group in - for nft in allUserNFTs { // for each NFT + for nft in allUserNFTs { // for all NFTs that have not yet been fetched its balance guard let networkForNFT = allNetworks.first(where: { $0.chainId == nft.chainId }) else { continue } @@ -292,13 +295,17 @@ public class NFTStore: ObservableObject, WalletObserverStore { of: [String: Int].self, body: { @MainActor group in for account in allAccounts where account.coin == nft.coin { - group.addTask { @MainActor in - let balanceForToken = await rpcService.balance( - for: nft, - in: account, - network: networkForNFT - ) - return [account.address: Int(balanceForToken ?? 0)] + if let cachedBalance = nftBalancesCache[nft.id]?[account.address] { // cached balance + return [account.address: cachedBalance] + } else { // no balance for this account + group.addTask { @MainActor in + let balanceForToken = await rpcService.balance( + for: nft, + in: account, + network: networkForNFT + ) + return [account.address: Int(balanceForToken ?? 0)] + } } } return await group.reduce(into: [String: Int](), { partialResult, new in @@ -315,7 +322,6 @@ public class NFTStore: ObservableObject, WalletObserverStore { }) }) } - guard !Task.isCancelled else { return } let (userNFTGroupsWithBalance, _) = buildNFTGroupModels( groupBy: filters.groupBy, @@ -325,11 +331,12 @@ public class NFTStore: ObservableObject, WalletObserverStore { ) self.userNFTGroups = userNFTGroupsWithBalance - // fetch nft metadata for all NFTs - let allMetadata = await rpcService.fetchNFTMetadata(tokens: allUserNFTs, ipfsApi: ipfsApi) - for (key, value) in allMetadata { // update cached values - metadataCache[key] = value - } + // Last, we fet fetch nft metadata for NFTs that do not have metadata loaded + // and update the UI + let nftsMissingMetadata = allUserNFTs.filter { metadataCache[$0.id] == nil } + let fetchedMetadata = await rpcService.fetchNFTMetadata(tokens: nftsMissingMetadata, ipfsApi: ipfsApi) + metadataCache.merge(with: fetchedMetadata) + guard !Task.isCancelled else { return } let (userNFTGroupsWithMetadata, _) = buildNFTGroupModels( groupBy: filters.groupBy, @@ -338,8 +345,6 @@ public class NFTStore: ObservableObject, WalletObserverStore { selectedNetworks: selectedNetworks ) self.userNFTGroups = userNFTGroupsWithMetadata - - isShowingNFTLoadingState = false } } @@ -569,6 +574,20 @@ public class NFTStore: ObservableObject, WalletObserverStore { return (groups, allUserNFTs) } + func fetchJunkNFTs() { + Task { @MainActor in + // all spam NFTs marked by SimpleHash (for all accounts on all networks) + self.isLoadingJunkNFTs = true + let allAccounts = await keyringService.allAccounts().accounts + .filter { account in + WalletConstants.supportedCoinTypes().contains(account.coin) + } + let allNetworks = await rpcService.allNetworksForSupportedCoins() + self.simpleHashSpamNFTs = await walletService.simpleHashSpamNFTs(for: allAccounts, on: allNetworks) + self.isLoadingJunkNFTs = false + } + } + @MainActor func isNFTDiscoveryEnabled() async -> Bool { await walletService.nftDiscoveryEnabled() } @@ -583,7 +602,6 @@ public class NFTStore: ObservableObject, WalletObserverStore { isSpam: Bool, isDeletedByUser: Bool ) { - isShowingNFTLoadingState = true assetManager.updateUserAsset( for: token, visible: visible, @@ -606,8 +624,6 @@ public class NFTStore: ObservableObject, WalletObserverStore { selectedAccounts: selectedAccounts, selectedNetworks: selectedNetworks ) - - isShowingNFTLoadingState = false } } diff --git a/Tests/BraveWalletTests/NFTStoreTests.swift b/Tests/BraveWalletTests/NFTStoreTests.swift index 0a042ecaf8d5..69791e615fae 100644 --- a/Tests/BraveWalletTests/NFTStoreTests.swift +++ b/Tests/BraveWalletTests/NFTStoreTests.swift @@ -121,7 +121,7 @@ class NFTStoreTests: XCTestCase { } let walletService = BraveWallet.TestBraveWalletService() walletService._addObserver = { _ in } - walletService._simpleHashSpamNfTs = {walletAddress, chainIds, coin, _, completion in + walletService._simpleHashSpamNfTs = { walletAddress, chainIds, coin, _, completion in if walletAddress == self.ethAccount1.address, chainIds.contains(BraveWallet.MainnetChainId), coin == .eth { completion([self.spamEthNFT], nil) } else { @@ -414,7 +414,7 @@ class NFTStoreTests: XCTestCase { } // MARK: Group By `None` - func testUpdateSpamGroupOnlyFromSimpleHash() async { + func testSpamOnlyFromSimpleHash() async { let mockEthUserAssets: [BraveWallet.BlockchainToken] = [ .previewToken.copy(asVisibleAsset: true), .mockUSDCToken.copy(asVisibleAsset: false), // Verify non-visible assets not displayed #6386 @@ -476,13 +476,13 @@ class NFTStoreTests: XCTestCase { XCTAssertEqual(spamNFTs[safe: 0]?.nftMetadata?.name, self.mockERC721Metadata.name) }.store(in: &cancellables) - store.update() + store.fetchJunkNFTs() await fulfillment(of: [userSpamNFTsException], timeout: 1) cancellables.removeAll() } // MARK: Group By `None` - func testUpdateSpamGroupFromSimpleHashAndUserMarked() async { + func testSpamFromSimpleHashAndUserMarked() async { let mockEthUserAssets: [BraveWallet.BlockchainToken] = [ .previewToken.copy(asVisibleAsset: true), .mockUSDCToken.copy(asVisibleAsset: false), // Verify non-visible assets not displayed #6386 @@ -546,13 +546,13 @@ class NFTStoreTests: XCTestCase { XCTAssertEqual(spamNFTs[safe: 1]?.nftMetadata?.name, self.mockERC721Metadata.name) }.store(in: &cancellables) - store.update() + store.fetchJunkNFTs() await fulfillment(of: [userSpamNFTsException], timeout: 1) cancellables.removeAll() } // MARK: Group By `None` - func testUpdateSpamGroupDuplicationFromSimpleHashAndUserMarked() async { + func testSpamDuplicationFromSimpleHashAndUserMarked() async { let mockEthUserAssets: [BraveWallet.BlockchainToken] = [ .previewToken.copy(asVisibleAsset: true), .mockUSDCToken.copy(asVisibleAsset: false), // Verify non-visible assets not displayed #6386 @@ -614,7 +614,7 @@ class NFTStoreTests: XCTestCase { XCTAssertEqual(spamNFTs[safe: 0]?.nftMetadata?.name, self.mockERC721Metadata.name) }.store(in: &cancellables) - store.update() + store.fetchJunkNFTs() await fulfillment(of: [userSpamNFTsException], timeout: 1) cancellables.removeAll() } @@ -688,6 +688,7 @@ class NFTStoreTests: XCTestCase { await fulfillment(of: [groupByAccountVisibleExpectation], timeout: 1) cancellables.removeAll() } + // MARK: Group by `Accounts` with `displayType`: `hidden` func testUpdateGroupByAccountsHiddenNFTs() async { let mockEthUserAssets: [BraveWallet.BlockchainToken] = [