SHARD-44: GlobalTx fixes #103
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/Data/Collector.ts
Outdated
| @@ -73,6 +73,11 @@ | |||
| executionGroupNodes: ConsensusNodeInfo[], | |||
| minConfirmations: number = config.RECEIPT_CONFIRMATIONS | |||
| ): Promise<{ success: boolean; newReceipt?: Receipt.ArchiverReceipt }> => { | |||
| // If robustQuery is disabled, do offline verification | |||
| if (!config.useRobustQueryForReceipt) { | |||
Check failure
Code scanning / CodeQL
User-controlled bypass of security check High
S0naliThakur
force-pushed
the
globalTx-fixes
branch
2 times, most recently
from
793f574 to
c131ed4
Compare
S0naliThakur
requested review from
jintukumardas,
abdulazeem-tk4vr and
aniketdivekar
S0naliThakur
force-pushed
the
globalTx-fixes
branch
2 times, most recently
from
f29133e to
cf6a9e3
Compare
| if (nestedCountersInstance) | ||
| nestedCountersInstance.countEvent( | ||
| 'receipt', | ||
| 'Invalid_receipt_globalModification_valid_signs_count_less_than_votingGroupCount' |
Check failure
Code scanning / CodeQL
User-controlled bypass of security check High
S0naliThakur
force-pushed
the
globalTx-fixes
branch
from
56d1083 to
832bdec
Compare
S0naliThakur
force-pushed
the
globalTx-fixes
branch
from
82ce0d0 to
780ed88
Compare
S0naliThakur
dismissed stale reviews from aniketdivekar and devendra-shardeum
The base branch was changed.
S0naliThakur
requested review from
aniketdivekar,
devendra-shardeum and
abdulazeem-tk4vr
|
@S0naliThakur Since we have disabled the workers. We can also comment out the code where we are initializing and setting up the worker process. Please refer to these lines -
CC: @aniketdivekar |
|
|
||
| const nodeMap = new Map<string, P2PTypes.NodeListTypes.Node>() | ||
| // Fill the map with nodes keyed by their public keys | ||
| cycleShardData.nodes.forEach((node) => { |
There was a problem hiding this comment.
I think it would be ok to just put the nodeMap on the cycleShardData the first time we compute it (lazy init)
that does not need to change to merge this. I suspect we will do a more general perf pass later as time permits.
| } | ||
| // Check if the node is in the execution group | ||
| if (!cycleShardData.parititionShardDataMap.get(homePartition).coveredBy[node.id]) { | ||
| Logger.mainLogger.error( |
There was a problem hiding this comment.
I think in the future we should generally be putting log lines behind if checks like in the validator. This is because even if you don't log it you still pay the string format cost.
(this is not a blocker though)
There was a problem hiding this comment.
I do have some logger pattern upgrades to share later anyhow.
| for (const signature of normalReceipt.signaturePack) { | ||
| if (!signature || !signature.owner) continue | ||
|
|
||
| const node = executionGroupNodes.find((n) => n.publicKey.toLowerCase() === signature.owner.toLowerCase()) |
There was a problem hiding this comment.
when we later do a perf pass a map would avoid O(n^2)
…loadReceipt, improved error logs, deprecated setupWorkerProcesses and offloadReceipt.
…rchiverReceipt types
S0naliThakur
dismissed stale reviews from afostr, aniketdivekar, and jintukumardas
via
6fc1783
S0naliThakur
force-pushed
the
globalTx-fixes
branch
from
d271eb0 to
6fc1783
Compare
Linear: https://linear.app/shm/issue/SHARD-44/globaltx-validation
Summary: Implement global receipt validation and enhance receipt verification logic
useRobustQueryForReceiptflagstoreReceiptmethod to useverifyArchiverReceiptdirectly.verifyGlobalTxreceiptOfflineandverifyReceiptMajorityfor better code readability.