Skip to content

Commit b9ebad1

Browse files
committed
refactor: static code analysis checks
Signed-off-by: Rob Walworth <[email protected]>
1 parent 869ef64 commit b9ebad1

File tree

2 files changed

+116
-60
lines changed

2 files changed

+116
-60
lines changed

Sources/Hiero/Client/NetworkUpdateTask.swift

Lines changed: 65 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import NIOCore
2323
/// - `ConsensusNetwork` - The network being updated
2424
/// - `MirrorNetwork` - Source of address book data
2525
/// - `NodeAddressBookQuery` - Fetches address book from network
26-
internal actor NetworkUpdateTask: Sendable {
26+
internal actor NetworkUpdateTask {
2727
// MARK: - Constants
2828

2929
/// Delay before the first network update after initialization
@@ -71,13 +71,15 @@ internal actor NetworkUpdateTask: Sendable {
7171

7272
if let updatePeriod {
7373
updateTask = Self.makeTask(
74-
eventLoop,
75-
consensusNetwork,
76-
mirrorNetwork,
77-
Self.networkFirstUpdateDelay,
78-
updatePeriod,
79-
shard,
80-
realm
74+
config: UpdateConfig(
75+
eventLoop: eventLoop,
76+
consensusNetwork: consensusNetwork,
77+
mirrorNetwork: mirrorNetwork,
78+
startDelay: Self.networkFirstUpdateDelay,
79+
updatePeriod: updatePeriod,
80+
shard: shard,
81+
realm: realm
82+
)
8183
)
8284
}
8385
}
@@ -94,25 +96,63 @@ internal actor NetworkUpdateTask: Sendable {
9496
self.updateTask?.cancel()
9597

9698
if let updatePeriod = duration {
97-
self.updateTask = Self.makeTask(eventLoop, consensusNetwork, mirrorNetwork, nil, updatePeriod, shard, realm)
99+
self.updateTask = Self.makeTask(
100+
config: UpdateConfig(
101+
eventLoop: eventLoop,
102+
consensusNetwork: consensusNetwork,
103+
mirrorNetwork: mirrorNetwork,
104+
startDelay: nil,
105+
updatePeriod: updatePeriod,
106+
shard: shard,
107+
realm: realm
108+
)
109+
)
98110
}
99111
}
100112

113+
// MARK: - Private Types
114+
115+
/// Configuration parameters for creating a network update task.
116+
///
117+
/// This struct groups all parameters needed to configure and run the periodic
118+
/// network update loop, avoiding the need for functions with many parameters.
119+
private struct UpdateConfig {
120+
/// Event loop for network operations
121+
internal let eventLoop: NIOCore.EventLoopGroup
122+
123+
/// Atomic reference to the consensus network being updated
124+
internal let consensusNetwork: ManagedAtomic<ConsensusNetwork>
125+
126+
/// Atomic reference to the mirror network for queries
127+
internal let mirrorNetwork: ManagedAtomic<MirrorNetwork>
128+
129+
/// Optional delay before the first update (nil for immediate start)
130+
internal let startDelay: Duration?
131+
132+
/// Interval between updates in nanoseconds
133+
internal let updatePeriod: UInt64
134+
135+
/// Shard number for address book file ID
136+
internal let shard: UInt64
137+
138+
/// Realm number for address book file ID
139+
internal let realm: UInt64
140+
}
141+
101142
// MARK: - Private Methods
102143

103144
/// Creates the background task that performs periodic network updates.
104-
private static func makeTask(
105-
_ eventLoop: NIOCore.EventLoopGroup,
106-
_ consensusNetwork: ManagedAtomic<ConsensusNetwork>,
107-
_ mirrorNetwork: ManagedAtomic<MirrorNetwork>,
108-
_ startDelay: Duration?,
109-
_ updatePeriod: UInt64,
110-
_ shard: UInt64,
111-
_ realm: UInt64
112-
) -> Task<(), Error> {
145+
///
146+
/// The task runs an infinite loop that fetches the network address book from the
147+
/// mirror network and atomically updates the consensus network. Errors are logged
148+
/// but don't terminate the loop.
149+
///
150+
/// - Parameter config: Configuration containing all necessary parameters for updates
151+
/// - Returns: A Task that can be canceled to stop the update loop
152+
private static func makeTask(config: UpdateConfig) -> Task<(), Error> {
113153
Task {
114154
// Initial delay before first update
115-
if let startDelay {
155+
if let startDelay = config.startDelay {
116156
let delayNanos = startDelay.seconds * Self.nanosecondsPerSecond
117157
try await Task.sleep(nanoseconds: delayNanos)
118158
}
@@ -123,14 +163,14 @@ internal actor NetworkUpdateTask: Sendable {
123163

124164
do {
125165
// Fetch address book from mirror network
126-
let mirror = mirrorNetwork.load(ordering: .relaxed)
166+
let mirror = config.mirrorNetwork.load(ordering: .relaxed)
127167
let addressBook = try await NodeAddressBookQuery(
128-
FileId.getAddressBookFileIdFor(shard: shard, realm: realm)
168+
FileId.getAddressBookFileIdFor(shard: config.shard, realm: config.realm)
129169
).executeChannel(mirror.channel)
130170

131171
// Apply updates to consensus network atomically
132-
let newNetwork = consensusNetwork.readCopyUpdate {
133-
ConsensusNetwork.withAddressBook($0, eventLoop: eventLoop.next(), addressBook)
172+
let newNetwork = config.consensusNetwork.readCopyUpdate { old in
173+
ConsensusNetwork.withAddressBook(old, eventLoop: config.eventLoop.next(), addressBook)
134174
}
135175

136176
// Log successful update with structured format
@@ -152,8 +192,8 @@ internal actor NetworkUpdateTask: Sendable {
152192

153193
// Wait for the remainder of the update period
154194
let elapsed = (Timestamp.now - start).seconds * Self.nanosecondsPerSecond
155-
if elapsed < updatePeriod {
156-
try await Task.sleep(nanoseconds: updatePeriod - elapsed)
195+
if elapsed < config.updatePeriod {
196+
try await Task.sleep(nanoseconds: config.updatePeriod - elapsed)
157197
}
158198
}
159199
}

Sources/Hiero/Execute.swift

Lines changed: 51 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ private func executeAnyInner<E: Execute>(
289289
}
290290
}
291291

292-
// MARK: - Execution Result Type
292+
// MARK: - Helper Types
293293

294294
/// Result of attempting to execute a request on a node.
295295
private enum ExecutionResult<Response> {
@@ -306,6 +306,33 @@ private enum ExecutionResult<Response> {
306306
case regenerateTransactionAndRetry(HError)
307307
}
308308

309+
/// Parameters for pre-check status handling.
310+
///
311+
/// This struct groups all parameters needed for handling pre-check status responses,
312+
/// avoiding the need for functions with many parameters and improving code clarity.
313+
private struct PrecheckParameters<E: Execute> {
314+
/// The parsed pre-check status code from the response
315+
internal let status: Status
316+
317+
/// The full GRPC response from the node
318+
internal let response: E.GrpcResponse
319+
320+
/// Context data from request creation
321+
internal let context: E.Context
322+
323+
/// Account ID of the node that processed the request
324+
internal let nodeAccountId: AccountId
325+
326+
/// Transaction ID used for the request (if applicable)
327+
internal let transactionId: TransactionId?
328+
329+
/// The transaction or query being executed
330+
internal let executable: E
331+
332+
/// Execution context with network and backoff configuration
333+
internal let ctx: ExecuteContext
334+
}
335+
309336
// MARK: - Transaction ID Management
310337

311338
/// Generates the initial transaction ID for a request.
@@ -392,13 +419,15 @@ private func executeOnNode<E: Execute>(
392419
let precheckStatus = Status(rawValue: rawPrecheckStatus)
393420

394421
return try handlePrecheckStatus(
395-
precheckStatus,
396-
response: response,
397-
context: context,
398-
nodeAccountId: nodeAccountId,
399-
transactionId: transactionId,
400-
executable: executable,
401-
ctx: ctx
422+
params: PrecheckParameters(
423+
status: precheckStatus,
424+
response: response,
425+
context: context,
426+
nodeAccountId: nodeAccountId,
427+
transactionId: transactionId,
428+
executable: executable,
429+
ctx: ctx
430+
)
402431
)
403432
}
404433

@@ -443,50 +472,37 @@ private func handleGrpcError<Response>(
443472
/// - Custom retry statuses: Retry with backoff
444473
/// - Everything else: Throw error
445474
///
446-
/// - Parameters:
447-
/// - status: Parsed pre-check status
448-
/// - response: GRPC response containing the status
449-
/// - context: Context data from request creation
450-
/// - nodeAccountId: Account ID of the node
451-
/// - transactionId: Transaction ID used for the request
452-
/// - executable: The transaction or query being executed
453-
/// - ctx: Execution context
475+
/// - Parameter params: Parameters containing status, response, context, and execution state
454476
/// - Returns: Execution result indicating success or retry strategy
455477
/// - Throws: HError for unrecoverable status codes
456-
private func handlePrecheckStatus<E: Execute>(
457-
_ status: Status,
458-
response: E.GrpcResponse,
459-
context: E.Context,
460-
nodeAccountId: AccountId,
461-
transactionId: TransactionId?,
462-
executable: E,
463-
ctx: ExecuteContext
464-
) throws -> ExecutionResult<E.Response> {
465-
switch status {
466-
case .ok where executable.shouldRetry(forResponse: response):
467-
return .retryWithBackoff(executable.makeErrorPrecheck(status, transactionId))
478+
private func handlePrecheckStatus<E: Execute>(params: PrecheckParameters<E>) throws -> ExecutionResult<E.Response> {
479+
switch params.status {
480+
case .ok where params.executable.shouldRetry(forResponse: params.response):
481+
return .retryWithBackoff(params.executable.makeErrorPrecheck(params.status, params.transactionId))
468482

469483
case .ok:
470-
let response = try executable.makeResponse(response, context, nodeAccountId, transactionId)
484+
let response = try params.executable.makeResponse(
485+
params.response, params.context, params.nodeAccountId, params.transactionId)
471486
return .success(response)
472487

473488
case .busy, .platformNotActive:
474-
return .retryImmediately(executable.makeErrorPrecheck(status, transactionId))
489+
return .retryImmediately(params.executable.makeErrorPrecheck(params.status, params.transactionId))
475490

476-
case .transactionExpired where executable.explicitTransactionId == nil && ctx.operatorAccountId != nil:
477-
return .regenerateTransactionAndRetry(executable.makeErrorPrecheck(status, transactionId))
491+
case .transactionExpired
492+
where params.executable.explicitTransactionId == nil && params.ctx.operatorAccountId != nil:
493+
return .regenerateTransactionAndRetry(params.executable.makeErrorPrecheck(params.status, params.transactionId))
478494

479495
case .unrecognized(let value):
480496
throw HError(
481497
kind: .responseStatusUnrecognized,
482498
description: "response status \(value) unrecognized"
483499
)
484500

485-
case let status where executable.shouldRetryPrecheck(forStatus: status):
486-
return .retryWithBackoff(executable.makeErrorPrecheck(status, transactionId))
501+
case let status where params.executable.shouldRetryPrecheck(forStatus: status):
502+
return .retryWithBackoff(params.executable.makeErrorPrecheck(status, params.transactionId))
487503

488504
default:
489-
throw executable.makeErrorPrecheck(status, transactionId)
505+
throw params.executable.makeErrorPrecheck(params.status, params.transactionId)
490506
}
491507
}
492508

0 commit comments

Comments
 (0)