Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Data/Collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 8 additions & 4 deletions src/Data/Cycles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion src/Data/Data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {
Expand Down
20 changes: 18 additions & 2 deletions src/checkpoint/CycleData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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')
Expand Down
7 changes: 4 additions & 3 deletions src/dbstore/cycles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(', ')})`
Expand Down Expand Up @@ -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(', ')
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions src/dbstore/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ export const initializeDB = async (config: Config): Promise<void> => {
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<void> => {
Expand Down
2 changes: 2 additions & 0 deletions src/dbstore/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
53 changes: 53 additions & 0 deletions test/unit/src/Data/Collector.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
41 changes: 40 additions & 1 deletion test/unit/src/Data/Cycles.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ describe('Data/Cycles', () => {
sig: 'signature',
},
},

lostAfterSelection: [],
}

Expand Down Expand Up @@ -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() }))
})

Expand All @@ -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', () => {
Expand Down
54 changes: 54 additions & 0 deletions test/unit/src/dbstore/cycles.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/unit/src/dbstore/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading