feat: removed fallback predefined gas value for eth_estimateGas when failed to retrieve gas estimate from Mirror Node (#4678)

Signed-off-by: Logan Nguyen <[email protected]>

Author: quiet-node

.env.http.example

@@ -22,7 +22,6 @@ OPERATOR_KEY_MAIN=               # Operator private key used to sign transaction
 # DEBUG_API_ENABLED=false        # Enables debug methods like debug_traceTransaction
 # TXPOOL_API_ENABLED=true        # Enables txpool related methods
 # FILTER_API_ENABLED=true        # Enables filter related methods
-# ESTIMATE_GAS_THROWS=true       # If true, throws actual error reason during contract reverts
 
 # ========== BATCH REQUESTS ==========
 # BATCH_REQUESTS_ENABLED=true    # Enable or disable batch requests

charts/hedera-json-rpc-relay/values.yaml

@@ -31,7 +31,6 @@ config:
   # DEBUG_API_ENABLED:
   # TXPOOL_API_ENABLED:
   # FILTER_API_ENABLED:
-  # ESTIMATE_GAS_THROWS:
   # SUBSCRIPTIONS_ENABLED:
 
   # ========== BATCH REQUESTS ==========

docs/configuration.md

@@ -38,7 +38,6 @@ Unless you need to set a non-default value, it is recommended to only populate o
 | `DEBUG_API_ENABLED`                         | "false"                               | Enables all debug related methods: `debug_traceTransaction`                                                                                                                                                                                                                                                                                                                                                                                                                                |
 | `DEFAULT_RATE_LIMIT`                        | "200"                                 | default fallback rate limit, if no other is configured.                                                                                                                                                                                                                                                                                                                                                                                                                                    |
 | `ENABLE_TX_POOL`                            | "false"                               | Flag to determine if the system should use a tx pool or not.                                                                                                                                                                                                                                                                                                                                                                                                                               |
-| `ESTIMATE_GAS_THROWS`                       | "true"                                | Flag to determine if the system should throw an error with the actual reason during contract reverts instead of returning a predefined gas value.                                                                                                                                                                                                                                                                                                                                          |
 | `ETH_BLOCK_NUMBER_CACHE_TTL_MS`             | "1000"                                | Time in ms to cache response from mirror node                                                                                                                                                                                                                                                                                                                                                                                                                                              |
 | `ETH_CALL_ACCEPTED_ERRORS`                  | "[]"                                  | A list of acceptable error codes for eth_call requests. If an error code in this list is returned, the request will be retried.                                                                                                                                                                                                                                                                                                                                                            |
 | `ETH_CALL_CACHE_TTL`                        | "200"                                 | Maximum time in ms to cache an eth_call response.                                                                                                                                                                                                                                                                                                                                                                                                                                          |

packages/config-service/src/services/globalConfig.ts

@@ -173,11 +173,6 @@ const _CONFIG = {
     required: false,
     defaultValue: false,
   },
-  ESTIMATE_GAS_THROWS: {
-    type: 'boolean',
-    required: false,
-    defaultValue: true,
-  },
   ETH_BLOCK_NUMBER_CACHE_TTL_MS: {
     type: 'number',
     required: false,

packages/relay/src/lib/errors/JsonRpcError.ts

@@ -196,10 +196,12 @@ export const predefined = {
       message: message,
     });
   },
-  COULD_NOT_ESTIMATE_GAS_PRICE: new JsonRpcError({
-    code: -32604,
-    message: 'Error encountered estimating the gas price',
-  }),
+  COULD_NOT_SIMULATE_TRANSACTION: (errMessage: string) => {
+    return new JsonRpcError({
+      code: -32000,
+      message: `Error occurred during transaction simulation: ${errMessage}`,
+    });
+  },
   COULD_NOT_RETRIEVE_LATEST_BLOCK: new JsonRpcError({
     code: -32607,
     message: 'Error encountered retrieving latest block',

packages/relay/src/lib/errors/MirrorNodeClientError.ts

@@ -9,7 +9,6 @@ export class MirrorNodeClientError extends Error {
 
   static ErrorCodes = {
     ECONNABORTED: 504,
-    CONTRACT_REVERT_EXECUTED: 400,
     NOT_SUPPORTED: 501,
   };
 
@@ -45,11 +44,7 @@ export class MirrorNodeClientError extends Error {
     return this.statusCode === MirrorNodeClientError.ErrorCodes.ECONNABORTED;
   }
 
-  public isContractReverted(): boolean {
-    return this.statusCode === MirrorNodeClientError.ErrorCodes.CONTRACT_REVERT_EXECUTED;
-  }
-
-  public isContractRevertOpcodeExecuted() {
+  public isContractRevert() {
     return this.message === MirrorNodeClientError.messages.CONTRACT_REVERT_EXECUTED;
   }
 

packages/relay/src/lib/services/ethService/contractService/ContractService.ts

@@ -167,24 +167,26 @@ export class ContractService implements IContractService {
     try {
       const response = await this.estimateGasFromMirrorNode(transaction, requestDetails);
 
-      if (response?.result) {
-        this.logger.info(`Returning gas: ${response.result}`);
-        return prepend0x(trimPrecedingZeros(response.result));
-      } else {
-        this.logger.error(`No gas estimate returned from mirror-node: ${JSON.stringify(response)}`);
-        return this.predefinedGasForTransaction(transaction, requestDetails);
+      if (!response?.result) {
+        if (this.logger.isLevelEnabled('debug')) {
+          this.logger.debug(`No gas estimate returned from mirror-node: ${JSON.stringify(response)}`);
+        }
+        return predefined.INTERNAL_ERROR('Fail to retrieve gas estimate');
       }
+
+      return prepend0x(trimPrecedingZeros(response.result));
     } catch (e: any) {
-      this.logger.error(`Error raised while fetching estimateGas from mirror-node: ${JSON.stringify(e)}`);
-      // in case of contract revert, we don't want to return a predefined gas but the actual error with the reason
-      if (
-        ConfigService.get('ESTIMATE_GAS_THROWS') &&
-        e instanceof MirrorNodeClientError &&
-        e.isContractRevertOpcodeExecuted()
-      ) {
-        return predefined.CONTRACT_REVERT(e.detail ?? e.message, e.data);
+      if (e instanceof MirrorNodeClientError) {
+        if (e.isContractRevert()) {
+          throw predefined.CONTRACT_REVERT(e.detail || e.message, e.data);
+        } else if (e.statusCode === 400) {
+          throw predefined.COULD_NOT_SIMULATE_TRANSACTION(e.detail || e.message);
+        }
       }
-      return this.predefinedGasForTransaction(transaction, requestDetails, e);
+
+      // for any other error or Mirror Node upstream server errors (429, 500, 502, 503, 504, etc.),
+      // preserve the original error and re-throw to the upper layer for further handling logic
+      throw e;
     }
   }
 
@@ -523,15 +525,14 @@ export class ContractService implements IContractService {
       return constants.EMPTY_HEX;
     }
 
-    if (e.isContractReverted()) {
-      if (this.logger.isLevelEnabled('trace')) {
-        this.logger.trace(
-          `mirror node eth_call request encountered contract revert. message: ${e.message}, details: ${e.detail}, data: ${e.data}`,
-        );
-      }
-      return predefined.CONTRACT_REVERT(e.detail || e.message, e.data);
+    if (e.isContractRevert()) {
+      throw predefined.CONTRACT_REVERT(e.detail || e.message, e.data);
+    } else if (e.statusCode === 400) {
+      throw predefined.COULD_NOT_SIMULATE_TRANSACTION(e.detail || e.message);
     }
-    // for any other Mirror Node upstream server errors (429, 500, 502, 503, 504, etc.), preserve the original error and re-throw to the upper layer
+
+    // for any other error or Mirror Node upstream server errors (429, 500, 502, 503, 504, etc.),
+    // preserve the original error and re-throw to the upper layer for further handling logic
     throw e;
   }
 
@@ -555,68 +556,6 @@ export class ContractService implements IContractService {
     return predefined.INTERNAL_ERROR(e.message.toString());
   }
 
-  /**
-   * Fallback calculations for the amount of gas to be used for a transaction.
-   * This method is used when the mirror node fails to return a gas estimate.
-   *
-   * @param {IContractCallRequest} transaction The transaction data for the contract call.
-   * @param {RequestDetails} requestDetails The request details for logging and tracking.
-   * @param error (Optional) received error from the mirror-node contract call request.
-   * @returns {Promise<string | JsonRpcError>} the calculated gas cost for the transaction
-   */
-  private async predefinedGasForTransaction(
-    transaction: IContractCallRequest,
-    requestDetails: RequestDetails,
-    error?: any,
-  ): Promise<string | JsonRpcError> {
-    const isSimpleTransfer = !!transaction?.to && (!transaction.data || transaction.data === '0x');
-    const isContractCall =
-      !!transaction?.to && transaction?.data && transaction.data.length >= constants.FUNCTION_SELECTOR_CHAR_LENGTH;
-    const isContractCreate = !transaction?.to && transaction?.data && transaction.data !== '0x';
-    const contractCallAverageGas = numberTo0x(constants.TX_CONTRACT_CALL_AVERAGE_GAS);
-    const gasTxBaseCost = numberTo0x(constants.TX_BASE_COST);
-
-    if (isSimpleTransfer) {
-      // Handle Simple Transaction and Hollow Account creation
-      const isZeroOrHigher = Number(transaction.value) >= 0;
-      if (!isZeroOrHigher) {
-        return predefined.INVALID_PARAMETER(
-          0,
-          `Invalid 'value' field in transaction param. Value must be greater than or equal to 0`,
-        );
-      }
-      // when account exists return default base gas
-      if (await this.common.getAccount(transaction.to!, requestDetails)) {
-        this.logger.warn(`Returning predefined gas for simple transfer: ${gasTxBaseCost}`);
-        return gasTxBaseCost;
-      }
-      const minGasTxHollowAccountCreation = numberTo0x(constants.MIN_TX_HOLLOW_ACCOUNT_CREATION_GAS);
-      // otherwise, return the minimum amount of gas for hollow account creation
-      this.logger.warn(`Returning predefined gas for hollow account creation: ${minGasTxHollowAccountCreation}`);
-      return minGasTxHollowAccountCreation;
-    } else if (isContractCreate) {
-      // The size limit of the encoded contract posted to the mirror node can
-      // cause contract deployment transactions to fail with a 400 response code.
-      // The contract is actually deployed on the consensus node, so the contract will work.
-      // In these cases, we don't want to return a CONTRACT_REVERT error.
-      if (
-        ConfigService.get('ESTIMATE_GAS_THROWS') &&
-        error?.isContractReverted() &&
-        error?.message !== MirrorNodeClientError.messages.INVALID_HEX
-      ) {
-        return predefined.CONTRACT_REVERT(error.detail, error.data);
-      }
-      this.logger.warn(`Returning predefined gas for contract creation: ${gasTxBaseCost}`);
-      return numberTo0x(Precheck.transactionIntrinsicGasCost(transaction.data!));
-    } else if (isContractCall) {
-      this.logger.warn(`Returning predefined gas for contract call: ${contractCallAverageGas}`);
-      return contractCallAverageGas;
-    } else {
-      this.logger.warn(`Returning predefined gas for unknown transaction: ${this.defaultGas}`);
-      return this.defaultGas;
-    }
-  }
-
   /**
    * Prepares the call data for mirror node request.
    *

packages/relay/src/lib/services/ethService/ethCommonService/CommonService.ts

@@ -458,7 +458,7 @@ export class CommonService implements ICommonService {
       }
     }
 
-    throw predefined.COULD_NOT_ESTIMATE_GAS_PRICE;
+    throw predefined.INTERNAL_ERROR('Failed to retrieve gas price from network fees');
   }
 
   /**

packages/relay/tests/helpers.ts

@@ -351,6 +351,58 @@ const mockData = {
       ],
     },
   },
+
+  genericBadRequest: {
+    _status: {
+      messages: [
+        {
+          message: 'Bad request',
+          detail: 'Invalid request parameters',
+          data: '',
+        },
+      ],
+    },
+  },
+
+  internalServerError: {
+    _status: {
+      messages: [
+        {
+          message: 'Internal Server Error',
+        },
+      ],
+    },
+  },
+
+  badGateway: {
+    _status: {
+      messages: [
+        {
+          message: 'Bad Gateway',
+        },
+      ],
+    },
+  },
+
+  serviceUnavailable: {
+    _status: {
+      messages: [
+        {
+          message: 'Service Unavailable',
+        },
+      ],
+    },
+  },
+
+  gatewayTimeout: {
+    _status: {
+      messages: [
+        {
+          message: 'Gateway Timeout',
+        },
+      ],
+    },
+  },
 };
 
 export { expectUnsupportedMethod, expectedError, signTransaction, mockData, random20BytesAddress, getQueryParams };