SHARD-2660: save certificates in cycles table

src/Data/Collector.ts

@@ -5,7 +5,7 @@ import * as Receipt from '../dbstore/receipts'
 import * as OriginalTxsData from '../dbstore/originalTxsData'
 import * as ProcessedTransaction from '../dbstore/processedTxs'
 import * as Crypto from '../Crypto'
-import { clearCombinedAccountsData, combineAccountsData, collectCycleData } from './Data'
+import { clearCombinedAccountsData, combineAccountsData, collectCycleData, subscriptionCycleData } from './Data'
 import { config } from '../Config'
 import * as Logger from '../Logger'
 import { nestedCountersInstance } from '../profiler/nestedCounters'
@@ -1200,6 +1200,7 @@ export const storeCycleData = async (cycles: P2PTypes.CycleCreatorTypes.CycleDat
       counter: cycleRecord.counter,
       cycleMarker: cycleRecord.marker,
       cycleRecord,
+      certificates: (cycleRecord as unknown as subscriptionCycleData).certificates,
     }
     if (config.dataLogWrite && CycleLogWriter) CycleLogWriter.writeToLog(`${StringUtils.safeStringify(cycleObj)}\n`)
     const cycleExist = await queryCycleByMarker(cycleObj.cycleMarker)

src/Data/Cycles.ts

@@ -188,7 +188,11 @@ export const validateCycleData = (
 ): boolean => {
   Logger.mainLogger.debug('validateCycleData: Validating cycle record', UtilsTypes.safeStringify(cycleRecord))
 
-  const err = Utils.validateTypes(cycleRecord, {
+  const cycleRecordCopy = { ...cycleRecord }
+  // remove certificates from the cycle record
+  delete (cycleRecordCopy as subscriptionCycleData).certificates
+
+  const err = Utils.validateTypes(cycleRecordCopy, {
     activated: 'a',
     activatedPublicKeys: 'a',
     active: 'n',
@@ -232,10 +236,10 @@ export const validateCycleData = (
     return false
   }
 
-  const cycleRecordWithoutMarker = { ...cycleRecord }
-  delete cycleRecordWithoutMarker.marker
+  // remove marker from the cycle record
+  delete cycleRecordCopy.marker
 
-  const computedMarker = computeCycleMarker(cycleRecordWithoutMarker)
+  const computedMarker = computeCycleMarker(cycleRecordCopy)
   Logger.mainLogger.debug(
     'validateCycleData: Computed marker: ',
     computedMarker,

src/Data/Data.ts

@@ -554,7 +554,10 @@ export function collectCycleData(
     Logger.mainLogger.debug(
       `collectCycleData: Received ${receivedCertSigners.length} certificate signers for cycle ${cycle.counter}`
     )
-    delete (cycle as subscriptionCycleData).certificates
+
+    // Don't delete certificates from the cycle record here, we need to store them
+    // validateCycleData will remove the certificates from the cycle record for its validations
+    // delete (cycle as subscriptionCycleData).certificates
 
     if (receivedCycleTracker[cycle.counter]) {
       if (receivedCycleTracker[cycle.counter][cycle.marker]) {

src/checkpoint/CycleData.ts

@@ -14,10 +14,24 @@ import { Utils as StringUtils } from '@shardeum-foundation/lib-types'
 import * as Logger from '../Logger'
 import { insertCycle } from '../dbstore/cycles'
 
+/**
+ * Strip non-consensus fields from the cycle
+ * @param cycle - The cycle to strip non-consensus fields from
+ * @returns The cycle with non-consensus fields stripped
+ */
+function stripNonConsensusFields(cycle: Cycle): Cycle {
+  // Shallow copy the receipt
+  const clone: any = { ...cycle }
+  delete clone.certificates
+
+  return clone
+}
+
 //Represents a single piece of cycle data
 export class CycleCheckpointData extends CheckpointData<Cycle> {
   constructor(cycle: Cycle) {
-    const cycleHash = Crypto.hash(StringUtils.safeStringify(cycle)).toLowerCase()
+    // strip non-consensus fields from the cycle record
+    const cycleHash = Crypto.hash(StringUtils.safeStringify(stripNonConsensusFields(cycle))).toLowerCase()
 
     super(
       cycleHash.substring(0, 2), // address (first 2 chars)
@@ -99,7 +113,9 @@ export class CycleRadixDigestTally extends RadixDigestTally {
 
 // Define the validateData function
 async function validateData(data: CheckpointData<Cycle>): Promise<boolean> {
-  const cycle = data.d
+  // strip non-consensus fields from the cycle record
+  const cycle = stripNonConsensusFields(data.d)
+
   // Basic validation checks
   if (!cycle || cycle.counter === undefined || !cycle.cycleMarker || !cycle.cycleRecord) {
     Logger.mainLogger.error('Missing required cycle fields')

src/dbstore/cycles.ts

@@ -11,7 +11,7 @@ import { bulkUpdateCheckpointStatusField, CheckpointStatusType } from './checkpo
 export async function insertCycle(cycle: Cycle, storeCheckpoints: boolean = true): Promise<void> {
   try {
     // Define the table columns based on schema
-    const columns = ['cycleMarker', 'counter', 'cycleRecord']
+    const columns = ['cycleMarker', 'counter', 'cycleRecord', 'certificates']
 
     // Construct the SQL query with placeholders
     const placeholders = `(${columns.map(() => '?').join(', ')})`
@@ -59,7 +59,7 @@ export async function bulkInsertCycles(cycles: Cycle[], storeCheckpoints: boolea
       }
     }
     // Then do the database operation
-    const columns = ['cycleMarker', 'counter', 'cycleRecord']
+    const columns = ['cycleMarker', 'counter', 'cycleRecord', 'certificates']
 
     // Construct the SQL query for bulk insertion with all placeholders
     const placeholders = cycles.map(() => `(${columns.map(() => '?').join(', ')})`).join(', ')
@@ -107,10 +107,11 @@ export async function updateCycle(marker: string, cycle: Cycle, storeCheckpoints
       const bucketID = calculateBucketID(cycle)
       cycleCheckpointManager.addData(checkpointData, bucketID)
     }
-    const sql = `UPDATE cycles SET counter = $counter, cycleRecord = $cycleRecord WHERE cycleMarker = $marker `
+    const sql = `UPDATE cycles SET counter = $counter, cycleRecord = $cycleRecord, certificates = $certificates WHERE cycleMarker = $marker `
     await db.run(cycleDatabase, sql, {
       $counter: cycle.counter,
       $cycleRecord: cycle.cycleRecord && SerializeToJsonString(cycle.cycleRecord),
+      $certificates: cycle.certificates && SerializeToJsonString(cycle.certificates),
       $marker: marker,
     })
     if (config.VERBOSE) {

src/dbstore/index.ts

@@ -114,6 +114,12 @@ export const initializeDB = async (config: Config): Promise<void> => {
     checkpointStatusDatabase,
     'CREATE INDEX if not exists `checkpoint_status_unified_status` ON `checkpoint_status` (`cycle`)'
   )
+
+  try {
+    await runCreate(cycleDatabase, 'ALTER TABLE `cycles` ADD COLUMN `certificates` JSON')
+  } catch (e: any) {
+    if (!/duplicate column name/i.test(e.message)) throw e
+  }
 }
 
 export const closeDatabase = async (): Promise<void> => {

src/dbstore/types.ts

@@ -3,8 +3,10 @@ export interface Cycle {
   counter: P2P.CycleCreatorTypes.CycleData['counter']
   cycleRecord: P2P.CycleCreatorTypes.CycleData
   cycleMarker: StateManager.StateMetaDataTypes.CycleMarker
+  certificates?: P2P.CycleCreatorTypes.CycleCert[]
 }
 
 export type DbCycle = Cycle & {
   cycleRecord: string
+  certificates: string
 }

test/unit/src/Data/Collector.test.ts

@@ -234,6 +234,59 @@ describe('Collector Module', () => {
 
       expect(cycles.bulkInsertCycles).not.toHaveBeenCalled()
     })
+
+    it('should store cycle data with certificates', async () => {
+      const mockCertificates = [
+        {
+          marker: 'test-marker',
+          score: 100,
+          sign: {
+            owner: 'node-pk-1',
+            sig: 'signature-1',
+          },
+        },
+      ]
+      const cycleData = {
+        counter: 1,
+        marker: 'test-marker',
+        certificates: mockCertificates,
+      } as any
+
+      ;(cycles.queryCycleByMarker as any).mockResolvedValue(null)
+      ;(cycles.bulkInsertCycles as any).mockResolvedValue(undefined)
+
+      await Collector.storeCycleData([cycleData])
+
+      expect(cycles.bulkInsertCycles).toHaveBeenCalledWith([
+        {
+          counter: 1,
+          cycleMarker: 'test-marker',
+          cycleRecord: cycleData,
+          certificates: mockCertificates,
+        },
+      ])
+    })
+
+    it('should store cycle data without certificates', async () => {
+      const cycleData = {
+        counter: 1,
+        marker: 'test-marker',
+      } as any
+
+      ;(cycles.queryCycleByMarker as any).mockResolvedValue(null)
+      ;(cycles.bulkInsertCycles as any).mockResolvedValue(undefined)
+
+      await Collector.storeCycleData([cycleData])
+
+      expect(cycles.bulkInsertCycles).toHaveBeenCalledWith([
+        {
+          counter: 1,
+          cycleMarker: 'test-marker',
+          cycleRecord: cycleData,
+          certificates: undefined,
+        },
+      ])
+    })
   })
 
   describe('storeAccountData', () => {

test/unit/src/Data/Cycles.test.ts

@@ -277,6 +277,7 @@ describe('Data/Cycles', () => {
         sig: 'signature',
       },
     },
+
     lostAfterSelection: [],
   }
 
@@ -445,7 +446,9 @@ describe('Data/Cycles', () => {
       const result = Cycles.validateCycleData(mockCycle)
 
       expect(result).toBe(true)
-      expect(Utils.validateTypes).toHaveBeenCalledWith(mockCycle, expect.any(Object))
+      // The validateCycleData function creates a copy and removes marker but keeps certificate
+      const { marker, ...expectedCycleCopy } = mockCycle
+      expect(Utils.validateTypes).toHaveBeenCalledWith(expectedCycleCopy, expect.any(Object))
       expect(Crypto.hashObj).toHaveBeenCalledWith(expect.not.objectContaining({ marker: expect.anything() }))
     })
 
@@ -468,6 +471,42 @@ describe('Data/Cycles', () => {
         'Invalid Cycle Record: cycle marker does not match with the computed marker'
       )
     })
+
+    it('should validate cycle with certificate', () => {
+      const cycleWithCert = { ...mockCycle, certificate: mockCycle.certificate }
+      const result = Cycles.validateCycleData(cycleWithCert)
+
+      expect(result).toBe(true)
+      // Verify certificate is preserved during validation (business logic only removes certificates plural)
+      const { marker, ...expectedCycleCopy } = cycleWithCert
+      expect(Utils.validateTypes).toHaveBeenCalledWith(expectedCycleCopy, expect.any(Object))
+      expect(Crypto.hashObj).toHaveBeenCalledWith(expect.not.objectContaining({ marker: expect.anything() }))
+    })
+
+    it('should validate cycle without certificate', () => {
+      const { certificate, ...cycleWithoutCert } = mockCycle
+      const result = Cycles.validateCycleData(cycleWithoutCert as any)
+
+      expect(result).toBe(true)
+      // Verify validation works without certificate
+      const { marker, ...expectedCycleCopy } = cycleWithoutCert
+      expect(Utils.validateTypes).toHaveBeenCalledWith(expectedCycleCopy, expect.any(Object))
+      expect(Crypto.hashObj).toHaveBeenCalledWith(expect.not.objectContaining({ marker: expect.anything() }))
+    })
+
+    it('should preserve certificate during validation process', () => {
+      const cycleWithCert = { ...mockCycle, certificate: mockCycle.certificate }
+      const originalCertificate = { ...cycleWithCert.certificate }
+
+      const result = Cycles.validateCycleData(cycleWithCert)
+
+      expect(result).toBe(true)
+      // Verify certificate is preserved in the original object
+      expect(cycleWithCert.certificate).toEqual(originalCertificate)
+      // Verify validation was called with a copy that preserves certificate (business logic only removes certificates plural)
+      const { marker, ...expectedCycleCopy } = cycleWithCert
+      expect(Utils.validateTypes).toHaveBeenCalledWith(expectedCycleCopy, expect.any(Object))
+    })
   })
 
   describe('computeCycleMarker', () => {

test/unit/src/dbstore/cycles.test.ts

@@ -225,6 +225,60 @@ describe('Cycles Module', () => {
       // Verify that error was logged
       expect(require('../../../../src/Logger').mainLogger.error).toHaveBeenCalled()
     })
+
+    it('should insert cycle with certificates', async () => {
+      // Setup
+      const mockCertificates = [
+        {
+          marker: 'sample-marker-123',
+          score: 100,
+          sign: {
+            owner: 'node-pk-1',
+            sig: 'signature-1',
+          },
+        },
+      ]
+      const cycleWithCerts = {
+        ...sampleCycle,
+        certificates: mockCertificates,
+      }
+
+      // Execute
+      await cyclesModule.insertCycle(cycleWithCerts)
+
+      // Verify
+      expect(db.run).toHaveBeenCalledWith(
+        cycleDatabase,
+        expect.stringContaining('INSERT OR REPLACE INTO cycles (cycleMarker, counter, cycleRecord, certificates)'),
+        expect.arrayContaining([
+          'sample-marker-123',
+          123,
+          expect.any(String), // serialized cycleRecord
+          expect.any(String), // serialized certificates
+        ])
+      )
+    })
+
+    it('should insert cycle without certificates', async () => {
+      // Setup
+      const cycleWithoutCerts = { ...sampleCycle }
+      delete cycleWithoutCerts.certificates
+
+      // Execute
+      await cyclesModule.insertCycle(cycleWithoutCerts)
+
+      // Verify
+      expect(db.run).toHaveBeenCalledWith(
+        cycleDatabase,
+        expect.stringContaining('INSERT OR REPLACE INTO cycles (cycleMarker, counter, cycleRecord, certificates)'),
+        expect.arrayContaining([
+          'sample-marker-123',
+          123,
+          expect.any(String), // serialized cycleRecord
+          undefined, // certificates is undefined when not present
+        ])
+      )
+    })
   })
 
   // Tests for bulkInsertCycles

test/unit/src/dbstore/index.test.ts

@@ -154,7 +154,7 @@ describe('dbstore/index', () => {
       })
 
       it('should run create table and index statements for all databases', () => {
-        expect(mockRunCreate).toHaveBeenCalledTimes(24)
+        expect(mockRunCreate).toHaveBeenCalledTimes(25)
 
         // Spot check table and index creation calls
         expect(mockRunCreate).toHaveBeenCalledWith(