Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BLUE-271 fix: applyTimestamp added to processed tx #76

Merged
merged 1 commit into from
Sep 18, 2024
Merged
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
4 changes: 2 additions & 2 deletions src/Data/Collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
0,
5 * config.RECEIPT_CONFIRMATIONS
)
const isReceiptEqual = (receipt1: any, receipt2: any): boolean => {

Check warning on line 103 in src/Data/Collector.ts

View workflow job for this annotation

GitHub Actions / ci / QA merge checks

Unexpected any. Specify a different type

Check warning on line 103 in src/Data/Collector.ts

View workflow job for this annotation

GitHub Actions / ci / QA merge checks

Unexpected any. Specify a different type
if (!receipt1 || !receipt2) return false
const r1 = Utils.deepCopy(receipt1)
const r2 = Utils.deepCopy(receipt2)
Expand All @@ -125,7 +125,7 @@
true
)
if (config.VERBOSE) Logger.mainLogger.debug('robustQuery', receipt.tx.txId, robustQuery)
if (!robustQuery || !robustQuery.value || !(robustQuery.value as any).receipt) {

Check warning on line 128 in src/Data/Collector.ts

View workflow job for this annotation

GitHub Actions / ci / QA merge checks

Unexpected any. Specify a different type
Logger.mainLogger.error(
`❌ 'null' response from all nodes in receipt-validation for txId: ${receipt.tx.txId} , ${receipt.cycle}, ${receipt.tx.timestamp}
}`
Expand All @@ -135,7 +135,7 @@
return result
}

const robustQueryReceipt = (robustQuery.value as any).receipt as Receipt.SignedReceipt

Check warning on line 138 in src/Data/Collector.ts

View workflow job for this annotation

GitHub Actions / ci / QA merge checks

Unexpected any. Specify a different type

if (robustQuery.count < minConfirmations) {
// Wait for 500ms and try fetching the receipt from the nodes that did not respond in the robustQuery
Expand All @@ -158,7 +158,7 @@
}
const node = nodesToQuery[0]
nodesToQuery.splice(0, 1)
const receiptResult: any = await queryReceipt(node)

Check warning on line 161 in src/Data/Collector.ts

View workflow job for this annotation

GitHub Actions / ci / QA merge checks

Unexpected any. Specify a different type
if (!receiptResult || !receiptResult.receipt) continue
if (isReceiptEqual(robustQueryReceipt, receiptResult.receipt)) {
requiredConfirmations--
Expand Down Expand Up @@ -968,7 +968,7 @@
txId: tx.txId,
cycle: cycle,
txTimestamp: tx.timestamp,
txApplyTimestamp: null,
applyTimestamp,
}

// await Transaction.insertTransaction(txObj)
Expand Down Expand Up @@ -1157,7 +1157,7 @@
txId: receipt.data.txId || receipt.txId,
cycle: receipt.cycleNumber,
txTimestamp: receipt.timestamp,
txApplyTimestamp: null,
applyTimestamp: receipt.timestamp,
Copy link

@devendra-shardeum devendra-shardeum Sep 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jairajdev what's the difference b/w applyTimestamp and txTimestamp ? both are receiving the same value ..i.e. receipt.timestamp and also what's the reasoning behind renaming applyTimestamp from txApplyTimestamp? I hope it won't break the other flow! in archive-server

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

txTimestamp is the timestamp set before putting the tx in the tx queue
txApplyTimestamp is the timestamp when the tx is processed/consensused

but in this storeAccountData function which was used to restore the tx receipt from the first node in the network ( this feature is not used anymore ), since there is no way we can calculate the txApplyTimestamp, we have to set the txTimestamp.

Copy link

@devendra-shardeum devendra-shardeum Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but what's the need to have 2 fields receiving the same value.. if applyTimestamp value can't be correctly calculated and inserted, why shouldn't it be removed ? what's the purpose of having this extra field then ? @jairajdev

}
combineTransactions.push(txObj)
combineProcessedTxs.push(processedTx)
Expand Down
2 changes: 1 addition & 1 deletion src/dbstore/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export const initializeDB = async (config: Config): Promise<void> => {
// Transaction digester service tables
await runCreate(
processedTxDatabase,
'CREATE TABLE if not exists `processedTxs` (`txId` VARCHAR(128) NOT NULL, `cycle` BIGINT NOT NULL, `txTimestamp` BIGINT NOT NULL, `txApplyTimestamp` BIGINT, PRIMARY KEY (`txId`))'
'CREATE TABLE if not exists `processedTxs` (`txId` VARCHAR(128) NOT NULL, `cycle` BIGINT NOT NULL, `txTimestamp` BIGINT NOT NULL, `applyTimestamp` BIGINT NOT NULL, PRIMARY KEY (`txId`))'
)
await runCreate(
processedTxDatabase,
Expand Down
6 changes: 3 additions & 3 deletions src/dbstore/processedTxs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export interface ProcessedTransaction {
txId: string
cycle: number
txTimestamp: number
txApplyTimestamp: number
applyTimestamp: number
}

export async function insertProcessedTx(processedTx: ProcessedTransaction): Promise<void> {
Expand All @@ -26,7 +26,7 @@ export async function insertProcessedTx(processedTx: ProcessedTransaction): Prom
') ON CONFLICT (txId) DO UPDATE SET ' +
'cycle = excluded.cycle, ' +
'txTimestamp = excluded.txTimestamp, ' +
'txApplyTimestamp = excluded.txApplyTimestamp'
'applyTimestamp = excluded.applyTimestamp'

await db.run(processedTxDatabase, sql, values)
if (config.VERBOSE) {
Expand Down Expand Up @@ -55,7 +55,7 @@ export async function bulkInsertProcessedTxs(processedTxs: ProcessedTransaction[
' ON CONFLICT (txId) DO UPDATE SET ' +
'cycle = excluded.cycle, ' +
'txTimestamp = excluded.txTimestamp, ' +
'txApplyTimestamp = excluded.txApplyTimestamp'
'applyTimestamp = excluded.applyTimestamp'

await db.run(processedTxDatabase, sql, values)
if (config.VERBOSE)
Expand Down
Loading