diff --git a/src/Data/Collector.ts b/src/Data/Collector.ts index 22b86b74..44d697ea 100644 --- a/src/Data/Collector.ts +++ b/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) diff --git a/src/Data/Cycles.ts b/src/Data/Cycles.ts index fc495328..ac87081f 100644 --- a/src/Data/Cycles.ts +++ b/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, diff --git a/src/Data/Data.ts b/src/Data/Data.ts index e09d55ca..18a325f1 100644 --- a/src/Data/Data.ts +++ b/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]) { diff --git a/src/checkpoint/CycleData.ts b/src/checkpoint/CycleData.ts index f7714397..4a335bfa 100644 --- a/src/checkpoint/CycleData.ts +++ b/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 { 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): Promise { - 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') diff --git a/src/dbstore/cycles.ts b/src/dbstore/cycles.ts index aaec1b43..3381abf8 100644 --- a/src/dbstore/cycles.ts +++ b/src/dbstore/cycles.ts @@ -11,7 +11,7 @@ import { bulkUpdateCheckpointStatusField, CheckpointStatusType } from './checkpo export async function insertCycle(cycle: Cycle, storeCheckpoints: boolean = true): Promise { 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) { diff --git a/src/dbstore/index.ts b/src/dbstore/index.ts index 5324841e..b6ad9a8d 100644 --- a/src/dbstore/index.ts +++ b/src/dbstore/index.ts @@ -114,6 +114,8 @@ export const initializeDB = async (config: Config): Promise => { checkpointStatusDatabase, 'CREATE INDEX if not exists `checkpoint_status_unified_status` ON `checkpoint_status` (`cycle`)' ) + + await runCreate(cycleDatabase, 'ALTER TABLE cycles ADD COLUMN IF NOT EXISTS certificates JSON') } export const closeDatabase = async (): Promise => { diff --git a/src/dbstore/types.ts b/src/dbstore/types.ts index 0598b573..914a7045 100644 --- a/src/dbstore/types.ts +++ b/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 } diff --git a/test/unit/src/Data/Collector.test.ts b/test/unit/src/Data/Collector.test.ts index 1f4f7e8a..6661270f 100644 --- a/test/unit/src/Data/Collector.test.ts +++ b/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', () => { diff --git a/test/unit/src/Data/Cycles.test.ts b/test/unit/src/Data/Cycles.test.ts index 28e94b1c..ce3b48ae 100644 --- a/test/unit/src/Data/Cycles.test.ts +++ b/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', () => { diff --git a/test/unit/src/dbstore/cycles.test.ts b/test/unit/src/dbstore/cycles.test.ts index be453904..f6468d6a 100644 --- a/test/unit/src/dbstore/cycles.test.ts +++ b/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 diff --git a/test/unit/src/dbstore/index.test.ts b/test/unit/src/dbstore/index.test.ts index b6065f60..d7479e1e 100644 --- a/test/unit/src/dbstore/index.test.ts +++ b/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(