From 96520aeb7c8183a3a4830aa64c24c98b11b939ef Mon Sep 17 00:00:00 2001 From: Francisco Gindre Date: Fri, 29 Jul 2022 10:07:08 -0300 Subject: [PATCH] [#440] Split constants for Download Batches and Scanning Batches (#441) this commit splits the batch sizes so that wallets can be tweaked to either scan or download more or less blocks depending of the CompactBlockProcessor.Config used. Defaults are provided This also bumps up the default time out for GRPC services to 30 seconds to unary calls and 100 seconds to streaming calls Also, adds some documentation formatting that won't hurt PR Suggestions PR Suggestions --- .../Processor/CompactBlockProcessor.swift | 64 +++++++++---------- .../CompactBlockScanningOperation.swift | 2 +- .../Constants/ZcashSDK.swift | 30 ++++----- Sources/ZcashLightClientKit/Initializer.swift | 6 +- .../Service/LightWalletGRPCService.swift | 15 ++++- .../CompactBlockProcessorTests.swift | 2 +- 6 files changed, 59 insertions(+), 60 deletions(-) diff --git a/Sources/ZcashLightClientKit/Block/Processor/CompactBlockProcessor.swift b/Sources/ZcashLightClientKit/Block/Processor/CompactBlockProcessor.swift index 78104571..3a84b9eb 100644 --- a/Sources/ZcashLightClientKit/Block/Processor/CompactBlockProcessor.swift +++ b/Sources/ZcashLightClientKit/Block/Processor/CompactBlockProcessor.swift @@ -38,8 +38,6 @@ check Notification.Name extensions for more details. */ public enum CompactBlockProcessorNotificationKey { public static let progress = "CompactBlockProcessorNotificationKey.progress" - // public static let progressStartHeight = "CompactBlockProcessorNotificationKey.progressStartHeight" - // public static let progressTargetHeight = "CompactBlockProcessorNotificationKey.progressTargetHeight" public static let progressBlockTime = "CompactBlockProcessorNotificationKey.progressBlockTime" public static let reorgHeight = "CompactBlockProcessorNotificationKey.reorgHeight" public static let latestScannedBlockHeight = "CompactBlockProcessorNotificationKey.latestScannedBlockHeight" @@ -211,21 +209,20 @@ public extension Notification.Name { static let blockProcessorConnectivityStateChanged = Notification.Name("CompactBlockProcessorConnectivityStateChanged") } -/** -The compact block processor is in charge of orchestrating the download and caching of compact blocks from a LightWalletEndpoint -when started the processor downloads does a download - validate - scan cycle until it reaches latest height on the blockchain. -*/ + +/// The compact block processor is in charge of orchestrating the download and caching of compact blocks from a LightWalletEndpoint +/// when started the processor downloads does a download - validate - scan cycle until it reaches latest height on the blockchain. public class CompactBlockProcessor { - /** - Compact Block Processor configuration - - Property: cacheDbPath absolute file path of the DB where raw, unprocessed compact blocks are stored. - Property: dataDbPath absolute file path of the DB where all information derived from the cache DB is stored. - */ + + /// Compact Block Processor configuration + /// + /// Property: cacheDbPath absolute file path of the DB where raw, unprocessed compact blocks are stored. + /// Property: dataDbPath absolute file path of the DB where all information derived from the cache DB is stored. public struct Configuration { public var cacheDb: URL public var dataDb: URL - public var downloadBatchSize = ZcashSDK.DefaultBatchSize + public var downloadBatchSize = ZcashSDK.DefaultDownloadBatch + public var scanningBatchSize = ZcashSDK.DefaultScanningBatch public var retries = ZcashSDK.defaultRetries public var maxBackoffInterval = ZcashSDK.defaultMaxBackOffInterval public var rewindDistance = ZcashSDK.defaultRewindDistance @@ -366,12 +363,13 @@ public class CompactBlockProcessor { return queue }() - /** - Initializes a CompactBlockProcessor instance - - Parameters: - - downloader: an instance that complies to CompactBlockDownloading protocol - - backend: a class that complies to ZcashRustBackendWelding - */ + + /// Initializes a CompactBlockProcessor instance + /// - Parameters: + /// - service: concrete implementation of `LightWalletService` protocol + /// - storage: concrete implementation of `CompactBlockStorage` protocol + /// - backend: a class that complies to `ZcashRustBackendWelding` + /// - config: `Configuration` struct for this processor convenience init( service: LightWalletService, storage: CompactBlockStorage, @@ -389,12 +387,10 @@ public class CompactBlockProcessor { accountRepository: AccountRepositoryBuilder.build(dataDbURL: config.dataDb, readOnly: true) ) } - - /** - Initializes a CompactBlockProcessor instance from an Initialized object - - Parameters: - - initializer: an instance that complies to CompactBlockDownloading protocol - */ + + /// Initializes a CompactBlockProcessor instance from an Initialized object + /// - Parameters: + /// - initializer: an instance that complies to CompactBlockDownloading protocol public convenience init(initializer: Initializer) { self.init( service: initializer.lightWalletService, @@ -477,17 +473,12 @@ public class CompactBlockProcessor { return lowerBound ... upperBound } - /** - Starts the CompactBlockProcessor instance and starts downloading and processing blocks - - triggers the blockProcessorStartedDownloading notification - - - Important: subscribe to the notifications before calling this method - - */ + /// Starts the CompactBlockProcessor instance and starts downloading and processing blocks + /// + /// triggers the blockProcessorStartedDownloading notification + /// + /// - Important: subscribe to the notifications before calling this method public func start(retry: Bool = false) throws { - // TODO: check if this validation makes sense at all - // try validateConfiguration() if retry { self.retryAttempts = 0 self.processingError = nil @@ -637,6 +628,8 @@ public class CompactBlockProcessor { storage: self.storage, startHeight: range.lowerBound, targetHeight: range.upperBound, + batchSize: self.config.downloadBatchSize, + maxRetries: self.config.retries, progressDelegate: self ) @@ -714,6 +707,7 @@ public class CompactBlockProcessor { dataDb: config.dataDb, transactionRepository: transactionRepository, range: range, + batchSize: UInt32(self.config.scanningBatchSize), networkType: self.config.network.networkType, progressDelegate: self ) diff --git a/Sources/ZcashLightClientKit/Block/Processor/CompactBlockScanningOperation.swift b/Sources/ZcashLightClientKit/Block/Processor/CompactBlockScanningOperation.swift index 442d8640..1ba32318 100644 --- a/Sources/ZcashLightClientKit/Block/Processor/CompactBlockScanningOperation.swift +++ b/Sources/ZcashLightClientKit/Block/Processor/CompactBlockScanningOperation.swift @@ -134,7 +134,7 @@ class CompactBlockBatchScanningOperation: ZcashOperation { dataDb: URL, transactionRepository: TransactionRepository, range: CompactBlockRange, - batchSize: UInt32 = 100, + batchSize: UInt32, networkType: NetworkType, progressDelegate: CompactBlockProgressDelegate? = nil ) { diff --git a/Sources/ZcashLightClientKit/Constants/ZcashSDK.swift b/Sources/ZcashLightClientKit/Constants/ZcashSDK.swift index 92ab9818..970f95c7 100644 --- a/Sources/ZcashLightClientKit/Constants/ZcashSDK.swift +++ b/Sources/ZcashLightClientKit/Constants/ZcashSDK.swift @@ -63,31 +63,36 @@ public enum ZcashSDK { /// The theoretical maximum number of blocks in a reorg, due to other bottlenecks in the protocol design. public static var maxReorgSize = 100 - /// The amount of blocks ahead of the current height where new transactions are set to expire. This value is controlled /// by the rust backend but it is helpful to know what it is set to and should be kept in sync. public static var expiryOffset = 20 - // mark: Defaults - /// Default size of batches of blocks to request from the compact block service. + /// Default size of batches of blocks to request from the compact block service. Which was used both for scanning and downloading. + /// consider basing your code assumptions on `DefaultDownloadBatch` and `DefaultScanningBatch` instead. + @available(*, deprecated, message: "this value is being deprecated in favor of `DefaultDownloadBatch` and `DefaultScanningBatch`") public static var DefaultBatchSize = 100 + /// Default batch size for downloading blocks for the compact block processor. This value was changed due to + /// full blocks causing block download memory usage significantly and also timeouts on grpc calls. + /// this value is subject to change in the future. + public static var DefaultDownloadBatch = 10 + + /// Default batch size for scanning blocks for the compact block processor + public static var DefaultScanningBatch = 100 + /// Default amount of time, in in seconds, to poll for new blocks. Typically, this should be about half the average /// block time. public static var defaultPollInterval: TimeInterval = 20 - /// Default attempts at retrying. public static var defaultRetries: Int = 5 - /// The default maximum amount of time to wait during retry backoff intervals. Failed loops will never wait longer than /// this before retyring. public static var defaultMaxBackOffInterval: TimeInterval = 600 - /// Default number of blocks to rewind when a chain reorg is detected. This should be large enough to recover from the /// reorg but smaller than the theoretical max reorg size of 100. public static var defaultRewindDistance: Int = 10 @@ -95,7 +100,6 @@ public enum ZcashSDK { /// The number of blocks to allow before considering our data to be stale. This usually helps with what to do when /// returning from the background and is exposed via the Synchronizer's isStale function. public static var defaultStaleTolerance: Int = 10 - /// Default Name for LibRustZcash data.db public static var defaultDataDbName = "data.db" @@ -105,7 +109,6 @@ public enum ZcashSDK { /// Default name for pending transactions db public static var defaultPendingDbName = "pending.db" - /// File name for the sapling spend params public static var spendParamFilename = "sapling-spend.params" @@ -113,7 +116,6 @@ public enum ZcashSDK { /// File name for the sapling output params public static var outputParamFilename = "sapling-output.params" - /// The Url that is used by default in zcashd. /// We'll want to make this externally configurable, rather than baking it into the SDK but /// this will do for now, since we're using a cloudfront URL that already redirects. @@ -121,11 +123,9 @@ public enum ZcashSDK { } public protocol NetworkConstants { - /// The height of the first sapling block. When it comes to shielded transactions, we do not need to consider any blocks /// prior to this height, at all. static var saplingActivationHeight: BlockHeight { get } - /// Default Name for LibRustZcash data.db static var defaultDataDbName: String { get } @@ -133,13 +133,11 @@ public protocol NetworkConstants { /// Default Name for Compact Block caches db static var defaultCacheDbName: String { get } - /// Default name for pending transactions db static var defaultPendingDbName: String { get } /// Default prefix for db filenames static var defaultDbNamePrefix: String { get } - /// fixed height where the SDK considers that the ZIP-321 was deployed. This is a workaround /// for librustzcash not figuring out the tx fee from the tx itself. @@ -169,7 +167,6 @@ public extension NetworkConstants { public class ZcashSDKMainnetConstants: NetworkConstants { private init() {} - /// The height of the first sapling block. When it comes to shielded transactions, we do not need to consider any blocks /// prior to this height, at all. @@ -191,26 +188,21 @@ public class ZcashSDKMainnetConstants: NetworkConstants { public class ZcashSDKTestnetConstants: NetworkConstants { private init() {} - /// The height of the first sapling block. When it comes to shielded transactions, we do not need to consider any blocks /// prior to this height, at all. public static var saplingActivationHeight: BlockHeight = 280_000 - /// Default Name for LibRustZcash data.db public static var defaultDataDbName = "data.db" - /// Default Name for Compact Block caches db public static var defaultCacheDbName = "caches.db" - /// Default name for pending transactions db public static var defaultPendingDbName = "pending.db" public static var defaultDbNamePrefix = "ZcashSdk_testnet_" - /// Estimated height where wallets are supposed to change the fee public static var feeChangeHeight: BlockHeight = 1_028_500 diff --git a/Sources/ZcashLightClientKit/Initializer.swift b/Sources/ZcashLightClientKit/Initializer.swift index fb1d0f46..8e04a21a 100644 --- a/Sources/ZcashLightClientKit/Initializer.swift +++ b/Sources/ZcashLightClientKit/Initializer.swift @@ -36,14 +36,14 @@ public struct LightWalletEndpoint { - address: a String containing the host address - port: string with the port of the host address - secure: true if connecting through TLS. Default value is true - - singleCallTimeoutInMillis: timeout for single calls in Milliseconds - - streamingCallTimeoutInMillis: timeout for streaming calls in Milliseconds + - singleCallTimeoutInMillis: timeout for single calls in Milliseconds. Default 30 seconds + - streamingCallTimeoutInMillis: timeout for streaming calls in Milliseconds. Default 100 seconds */ public init( address: String, port: Int, secure: Bool = true, - singleCallTimeoutInMillis: Int64 = 10000, + singleCallTimeoutInMillis: Int64 = 30000, streamingCallTimeoutInMillis: Int64 = 100000 ) { self.host = address diff --git a/Sources/ZcashLightClientKit/Service/LightWalletGRPCService.swift b/Sources/ZcashLightClientKit/Service/LightWalletGRPCService.swift index 70ff36f1..b44babad 100644 --- a/Sources/ZcashLightClientKit/Service/LightWalletGRPCService.swift +++ b/Sources/ZcashLightClientKit/Service/LightWalletGRPCService.swift @@ -106,7 +106,20 @@ public class LightWalletGRPCService { ) } - public init(host: String, port: Int = 9067, secure: Bool = true, singleCallTimeout: Int64 = 10000, streamingCallTimeout: Int64 = 10000) { + /// Inits a connection to a Lightwalletd service to the given + /// - Parameters: + /// - host: the hostname of the lightwalletd server + /// - port: port of the server. Default is 9067 + /// - secure: whether this server is TLS or plaintext. default True (TLS) + /// - singleCallTimeout: Timeout for unary calls in milliseconds. + /// - streamingCallTimeout: Timeout for streaming calls in milliseconds. + public init( + host: String, + port: Int = 9067, + secure: Bool = true, + singleCallTimeout: Int64, + streamingCallTimeout: Int64 + ) { self.connectionManager = ConnectionStatusManager() self.queue = DispatchQueue.init(label: "LightWalletGRPCService") self.streamingCallTimeout = TimeLimit.timeout(.milliseconds(streamingCallTimeout)) diff --git a/Tests/NetworkTests/CompactBlockProcessorTests.swift b/Tests/NetworkTests/CompactBlockProcessorTests.swift index 3a85bc97..8e273b53 100644 --- a/Tests/NetworkTests/CompactBlockProcessorTests.swift +++ b/Tests/NetworkTests/CompactBlockProcessorTests.swift @@ -158,7 +158,7 @@ class CompactBlockProcessorTests: XCTestCase { ) // Test mid-range - latestDownloadedHeight = BlockHeight(network.constants.saplingActivationHeight + ZcashSDK.DefaultBatchSize) + latestDownloadedHeight = BlockHeight(network.constants.saplingActivationHeight + ZcashSDK.DefaultDownloadBatch) latestBlockchainHeight = BlockHeight(network.constants.saplingActivationHeight + 1000) expectedBatchRange = CompactBlockRange(uncheckedBounds: (lower: latestDownloadedHeight + 1, upper: latestBlockchainHeight))