From 16d15cca27939ccd835db750b5b576395654b244 Mon Sep 17 00:00:00 2001 From: StephenHeaps <5314553+StephenHeaps@users.noreply.github.com> Date: Fri, 8 Dec 2023 12:31:49 -0500 Subject: [PATCH] Fix #8529: Update logic to support unknown tokens from non-mainnet transactions (#8530) Update logic for fetching unknown tokens to support non-mainnet ethereum transactions. Add unknown token fetch to Activity tab. --- .../Crypto/Stores/AccountActivityStore.swift | 26 +++---- .../Crypto/Stores/AssetDetailStore.swift | 33 ++++++++- .../Stores/TransactionConfirmationStore.swift | 34 ++++----- .../Stores/TransactionDetailsStore.swift | 37 +++------- .../Stores/TransactionsActivityStore.swift | 34 ++++++++- .../AssetRatioServiceExtensions.swift | 16 ---- .../Extensions/RpcServiceExtensions.swift | 74 +++++++++++++++++++ .../TransactionConfirmationStoreTests.swift | 6 +- 8 files changed, 175 insertions(+), 85 deletions(-) diff --git a/Sources/BraveWallet/Crypto/Stores/AccountActivityStore.swift b/Sources/BraveWallet/Crypto/Stores/AccountActivityStore.swift index b7137e35961..1c79511223a 100644 --- a/Sources/BraveWallet/Crypto/Stores/AccountActivityStore.swift +++ b/Sources/BraveWallet/Crypto/Stores/AccountActivityStore.swift @@ -36,7 +36,7 @@ class AccountActivityStore: ObservableObject, WalletObserverStore { private let assetManager: WalletUserAssetManagerType /// Cache for storing `BlockchainToken`s that are not in user assets or our token registry. /// This could occur with a dapp creating a transaction. - private var tokenInfoCache: [String: BraveWallet.BlockchainToken] = [:] + private var tokenInfoCache: [BraveWallet.BlockchainToken] = [] private var keyringServiceObserver: KeyringServiceObserver? private var rpcServiceObserver: JsonRpcServiceObserver? @@ -278,20 +278,16 @@ class AccountActivityStore: ObservableObject, WalletObserverStore { if account.coin == .sol { solEstimatedTxFees = await solTxManagerProxy.estimatedTxFees(for: transactions) } - let unknownTokenContractAddresses = transactions - .flatMap { $0.tokenContractAddresses } - .filter { contractAddress in - !userAssets.contains(where: { $0.contractAddress.caseInsensitiveCompare(contractAddress) == .orderedSame }) - && !allTokens.contains(where: { $0.contractAddress.caseInsensitiveCompare(contractAddress) == .orderedSame }) - && !tokenInfoCache.keys.contains(where: { $0.caseInsensitiveCompare(contractAddress) == .orderedSame }) - } - var allTokens = allTokens - if !unknownTokenContractAddresses.isEmpty { - let unknownTokens = await assetRatioService.fetchTokens(for: unknownTokenContractAddresses) - for unknownToken in unknownTokens { - tokenInfoCache[unknownToken.contractAddress] = unknownToken + let ethTransactions = transactions.filter { $0.coin == .eth } + if !ethTransactions.isEmpty { + // Gather known information about the transaction(s) tokens + let unknownTokenInfo = ethTransactions.unknownTokenContractAddressChainIdPairs( + knownTokens: userAssets + allTokens + tokenInfoCache + ) + if !unknownTokenInfo.isEmpty { + let unknownTokens: [BraveWallet.BlockchainToken] = await rpcService.fetchEthTokens(for: unknownTokenInfo) + tokenInfoCache.append(contentsOf: unknownTokens) } - allTokens.append(contentsOf: unknownTokens) } return transactions .compactMap { transaction in @@ -303,7 +299,7 @@ class AccountActivityStore: ObservableObject, WalletObserverStore { network: network, accountInfos: accountInfos, userAssets: userAssets, - allTokens: allTokens, + allTokens: allTokens + tokenInfoCache, assetRatios: assetRatios, nftMetadata: [:], solEstimatedTxFee: solEstimatedTxFees[transaction.id], diff --git a/Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift b/Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift index 5813d388dcb..8a73c3b90e8 100644 --- a/Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift +++ b/Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift @@ -82,6 +82,9 @@ class AssetDetailStore: ObservableObject, WalletObserverStore { /// A list of tokens that are supported with the current selected network for all supported /// on-ramp providers. private var allBuyTokensAllOptions: [BraveWallet.OnRampProvider: [BraveWallet.BlockchainToken]] = [:] + /// Cache for storing `BlockchainToken`s that are not in user assets or our token registry. + /// This could occur with a dapp creating a transaction. + private var tokenInfoCache: [BraveWallet.BlockchainToken] = [] private var keyringServiceObserver: KeyringServiceObserver? private var txServiceObserver: TxServiceObserver? private var walletServiceObserver: WalletServiceObserver? @@ -340,8 +343,19 @@ class AssetDetailStore: ObservableObject, WalletObserverStore { }) } var solEstimatedTxFees: [String: UInt64] = [:] - if token.coin == .sol { + switch token.coin { + case .eth: + let ethTransactions = allTransactions.filter { $0.coin == .eth } + if !ethTransactions.isEmpty { // we can only fetch unknown Ethereum tokens + let unknownTokenInfo = ethTransactions.unknownTokenContractAddressChainIdPairs( + knownTokens: userAssets + allTokens + tokenInfoCache + ) + updateUnknownTokens(for: unknownTokenInfo) + } + case .sol: solEstimatedTxFees = await solTxManagerProxy.estimatedTxFees(for: allTransactions) + default: + break } return allTransactions .filter { tx in @@ -378,7 +392,7 @@ class AssetDetailStore: ObservableObject, WalletObserverStore { network: network, accountInfos: accounts, userAssets: userAssets, - allTokens: allTokens, + allTokens: allTokens + tokenInfoCache, assetRatios: assetRatios, nftMetadata: [:], solEstimatedTxFee: solEstimatedTxFees[transaction.id], @@ -420,6 +434,21 @@ class AssetDetailStore: ObservableObject, WalletObserverStore { return false } } + + private func updateUnknownTokens( + for contractAddressesChainIdPairs: [ContractAddressChainIdPair] + ) { + guard !contractAddressesChainIdPairs.isEmpty else { return } + Task { @MainActor in + // Gather known information about the transaction(s) tokens + let unknownTokens: [BraveWallet.BlockchainToken] = await rpcService.fetchEthTokens( + for: contractAddressesChainIdPairs + ) + guard !unknownTokens.isEmpty else { return } + tokenInfoCache.append(contentsOf: unknownTokens) + update() + } + } } extension AssetDetailStore: BraveWalletKeyringServiceObserver { diff --git a/Sources/BraveWallet/Crypto/Stores/TransactionConfirmationStore.swift b/Sources/BraveWallet/Crypto/Stores/TransactionConfirmationStore.swift index 1bc840338ae..7901e383bf8 100644 --- a/Sources/BraveWallet/Crypto/Stores/TransactionConfirmationStore.swift +++ b/Sources/BraveWallet/Crypto/Stores/TransactionConfirmationStore.swift @@ -309,7 +309,7 @@ public class TransactionConfirmationStore: ObservableObject, WalletObserverStore } return } - let allTokens = await blockchainRegistry.allTokens(network.chainId, coin: coin) + tokenInfoCache.map(\.value) + let allTokens = await blockchainRegistry.allTokens(network.chainId, coin: coin) + tokenInfoCache let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingUserDeleted: true).flatMap { $0.tokens } let solEstimatedTxFee: UInt64? = solEstimatedTxFeeCache[transaction.id] @@ -393,7 +393,7 @@ public class TransactionConfirmationStore: ObservableObject, WalletObserverStore private var gasTokenBalanceCache: [String: Double] = [:] /// Cache for storing `BlockchainToken`s that are not in user assets or our token registry. /// This could occur with a dapp creating a transaction. - private var tokenInfoCache: [String: BraveWallet.BlockchainToken] = [:] + private var tokenInfoCache: [BraveWallet.BlockchainToken] = [] /// Cache for storing the estimated transaction fee for each Solana transaction. The key is the transaction id. private var solEstimatedTxFeeCache: [String: UInt64] = [:] @@ -444,27 +444,19 @@ public class TransactionConfirmationStore: ObservableObject, WalletObserverStore @MainActor private func fetchUnknownTokens( for transactions: [BraveWallet.TransactionInfo] ) async { - // `AssetRatioService` can only fetch tokens from Ethereum Mainnet - let mainnetTransactions = transactions.filter { $0.chainId == BraveWallet.MainnetChainId } - guard !mainnetTransactions.isEmpty else { return } + let ethTransactions = transactions.filter { $0.coin == .eth } + guard !ethTransactions.isEmpty else { return } // we can only fetch unknown Ethereum tokens let coin: BraveWallet.CoinType = .eth let allNetworks = await rpcService.allNetworks(coin) - guard let network = allNetworks.first(where: { $0.chainId == BraveWallet.MainnetChainId }) else { - return - } - let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingUserDeleted: true).flatMap { $0.tokens } - let allTokens = await blockchainRegistry.allTokens(network.chainId, coin: network.coin) - let unknownTokenContractAddresses = mainnetTransactions.flatMap(\.tokenContractAddresses) - .filter { contractAddress in - !userAssets.contains(where: { $0.contractAddress(in: network).caseInsensitiveCompare(contractAddress) == .orderedSame }) - && !allTokens.contains(where: { $0.contractAddress(in: network).caseInsensitiveCompare(contractAddress) == .orderedSame }) - && !tokenInfoCache.keys.contains(where: { $0.caseInsensitiveCompare(contractAddress) == .orderedSame }) - } - guard !unknownTokenContractAddresses.isEmpty else { return } - let unknownTokens = await assetRatioService.fetchTokens(for: unknownTokenContractAddresses) - for unknownToken in unknownTokens { - tokenInfoCache[unknownToken.contractAddress] = unknownToken - } + let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: allNetworks, includingUserDeleted: true).flatMap(\.tokens) + let allTokens = await blockchainRegistry.allTokens(in: allNetworks).flatMap(\.tokens) + // Gather known information about the transaction(s) tokens + let unknownTokenInfo = ethTransactions.unknownTokenContractAddressChainIdPairs( + knownTokens: userAssets + allTokens + tokenInfoCache + ) + guard !unknownTokenInfo.isEmpty else { return } // Only if we have unknown tokens + let unknownTokens: [BraveWallet.BlockchainToken] = await rpcService.fetchEthTokens(for: unknownTokenInfo) + tokenInfoCache.append(contentsOf: unknownTokens) updateTransaction(with: activeTransaction, shouldFetchCurrentAllowance: false, shouldFetchGasTokenBalance: false) } diff --git a/Sources/BraveWallet/Crypto/Stores/TransactionDetailsStore.swift b/Sources/BraveWallet/Crypto/Stores/TransactionDetailsStore.swift index e6e3565469a..3f9f5136af3 100644 --- a/Sources/BraveWallet/Crypto/Stores/TransactionDetailsStore.swift +++ b/Sources/BraveWallet/Crypto/Stores/TransactionDetailsStore.swift @@ -36,7 +36,7 @@ class TransactionDetailsStore: ObservableObject, WalletObserverStore { private let assetManager: WalletUserAssetManagerType /// Cache for storing `BlockchainToken`s that are not in user assets or our token registry. /// This could occur with a dapp creating a transaction. - private var tokenInfoCache: [String: BraveWallet.BlockchainToken] = [:] + private var tokenInfoCache: [BraveWallet.BlockchainToken] = [] private var nftMetadataCache: [String: NFTMetadata] = [:] var isObserving: Bool { @@ -111,20 +111,17 @@ class TransactionDetailsStore: ObservableObject, WalletObserverStore { return } self.network = network - var allTokens: [BraveWallet.BlockchainToken] = await blockchainRegistry.allTokens(network.chainId, coin: network.coin) + tokenInfoCache.map(\.value) - let userAssets: [BraveWallet.BlockchainToken] = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingUserDeleted: true).flatMap { $0.tokens } - let unknownTokenContractAddresses = transaction.tokenContractAddresses - .filter { contractAddress in - !userAssets.contains(where: { $0.contractAddress(in: network).caseInsensitiveCompare(contractAddress) == .orderedSame }) - && !allTokens.contains(where: { $0.contractAddress(in: network).caseInsensitiveCompare(contractAddress) == .orderedSame }) - && !tokenInfoCache.keys.contains(where: { $0.caseInsensitiveCompare(contractAddress) == .orderedSame }) + var allTokens: [BraveWallet.BlockchainToken] = await blockchainRegistry.allTokens(network.chainId, coin: network.coin) + let userAssets: [BraveWallet.BlockchainToken] = assetManager.getAllUserAssetsInNetworkAssets(networks: [network], includingUserDeleted: true).flatMap(\.tokens) + if coin == .eth { + // Gather known information about the transaction(s) tokens + let unknownTokenInfo = [transaction].unknownTokenContractAddressChainIdPairs( + knownTokens: userAssets + allTokens + tokenInfoCache + ) + if !unknownTokenInfo.isEmpty { + let unknownTokens: [BraveWallet.BlockchainToken] = await rpcService.fetchEthTokens(for: unknownTokenInfo) + tokenInfoCache.append(contentsOf: unknownTokens) } - if !unknownTokenContractAddresses.isEmpty { - let unknownTokens = await assetRatioService.fetchTokens(for: unknownTokenContractAddresses) - for unknownToken in unknownTokens { - tokenInfoCache[unknownToken.contractAddress] = unknownToken - } - allTokens.append(contentsOf: unknownTokens) } let priceResult = await assetRatioService.priceWithIndividualRetry( @@ -144,7 +141,7 @@ class TransactionDetailsStore: ObservableObject, WalletObserverStore { network: network, accountInfos: allAccounts, userAssets: userAssets, - allTokens: allTokens, + allTokens: allTokens + tokenInfoCache, assetRatios: assetRatios, nftMetadata: nftMetadataCache, solEstimatedTxFee: solEstimatedTxFee, @@ -191,14 +188,4 @@ class TransactionDetailsStore: ObservableObject, WalletObserverStore { self.parsedTransaction = parsedTransaction } } - - @MainActor private func fetchTokenInfo(for contractAddress: String) async -> BraveWallet.BlockchainToken? { - if let cachedToken = tokenInfoCache[contractAddress] { - return cachedToken - } - let tokenInfo = await assetRatioService.tokenInfo(contractAddress) - guard let tokenInfo = tokenInfo else { return nil } - self.tokenInfoCache[contractAddress] = tokenInfo - return tokenInfo - } } diff --git a/Sources/BraveWallet/Crypto/Stores/TransactionsActivityStore.swift b/Sources/BraveWallet/Crypto/Stores/TransactionsActivityStore.swift index 7fbb1277b80..bba97f6c900 100644 --- a/Sources/BraveWallet/Crypto/Stores/TransactionsActivityStore.swift +++ b/Sources/BraveWallet/Crypto/Stores/TransactionsActivityStore.swift @@ -34,6 +34,9 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore { private var assetPricesCache: [String: Double] = [:] /// Cache of metadata for NFTs. The key is the token's `id`. private var metadataCache: [String: NFTMetadata] = [:] + /// Cache for storing `BlockchainToken`s that are not in user assets or our token registry. + /// This could occur with a dapp creating a transaction. + private var tokenInfoCache: [BraveWallet.BlockchainToken] = [] private let keyringService: BraveWalletKeyringService private let rpcService: BraveWalletJsonRpcService @@ -147,10 +150,20 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore { let allTransactions = await txService.allTransactions( networksForCoin: networksForCoin, for: allAccountInfos ).filter { $0.txStatus != .rejected } - let userAssets = assetManager.getAllUserAssetsInNetworkAssets(networks: allNetworksAllCoins, includingUserDeleted: true).flatMap(\.tokens) + let userAssets = assetManager.getAllUserAssetsInNetworkAssets( + networks: allNetworksAllCoins, + includingUserDeleted: true + ).flatMap(\.tokens) let allTokens = await blockchainRegistry.allTokens( in: allNetworksAllCoins ).flatMap(\.tokens) + let ethTransactions = allTransactions.filter { $0.coin == .eth } + if !ethTransactions.isEmpty { // we can only fetch unknown Ethereum tokens + let unknownTokenInfo = ethTransactions.unknownTokenContractAddressChainIdPairs( + knownTokens: userAssets + allTokens + tokenInfoCache + ) + updateUnknownTokens(for: unknownTokenInfo) + } guard !Task.isCancelled else { return } // display transactions prior to network request to fetch // estimated solana tx fees & asset prices @@ -159,7 +172,7 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore { networksForCoin: networksForCoin, accountInfos: allAccountInfos, userAssets: userAssets, - allTokens: allTokens, + allTokens: allTokens + tokenInfoCache, assetRatios: assetPricesCache, nftMetadata: metadataCache, solEstimatedTxFees: solEstimatedTxFeesCache @@ -254,7 +267,7 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore { network: network, accountInfos: accountInfos, userAssets: userAssets, - allTokens: allTokens, + allTokens: allTokens + tokenInfoCache, assetRatios: assetRatios, nftMetadata: nftMetadata, solEstimatedTxFee: solEstimatedTxFees[transaction.id], @@ -287,6 +300,21 @@ class TransactionsActivityStore: ObservableObject, WalletObserverStore { } } + private func updateUnknownTokens( + for contractAddressesChainIdPairs: [ContractAddressChainIdPair] + ) { + guard !contractAddressesChainIdPairs.isEmpty else { return } + Task { @MainActor in + // Gather known information about the transaction(s) tokens + let unknownTokens: [BraveWallet.BlockchainToken] = await rpcService.fetchEthTokens( + for: contractAddressesChainIdPairs + ) + guard !unknownTokens.isEmpty else { return } + tokenInfoCache.append(contentsOf: unknownTokens) + update() + } + } + private var transactionDetailsStore: TransactionDetailsStore? func transactionDetailsStore( for transaction: BraveWallet.TransactionInfo diff --git a/Sources/BraveWallet/Extensions/AssetRatioServiceExtensions.swift b/Sources/BraveWallet/Extensions/AssetRatioServiceExtensions.swift index d700b53f611..f7df9bd8e85 100644 --- a/Sources/BraveWallet/Extensions/AssetRatioServiceExtensions.swift +++ b/Sources/BraveWallet/Extensions/AssetRatioServiceExtensions.swift @@ -108,20 +108,4 @@ extension BraveWalletAssetRatioService { let prices = Dictionary(uniqueKeysWithValues: priceResult.assetPrices.map { ($0.fromAsset, $0.price) }) return prices } - - /// Fetches the BlockchainToken for the given contract addresses. The token for a given contract - /// address is not guaranteed to be found, and will not be provided in the result if not found. - @MainActor func fetchTokens( - for contractAddresses: [String] - ) async -> [BraveWallet.BlockchainToken] { - await withTaskGroup(of: [BraveWallet.BlockchainToken?].self) { @MainActor group in - for contractAddress in contractAddresses { - group.addTask { @MainActor in - let token = await self.tokenInfo(contractAddress) - return [token] - } - } - return await group.reduce([BraveWallet.BlockchainToken?](), { $0 + $1 }) - }.compactMap { $0 } - } } diff --git a/Sources/BraveWallet/Extensions/RpcServiceExtensions.swift b/Sources/BraveWallet/Extensions/RpcServiceExtensions.swift index 6c6a326dbd1..ef9948b7fbf 100644 --- a/Sources/BraveWallet/Extensions/RpcServiceExtensions.swift +++ b/Sources/BraveWallet/Extensions/RpcServiceExtensions.swift @@ -385,4 +385,78 @@ extension BraveWalletJsonRpcService { }) } } + + /// Helper to fetch `symbol` and `decimals` ONLY given a token contract address & chainId. + /// This function will be replaced by a BraveCore function that will also fetch `name` & `coingeckoId`. + func getEthTokenInfo( + contractAddress: String, + chainId: String + ) async -> BraveWallet.BlockchainToken? { + let (symbol, symbolStatus, _) = await ethTokenSymbol(contractAddress, chainId: chainId) + let (decimals, decimalsStatus, _) = await ethTokenDecimals(contractAddress, chainId: chainId) + guard symbolStatus == .success || decimalsStatus == .success else { return nil } + return .init( + contractAddress: contractAddress, + name: "", + logo: "", + isErc20: false, // rpcService.getSupportsInterface() is private / internal. + isErc721: false, // rpcService.getSupportsInterface() is private / internal. + isErc1155: false, // rpcService.getSupportsInterface() is private / internal. + isNft: false, + isSpam: false, + symbol: symbol, + decimals: Int32(decimals.removingHexPrefix, radix: 16) ?? 0, + visible: false, + tokenId: "", + coingeckoId: "", // blockchainRegistry can fetch this for us, but not needed in Tx Confirmation. + chainId: chainId, + coin: .eth + ) + } + + /// Fetches the BlockchainToken for the given contract addresses. The token for a given contract + /// address is not guaranteed to be found, and will not be provided in the result if not found. + @MainActor func fetchEthTokens( + for contractAddressesChainIdPairs: [ContractAddressChainIdPair] + ) async -> [BraveWallet.BlockchainToken] { + await withTaskGroup(of: [BraveWallet.BlockchainToken?].self) { @MainActor group in + for contractAddressesChainIdPair in contractAddressesChainIdPairs { + group.addTask { + let token = await self.getEthTokenInfo( + contractAddress: contractAddressesChainIdPair.contractAddress, + chainId: contractAddressesChainIdPair.chainId + ) + if let token { + return [token] + } + return [] + } + } + return await group.reduce([BraveWallet.BlockchainToken?](), { $0 + $1 }) + }.compactMap { $0 } + } +} + +struct ContractAddressChainIdPair: Equatable { + let contractAddress: String + let chainId: String +} + +extension Array where Element == BraveWallet.TransactionInfo { + func unknownTokenContractAddressChainIdPairs( + knownTokens: [BraveWallet.BlockchainToken] + ) -> [ContractAddressChainIdPair] { + flatMap { transaction in + transaction.tokenContractAddresses + .filter { contractAddress in + !knownTokens.contains(where: { knownToken in + knownToken.contractAddress.caseInsensitiveCompare(contractAddress) == .orderedSame && + knownToken.chainId.caseInsensitiveCompare(knownToken.chainId) == .orderedSame + }) + } + .map { contractAddress in + ContractAddressChainIdPair(contractAddress: contractAddress, chainId: transaction.chainId) + } + } + } } diff --git a/Tests/BraveWalletTests/TransactionConfirmationStoreTests.swift b/Tests/BraveWalletTests/TransactionConfirmationStoreTests.swift index c75e81f93ff..28b4dececcd 100644 --- a/Tests/BraveWalletTests/TransactionConfirmationStoreTests.swift +++ b/Tests/BraveWalletTests/TransactionConfirmationStoreTests.swift @@ -344,11 +344,11 @@ import Preferences ) let networkExpectation = expectation(description: "network-expectation") store.$network - .dropFirst(7) // `network` is assigned multiple times during setup - .collect(5) // collect all transactions + .dropFirst(8) // `network` is assigned multiple times during setup + .collect(6) // collect all updates (1 extra for final tx network) .sink { networks in defer { networkExpectation.fulfill() } - XCTAssertEqual(networks.count, 5) + XCTAssertEqual(networks.count, 6) XCTAssertEqual(networks[safe: 0], BraveWallet.NetworkInfo.mockFilecoinMainnet) XCTAssertEqual(networks[safe: 1], BraveWallet.NetworkInfo.mockSolanaTestnet) XCTAssertEqual(networks[safe: 2], BraveWallet.NetworkInfo.mockSolana)