Further refactor to extract the handling of the method on the class, …

…also added tests and improve on the logging. Signed-off-by: Alfredo Gutierrez <alfredo@swirldslabs.com>
hashgraph · Jun 16, 2023 · bf27860 · bf27860
1 parent cb54cda
commit bf27860
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 12 deletions.
diff --git a/packages/relay/src/lib/clients/mirrorNodeClient.ts b/packages/relay/src/lib/clients/mirrorNodeClient.ts
@@ -321,7 +321,8 @@ export class MirrorNodeClient {
         this.logger.error(new Error(error.message), `${requestIdPrefix} [${method}] ${path} ${effectiveStatusCode} status`);
 
         // we only need contract revert errors here as it's not the same as not supported
-        if (mirrorError.isContractReverted() && !mirrorError.isNotSupported() && !mirrorError.isNotSupportedSystemContractOperaton()) {
+        // Contract Revert error is valid only for contract calls
+        if (pathLabel == MirrorNodeClient.CONTRACT_CALL_ENDPOINT && mirrorError.isContractReverted() && !mirrorError.isNotSupported() && !mirrorError.isNotSupportedSystemContractOperaton()) {
             throw predefined.CONTRACT_REVERT(mirrorError.errorMessage);
         }
 

diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts
@@ -493,15 +493,7 @@ export class EthImpl implements Eth {
           const accountCacheKey = `${constants.CACHE_KEY.ACCOUNT}_${transaction.to}`;
           let toAccount: object | null = this.cache.get(accountCacheKey);
           if (!toAccount) {
-            try {
-              toAccount = await this.mirrorNodeClient.getAccount(transaction.to, requestId);
-            } catch (error: any) {
-              if(error instanceof MirrorNodeClientError && error.statusCode == 404) {
-                toAccount = null;
-              } else {
-                throw error;
-              }
-            }
+            toAccount = await this.getAccountOrNull(transaction.to, requestId);
           }
 
           // when account exists return default base gas, otherwise return the minimum amount of gas to create an account entity
@@ -526,6 +518,31 @@ export class EthImpl implements Eth {
     return gas;
   }
 
+  /**
+   * Gets the account data using the mirror node and handles the potential 404 error and returns null instead.
+   * */
+    async getAccountOrNull(address: string, requestId?: string) {
+      let account;
+      try {
+        account = await this.mirrorNodeClient.getAccount(address, requestId);
+      } catch (error: any) {
+        if(error instanceof MirrorNodeClientError) {
+          if(error.statusCode == 404){
+            return null;
+
+          } else if(error.statusCode == 400){
+            this.logger.debug(`${formatRequestIdMessage(requestId)} Got Invalid Parameter when trying to fetch account from mirror node: ${JSON.stringify(error)}`);
+            throw predefined.INVALID_PARAMETER(address, `Invalid 'address' field in transaction param. Address must be a valid 20 bytes hex string`);
+          }
+        } else {
+          this.logger.error(`${formatRequestIdMessage(requestId)} Unexpected error raised while fetching account from mirror-node: ${JSON.stringify(error)}`);
+          throw error;
+        }
+      }
+
+      return account;
+    }
+
   /**
    * Gets the current gas price of the network.
    */
@@ -1090,7 +1107,7 @@ export class EthImpl implements Eth {
       return EthImpl.zeroHex;
     } else if (address && !blockNumOrTag) {
       // get latest ethereumNonce
-      const mirrorAccount = await this.mirrorNodeClient.getAccount(address, requestId);
+      const mirrorAccount = await this.getAccountOrNull(address, requestId);
       if (mirrorAccount?.ethereum_nonce) {
         return EthImpl.numberTo0x(mirrorAccount.ethereum_nonce);
       }
@@ -1413,7 +1430,7 @@ export class EthImpl implements Eth {
       const accountCacheKey = `${constants.CACHE_KEY.ACCOUNT}_${fromAddress}`;
       let accountResult: any | null = this.cache.get(accountCacheKey);
       if (!accountResult) {
-        accountResult = await this.mirrorNodeClient.getAccount(fromAddress, requestId);
+        accountResult = await this.getAccountOrNull(fromAddress, requestId);
         if (accountResult) {
           this.logger.trace(`${requestIdPrefix} caching ${accountCacheKey}:${JSON.stringify(accountResult)} for ${constants.CACHE_TTL.ONE_HOUR} ms`);
           this.cache.set(accountCacheKey, accountResult);

diff --git a/packages/relay/tests/lib/eth.spec.ts b/packages/relay/tests/lib/eth.spec.ts
@@ -3337,6 +3337,45 @@ describe('Eth calls using MirrorNode', async function () {
     }
   });
 
+  describe('getAccountOrNull helper', async function () {
+    it('getAccountOrNull with no account found', async function () {
+      restMock.onGet(`accounts/${contractAddress1}${limitOrderPostFix}`).reply(404);
+      const account = await ethImpl.getAccountOrNull(contractAddress1);
+      expect(account).to.be.null;
+    });
+
+    it('getAccountOrNull with account found', async function () {
+      restMock.onGet(`accounts/${contractAddress1}${limitOrderPostFix}`).replyOnce(200, defaultContract);
+      const account = await ethImpl.getAccountOrNull(contractAddress1);
+      expect(account).to.deep.equal(defaultContract);
+    });
+
+    it('getAccountOrNull with invalid parameter throws error', async function () {
+      const invalidAddress = "0x123";
+      restMock.onGet(`accounts/${invalidAddress}${limitOrderPostFix}`).reply(400);
+      let errorRaised = false;
+      try {
+        await ethImpl.getAccountOrNull(invalidAddress);
+      } catch (error: any) {
+        errorRaised = true;
+        expect(error.message).to.equal(`Invalid parameter ${invalidAddress}: Invalid 'address' field in transaction param. Address must be a valid 20 bytes hex string`);
+      }
+      expect(errorRaised).to.be.true;
+    });
+
+    it('getAccountOrNull with valid parameter throws unexpected error', async function () {
+        restMock.onGet(`accounts/${contractAddress1}${limitOrderPostFix}`).replyOnce(500, { error: 'unexpected error' });
+        let errorRaised = false;
+        try {
+            await ethImpl.getAccountOrNull(contractAddress1);
+        }
+        catch (error: any) {
+            errorRaised = true;
+            expect(error.message).to.equal(`Request failed with status code 500`);
+        }
+    });
+  });
+
   describe('eth_call precheck failures', async function () {
     let callConsensusNodeSpy: sinon.SinonSpy;
     let callMirrorNodeSpy: sinon.SinonSpy;