Improvements for cycle tracker

[jintukumardas]

PR Type

Enhancement, Bug fix

---

Description

• Refactored cycle tracker to use async/await for file and DB operations

• Improved fallback logic for missing cycle tracker file

• Added DB query for specific unified cycle skipping recent cycles

• Updated checkpoint sync logic to mark cycles unified after syncing

---

Changes walkthrough 📝

Relevant files

Enhancement

cycleTracker.ts Async refactor and fallback improvements for cycle tracker src/utils/cycleTracker.ts Refactored getLastUpdatedCycle and updateLastUpdatedCycle to async/await Enhanced fallback logic for missing tracker file using DB status Replaced unified cycle query logic with DB-level skip/offset Improved error handling and logging throughout +100/-54 checkpointStatus.ts Add DB unified cycle query and sync status update                src/dbstore/checkpointStatus.ts Added getSpecificUnifiedCycle for DB-level cycle skipping Updated processCyclesNeedingSync to mark cycles unified after syncing Improved error handling and logging +43/-1    Data.ts Use async cycle tracker in data sync functions                      src/Data/Data.ts Refactored all usages of getLastUpdatedCycle to async/await Improved cycle determination logic for syncing +3/-3      CheckpointV2.ts Use async cycle tracker in checkpoint sync                              src/checkpoint/CheckpointV2.ts Refactored to use async getLastUpdatedCycle Improved logging for checkpoint sync +1/-1      server.ts Use async cycle tracker in server startup and sync              src/server.ts Refactored to use async getLastUpdatedCycle in server startup and sync Improved state initialization logic +2/-2

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🏅 Score: 85

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Async File and DB Operations The refactor introduces async/await for file and database operations in cycle tracking logic. Ensure that all asynchronous flows are properly handled, especially error cases and race conditions when multiple processes might access or create the tracker file concurrently. export async function getLastUpdatedCycle(): Promise<number> { try { let data: string try { data = fs.readFileSync(CYCLE_TRACKER_FILE, 'utf8') } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ENOENT') { // If file doesn't exist, try to get the oldest pending or failed checkpoint status try { const oldestPendingOrFailedStatus = await getOldestPendingOrFailedCheckpointStatus() if (oldestPendingOrFailedStatus) { Logger.mainLogger.info(`No cycle tracker file found. Using oldest pending/failed checkpoint cycle: ${oldestPendingOrFailedStatus.cycle}`) // Create the tracker file with the oldest pending/failed cycle const trackerData: CycleTrackerData = { lastUpdatedCycle: oldestPendingOrFailedStatus.cycle, lastUpdatedTimestamp: Date.now(), } try { const fd = fs.openSync( CYCLE_TRACKER_FILE, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY, 0o600 ) fs.writeFileSync(fd, JSON.stringify(trackerData, null, 2), 'utf8') fs.closeSync(fd) return oldestPendingOrFailedStatus.cycle } catch (createError) { if ((createError as NodeJS.ErrnoException).code === 'EEXIST') { data = fs.readFileSync(CYCLE_TRACKER_FILE, 'utf8') } else { throw createError } } } else { // No pending/failed status found, create file with cycle 0 Logger.mainLogger.info('No cycle tracker file and no pending/failed checkpoint statuses found. Starting from cycle 0.') const trackerData: CycleTrackerData = { lastUpdatedCycle: 0, lastUpdatedTimestamp: 0, } try { const fd = fs.openSync( CYCLE_TRACKER_FILE, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY, 0o600 ) fs.writeFileSync(fd, JSON.stringify(trackerData, null, 2), 'utf8') fs.closeSync(fd) return 0 } catch (createError) { if ((createError as NodeJS.ErrnoException).code === 'EEXIST') { data = fs.readFileSync(CYCLE_TRACKER_FILE, 'utf8') } else { throw createError } } } } catch (statusError) { Logger.mainLogger.error('Error getting oldest pending/failed checkpoint status:', statusError) // Create file with cycle 0 as fallback const trackerData: CycleTrackerData = { lastUpdatedCycle: 0, lastUpdatedTimestamp: 0, } try { const fd = fs.openSync( CYCLE_TRACKER_FILE, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY, 0o600 ) fs.writeFileSync(fd, JSON.stringify(trackerData, null, 2), 'utf8') fs.closeSync(fd) return 0 } catch (createError) { if ((createError as NodeJS.ErrnoException).code === 'EEXIST') { data = fs.readFileSync(CYCLE_TRACKER_FILE, 'utf8') } else { throw createError } } } } else { throw error } } const trackerData: CycleTrackerData = JSON.parse(data) return trackerData.lastUpdatedCycle } catch (error) { Logger.mainLogger.error('Error reading cycle tracker file:', error) return 0 } } Database Query Robustness The new getSpecificUnifiedCycle function builds SQL queries dynamically and uses parameterized inputs. Validate that the query logic and parameter binding are secure and robust, and that edge cases (e.g., no results, large skipCount) are handled gracefully. export async function getSpecificUnifiedCycle(minCycle: number = 0, skipCount: number = 0): Promise<number | null> { try { // Build a query that gets the nth unified cycle in descending order const sql = ` SELECT cycle FROM checkpoint_status WHERE unifiedStatus = 1 ${minCycle > 0 ? 'AND cycle >= ?' : ''} ORDER BY cycle DESC LIMIT 1 OFFSET ? ` const params = [] if (minCycle > 0) { params.push(minCycle) } params.push(skipCount) const row = await db.get(checkpointStatusDatabase, sql, params) if (!row) { return null } const typedRow = row as { cycle: number } return typedRow.cycle } catch (error) { Logger.mainLogger.error(`Error getting specific unified cycle: ${error}`) throw error } } Side Effects in processCyclesNeedingSync The process now updates multiple checkpoint status fields after syncing each cycle. Confirm that these updates are atomic and that error handling ensures consistency if any update fails. // Update the checkpoint status to mark it as unified after syncing await updateCheckpointStatusField(cycle, CheckpointStatusType.CYCLE, true) await updateCheckpointStatusField(cycle, CheckpointStatusType.RECEIPT, true) await updateCheckpointStatusField(cycle, CheckpointStatusType.ORIGINAL_TX, true) Logger.mainLogger.debug(`[processCyclesNeedingSync] Updated cycle ${cycle} unifiedStatus to true`)

[github-actions]

Suggestion: Updating all checkpoint status fields to true immediately after the callback may cause data inconsistency if the callback fails or is only partially successful. Ensure that the callback completes successfully before marking statuses as unified, and consider handling errors from the callback to avoid marking failed cycles as complete. [possible issue, importance: 8]

Suggested change

	await callback(cycle)
	// Update the checkpoint status to mark it as unified after syncing
	await updateCheckpointStatusField(cycle, CheckpointStatusType.CYCLE, true)
	await updateCheckpointStatusField(cycle, CheckpointStatusType.RECEIPT, true)
	await updateCheckpointStatusField(cycle, CheckpointStatusType.ORIGINAL_TX, true)
	Logger.mainLogger.debug(`[processCyclesNeedingSync] Updated cycle ${cycle} unifiedStatus to true`)
	}
	try {
	await callback(cycle)
	// Update the checkpoint status to mark it as unified after successful sync
	await updateCheckpointStatusField(cycle, CheckpointStatusType.CYCLE, true)
	await updateCheckpointStatusField(cycle, CheckpointStatusType.RECEIPT, true)
	await updateCheckpointStatusField(cycle, CheckpointStatusType.ORIGINAL_TX, true)
	Logger.mainLogger.debug(`[processCyclesNeedingSync] Updated cycle ${cycle} unifiedStatus to true`)
	} catch (err) {
	Logger.mainLogger.error(`[processCyclesNeedingSync] Failed to sync cycle ${cycle}: ${err}`)
	// Do not update checkpoint status if callback fails
	}

[github-actions]

Suggestion: If cycle is less than or equal to lastUpdatedCycle, the function silently does nothing. This could mask logic errors or missed updates. Log a warning or throw an error when an attempt is made to update with a non-incremental cycle value to aid debugging and data integrity. [general, importance: 6]

New proposed code:

 export async function updateLastUpdatedCycle(cycle: number): Promise<void> {
   const lastUpdatedCycle = await getLastUpdatedCycle()
   if (cycle > lastUpdatedCycle) {
     try {
       const trackerData: CycleTrackerData = {
         lastUpdatedCycle: cycle,
+  ...
+  } else {
+    Logger.mainLogger.warn(`Attempted to update lastUpdatedCycle to ${cycle}, which is not greater than current value ${lastUpdatedCycle}`)
+  }

To fix the issue, we need to sanitize the cycle value before logging it. Since cycle is expected to be a numeric value, we can ensure it is properly sanitized by converting it to a string and removing any unexpected characters (e.g., newline or carriage return characters). This can be achieved using String.prototype.replace or similar methods.

The fix will involve modifying the log statement on line 514 in src/dbstore/checkpointStatus.ts to sanitize the cycle value before including it in the log message.

---