Skip to content

Commit

Permalink
Merge pull request #1516 from LukasKorba/1512-Ensure-that-the-SDK-doe…
Browse files Browse the repository at this point in the history
…s-not-assume-a-default-account-anywhere

[#1512] Ensure that the SDK does not assume a default account anywhere
  • Loading branch information
LukasKorba authored Dec 3, 2024
2 parents 670f2ce + db3a4b9 commit a48a75d
Show file tree
Hide file tree
Showing 20 changed files with 236 additions and 180 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this library adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Changed
- `zcashlc_propose_transfer`, `zcashlc_propose_transfer_from_uri` and `zcashlc_propose_shielding` no longer accpt a `use_zip317_fees` parameter; ZIP 317 standard fees are now always used and are not configurable.
- The SDK no longer assumes a default account. All business logic with instances of Zip32AccountIndex(<index>) has been refactored.

## Checkpoints

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ class GetBalanceViewController: UIViewController {
self.title = "Account 0 Balance"

Task { @MainActor [weak self] in
self?.accountBalance = try? await synchronizer.getAccountBalance(accountIndex: accountIndex)
guard let accountIndex = self?.accountIndex else { return }

self?.accountBalance = try? await synchronizer.getAccountsBalances()[accountIndex]
self?.updateLabels()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class GetUTXOsViewController: UIViewController {
self.transparentAddressLabel.text = tAddress

// swiftlint:disable:next force_try
let balance = try! await AppDelegate.shared.sharedSynchronizer.getAccountBalance(accountIndex: accountIndex)?.unshielded ?? .zero
let balance = try! await AppDelegate.shared.sharedSynchronizer.getAccountsBalances()[accountIndex]?.unshielded ?? .zero

self.totalBalanceLabel.text = NumberFormatter.zcashNumberFormatter.string(from: NSNumber(value: balance.amount))
self.verifiedBalanceLabel.text = NumberFormatter.zcashNumberFormatter.string(from: NSNumber(value: balance.amount))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,10 @@ class SendViewController: UIViewController {

func updateBalance() async {
balanceLabel.text = format(
balance: (try? await synchronizer.getAccountBalance(accountIndex: accountIndex))?.saplingBalance.total() ?? .zero
balance: (try? await synchronizer.getAccountsBalances()[accountIndex])?.saplingBalance.total() ?? .zero
)
verifiedBalanceLabel.text = format(
balance: (try? await synchronizer.getAccountBalance(accountIndex: accountIndex))?.saplingBalance.spendableValue ?? .zero
balance: (try? await synchronizer.getAccountsBalances()[accountIndex])?.saplingBalance.spendableValue ?? .zero
)
}

Expand All @@ -123,7 +123,7 @@ class SendViewController: UIViewController {
func maxFundsOn() {
Task { @MainActor in
let fee = Zatoshi(10000)
let max: Zatoshi = ((try? await synchronizer.getAccountBalance(accountIndex: accountIndex))?.saplingBalance.spendableValue ?? .zero) - fee
let max: Zatoshi = ((try? await synchronizer.getAccountsBalances()[accountIndex])?.saplingBalance.spendableValue ?? .zero) - fee
amountTextField.text = format(balance: max)
amountTextField.isEnabled = false
}
Expand All @@ -145,12 +145,12 @@ class SendViewController: UIViewController {
}

func isBalanceValid() async -> Bool {
let balance = (try? await synchronizer.getAccountBalance(accountIndex: accountIndex))?.saplingBalance.spendableValue ?? .zero
let balance = (try? await synchronizer.getAccountsBalances()[accountIndex])?.saplingBalance.spendableValue ?? .zero
return balance > .zero
}

func isAmountValid() async -> Bool {
let balance = (try? await synchronizer.getAccountBalance(accountIndex: accountIndex))?.saplingBalance.spendableValue ?? .zero
let balance = (try? await synchronizer.getAccountsBalances()[accountIndex])?.saplingBalance.spendableValue ?? .zero
guard
let value = amountTextField.text,
let amount = NumberFormatter.zcashNumberFormatter.number(from: value).flatMap({ Zatoshi($0.int64Value) }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ extension SaplingParamsAction: Action {

func run(with context: ActionContext, didUpdate: @escaping (CompactBlockProcessor.Event) async -> Void) async throws -> ActionContext {
logger.debug("Fetching sapling parameters")
// TODO: [#1512] This is hardcoded Zip32AccountIndex for index 0, must be updated
// https://github.com/Electric-Coin-Company/zcash-swift-wallet-sdk/issues/1512
try await saplingParametersHandler.handleIfNeeded(accountIndex: Zip32AccountIndex(0))
try await saplingParametersHandler.handleIfNeeded()

await context.update(state: .updateSubtreeRoots)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ struct SaplingParametersHandlerConfig {
}

protocol SaplingParametersHandler {
func handleIfNeeded(accountIndex: Zip32AccountIndex) async throws
func handleIfNeeded() async throws
}

struct SaplingParametersHandlerImpl {
Expand All @@ -24,17 +24,36 @@ struct SaplingParametersHandlerImpl {
}

extension SaplingParametersHandlerImpl: SaplingParametersHandler {
func handleIfNeeded(accountIndex: Zip32AccountIndex) async throws {
func handleIfNeeded() async throws {
try Task.checkCancellation()

do {
let totalSaplingBalance =
try await rustBackend.getWalletSummary()?.accountBalances[accountIndex]?.saplingBalance.total().amount
?? 0
let totalTransparentBalance = try await rustBackend.getTransparentBalance(accountIndex: accountIndex)
let accounts = try await rustBackend.listAccounts()

var totalSaplingBalanceTrigger = false
var totalTransparentBalanceTrigger = false
let accountBalances = try await rustBackend.getWalletSummary()?.accountBalances

for account in accounts {
let zip32AccountIndex = Zip32AccountIndex(account.index)

let totalSaplingBalance = accountBalances?[zip32AccountIndex]?.saplingBalance.total().amount ?? 0

if totalSaplingBalance > 0 {
totalSaplingBalanceTrigger = true
break
}

let totalTransparentBalance = try await rustBackend.getTransparentBalance(accountIndex: zip32AccountIndex)

if totalTransparentBalance > 0 {
totalTransparentBalanceTrigger = true
break
}
}

// Download Sapling parameters only if sapling funds are detected.
guard totalSaplingBalance > 0 || totalTransparentBalance > 0 else { return }
guard totalSaplingBalanceTrigger || totalTransparentBalanceTrigger else { return }
} catch {
// if sapling balance can't be detected of we fail to obtain the balance
// for some reason we shall not proceed to download the parameters and
Expand Down
2 changes: 1 addition & 1 deletion Sources/ZcashLightClientKit/ClosureSynchronizer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public protocol ClosureSynchronizer {

func refreshUTXOs(address: TransparentAddress, from height: BlockHeight, completion: @escaping (Result<RefreshedUTXOs, Error>) -> Void)

func getAccountBalance(accountIndex: Zip32AccountIndex, completion: @escaping (Result<AccountBalance?, Error>) -> Void)
func getAccountsBalances(completion: @escaping (Result<[Zip32AccountIndex: AccountBalance], Error>) -> Void)

func refreshExchangeRateUSD()

Expand Down
17 changes: 12 additions & 5 deletions Sources/ZcashLightClientKit/Metrics/SDKMetrics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -105,18 +105,25 @@ final class SDKMetricsImpl: SDKMetrics {
func logCBPOverviewReport(_ logger: Logger, walletSummary: WalletSummary?) async {
actionStop()

// TODO: [#1512] This is hardcoded Zip32AccountIndex for index 0, must be updated
// https://github.com/Electric-Coin-Company/zcash-swift-wallet-sdk/issues/1512
let accountBalance = walletSummary?.accountBalances[Zip32AccountIndex(0)]
logger.sync(
"""
SYNC (\(syncs)) REPORT
finished in: \(Date().timeIntervalSince1970 - cbpStartTime)
verified balance: \(accountBalance?.saplingBalance.spendableValue.amount ?? 0)
total balance: \(accountBalance?.saplingBalance.total().amount ?? 0)
"""
)

if let accountBalances = walletSummary?.accountBalances {
for accountBalance in accountBalances {
logger.sync(
"""
account index: \(accountBalance.key)
verified balance: \(accountBalance.value.saplingBalance.spendableValue.amount)
total balance: \(accountBalance.value.saplingBalance.total().amount)
"""
)
}
}

try? await Task.sleep(nanoseconds: 100_000)

for action in cbpOverview {
Expand Down
15 changes: 7 additions & 8 deletions Sources/ZcashLightClientKit/Synchronizer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public struct SynchronizerState: Equatable {
/// SyncSessionIDs are provided to users
public var syncSessionID: UUID
/// account balance known to this synchronizer given the data that has processed locally
public var accountBalance: AccountBalance?
public var accountsBalances: [Zip32AccountIndex: AccountBalance]
/// status of the whole sync process
var internalSyncStatus: InternalSyncStatus
public var syncStatus: SyncStatus
Expand All @@ -47,20 +47,20 @@ public struct SynchronizerState: Equatable {
public static var zero: SynchronizerState {
SynchronizerState(
syncSessionID: .nullID,
accountBalance: .zero,
accountsBalances: [:],
internalSyncStatus: .unprepared,
latestBlockHeight: .zero
)
}

init(
syncSessionID: UUID,
accountBalance: AccountBalance?,
accountsBalances: [Zip32AccountIndex: AccountBalance],
internalSyncStatus: InternalSyncStatus,
latestBlockHeight: BlockHeight
) {
self.syncSessionID = syncSessionID
self.accountBalance = accountBalance
self.accountsBalances = accountsBalances
self.internalSyncStatus = internalSyncStatus
self.latestBlockHeight = latestBlockHeight
self.syncStatus = internalSyncStatus.mapToSyncStatus()
Expand Down Expand Up @@ -307,10 +307,9 @@ public protocol Synchronizer: AnyObject {
/// `SynchronizerErrors.notPrepared`.
func refreshUTXOs(address: TransparentAddress, from height: BlockHeight) async throws -> RefreshedUTXOs

/// Account balances from the given account index
/// - Parameter accountIndex: the ZIP 32 index of the account
/// - Returns: `AccountBalance`, struct that holds Sapling and unshielded balances, or `nil` when no account is associated with the given ZIP 32 index
func getAccountBalance(accountIndex: Zip32AccountIndex) async throws -> AccountBalance?
/// Accounts balances
/// - Returns: `[Zip32AccountIndex: AccountBalance]`, struct that holds Sapling and unshielded balances per account
func getAccountsBalances() async throws -> [Zip32AccountIndex: AccountBalance]

/// Fetches the latest ZEC-USD exchange rate and updates `exchangeRateUSDSubject`.
func refreshExchangeRateUSD()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,9 @@ extension ClosureSDKSynchronizer: ClosureSynchronizer {
}
}

public func getAccountBalance(accountIndex: Zip32AccountIndex, completion: @escaping (Result<AccountBalance?, Error>) -> Void) {
public func getAccountsBalances(completion: @escaping (Result<[Zip32AccountIndex: AccountBalance], Error>) -> Void) {
AsyncToClosureGateway.executeThrowingAction(completion) {
try await self.synchronizer.getAccountBalance(accountIndex: accountIndex)
try await self.synchronizer.getAccountsBalances()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,9 @@ extension CombineSDKSynchronizer: CombineSynchronizer {
}
}

public func getAccountBalance(accountIndex: Zip32AccountIndex) -> SinglePublisher<AccountBalance?, Error> {
public func getAccountsBalances() -> SinglePublisher<[Zip32AccountIndex: AccountBalance], Error> {
AsyncToCombineGateway.executeThrowingAction() {
try await self.synchronizer.getAccountBalance(accountIndex: accountIndex)
try await self.synchronizer.getAccountsBalances()
}
}

Expand Down
15 changes: 6 additions & 9 deletions Sources/ZcashLightClientKit/Synchronizer/SDKSynchronizer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ public class SDKSynchronizer: Synchronizer {
let metrics: SDKMetrics
public let logger: Logger
var tor: TorClient?
// TODO: [#1512] Only one instance of hardcoded account index has been removed in deeper levels and moved here, needs to be resolved in #1512
// https://github.com/Electric-Coin-Company/zcash-swift-wallet-sdk/issues/1512
private var accountIndex = Zip32AccountIndex(0)

// Don't read this variable directly. Use `status` instead. And don't update this variable directly use `updateStatus()` methods instead.
private var underlyingStatus: GenericActor<InternalSyncStatus>
Expand Down Expand Up @@ -397,7 +394,7 @@ public class SDKSynchronizer: Synchronizer {
try throwIfUnprepared()

// let's see if there are funds to shield
guard let tBalance = try await self.getAccountBalance(accountIndex: spendingKey.accountIndex)?.unshielded else {
guard let tBalance = try await self.getAccountsBalances()[spendingKey.accountIndex]?.unshielded else {
throw ZcashError.synchronizerSpendingKeyDoesNotBelongToTheWallet
}

Expand Down Expand Up @@ -511,8 +508,8 @@ public class SDKSynchronizer: Synchronizer {
return try await blockProcessor.refreshUTXOs(tAddress: address, startHeight: height)
}

public func getAccountBalance(accountIndex: Zip32AccountIndex) async throws -> AccountBalance? {
try await initializer.rustBackend.getWalletSummary()?.accountBalances[accountIndex]
public func getAccountsBalances() async throws -> [Zip32AccountIndex: AccountBalance] {
try await initializer.rustBackend.getWalletSummary()?.accountBalances ?? [:]
}

/// Fetches the latest ZEC-USD exchange rate.
Expand Down Expand Up @@ -923,10 +920,10 @@ public class SDKSynchronizer: Synchronizer {

// MARK: notify state

private func snapshotState(status: InternalSyncStatus, accountIndex: Zip32AccountIndex) async -> SynchronizerState {
private func snapshotState(status: InternalSyncStatus) async -> SynchronizerState {
await SynchronizerState(
syncSessionID: syncSession.value,
accountBalance: try? await getAccountBalance(accountIndex: accountIndex),
accountsBalances: (try? await getAccountsBalances()) ?? [:],
internalSyncStatus: status,
latestBlockHeight: latestBlocksDataProvider.latestBlockHeight
)
Expand All @@ -951,7 +948,7 @@ public class SDKSynchronizer: Synchronizer {
if SessionTicker.live.isNewSyncSession(oldStatus, newStatus) {
await self.syncSession.newSession(with: self.syncSessionIDGenerator)
}
newState = await snapshotState(status: newStatus, accountIndex: accountIndex)
newState = await snapshotState(status: newStatus)
}

latestState = newState
Expand Down
Loading

0 comments on commit a48a75d

Please sign in to comment.