From 786c8c2cab5ada9ff87553fc164271232900d60d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Honza=20Rychnovsk=C3=BD?= Date: Tue, 23 Apr 2024 08:42:34 +0200 Subject: [PATCH] [#1371] Improve Balances widget loader logic - Closes #1371 - This also covers the security audit recommendation: 4.2.14 (App) This comment in WalletSnapshot.kt should be ticketed --- CHANGELOG.md | 1 + .../zcash/ui/common/model/WalletSnapshot.kt | 6 +++ .../ui/common/viewmodel/WalletViewModel.kt | 45 +++++++++---------- .../ui/screen/balances/view/BalancesView.kt | 10 ++--- 4 files changed, 33 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5086f72e..e5106b14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ directly impact users rather than highlighting other key architectural updates.* ### Fixed - Sending zero funds is allowed only for shielded recipient address type +- The Balances widget loader has been improved to better handle cases, like a wallet with only transparent funds ## [0.2.0 (609)] - 2024-04-18 diff --git a/ui-lib/src/main/java/co/electriccoin/zcash/ui/common/model/WalletSnapshot.kt b/ui-lib/src/main/java/co/electriccoin/zcash/ui/common/model/WalletSnapshot.kt index 2ef6cd66..a5dba40e 100644 --- a/ui-lib/src/main/java/co/electriccoin/zcash/ui/common/model/WalletSnapshot.kt +++ b/ui-lib/src/main/java/co/electriccoin/zcash/ui/common/model/WalletSnapshot.kt @@ -34,6 +34,8 @@ data class WalletSnapshot( val isSendEnabled: Boolean get() = hasSaplingFunds && hasOrchardFunds } +// TODO [#1370]: WalletSnapshot.canSpend() calculation limitation +// TODO [#1370]: https://github.com/Electric-Coin-Company/zashi-android/issues/1370 // Note this check is not entirely correct - it does not calculate the resulting fee using the new Proposal API. It's // fine for now, but it's subject to improvement later once we figure out how to handle it in such cases. fun WalletSnapshot.canSpend(amount: Zatoshi): Boolean = spendableBalance() >= amount @@ -48,5 +50,9 @@ fun WalletSnapshot.spendableBalance() = orchardBalance.available + saplingBalanc // Note that summing both values could be confusing, and we might prefer dividing them in the future fun WalletSnapshot.changePendingBalance() = orchardBalance.changePending + saplingBalance.changePending +fun WalletSnapshot.hasChangePending() = changePendingBalance().value > 0L + // Note that summing both values could be confusing, and we might prefer dividing them in the future fun WalletSnapshot.valuePendingBalance() = orchardBalance.valuePending + saplingBalance.valuePending + +fun WalletSnapshot.hasValuePending() = valuePendingBalance().value > 0L diff --git a/ui-lib/src/main/java/co/electriccoin/zcash/ui/common/viewmodel/WalletViewModel.kt b/ui-lib/src/main/java/co/electriccoin/zcash/ui/common/viewmodel/WalletViewModel.kt index 141595c1..40464f45 100644 --- a/ui-lib/src/main/java/co/electriccoin/zcash/ui/common/viewmodel/WalletViewModel.kt +++ b/ui-lib/src/main/java/co/electriccoin/zcash/ui/common/viewmodel/WalletViewModel.kt @@ -32,6 +32,7 @@ import co.electriccoin.zcash.ui.common.extension.throttle import co.electriccoin.zcash.ui.common.model.OnboardingState import co.electriccoin.zcash.ui.common.model.WalletRestoringState import co.electriccoin.zcash.ui.common.model.WalletSnapshot +import co.electriccoin.zcash.ui.common.model.hasChangePending import co.electriccoin.zcash.ui.common.model.spendableBalance import co.electriccoin.zcash.ui.common.model.totalBalance import co.electriccoin.zcash.ui.preference.EncryptedPreferenceKeys @@ -242,34 +243,32 @@ class WalletViewModel(application: Application) : AndroidViewModel(application) ) /** - * A flow of the wallet balances state used for the UI layer. It combines [WalletSnapshot] with - * [WalletRestoringState] and provides the correct [BalanceState] UI state. + * A flow of the wallet balances state used for the UI layer. It's computed form [WalletSnapshot]'s properties + * and provides the result [BalanceState] UI state. */ val balanceState: StateFlow = walletSnapshot .filterNotNull() - .combine(walletRestoringState) { - walletSnapshot: WalletSnapshot, walletRestoringState: WalletRestoringState -> - when (walletRestoringState) { - WalletRestoringState.NONE -> BalanceState.None - WalletRestoringState.INITIATING -> - BalanceState.Available( - totalBalance = walletSnapshot.totalBalance(), - spendableBalance = walletSnapshot.spendableBalance() + .map { snapshot -> + when { + // Show the loader only under these conditions: + // - Available balance is currently zero + // - Wallet has some ChangePending in progress + // - And Total balance is non-zero + ( + snapshot.spendableBalance().value == 0L && + snapshot.hasChangePending() && + snapshot.totalBalance().value > 0L + ) -> { + BalanceState.Loading( + totalBalance = snapshot.totalBalance() + ) + } + else -> { + BalanceState.Available( + totalBalance = snapshot.totalBalance(), + spendableBalance = snapshot.spendableBalance() ) - WalletRestoringState.RESTORING, WalletRestoringState.SYNCING -> { - if (walletSnapshot.spendableBalance().value == 0L && - walletSnapshot.totalBalance().value > 0L - ) { - BalanceState.Loading( - totalBalance = walletSnapshot.totalBalance() - ) - } else { - BalanceState.Available( - totalBalance = walletSnapshot.totalBalance(), - spendableBalance = walletSnapshot.spendableBalance() - ) - } } } }.stateIn( diff --git a/ui-lib/src/main/java/co/electriccoin/zcash/ui/screen/balances/view/BalancesView.kt b/ui-lib/src/main/java/co/electriccoin/zcash/ui/screen/balances/view/BalancesView.kt index aed8b2bd..92fab97c 100644 --- a/ui-lib/src/main/java/co/electriccoin/zcash/ui/screen/balances/view/BalancesView.kt +++ b/ui-lib/src/main/java/co/electriccoin/zcash/ui/screen/balances/view/BalancesView.kt @@ -57,6 +57,8 @@ import co.electriccoin.zcash.ui.common.compose.SynchronizationStatus import co.electriccoin.zcash.ui.common.model.WalletRestoringState import co.electriccoin.zcash.ui.common.model.WalletSnapshot import co.electriccoin.zcash.ui.common.model.changePendingBalance +import co.electriccoin.zcash.ui.common.model.hasChangePending +import co.electriccoin.zcash.ui.common.model.hasValuePending import co.electriccoin.zcash.ui.common.model.spendableBalance import co.electriccoin.zcash.ui.common.model.valuePendingBalance import co.electriccoin.zcash.ui.common.test.CommonTag @@ -569,8 +571,6 @@ fun ChangePendingRow(walletSnapshot: WalletSnapshot) { ) Row(verticalAlignment = Alignment.CenterVertically) { - val changePendingHasValue = walletSnapshot.changePendingBalance().value > 0L - StyledBalance( balanceString = walletSnapshot.changePendingBalance().toZecString(), textStyles = @@ -584,7 +584,7 @@ fun ChangePendingRow(walletSnapshot: WalletSnapshot) { Spacer(modifier = Modifier.width(12.dp)) Box(Modifier.width(ZcashTheme.dimens.circularSmallProgressWidth)) { - if (changePendingHasValue) { + if (walletSnapshot.hasChangePending()) { CircularSmallProgressIndicator() } } @@ -605,8 +605,6 @@ fun PendingTransactionsRow(walletSnapshot: WalletSnapshot) { ) Row(verticalAlignment = Alignment.CenterVertically) { - val valuePendingHasValue = walletSnapshot.valuePendingBalance().value > 0L - StyledBalance( balanceString = walletSnapshot.valuePendingBalance().toZecString(), textStyles = @@ -620,7 +618,7 @@ fun PendingTransactionsRow(walletSnapshot: WalletSnapshot) { Spacer(modifier = Modifier.width(12.dp)) Box(Modifier.width(ZcashTheme.dimens.circularSmallProgressWidth)) { - if (valuePendingHasValue) { + if (walletSnapshot.hasValuePending()) { CircularSmallProgressIndicator() } }