Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Fix #8435: NFT images failing to show in Activity tab Send transactions #8436

Merged
merged 2 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Sources/BraveWallet/Crypto/AssetIconView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,10 @@ struct NFTIconView: View {
}

var body: some View {
NFTImageView(urlString: url?.absoluteString ?? "", isLoading: isLoadingMetadata) {
NFTImageView( // logo populated for auto-discovered NFTs
urlString: url?.absoluteString ?? token.logo,
isLoading: isLoadingMetadata
) {
LoadingNFTView(shimmer: false)
}
.cornerRadius(5)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ class AccountActivityStore: ObservableObject, WalletObserverStore {
userAssets: userAssets,
allTokens: allTokens,
assetRatios: assetRatios,
nftMetadata: [:],
solEstimatedTxFee: solEstimatedTxFees[transaction.id],
currencyFormatter: currencyFormatter
)
Expand Down
1 change: 1 addition & 0 deletions Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ class AssetDetailStore: ObservableObject, WalletObserverStore {
userAssets: userAssets,
allTokens: allTokens,
assetRatios: assetRatios,
nftMetadata: [:],
solEstimatedTxFee: solEstimatedTxFees[transaction.id],
currencyFormatter: self.currencyFormatter
)
Expand Down
1 change: 1 addition & 0 deletions Sources/BraveWallet/Crypto/Stores/CryptoStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ public class CryptoStore: ObservableObject, WalletObserverStore {
blockchainRegistry: blockchainRegistry,
txService: txService,
solTxManagerProxy: solTxManagerProxy,
ipfsApi: ipfsApi,
userAssetManager: userAssetManager
)
self.marketStore = .init(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ public class TransactionConfirmationStore: ObservableObject, WalletObserverStore
userAssets: userAssets,
allTokens: allTokens,
assetRatios: assetRatios,
nftMetadata: [:],
solEstimatedTxFee: solEstimatedTxFee,
currencyFormatter: currencyFormatter
) else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class TransactionDetailsStore: ObservableObject, WalletObserverStore {
userAssets: userAssets,
allTokens: allTokens,
assetRatios: assetRatios,
nftMetadata: [:],
solEstimatedTxFee: solEstimatedTxFee,
currencyFormatter: currencyFormatter
) else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore {

private var solEstimatedTxFeesCache: [String: UInt64] = [:]
private var assetPricesCache: [String: Double] = [:]
/// Cache of metadata for NFTs. The key is the token's `id`.
private var metadataCache: [String: NFTMetadata] = [:]

private let keyringService: BraveWalletKeyringService
private let rpcService: BraveWalletJsonRpcService
Expand All @@ -39,6 +41,7 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore {
private let blockchainRegistry: BraveWalletBlockchainRegistry
private let txService: BraveWalletTxService
private let solTxManagerProxy: BraveWalletSolanaTxManagerProxy
private let ipfsApi: IpfsAPI
private let assetManager: WalletUserAssetManagerType
private var keyringServiceObserver: KeyringServiceObserver?
private var txServiceObserver: TxServiceObserver?
Expand All @@ -56,6 +59,7 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore {
blockchainRegistry: BraveWalletBlockchainRegistry,
txService: BraveWalletTxService,
solTxManagerProxy: BraveWalletSolanaTxManagerProxy,
ipfsApi: IpfsAPI,
userAssetManager: WalletUserAssetManagerType
) {
self.keyringService = keyringService
Expand All @@ -65,6 +69,7 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore {
self.blockchainRegistry = blockchainRegistry
self.txService = txService
self.solTxManagerProxy = solTxManagerProxy
self.ipfsApi = ipfsApi
self.assetManager = userAssetManager

self.setupObservers()
Expand Down Expand Up @@ -154,6 +159,7 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore {
userAssets: userAssets,
allTokens: allTokens,
assetRatios: assetPricesCache,
nftMetadata: metadataCache,
solEstimatedTxFees: solEstimatedTxFeesCache
)
guard !self.transactionSections.isEmpty else { return }
Expand All @@ -174,6 +180,42 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore {
userAssets: userAssets,
allTokens: allTokens,
assetRatios: assetPricesCache,
nftMetadata: metadataCache,
solEstimatedTxFees: solEstimatedTxFeesCache
)

let nftsWithoutMetadata = transactionSections.flatMap(\.transactions)
.compactMap { parsedTx in
switch parsedTx.details {
case .erc721Transfer(let details):
return details.fromToken
case .solSplTokenTransfer(let details):
if let fromToken = details.fromToken, fromToken.isNft {
return fromToken
}
return nil
default:
return nil
}
}
.filter { token in // filter out already fetched metadata
!metadataCache.keys.contains(where: { $0.caseInsensitiveCompare(token.contractAddress) == .orderedSame })
}
guard !Task.isCancelled, !nftsWithoutMetadata.isEmpty else { return }
// fetch nft metadata for all NFTs
let allMetadata = await rpcService.fetchNFTMetadata(tokens: nftsWithoutMetadata, ipfsApi: ipfsApi)
for (key, value) in allMetadata { // update cached values
metadataCache[key] = value
}
guard !Task.isCancelled else { return }
self.transactionSections = buildTransactionSections(
transactions: allTransactions,
networksForCoin: networksForCoin,
accountInfos: allAccountInfos,
userAssets: userAssets,
allTokens: allTokens,
assetRatios: assetPricesCache,
nftMetadata: metadataCache,
solEstimatedTxFees: solEstimatedTxFeesCache
)
}
Expand All @@ -186,6 +228,7 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore {
userAssets: [BraveWallet.BlockchainToken],
allTokens: [BraveWallet.BlockchainToken],
assetRatios: [String: Double],
nftMetadata: [String: NFTMetadata],
solEstimatedTxFees: [String: UInt64]
) -> [TransactionSection] {
// Group transactions by day (only compare day/month/year)
Expand All @@ -211,6 +254,7 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore {
userAssets: userAssets,
allTokens: allTokens,
assetRatios: assetRatios,
nftMetadata: nftMetadata,
solEstimatedTxFee: solEstimatedTxFees[transaction.id],
currencyFormatter: currencyFormatter,
decimalFormatStyle: .decimals(precision: 4)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ extension TransactionParser {
userAssets: [BraveWallet.BlockchainToken],
allTokens: [BraveWallet.BlockchainToken],
assetRatios: [String: Double],
nftMetadata: [String: NFTMetadata],
solEstimatedTxFee: UInt64?,
currencyFormatter: NumberFormatter
) -> TransactionSummary {
Expand All @@ -54,6 +55,7 @@ extension TransactionParser {
userAssets: userAssets,
allTokens: allTokens,
assetRatios: assetRatios,
nftMetadata: nftMetadata,
solEstimatedTxFee: solEstimatedTxFee,
currencyFormatter: currencyFormatter,
decimalFormatStyle: .balance // use 4 digit precision for summary
Expand Down
36 changes: 32 additions & 4 deletions Sources/BraveWallet/Crypto/Transactions/TransactionParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ enum TransactionParser {
gasFee = .init(fee: gasFeeString, fiat: "$0.00")
}
}
case .btc:
case .btc, .zec:
break
@unknown default:
break
Expand Down Expand Up @@ -103,6 +103,7 @@ enum TransactionParser {
userAssets: [BraveWallet.BlockchainToken],
allTokens: [BraveWallet.BlockchainToken],
assetRatios: [String: Double],
nftMetadata: [String: NFTMetadata],
solEstimatedTxFee: UInt64?,
currencyFormatter: NumberFormatter,
decimalFormatStyle: WeiFormatter.DecimalFormatStyle? = nil
Expand Down Expand Up @@ -185,6 +186,7 @@ enum TransactionParser {
fromValue: fromValue,
fromAmount: fromValueFormatted,
fromFiat: fromFiat,
fromTokenMetadata: nil,
gasFee: gasFee(
from: transaction,
network: network,
Expand Down Expand Up @@ -230,6 +232,7 @@ enum TransactionParser {
fromValue: fromValue,
fromAmount: fromAmount,
fromFiat: fromFiat,
fromTokenMetadata: nil,
gasFee: gasFee(
from: transaction,
network: network,
Expand Down Expand Up @@ -359,6 +362,12 @@ enum TransactionParser {
return nil
}
let token = token(for: tokenContractAddress, network: network, userAssets: userAssets, allTokens: allTokens)
let tokenNFTMetadata: NFTMetadata?
if let token {
tokenNFTMetadata = nftMetadata[token.id]
} else {
tokenNFTMetadata = nil
}

return .init(
transaction: transaction,
Expand All @@ -372,6 +381,7 @@ enum TransactionParser {
fromToken: token,
fromValue: "1", // Can only send 1 erc721 at a time
fromAmount: "1",
nftMetadata: tokenNFTMetadata,
owner: owner,
tokenId: tokenId,
gasFee: gasFee(
Expand Down Expand Up @@ -413,6 +423,7 @@ enum TransactionParser {
fromValue: fromValue,
fromAmount: fromValueFormatted,
fromFiat: fromFiat,
fromTokenMetadata: nil,
gasFee: gasFee(
from: transaction,
network: network,
Expand All @@ -431,6 +442,12 @@ enum TransactionParser {
return nil
}
let fromToken = token(for: splTokenMintAddress, network: network, userAssets: userAssets, allTokens: allTokens)
let tokenNFTMetadata: NFTMetadata?
if let fromToken {
tokenNFTMetadata = nftMetadata[fromToken.id]
} else {
tokenNFTMetadata = nil
}
let fromValue = "\(amount)"
var fromValueFormatted = ""
var fromFiat = "$0.00"
Expand Down Expand Up @@ -464,6 +481,7 @@ enum TransactionParser {
fromValue: fromValue,
fromAmount: fromValueFormatted,
fromFiat: fromFiat,
fromTokenMetadata: tokenNFTMetadata,
gasFee: gasFee(
from: transaction,
network: network,
Expand Down Expand Up @@ -696,7 +714,9 @@ struct ParsedTransaction: Equatable {
case let .solDappTransaction(details),
let .solSwapTransaction(details):
return details.gasFee
case .erc721Transfer, .other:
case let .erc721Transfer(details):
return details.gasFee
case .other:
return nil
case let .filSend(details):
return details.gasFee
Expand Down Expand Up @@ -801,6 +821,8 @@ struct SendDetails: Equatable {
let fromAmount: String
/// The amount formatted as currency
let fromFiat: String?
/// Metadata if `fromToken` is an NFT
let fromTokenMetadata: NFTMetadata?

/// Gas fee for the transaction
let gasFee: GasFee?
Expand Down Expand Up @@ -836,6 +858,8 @@ struct Eth721TransferDetails: Equatable {
let fromValue: String
/// From amount formatted
let fromAmount: String
/// Metadata for the NFT being sent
let nftMetadata: NFTMetadata?

/// Owner (must not be confused with the caller (fromAddress)
let owner: String
Expand Down Expand Up @@ -906,6 +930,7 @@ extension BraveWallet.TransactionInfo {
userAssets: [BraveWallet.BlockchainToken],
allTokens: [BraveWallet.BlockchainToken],
assetRatios: [String: Double],
nftMetadata: [String: NFTMetadata],
solEstimatedTxFee: UInt64? = nil,
currencyFormatter: NumberFormatter,
decimalFormatStyle: WeiFormatter.DecimalFormatStyle? = nil
Expand All @@ -917,6 +942,7 @@ extension BraveWallet.TransactionInfo {
userAssets: userAssets,
allTokens: allTokens,
assetRatios: assetRatios,
nftMetadata: nftMetadata,
solEstimatedTxFee: solEstimatedTxFee,
currencyFormatter: currencyFormatter,
decimalFormatStyle: decimalFormatStyle
Expand Down Expand Up @@ -984,11 +1010,13 @@ extension BraveWallet.TransactionInfo {
if let erc721ContractAddress {
return [erc721ContractAddress]
}
case .solanaSplTokenTransfer, .solanaSplTokenTransferWithAssociatedTokenAccountCreation:
if let splTokenMintAddress = txDataUnion.solanaTxData?.splTokenMintAddress {
return [splTokenMintAddress]
}
case .ethSend, .erc1155SafeTransferFrom, .other:
break
case .solanaSystemTransfer,
.solanaSplTokenTransfer,
.solanaSplTokenTransferWithAssociatedTokenAccountCreation,
.solanaDappSignTransaction,
.solanaDappSignAndSendTransaction,
.solanaSwap:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ struct TransactionSummaryViewContainer: View {
SendTransactionSummaryView(
sentFromAccountName: parsedTransaction.namedFromAddress,
token: details.fromToken,
nftMetadata: details.fromTokenMetadata,
network: parsedTransaction.network,
valueSent: details.fromAmount,
fiatValueSent: details.fromFiat ?? "",
Expand Down Expand Up @@ -67,6 +68,7 @@ struct TransactionSummaryViewContainer: View {
SendTransactionSummaryView(
sentFromAccountName: parsedTransaction.namedFromAddress,
token: details.fromToken,
nftMetadata: details.nftMetadata,
network: parsedTransaction.network,
valueSent: nil,
fiatValueSent: nil,
Expand Down Expand Up @@ -101,6 +103,7 @@ struct SendTransactionSummaryView: View {

let sentFromAccountName: String
let token: BraveWallet.BlockchainToken?
let nftMetadata: NFTMetadata?
let network: BraveWallet.NetworkInfo
let valueSent: String?
let fiatValueSent: String?
Expand All @@ -110,13 +113,15 @@ struct SendTransactionSummaryView: View {
init(
sentFromAccountName: String,
token: BraveWallet.BlockchainToken?,
nftMetadata: NFTMetadata? = nil,
network: BraveWallet.NetworkInfo,
valueSent: String?,
fiatValueSent: String?,
status: BraveWallet.TransactionStatus,
time: Date
) {
self.sentFromAccountName = sentFromAccountName
self.nftMetadata = nftMetadata
self.token = token
self.network = network
self.valueSent = valueSent
Expand Down Expand Up @@ -162,7 +167,7 @@ struct SendTransactionSummaryView: View {
NFTIconView(
token: token,
network: network,
url: nil,
url: nftMetadata?.imageURL,
shouldShowNetworkIcon: true,
length: length,
maxLength: maxLength,
Expand Down
Loading