fix: Adds retry to send raw transaction (#3161) (#3208)

* Adds retry to eth send raw transaction * Adds mirror node check before retry and expands SDKClientError object to include transactionId * Adds unit tests * Adds improvements * adds relevant comment for hardcoded values * removes retry and adds mirror node check in the sdk client * Adds unit tests * Addresses PR comments * Changes the error thrown when timeout occurs to SDKClientError * Ensure other error types are thrown --------- Signed-off-by: Konstantina Blazhukova <[email protected]> Signed-off-by: Eric Badiere <[email protected]> Co-authored-by: konstantinabl <[email protected]>
hashgraph · Nov 4, 2024 · 01c893a · 01c893a
1 parent 90bf36a
commit 01c893a
Show file tree

Hide file tree

Showing 6 changed files with 158 additions and 16 deletions.
diff --git a/packages/relay/src/lib/clients/sdkClient.ts b/packages/relay/src/lib/clients/sdkClient.ts
@@ -69,6 +69,8 @@ import {
   ITransactionRecordMetric,
   RequestDetails,
 } from '../types';
+import { MirrorNodeClient } from './mirrorNodeClient';
+import { Registry } from 'prom-client';
 
 const _ = require('lodash');
 
@@ -122,6 +124,14 @@ export class SDKClient {
    */
   private readonly hbarLimitService: HbarLimitService;
 
+  /**
+   * An instance of the MirrorNodeClient
+   * @private
+   * @readonly
+   * @type {MirrorNodeClient}
+   */
+  private readonly mirrorNodeClient: MirrorNodeClient;
+
   /**
    * Constructs an instance of the SDKClient and initializes various services and settings.
    *
@@ -136,6 +146,7 @@ export class SDKClient {
     cacheService: CacheService,
     eventEmitter: EventEmitter,
     hbarLimitService: HbarLimitService,
+    register: Registry,
   ) {
     this.clientMain = clientMain;
 
@@ -151,6 +162,13 @@ export class SDKClient {
     this.hbarLimitService = hbarLimitService;
     this.maxChunks = Number(ConfigService.get('FILE_APPEND_MAX_CHUNKS')) || 20;
     this.fileAppendChunkSize = Number(ConfigService.get('FILE_APPEND_CHUNK_SIZE')) || 5120;
+    this.mirrorNodeClient = new MirrorNodeClient(
+      // @ts-ignore
+      ConfigService.get('MIRROR_NODE_URL') || '',
+      logger,
+      register,
+      cacheService,
+    );
   }
 
   /**
@@ -441,19 +459,61 @@ export class SDKClient {
       Hbar.fromTinybars(Math.floor(networkGasPriceInTinyBars * constants.MAX_GAS_PER_SEC)),
     );
 
-    return {
-      fileId,
-      txResponse: await this.executeTransaction(
+    let txResponse;
+    try {
+      txResponse = await this.executeTransaction(
         ethereumTransaction,
         callerName,
         interactingEntity,
         requestDetails,
         true,
         originalCallerAddress,
-      ),
+      );
+    } catch (e: any) {
+      if (e instanceof SDKClientError && (e.isConnectionDropped() || e.isTimeoutExceeded())) {
+        const isFailed = await this.isFailedTransaction(requestDetails, e.transactionId);
+        if (isFailed) {
+          throw e;
+        }
+        txResponse = { transactionId: e.transactionId };
+      } else {
+        throw e;
+      }
+    }
+
+    return {
+      fileId,
+      txResponse,
     };
   }
 
+  /**
+   * Checks if a transaction has failed by querying the mirror node.
+   *
+   * @param {RequestDetails} requestDetails - The details of the request.
+   * @param {string} [transactionId] - The ID of the transaction to check.
+   * @returns {Promise<boolean>} - A promise that resolves to `true` if the transaction has failed, `false` otherwise.
+   */
+  async isFailedTransaction(requestDetails: RequestDetails, transactionId?: string): Promise<boolean> {
+    const retryCount = 5;
+    try {
+      const transaction = await this.mirrorNodeClient.repeatedRequest(
+        this.mirrorNodeClient.getTransactionById.name,
+        [transactionId, requestDetails],
+        retryCount,
+        requestDetails,
+      );
+      const isFailed = transaction !== null;
+      return isFailed;
+    } catch (e: any) {
+      this.logger.error(
+        e,
+        `${requestDetails.formattedRequestId} Failed to check if transaction ${transactionId} is failed`,
+      );
+      return true;
+    }
+  }
+
   /**
    * Submits a contract call query to a smart contract on the Hedera network.
    * @param {string} to - The address of the contract to call, in either Solidity or EVM format.
@@ -724,7 +784,7 @@ export class SDKClient {
         throw e;
       }
 
-      const sdkClientError = new SDKClientError(e, e.message);
+      const sdkClientError = new SDKClientError(e, e.message, transaction.transactionId?.toString());
 
       // Throw WRONG_NONCE error as more error handling logic for WRONG_NONCE is awaited in eth.sendRawTransactionErrorHandler().
       if (sdkClientError.status && sdkClientError.status === Status.WrongNonce) {
@@ -737,9 +797,13 @@ export class SDKClient {
       );
 
       if (!transactionResponse) {
-        throw predefined.INTERNAL_ERROR(
-          `${requestDetails.formattedRequestId} Transaction execution returns a null value: transactionId=${transaction.transactionId}, callerName=${callerName}, txConstructorName=${txConstructorName}`,
-        );
+        if (sdkClientError.isConnectionDropped() || sdkClientError.isTimeoutExceeded()) {
+          throw sdkClientError;
+        } else {
+          throw predefined.INTERNAL_ERROR(
+            `${requestDetails.formattedRequestId} Transaction execution returns a null value: transactionId=${transaction.transactionId}, callerName=${callerName}, txConstructorName=${txConstructorName}`,
+          );
+        }
       }
       return transactionResponse;
     } finally {

diff --git a/packages/relay/src/lib/errors/SDKClientError.ts b/packages/relay/src/lib/errors/SDKClientError.ts
@@ -23,22 +23,27 @@ import { Status } from '@hashgraph/sdk';
 export class SDKClientError extends Error {
   public status: Status = Status.Unknown;
   private validNetworkError: boolean = false;
+  private failedTransactionId: string | undefined;
 
-  constructor(e: any, message?: string) {
+  constructor(e: any, message?: string, transactionId?: string) {
     super(e?.status?._code ? e.message : message);
 
     if (e?.status?._code) {
       this.validNetworkError = true;
       this.status = e.status;
     }
-
+    this.failedTransactionId = transactionId || '';
     Object.setPrototypeOf(this, SDKClientError.prototype);
   }
 
   get statusCode(): number {
     return this.status._code;
   }
 
+  get transactionId(): string | undefined {
+    return this.failedTransactionId;
+  }
+
   public isValidNetworkError(): boolean {
     return this.validNetworkError;
   }

diff --git a/packages/relay/src/lib/services/hapiService/hapiService.ts b/packages/relay/src/lib/services/hapiService/hapiService.ts
@@ -208,7 +208,7 @@ export default class HAPIService {
     this.clientMain = this.initClient(logger, this.hederaNetwork);
 
     this.cacheService = cacheService;
-    this.client = this.initSDKClient(logger);
+    this.client = this.initSDKClient(logger, register);
 
     const currentDateNow = Date.now();
     // @ts-ignore
@@ -287,7 +287,7 @@ export default class HAPIService {
       .inc(1);
 
     this.clientMain = this.initClient(this.logger, this.hederaNetwork);
-    this.client = this.initSDKClient(this.logger);
+    this.client = this.initSDKClient(this.logger, this.register);
     this.resetCounters();
   }
 
@@ -306,13 +306,14 @@ export default class HAPIService {
    * @param {Logger} logger
    * @returns SDK Client
    */
-  private initSDKClient(logger: Logger): SDKClient {
+  private initSDKClient(logger: Logger, register: Registry): SDKClient {
     return new SDKClient(
       this.clientMain,
       logger.child({ name: `consensus-node` }),
       this.cacheService,
       this.eventEmitter,
       this.hbarLimitService,
+      register,
     );
   }
 

diff --git a/packages/relay/tests/lib/eth/eth_sendRawTransaction.spec.ts b/packages/relay/tests/lib/eth/eth_sendRawTransaction.spec.ts
@@ -35,23 +35,35 @@ import {
 import { HbarLimitService } from '../../../src/lib/services/hbarLimitService';
 import { EventEmitter } from 'events';
 import pino from 'pino';
-import { SDKClient } from '../../../src/lib/clients';
+import { MirrorNodeClient, SDKClient } from '../../../src/lib/clients';
 import { ACCOUNT_ADDRESS_1, DEFAULT_NETWORK_FEES, MAX_GAS_LIMIT_HEX, NO_TRANSACTIONS } from './eth-config';
-import { JsonRpcError, predefined } from '../../../src';
+import { Eth, JsonRpcError, predefined } from '../../../src';
 import RelayAssertions from '../../assertions';
 import { getRequestId, mockData, overrideEnvsInMochaDescribe, signTransaction } from '../../helpers';
 import { generateEthTestEnv } from './eth-helpers';
 import { SDKClientError } from '../../../src/lib/errors/SDKClientError';
 import { RequestDetails } from '../../../src/lib/types';
+import MockAdapter from 'axios-mock-adapter';
+import HAPIService from '../../../src/lib/services/hapiService/hapiService';
+import { CacheService } from '../../../src/lib/services/cacheService/cacheService';
+import * as utils from '../../../src/formatters';
 
 use(chaiAsPromised);
 
 let sdkClientStub: sinon.SinonStubbedInstance<SDKClient>;
+let mirrorNodeStub: sinon.SinonStubbedInstance<MirrorNodeClient>;
 let getSdkClientStub: sinon.SinonStub;
+let formatTransactionIdWithoutQueryParamsStub: sinon.SinonStub;
 
 describe('@ethSendRawTransaction eth_sendRawTransaction spec', async function () {
   this.timeout(10000);
-  let { restMock, hapiServiceInstance, ethImpl, cacheService } = generateEthTestEnv();
+  const {
+    restMock,
+    hapiServiceInstance,
+    ethImpl,
+    cacheService,
+  }: { restMock: MockAdapter; hapiServiceInstance: HAPIService; ethImpl: Eth; cacheService: CacheService } =
+    generateEthTestEnv();
 
   const requestDetails = new RequestDetails({ requestId: 'eth_sendRawTransactionTest', ipAddress: '0.0.0.0' });
 
@@ -62,6 +74,7 @@ describe('@ethSendRawTransaction eth_sendRawTransaction spec', async function ()
     await cacheService.clear(requestDetails);
     restMock.reset();
     sdkClientStub = sinon.createStubInstance(SDKClient);
+    mirrorNodeStub = sinon.createStubInstance(MirrorNodeClient);
     getSdkClientStub = sinon.stub(hapiServiceInstance, 'getSDKClient').returns(sdkClientStub);
     restMock.onGet('network/fees').reply(200, DEFAULT_NETWORK_FEES);
   });
@@ -281,5 +294,62 @@ describe('@ethSendRawTransaction eth_sendRawTransaction spec', async function ()
         [signed, getRequestId()],
       );
     });
+
+    it('should call mirror node upon time out and return successful if found', async function () {
+      const transactionId = '0.0.902';
+      const contractResultEndpoint = `contracts/results/${transactionId}`;
+      formatTransactionIdWithoutQueryParamsStub = sinon.stub(utils, 'formatTransactionIdWithoutQueryParams');
+      formatTransactionIdWithoutQueryParamsStub.returns(transactionId);
+
+      restMock.onGet(contractResultEndpoint).reply(200, { hash: ethereumHash });
+
+      sdkClientStub.submitEthereumTransaction.restore();
+      mirrorNodeStub.repeatedRequest = sinon.stub();
+      mirrorNodeStub.getTransactionById = sinon.stub();
+      sdkClientStub.deleteFile.resolves();
+      sdkClientStub.createFile.resolves(new FileId(0, 0, 5644));
+      sdkClientStub.executeTransaction
+        .onCall(0)
+        .throws(new SDKClientError({ status: 21 }, 'timeout exceeded', transactionId));
+      sdkClientStub.isFailedTransaction.resolves(false);
+      const signed = await signTransaction(transaction);
+
+      const resultingHash = await ethImpl.sendRawTransaction(signed, requestDetails);
+      expect(resultingHash).to.equal(ethereumHash);
+    });
+
+    it('should call mirror node upon time out and throw error if not found', async function () {
+      sdkClientStub.submitEthereumTransaction.restore();
+      mirrorNodeStub.repeatedRequest = sinon.stub();
+      mirrorNodeStub.getTransactionById = sinon.stub();
+
+      sdkClientStub.createFile.resolves(new FileId(0, 0, 5644));
+      sdkClientStub.executeTransaction.onCall(0).throws(new SDKClientError({ status: 21 }, 'timeout exceeded'));
+      sdkClientStub.isFailedTransaction.resolves(true);
+      const signed = await signTransaction(transaction);
+
+      const response = (await ethImpl.sendRawTransaction(signed, requestDetails)) as JsonRpcError;
+      console.log(response);
+      expect(response).to.be.instanceOf(JsonRpcError);
+      expect(response.message).to.include('timeout exceeded');
+      sinon.assert.called(sdkClientStub.isFailedTransaction);
+    });
+
+    it('should call mirror node upon connection dropped and throw error if not found', async function () {
+      sdkClientStub.submitEthereumTransaction.restore();
+      mirrorNodeStub.repeatedRequest = sinon.stub();
+      mirrorNodeStub.getTransactionById = sinon.stub();
+
+      sdkClientStub.createFile.resolves(new FileId(0, 0, 5644));
+      sdkClientStub.executeTransaction.onCall(0).throws(new SDKClientError({ status: 21 }, 'Connection dropped'));
+      sdkClientStub.isFailedTransaction.resolves(true);
+      const signed = await signTransaction(transaction);
+
+      const response = (await ethImpl.sendRawTransaction(signed, requestDetails)) as JsonRpcError;
+      console.log(response);
+      expect(response).to.be.instanceOf(JsonRpcError);
+      expect(response.message).to.include('Connection dropped');
+      sinon.assert.called(sdkClientStub.isFailedTransaction);
+    });
   });
 });
diff --git a/packages/relay/tests/lib/sdkClient.spec.ts b/packages/relay/tests/lib/sdkClient.spec.ts
@@ -137,6 +137,7 @@ describe('SdkClient', async function () {
       new CacheService(logger.child({ name: `cache` }), registry),
       eventEmitter,
       hbarLimitService,
+      register,
     );
 
     instance = axios.create({

diff --git a/packages/relay/tests/lib/services/metricService/metricService.spec.ts b/packages/relay/tests/lib/services/metricService/metricService.spec.ts
@@ -163,6 +163,7 @@ describe('Metric Service', function () {
       new CacheService(logger.child({ name: `cache` }), registry),
       eventEmitter,
       hbarLimitService,
+      register,
     );
     // Init new MetricService instance
     metricService = new MetricService(logger, sdkClient, mirrorNodeClient, registry, eventEmitter, hbarLimitService);