[#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

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
+/// 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.
+/// 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
+    /// Compact Block Processor configuration
-     
+    ///
-    Property: cacheDbPath absolute file path of the DB where raw, unprocessed compact blocks are stored.
+    /// 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.
+    /// 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
+    /// Initializes a CompactBlockProcessor instance
-    - Parameters:
+    /// - Parameters:
-    - downloader: an instance that complies to CompactBlockDownloading protocol
+    ///  - service: concrete implementation of `LightWalletService` protocol
-    - backend: a class that complies to ZcashRustBackendWelding
+    ///  - 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
-    Initializes a CompactBlockProcessor instance from an Initialized object
+    /// - Parameters:
-    - Parameters:
+    ///     - initializer: an instance that complies to CompactBlockDownloading protocol
-        - 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
-    Starts the CompactBlockProcessor instance and starts downloading and processing blocks
+    ///
-
+    /// triggers the blockProcessorStartedDownloading notification
-    triggers the blockProcessorStartedDownloading notification
+    ///
-
+    /// - Important: subscribe to the notifications before calling this method
-    - 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
         )

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
     ) {

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

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
+        - singleCallTimeoutInMillis: timeout for single calls in Milliseconds. Default 30 seconds
-        - streamingCallTimeoutInMillis: timeout for streaming calls in Milliseconds
+        - 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

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))

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))