Fix #8560, #8542, #8558: Two NFT UI bugs and one caching issue

Sources/BraveWallet/Crypto/NFT/NFTDetailView.swift

@@ -29,7 +29,7 @@ struct NFTDetailView: View {
   }
 
   @ViewBuilder private var nftLogo: some View {
-    if let image = nftDetailStore.networkInfo.nativeTokenLogoImage, !nftDetailStore.isLoading {
+    if let image = nftDetailStore.networkInfo?.nativeTokenLogoImage, !nftDetailStore.isLoading {
       Image(uiImage: image)
         .resizable()
         .frame(width: 32, height: 32)
@@ -103,10 +103,10 @@ struct NFTDetailView: View {
             Text(nftDetailStore.nft.name)
               .foregroundColor(Color(.secondaryBraveLabel))
           }
-          .transaction { transaction in
-            transaction.animation = nil
-            transaction.disablesAnimations = true
-          }
+        }
+        .transaction { transaction in
+          transaction.animation = nil
+          transaction.disablesAnimations = true
         }
         .listRowInsets(.zero)
         .listRowBackground(Color.clear)
@@ -135,7 +135,7 @@ struct NFTDetailView: View {
           }
           NFTDetailRow(title: nftDetailStore.nft.isErc721 ? Strings.Wallet.contractAddressAccessibilityLabel : Strings.Wallet.tokenMintAddress) {
             Button {
-              if let url = nftDetailStore.networkInfo.nftBlockExplorerURL(nftDetailStore.nft) {
+              if let url = nftDetailStore.networkInfo?.nftBlockExplorerURL(nftDetailStore.nft) {
                 openWalletURL(url)
               }
             } label: {
@@ -147,10 +147,12 @@ struct NFTDetailView: View {
               .foregroundColor(Color(.braveBlurpleTint))
             }
           }
-          NFTDetailRow(title: Strings.Wallet.nftDetailBlockchain) {
-            Text(nftDetailStore.networkInfo.chainName)
-              .font(.subheadline)
-              .foregroundColor(Color(.braveLabel))
+          if let networkInfo = nftDetailStore.networkInfo {
+            NFTDetailRow(title: Strings.Wallet.nftDetailBlockchain) {
+              Text(networkInfo.chainName)
+                .font(.subheadline)
+                .foregroundColor(Color(.braveLabel))
+            }
           }
           NFTDetailRow(title: Strings.Wallet.nftDetailTokenStandard) {
             Text(nftDetailStore.nft.isErc721 ? Strings.Wallet.nftDetailERC721 : Strings.Wallet.nftDetailSPL)
@@ -208,9 +210,6 @@ struct NFTDetailView: View {
         isPresentingRemoveAlert = false
       }
     })
-    .onAppear {
-      nftDetailStore.update()
-    }
     .background(Color(UIColor.braveGroupedBackground).ignoresSafeArea())
     .navigationBarTitle(nftDetailStore.nft.nftDetailTitle)
     .toolbar {

Sources/BraveWallet/Crypto/NFT/NFTView.swift

@@ -147,6 +147,10 @@ struct NFTView: View {
     }
     .padding(.horizontal)
     .frame(maxWidth: .infinity, alignment: .leading)
+    .transaction { transaction in
+      transaction.animation = nil
+      transaction.disablesAnimations = true
+    }
   }
 
   private var addCustomAssetButton: some View {
@@ -296,24 +300,28 @@ struct NFTView: View {
       NavigationLink(
         isActive: Binding(
           get: { selectedNFTViewModel != nil },
-          set: { if !$0 { selectedNFTViewModel = nil } }
+          set: {
+            if !$0 {
+              if let viewModel = selectedNFTViewModel {
+                cryptoStore.closeNFTDetailStore(for: viewModel.token)
+              }
+              selectedNFTViewModel = nil
+            }
+          }
         ),
         destination: {
-          if let nftViewModel = selectedNFTViewModel {
+          if let selectedNFTViewModel {
             NFTDetailView(
               keyringStore: keyringStore,
-              nftDetailStore: cryptoStore.nftDetailStore(for: nftViewModel.token, nftMetadata: nftViewModel.nftMetadata, owner: nftStore.owner(for: nftViewModel.token)),
+              nftDetailStore: cryptoStore.nftDetailStore(for: selectedNFTViewModel.token, nftMetadata: selectedNFTViewModel.nftMetadata, owner: nftStore.owner(for: selectedNFTViewModel.token)),
               buySendSwapDestination: buySendSwapDestination,
-              onNFTMetadataRefreshed: {  nftMetadata in
-                nftStore.updateNFTMetadataCache(for: nftViewModel.token, metadata: nftMetadata)
+              onNFTMetadataRefreshed: { nftMetadata in
+                nftStore.updateNFTMetadataCache(for: selectedNFTViewModel.token, metadata: nftMetadata)
               },
               onNFTStatusUpdated: {
                 nftStore.update()
               }
             )
-            .onDisappear {
-              cryptoStore.closeNFTDetailStore(for: nftViewModel.token)
-            }
           }
         },
         label: {

Sources/BraveWallet/Crypto/NFTImageView.swift

@@ -90,7 +90,7 @@ struct LoadingNFTView: View {
             .preference(key: SizePreferenceKey.self, value: geometryProxy.size)
         }
       )
-      .frame(height: viewSize.width)
+      .aspectRatio(1, contentMode: .fit)
       .onPreferenceChange(SizePreferenceKey.self) { newSize in
         viewSize = newSize
       }

Sources/BraveWallet/Crypto/Stores/CryptoStore.swift

@@ -180,7 +180,8 @@ public class CryptoStore: ObservableObject, WalletObserverStore {
       assetRatioService: assetRatioService,
       blockchainRegistry: blockchainRegistry,
       ipfsApi: ipfsApi,
-      userAssetManager: userAssetManager
+      userAssetManager: userAssetManager,
+      txService: txService
     )
     self.transactionsActivityStore = .init(
       keyringService: keyringService,
@@ -527,6 +528,7 @@ public class CryptoStore: ObservableObject, WalletObserverStore {
       assetManager: userAssetManager,
       keyringService: keyringService,
       rpcService: rpcService,
+      txService: txService,
       ipfsApi: ipfsApi,
       nft: nft,
       nftMetadata: nftMetadata,

Sources/BraveWallet/Crypto/Stores/NFTDetailStore.swift

@@ -69,19 +69,24 @@ class NFTDetailStore: ObservableObject, WalletObserverStore {
   private let assetManager: WalletUserAssetManagerType
   private let keyringService: BraveWalletKeyringService
   private let rpcService: BraveWalletJsonRpcService
+  private let txService: BraveWalletTxService
   private let ipfsApi: IpfsAPI
+  private var txServiceObserver: TxServiceObserver?
   @Published var owner: BraveWallet.AccountInfo?
   @Published var nft: BraveWallet.BlockchainToken
   @Published var isLoading: Bool = false
   @Published var nftMetadata: NFTMetadata?
-  @Published var networkInfo: BraveWallet.NetworkInfo = .init()
+  @Published var networkInfo: BraveWallet.NetworkInfo?
 
-  var isObserving: Bool = false
+  var isObserving: Bool {
+    txServiceObserver != nil
+  }
 
   init(
     assetManager: WalletUserAssetManagerType,
     keyringService: BraveWalletKeyringService,
     rpcService: BraveWalletJsonRpcService,
+    txService: BraveWalletTxService,
     ipfsApi: IpfsAPI,
     nft: BraveWallet.BlockchainToken,
     nftMetadata: NFTMetadata?,
@@ -90,10 +95,30 @@ class NFTDetailStore: ObservableObject, WalletObserverStore {
     self.assetManager = assetManager
     self.keyringService = keyringService
     self.rpcService = rpcService
+    self.txService = txService
     self.ipfsApi = ipfsApi
     self.nft = nft
     self.nftMetadata = nftMetadata?.httpfyIpfsUrl(ipfsApi: ipfsApi)
     self.owner = owner
+
+    self.setupObservers()
+
+    self.update()
+  }
+
+  func setupObservers() {
+    self.txServiceObserver = TxServiceObserver(
+      txService: txService,
+      _onTransactionStatusChanged: { [weak self] txInfo in
+        if txInfo.txStatus == .confirmed, txInfo.isSend, (txInfo.coin == .eth || txInfo.coin == .sol) {
+          self?.updateOwner()
+        }
+      }
+    )
+  }
+
+  func tearDown() {
+    txServiceObserver = nil
   }
 
   func update() {
@@ -104,35 +129,7 @@ class NFTDetailStore: ObservableObject, WalletObserverStore {
       }
 
       if owner == nil {
-        let accounts = await keyringService.allAccounts().accounts
-        let nftBalances: [String: Int] = await withTaskGroup(
-          of: [String: Int].self,
-          body: { @MainActor [rpcService, networkInfo, nft] group in
-            for account in accounts where account.coin == nft.coin {
-              group.addTask { @MainActor in
-                let balanceForToken = await rpcService.balance(
-                  for: nft,
-                  in: account,
-                  network: networkInfo
-                )
-                return [account.address: Int(balanceForToken ?? 0)]
-              }
-            }
-            return await group.reduce(into: [String: Int](), { partialResult, new in
-              for key in new.keys {
-                partialResult[key] = new[key]
-              }
-            })
-          }
-        )
-        if let address = nftBalances.first(where: { address, balance in
-          balance > 0
-        })?.key,
-           let account = accounts.first(where: { accountInfo in
-             accountInfo.address.caseInsensitiveCompare(address) == .orderedSame
-           }) {
-          owner = account
-        }
+        updateOwner()
       }
 
       if nftMetadata == nil {
@@ -162,4 +159,43 @@ class NFTDetailStore: ObservableObject, WalletObserverStore {
       completion()
     }
   }
+
+  var updateOwnerTask: Task<(), Never>?
+  private func updateOwner() {
+    updateOwnerTask?.cancel()
+    updateOwnerTask = Task { @MainActor in
+      guard let network = networkInfo else { return }
+      let accounts = await keyringService.allAccounts().accounts
+      let nftBalances: [String: Int] = await withTaskGroup(
+        of: [String: Int].self,
+        body: { @MainActor [rpcService, nft] group in
+          for account in accounts where account.coin == nft.coin {
+            group.addTask { @MainActor in
+              let balanceForToken = await rpcService.balance(
+                for: nft,
+                in: account,
+                network: network
+              )
+              return [account.address: Int(balanceForToken ?? 0)]
+            }
+          }
+          return await group.reduce(into: [String: Int](), { partialResult, new in
+            for key in new.keys {
+              partialResult[key] = new[key]
+            }
+          })
+        }
+      )
+      if let address = nftBalances.first(where: { address, balance in
+        balance > 0
+      })?.key,
+         let account = accounts.first(where: { accountInfo in
+           accountInfo.address.caseInsensitiveCompare(address) == .orderedSame
+         }) {
+        owner = account
+      } else {
+        owner = nil
+      }
+    }
+  }
 }

Sources/BraveWallet/Crypto/Stores/NFTStore.swift

@@ -143,9 +143,11 @@ public class NFTStore: ObservableObject, WalletObserverStore {
   private let blockchainRegistry: BraveWalletBlockchainRegistry
   private let ipfsApi: IpfsAPI
   private let assetManager: WalletUserAssetManagerType
+  private let txService: BraveWalletTxService
   private var rpcServiceObserver: JsonRpcServiceObserver?
   private var keyringServiceObserver: KeyringServiceObserver?
-  private var walletServiveObserber: WalletServiceObserver?
+  private var walletServiveObserver: WalletServiceObserver?
+  private var txServiceObserver: TxServiceObserver?
 
   /// Cancellable for the last running `update()` Task.
   private var updateTask: Task<(), Never>?
@@ -159,7 +161,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
   }
 
   var isObserving: Bool {
-    rpcServiceObserver != nil && keyringServiceObserver != nil && walletServiveObserber != nil
+    rpcServiceObserver != nil && keyringServiceObserver != nil && walletServiveObserver != nil && txServiceObserver != nil
   }
 
   var isShowingNFTEmptyState: Bool {
@@ -180,7 +182,8 @@ public class NFTStore: ObservableObject, WalletObserverStore {
     assetRatioService: BraveWalletAssetRatioService,
     blockchainRegistry: BraveWalletBlockchainRegistry,
     ipfsApi: IpfsAPI,
-    userAssetManager: WalletUserAssetManagerType
+    userAssetManager: WalletUserAssetManagerType,
+    txService: BraveWalletTxService
   ) {
     self.keyringService = keyringService
     self.rpcService = rpcService
@@ -189,6 +192,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
     self.blockchainRegistry = blockchainRegistry
     self.ipfsApi = ipfsApi
     self.assetManager = userAssetManager
+    self.txService = txService
 
     self.setupObservers()
 
@@ -207,7 +211,8 @@ public class NFTStore: ObservableObject, WalletObserverStore {
   func tearDown() {
     rpcServiceObserver = nil
     keyringServiceObserver = nil
-    walletServiveObserber = nil
+    walletServiveObserver = nil
+    txServiceObserver = nil
 
     userAssetsStore.tearDown()
   }
@@ -231,7 +236,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
         self?.update()
       }
     )
-    self.walletServiveObserber = WalletServiceObserver(
+    self.walletServiveObserver = WalletServiceObserver(
       walletService: walletService,
       _onNetworkListChanged: { [weak self] in
         // A network was added or removed, `update()` will update `allNetworks`.
@@ -245,14 +250,21 @@ public class NFTStore: ObservableObject, WalletObserverStore {
         // assets update will be called via `CryptoStore`
       }
     )
+    self.txServiceObserver = TxServiceObserver(
+      txService: txService, _onTransactionStatusChanged: { [weak self] txInfo in
+        if txInfo.txStatus == .confirmed, txInfo.isSend, (txInfo.coin == .eth || txInfo.coin == .sol) {
+          self?.update(forceUpdateNFTBalances: true)
+        }
+      }
+    )
 
     userAssetsStore.setupObservers()
   }
 
   /// Cache of NFT balances for each account tokenBalances: [token.contractAddress]
   private var nftBalancesCache: [String: [String: Int]] = [:]
 
-  func update() {
+  func update(forceUpdateNFTBalances: Bool = false) {
     self.updateTask?.cancel()
     self.updateTask = Task { @MainActor in
       self.allAccounts = await keyringService.allAccounts().accounts
@@ -295,7 +307,7 @@ public class NFTStore: ObservableObject, WalletObserverStore {
                   of: [String: Int].self,
                   body: { @MainActor group in
                     for account in allAccounts where account.coin == nft.coin {
-                      if let cachedBalance = nftBalancesCache[nft.id]?[account.address] { // cached balance
+                      if !forceUpdateNFTBalances, let cachedBalance = nftBalancesCache[nft.id]?[account.address] { // cached balance
                         return [account.address: cachedBalance]
                       } else { // no balance for this account
                         group.addTask { @MainActor in