feat: removed fallback predefined gas value for estimate gas when failed to retrieve gas estimate from Mirror Node

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_ESTIMATE_GAS_PRICE: (errMessage: string) => {
+    return new JsonRpcError({
+      code: -32000,
+      message: `Error occurred during gas price estimation: ${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,20 @@ 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 && e.isContractRevert()) {
+        throw predefined.CONTRACT_REVERT(e.detail || e.message, e.data);
+      } else {
+        throw predefined.COULD_NOT_ESTIMATE_GAS_PRICE(e.detail || e.message);
       }
-      return this.predefinedGasForTransaction(transaction, requestDetails, e);
     }
   }
 
@@ -523,7 +519,7 @@ export class ContractService implements IContractService {
       return constants.EMPTY_HEX;
     }
 
-    if (e.isContractReverted()) {
+    if (e.isContractRevert()) {
       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}`,
@@ -555,68 +551,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/tests/lib/eth/eth_estimateGas.spec.ts

@@ -396,21 +396,6 @@ describe('@ethEstimateGas Estimate Gas spec', async function () {
     expect(estimatedGas).to.equal(numberTo0x(Precheck.transactionIntrinsicGasCost(transaction.data!)));
   });
 
-  withOverriddenEnvsInMochaTest({ ESTIMATE_GAS_THROWS: 'false' }, () => {
-    it('should eth_estimateGas with contract revert and message does not equal executionReverted and ESTIMATE_GAS_THROWS is set to false', async function () {
-      const originalEstimateGas = contractService.estimateGas;
-      contractService.estimateGas = async () => {
-        return numberTo0x(Precheck.transactionIntrinsicGasCost(transaction.data!));
-      };
-
-      const result = await ethImpl.estimateGas(transaction, id, requestDetails);
-
-      expect(result).to.equal(numberTo0x(Precheck.transactionIntrinsicGasCost(transaction.data!)));
-
-      contractService.estimateGas = originalEstimateGas;
-    });
-  });
-
   it('should eth_estimateGas with contract revert and message equals "execution reverted: Invalid number of recipients"', async function () {
     await mockContractCall(
       transaction,