[#801] Improve wipe implementation

Closes #801

- `SDKSynchronizer.wipe()` can be now called anytime.
- If the sync is in progress then the sync is first stopped and then
  wipe is executed.
- Wipe now returns AnyPublisher which completes or fails when wipe is
  done.
- Majority of wipe's work is to delete files. That is only operation
  that can throw error during wipe. This operation should succeed every
  time. If it fails that something is seriously wrong. When this happens
  the SDK can happen in inconsistent state. There is no recovery for
  now. Only way how to fix this is to reinstall the app in the device.
- Added hooks mechanism. This is implemented in `AfterSyncHooksManager`
  and it is used by `CompactBlockProcessor`. It's just more organized
  way how to track what should happen when the sync process is canceled.
This commit is contained in:
Michal Fousek 2023-02-20 10:53:04 +01:00
parent e439e66896
commit c1b640b44e
13 changed files with 458 additions and 65 deletions

View File

@ -25,6 +25,7 @@ disabled_rules:
- multiple_closures_with_trailing_closure
- generic_type_name # allow for arbitrarily long generic type names
- redundant_void_return
- empty_parentheses_with_trailing_closure
opt_in_rules:
- mark
@ -148,6 +149,7 @@ identifier_name:
indentation_width:
indentation_width: 4
include_comments: false
line_length:
warning: 150

View File

@ -21,6 +21,7 @@ disabled_rules:
- multiple_closures_with_trailing_closure
- generic_type_name # allow for arbitrarily long generic type names
- redundant_void_return
- empty_parentheses_with_trailing_closure
- implicitly_unwrapped_optional
- force_unwrapping
- type_body_length
@ -138,6 +139,7 @@ identifier_name:
indentation_width:
indentation_width: 4
include_comments: false
line_length:
warning: 150

View File

@ -1,4 +1,10 @@
# Unreleased
- [#801] Improve how wipe call can be used
`SDKSynchronizer.wipe()` function can be now called anytime. It returns `AnyPublisher` which
completes or fails when the wipe is done. For more details read the documentation for this method
in the code.
- [#793] Send synchronizerStopped notification only when sync process stops
`synchronizerStopped` notification is now sent after the sync process stops. It's

View File

@ -5,6 +5,8 @@
// Created by Francisco Gindre on 06/09/2019.
// Copyright © 2019 Electric Coin Company. All rights reserved.
//
import Combine
import UIKit
import ZcashLightClientKit
import NotificationBubbles
@ -14,6 +16,7 @@ var loggerProxy = OSLogger(logLevel: .debug)
// swiftlint:disable force_cast force_try force_unwrapping
@UIApplicationMain
class AppDelegate: UIResponder, UIApplicationDelegate {
var cancellables: [AnyCancellable] = []
var window: UIWindow?
private var wallet: Initializer?
private var synchronizer: SDKSynchronizer?
@ -119,31 +122,29 @@ extension AppDelegate {
static var shared: AppDelegate {
UIApplication.shared.delegate as! AppDelegate
}
func clearDatabases() {
do {
try FileManager.default.removeItem(at: try cacheDbURLHelper())
} catch {
loggerProxy.error("error clearing cache DB: \(error)")
}
do {
try FileManager.default.removeItem(at: try fsBlockDbRootURLHelper())
} catch {
loggerProxy.error("error clearing FsBlockDBRoot: \(error)")
}
func wipe(completion completionClosure: @escaping (Error?) -> Void) {
guard let synchronizer = (UIApplication.shared.delegate as? AppDelegate)?.sharedSynchronizer else { return }
do {
try FileManager.default.removeItem(at: try dataDbURLHelper())
} catch {
loggerProxy.error("error clearing data db: \(error)")
}
do {
try FileManager.default.removeItem(at: try pendingDbURLHelper())
} catch {
loggerProxy.error("error clearing data db: \(error)")
}
// At this point app should show some loader or some UI that indicates action. If the sync is not running then wipe happens immediately.
// But if the sync is in progress then the SDK must first stop it. And it may take some time.
synchronizer.wipe()
// Delay is here to be sure that previously showed alerts are gone and it's safe to show another. Or I want to show loading UI for at
// least one second in case that wipe happens immediately.
.delay(for: .seconds(1), scheduler: DispatchQueue.main, options: .none)
.sink(
receiveCompletion: { completion in
switch completion {
case .finished:
completionClosure(nil)
case .failure(let error):
completionClosure(error)
}
},
receiveValue: { _ in }
)
.store(in: &cancellables)
}
}

View File

@ -45,6 +45,12 @@ class SyncBlocksViewController: UIViewController {
override func viewDidLoad() {
super.viewDidLoad()
self.navigationItem.rightBarButtonItem = UIBarButtonItem(
barButtonSystemItem: .trash,
target: self,
action: #selector(wipe(_:))
)
statusLabel.text = textFor(state: synchronizer.status)
progressBar.progress = 0
let center = NotificationCenter.default
@ -341,3 +347,67 @@ extension SDKMetrics.BlockMetricReport: CustomDebugStringConvertible {
"""
}
}
// MARK: Wipe
extension SyncBlocksViewController {
@objc func wipe(_ sender: Any?) {
let alert = UIAlertController(
title: "Wipe the wallet?",
message: """
You are about to clear existing databases. All synced blocks, stored TXs, etc will be removed form this device only. If the sync is in
progress it may take some time. Please be patient.
""",
preferredStyle: .alert
)
alert.addAction(
UIAlertAction(
title: "Drop it like it's FIAT",
style: UIAlertAction.Style.destructive
) { [weak self] _ in
guard let appDelegate = UIApplication.shared.delegate as? AppDelegate else { return }
appDelegate.wipe() { [weak self] possibleError in
if let error = possibleError {
self?.wipeFailedAlert(error: error)
} else {
self?.wipeSuccessfullAlert()
}
}
}
)
alert.addAction(UIAlertAction(title: "No please! Have mercy!", style: UIAlertAction.Style.cancel, handler: nil))
self.present(alert, animated: true, completion: nil)
}
private func wipeSuccessfullAlert() {
let alert = UIAlertController(
title: "Wipe is done!",
message: nil,
preferredStyle: .alert
)
alert.addAction(UIAlertAction(title: "Ok", style: UIAlertAction.Style.default, handler: nil))
self.present(alert, animated: true, completion: nil)
}
private func wipeFailedAlert(error: Error) {
loggerProxy.error("Wipe error: \(error)")
let alert = UIAlertController(
title: "Wipe FAILED!",
message: """
Something bad happened and wipe failed. This may happen only when some basic IO disk operations failed. The SDK may end up in \
inconsistent state. It's suggested to call the wipe again until it succeeds. Sorry.
\(error)
""",
preferredStyle: .alert
)
alert.addAction(UIAlertAction(title: "Ok", style: UIAlertAction.Style.default, handler: nil))
self.present(alert, animated: true, completion: nil)
}
}

View File

@ -57,7 +57,7 @@ final class TransactionDetailModel {
self.minedHeight = transaction.minedHeight?.description
self.expiryHeight = transaction.expiryHeight?.description
self.created = transaction.blockTime?.description
self.zatoshi = "not available in this entity"
self.zatoshi = NumberFormatter.zcashNumberFormatter.string(from: NSNumber(value: transaction.value.amount))
self.memo = memos.first?.toString()
}
}

View File

@ -6,6 +6,7 @@
// Copyright © 2019 Electric Coin Company. All rights reserved.
//
import Combine
import UIKit
class MainTableViewController: UITableViewController {
@ -15,14 +16,17 @@ class MainTableViewController: UITableViewController {
self.navigationItem.rightBarButtonItem = UIBarButtonItem(
barButtonSystemItem: .trash,
target: self,
action: #selector(clearDatabases(_:))
action: #selector(wipe(_:))
)
}
@objc func clearDatabases(_ sender: Any?) {
@objc func wipe(_ sender: Any?) {
let alert = UIAlertController(
title: "Clear Databases?",
message: "You are about to clear existing databases. You will lose all synced blocks, stored TXs, etc",
title: "Wipe the wallet?",
message: """
You are about to clear existing databases. All synced blocks, stored TXs, etc will be removed form this device only. If the sync is in
progress it may take some time. Please be patient.
""",
preferredStyle: .alert
)
@ -30,12 +34,15 @@ class MainTableViewController: UITableViewController {
UIAlertAction(
title: "Drop it like it's FIAT",
style: UIAlertAction.Style.destructive
) { _ in
guard let appDelegate = UIApplication.shared.delegate as? AppDelegate else {
return
) { [weak self] _ in
guard let appDelegate = UIApplication.shared.delegate as? AppDelegate else { return }
appDelegate.wipe() { [weak self] possibleError in
if let error = possibleError {
self?.wipeFailedAlert(error: error)
} else {
self?.wipeSuccessfullAlert()
}
}
appDelegate.clearDatabases()
}
)
@ -43,6 +50,35 @@ class MainTableViewController: UITableViewController {
self.present(alert, animated: true, completion: nil)
}
private func wipeSuccessfullAlert() {
let alert = UIAlertController(
title: "Wipe is done!",
message: nil,
preferredStyle: .alert
)
alert.addAction(UIAlertAction(title: "Ok", style: UIAlertAction.Style.default, handler: nil))
self.present(alert, animated: true, completion: nil)
}
private func wipeFailedAlert(error: Error) {
loggerProxy.error("Wipe error: \(error)")
let alert = UIAlertController(
title: "Wipe FAILED!",
message: """
Something bad happened and wipe failed. This may happen only when some basic IO disk operations failed. The SDK may end up in \
inconsistent state. It's suggested to call the wipe again until it succeeds. Sorry.
\(error)
""",
preferredStyle: .alert
)
alert.addAction(UIAlertAction(title: "Ok", style: UIAlertAction.Style.default, handler: nil))
self.present(alert, animated: true, completion: nil)
}
override func prepare(for segue: UIStoryboardSegue, sender: Any?) {
if let destination = segue.destination as? TransactionsTableViewController {

View File

@ -253,14 +253,15 @@ actor CompactBlockProcessor {
*/
case synced
}
private var afterSyncHooksManager = AfterSyncHooksManager()
var state: State = .stopped {
didSet {
transitionState(from: oldValue, to: self.state)
}
}
private var needsToStartScanningWhenStopped = false
var config: Configuration {
willSet {
self.stop()
@ -504,7 +505,7 @@ actor CompactBlockProcessor {
notifyError(CompactBlockProcessorError.maxAttemptsReached(attempts: self.maxAttempts))
case .syncing, .enhancing, .fetching, .handlingSaplingFiles:
LoggerProxy.debug("Warning: compact block processor was started while busy!!!!")
self.needsToStartScanningWhenStopped = true
afterSyncHooksManager.insert(hook: .anotherSync)
}
return
}
@ -574,20 +575,44 @@ actor CompactBlockProcessor {
return rewindBlockHeight
}
func wipe() async throws {
func wipe(context: AfterSyncHooksManager.WipeContext) async {
LoggerProxy.debug("Starting wipe")
switch self.state {
case .syncing, .enhancing, .fetching, .handlingSaplingFiles:
throw CompactBlockProcessorError.wipeAttemptWhileProcessing
LoggerProxy.debug("Stopping sync because of wipe")
afterSyncHooksManager.insert(hook: .wipe(context))
stop()
case .stopped, .error, .synced:
break
LoggerProxy.debug("Sync doesn't run. Executing wipe.")
await doWipe(context: context)
}
}
private func doWipe(context: AfterSyncHooksManager.WipeContext) async {
context.prewipe()
state = .stopped
try await self.storage.clear()
await internalSyncProgress.rewind(to: 0)
do {
try await self.storage.clear()
await internalSyncProgress.rewind(to: 0)
wipeLegacyCacheDbIfNeeded()
wipeLegacyCacheDbIfNeeded()
let fileManager = FileManager.default
if fileManager.fileExists(atPath: config.dataDb.path) {
try FileManager.default.removeItem(at: config.dataDb)
}
if fileManager.fileExists(atPath: context.pendingDbURL.path) {
try FileManager.default.removeItem(at: context.pendingDbURL)
}
context.completion(nil)
} catch {
context.completion(error)
}
}
func validateServer() async {
@ -679,9 +704,17 @@ actor CompactBlockProcessor {
LoggerProxy.error("Sync failed with error: \(error)")
if Task.isCancelled {
LoggerProxy.info("processing cancelled.")
LoggerProxy.info("Processing cancelled.")
state = .stopped
if needsToStartScanningWhenStopped {
let afterSyncHooksManager = self.afterSyncHooksManager
self.afterSyncHooksManager = AfterSyncHooksManager()
if let wipeContext = afterSyncHooksManager.shouldExecuteWipeHook() {
LoggerProxy.debug("Executing wipe.")
await doWipe(context: wipeContext)
} else if afterSyncHooksManager.shouldExecuteAnotherSyncHook() {
LoggerProxy.debug("Starting new sync.")
await nextBatch()
}
} else {

View File

@ -0,0 +1,69 @@
//
// AfterSyncHooksManager.swift
//
//
// Created by Michal Fousek on 20.02.2023.
//
import Foundation
class AfterSyncHooksManager {
struct WipeContext {
let pendingDbURL: URL
let prewipe: () -> Void
let completion: (Error?) -> Void
}
enum Hook: Equatable, Hashable {
case wipe(WipeContext)
case anotherSync
static func == (lhs: Hook, rhs: Hook) -> Bool {
switch (lhs, rhs) {
case (.wipe, .wipe):
return true
case (.anotherSync, .anotherSync):
return true
default:
return false
}
}
func hash(into hasher: inout Hasher) {
switch self {
case .wipe: hasher.combine(0)
case .anotherSync: hasher.combine(1)
}
}
static var emptyWipe: Hook {
return .wipe(
WipeContext(
pendingDbURL: URL(fileURLWithPath: "/"),
prewipe: { },
completion: { _ in }
)
)
}
}
private var hooks: Set<Hook> = []
init() { }
func insert(hook: Hook) {
hooks.insert(hook)
}
func shouldExecuteWipeHook() -> WipeContext? {
if case let .wipe(wipeContext) = hooks.first(where: { $0 == Hook.emptyWipe }) {
return wipeContext
} else {
return nil
}
}
func shouldExecuteAnotherSyncHook() -> Bool {
return hooks.first(where: { $0 == .anotherSync }) != nil
}
}

View File

@ -6,6 +6,7 @@
// Copyright © 2019 Electric Coin Company. All rights reserved.
//
import Combine
import Foundation
/// Represents errors thrown by a Synchronizer
@ -212,8 +213,17 @@ public protocol Synchronizer {
/// Wipes out internal data structures of the SDK. After this call, everything is the same as before any sync. The state of the synchronizer is
/// switched to `unprepared`. So before the next sync, it's required to call `prepare()`.
///
/// If this is called while the sync process is in progress then `SynchronizerError.wipeAttemptWhileProcessing` is thrown.
func wipe() async throws
/// `wipe()` can be called anytime. If the sync process is in progress then it is stopped first. In this case, it make some significant time
/// before wipe finishes. If `wipe()` is called don't call it again until publisher returned from first call finishes. Calling it again earlier
/// results in undefined behavior.
///
/// Returned publisher either completes or fails when the wipe is done. It doesn't emits any value.
///
/// Majority of wipe's work is to delete files. That is only operation that can throw error during wipe. This should succeed every time. If this
/// fails then something is seriously wrong. If the wipe fails then the SDK may be in inconsistent state. It's suggested to call wipe again until
/// it succeed.
///
func wipe() -> AnyPublisher<Void, Error>
}
public enum SyncStatus: Equatable {

View File

@ -630,21 +630,29 @@ public class SDKSynchronizer: Synchronizer {
}
}
public func wipe() async throws {
do {
try await blockProcessor.wipe()
} catch {
throw SynchronizerError.wipeAttemptWhileProcessing
public func wipe() -> AnyPublisher<Void, Error> {
let publisher = PassthroughSubject<Void, Error>()
Task(priority: .high) {
let context = AfterSyncHooksManager.WipeContext(
pendingDbURL: initializer.pendingDbURL,
prewipe: { [weak self] in
self?.transactionManager.closeDBConnection()
self?.transactionRepository.closeDBConnection()
},
completion: { [weak self] possibleError in
self?.status = .unprepared
if let error = possibleError {
publisher.send(completion: .failure(error))
} else {
publisher.send(completion: .finished)
}
}
)
await blockProcessor.wipe(context: context)
}
transactionManager.closeDBConnection()
transactionRepository.closeDBConnection()
try? FileManager.default.removeItem(at: initializer.fsBlockDbRoot)
try? FileManager.default.removeItem(at: initializer.pendingDbURL)
try? FileManager.default.removeItem(at: initializer.dataDbURL)
status = .unprepared
return publisher.eraseToAnyPublisher()
}
// MARK: notify state

View File

@ -25,6 +25,7 @@ class SychronizerDarksideTests: XCTestCase {
var expectedRewindHeight: BlockHeight = 665188
var reorgExpectation = XCTestExpectation(description: "reorg")
var foundTransactions: [ZcashTransaction.Overview] = []
var cancellables: [AnyCancellable] = []
override func setUpWithError() throws {
try super.setUpWithError()
@ -46,6 +47,7 @@ class SychronizerDarksideTests: XCTestCase {
try? FileManager.default.removeItem(at: coordinator.databases.pendingDB)
coordinator = nil
foundTransactions = []
cancellables = []
}
func testFoundTransactions() throws {
@ -193,7 +195,26 @@ class SychronizerDarksideTests: XCTestCase {
wait(for: [firsSyncExpectation], timeout: 10)
try await coordinator.synchronizer.wipe()
let wipeFinished = XCTestExpectation(description: "SynchronizerWipeFinished Expectation")
coordinator.synchronizer.wipe()
.sink(
receiveCompletion: { completion in
switch completion {
case .finished:
wipeFinished.fulfill()
case .failure(let error):
XCTFail("Wipe should finish successfully. \(error)")
}
},
receiveValue: {
XCTFail("No no value should be received from wipe.")
}
)
.store(in: &cancellables)
wait(for: [wipeFinished], timeout: 1)
_ = try coordinator.synchronizer.prepare(with: nil)

View File

@ -15,8 +15,6 @@ final class SynchronizerTests: XCTestCase {
var birthday: BlockHeight = 663150
let defaultLatestHeight: BlockHeight = 663175
var coordinator: TestCoordinator!
var syncedExpectation = XCTestExpectation(description: "synced")
var sentTransactionExpectation = XCTestExpectation(description: "sent")
var expectedReorgHeight: BlockHeight = 665188
var expectedRewindHeight: BlockHeight = 665188
var reorgExpectation = XCTestExpectation(description: "reorg")
@ -24,7 +22,6 @@ final class SynchronizerTests: XCTestCase {
let chainName = "main"
let network = DarksideWalletDNetwork()
var cancellables: [AnyCancellable] = []
var processorEventHandler: CompactBlockProcessorEventHandler! = CompactBlockProcessorEventHandler()
override func setUpWithError() throws {
try super.setUpWithError()
@ -57,7 +54,6 @@ final class SynchronizerTests: XCTestCase {
try? FileManager.default.removeItem(at: coordinator.databases.pendingDB)
coordinator = nil
cancellables = []
processorEventHandler = nil
}
func handleReorg(event: CompactBlockProcessor.Event) {
@ -102,13 +98,152 @@ final class SynchronizerTests: XCTestCase {
try await Task.sleep(nanoseconds: 5_000_000_000)
self.coordinator.synchronizer.stop()
wait(for: [syncStoppedExpectation], timeout: 6, enforceOrder: true)
wait(for: [syncStoppedExpectation], timeout: 6)
XCTAssertEqual(coordinator.synchronizer.status, .stopped)
let state = await coordinator.synchronizer.blockProcessor.state
XCTAssertEqual(state, .stopped)
}
@MainActor func testWipeCalledWhichSyncDoesntRun() async throws {
/*
create fake chain
*/
let fullSyncLength = 1000
try FakeChainBuilder.buildChain(darksideWallet: coordinator.service, branchID: branchID, chainName: chainName, length: fullSyncLength)
try coordinator.applyStaged(blockheight: birthday + fullSyncLength)
sleep(2)
let syncFinished = XCTestExpectation(description: "SynchronizerSyncFinished Expectation")
/*
sync to latest height
*/
try coordinator.sync(
completion: { _ in
syncFinished.fulfill()
},
error: handleError
)
wait(for: [syncFinished], timeout: 3)
let wipeFinished = XCTestExpectation(description: "SynchronizerWipeFinished Expectation")
/*
Call wipe
*/
coordinator.synchronizer.wipe()
.sink(
receiveCompletion: { completion in
switch completion {
case .finished:
wipeFinished.fulfill()
case .failure(let error):
XCTFail("Wipe should finish successfully. \(error)")
}
},
receiveValue: {
XCTFail("No no value should be received from wipe.")
}
)
.store(in: &cancellables)
wait(for: [wipeFinished], timeout: 1)
/*
Check that wipe cleared everything that is expected
*/
await checkThatWipeWorked()
}
@MainActor func testWipeCalledWhileSyncRuns() async throws {
/*
1. create fake chain
*/
let fullSyncLength = 50_000
try FakeChainBuilder.buildChain(darksideWallet: coordinator.service, branchID: branchID, chainName: chainName, length: fullSyncLength)
try coordinator.applyStaged(blockheight: birthday + fullSyncLength)
sleep(5)
/*
Start sync
*/
try coordinator.sync(completion: { _ in
XCTFail("Sync should have stopped")
}, error: { error in
_ = try? self.coordinator.stop()
guard let testError = error else {
XCTFail("failed with nil error")
return
}
XCTFail("Failed with error: \(testError)")
})
try await Task.sleep(nanoseconds: 2_000_000_000)
// Just to be sure that blockProcessor is still syncing and that this test does what it should.
let blockProcessorState = await coordinator.synchronizer.blockProcessor.state
XCTAssertEqual(blockProcessorState, .syncing)
let wipeFinished = XCTestExpectation(description: "SynchronizerWipeFinished Expectation")
/*
Call wipe
*/
coordinator.synchronizer.wipe()
.sink(
receiveCompletion: { completion in
switch completion {
case .finished:
wipeFinished.fulfill()
case .failure(let error):
XCTFail("Wipe should finish successfully. \(error)")
}
},
receiveValue: {
XCTFail("No no value should be received from wipe.")
}
)
.store(in: &cancellables)
wait(for: [wipeFinished], timeout: 6)
/*
Check that wipe cleared everything that is expected
*/
await checkThatWipeWorked()
}
private func checkThatWipeWorked() async {
let storage = await self.coordinator.synchronizer.blockProcessor.storage as! FSCompactBlockRepository
let fm = FileManager.default
XCTAssertFalse(fm.fileExists(atPath: coordinator.synchronizer.initializer.dataDbURL.path))
XCTAssertFalse(fm.fileExists(atPath: coordinator.synchronizer.initializer.pendingDbURL.path))
XCTAssertFalse(fm.fileExists(atPath: storage.blocksDirectory.path))
let internalSyncProgress = InternalSyncProgress(storage: UserDefaults.standard)
let latestDownloadedBlockHeight = await internalSyncProgress.load(.latestDownloadedBlockHeight)
let latestEnhancedHeight = await internalSyncProgress.load(.latestEnhancedHeight)
let latestUTXOFetchedHeight = await internalSyncProgress.load(.latestUTXOFetchedHeight)
XCTAssertEqual(latestDownloadedBlockHeight, 0)
XCTAssertEqual(latestEnhancedHeight, 0)
XCTAssertEqual(latestUTXOFetchedHeight, 0)
let blockProcessorState = await coordinator.synchronizer.blockProcessor.state
XCTAssertEqual(blockProcessorState, .stopped)
}
func handleError(_ error: Error?) {
_ = try? coordinator.stop()
guard let testError = error else {