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-130 Global txreceipt verification #80

Merged
merged 5 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
"@fastify/rate-limit": "^7.6.0",
"@shardus/archiver-discovery": "^1.1.0",
"@shardus/crypto-utils": "4.1.4",
"@shardus/types": "1.2.21-1",
"@shardus/types": "1.2.21",
"deepmerge": "^4.2.2",
"fastify": "4.12.0",
"log4js": "^6.3.0",
Expand Down
3 changes: 3 additions & 0 deletions src/Config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ export interface Config {
usePOQo: boolean
// The percentage of votes required to confirm transaction
requiredVotesPercentage: number
// The percentage of votes required for majority
requiredMajorityVotesPercentage: number
// max number of recent cycle shard data to keep
maxCyclesShardDataToKeep: number
// the number of cycles within which we want to keep \changes to a config*/
Expand Down Expand Up @@ -174,6 +176,7 @@ let config: Config = {
stopGossipTxData: false,
usePOQo: true,
requiredVotesPercentage: 2 / 3,
requiredMajorityVotesPercentage: 60,
maxCyclesShardDataToKeep: 10,
configChangeMaxCyclesToKeep: 5,
configChangeMaxChangesToKeep: 1000,
Expand Down
233 changes: 212 additions & 21 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 @@ -219,7 +219,7 @@
return { success: false }
}
}
if (config.verifyAccountData) {
if (config.verifyAccountData && receipt.globalModification === false) {
Dismissed Show dismissed Hide dismissed
if (profilerInstance) profilerInstance.profileSectionStart('Verify_receipt_account_data')
if (nestedCountersInstance)
nestedCountersInstance.countEvent('receipt', 'Verify_receipt_account_data')
Expand Down Expand Up @@ -328,17 +328,51 @@
return false
}
}
if (receipt.globalModification) return true
if (receipt.globalModification) {
const signedReceipt = receipt.signedReceipt as P2PTypes.GlobalAccountsTypes.GlobalTxReceipt
err = Utils.validateTypes(signedReceipt, {
tx: 'o',
signs: 'a',
})
if (err) {
Logger.mainLogger.error('Invalid receipt globalModification data', err)
return false
}
err = Utils.validateTypes(signedReceipt.tx, {
address: 's',
addressHash: 's',
value: 'o',
when: 'n',
source: 's',
})
if (err) {
Logger.mainLogger.error('Invalid receipt globalModification tx data', err)
return false
}
for (const sign of signedReceipt.signs) {
err = Utils.validateTypes(sign, {
owner: 's',
sig: 's',
})
if (err) {
Logger.mainLogger.error('Invalid receipt globalModification signs data', err)
return false
}
}
return true
}
// Global Modification Tx does not have appliedReceipt
const signedReceipt = receipt.signedReceipt as Receipt.SignedReceipt
const signedReceiptToValidate = {
proposal: 'o',
proposalHash: 's',
signaturePack: 'a',
voteOffsets: 'a',
}
// if (config.newPOQReceipt === false) delete appliedReceiptToValidate.confirmOrChallenge
err = Utils.validateTypes(receipt.signedReceipt, signedReceiptToValidate)
err = Utils.validateTypes(signedReceipt, signedReceiptToValidate)
if (err) {
Logger.mainLogger.error('Invalid receipt appliedReceipt data', err)
Logger.mainLogger.error('Invalid receipt signedReceipt data', err)
return false
}
const proposalToValidate = {
Expand All @@ -354,18 +388,25 @@
// delete appliedVoteToValidate.node_id
// delete appliedVoteToValidate.sign
// }
err = Utils.validateTypes(receipt.signedReceipt.proposal, proposalToValidate)
err = Utils.validateTypes(signedReceipt.proposal, proposalToValidate)
if (err) {
Logger.mainLogger.error('Invalid receipt appliedReceipt appliedVote data', err)
Logger.mainLogger.error('Invalid receipt signedReceipt appliedVote data', err)
return false
}
for (const signature of receipt.signedReceipt.signaturePack) {
for (const signature of signedReceipt.signaturePack) {
err = Utils.validateTypes(signature, {
owner: 's',
sig: 's',
})
if (err) {
Logger.mainLogger.error('Invalid receipt appliedReceipt signatures data', err)
Logger.mainLogger.error('Invalid receipt signedReceipt signatures data', err)
return false
}
}
for (const voteOffset of signedReceipt.voteOffsets) {
const isValid = typeof voteOffset === 'number' || !isNaN(voteOffset)
if (!isValid) {
Logger.mainLogger.error('Invalid receipt signedReceipt voteOffsets data', voteOffset)
return false
}
}
Expand Down Expand Up @@ -405,9 +446,7 @@
): Promise<{ success: boolean; requiredSignatures?: number; newReceipt?: Receipt.ArchiverReceipt }> => {
const result = { success: false }
// Check the signed nodes are part of the execution group nodes of the tx
const { executionShardKey, cycle, signedReceipt, globalModification } = receipt
if (globalModification && config.skipGlobalTxReceiptVerification) return { success: true }
const { signaturePack } = signedReceipt
const { executionShardKey, cycle, globalModification } = receipt
const { txId, timestamp } = receipt.tx
if (config.VERBOSE) {
const currentTimestamp = Date.now()
Expand All @@ -432,6 +471,93 @@
}
// Determine the home partition index of the primary account (executionShardKey)
const { homePartition } = ShardFunction.addressToPartition(cycleShardData.shardGlobals, executionShardKey)
if (globalModification) {
const appliedReceipt = receipt.signedReceipt as P2PTypes.GlobalAccountsTypes.GlobalTxReceipt
if (config.skipGlobalTxReceiptVerification) return { success: true }
else {
const { signs } = appliedReceipt
// Refer to https://github.com/shardeum/shardus-core/blob/7d8877b7e1a5b18140f898a64b932182d8a35298/src/p2p/GlobalAccounts.ts#L397
let votingGroupCount = cycleShardData.shardGlobals.nodesPerConsenusGroup

Check failure on line 480 in src/Data/Collector.ts

View workflow job for this annotation

GitHub Actions / ci / QA merge checks

'votingGroupCount' is never reassigned. Use 'const' instead
if (votingGroupCount > cycleShardData.nodes.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have a lower bound to the acceptable voting group count?

Copy link
Member

Choose a reason for hiding this comment

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

addressed

if (nestedCountersInstance)
nestedCountersInstance.countEvent('receipt', 'votingGroupCount_greater_than_nodes_length')
Logger.mainLogger.error(
'votingGroupCount_greater_than_nodes_length',
votingGroupCount,
cycleShardData.nodes.length
)
// votingGroupCount = cycleShardData.nodes.length
}
let isReceiptMajority =
(signs.length / votingGroupCount) * 100 >= config.requiredMajorityVotesPercentage
if (!isReceiptMajority) {
Logger.mainLogger.error(
`Invalid receipt globalModification signs count is less than ${config.requiredMajorityVotesPercentage}% of the votingGroupCount, ${signs.length}, ${votingGroupCount}`

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
.

Copilot Autofix AI 8 days ago

To fix the log injection issue, we need to sanitize the user input before logging it. Specifically, we should remove any newline characters from the config.requiredMajorityVotesPercentage value before including it in the log message. This can be done using String.prototype.replace to ensure no line endings are present in the user input.

Suggested changeset 1
src/Data/Collector.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/Data/Collector.ts b/src/Data/Collector.ts
--- a/src/Data/Collector.ts
+++ b/src/Data/Collector.ts
@@ -494,3 +494,3 @@
         Logger.mainLogger.error(
-          `Invalid receipt globalModification signs count is less than ${config.requiredMajorityVotesPercentage}% of the votingGroupCount, ${signs.length}, ${votingGroupCount}`
+          `Invalid receipt globalModification signs count is less than ${config.requiredMajorityVotesPercentage.replace(/\n|\r/g, "")}% of the votingGroupCount, ${signs.length}, ${votingGroupCount}`
         )
EOF
@@ -494,3 +494,3 @@
Logger.mainLogger.error(
`Invalid receipt globalModification signs count is less than ${config.requiredMajorityVotesPercentage}% of the votingGroupCount, ${signs.length}, ${votingGroupCount}`
`Invalid receipt globalModification signs count is less than ${config.requiredMajorityVotesPercentage.replace(/\n|\r/g, "")}% of the votingGroupCount, ${signs.length}, ${votingGroupCount}`
)
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
)
if (nestedCountersInstance)
nestedCountersInstance.countEvent(
'receipt',
`Invalid_receipt_globalModification_signs_count_less_than_${config.requiredMajorityVotesPercentage}%`
)
return result
}

const nodeMap = new Map<string, P2PTypes.NodeListTypes.Node>()
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this map is only going to change once per cycle we can move this up the level for further optimisation. But for now LGTM.

// Fill the map with nodes keyed by their public keys
cycleShardData.nodes.forEach((node) => {
if (node.publicKey) {
nodeMap.set(node.publicKey, node)
}
})
// Using a set to store the unique signers to avoid duplicates
const uniqueSigners = new Set()
for (const sign of signs) {
const { owner: nodePubKey } = sign
// Get the node id from the public key
const node = nodeMap.get(nodePubKey)
if (node == null) {
Logger.mainLogger.error(
`The node with public key ${nodePubKey} of the receipt ${txId} with ${timestamp} is not in the active nodesList of cycle ${cycle}`
)
if (nestedCountersInstance)
nestedCountersInstance.countEvent(
'receipt',
'globalModification_sign_owner_not_in_active_nodesList'
)
continue
}
// Check if the node is in the execution group
if (!cycleShardData.parititionShardDataMap.get(homePartition).coveredBy[node.id]) {
Logger.mainLogger.error(
`The node with public key ${nodePubKey} of the receipt ${txId} with ${timestamp} is not in the execution group of the tx`
)
if (nestedCountersInstance)
nestedCountersInstance.countEvent(
'receipt',
'globalModification_sign_node_not_in_execution_group_of_tx'
)
continue
}
uniqueSigners.add(nodePubKey)
}
isReceiptMajority =
(uniqueSigners.size / votingGroupCount) * 100 >= config.requiredMajorityVotesPercentage
if (isReceiptMajority) {
Logger.mainLogger.error(
`Invalid receipt globalModification valid signs count is less than votingGroupCount ${uniqueSigners.size}, ${votingGroupCount}`
)
if (nestedCountersInstance)
nestedCountersInstance.countEvent(
'receipt',
'Invalid_receipt_globalModification_valid_signs_count_less_than_votingGroupCount'
)
return result
}
const requiredSignatures = Math.floor(votingGroupCount * (config.requiredMajorityVotesPercentage / 100))
return { success: true, requiredSignatures }
}
}
const { signaturePack } = receipt.signedReceipt as Receipt.SignedReceipt
if (config.newPOQReceipt === false) {
// Refer to https://github.com/shardeum/shardus-core/blob/f7000c36faa0cd1e0832aa1e5e3b1414d32dcf66/src/state-manager/TransactionConsensus.ts#L1406
let votingGroupCount = cycleShardData.shardGlobals.nodesPerConsenusGroup
Expand Down Expand Up @@ -497,7 +623,8 @@
}
return { success: true, requiredSignatures }
}
// const { confirmOrChallenge } = appliedReceipt

// const { confirmOrChallenge } = appliedReceipt as Receipt.AppliedReceipt2
// // Check if the appliedVote node is in the execution group
// if (!cycleShardData.nodeShardDataMap.has(appliedVote.node_id)) {
// Logger.mainLogger.error('Invalid receipt appliedReceipt appliedVote node is not in the active nodesList')
Expand Down Expand Up @@ -612,10 +739,73 @@
nestedCounterMessages = []
): { success: boolean } => {
const result = { success: false, failedReasons, nestedCounterMessages }
const { signedReceipt, globalModification } = receipt
if (globalModification && config.skipGlobalTxReceiptVerification) return { success: true }
const { proposal, signaturePack, voteOffsets } = signedReceipt
const { txId: txid } = receipt.tx
const { globalModification, cycle, executionShardKey } = receipt
const { txId: txid, timestamp } = receipt.tx
if (globalModification) {
const appliedReceipt = receipt.signedReceipt as P2PTypes.GlobalAccountsTypes.GlobalTxReceipt
// Refer to https://github.com/shardeum/shardus-core/blob/7d8877b7e1a5b18140f898a64b932182d8a35298/src/p2p/GlobalAccounts.ts#L294

const { signs, tx } = appliedReceipt
const cycleShardData = shardValuesByCycle.get(cycle)
const { homePartition } = ShardFunction.addressToPartition(cycleShardData.shardGlobals, executionShardKey)
const nodeMap = new Map<string, P2PTypes.NodeListTypes.Node>()
// Fill the map with nodes keyed by their public keys
cycleShardData.nodes.forEach((node) => {
if (node.publicKey) {
nodeMap.set(node.publicKey, node)
}
})
const acceptableSigners = new Set<P2PTypes.P2PTypes.Signature>()
for (const sign of signs) {
const { owner: nodePubKey } = sign
// Get the node id from the public key
const node = nodeMap.get(nodePubKey)
if (node == null) {
Logger.mainLogger.error(
`The node with public key ${nodePubKey} of the receipt ${txid} with ${timestamp} is not in the active nodesList of cycle ${cycle}`
)
if (nestedCountersInstance)
nestedCountersInstance.countEvent(
'receipt',
'globalModification_sign_owner_not_in_active_nodesList'
)
continue
}
// Check if the node is in the execution group
if (!cycleShardData.parititionShardDataMap.get(homePartition).coveredBy[node.id]) {
Logger.mainLogger.error(
`The node with public key ${nodePubKey} of the receipt ${txid} with ${timestamp} is not in the execution group of the tx`
)
if (nestedCountersInstance)
nestedCountersInstance.countEvent(
'receipt',
'globalModification_sign_node_not_in_execution_group_of_tx'
)
continue
}
acceptableSigners.add(sign)
}
// Using a map to store the good signatures to avoid duplicates
const goodSignatures = new Map()
for (const sign of acceptableSigners) {
if (Crypto.verify({ ...tx, sign: sign })) {
goodSignatures.set(sign.owner, sign)
// Break the loop if the required number of good signatures are found
if (goodSignatures.size >= requiredSignatures) break
}
}
if (goodSignatures.size < requiredSignatures) {
failedReasons.push(
`Invalid receipt globalModification valid signs count is less than requiredSignatures ${txid}, ${goodSignatures.size}, ${requiredSignatures}`
)
nestedCounterMessages.push(
'Invalid_receipt_globalModification_valid_signs_count_less_than_requiredSignatures'
)
return result
}
return { success: true }
}
const { proposal, signaturePack, voteOffsets } = receipt.signedReceipt as Receipt.SignedReceipt
// Refer to https://github.com/shardeum/shardus-core/blob/50b6d00f53a35996cd69210ea817bee068a893d6/src/state-manager/TransactionConsensus.ts#L2799
const voteHash = calculateVoteHash(proposal, failedReasons, nestedCounterMessages)
// Refer to https://github.com/shardeum/shardus-core/blob/50b6d00f53a35996cd69210ea817bee068a893d6/src/state-manager/TransactionConsensus.ts#L2663
Expand All @@ -639,7 +829,7 @@
}
if (goodSignatures.size < requiredSignatures) {
failedReasons.push(
`Invalid receipt signedReceipt valid signatures count is less than requiredSignatures ${goodSignatures.size}, ${requiredSignatures}`
`Invalid receipt signedReceipt valid signatures count is less than requiredSignatures ${txid}, ${goodSignatures.size}, ${requiredSignatures}`
)
nestedCounterMessages.push(
'Invalid_receipt_signedReceipt_valid_signatures_count_less_than_requiredSignatures'
Expand Down Expand Up @@ -869,16 +1059,17 @@
// receiptId: tx.txId,
// timestamp: tx.timestamp,
// })
const { afterStates, cycle, tx, appReceiptData, signedReceipt } = receipt
receipt.beforeStates = config.storeReceiptBeforeStates ? receipt.beforeStates : []

const sortedVoteOffsets = (signedReceipt.voteOffsets ?? []).sort()
const { afterStates, cycle, tx, appReceiptData, signedReceipt, globalModification } = receipt
const sortedVoteOffsets = globalModification
? []
: (signedReceipt as Receipt.SignedReceipt).voteOffsets.sort()
const medianOffset = sortedVoteOffsets[Math.floor(sortedVoteOffsets.length / 2)] ?? 0
const applyTimestamp = tx.timestamp + medianOffset * 1000
if (config.VERBOSE) console.log('RECEIPT', 'Save', txId, timestamp, senderInfo)
processedReceiptsMap.set(tx.txId, tx.timestamp)
receiptsInValidationMap.delete(tx.txId)
if (missingReceiptsMap.has(tx.txId)) missingReceiptsMap.delete(tx.txId)
receipt.beforeStates = globalModification || config.storeReceiptBeforeStates ? receipt.beforeStates : [] // Store beforeStates for globalModification tx, or if config.storeReceiptBeforeStates is true
combineReceipts.push({
...receipt,
receiptId: tx.txId,
Expand Down
6 changes: 5 additions & 1 deletion src/State.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ export let isFirst = false
export let isActive = false
export const archiversReputation: Map<string, string> = new Map()

export async function initFromConfig(config: Config, shutDownMode = false, useArchiverDiscovery = true): Promise<void> {
export async function initFromConfig(
config: Config,
shutDownMode = false,
useArchiverDiscovery = true
): Promise<void> {
// Get own nodeInfo from config
nodeState.ip = config.ARCHIVER_IP
nodeState.port = config.ARCHIVER_PORT
Expand Down
1 change: 1 addition & 0 deletions src/dbstore/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export const initializeDB = async (config: Config): Promise<void> => {
receiptDatabase,
'CREATE INDEX if not exists `receipts_timestamp` ON `receipts` (`timestamp` ASC)'
)
await runCreate(receiptDatabase, 'CREATE INDEX if not exists `receipts_cycle` ON `receipts` (`cycle` ASC)')
await runCreate(
receiptDatabase,
'CREATE INDEX if not exists `receipts_cycle_timestamp` ON `receipts` (`cycle` ASC, `timestamp` ASC)'
Expand Down
Loading
Loading