zec
/
ZcashLightClientKit
mirror of https://github.com/zcash/ZcashLightClientKit.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

[#700] Get rid of ScanDownloadedButUnscannedAction 

Before the state machine download and scan was called in one loop. And
processing range for one batch was same for both of them. Therefore
there was code which scanned downloaded but not scanned blocks.

But now download and scan are independent. So it is possible to remove
`ScanDownloadedButUnscannedAction`.
This commit is contained in:
Michal Fousek 2023-05-22 09:23:32 +02:00
parent 2541fe5a8a
commit 09fca40287
17 changed files with 59 additions and 120 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Sources/ZcashLightClientKit/Block/Actions/Action.swift
View File

@ -28,7 +28,6 @@ enum CBPState: CaseIterable {
case validateServer
case computeSyncRanges
case checksBeforeSync
case scanDownloaded
case download
case validate
case scan

6
Sources/ZcashLightClientKit/Block/Actions/ChecksBeforeSyncAction.swift
View File

@ -25,11 +25,9 @@ class ChecksBeforeSyncAction {
/// exiting the app.
/// - Returns: an ``Optional<BlockHeight>`` where Some represents what's the
/// new state the internal state should reflect and indicating that the cache should be cleared
/// as well. c`None` means that no action is required.
/// as well. `nil` means that no action is required.
func shouldClearBlockCacheAndUpdateInternalState(syncRange: SyncRanges) -> BlockHeight? {
guard syncRange.downloadedButUnscannedRange != nil else {
return nil
}
guard syncRange.downloadRange != nil, syncRange.scanRange != nil else { return nil }
guard
let latestScannedHeight = syncRange.latestScannedHeight,

11
Sources/ZcashLightClientKit/Block/Actions/ComputeSyncRangesAction.swift
View File

@ -25,15 +25,15 @@ class ComputeSyncRangesAction {
/// It may happen that sync process start with syncing blocks that were downloaded but not synced in previous run of the sync process. This
/// methods analyses what must be done and computes range that should be used to compute reported progress.
private func computeTotalProgressRange(from syncRanges: SyncRanges) -> CompactBlockRange {
guard syncRanges.downloadedButUnscannedRange != nil || syncRanges.downloadAndScanRange != nil else {
guard syncRanges.downloadRange != nil || syncRanges.scanRange != nil else {
// In this case we are sure that no downloading or scanning happens so this returned range won't be even used. And it's easier to return
// this "fake" range than to handle nil.
return 0...0
}
// Thanks to guard above we can be sure that one of these two ranges is not nil.
let lowerBound = syncRanges.downloadedButUnscannedRange?.lowerBound ?? syncRanges.downloadAndScanRange?.lowerBound ?? 0
let upperBound = syncRanges.downloadAndScanRange?.upperBound ?? syncRanges.downloadedButUnscannedRange?.upperBound ?? 0
let lowerBound = syncRanges.scanRange?.lowerBound ?? syncRanges.downloadRange?.lowerBound ?? 0
let upperBound = syncRanges.scanRange?.upperBound ?? syncRanges.downloadRange?.upperBound ?? 0
return lowerBound...upperBound
}
@ -70,9 +70,8 @@ extension ComputeSyncRangesAction: Action {
logger.debug("""
Syncing with ranges:
downloaded but not scanned: \
\(ranges.downloadedButUnscannedRange?.lowerBound ?? -1)...\(ranges.downloadedButUnscannedRange?.upperBound ?? -1)
download and scan: \(ranges.downloadAndScanRange?.lowerBound ?? -1)...\(ranges.downloadAndScanRange?.upperBound ?? -1)
download: \(ranges.downloadRange?.lowerBound ?? -1)...\(ranges.downloadRange?.upperBound ?? -1)
scan: \(ranges.scanRange?.lowerBound ?? -1)...\(ranges.scanRange?.upperBound ?? -1)
enhance range: \(ranges.enhanceRange?.lowerBound ?? -1)...\(ranges.enhanceRange?.upperBound ?? -1)
fetchUTXO range: \(ranges.fetchUTXORange?.lowerBound ?? -1)...\(ranges.fetchUTXORange?.upperBound ?? -1)
total progress range: \(totalProgressRange.lowerBound)...\(totalProgressRange.upperBound)

2
Sources/ZcashLightClientKit/Block/Actions/DownloadAction.swift
View File

@ -30,7 +30,7 @@ extension DownloadAction: Action {
var removeBlocksCacheWhenFailed: Bool { true }
func run(with context: ActionContext, didUpdate: @escaping (CompactBlockProcessor.Event) async -> Void) async throws -> ActionContext {
guard let downloadRange = await context.syncRanges.downloadAndScanRange else {
guard let downloadRange = await context.syncRanges.downloadRange else {
return await update(context: context)
}

4
Sources/ZcashLightClientKit/Block/Actions/EnhanceAction.swift
View File

@ -22,12 +22,12 @@ class EnhanceAction {
}
func decideWhatToDoNext(context: ActionContext, lastScannedHeight: BlockHeight) async -> ActionContext {
guard let downloadAndScanRange = await context.syncRanges.downloadAndScanRange else {
guard let scanRange = await context.syncRanges.scanRange else {
await context.update(state: .clearCache)
return context
}
if lastScannedHeight >= downloadAndScanRange.upperBound {
if lastScannedHeight >= scanRange.upperBound {
await context.update(state: .clearCache)
} else {
await context.update(state: .download)

2
Sources/ZcashLightClientKit/Block/Actions/SaplingParamsAction.swift
View File

@ -23,7 +23,7 @@ extension SaplingParamsAction: Action {
func run(with context: ActionContext, didUpdate: @escaping (CompactBlockProcessor.Event) async -> Void) async throws -> ActionContext {
logger.debug("Fetching sapling parameters")
try await saplingParametersHandler.handleIfNeeded()
await context.update(state: .scanDownloaded)
await context.update(state: .download)
return context
}

2
Sources/ZcashLightClientKit/Block/Actions/ScanAction.swift
View File

@ -30,7 +30,7 @@ extension ScanAction: Action {
var removeBlocksCacheWhenFailed: Bool { true }
func run(with context: ActionContext, didUpdate: @escaping (CompactBlockProcessor.Event) async -> Void) async throws -> ActionContext {
guard let scanRange = await context.syncRanges.downloadAndScanRange else {
guard let scanRange = await context.syncRanges.scanRange else {
return await update(context: context)
}

34
Sources/ZcashLightClientKit/Block/Actions/ScanDownloadedButUnscannedAction.swift
View File

@ -1,34 +0,0 @@
//
// ScandownloadedButUnscannedAction.swift
//
//
// Created by Michal Fousek on 05.05.2023.
//
import Foundation
class ScanDownloadedButUnscannedAction {
let logger: Logger
let blockScanner: BlockScanner
init(container: DIContainer) {
logger = container.resolve(Logger.self)
blockScanner = container.resolve(BlockScanner.self)
}
}
extension ScanDownloadedButUnscannedAction: Action {
var removeBlocksCacheWhenFailed: Bool { false }
func run(with context: ActionContext, didUpdate: @escaping (CompactBlockProcessor.Event) async -> Void) async throws -> ActionContext {
if let range = await context.syncRanges.downloadedButUnscannedRange {
logger.debug("Starting scan with downloaded but not scanned blocks with range: \(range.lowerBound)...\(range.upperBound)")
let totalProgressRange = await context.totalProgressRange
try await blockScanner.scanBlocks(at: range, totalProgressRange: totalProgressRange) { _ in }
}
await context.update(state: .download)
return context
}
func stop() async { }
}

4
Sources/ZcashLightClientKit/Block/CompactBlockProcessor.swift
View File

@ -215,8 +215,6 @@ actor CompactBlockProcessor {
action = ComputeSyncRangesAction(container: container, config: config)
case .checksBeforeSync:
action = ChecksBeforeSyncAction(container: container)
case .scanDownloaded:
action = ScanDownloadedButUnscannedAction(container: container)
case .download:
action = DownloadAction(container: container, config: config)
case .validate:
@ -560,8 +558,6 @@ extension CompactBlockProcessor {
break
case .checksBeforeSync:
break
case .scanDownloaded:
break
case .download:
break
case .validate:

40
Sources/ZcashLightClientKit/Block/Utils/InternalSyncProgress.swift
View File

@ -9,26 +9,23 @@ import Foundation
struct SyncRanges: Equatable {
let latestBlockHeight: BlockHeight
/// The sync process can be interrupted in any phase. It may happen that it's interrupted while downloading blocks. In that case in next sync
/// process already downloaded blocks needs to be scanned before the sync process starts to download new blocks. And the range of blocks that are
/// already downloaded but not scanned is stored in this variable.
let downloadedButUnscannedRange: CompactBlockRange?
/// Range of blocks that are not yet downloaded and not yet scanned.
let downloadAndScanRange: CompactBlockRange?
// Range of blocks that are not yet downloaded
let downloadRange: CompactBlockRange?
/// Range of blocks that are not yet scanned.
let scanRange: CompactBlockRange?
/// Range of blocks that are not enhanced yet.
let enhanceRange: CompactBlockRange?
/// Range of blocks for which no UTXOs are fetched yet.
let fetchUTXORange: CompactBlockRange?
let latestScannedHeight: BlockHeight?
let latestDownloadedBlockHeight: BlockHeight?
static var empty: SyncRanges {
SyncRanges(
latestBlockHeight: 0,
downloadedButUnscannedRange: nil,
downloadAndScanRange: nil,
downloadRange: nil,
scanRange: nil,
enhanceRange: nil,
fetchUTXORange: nil,
latestScannedHeight: nil,
@ -169,15 +166,6 @@ actor InternalSyncProgress {
latestBlockHeight: BlockHeight,
latestScannedHeight: BlockHeight
) -> SyncRanges {
// If there is more downloaded then scanned blocks we have to range for these blocks. The sync process will then start with scanning these
// blocks instead of downloading new ones.
let downloadedButUnscannedRange: CompactBlockRange?
if latestScannedHeight < latestDownloadedBlockHeight {
downloadedButUnscannedRange = latestScannedHeight + 1...latestDownloadedBlockHeight
} else {
downloadedButUnscannedRange = nil
}
if latestScannedHeight > latestDownloadedBlockHeight {
logger.warn("""
InternalSyncProgress found inconsistent state.
@ -186,24 +174,16 @@ actor InternalSyncProgress {
latestScannedHeight: \(latestScannedHeight)
latestEnhancedHeight: \(latestEnhancedHeight)
latestUTXOFetchedHeight: \(latestUTXOFetchedHeight)
latest downloaded height
""")
}
// compute the range that must be downloaded and scanned based on
// birthday, `latestDownloadedBlockHeight`, `latestScannedHeight` and
// latest block height fetched from the chain.
let downloadAndScanRange = computeRange(
latestHeight: max(latestDownloadedBlockHeight, latestScannedHeight),
birthday: birthday,
latestBlockHeight: latestBlockHeight
)
let downloadRange = computeRange(latestHeight: latestDownloadedBlockHeight, birthday: birthday, latestBlockHeight: latestBlockHeight)
let scanRange = computeRange(latestHeight: latestScannedHeight, birthday: birthday, latestBlockHeight: latestBlockHeight)
return SyncRanges(
latestBlockHeight: latestBlockHeight,
downloadedButUnscannedRange: downloadedButUnscannedRange,
downloadAndScanRange: downloadAndScanRange,
downloadRange: downloadRange,
scanRange: scanRange,
enhanceRange: computeRange(latestHeight: latestEnhancedHeight, birthday: birthday, latestBlockHeight: latestBlockHeight),
fetchUTXORange: computeRange(latestHeight: latestUTXOFetchedHeight, birthday: birthday, latestBlockHeight: latestBlockHeight),
latestScannedHeight: latestScannedHeight,

24
Tests/NetworkTests/CompactBlockProcessorTests.swift
View File

@ -197,8 +197,8 @@ class CompactBlockProcessorTests: ZcashTestCase {
var expectedSyncRanges = SyncRanges(
latestBlockHeight: latestBlockchainHeight,
downloadedButUnscannedRange: 1...latestDownloadedHeight,
downloadAndScanRange: latestDownloadedHeight...latestBlockchainHeight,
downloadRange: latestDownloadedHeight...latestBlockchainHeight,
scanRange: latestDownloadedHeight...latestBlockchainHeight,
enhanceRange: processorConfig.walletBirthday...latestBlockchainHeight,
fetchUTXORange: processorConfig.walletBirthday...latestBlockchainHeight,
latestScannedHeight: 0,
@ -230,8 +230,8 @@ class CompactBlockProcessorTests: ZcashTestCase {
expectedSyncRanges = SyncRanges(
latestBlockHeight: latestBlockchainHeight,
downloadedButUnscannedRange: 1...latestDownloadedHeight,
downloadAndScanRange: latestDownloadedHeight + 1...latestBlockchainHeight,
downloadRange: latestDownloadedHeight + 1...latestBlockchainHeight,
scanRange: latestDownloadedHeight + 1...latestBlockchainHeight,
enhanceRange: processorConfig.walletBirthday...latestBlockchainHeight,
fetchUTXORange: processorConfig.walletBirthday...latestBlockchainHeight,
latestScannedHeight: 0,
@ -264,8 +264,8 @@ class CompactBlockProcessorTests: ZcashTestCase {
expectedSyncRanges = SyncRanges(
latestBlockHeight: latestBlockchainHeight,
downloadedButUnscannedRange: 1...latestDownloadedHeight,
downloadAndScanRange: latestDownloadedHeight + 1...latestBlockchainHeight,
downloadRange: latestDownloadedHeight + 1...latestBlockchainHeight,
scanRange: latestDownloadedHeight + 1...latestBlockchainHeight,
enhanceRange: processorConfig.walletBirthday...latestBlockchainHeight,
fetchUTXORange: processorConfig.walletBirthday...latestBlockchainHeight,
latestScannedHeight: 0,
@ -303,8 +303,8 @@ class CompactBlockProcessorTests: ZcashTestCase {
let range = SyncRanges(
latestBlockHeight: 2255953,
downloadedButUnscannedRange: -1 ... -1,
downloadAndScanRange: 1493120...2255953,
downloadRange: 1493120...2255953,
scanRange: 1493120...2255953,
enhanceRange: 1410000...2255953,
fetchUTXORange: 1410000...2255953,
latestScannedHeight: 1493119,
@ -325,8 +325,8 @@ class CompactBlockProcessorTests: ZcashTestCase {
let range = SyncRanges(
latestBlockHeight: 2255953,
downloadedButUnscannedRange: -1 ... -1,
downloadAndScanRange: 1493120...2255953,
downloadRange: 1493120...2255953,
scanRange: 1493120...2255953,
enhanceRange: 1410000...2255953,
fetchUTXORange: 1410000...2255953,
latestScannedHeight: 1493129,
@ -347,8 +347,8 @@ class CompactBlockProcessorTests: ZcashTestCase {
let range = SyncRanges(
latestBlockHeight: 2255953,
downloadedButUnscannedRange: 1493120...1494120,
downloadAndScanRange: 1494121...2255953,
downloadRange: 1493120...2255953,
scanRange: 1493120...2255953,
enhanceRange: 1410000...2255953,
fetchUTXORange: 1410000...2255953,
latestScannedHeight: 1493119,

28
Tests/OfflineTests/CompactBlockProcessorActions/EnhanceActionTests.swift
View File

@ -77,7 +77,7 @@ final class EnhanceActionTests: ZcashTestCase {
let syncContext = await setupActionContext()
do {
let _ = try await enhanceAction.run(with: syncContext) { _ in }
_ = try await enhanceAction.run(with: syncContext) { _ in }
XCTAssertTrue(transactionRepositoryMock.lastScannedHeightCalled, "transactionRepository.lastScannedHeight() is expected to be called.")
XCTAssertFalse(blockEnhancerMock.enhanceAtDidEnhanceCalled, "blockEnhancer.enhance() is not expected to be called.")
XCTAssertFalse(internalSyncProgressStorageMock.integerForKeyCalled, "internalSyncProgress.load() is not expected to be called.")
@ -105,7 +105,7 @@ final class EnhanceActionTests: ZcashTestCase {
let syncContext = await setupActionContext()
do {
let _ = try await enhanceAction.run(with: syncContext) { _ in }
_ = try await enhanceAction.run(with: syncContext) { _ in }
XCTAssertTrue(transactionRepositoryMock.lastScannedHeightCalled, "transactionRepository.lastScannedHeight() is expected to be called.")
XCTAssertTrue(internalSyncProgressStorageMock.integerForKeyCalled, "internalSyncProgress.load() is expected to be called.")
XCTAssertFalse(blockEnhancerMock.enhanceAtDidEnhanceCalled, "blockEnhancer.enhance() is not expected to be called.")
@ -156,7 +156,7 @@ final class EnhanceActionTests: ZcashTestCase {
let syncContext = await setupActionContext()
do {
let _ = try await enhanceAction.run(with: syncContext) { event in
_ = try await enhanceAction.run(with: syncContext) { event in
guard case let .foundTransactions(transactions, _) = event else {
XCTFail("Event is expected to be .foundTransactions but received \(event)")
return
@ -204,12 +204,14 @@ final class EnhanceActionTests: ZcashTestCase {
)
blockEnhancerMock.enhanceAtDidEnhanceClosure = { _, didEnhance in
await didEnhance(EnhancementProgress(
totalTransactions: 0,
enhancedTransactions: 0,
lastFoundTransaction: transaction,
range: 0...0,
newlyMined: true)
await didEnhance(
EnhancementProgress(
totalTransactions: 0,
enhancedTransactions: 0,
lastFoundTransaction: transaction,
range: 0...0,
newlyMined: true
)
)
return nil
}
@ -225,7 +227,7 @@ final class EnhanceActionTests: ZcashTestCase {
let syncContext = await setupActionContext()
do {
let _ = try await enhanceAction.run(with: syncContext) { event in
_ = try await enhanceAction.run(with: syncContext) { event in
guard case .minedTransaction(let minedTransaction) = event else {
XCTFail("Event is expected to be .minedTransaction but received \(event)")
return
@ -245,8 +247,8 @@ final class EnhanceActionTests: ZcashTestCase {
let syncRanges = SyncRanges(
latestBlockHeight: 0,
downloadedButUnscannedRange: nil,
downloadAndScanRange: underlyingDownloadAndScanRange,
downloadRange: underlyingDownloadAndScanRange,
scanRange: underlyingDownloadAndScanRange,
enhanceRange: underlyingEnhanceRange,
fetchUTXORange: nil,
latestScannedHeight: nil,
@ -265,7 +267,7 @@ final class EnhanceActionTests: ZcashTestCase {
_ internalSyncProgressStorageMock: InternalSyncProgressStorageMock = InternalSyncProgressStorageMock(),
_ loggerMock: LoggerMock = LoggerMock()
) -> EnhanceAction {
mockContainer.register(type: InternalSyncProgress.self, isSingleton: true) { di in
mockContainer.register(type: InternalSyncProgress.self, isSingleton: true) { _ in
InternalSyncProgress(alias: .default, storage: internalSyncProgressStorageMock, logger: loggerMock)
}

4
Tests/OfflineTests/CompactBlockProcessorActions/FetchUTXOsActionTests.swift
View File

@ -28,8 +28,8 @@ final class FetchUTXOsActionTests: ZcashTestCase {
let syncRanges = SyncRanges(
latestBlockHeight: 0,
downloadedButUnscannedRange: nil,
downloadAndScanRange: nil,
downloadRange: nil,
scanRange: nil,
enhanceRange: nil,
fetchUTXORange: CompactBlockRange(uncheckedBounds: (1000, 2000)),
latestScannedHeight: nil,

9
Tests/OfflineTests/CompactBlockProcessorActions/ScanActionTests.swift
View File

@ -55,7 +55,10 @@ final class ScanActionTests: ZcashTestCase {
do {
_ = try await scanAction.run(with: syncContext) { _ in }
XCTAssertFalse(transactionRepositoryMock.lastScannedHeightCalled, "transactionRepository.lastScannedHeight() is not expected to be called.")
XCTAssertFalse(
transactionRepositoryMock.lastScannedHeightCalled,
"transactionRepository.lastScannedHeight() is not expected to be called."
)
XCTAssertFalse(loggerMock.debugFileFunctionLineCalled, "logger.debug(...) is not expected to be called.")
XCTAssertFalse(blockScannerMock.scanBlocksAtTotalProgressRangeDidScanCalled, "blockScanner.scanBlocks(...) is not expected to be called.")
} catch {
@ -107,8 +110,8 @@ final class ScanActionTests: ZcashTestCase {
let syncRanges = SyncRanges(
latestBlockHeight: 0,
downloadedButUnscannedRange: nil,
downloadAndScanRange: CompactBlockRange(uncheckedBounds: (1000, 2000)),
downloadRange: CompactBlockRange(uncheckedBounds: (1000, 2000)),
scanRange: CompactBlockRange(uncheckedBounds: (1000, 2000)),
enhanceRange: nil,
fetchUTXORange: nil,
latestScannedHeight: nil,

1
Tests/TestUtils/Sourcery/AutoMockable.swift
View File

@ -24,4 +24,5 @@ extension Synchronizer { }
extension TransactionRepository { }
extension UTXOFetcher { }
extension ZcashRustBackendWelding { }
// sourcery:end:

7
docs/cbp_state_machine.puml
View File

@ -41,15 +41,10 @@ fetchUTXO -[#red]-> failed : Error occured.
fetchUTXO -[#blue]-> stopped : Sync was stopped.
handleSaplingParams : SaplingParamsAction
handleSaplingParams -[#green,bold]-> scanDownloaded
handleSaplingParams -[#green,bold]-> download
handleSaplingParams -[#red]-> failed : Error occured.
handleSaplingParams -[#blue]-> stopped : Sync was stopped.
scanDownloaded : ScanDownloadedButUnscannedAction
scanDownloaded -[#green,bold]-> download
scanDownloaded -[#red]-> failed : Error occured.
scanDownloaded -[#blue]-> stopped : Sync was stopped.
download : DownloadAction
download -[#green,bold]-> validate
download -[#red]-> failed : Error occured.

BIN
docs/images/cbp_state_machine.png
View File

Binary file not shown.
Side by Side Swipe Overlay

Before

Width:  |  Height:  |  Size: 278 KiB

After

Width:  |  Height:  |  Size: 253 KiB