From a5a0e1f69e4442e77700164887a377e83c62129e Mon Sep 17 00:00:00 2001 From: Logan Nguyen Date: Sun, 22 Dec 2024 14:49:14 -0600 Subject: [PATCH] fix: modified getContractResultWithRetry() to accept more general getContract MN methods Signed-off-by: Logan Nguyen --- .../relay/src/lib/clients/mirrorNodeClient.ts | 35 +++++++---- packages/relay/src/lib/eth.ts | 17 ++++-- .../src/lib/services/debugService/index.ts | 19 +++--- .../relay/tests/lib/mirrorNodeClient.spec.ts | 60 +++++++++++++------ 4 files changed, 93 insertions(+), 38 deletions(-) diff --git a/packages/relay/src/lib/clients/mirrorNodeClient.ts b/packages/relay/src/lib/clients/mirrorNodeClient.ts index bb0990ae8d..feeddb35c2 100644 --- a/packages/relay/src/lib/clients/mirrorNodeClient.ts +++ b/packages/relay/src/lib/clients/mirrorNodeClient.ts @@ -757,20 +757,35 @@ 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} - 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); + public async getContractResultWithRetry( + methodName: string, + args: any[], + requestDetails: RequestDetails, + ): Promise { + const shortDelay = 500; + const contractResult = await this[methodName](...args); + if ( contractResult && - !( - contractResult.transaction_index && - contractResult.block_number && - contractResult.block_hash != EthImpl.emptyHex - ) + (contractResult.transaction_index == null || + contractResult.block_number == null || + contractResult.block_hash == EthImpl.emptyHex) ) { - return this.getContractResult(transactionIdOrHash, requestDetails); + if (this.logger.isLevelEnabled('debug')) { + this.logger.debug( + `${requestDetails.formattedRequestId} Contract result contains undefined transaction_index, block_number, or block_hash set to 0x: transaction_hash:${contractResult.hash}, transaction_index:${contractResult.transaction_index}, block_number=${contractResult.block_number}, block_hash=${contractResult.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; } diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts index e3bc5f4a3b..6584ff9d82 100644 --- a/packages/relay/src/lib/eth.ts +++ b/packages/relay/src/lib/eth.ts @@ -1,8 +1,8 @@ -/* - +/*- * * Hedera JSON RPC Relay * - * Copyright (C) 2022-2024 Hedera Hashgraph, LLC + * Copyright (C) 2024 Hedera Hashgraph, LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -2201,7 +2201,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 +2269,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( diff --git a/packages/relay/src/lib/services/debugService/index.ts b/packages/relay/src/lib/services/debugService/index.ts index 5ec317110b..a92e16f3bd 100644 --- 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/tests/lib/mirrorNodeClient.spec.ts b/packages/relay/tests/lib/mirrorNodeClient.spec.ts index 9e9d2fda21..ea86aa75bb 100644 --- a/packages/relay/tests/lib/mirrorNodeClient.spec.ts +++ b/packages/relay/tests/lib/mirrorNodeClient.spec.ts @@ -19,23 +19,25 @@ */ import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services'; -import { expect } from 'chai'; -import { Registry } from 'prom-client'; -import { MirrorNodeClient } from '../../src/lib/clients'; -import constants from '../../src/lib/constants'; import axios, { AxiosInstance } from 'axios'; import MockAdapter from 'axios-mock-adapter'; -import { getRequestId, mockData, random20BytesAddress, withOverriddenEnvsInMochaTest } from '../helpers'; -import pino from 'pino'; +import { expect } from 'chai'; import { ethers } from 'ethers'; +import pino from 'pino'; +import { Registry } from 'prom-client'; + import { MirrorNodeClientError, predefined } from '../../src'; +import { MirrorNodeClient } from '../../src/lib/clients'; +import constants from '../../src/lib/constants'; import { CacheService } from '../../src/lib/services/cacheService/cacheService'; +import { getRequestId, mockData, random20BytesAddress, withOverriddenEnvsInMochaTest } from '../helpers'; const registry = new Registry(); -import { MirrorNodeTransactionRecord, RequestDetails } from '../../src/lib/types'; -import { SDKClientError } from '../../src/lib/errors/SDKClientError'; import { BigNumber } from 'bignumber.js'; +import { SDKClientError } from '../../src/lib/errors/SDKClientError'; +import { MirrorNodeTransactionRecord, RequestDetails } from '../../src/lib/types'; + const logger = pino(); const noTransactions = '?transactions=false'; const requestDetails = new RequestDetails({ requestId: getRequestId(), ipAddress: '0.0.0.0' }); @@ -83,7 +85,7 @@ describe('MirrorNodeClient', async function () { for (const code of nullResponseCodes) { it(`returns null when ${code} is returned`, async () => { - let error = new Error('test error'); + const error = new Error('test error'); error['response'] = 'test error'; const result = mirrorNodeInstance.handleError( @@ -101,7 +103,7 @@ describe('MirrorNodeClient', async function () { for (const code of errorRepsonseCodes) { it(`throws an error when ${code} is returned`, async () => { try { - let error = new Error('test error'); + const error = new Error('test error'); error['response'] = 'test error'; mirrorNodeInstance.handleError( error, @@ -568,7 +570,11 @@ describe('MirrorNodeClient', async function () { const hash = '0x4a563af33c4871b51a8b108aa2fe1dd5280a30dfb7236170ae5e5e7957eb6399'; mock.onGet(`contracts/results/${hash}`).reply(200, detailedContractResult); - const result = await mirrorNodeInstance.getContractResultWithRetry(hash, requestDetails); + const result = await mirrorNodeInstance.getContractResultWithRetry( + mirrorNodeInstance.getContractResult.name, + [hash, requestDetails], + requestDetails, + ); expect(result).to.exist; expect(result.contract_id).equal(detailedContractResult.contract_id); expect(result.to).equal(detailedContractResult.to); @@ -585,7 +591,11 @@ describe('MirrorNodeClient', async function () { mock.onGet(`contracts/results/${hash}`).replyOnce(200, { ...detailedContractResult, transaction_index: undefined }); mock.onGet(`contracts/results/${hash}`).reply(200, detailedContractResult); - const result = await mirrorNodeInstance.getContractResultWithRetry(hash, requestDetails); + const result = await mirrorNodeInstance.getContractResultWithRetry( + mirrorNodeInstance.getContractResult.name, + [hash, requestDetails], + requestDetails, + ); expect(result).to.exist; expect(result.contract_id).equal(detailedContractResult.contract_id); expect(result.to).equal(detailedContractResult.to); @@ -601,7 +611,11 @@ describe('MirrorNodeClient', async function () { .replyOnce(200, { ...detailedContractResult, transaction_index: undefined, block_number: undefined }); mock.onGet(`contracts/results/${hash}`).reply(200, detailedContractResult); - const result = await mirrorNodeInstance.getContractResultWithRetry(hash, requestDetails); + const result = await mirrorNodeInstance.getContractResultWithRetry( + mirrorNodeInstance.getContractResult.name, + [hash, requestDetails], + requestDetails, + ); expect(result).to.exist; expect(result.contract_id).equal(detailedContractResult.contract_id); expect(result.to).equal(detailedContractResult.to); @@ -616,7 +630,11 @@ describe('MirrorNodeClient', async function () { mock.onGet(`contracts/results/${hash}`).replyOnce(200, { ...detailedContractResult, block_number: undefined }); mock.onGet(`contracts/results/${hash}`).reply(200, detailedContractResult); - const result = await mirrorNodeInstance.getContractResultWithRetry(hash, requestDetails); + const result = await mirrorNodeInstance.getContractResultWithRetry( + mirrorNodeInstance.getContractResult.name, + [hash, requestDetails], + requestDetails, + ); expect(result).to.exist; expect(result.contract_id).equal(detailedContractResult.contract_id); expect(result.to).equal(detailedContractResult.to); @@ -630,7 +648,11 @@ describe('MirrorNodeClient', async function () { mock.onGet(`contracts/results/${hash}`).replyOnce(200, { ...detailedContractResult, block_hash: '0x' }); mock.onGet(`contracts/results/${hash}`).reply(200, detailedContractResult); - const result = await mirrorNodeInstance.getContractResultWithRetry(hash, requestDetails); + const result = await mirrorNodeInstance.getContractResultWithRetry( + mirrorNodeInstance.getContractResult.name, + [hash, requestDetails], + requestDetails, + ); expect(result).to.exist; expect(result.block_hash).equal(detailedContractResult.block_hash); expect(mock.history.get.length).to.eq(2); @@ -646,7 +668,11 @@ describe('MirrorNodeClient', async function () { }); mock.onGet(`contracts/results/${hash}`).reply(200, detailedContractResult); - const result = await mirrorNodeInstance.getContractResultWithRetry(hash, requestDetails); + const result = await mirrorNodeInstance.getContractResultWithRetry( + mirrorNodeInstance.getContractResult.name, + [hash, requestDetails], + requestDetails, + ); expect(result).to.exist; expect(result.transaction_index).equal(detailedContractResult.transaction_index); expect(result.block_number).equal(detailedContractResult.block_number); @@ -1558,7 +1584,7 @@ describe('MirrorNodeClient', async function () { it('should fetch contract for existing contract from cache on additional calls', async () => { mock.onGet(contractPath).reply(200, mockData.contract); - let id = await mirrorNodeInstance.getContractId(evmAddress, requestDetails); + const id = await mirrorNodeInstance.getContractId(evmAddress, requestDetails); expect(id).to.exist; expect(id).to.be.equal(mockData.contract.contract_id);