Skip to content

Commit

Permalink
Fix brave/brave-ios#8107: Fix incorrect account opening Send from Fil…
Browse files Browse the repository at this point in the history
…ecoin asset detail (brave/brave-ios#8118)
  • Loading branch information
nuo-xu committed Sep 22, 2023
1 parent f2511b2 commit 3adcf0e
Show file tree
Hide file tree
Showing 9 changed files with 174 additions and 54 deletions.
48 changes: 35 additions & 13 deletions Sources/BraveWallet/Crypto/Accounts/Add/AddAccountView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@ struct AddAccountView: View {
private var allFilNetworks: [BraveWallet.NetworkInfo]

var preSelectedCoin: BraveWallet.CoinType?
var preSelectedFilecoinNetwork: BraveWallet.NetworkInfo?
var onCreate: (() -> Void)?
var onDismiss: (() -> Void)?

init(
keyringStore: KeyringStore,
networkStore: NetworkStore,
preSelectedCoin: BraveWallet.CoinType? = nil,
preSelectedFilecoinNetwork: BraveWallet.NetworkInfo? = nil,
onCreate: (() -> Void)? = nil,
onDismiss: (() -> Void)? = nil
) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
28 changes: 27 additions & 1 deletion Sources/BraveWallet/Crypto/Asset Details/AssetDetailView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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+
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
}
}
)
}
}

Expand Down
2 changes: 2 additions & 0 deletions Sources/BraveWallet/Crypto/BuySendSwap/SendTokenView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import Strings
import DesignSystem
import BigNumber
import BraveUI
import BraveCore

struct SendTokenView: View {
@ObservedObject var keyringStore: KeyringStore
Expand All @@ -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
Expand Down
53 changes: 17 additions & 36 deletions Sources/BraveWallet/Crypto/NetworkSelectionView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
}
)
}

Expand Down
12 changes: 12 additions & 0 deletions Sources/BraveWallet/Crypto/Stores/AssetDetailStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions Sources/BraveWallet/Crypto/Stores/NetworkStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 11 additions & 0 deletions Sources/BraveWallet/Extensions/KeyringServiceExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
45 changes: 45 additions & 0 deletions Sources/BraveWallet/Extensions/ViewExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -30,4 +31,48 @@ extension View {
vc.navigationItem.backButtonDisplayMode = backButtonDisplayMode
}
}

func addAccount(
keyringStore: KeyringStore,
networkStore: NetworkStore,
accountNetwork: BraveWallet.NetworkInfo?,
isShowingConfirmation: Binding<Bool>,
isShowingAddAccount: Binding<Bool>,
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() }
}
)
}
}

0 comments on commit 3adcf0e

Please sign in to comment.