From 3adcf0e29c9b1872c877240a6a75afeb40da48e4 Mon Sep 17 00:00:00 2001 From: Nuo Xu Date: Thu, 21 Sep 2023 17:19:53 -0700 Subject: [PATCH] Fix brave/brave-ios#8107: Fix incorrect account opening Send from Filecoin asset detail (brave/brave-ios#8118) --- .../Crypto/Accounts/Add/AddAccountView.swift | 48 ++++++++++++----- .../Asset Details/AssetDetailHeaderView.swift | 25 +++++++-- .../Asset Details/AssetDetailView.swift | 28 +++++++++- .../Crypto/BuySendSwap/SendTokenView.swift | 2 + .../Crypto/NetworkSelectionView.swift | 53 ++++++------------- .../Crypto/Stores/AssetDetailStore.swift | 12 +++++ .../Crypto/Stores/NetworkStore.swift | 4 ++ .../Extensions/KeyringServiceExtensions.swift | 11 ++++ .../Extensions/ViewExtensions.swift | 45 ++++++++++++++++ 9 files changed, 174 insertions(+), 54 deletions(-) diff --git a/Sources/BraveWallet/Crypto/Accounts/Add/AddAccountView.swift b/Sources/BraveWallet/Crypto/Accounts/Add/AddAccountView.swift index 8d29ce2b2804..212125d50b15 100644 --- a/Sources/BraveWallet/Crypto/Accounts/Add/AddAccountView.swift +++ b/Sources/BraveWallet/Crypto/Accounts/Add/AddAccountView.swift @@ -29,6 +29,7 @@ struct AddAccountView: View { private var allFilNetworks: [BraveWallet.NetworkInfo] var preSelectedCoin: BraveWallet.CoinType? + var preSelectedFilecoinNetwork: BraveWallet.NetworkInfo? var onCreate: (() -> Void)? var onDismiss: (() -> Void)? @@ -36,6 +37,7 @@ struct AddAccountView: View { keyringStore: KeyringStore, networkStore: NetworkStore, preSelectedCoin: BraveWallet.CoinType? = nil, + preSelectedFilecoinNetwork: BraveWallet.NetworkInfo? = nil, onCreate: (() -> Void)? = nil, onDismiss: (() -> Void)? = nil ) { @@ -44,11 +46,17 @@ struct AddAccountView: View { self.preSelectedCoin = preSelectedCoin self.onCreate = onCreate self.onDismiss = onDismiss - _filNetwork = .init(initialValue: self.allFilNetworks.first ?? .init()) + if let preSelectedFilecoinNetwork, preSelectedFilecoinNetwork.coin == .fil { // make sure the prefilled + // network is a Filecoin network + self.preSelectedFilecoinNetwork = preSelectedFilecoinNetwork + _filNetwork = .init(initialValue: preSelectedFilecoinNetwork) + } else { + _filNetwork = .init(initialValue: self.allFilNetworks.first ?? .init()) + } } private func addAccount(for coin: BraveWallet.CoinType) { - let accountName = name.isEmpty ? defaultAccountName(for: coin, isPrimary: privateKey.isEmpty) : name + let accountName = name.isEmpty ? defaultAccountName(for: coin, chainId: filNetwork.chainId, isPrimary: privateKey.isEmpty) : name guard accountName.isValidAccountName else { return } if privateKey.isEmpty { @@ -104,16 +112,29 @@ struct AddAccountView: View { @ViewBuilder private var addAccountView: some View { List { if (selectedCoin == .fil || preSelectedCoin == .fil) && !allFilNetworks.isEmpty { - Picker(selection: $filNetwork) { - ForEach(allFilNetworks) { network in - Text(network.chainName) - .foregroundColor(Color(.secondaryBraveLabel)) - .tag(network) + Menu(content: { + Picker(selection: $filNetwork) { + ForEach(allFilNetworks) { network in + Text(network.chainName) + .foregroundColor(Color(.secondaryBraveLabel)) + .tag(network) + } + } label: { + EmptyView() } - } label: { - Text(Strings.Wallet.transactionDetailsNetworkTitle) - .foregroundColor(Color(.braveLabel)) - } + }, label: { + HStack { + Text(Strings.Wallet.transactionDetailsNetworkTitle) + .foregroundColor(Color(.braveLabel)) + Spacer() + Group { + Text(filNetwork.chainName) + Image(systemName: "chevron.up.chevron.down") + } + .foregroundColor(Color(.secondaryBraveLabel)) + } + }) + .disabled(preSelectedFilecoinNetwork != nil) .listRowBackground(Color(.secondaryBraveGroupedBackground)) } accountNameSection @@ -326,8 +347,9 @@ struct AddAccountView: View { } } - private func defaultAccountName(for coin: BraveWallet.CoinType, isPrimary: Bool) -> String { - let keyringInfo = keyringStore.allKeyrings.first { $0.coin == coin } + private func defaultAccountName(for coin: BraveWallet.CoinType, chainId: String, isPrimary: Bool) -> String { + let keyringId = BraveWallet.KeyringId.keyringId(for: coin, on: chainId) + let keyringInfo = keyringStore.allKeyrings.first { $0.id == keyringId } if isPrimary { return String.localizedStringWithFormat(coin.defaultAccountName, (keyringInfo?.accountInfos.filter(\.isPrimary).count ?? 0) + 1) } else { diff --git a/Sources/BraveWallet/Crypto/Asset Details/AssetDetailHeaderView.swift b/Sources/BraveWallet/Crypto/Asset Details/AssetDetailHeaderView.swift index 9abf5cf3c010..713bfb733fcb 100644 --- a/Sources/BraveWallet/Crypto/Asset Details/AssetDetailHeaderView.swift +++ b/Sources/BraveWallet/Crypto/Asset Details/AssetDetailHeaderView.swift @@ -22,6 +22,7 @@ struct AssetDetailHeaderView: View { @ObservedObject var networkStore: NetworkStore @Binding var buySendSwapDestination: BuySendSwapDestination? @Binding var isShowingBridgeAlert: Bool + var onAccountCreationNeeded: (_ savedDestination: BuySendSwapDestination) -> Void @Environment(\.sizeCategory) private var sizeCategory @Environment(\.horizontalSizeClass) private var horizontalSizeClass @@ -73,10 +74,15 @@ struct AssetDetailHeaderView: View { if assetDetailStore.isBuySupported { Button( action: { - buySendSwapDestination = BuySendSwapDestination( + let destination = BuySendSwapDestination( kind: .buy, initialToken: assetDetailStore.assetDetailToken ) + if assetDetailStore.accounts.isEmpty { + onAccountCreationNeeded(destination) + } else { + buySendSwapDestination = destination + } } ) { Text(Strings.Wallet.buy) @@ -85,10 +91,15 @@ struct AssetDetailHeaderView: View { if assetDetailStore.isSendSupported { Button( action: { - buySendSwapDestination = BuySendSwapDestination( + let destination = BuySendSwapDestination( kind: .send, initialToken: assetDetailStore.assetDetailToken ) + if assetDetailStore.accounts.isEmpty { + onAccountCreationNeeded(destination) + } else { + buySendSwapDestination = destination + } } ) { Text(Strings.Wallet.send) @@ -97,10 +108,15 @@ struct AssetDetailHeaderView: View { if assetDetailStore.isSwapSupported && assetDetailStore.assetDetailToken.isFungibleToken { Button( action: { - buySendSwapDestination = BuySendSwapDestination( + let destination = BuySendSwapDestination( kind: .swap, initialToken: assetDetailStore.assetDetailToken ) + if assetDetailStore.accounts.isEmpty { + onAccountCreationNeeded(destination) + } else { + buySendSwapDestination = destination + } } ) { Text(Strings.Wallet.swap) @@ -251,7 +267,8 @@ struct CurrencyDetailHeaderView_Previews: PreviewProvider { keyringStore: .previewStore, networkStore: .previewStore, buySendSwapDestination: .constant(nil), - isShowingBridgeAlert: .constant(false) + isShowingBridgeAlert: .constant(false), + onAccountCreationNeeded: { _ in } ) .padding(.vertical) .previewLayout(.sizeThatFits) diff --git a/Sources/BraveWallet/Crypto/Asset Details/AssetDetailView.swift b/Sources/BraveWallet/Crypto/Asset Details/AssetDetailView.swift index cbf19c78ff81..4b1320a04064 100644 --- a/Sources/BraveWallet/Crypto/Asset Details/AssetDetailView.swift +++ b/Sources/BraveWallet/Crypto/Asset Details/AssetDetailView.swift @@ -23,6 +23,9 @@ struct AssetDetailView: View { @State private var isShowingAddAccount: Bool = false @State private var transactionDetails: TransactionDetailsStore? @State private var isShowingAuroraBridgeAlert: Bool = false + @State private var isPresentingAddAccount: Bool = false + @State private var isPresentingAddAccountConfirmation: Bool = false + @State private var savedBSSDestination: BuySendSwapDestination? @Environment(\.sizeCategory) private var sizeCategory /// Reference to the collection view used to back the `List` on iOS 16+ @@ -160,7 +163,11 @@ struct AssetDetailView: View { keyringStore: keyringStore, networkStore: networkStore, buySendSwapDestination: buySendSwapDestination, - isShowingBridgeAlert: $isShowingAuroraBridgeAlert + isShowingBridgeAlert: $isShowingAuroraBridgeAlert, + onAccountCreationNeeded: { savedDestination in + isPresentingAddAccountConfirmation = true + savedBSSDestination = savedDestination + } ) .resetListHeaderStyle() .padding(.horizontal, tableInset) // inset grouped layout margins workaround @@ -300,6 +307,25 @@ struct AssetDetailView: View { isShowingAuroraBridgeAlert = false } } + .addAccount( + keyringStore: keyringStore, + networkStore: networkStore, + accountNetwork: networkStore.network(for: assetDetailStore.assetDetailToken), + isShowingConfirmation: $isPresentingAddAccountConfirmation, + isShowingAddAccount: $isPresentingAddAccount, + onConfirmAddAccount: { isPresentingAddAccount = true }, + onCancelAddAccount: nil, + onAddAccountDismissed: { + Task { @MainActor in + if await assetDetailStore.handleDismissAddAccount() { + if let savedBSSDestination { + buySendSwapDestination.wrappedValue = savedBSSDestination + self.savedBSSDestination = nil + } + } + } + } + ) } } diff --git a/Sources/BraveWallet/Crypto/BuySendSwap/SendTokenView.swift b/Sources/BraveWallet/Crypto/BuySendSwap/SendTokenView.swift index b4e452eafc00..c06754190c6b 100644 --- a/Sources/BraveWallet/Crypto/BuySendSwap/SendTokenView.swift +++ b/Sources/BraveWallet/Crypto/BuySendSwap/SendTokenView.swift @@ -8,6 +8,7 @@ import Strings import DesignSystem import BigNumber import BraveUI +import BraveCore struct SendTokenView: View { @ObservedObject var keyringStore: KeyringStore @@ -23,6 +24,7 @@ struct SendTokenView: View { @Environment(\.appRatingRequestAction) private var appRatingRequest @Environment(\.openURL) private var openURL + @Environment(\.presentationMode) @Binding private var presentationMode var completion: ((_ success: Bool) -> Void)? var onDismiss: () -> Void diff --git a/Sources/BraveWallet/Crypto/NetworkSelectionView.swift b/Sources/BraveWallet/Crypto/NetworkSelectionView.swift index 731b78be316f..9ad78019bfc0 100644 --- a/Sources/BraveWallet/Crypto/NetworkSelectionView.swift +++ b/Sources/BraveWallet/Crypto/NetworkSelectionView.swift @@ -53,44 +53,25 @@ struct NetworkSelectionView: View { selectNetwork(network) } ) - .background( - Color.clear - .alert( - isPresented: $store.isPresentingNextNetworkAlert - ) { - Alert( - title: Text(String.localizedStringWithFormat(Strings.Wallet.createAccountAlertTitle, store.nextNetwork?.shortChainName ?? "")), - message: Text(Strings.Wallet.createAccountAlertMessage), - primaryButton: .default(Text(Strings.yes), action: { - store.handleCreateAccountAlertResponse(shouldCreateAccount: true) - }), - secondaryButton: .cancel(Text(Strings.no), action: { - store.handleCreateAccountAlertResponse(shouldCreateAccount: false) - }) - ) - } - ) - .background( - Color.clear - .sheet( - isPresented: $store.isPresentingAddAccount - ) { - NavigationView { - AddAccountView( - keyringStore: keyringStore, - networkStore: networkStore, - preSelectedCoin: store.nextNetwork?.coin - ) - } - .navigationViewStyle(.stack) - .onDisappear { - Task { @MainActor in - if await store.handleDismissAddAccount() { - presentationMode.dismiss() - } - } + .addAccount( + keyringStore: keyringStore, + networkStore: networkStore, + accountNetwork: store.nextNetwork, + isShowingConfirmation: $store.isPresentingNextNetworkAlert, + isShowingAddAccount: $store.isPresentingAddAccount, + onConfirmAddAccount: { + store.handleCreateAccountAlertResponse(shouldCreateAccount: true) + }, + onCancelAddAccount: { + store.handleCreateAccountAlertResponse(shouldCreateAccount: false) + }, + onAddAccountDismissed: { + Task { @MainActor in + if await store.handleDismissAddAccount() { + presentationMode.dismiss() } } + } ) } diff --git a/Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift b/Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift index c78954e29f16..36d08dcb514f 100644 --- a/Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift +++ b/Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift @@ -326,6 +326,8 @@ class AssetDetailStore: ObservableObject { return tokenContractAddress.caseInsensitiveCompare(token.contractAddress) == .orderedSame case .erc1155SafeTransferFrom, .solanaDappSignTransaction, .solanaDappSignAndSendTransaction, .solanaSwap: return false + case .ethFilForwarderTransfer: + return false @unknown default: return false } @@ -357,6 +359,16 @@ class AssetDetailStore: ObservableObject { userAssetManager: assetManager ) } + + /// Should be called after dismissing create account. Returns true if an account was created + @MainActor func handleDismissAddAccount() async -> Bool { + if await keyringService.isAccountAvailable(for: assetDetailToken.coin, chainId: assetDetailToken.chainId) { + self.update() + return true + } else { + return false + } + } } extension AssetDetailStore: BraveWalletKeyringServiceObserver { diff --git a/Sources/BraveWallet/Crypto/Stores/NetworkStore.swift b/Sources/BraveWallet/Crypto/Stores/NetworkStore.swift index a092774961da..b58dd6994460 100644 --- a/Sources/BraveWallet/Crypto/Stores/NetworkStore.swift +++ b/Sources/BraveWallet/Crypto/Stores/NetworkStore.swift @@ -267,6 +267,10 @@ public class NetworkStore: ObservableObject { func customNetworkNativeAssetMigration(_ network: BraveWallet.NetworkInfo, completion: (() -> Void)? = nil) { assetManager.addUserAsset(network.nativeToken, completion: completion) } + + func network(for token: BraveWallet.BlockchainToken) -> BraveWallet.NetworkInfo? { + return allChains.first { $0.chainId == token.chainId } ?? customChains.first { $0.chainId == token.chainId } + } } extension NetworkStore: BraveWalletJsonRpcServiceObserver { diff --git a/Sources/BraveWallet/Extensions/KeyringServiceExtensions.swift b/Sources/BraveWallet/Extensions/KeyringServiceExtensions.swift index df340c6332b8..57de683426eb 100644 --- a/Sources/BraveWallet/Extensions/KeyringServiceExtensions.swift +++ b/Sources/BraveWallet/Extensions/KeyringServiceExtensions.swift @@ -63,4 +63,15 @@ extension BraveWalletKeyringService { ) return allKeyrings } + + /// Check if any wallet account has been created given a coin type and a chain id + @MainActor func isAccountAvailable(for coin: BraveWallet.CoinType, chainId: String) async -> Bool { + let keyringId = BraveWallet.KeyringId.keyringId(for: coin, on: chainId) + let keyringInfo = await keyringInfo(keyringId) + // If user restore a wallet, `BraveWallet.KeyringInfo.isKeyringCreated` can be true, + // but `BraveWallet.KeyringInfo.accountInfos` will be empty. + // Hence, we will have to check if `BraveWallet.KeyringInfo.accountInfos` is empty instead of + // checking the boolean of `BraveWallet.KeyringInfo.isKeyringCreated` + return !keyringInfo.accountInfos.isEmpty + } } diff --git a/Sources/BraveWallet/Extensions/ViewExtensions.swift b/Sources/BraveWallet/Extensions/ViewExtensions.swift index 6ed50a13c19e..908a84739d35 100644 --- a/Sources/BraveWallet/Extensions/ViewExtensions.swift +++ b/Sources/BraveWallet/Extensions/ViewExtensions.swift @@ -5,6 +5,7 @@ import SwiftUI import UIKit +import BraveCore extension View { /// Helper for `hidden()` modifier that accepts a boolean determining if we should hide the view or not. @@ -30,4 +31,48 @@ extension View { vc.navigationItem.backButtonDisplayMode = backButtonDisplayMode } } + + func addAccount( + keyringStore: KeyringStore, + networkStore: NetworkStore, + accountNetwork: BraveWallet.NetworkInfo?, + isShowingConfirmation: Binding, + isShowingAddAccount: Binding, + onConfirmAddAccount: @escaping () -> Void, + onCancelAddAccount: (() -> Void)?, + onAddAccountDismissed: @escaping () -> Void + ) -> some View { + self.background( + Color.clear + .alert(isPresented: isShowingConfirmation) { + Alert( + title: Text(String.localizedStringWithFormat(Strings.Wallet.createAccountAlertTitle, accountNetwork?.shortChainName ?? "")), + message: Text(Strings.Wallet.createAccountAlertMessage), + primaryButton: .default(Text(Strings.yes), action: { + onConfirmAddAccount() + }), + secondaryButton: .cancel(Text(Strings.no), action: { + onCancelAddAccount?() + }) + ) + } + ) + .background( + Color.clear + .sheet( + isPresented: isShowingAddAccount + ) { + NavigationView { + AddAccountView( + keyringStore: keyringStore, + networkStore: networkStore, + preSelectedCoin: accountNetwork?.coin, + preSelectedFilecoinNetwork: accountNetwork + ) + } + .navigationViewStyle(.stack) + .onDisappear { onAddAccountDismissed() } + } + ) + } }