From 60aa28628bf0f36d20789f3be9f576df71795b73 Mon Sep 17 00:00:00 2001 From: Francisco Gindre Date: Fri, 16 Sep 2022 16:59:31 -0300 Subject: [PATCH 1/3] [#532] [0.16.x-beta] Download does not stop correctly Issue Reported: When the synchronizer is stopped, the processor does not cancel the download correctly. Then when attempting to resume sync, the synchronizer is not on .stopped and can't be resumed this doesn't appear to happen in master branch that uses structured concurrency for operations. Fix: This commit makes sure that the download streamer checks cancelation before processing any block, or getting called back to report progress --- .../CompactBlockDownloadOperation.swift | 29 +++- Tests/DarksideTests/SynchronizerTests.swift | 128 ++++++++++++++++++ 2 files changed, 151 insertions(+), 6 deletions(-) create mode 100644 Tests/DarksideTests/SynchronizerTests.swift diff --git a/Sources/ZcashLightClientKit/Block/Processor/CompactBlockDownloadOperation.swift b/Sources/ZcashLightClientKit/Block/Processor/CompactBlockDownloadOperation.swift index 8ecf6516..06cd0601 100644 --- a/Sources/ZcashLightClientKit/Block/Processor/CompactBlockDownloadOperation.swift +++ b/Sources/ZcashLightClientKit/Block/Processor/CompactBlockDownloadOperation.swift @@ -111,41 +111,58 @@ class CompactBlockStreamDownloadOperation: ZcashOperation { let startHeight = max(self.startHeight ?? BlockHeight.empty(), latestDownloaded) self.cancelable = self.service.blockStream(startHeight: startHeight, endHeight: latestHeight) { [weak self] blockResult in + guard let self = self, !self.isCancelled else { + self?.cancel() + return + } + + switch blockResult { case .success(let result): switch result { case .success: do { - try self?.flush() - self?.done = true + try self.flush() + self.done = true } catch { - self?.fail(error: error) + self.fail(error: error) } return case .error(let e): - self?.fail(error: e) + self.fail(error: e) } case .failure(let e): if case .userCancelled = e { - self?.done = true + self.done = true } else { - self?.fail(error: e) + self.fail(error: e) } } } handler: {[weak self] block in guard let self = self else { return } + guard !self.isCancelled else { + self.cancel() + return + } do { try self.cache(block, flushCache: false) } catch { self.fail(error: error) } } progress: { progress in + guard !self.isCancelled else { + self.cancel() + return + } self.progressDelegate?.progressUpdated(.download(progress)) } while !done && !isCancelled { sleep(1) } + if isCancelled { + self.cancel() + } } catch { self.fail(error: error) } diff --git a/Tests/DarksideTests/SynchronizerTests.swift b/Tests/DarksideTests/SynchronizerTests.swift new file mode 100644 index 00000000..c500f0e4 --- /dev/null +++ b/Tests/DarksideTests/SynchronizerTests.swift @@ -0,0 +1,128 @@ +// +// SynchronizerTests.swift +// DarksideTests +// +// Created by Francisco Gindre on 9/16/22. +// + +import XCTest +@testable import TestUtils +@testable import ZcashLightClientKit + +// swiftlint:disable implicitly_unwrapped_optional force_unwrapping type_body_length +final class SynchronizerTests: XCTestCase { + + // TODO: Parameterize this from environment? + // swiftlint:disable:next line_length + var seedPhrase = "still champion voice habit trend flight survey between bitter process artefact blind carbon truly provide dizzy crush flush breeze blouse charge solid fish spread" + + // TODO: Parameterize this from environment + let testRecipientAddress = "zs17mg40levjezevuhdp5pqrd52zere7r7vrjgdwn5sj4xsqtm20euwahv9anxmwr3y3kmwuz8k55a" + + let sendAmount = Zatoshi(1000) + 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") + let branchID = "2bb40e60" + let chainName = "main" + let network = DarksideWalletDNetwork() + + override func setUpWithError() throws { + try super.setUpWithError() + coordinator = try TestCoordinator( + seed: seedPhrase, + walletBirthday: birthday + 50, //don't use an exact birthday, users never do. + channelProvider: ChannelProvider(), + network: network + ) + try coordinator.reset(saplingActivation: 663150, branchID: self.branchID, chainName: self.chainName) + } + + override func tearDownWithError() throws { + try super.tearDownWithError() + NotificationCenter.default.removeObserver(self) + try coordinator.stop() + try? FileManager.default.removeItem(at: coordinator.databases.cacheDB) + try? FileManager.default.removeItem(at: coordinator.databases.dataDB) + try? FileManager.default.removeItem(at: coordinator.databases.pendingDB) + } + + @objc func handleReorg(_ notification: Notification) { + guard + let reorgHeight = notification.userInfo?[CompactBlockProcessorNotificationKey.reorgHeight] as? BlockHeight, + let rewindHeight = notification.userInfo?[CompactBlockProcessorNotificationKey.rewindHeight] as? BlockHeight + else { + XCTFail("empty reorg notification") + return + } + + logger!.debug("--- REORG DETECTED \(reorgHeight)--- RewindHeight: \(rewindHeight)", file: #file, function: #function, line: #line) + + XCTAssertEqual(reorgHeight, expectedReorgHeight) + reorgExpectation.fulfill() + } + + func testSynchronizerStops() throws { + hookToReOrgNotification() + + /* + 1. create fake chain + */ + let fullSyncLength = 100_000 + + try FakeChainBuilder.buildChain(darksideWallet: coordinator.service, branchID: branchID, chainName: chainName, length: fullSyncLength) + + try coordinator.applyStaged(blockheight: birthday + fullSyncLength) + + sleep(10) + + let syncStoppedExpectation = XCTestExpectation(description: "SynchronizerStopped Expectation") + syncStoppedExpectation.subscribe(to: .synchronizerStopped, object: nil) + + let processorStoppedExpectation = XCTestExpectation(description: "ProcessorStopped Expectation") + processorStoppedExpectation.subscribe(to: .blockProcessorStopped, object: nil) + + /* + sync to latest height + */ + 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)") + }) + + DispatchQueue.main.asyncAfter(deadline: .now() + 5) { + self.coordinator.synchronizer.stop() + } + + wait(for: [processorStoppedExpectation,syncStoppedExpectation], timeout: 6, enforceOrder: true) + + XCTAssertEqual(coordinator.synchronizer.status, .stopped) + XCTAssertEqual(coordinator.synchronizer.blockProcessor.state, .stopped) + } + + func handleError(_ error: Error?) { + _ = try? coordinator.stop() + guard let testError = error else { + XCTFail("failed with nil error") + return + } + XCTFail("Failed with error: \(testError)") + } + + func hookToReOrgNotification() { + NotificationCenter.default.addObserver(self, selector: #selector(handleReorg(_:)), name: .blockProcessorHandledReOrg, object: nil) + + } +} From 044d7d99d5b1f5d63c4f068b4f2b897ba5461b0e Mon Sep 17 00:00:00 2001 From: Francisco Gindre Date: Fri, 16 Sep 2022 17:41:47 -0300 Subject: [PATCH 2/3] Bump version and add new checkpoints --- .../checkpoints/mainnet/1807500.json | 8 +++++++ .../checkpoints/mainnet/1810000.json | 8 +++++++ ZcashLightClientKit.podspec | 2 +- changelog.md | 22 +++++++++++++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 Sources/ZcashLightClientKit/Resources/checkpoints/mainnet/1807500.json create mode 100644 Sources/ZcashLightClientKit/Resources/checkpoints/mainnet/1810000.json diff --git a/Sources/ZcashLightClientKit/Resources/checkpoints/mainnet/1807500.json b/Sources/ZcashLightClientKit/Resources/checkpoints/mainnet/1807500.json new file mode 100644 index 00000000..902d00ec --- /dev/null +++ b/Sources/ZcashLightClientKit/Resources/checkpoints/mainnet/1807500.json @@ -0,0 +1,8 @@ +{ + "network": "main", + "height": "1807500", + "hash": "0000000000b92234ffe6efd360a2fb1528e2f5cf891a54e58a9b83cd6c513ad9", + "time": 1663095814, + "saplingTree": "0146b9cd1dc80b44c19108d53ecf72b5bacd405e8ed5c8038c413ad2d7bf9f486201fb61737c80f0c3816ddc487ff9ba360c60b1663151cbd07c08dd8c09c648760d190177aa27c08a1ff4754710984f2f94c5a101a10f57eb6ad49f7b1df47f7e8a5730014c0fb4e36c1b7968082d0355f318a67e265e5a5533ffcc01b939e4bf1779841b000000019315fe44f132446949cd9ea1f34ec20f32c0dd0ba4f732e563c040c692fd5d49010e6b6ae8956bd722b0f5e3f355adfc7c9b6a39542c07fb0bcd317d0eff8d99080001382db97fabb726a88e7a7baab050a1c9169d2142a9ce0b7f3022349acf74981a0175d44cdd04e213c2d95fb281f96fc7d734c65be70a77b7aa4fef8a4b6fb2475b015fe654f11132fc9c133b46dbbf19b0113cc45715f52dfd3282ede76f6880740c01d14f83f0fd7d09f4f52c8ae9d39c63b57c59a37666d211b9a2deb290808c02720001701df279d9a2270a82379486df546fafeaeb831993cde1cd9e5c9cb17be5191f01cc6ae86e9147b0b1c3f5fb32fb7acc012b4cb4384a1a1331ae3c2324a804483e00017fb2e2890b05355ba797af2f77e38cab3e8ec1623d29a912bdd0dc4a78cf554a0001de17a599d0c6d73eaf1a5939e95af4427f75b89f703749e00d853cce3d6af84f00014f6313837ed19d2b480ec531529fb6b425006f2c1d981077640be21627659410018c2d6adea2ad4faf20eccfc2c2a2c59192fb53d3204b3a2757f1c247dadec16b0001c5d9822e7aa76d3a758e8dc25ffd8b8c9d4051c66fb92fd7aa263905e238920e0139af7ec003ac4e37c263bab5642fcc4ec294a1e328ff025832eec45239041f6e", + "orchardTree": "013de3a4ad28b5df77a979fc925643e69b7dd26ba787a3122fcd6a445c47d9280e01b68287983d323c90e6a1ed5980ab5b1a846d49340ee6d40d2349795c132a382c1f0001c2db91c5b9baa1c09623429cb4005ae12e521a018eb8df2d051d6793a307eb3e01c6b2f8d93635310d470d4a6d011ea77f59e28bee6ffca3df88f5f2a98980331a01777860ffe739b8047045e2dff8ba77070666075214a0f7702568205410351b39019ed7c5c3e958cb8b9d5324c290ff384dc3ca6cbc870950002f64398478ff1904000114b5ad56c8f210854a1688f47116b5d272fea09559646cee33ad3e6958306a15000001110e689714d772b170f63bfbf4b144c6a48e0a57c4a2780513633d5c799a0d1300010cf94eaf4d5268d9e0878064a458eaa3363dfcfcf2602681c443baf989d5de20000000019776dec2ea06cc5ecd2d212d37023972f526cb2ffa7ce1e8cf8eb4ef04700b01000001c7146e487b3ae97b190ebf93eac554968e683d31115d13fe83dd620859d9a92d000001020d51b05be2e276efa73f2977f5a82defed553b1086897f0bea9b03f4dbd41a000000000000000000" +} diff --git a/Sources/ZcashLightClientKit/Resources/checkpoints/mainnet/1810000.json b/Sources/ZcashLightClientKit/Resources/checkpoints/mainnet/1810000.json new file mode 100644 index 00000000..e1edd90b --- /dev/null +++ b/Sources/ZcashLightClientKit/Resources/checkpoints/mainnet/1810000.json @@ -0,0 +1,8 @@ +{ + "network": "main", + "height": "1810000", + "hash": "0000000000bdf01be068ff1f0b21b8b266f839b31ed066b72888b67f672c9800", + "time": 1663283618, + "saplingTree": "019dbc466ad114f2b4a6d0d91198c0a2c5c20c1dec7c2e7f932c9f9197d76b80020019015b272118101cac0ee6b9b8cf26d5104ab42913d2f5253388bac28e7998f2c41b0001868d5008c587f0fa3d26fc42097d34df49a70508a85a6acccf1263d39cb62c28000001b2325a6ecbf023f3ce4b74c1c14bac8d9c462560dce9611bd7b08cd55ecf4b0100000001cd62e4e2142c664656f956fd0ea8d1de1027c327580398fe8912e6264801650201ccc9b305f6e65d641a4e461ea5e853354a3ac4b2acf2591849e00421bd58fb48000000013a591632f71e1fe214ab46b464112520e98f15d171da599a350f7d8dba79595d018d254447626cf40828102a60e2b433d05498a780599cdf56a14f3888c2f42008000148af2e64d92d1944a451180f1738c9f468f608525b0273967db19029b53ba16d0000000001f416eb7e062c981dbbf76f8845fda959b948bc742fc62d9edb2f36bae852ba4e01c5d9822e7aa76d3a758e8dc25ffd8b8c9d4051c66fb92fd7aa263905e238920e0139af7ec003ac4e37c263bab5642fcc4ec294a1e328ff025832eec45239041f6e", + "orchardTree": "0180c286284cd52360af960fce56fe7dc339667c35580c75e9bc339483230e932701260b8984058125b5e3558fb65172d59718427111cbc06891017cc7633f74e1001f00000000000001cde58ba8e982a34499406e06a763ede11353d39ab93a5af1b34905cf268c033701226087d6a9bb28c2ed6890b989224791093cd5012a27285040025c0a72b2ce06000001ba71f8a1897b754e9fe37a29eb2c1a93ddc1678298498b4a84da732ebf056f15018073f4aff677a24eaac68c20d271ea228041f1e77710b0504d3f6c0b71d63d24000146b37a3e6167ae7f07725ab4e32247619c37e2a91c87182dd68b8feb99d5a22201e18dad85447ef2e9b8d647c9b9f1e6cef3e1d03f908975cd5e1d5c5808e443010001898b4a8f384f342a67efb3f6c4afd87310df4ff1532b86ca8d1394975aab5a1e0001c7146e487b3ae97b190ebf93eac554968e683d31115d13fe83dd620859d9a92d000001020d51b05be2e276efa73f2977f5a82defed553b1086897f0bea9b03f4dbd41a000000000000000000" +} diff --git a/ZcashLightClientKit.podspec b/ZcashLightClientKit.podspec index 9f9c6728..6b23ca6c 100644 --- a/ZcashLightClientKit.podspec +++ b/ZcashLightClientKit.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = 'ZcashLightClientKit' - s.version = '0.16.9-beta' + s.version = '0.16.10-beta' s.summary = 'Zcash Light Client wallet SDK for iOS' s.description = <<-DESC diff --git a/changelog.md b/changelog.md index 935b6aec..eacab72e 100644 --- a/changelog.md +++ b/changelog.md @@ -1,3 +1,25 @@ +# 0.16.10-beta +- [#532] [0.16.x-beta] Download does not stop correctly + +Issue Reported: + +When the synchronizer is stopped, the processor does not cancel +the download correctly. Then when attempting to resume sync, the +synchronizer is not on `.stopped` and can't be resumed + +this doesn't appear to happen in `master` branch that uses +structured concurrency for operations. + +Fix: +This commit makes sure that the download streamer checks cancelation +before processing any block, or getting called back to report progress + +Checkpoints added: +Mainnet +```` +Sources/ZcashLightClientKit/Resources/checkpoints/mainnet/1807500.json +Sources/ZcashLightClientKit/Resources/checkpoints/mainnet/1810000.json +```` # 0.16.9-beta Checkpoints added: Mainnet From d8ee9ff277e0cb3a9b4ded398b069f98081e6268 Mon Sep 17 00:00:00 2001 From: Francisco Gindre Date: Fri, 16 Sep 2022 18:08:31 -0300 Subject: [PATCH 3/3] - [#532] Download does not stop correctly Issue Reported on [0.16.x-beta] When the synchronizer is stopped, the processor does not cancel the download correctly. Then when attempting to resume sync, the synchronizer is not on `.stopped` and can't be resumed Fix: This commit makes sure that the download streamer checks cancelation before processing any block, or getting called back to report progress --- .../Block/Processor/CompactBlockDownload.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/ZcashLightClientKit/Block/Processor/CompactBlockDownload.swift b/Sources/ZcashLightClientKit/Block/Processor/CompactBlockDownload.swift index 58eefb02..db6cd260 100644 --- a/Sources/ZcashLightClientKit/Block/Processor/CompactBlockDownload.swift +++ b/Sources/ZcashLightClientKit/Block/Processor/CompactBlockDownload.swift @@ -29,6 +29,7 @@ extension CompactBlockProcessor { guard let latestHeight = targetHeightInternal else { throw LightWalletServiceError.generalError(message: "missing target height on compactBlockStreamDownload") } + try Task.checkCancellation() let latestDownloaded = try await storage.latestHeightAsync() let startHeight = max(startHeight ?? BlockHeight.empty(), latestDownloaded) @@ -38,6 +39,7 @@ extension CompactBlockProcessor { ) for try await zcashCompactBlock in stream { + try Task.checkCancellation() buffer.append(zcashCompactBlock) if buffer.count >= blockBufferSize { // TODO: writeAsync doesn't make sense here, awaiting it or calling blocking API have the same result and impact