Gold AJV and Internal endpoint hardening for review

src/p2p/Apoptosis.ts

@@ -48,6 +48,9 @@ import { TypeIdentifierEnum } from '../types/enum/TypeIdentifierEnum'
 import { SQLDataTypes } from '../storage/utils/schemaDefintions'
 import { InternalRouteEnum } from '../types/enum/InternalRouteEnum'
 import { Utils } from '@shardus/types'
+import { BadRequest, serializeResponseError } from '../types/ResponseError'
+import { RequestErrorEnum } from '../types/enum/RequestErrorEnum'
+import { getStreamWithTypeCheck, requestErrorHandler } from '../types/Helpers'
 
 
 /** STATE */
@@ -96,17 +99,23 @@ const failExternalRoute: P2P.P2PTypes.Route<Handler> = {
 const apoptosisInternalRoute: P2P.P2PTypes.Route<InternalBinaryHandler<Buffer>> = {
   name: InternalRouteEnum.apoptosize,
   handler: (payload, response, header, sign) => {
-    profilerInstance.scopedProfileSectionStart('apoptosize')
+    const route = InternalRouteEnum.apoptosize
+    nestedCountersInstance.countEvent('internal', route)
+    profilerInstance.scopedProfileSectionStart(route)
+    const errorHandler = (
+      errorType: RequestErrorEnum,
+      opts?: { customErrorLog?: string; customCounterSuffix?: string }
+    ): void => requestErrorHandler(route, errorType, header, opts)
+
     try {
-      const requestStream = VectorBufferStream.fromBuffer(payload)
-      const requestType = requestStream.readUInt16()
-      if (requestType !== TypeIdentifierEnum.cApoptosisProposalReq) {
-        /* prettier-ignore */ if (logFlags.error) warn(`apoptosisInternalRoute: bad requestType: ${requestType}`)
-        let resp: ApoptosisProposalResp = { s: 'bad request', r: 1 }
-        response(resp, serializeApoptosisProposalResp)
-        return
+      const requestStream = getStreamWithTypeCheck(payload, TypeIdentifierEnum.cApoptosisProposalReq)
+      if (!requestStream) {
+        errorHandler(RequestErrorEnum.InvalidRequest)
+        return response(BadRequest('Invalid apoptosisInternalRoute request stream'), serializeResponseError)
       }
+
       const req = deserializeApoptosisProposalReq(requestStream)
+
       const apopProposal: P2P.ApoptosisTypes.SignedApoptosisProposal = {
         id: req.id,
         when: req.when,
@@ -125,15 +134,13 @@ const apoptosisInternalRoute: P2P.P2PTypes.Route<InternalBinaryHandler<Buffer>>
 
         /* prettier-ignore */ nestedCountersInstance.countEvent('p2p', `self-isDownCheck c:${currentCycle} ${down_msg}`, 1)
         let resp: ApoptosisProposalResp = { s: down_msg, r: 1 }
-        response(resp, serializeApoptosisProposalResp)
-        return
+        return response(resp, serializeApoptosisProposalResp)
       }
 
       const when = apopProposal.when
       if (when > currentCycle + 1 || when < currentCycle - 1) {
         let resp: ApoptosisProposalResp = { s: 'fail', r: 2 }
-        response(resp, serializeApoptosisProposalResp)
-        return
+        return response(resp, serializeApoptosisProposalResp)
       }
       //  check that the node which sent this is the same as the node that signed it, otherwise this is not original message so ignore it
       if (header.sender_id === apopProposal.id) {
@@ -147,19 +154,16 @@ const apoptosisInternalRoute: P2P.P2PTypes.Route<InternalBinaryHandler<Buffer>>
             Comms.sendGossip(gossipRouteName, apopProposal)
           }
           let resp: ApoptosisProposalResp = { s: 'pass', r: 1 }
-          response(resp, serializeApoptosisProposalResp)
-          return
+          return response(resp, serializeApoptosisProposalResp)
         } else {
           /* prettier-ignore */ if (logFlags.error) warn(`addProposal failed for payload: ${Utils.safeStringify(apopProposal)}`)
           let resp: ApoptosisProposalResp = { s: 'fail', r: 4 }
-          response(resp, serializeApoptosisProposalResp)
-          return
+          return response(resp, serializeApoptosisProposalResp)
         }
       } else {
         /* prettier-ignore */ if (logFlags.error) warn(`sender is not apop node: sender:${header.sender_id} apop:${apopProposal.id}`)
         let resp: ApoptosisProposalResp = { s: 'fail', r: 3 }
-        response(resp, serializeApoptosisProposalResp)
-        return
+        return response(resp, serializeApoptosisProposalResp)
       }
     } finally {
       profilerInstance.scopedProfileSectionEnd('apoptosize')

src/p2p/CycleCreator.ts

@@ -48,6 +48,7 @@ import { RequestErrorEnum } from '../types/enum/RequestErrorEnum'
 import { BadRequest, InternalError, NotFound, serializeResponseError } from '../types/ResponseError'
 import { Utils } from '@shardus/types'
 import { nodeListFromStates } from './Join'
+import { AJVSchemaEnum } from '../types/enum/AJVSchemaEnum'
 
 /** CONSTANTS */
 
@@ -194,7 +195,7 @@ const compareCertBinaryHandler: P2P.P2PTypes.Route<InternalBinaryHandler<Buffer>
 
       const req: CompareCertReq = deserializeCompareCertReq(requestStream)
 
-      const errors = verifyPayload('CompareCertReq', req)
+      const errors = verifyPayload(AJVSchemaEnum.CompareCertReq, req)
       if (errors && errors.length > 0) {
         p2pLogger.error(`compareCert request validation errors: ${errors}`)
         return respond(BadRequest('Request validation errors'), serializeResponseError)

src/state-manager/AccountPatcher.ts

@@ -493,44 +493,65 @@ class AccountPatcher {
         nestedCountersInstance.countEvent('internal', route)
         this.profiler.scopedProfileSectionStart(route, false, payloadBuffer.length)
         try {
-          const requestStream = getStreamWithTypeCheck(payloadBuffer, TypeIdentifierEnum.cRepairOOSAccountsReq)
+          const requestStream = getStreamWithTypeCheck(
+            payloadBuffer,
+            TypeIdentifierEnum.cRepairOOSAccountsReq
+          )
           if (!requestStream) {
             return
           }
-          // (Optional) Check verification data in the header 
+          // (Optional) Check verification data in the header
           const payload = deserializeRepairOOSAccountsReq(requestStream)
-          // verifyPayload('RepairOOSAccountsReq', payload)
+          // verifyPayload(AJVSchemaEnum.RepairOOSAccountsReq', payload)
           for (const repairInstruction of payload?.repairInstructions) {
             const { accountID, txId, hash, accountData, targetNodeId, receipt2 } = repairInstruction
 
             // check if we are the target node
             if (targetNodeId !== Self.id) {
-              nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: not target node for txId: ${txId}`)
+              nestedCountersInstance.countEvent(
+                'accountPatcher',
+                `binary/repair_oos_accounts: not target node for txId: ${txId}`
+              )
               continue
             }
 
             // check if we cover this accountId
             const storageNodes = this.stateManager.transactionQueue.getStorageGroupForAccount(accountID)
             const isInStorageGroup = storageNodes.map((node) => node.id).includes(Self.id)
             if (!isInStorageGroup) {
-              nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: not in storage group for account: ${accountID}`)
+              nestedCountersInstance.countEvent(
+                'accountPatcher',
+                `binary/repair_oos_accounts: not in storage group for account: ${accountID}`
+              )
               continue
             }
             // check if we have already repaired this account
             const accountHashCache = this.stateManager.accountCache.getAccountHash(accountID)
             if (accountHashCache != null && accountHashCache.h === hash) {
-              nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: already repaired account: ${accountID}`)
+              nestedCountersInstance.countEvent(
+                'accountPatcher',
+                `binary/repair_oos_accounts: already repaired account: ${accountID}`
+              )
               continue
             }
             if (accountHashCache != null && accountHashCache.t > accountData.timestamp) {
-              nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: we have newer account: ${accountID}`)
+              nestedCountersInstance.countEvent(
+                'accountPatcher',
+                `binary/repair_oos_accounts: we have newer account: ${accountID}`
+              )
               continue
             }
 
-            const archivedQueueEntry = this.stateManager.transactionQueue.getQueueEntryArchived(txId, 'repair_oos_accounts')
+            const archivedQueueEntry = this.stateManager.transactionQueue.getQueueEntryArchived(
+              txId,
+              'repair_oos_accounts'
+            )
 
             if (archivedQueueEntry == null) {
-              nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: no archivedQueueEntry for txId: ${txId}`)
+              nestedCountersInstance.countEvent(
+                'accountPatcher',
+                `binary/repair_oos_accounts: no archivedQueueEntry for txId: ${txId}`
+              )
               this.mainLogger.debug(`repair_oos_accounts: no archivedQueueEntry for txId: ${txId}`)
               continue
             }
@@ -541,23 +562,37 @@ class AccountPatcher {
 
             if (receivedBestVote != null) {
               // Check if vote is from eligible list of voters for this TX
-              if(this.stateManager.transactionQueue.useNewPOQ && !archivedQueueEntry.eligibleNodeIdsToVote.has(receivedBestVote.node_id)) {
-                nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: vote from ineligible node for txId: ${txId}`)
+              if (
+                this.stateManager.transactionQueue.useNewPOQ &&
+                !archivedQueueEntry.eligibleNodeIdsToVote.has(receivedBestVote.node_id)
+              ) {
+                nestedCountersInstance.countEvent(
+                  'accountPatcher',
+                  `binary/repair_oos_accounts: vote from ineligible node for txId: ${txId}`
+                )
                 continue
               }
 
               // Check signature of the vote
-              if (!this.crypto.verify(
-                receivedBestVote as SignedObject,
-                archivedQueueEntry.executionGroupMap.get(receivedBestVote.node_id).publicKey
-              )) {
-                nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: vote signature invalid for txId: ${txId}`)
+              if (
+                !this.crypto.verify(
+                  receivedBestVote as SignedObject,
+                  archivedQueueEntry.executionGroupMap.get(receivedBestVote.node_id).publicKey
+                )
+              ) {
+                nestedCountersInstance.countEvent(
+                  'accountPatcher',
+                  `binary/repair_oos_accounts: vote signature invalid for txId: ${txId}`
+                )
                 continue
               }
 
               // Check transaction result from vote
               if (!receivedBestVote.transaction_result) {
-                nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: vote result not true for txId ${txId}`)
+                nestedCountersInstance.countEvent(
+                  'accountPatcher',
+                  `binary/repair_oos_accounts: vote result not true for txId ${txId}`
+                )
                 continue
               }
 
@@ -568,7 +603,10 @@ class AccountPatcher {
               for (let i = 0; i < receivedBestVote.account_id.length; i++) {
                 if (receivedBestVote.account_id[i] === accountID) {
                   if (receivedBestVote.account_state_hash_after[i] !== calculatedAccountHash) {
-                    nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: account hash mismatch for txId: ${txId}`)
+                    nestedCountersInstance.countEvent(
+                      'accountPatcher',
+                      `binary/repair_oos_accounts: account hash mismatch for txId: ${txId}`
+                    )
                     accountHashMatch = false
                   } else {
                     accountHashMatch = true
@@ -577,40 +615,60 @@ class AccountPatcher {
                 }
               }
               if (accountHashMatch === false) {
-                nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: vote account hash mismatch for txId: ${txId}`)
+                nestedCountersInstance.countEvent(
+                  'accountPatcher',
+                  `binary/repair_oos_accounts: vote account hash mismatch for txId: ${txId}`
+                )
                 continue
               }
             } else {
               // Skip this account apply as we were not able to get the best vote for this tx
-              nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: no vote for txId: ${txId}`)
+              nestedCountersInstance.countEvent(
+                'accountPatcher',
+                `binary/repair_oos_accounts: no vote for txId: ${txId}`
+              )
               continue
             }
 
             if (this.stateManager.transactionQueue.useNewPOQ) {
               if (bestMessage != null) {
                 // Skip if challenge receipt
                 if (bestMessage.message === 'challenge') {
-                  nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: challenge for txId: ${txId}`)
+                  nestedCountersInstance.countEvent(
+                    'accountPatcher',
+                    `binary/repair_oos_accounts: challenge for txId: ${txId}`
+                  )
                   continue
                 }
 
                 // Check if mesasge is from eligible list of responders for this TX
-                if(!archivedQueueEntry.eligibleNodeIdsToConfirm.has(bestMessage.nodeId)) {
-                  nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: confirmation from ineligible node for txId: ${txId}`)
+                if (!archivedQueueEntry.eligibleNodeIdsToConfirm.has(bestMessage.nodeId)) {
+                  nestedCountersInstance.countEvent(
+                    'accountPatcher',
+                    `binary/repair_oos_accounts: confirmation from ineligible node for txId: ${txId}`
+                  )
                   continue
                 }
 
                 // Check signature of the message
-                if(!this.crypto.verify(
-                  bestMessage as SignedObject,
-                  archivedQueueEntry.executionGroupMap.get(bestMessage.nodeId).publicKey
-                )) {
-                  nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: confirmation signature invalid for txId: ${txId}`)
+                if (
+                  !this.crypto.verify(
+                    bestMessage as SignedObject,
+                    archivedQueueEntry.executionGroupMap.get(bestMessage.nodeId).publicKey
+                  )
+                ) {
+                  nestedCountersInstance.countEvent(
+                    'accountPatcher',
+                    `binary/repair_oos_accounts: confirmation signature invalid for txId: ${txId}`
+                  )
                   continue
                 }
               } else {
                 // Skip this account apply as we were not able to get the best confirmation for this tx
-                nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts: no confirmation for txId: ${txId}`)
+                nestedCountersInstance.countEvent(
+                  'accountPatcher',
+                  `binary/repair_oos_accounts: no confirmation for txId: ${txId}`
+                )
                 continue
               }
             }
@@ -625,9 +683,21 @@ class AccountPatcher {
               true,
               updatedAccounts
             )
-            if (logFlags.debug) this.mainLogger.debug(`binary/repair_oos_accounts: ${updatedAccounts.length} updated, ${failedHashes.length} failed`)
-            nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts:${updatedAccounts.length} updated, accountId: ${utils.makeShortHash(accountID)}, cycle: ${this.stateManager.currentCycleShardData.cycleNumber}`)
-            if (failedHashes.length > 0) nestedCountersInstance.countEvent('accountPatcher', `binary/repair_oos_accounts:${failedHashes.length} failed`)
+            if (logFlags.debug)
+              this.mainLogger.debug(
+                `binary/repair_oos_accounts: ${updatedAccounts.length} updated, ${failedHashes.length} failed`
+              )
+            nestedCountersInstance.countEvent(
+              'accountPatcher',
+              `binary/repair_oos_accounts:${updatedAccounts.length} updated, accountId: ${utils.makeShortHash(
+                accountID
+              )}, cycle: ${this.stateManager.currentCycleShardData.cycleNumber}`
+            )
+            if (failedHashes.length > 0)
+              nestedCountersInstance.countEvent(
+                'accountPatcher',
+                `binary/repair_oos_accounts:${failedHashes.length} failed`
+              )
             let success = false
             if (updatedAccounts.length > 0 && failedHashes.length === 0) {
               success = true

src/state-manager/AccountSync.ts

@@ -55,6 +55,7 @@
 } from '../types/GlobalAccountReportResp'
 import { BadRequest, InternalError, serializeResponseError } from '../types/ResponseError'
 import { Utils } from '@shardus/types'
+import { AJVSchemaEnum } from '../types/enum/AJVSchemaEnum'
 
 const REDUNDANCY = 3
 
@@ -282,7 +283,7 @@
         this.profiler.scopedProfileSectionStart('get_account_data3', false, msgSize)
         const result = {} as { data: GetAccountDataByRangeSmart; errors?: string[] } //TSConversion  This is complicated !!(due to app wrapping)  as {data: Shardus.AccountData[] | null}
 
-        const errors = verifyPayload('GetAccountData3Req', payload)
+        const errors = verifyPayload(AJVSchemaEnum.GetAccountDataReq, payload)
         if (errors && errors.length > 0) {
           this.mainLogger.error(`get_account_data3: request validation errors: ${errors}`)
           result.errors = errors
@@ -341,7 +342,7 @@
           }
           const readableReq = deserializeGetAccountDataReq(reqStream)
 
-          // validate the request
+          // validate the addres range for shardus addresses
           const valid = verifyGetAccountDataReq(readableReq)
           if (valid === false) {
             nestedCountersInstance.countEvent('internal', `${route}-invalid_account_ids`)
@@ -928,7 +929,7 @@
    * @param tag a debug tag so that logs and counters will give more context
    * @returns GlobalAccountReportResp
    */
   async getRobustGlobalReport(tag = '', syncFromArchiver: boolean = false): Promise<GlobalAccountReportResp> {
     console.log('getRobustGlobalReport start', tag, syncFromArchiver)
     if (!syncFromArchiver) {
       this.lastWinningGlobalReportNodes = []
@@ -1004,7 +1005,7 @@
       const signedPayload = this.crypto.sign(payload)
       console.log('getGlobalAccountReportFromArchiver messsage', signedPayload)
 
       const getGlobalAccountReportFromArchiver = async () => {
         const globalAccountReportArchiverUrl = `http://${archiver.ip}:${archiver.port}/get_globalaccountreport_archiver`
 
         try {
@@ -1018,14 +1019,14 @@
       }
 
       let result: Partial<GlobalAccountReportResp> & { msg: string }
       result = await getGlobalAccountReportFromArchiver()
       return checkResultFn(result, archiver.publicKey, true)
     }
 
     const checkResultFn = (
       result: (Partial<GlobalAccountReportResp> & { msg: string }) | boolean,
       nodeId: string,
       resultFromArchiver: boolean = false
     ) => {
       // Various failure cases will alter the returned result so that it is tallied in a more orderly way.
       // The random numbers were kept to prevent the hash of results from being equal, but now custom equalFn takes care of this concern
@@ -1368,7 +1369,7 @@
     range: StateManagerTypes.shardFunctionTypes.BasicAddressRange,
     cycle: number,
     initalSync = false,
     syncFromArchiver: boolean = false
   ): SyncTrackerInterface {
     const index = this.syncTrackerIndex++
 
@@ -1392,7 +1393,7 @@
   createSyncTrackerByForGlobals(
     cycle: number,
     initalSync = false,
     syncFromArchiver: boolean = false
   ): SyncTrackerInterface {
     const index = this.syncTrackerIndex++
 

src/state-manager/TransactionQueue.ts

@@ -97,9 +97,10 @@ import {
   serializeRequestReceiptForTxReq,
 } from '../types/RequestReceiptForTxReq'
 import { isNodeInRotationBounds } from '../p2p/Utils'
-import { ResponseError } from '../types/ResponseError'
+import { BadRequest, ResponseError, serializeResponseError } from '../types/ResponseError'
 import { error } from 'console'
 import { PoqoDataAndReceiptReq, serializePoqoDataAndReceiptReq } from '../types/PoqoDataAndReceiptReq'
+import { AJVSchemaEnum } from '../types/enum/AJVSchemaEnum'
 
 interface Receipt {
   tx: AcceptedTx
@@ -404,7 +405,6 @@ class TransactionQueue {
           if (!requestStream) {
             return errorHandler(RequestErrorEnum.InvalidRequest)
           }
-
           // verification data checks
           if (header.verification_data == null) {
             return errorHandler(RequestErrorEnum.MissingVerificationData)
@@ -698,7 +698,7 @@ class TransactionQueue {
 
           const req: SpreadTxToGroupSyncingReq = deserializeSpreadTxToGroupSyncingReq(requestStream)
 
-          const ajvErrors = verifyPayload('SpreadTxToGroupSyncingReq', req)
+          const ajvErrors = verifyPayload(AJVSchemaEnum.SpreadTxToGroupSyncingReq, req)
           if (ajvErrors && ajvErrors.length > 0) {
             this.mainLogger.error(`${route}: request validation errors: ${ajvErrors}`)
             return errorHandler(RequestErrorEnum.InvalidPayload)