fix: enhanced reliability of eth RPC methods with null checks and ret…

…ry mechanisms (#3349) * fix: fixed flaky precheck test Signed-off-by: Logan Nguyen <[email protected]> * fix: handle log null entities more gracefully in getBlock() Signed-off-by: Logan Nguyen <[email protected]> * fix: added null check to root hash builder Signed-off-by: Logan Nguyen <[email protected]> * fix: modified getContractResultWithRetry() to accept more general getContract MN methods Signed-off-by: Logan Nguyen <[email protected]> * fix: reused getContractResultWithRetry for getContractResults Signed-off-by: Logan Nguyen <[email protected]> * fix: added getContractResultsLogsWithRetry() Signed-off-by: Logan Nguyen <[email protected]> * fix: reverted licenses Signed-off-by: Logan Nguyen <[email protected]> Revert "fix: reverted licenses" This reverts commit d3c860a. Reapply "fix: reverted licenses" This reverts commit 50a9acd5031490f3f805042a70bfdae7c589146d. * fix: checked empty blockHash for getHistoricalBlockResponse() Signed-off-by: Logan Nguyen <[email protected]> * fix: throw error if log.block_number or log.index is null or log.block_hash is empty Signed-off-by: Logan Nguyen <[email protected]> --------- Signed-off-by: Logan Nguyen <[email protected]>
hashgraph · Jan 9, 2025 · cb93114 · cb93114
1 parent b2bff54
commit cb93114
Show file tree

Hide file tree

Showing 10 changed files with 316 additions and 87 deletions.
diff --git a/packages/relay/src/lib/clients/mirrorNodeClient.ts b/packages/relay/src/lib/clients/mirrorNodeClient.ts
@@ -1,4 +1,4 @@
-/* -
+/*-
  *
  * Hedera JSON RPC Relay
  *
@@ -620,7 +620,10 @@ export class MirrorNodeClient {
       requestDetails,
     );
 
-    await this.cacheService.set(cachedLabel, block, MirrorNodeClient.GET_BLOCK_ENDPOINT, requestDetails);
+    if (block) {
+      await this.cacheService.set(cachedLabel, block, MirrorNodeClient.GET_BLOCK_ENDPOINT, requestDetails);
+    }
+
     return block;
   }
 
@@ -754,21 +757,42 @@ export class MirrorNodeClient {
    * In some very rare cases the /contracts/results api is called before all the data is saved in
    * the mirror node DB and `transaction_index` or `block_number` is returned as `undefined` or `block_hash` as `0x`.
    * A single re-fetch is sufficient to resolve this problem.
-   * @param {string} transactionIdOrHash - The transaction ID or hash
-   * @param {RequestDetails} requestDetails - The request details for logging and tracking.
+   *
+   * @param {string} methodName - The name of the method used to fetch contract results.
+   * @param {any[]} args - The arguments to be passed to the specified method for fetching contract results.
+   * @param {RequestDetails} requestDetails - Details used for logging and tracking the request.
+   * @returns {Promise<any>} - A promise resolving to the fetched contract result, either on the first attempt or after a retry.
    */
-  public async getContractResultWithRetry(transactionIdOrHash: string, requestDetails: RequestDetails) {
-    const contractResult = await this.getContractResult(transactionIdOrHash, requestDetails);
-    if (
-      contractResult &&
-      !(
-        contractResult.transaction_index &&
-        contractResult.block_number &&
-        contractResult.block_hash != EthImpl.emptyHex
-      )
-    ) {
-      return this.getContractResult(transactionIdOrHash, requestDetails);
+  public async getContractResultWithRetry(
+    methodName: string,
+    args: any[],
+    requestDetails: RequestDetails,
+  ): Promise<any> {
+    const shortDelay = 500;
+    const contractResult = await this[methodName](...args);
+
+    if (contractResult) {
+      const contractObjects = Array.isArray(contractResult) ? contractResult : [contractResult];
+      for (const contractObject of contractObjects) {
+        if (
+          contractObject &&
+          (contractObject.transaction_index == null ||
+            contractObject.block_number == null ||
+            contractObject.block_hash == EthImpl.emptyHex)
+        ) {
+          if (this.logger.isLevelEnabled('debug')) {
+            this.logger.debug(
+              `${requestDetails.formattedRequestId} Contract result contains undefined transaction_index, block_number, or block_hash is an empty hex (0x): transaction_hash:${contractObject.hash}, transaction_index:${contractObject.transaction_index}, block_number=${contractObject.block_number}, block_hash=${contractObject.block_hash}. Retrying after a delay of ${shortDelay} ms `,
+            );
+          }
+
+          // Backoff before repeating request
+          await new Promise((r) => setTimeout(r, shortDelay));
+          return await this[methodName](...args);
+        }
+      }
     }
+
     return contractResult;
   }
 
@@ -870,14 +894,25 @@ export class MirrorNodeClient {
     return this.getQueryParams(queryParamObject);
   }
 
-  public async getContractResultsLogs(
+  /**
+   * In some very rare cases the /contracts/results/logs api is called before all the data is saved in
+   * the mirror node DB and `transaction_index`, `block_number`, `index` is returned as `undefined`, or block_hash is an empty hex (0x).
+   * A single re-fetch is sufficient to resolve this problem.
+   *
+   * @param {RequestDetails} requestDetails - Details used for logging and tracking the request.
+   * @param {IContractLogsResultsParams} [contractLogsResultsParams] - Parameters for querying contract logs results.
+   * @param {ILimitOrderParams} [limitOrderParams] - Parameters for limit and order when fetching the logs.
+   * @returns {Promise<any[]>} - A promise resolving to the paginated contract logs results.
+   */
+  public async getContractResultsLogsWithRetry(
     requestDetails: RequestDetails,
     contractLogsResultsParams?: IContractLogsResultsParams,
     limitOrderParams?: ILimitOrderParams,
-  ) {
+  ): Promise<any[]> {
+    const shortDelay = 500;
     const queryParams = this.prepareLogsParams(contractLogsResultsParams, limitOrderParams);
 
-    return this.getPaginatedResults(
+    const logResults = await this.getPaginatedResults(
       `${MirrorNodeClient.GET_CONTRACT_RESULT_LOGS_ENDPOINT}${queryParams}`,
       MirrorNodeClient.GET_CONTRACT_RESULT_LOGS_ENDPOINT,
       MirrorNodeClient.CONTRACT_RESULT_LOGS_PROPERTY,
@@ -886,6 +921,38 @@ export class MirrorNodeClient {
       1,
       MirrorNodeClient.mirrorNodeContractResultsLogsPageMax,
     );
+
+    if (logResults) {
+      for (const log of logResults) {
+        if (
+          log &&
+          (log.transaction_index == null ||
+            log.block_number == null ||
+            log.index == null ||
+            log.block_hash === EthImpl.emptyHex)
+        ) {
+          if (this.logger.isLevelEnabled('debug')) {
+            this.logger.debug(
+              `${requestDetails.formattedRequestId} Contract result log contains undefined transaction_index, block_number, index, or block_hash is an empty hex (0x): transaction_hash:${log.transaction_hash}, transaction_index:${log.transaction_index}, block_number=${log.block_number}, log_index=${log.index}, block_hash=${log.block_hash}. Retrying after a delay of ${shortDelay} ms.`,
+            );
+          }
+
+          // Backoff before repeating request
+          await new Promise((r) => setTimeout(r, shortDelay));
+          return await this.getPaginatedResults(
+            `${MirrorNodeClient.GET_CONTRACT_RESULT_LOGS_ENDPOINT}${queryParams}`,
+            MirrorNodeClient.GET_CONTRACT_RESULT_LOGS_ENDPOINT,
+            MirrorNodeClient.CONTRACT_RESULT_LOGS_PROPERTY,
+            requestDetails,
+            [],
+            1,
+            MirrorNodeClient.mirrorNodeContractResultsLogsPageMax,
+          );
+        }
+      }
+    }
+
+    return logResults;
   }
 
   public async getContractResultsLogsByAddress(

diff --git a/packages/relay/src/lib/clients/sdkClient.ts b/packages/relay/src/lib/clients/sdkClient.ts
@@ -724,6 +724,11 @@ export class SDKClient {
       );
       return transactionResponse;
     } catch (e: any) {
+      this.logger.warn(
+        e,
+        `${requestDetails.formattedRequestId} Transaction failed while executing transaction via the SDK: transactionId=${transaction.transactionId}, callerName=${callerName}, txConstructorName=${txConstructorName}`,
+      );
+
       if (e instanceof JsonRpcError) {
         throw e;
       }

diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts
@@ -1,4 +1,4 @@
-/* -
+/*-
  *
  * Hedera JSON RPC Relay
  *
@@ -1922,13 +1922,17 @@ export class EthImpl implements Eth {
     transactionIndex: string,
     requestDetails: RequestDetails,
   ): Promise<Transaction | null> {
-    const contractResults = await this.mirrorNodeClient.getContractResults(
+    const contractResults = await this.mirrorNodeClient.getContractResultWithRetry(
+      this.mirrorNodeClient.getContractResults.name,
+      [
+        requestDetails,
+        {
+          [blockParam.title]: blockParam.value,
+          transactionIndex: Number(transactionIndex),
+        },
+        undefined,
+      ],
       requestDetails,
-      {
-        [blockParam.title]: blockParam.value,
-        transactionIndex: Number(transactionIndex),
-      },
-      undefined,
     );
 
     if (!contractResults[0]) return null;
@@ -2201,7 +2205,11 @@ export class EthImpl implements Eth {
       this.logger.trace(`${requestIdPrefix} getTransactionByHash(hash=${hash})`, hash);
     }
 
-    const contractResult = await this.mirrorNodeClient.getContractResultWithRetry(hash, requestDetails);
+    const contractResult = await this.mirrorNodeClient.getContractResultWithRetry(
+      this.mirrorNodeClient.getContractResult.name,
+      [hash, requestDetails],
+      requestDetails,
+    );
     if (contractResult === null || contractResult.hash === undefined) {
       // handle synthetic transactions
       const syntheticLogs = await this.common.getLogsWithParams(
@@ -2265,7 +2273,12 @@ export class EthImpl implements Eth {
       return cachedResponse;
     }
 
-    const receiptResponse = await this.mirrorNodeClient.getContractResultWithRetry(hash, requestDetails);
+    const receiptResponse = await this.mirrorNodeClient.getContractResultWithRetry(
+      this.mirrorNodeClient.getContractResult.name,
+      [hash, requestDetails],
+      requestDetails,
+    );
+
     if (receiptResponse === null || receiptResponse.hash === undefined) {
       // handle synthetic transactions
       const syntheticLogs = await this.common.getLogsWithParams(
@@ -2531,10 +2544,11 @@ export class EthImpl implements Eth {
     if (blockResponse == null) return null;
     const timestampRange = blockResponse.timestamp;
     const timestampRangeParams = [`gte:${timestampRange.from}`, `lte:${timestampRange.to}`];
-    const contractResults = await this.mirrorNodeClient.getContractResults(
+
+    const contractResults = await this.mirrorNodeClient.getContractResultWithRetry(
+      this.mirrorNodeClient.getContractResults.name,
+      [requestDetails, { timestamp: timestampRangeParams }, undefined],
       requestDetails,
-      { timestamp: timestampRangeParams },
-      undefined,
     );
     const gasUsed = blockResponse.gas_used;
     const params = { timestamp: timestampRangeParams };

diff --git a/packages/relay/src/lib/services/debugService/index.ts b/packages/relay/src/lib/services/debugService/index.ts
@@ -18,18 +18,19 @@
  *
  */
 
+import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services';
 import type { Logger } from 'pino';
-import type { MirrorNodeClient } from '../../clients';
-import type { IDebugService } from './IDebugService';
-import type { CommonService } from '../ethService';
+
 import { decodeErrorMessage, mapKeysAndValues, numberTo0x, strip0x } from '../../../formatters';
+import type { MirrorNodeClient } from '../../clients';
+import { IOpcode } from '../../clients/models/IOpcode';
+import { IOpcodesResponse } from '../../clients/models/IOpcodesResponse';
 import constants, { CallType, TracerType } from '../../constants';
 import { predefined } from '../../errors/JsonRpcError';
 import { EthImpl } from '../../eth';
-import { IOpcodesResponse } from '../../clients/models/IOpcodesResponse';
-import { IOpcode } from '../../clients/models/IOpcode';
 import { ICallTracerConfig, IOpcodeLoggerConfig, ITracerConfig, RequestDetails } from '../../types';
-import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services';
+import type { CommonService } from '../ethService';
+import type { IDebugService } from './IDebugService';
 
 /**
  * Represents a DebugService for tracing and debugging transactions and debugging
@@ -300,7 +301,11 @@ export class DebugService implements IDebugService {
     try {
       const [actionsResponse, transactionsResponse] = await Promise.all([
         this.mirrorNodeClient.getContractsResultsActions(transactionHash, requestDetails),
-        this.mirrorNodeClient.getContractResultWithRetry(transactionHash, requestDetails),
+        this.mirrorNodeClient.getContractResultWithRetry(
+          this.mirrorNodeClient.getContractResult.name,
+          [transactionHash, requestDetails],
+          requestDetails,
+        ),
       ]);
       if (!actionsResponse || !transactionsResponse) {
         throw predefined.RESOURCE_NOT_FOUND(`Failed to retrieve contract results for transaction ${transactionHash}`);

diff --git a/packages/relay/src/lib/services/ethService/ethCommonService/index.ts b/packages/relay/src/lib/services/ethService/ethCommonService/index.ts
@@ -18,19 +18,21 @@
  *
  */
 
-import constants from '../../../constants';
-import { JsonRpcError, predefined } from '../../../errors/JsonRpcError';
-import { ICommonService } from './ICommonService';
+import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services';
+import * as _ from 'lodash';
 import { Logger } from 'pino';
-import { MirrorNodeClient } from '../../../clients';
+
 import { nullableNumberTo0x, numberTo0x, parseNumericEnvVar, toHash32 } from '../../../../formatters';
-import { SDKClientError } from '../../../errors/SDKClientError';
+import { MirrorNodeClient } from '../../../clients';
+import constants from '../../../constants';
+import { JsonRpcError, predefined } from '../../../errors/JsonRpcError';
 import { MirrorNodeClientError } from '../../../errors/MirrorNodeClientError';
+import { SDKClientError } from '../../../errors/SDKClientError';
+import { EthImpl } from '../../../eth';
 import { Log } from '../../../model';
-import * as _ from 'lodash';
-import { CacheService } from '../../cacheService/cacheService';
-import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services';
 import { RequestDetails } from '../../../types';
+import { CacheService } from '../../cacheService/cacheService';
+import { ICommonService } from './ICommonService';
 
 /**
  * Create a new Common Service implementation.
@@ -175,6 +177,20 @@ export class CommonService implements ICommonService {
     returnLatest?: boolean,
   ): Promise<any> {
     if (!returnLatest && this.blockTagIsLatestOrPending(blockNumberOrTagOrHash)) {
+      if (this.logger.isLevelEnabled('debug')) {
+        this.logger.debug(
+          `${requestDetails.formattedRequestId} Detected a contradiction between blockNumberOrTagOrHash and returnLatest. The request does not target the latest block, yet blockNumberOrTagOrHash representing latest or pending: returnLatest=${returnLatest}, blockNumberOrTagOrHash=${blockNumberOrTagOrHash}`,
+        );
+      }
+      return null;
+    }
+
+    if (blockNumberOrTagOrHash === EthImpl.emptyHex) {
+      if (this.logger.isLevelEnabled('debug')) {
+        this.logger.debug(
+          `${requestDetails.formattedRequestId} Invalid input detected in getHistoricalBlockResponse(): blockNumberOrTagOrHash=${blockNumberOrTagOrHash}.`,
+        );
+      }
       return null;
     }
 
@@ -321,7 +337,7 @@ export class CommonService implements ICommonService {
     if (address) {
       logResults = await this.getLogsByAddress(address, params, requestDetails);
     } else {
-      logResults = await this.mirrorNodeClient.getContractResultsLogs(requestDetails, params);
+      logResults = await this.mirrorNodeClient.getContractResultsLogsWithRetry(requestDetails, params);
     }
 
     if (!logResults) {
@@ -330,13 +346,28 @@ export class CommonService implements ICommonService {
 
     const logs: Log[] = [];
     for (const log of logResults) {
+      if (log.block_number == null || log.index == null || log.block_hash === EthImpl.emptyHex) {
+        if (this.logger.isLevelEnabled('debug')) {
+          this.logger.debug(
+            `${
+              requestDetails.formattedRequestId
+            } Log entry is missing required fields: block_number, index, or block_hash is an empty hex (0x). log=${JSON.stringify(
+              log,
+            )}`,
+          );
+        }
+        throw predefined.INTERNAL_ERROR(
+          `The log entry from the remote Mirror Node server is missing required fields. `,
+        );
+      }
+
       logs.push(
         new Log({
           address: log.address,
           blockHash: toHash32(log.block_hash),
           blockNumber: numberTo0x(log.block_number),
           data: log.data,
-          logIndex: nullableNumberTo0x(log.index),
+          logIndex: numberTo0x(log.index),
           removed: false,
           topics: log.topics,
           transactionHash: toHash32(log.transaction_hash),

diff --git a/packages/relay/src/receiptsRootUtils.ts b/packages/relay/src/receiptsRootUtils.ts
@@ -21,8 +21,9 @@
 import { RLP } from '@ethereumjs/rlp';
 import { Trie } from '@ethereumjs/trie';
 import { bytesToInt, concatBytes, hexToBytes, intToBytes, intToHex } from '@ethereumjs/util';
-import { EthImpl } from './lib/eth';
+
 import { prepend0x } from './formatters';
+import { EthImpl } from './lib/eth';
 import { Log } from './lib/model';
 import { LogsBloomUtils } from './logsBloomUtils';
 
@@ -93,16 +94,29 @@ export class ReceiptsRootUtils {
   public static buildReceiptRootHashes(txHashes: string[], contractResults: any[], logs: Log[]): IReceiptRootHash[] {
     const receipts: IReceiptRootHash[] = [];
 
-    for (let i in txHashes) {
+    for (const i in txHashes) {
       const txHash: string = txHashes[i];
       const logsPerTx: Log[] = logs.filter((log) => log.transactionHash == txHash);
       const crPerTx: any[] = contractResults.filter((cr) => cr.hash == txHash);
+
+      // Determine the transaction index for the current transaction hash:
+      // - Prefer the `transaction_index` from the contract results (`crPerTx`) if available.
+      // - Fallback to the `transactionIndex` from logs (`logsPerTx`) if no valid `transaction_index` is found in `crPerTx`.
+      // - If neither source provides a valid value, `transactionIndex` remains `null`.
+      let transactionIndex: any = null;
+      if (crPerTx.length && crPerTx[0].transaction_index != null) {
+        transactionIndex = intToHex(crPerTx[0].transaction_index);
+      } else if (logsPerTx.length) {
+        transactionIndex = logsPerTx[0].transactionIndex;
+      }
+
       receipts.push({
-        transactionIndex: crPerTx.length ? intToHex(crPerTx[0].transaction_index) : logsPerTx[0].transactionIndex,
+        transactionIndex,
         type: crPerTx.length && crPerTx[0].type ? intToHex(crPerTx[0].type) : null,
         root: crPerTx.length ? crPerTx[0].root : EthImpl.zeroHex32Byte,
         status: crPerTx.length ? crPerTx[0].status : EthImpl.oneHex,
-        cumulativeGasUsed: crPerTx.length ? intToHex(crPerTx[0].block_gas_used) : EthImpl.zeroHex,
+        cumulativeGasUsed:
+          crPerTx.length && crPerTx[0].block_gas_used ? intToHex(crPerTx[0].block_gas_used) : EthImpl.zeroHex,
         logsBloom: crPerTx.length
           ? crPerTx[0].bloom
           : LogsBloomUtils.buildLogsBloom(logs[0].address, logsPerTx[0].topics),