Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(wallet): Bitcoin Balance Fetching #23117

Merged
merged 3 commits into from
Apr 22, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ struct Filters {
Preferences.Wallet.nonSelectedAccountsFilter.value =
accounts
.filter({ !$0.isSelected })
.map(\.model.address)
.map(\.model.id)
Preferences.Wallet.nonSelectedNetworksFilter.value =
networks
.filter({ !$0.isSelected })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class AccountActivityStore: ObservableObject, WalletObserverStore {
private let blockchainRegistry: BraveWalletBlockchainRegistry
private let solTxManagerProxy: BraveWalletSolanaTxManagerProxy
private let ipfsApi: IpfsAPI
private let bitcoinWalletService: BraveWalletBitcoinWalletService
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.
Expand Down Expand Up @@ -75,6 +76,7 @@ class AccountActivityStore: ObservableObject, WalletObserverStore {
blockchainRegistry: BraveWalletBlockchainRegistry,
solTxManagerProxy: BraveWalletSolanaTxManagerProxy,
ipfsApi: IpfsAPI,
bitcoinWalletService: BraveWalletBitcoinWalletService,
userAssetManager: WalletUserAssetManagerType
) {
self.account = account
Expand All @@ -88,6 +90,7 @@ class AccountActivityStore: ObservableObject, WalletObserverStore {
self.blockchainRegistry = blockchainRegistry
self.solTxManagerProxy = solTxManagerProxy
self.ipfsApi = ipfsApi
self.bitcoinWalletService = bitcoinWalletService
self.assetManager = userAssetManager
self._isSwapSupported = .init(
wrappedValue: account.coin == .eth || account.coin == .sol
Expand Down Expand Up @@ -223,10 +226,27 @@ class AccountActivityStore: ObservableObject, WalletObserverStore {
)

self.isLoadingAccountFiat = true
let tokenBalances = await self.rpcService.fetchBalancesForTokens(
account: account,
networkAssets: allUserNetworkAssets
)
// TODO: cleanup with balance caching with issue
// https://github.com/brave/brave-browser/issues/36764
var tokenBalances: [String: Double] = [:]
if account.coin == .btc {
let networkAsset = allUserNetworkAssets.first {
$0.network.supportedKeyrings.contains(account.keyringId.rawValue as NSNumber)
}
if let btc = networkAsset?.tokens.first,
let btcBalance = await self.bitcoinWalletService.fetchBTCBalance(
accountId: account.accountId,
type: .total
)
{
tokenBalances = [btc.id: btcBalance]
}
} else {
tokenBalances = await self.rpcService.fetchBalancesForTokens(
account: account,
networkAssets: allUserNetworkAssets
)
}
tokenBalanceCache.merge(with: tokenBalances)

// fetch price for every user asset
Expand Down Expand Up @@ -363,7 +383,7 @@ class AccountActivityStore: ObservableObject, WalletObserverStore {
groupType: .none,
token: token,
network: networkAssets.network,
balanceForAccounts: [account.address: Int(tokenBalances[token.id] ?? 0)],
balanceForAccounts: [account.id: Int(tokenBalances[token.id] ?? 0)],
nftMetadata: nftMetadata[token.id]
)
)
Expand All @@ -375,7 +395,7 @@ class AccountActivityStore: ObservableObject, WalletObserverStore {
network: networkAssets.network,
price: tokenPrices[token.assetRatioId.lowercased()] ?? "",
history: [],
balanceForAccounts: [account.address: tokenBalances[token.id] ?? 0]
balanceForAccounts: [account.id: tokenBalances[token.id] ?? 0]
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,17 @@ class AccountsStore: ObservableObject, WalletObserverStore {

let currencyFormatter: NumberFormatter = .usdCurrencyFormatter

/// Cache of token balances for each account. [account.address: [token.id: balance]]
private var tokenBalancesCache: [String: [String: Double]] = [:]
private typealias TokenBalanceCache = [String: [String: Double]]
/// Cache of token balances for each account. [account.cacheBalanceKey: [token.id: balance]]
private var tokenBalancesCache: TokenBalanceCache = [:]
/// Cache of prices for each token. The key is the token's `assetRatioId`.
private var pricesCache: [String: String] = [:]

private let keyringService: BraveWalletKeyringService
private let rpcService: BraveWalletJsonRpcService
private let walletService: BraveWalletBraveWalletService
private let assetRatioService: BraveWalletAssetRatioService
private let bitcoinWalletService: BraveWalletBitcoinWalletService
private let userAssetManager: WalletUserAssetManagerType

private var keyringServiceObserver: KeyringServiceObserver?
Expand All @@ -56,12 +58,14 @@ class AccountsStore: ObservableObject, WalletObserverStore {
rpcService: BraveWalletJsonRpcService,
walletService: BraveWalletBraveWalletService,
assetRatioService: BraveWalletAssetRatioService,
bitcoinWalletService: BraveWalletBitcoinWalletService,
userAssetManager: WalletUserAssetManagerType
) {
self.keyringService = keyringService
self.rpcService = rpcService
self.walletService = walletService
self.assetRatioService = assetRatioService
self.bitcoinWalletService = bitcoinWalletService
self.userAssetManager = userAssetManager
self.setupObservers()
}
Expand Down Expand Up @@ -139,33 +143,50 @@ class AccountsStore: ObservableObject, WalletObserverStore {
networkAssets allNetworkAssets: [NetworkAssets]
) async {
let balancesForAccounts = await withTaskGroup(
of: [String: [String: Double]].self,
of: TokenBalanceCache.self,
body: { group in
for account in accounts {
group.addTask {
let balancesForTokens: [String: Double] = await self.rpcService.fetchBalancesForTokens(
account: account,
networkAssets: allNetworkAssets
)
return [account.address: balancesForTokens]
// TODO: cleanup with balance caching with issue
// https://github.com/brave/brave-browser/issues/36764
var balancesForTokens: [String: Double] = [:]
if account.coin == .btc {
let networkAssets = allNetworkAssets.first {
$0.network.supportedKeyrings.contains(account.keyringId.rawValue as NSNumber)
}
if let btc = networkAssets?.tokens.first,
let btcBalance = await self.bitcoinWalletService.fetchBTCBalance(
accountId: account.accountId,
type: .total
)
{
balancesForTokens = [btc.id: btcBalance]
}
} else {
balancesForTokens = await self.rpcService.fetchBalancesForTokens(
account: account,
networkAssets: allNetworkAssets
)
}
return [account.id: balancesForTokens]
}
}
return await group.reduce(
into: [String: [String: Double]](),
into: TokenBalanceCache(),
{ partialResult, new in
partialResult.merge(with: new)
}
)
}
)
for account in accounts {
if let updatedBalancesForAccount = balancesForAccounts[account.address] {
if let updatedBalancesForAccount = balancesForAccounts[account.id] {
// if balance fetch failed that we already have cached, don't overwrite existing
if var existing = self.tokenBalancesCache[account.address] {
if var existing = self.tokenBalancesCache[account.id] {
existing.merge(with: updatedBalancesForAccount)
self.tokenBalancesCache[account.address] = existing
self.tokenBalancesCache[account.id] = existing
} else {
self.tokenBalancesCache[account.address] = updatedBalancesForAccount
self.tokenBalancesCache[account.id] = updatedBalancesForAccount
}
}
}
Expand Down Expand Up @@ -223,7 +244,7 @@ class AccountsStore: ObservableObject, WalletObserverStore {
for account: BraveWallet.AccountInfo,
tokens: [BraveWallet.BlockchainToken]
) -> [BraveWallet.BlockchainToken] {
guard let tokenBalancesForAccount = tokenBalancesCache[account.address] else {
guard let tokenBalancesForAccount = tokenBalancesCache[account.id] else {
return []
}
var tokensFiatForAccount: [(token: BraveWallet.BlockchainToken, fiat: Double)] = []
Expand All @@ -247,7 +268,7 @@ class AccountsStore: ObservableObject, WalletObserverStore {
for account: BraveWallet.AccountInfo,
tokens: [BraveWallet.BlockchainToken]
) -> Double {
guard let accountBalanceCache = tokenBalancesCache[account.address] else { return 0 }
guard let accountBalanceCache = tokenBalancesCache[account.id] else { return 0 }
return accountBalanceCache.keys.reduce(0.0) { partialResult, tokenId in
guard let tokenBalanceForAccount = tokenBalanceForAccount(tokenId: tokenId, account: account)
else {
Expand All @@ -271,7 +292,7 @@ class AccountsStore: ObservableObject, WalletObserverStore {
tokenId: String,
account: BraveWallet.AccountInfo
) -> Double? {
tokenBalancesCache[account.address]?[tokenId]
tokenBalancesCache[account.id]?[tokenId]
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class AssetDetailStore: ObservableObject, WalletObserverStore {
private let solTxManagerProxy: BraveWalletSolanaTxManagerProxy
private let ipfsApi: IpfsAPI
private let swapService: BraveWalletSwapService
private let bitcoinWalletService: BraveWalletBitcoinWalletService
private let assetManager: WalletUserAssetManagerType
/// A list of tokens that are supported with the current selected network for all supported
/// on-ramp providers.
Expand Down Expand Up @@ -138,6 +139,7 @@ class AssetDetailStore: ObservableObject, WalletObserverStore {
solTxManagerProxy: BraveWalletSolanaTxManagerProxy,
ipfsApi: IpfsAPI,
swapService: BraveWalletSwapService,
bitcoinWalletService: BraveWalletBitcoinWalletService,
userAssetManager: WalletUserAssetManagerType,
assetDetailType: AssetDetailType
) {
Expand All @@ -150,6 +152,7 @@ class AssetDetailStore: ObservableObject, WalletObserverStore {
self.solTxManagerProxy = solTxManagerProxy
self.ipfsApi = ipfsApi
self.swapService = swapService
self.bitcoinWalletService = bitcoinWalletService
self.assetManager = userAssetManager
self.assetDetailType = assetDetailType

Expand Down Expand Up @@ -456,19 +459,29 @@ class AssetDetailStore: ObservableObject, WalletObserverStore {
@MainActor group -> [AccountBalance] in
for accountAssetViewModel in accountAssetViewModels {
group.addTask { @MainActor in
let balance = await self.rpcService.balance(
for: token,
in: accountAssetViewModel.account,
network: network
)
return [AccountBalance(accountAssetViewModel.account, balance)]
// TODO: cleanup with balance caching with issue
// https://github.com/brave/brave-browser/issues/36764
var tokenBalance: Double?
if accountAssetViewModel.account.coin == .btc {
tokenBalance = await self.bitcoinWalletService.fetchBTCBalance(
accountId: accountAssetViewModel.account.accountId,
type: .total
)
} else {
tokenBalance = await self.rpcService.balance(
for: token,
in: accountAssetViewModel.account,
network: network
)
}
return [AccountBalance(accountAssetViewModel.account, tokenBalance)]
}
}
return await group.reduce([AccountBalance](), { $0 + $1 })
}
for tokenBalance in tokenBalances {
if let index = accountAssetViewModels.firstIndex(where: {
$0.account.address == tokenBalance.account.address
$0.account.id == tokenBalance.account.id
}) {
accountAssetViewModels[index].decimalBalance = tokenBalance.balance ?? 0.0
accountAssetViewModels[index].balance = String(format: "%.4f", tokenBalance.balance ?? 0.0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not specific to Bitcoin, but we need to update balance to be a Double or BDouble on this view model and format as a String in UI code instead of view model. Right now the totalBalance computed property uses this string to calculate the total balance, but this value is rounded to 4 decimal places, however total balance is being displayed with 6 decimal places.
So if I have a balance of 0.00001 BTC it will show as 0.000000 BTC total balance.

Since this isn't specific to Bitcoin, I opened a separate issue for this rounding bug:
brave/brave-browser#37670

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is actually an idea somehere deep in backlog to return balance in weis, satoshis, lamports, etc. So any rounding happens only when we want to display that balance on screen.
Using double in money related ares is discouraged in general

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ public class CryptoStore: ObservableObject, WalletObserverStore {
keyringService: keyringService,
rpcService: rpcService,
walletService: walletService,
txService: txService
txService: txService,
bitcoinWalletService: bitcoinWalletService
)
self.origin = origin

Expand All @@ -184,6 +185,7 @@ public class CryptoStore: ObservableObject, WalletObserverStore {
assetRatioService: assetRatioService,
blockchainRegistry: blockchainRegistry,
ipfsApi: ipfsApi,
bitcoinWalletService: bitcoinWalletService,
userAssetManager: userAssetManager
)
self.nftStore = .init(
Expand Down Expand Up @@ -213,6 +215,7 @@ public class CryptoStore: ObservableObject, WalletObserverStore {
rpcService: rpcService,
walletService: walletService,
assetRatioService: assetRatioService,
bitcoinWalletService: bitcoinWalletService,
userAssetManager: userAssetManager
)
self.marketStore = .init(
Expand Down Expand Up @@ -504,6 +507,7 @@ public class CryptoStore: ObservableObject, WalletObserverStore {
solTxManagerProxy: solTxManagerProxy,
ipfsApi: ipfsApi,
swapService: swapService,
bitcoinWalletService: bitcoinWalletService,
userAssetManager: userAssetManager,
assetDetailType: assetDetailType
)
Expand Down Expand Up @@ -541,6 +545,7 @@ public class CryptoStore: ObservableObject, WalletObserverStore {
blockchainRegistry: blockchainRegistry,
solTxManagerProxy: solTxManagerProxy,
ipfsApi: ipfsApi,
bitcoinWalletService: bitcoinWalletService,
userAssetManager: userAssetManager
)
accountActivityStore = store
Expand Down Expand Up @@ -876,7 +881,7 @@ public class CryptoStore: ObservableObject, WalletObserverStore {
for account in accounts {
if let balancesForAccount = userAssetManager.getBalances(
for: nil,
account: account.address
account: account.id
) {
let balancesScopedForP3A = balancesForAccount.optionallyFilter(
shouldFilter: !shouldCountTestNetworks,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ class NFTDetailStore: ObservableObject, WalletObserverStore {
in: account,
network: network
)
return [account.address: Int(balanceForToken ?? 0)]
return [account.id: Int(balanceForToken ?? 0)]
}
}
return await group.reduce(
Expand All @@ -203,11 +203,11 @@ class NFTDetailStore: ObservableObject, WalletObserverStore {
)
}
)
if let address = nftBalances.first(where: { address, balance in
if let uniqueKey = nftBalances.first(where: { _, balance in
balance > 0
})?.key,
let account = accounts.first(where: { accountInfo in
accountInfo.address.caseInsensitiveCompare(address) == .orderedSame
accountInfo.id.caseInsensitiveCompare(uniqueKey) == .orderedSame
})
{
owner = account
Expand Down
Loading
Loading