From 79ab841f754bd01531d797b4a2ef858133c8a967 Mon Sep 17 00:00:00 2001 From: Lukas Korba Date: Mon, 29 Jan 2024 04:28:18 +0100 Subject: [PATCH] [#997] Keys missing handling (#1000) - changelog update - the keys missing error state has been tweaked to try 3 retry attempts because of unresponsiveness keychain API - in case of true missing keys, the user is no longer locked on a splash screen but rather let land to the Account tab so the rest of the Zashi can be used - unit tests fixed + implemented new ones for the 3-attempt retry logic --- CHANGELOG.md | 3 + .../RecoveryPhraseDisplayView.swift | 2 + .../Features/Root/RootInitialization.swift | 19 ++++- modules/Sources/Features/Root/RootStore.swift | 6 ++ modules/Sources/Generated/L10n.swift | 4 +- .../Generated/Resources/Localizable.strings | 2 +- .../RootTests/AppInitializationTests.swift | 75 ++++++++++++++++++- secantTests/RootTests/RootTests.swift | 16 +++- 8 files changed, 118 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f716562..d3b3c2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ directly impact users rather than highlighting other crucial architectural updat ### Added - Share QR code of addresses via system share dialog. +### Fixed +- `Keys Missing` error dialog was sometimes triggered as a false positive due to system overload and keychain API unresponsivity in expected time. Retry logic was implemented to pass this state. Also the app always lands users to the Account tab instead of lock them on a splash screen with no options to solve this state. + ## 0.2.0 build 12 (2024-01-20) ### Added diff --git a/modules/Sources/Features/RecoveryPhraseDisplay/RecoveryPhraseDisplayView.swift b/modules/Sources/Features/RecoveryPhraseDisplay/RecoveryPhraseDisplayView.swift index 6ec91bd..22ca886 100644 --- a/modules/Sources/Features/RecoveryPhraseDisplay/RecoveryPhraseDisplayView.swift +++ b/modules/Sources/Features/RecoveryPhraseDisplay/RecoveryPhraseDisplayView.swift @@ -82,6 +82,8 @@ public struct RecoveryPhraseDisplayView: View { .padding(.bottom, 50) } else { Text(L10n.RecoveryPhraseDisplay.noWords) + .font(.custom(FontFamily.Inter.regular.name, size: 14)) + .multilineTextAlignment(.center) } } .padding(.horizontal, 60) diff --git a/modules/Sources/Features/Root/RootInitialization.swift b/modules/Sources/Features/Root/RootInitialization.swift index 7dc4190..94a0693 100644 --- a/modules/Sources/Features/Root/RootInitialization.swift +++ b/modules/Sources/Features/Root/RootInitialization.swift @@ -25,6 +25,7 @@ extension RootReducer { case initialSetups case initializationFailed(ZcashError) case initializationSuccessfullyDone(UnifiedAddress?) + case retryKeychainRead(InitializationState) case nukeWallet case nukeWalletRequest case respondToWalletInitializationState(InitializationState) @@ -183,12 +184,10 @@ extension RootReducer { switch walletState { case .failed: state.appInitializationState = .failed - state.alert = AlertState.walletStateFailed(walletState) - return .none + return .send(.initialization(.retryKeychainRead(walletState))) case .keysMissing: state.appInitializationState = .keysMissing - state.alert = AlertState.walletStateFailed(walletState) - return .none + return .send(.initialization(.retryKeychainRead(walletState))) case .initialized, .filesMissing: if walletState == .filesMissing { state.appInitializationState = .filesMissing @@ -206,6 +205,18 @@ extension RootReducer { .cancellable(id: CancelId.timer, cancelInFlight: true) } + case .initialization(.retryKeychainRead(let walletState)): + if state.keychainReadRetries < state.maxKeychainReadRetries { + state.keychainReadRetries += 1 + return .run { [retries = state.keychainReadRetries] send in + try await mainQueue.sleep(for: .seconds(0.1 * Double(retries))) + await send(.initialization(.checkWalletInitialization)) + } + } else { + state.alert = AlertState.walletStateFailed(walletState) + return .send(.destination(.updateDestination(RootReducer.DestinationState.Destination.tabs))) + } + /// Stored wallet is present, database files may or may not be present, trying to initialize app state variables and environments. /// When initialization succeeds user is taken to the home screen. case .initialization(.initializeSDK(let walletMode)): diff --git a/modules/Sources/Features/Root/RootStore.swift b/modules/Sources/Features/Root/RootStore.swift index a304a3b..cd81002 100644 --- a/modules/Sources/Features/Root/RootStore.swift +++ b/modules/Sources/Features/Root/RootStore.swift @@ -40,6 +40,8 @@ public struct RootReducer: Reducer { public var destinationState: DestinationState public var exportLogsState: ExportLogsReducer.State public var isRestoringWallet = false + public var keychainReadRetries = 0 + public var maxKeychainReadRetries = 3 public var onboardingState: OnboardingFlowReducer.State public var phraseDisplayState: RecoveryPhraseDisplayReducer.State public var sandboxState: SandboxReducer.State @@ -55,6 +57,8 @@ public struct RootReducer: Reducer { destinationState: DestinationState, exportLogsState: ExportLogsReducer.State, isRestoringWallet: Bool = false, + keychainReadRetries: Int = 0, + maxKeychainReadRetries: Int = 3, onboardingState: OnboardingFlowReducer.State, phraseDisplayState: RecoveryPhraseDisplayReducer.State, sandboxState: SandboxReducer.State, @@ -68,6 +72,8 @@ public struct RootReducer: Reducer { self.destinationState = destinationState self.exportLogsState = exportLogsState self.isRestoringWallet = isRestoringWallet + self.keychainReadRetries = keychainReadRetries + self.maxKeychainReadRetries = maxKeychainReadRetries self.onboardingState = onboardingState self.phraseDisplayState = phraseDisplayState self.sandboxState = sandboxState diff --git a/modules/Sources/Generated/L10n.swift b/modules/Sources/Generated/L10n.swift index f72b7c9..f8c6d57 100644 --- a/modules/Sources/Generated/L10n.swift +++ b/modules/Sources/Generated/L10n.swift @@ -306,8 +306,8 @@ public enum L10n { } /// The following 24 words are the keys to your funds and are the only way to recover your funds if you get locked out or get a new device. Protect your ZEC by storing this phrase in a place you trust and never share it with anyone! public static let description = L10n.tr("Localizable", "recoveryPhraseDisplay.description", fallback: "The following 24 words are the keys to your funds and are the only way to recover your funds if you get locked out or get a new device. Protect your ZEC by storing this phrase in a place you trust and never share it with anyone!") - /// Oops no words - public static let noWords = L10n.tr("Localizable", "recoveryPhraseDisplay.noWords", fallback: "Oops no words") + /// The keys are missing. No backup phrase is stored in the keychain. + public static let noWords = L10n.tr("Localizable", "recoveryPhraseDisplay.noWords", fallback: "The keys are missing. No backup phrase is stored in the keychain.") /// Your Secret public static let titlePart1 = L10n.tr("Localizable", "recoveryPhraseDisplay.titlePart1", fallback: "Your Secret") /// Recovery Phrase diff --git a/modules/Sources/Generated/Resources/Localizable.strings b/modules/Sources/Generated/Resources/Localizable.strings index dab0a5e..46c15a6 100644 --- a/modules/Sources/Generated/Resources/Localizable.strings +++ b/modules/Sources/Generated/Resources/Localizable.strings @@ -39,7 +39,7 @@ "recoveryPhraseDisplay.description" = "The following 24 words are the keys to your funds and are the only way to recover your funds if you get locked out or get a new device. Protect your ZEC by storing this phrase in a place you trust and never share it with anyone!"; "recoveryPhraseDisplay.button.wroteItDown" = "I got it!"; "recoveryPhraseDisplay.button.copyToBuffer" = "Copy To Buffer"; -"recoveryPhraseDisplay.noWords" = "Oops no words"; +"recoveryPhraseDisplay.noWords" = "The keys are missing. No backup phrase is stored in the keychain."; "recoveryPhraseDisplay.birthdayHeight" = "Wallet birthday height: %@"; "recoveryPhraseDisplay.alert.failed.title" = "Failed to load stored wallet"; "recoveryPhraseDisplay.alert.failed.message" = "Attempt to load the stored wallet from the keychain failed. Error: %@ (code: %@)"; diff --git a/secantTests/RootTests/AppInitializationTests.swift b/secantTests/RootTests/AppInitializationTests.swift index e5e1799..0b905a7 100644 --- a/secantTests/RootTests/AppInitializationTests.swift +++ b/secantTests/RootTests/AppInitializationTests.swift @@ -174,8 +174,11 @@ class AppInitializationTests: XCTestCase { /// Integration test validating the side effects work together properly when no wallet is stored but database files are present. @MainActor func testDidFinishLaunching_to_KeysMissing() async throws { + var initialState = RootReducer.State.initial + initialState.keychainReadRetries = 3 + let store = TestStore( - initialState: .initial + initialState: initialState ) { RootReducer(tokenName: "ZEC", zcashNetwork: ZcashNetworkBuilder.network(for: .testnet)) } @@ -199,9 +202,79 @@ class AppInitializationTests: XCTestCase { await store.receive(.initialization(.respondToWalletInitializationState(.keysMissing))) { state in state.appInitializationState = .keysMissing + } + + await store.receive(.initialization(.retryKeychainRead(.keysMissing))) { state in state.alert = AlertState.walletStateFailed(.keysMissing) } + await store.receive(.destination(.updateDestination(.tabs))) { state in + state.destinationState.internalDestination = .tabs + state.destinationState.previousDestination = .welcome + } + + await store.finish() + } + + @MainActor func testDidFinishLaunching_to_KeysMissing_3attempts() async throws { + let store = TestStore( + initialState: .initial + ) { + RootReducer(tokenName: "ZEC", zcashNetwork: ZcashNetworkBuilder.network(for: .testnet)) + } + + store.dependencies.databaseFiles = .noOp + store.dependencies.databaseFiles.areDbFilesPresentFor = { _ in true } + store.dependencies.walletStorage = .noOp + store.dependencies.mainQueue = .immediate + store.dependencies.walletConfigProvider = .noOp + store.dependencies.crashReporter = .noOp + store.dependencies.restoreWalletStorage = .noOp + + // Root of the test, the app finished the launch process and triggers the checks and initializations. + await store.send(.initialization(.appDelegate(.didFinishLaunching))) + + await store.receive(.initialization(.initialSetups)) + + await store.receive(.initialization(.configureCrashReporter)) + + // initial attempt + await store.receive(.initialization(.checkWalletInitialization)) + + await store.receive(.initialization(.respondToWalletInitializationState(.keysMissing))) { state in + state.appInitializationState = .keysMissing + } + + await store.receive(.initialization(.retryKeychainRead(.keysMissing))) { state in + state.keychainReadRetries = 1 + } + + // 1st attempt + await store.receive(.initialization(.checkWalletInitialization)) + await store.receive(.initialization(.respondToWalletInitializationState(.keysMissing))) + await store.receive(.initialization(.retryKeychainRead(.keysMissing))) { state in + state.keychainReadRetries = 2 + } + + // 2nd attempt + await store.receive(.initialization(.checkWalletInitialization)) + await store.receive(.initialization(.respondToWalletInitializationState(.keysMissing))) + await store.receive(.initialization(.retryKeychainRead(.keysMissing))) { state in + state.keychainReadRetries = 3 + } + + // 3rd attempt + await store.receive(.initialization(.checkWalletInitialization)) + await store.receive(.initialization(.respondToWalletInitializationState(.keysMissing))) + await store.receive(.initialization(.retryKeychainRead(.keysMissing))) { state in + state.alert = AlertState.walletStateFailed(.keysMissing) + } + + await store.receive(.destination(.updateDestination(.tabs))) { state in + state.destinationState.internalDestination = .tabs + state.destinationState.previousDestination = .welcome + } + await store.finish() } diff --git a/secantTests/RootTests/RootTests.swift b/secantTests/RootTests/RootTests.swift index 99d2d7d..6a0d791 100644 --- a/secantTests/RootTests/RootTests.swift +++ b/secantTests/RootTests/RootTests.swift @@ -133,7 +133,21 @@ class RootTests: XCTestCase { await store.send(.initialization(.respondToWalletInitializationState(.keysMissing))) { state in state.appInitializationState = .keysMissing - state.alert = AlertState.walletStateFailed(.keysMissing) + } + + await store.receive(.initialization(.retryKeychainRead(.keysMissing))) { state in + state.keychainReadRetries = 1 + } + + await store.receive(.initialization(.checkWalletInitialization)) + + await store.receive(.initialization(.respondToWalletInitializationState(.uninitialized))) { state in + state.appInitializationState = .uninitialized + } + + await store.receive(.destination(.updateDestination(.onboarding))) { state in + state.destinationState.internalDestination = .onboarding + state.destinationState.previousDestination = .welcome } await store.finish()