fix: eth_estimateGas opts to fallback estimation for contract revert (

#2834) * fix: `eth_estimateGas` opts to fallback estimation when a contract reverts Signed-off-by: Victor Yanev <victor.yanev@limechain.tech> * fix: `eth_estimateGas` opts to fallback estimation when a contract reverts Signed-off-by: Victor Yanev <victor.yanev@limechain.tech> * fix: `eth_estimateGas` opts to fallback estimation when a contract reverts Signed-off-by: Victor Yanev <victor.yanev@limechain.tech> * test: extend tests in eth_estimateGas.spec.ts to cover the new logic Signed-off-by: Victor Yanev <victor.yanev@limechain.tech> * test: fix failing postman tests due to missing 'from' field Signed-off-by: Victor Yanev <victor.yanev@limechain.tech> * fix: throw predefined.CONTRACT_REVERT only for CONTRACT_REVERT_EXECUTED error message Signed-off-by: Victor Yanev <victor.yanev@limechain.tech> * fix: use `Status.ContractRevertExecuted` from `@hashgraph/sdk` Signed-off-by: Victor Yanev <victor.yanev@limechain.tech> --------- Signed-off-by: Victor Yanev <victor.yanev@limechain.tech>
hashgraph · Aug 21, 2024 · 1c75656 · 1c75656
1 parent f66ba3e
commit 1c75656
Show file tree

Hide file tree

Showing 6 changed files with 167 additions and 6 deletions.
diff --git a/packages/relay/src/lib/errors/JsonRpcError.ts b/packages/relay/src/lib/errors/JsonRpcError.ts
@@ -33,12 +33,20 @@ export class JsonRpcError {
 }
 
 export const predefined = {
-  CONTRACT_REVERT: (errorMessage?: string, data: string = '') =>
-    new JsonRpcError({
+  CONTRACT_REVERT: (errorMessage?: string, data: string = '') => {
+    let message: string;
+    if (errorMessage?.length) {
+      message = `execution reverted: ${decodeErrorMessage(errorMessage)}`;
+    } else {
+      const decodedData = decodeErrorMessage(data);
+      message = decodedData.length ? `execution reverted: ${decodedData}` : 'execution reverted';
+    }
+    return new JsonRpcError({
       code: 3,
-      message: `execution reverted: ${decodeErrorMessage(errorMessage)}`,
-      data: data,
-    }),
+      message,
+      data,
+    });
+  },
   GAS_LIMIT_TOO_HIGH: (gasLimit, maxGas) =>
     new JsonRpcError({
       code: -32005,

diff --git a/packages/relay/src/lib/errors/MirrorNodeClientError.ts b/packages/relay/src/lib/errors/MirrorNodeClientError.ts
@@ -18,6 +18,8 @@
  *
  */
 
+import { Status } from '@hashgraph/sdk';
+
 export class MirrorNodeClientError extends Error {
   public statusCode: number;
   public data?: string;
@@ -37,6 +39,7 @@ export class MirrorNodeClientError extends Error {
 
   static messages = {
     INVALID_HEX: 'data field invalid hexadecimal string',
+    CONTRACT_REVERT_EXECUTED: Status.ContractRevertExecuted.toString(),
   };
 
   constructor(error: any, statusCode: number) {
@@ -64,6 +67,10 @@ export class MirrorNodeClientError extends Error {
     return this.statusCode === MirrorNodeClientError.ErrorCodes.CONTRACT_REVERT_EXECUTED;
   }
 
+  public isContractRevertOpcodeExecuted() {
+    return this.message === MirrorNodeClientError.messages.CONTRACT_REVERT_EXECUTED;
+  }
+
   public isNotFound(): boolean {
     return this.statusCode === MirrorNodeClientError.statusCodes.NOT_FOUND;
   }

diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts
@@ -604,12 +604,17 @@ export class EthImpl implements Eth {
         this.logger.info(`${requestIdPrefix} Returning gas: ${response.result}`);
         return prepend0x(trimPrecedingZeros(response.result));
       } else {
+        this.logger.error(`${requestIdPrefix} No gas estimate returned from mirror-node: ${JSON.stringify(response)}`);
         return this.predefinedGasForTransaction(transaction, requestIdPrefix);
       }
     } catch (e: any) {
       this.logger.error(
         `${requestIdPrefix} 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 (this.estimateGasThrows && e instanceof MirrorNodeClientError && e.isContractRevertOpcodeExecuted()) {
+        return predefined.CONTRACT_REVERT(e.detail ?? e.message, e.data);
+      }
       return this.predefinedGasForTransaction(transaction, requestIdPrefix, e);
     }
   }

diff --git a/packages/relay/tests/lib/errors/JsonRpcError.spec.ts b/packages/relay/tests/lib/errors/JsonRpcError.spec.ts
@@ -19,7 +19,8 @@
  */
 
 import { expect } from 'chai';
-import { JsonRpcError } from '../../../src/lib/errors/JsonRpcError';
+import { JsonRpcError, predefined } from '../../../src';
+import { AbiCoder, keccak256 } from 'ethers';
 
 describe('Errors', () => {
   describe('JsonRpcError', () => {
@@ -50,5 +51,84 @@ describe('Errors', () => {
       // Check that request ID is prefixed
       expect(err.message).to.eq('[Request ID: abcd-1234] test error: foo');
     });
+
+    describe('predefined.CONTRACT_REVERT', () => {
+      const defaultErrorSignature = keccak256(Buffer.from('Error(string)')).slice(0, 10); // 0x08c379a0
+      const customErrorSignature = keccak256(Buffer.from('CustomError(string)')).slice(0, 10); // 0x8d6ea8be
+      const decodedMessage = 'Some error message';
+      const encodedMessage = new AbiCoder().encode(['string'], [decodedMessage]).replace('0x', '');
+      const encodedCustomError = customErrorSignature + encodedMessage;
+      const encodedDefaultError = defaultErrorSignature + encodedMessage;
+
+      it('Returns decoded message when decoded message is provided as errorMessage and encoded default error is provided as data', () => {
+        const error = predefined.CONTRACT_REVERT(decodedMessage, encodedDefaultError);
+        expect(error.message).to.eq(`execution reverted: ${decodedMessage}`);
+      });
+
+      it('Returns decoded message when decoded message is provided as errorMessage and encoded custom error is provided as data', () => {
+        const error = predefined.CONTRACT_REVERT(decodedMessage, encodedCustomError);
+        expect(error.message).to.eq(`execution reverted: ${decodedMessage}`);
+      });
+
+      it('Returns decoded message when encoded default error is provided as errorMessage and data', () => {
+        const error = predefined.CONTRACT_REVERT(encodedDefaultError, encodedDefaultError);
+        expect(error.message).to.eq(`execution reverted: ${decodedMessage}`);
+      });
+
+      it('Returns decoded message when encoded custom error is provided as errorMessage and data', () => {
+        const error = predefined.CONTRACT_REVERT(encodedCustomError, encodedCustomError);
+        expect(error.message).to.eq(`execution reverted: ${decodedMessage}`);
+      });
+
+      it('Returns decoded message when decoded errorMessage is provided', () => {
+        const error = predefined.CONTRACT_REVERT(decodedMessage);
+        expect(error.message).to.eq(`execution reverted: ${decodedMessage}`);
+      });
+
+      it('Returns decoded message when encoded default error is provided as errorMessage', () => {
+        const error = predefined.CONTRACT_REVERT(encodedDefaultError);
+        expect(error.message).to.eq(`execution reverted: ${decodedMessage}`);
+      });
+
+      it('Returns decoded message when encoded custom error is provided as errorMessage', () => {
+        const error = predefined.CONTRACT_REVERT(encodedCustomError);
+        expect(error.message).to.eq(`execution reverted: ${decodedMessage}`);
+      });
+
+      it('Returns decoded message when encoded default error is provided as data', () => {
+        const error = predefined.CONTRACT_REVERT(undefined, encodedDefaultError);
+        expect(error.message).to.eq(`execution reverted: ${decodedMessage}`);
+      });
+
+      it('Returns decoded message when encoded custom error is provided as data', () => {
+        const error = predefined.CONTRACT_REVERT(undefined, encodedCustomError);
+        expect(error.message).to.eq(`execution reverted: ${decodedMessage}`);
+      });
+
+      it('Returns decoded message when message is empty and encoded default error is provided as data', () => {
+        const error = predefined.CONTRACT_REVERT('', encodedDefaultError);
+        expect(error.message).to.eq(`execution reverted: ${decodedMessage}`);
+      });
+
+      it('Returns decoded message when message is empty and encoded custom error is provided as data', () => {
+        const error = predefined.CONTRACT_REVERT('', encodedCustomError);
+        expect(error.message).to.eq(`execution reverted: ${decodedMessage}`);
+      });
+
+      it('Returns default message when errorMessage is empty', () => {
+        const error = predefined.CONTRACT_REVERT('');
+        expect(error.message).to.eq('execution reverted');
+      });
+
+      it('Returns default message when data is empty', () => {
+        const error = predefined.CONTRACT_REVERT(undefined, '');
+        expect(error.message).to.eq('execution reverted');
+      });
+
+      it('Returns default message when neither errorMessage nor data is provided', () => {
+        const error = predefined.CONTRACT_REVERT();
+        expect(error.message).to.eq('execution reverted');
+      });
+    });
   });
 });
diff --git a/packages/relay/tests/lib/eth/eth_estimateGas.spec.ts b/packages/relay/tests/lib/eth/eth_estimateGas.spec.ts
@@ -32,6 +32,7 @@ import { Precheck } from '../../../src/lib/precheck';
 import { createStubInstance, stub, SinonStub, SinonStubbedInstance } from 'sinon';
 import { IContractCallRequest, IContractCallResponse } from '../../../src/lib/types/IMirrorNode';
 import { DEFAULT_NETWORK_FEES, NO_TRANSACTIONS, ONE_TINYBAR_IN_WEI_HEX, RECEIVER_ADDRESS } from './eth-config';
+import { AbiCoder, keccak256 } from 'ethers';
 
 dotenv.config({ path: path.resolve(__dirname, '../test.env') });
 use(chaiAsPromised);
@@ -431,6 +432,54 @@ describe('@ethEstimateGas Estimate Gas spec', async function () {
     expect(result.message).to.equal('execution reverted: Invalid number of recipients');
   });
 
+  it('should eth_estimateGas with contract revert for contract call and custom contract error', async function () {
+    const decodedMessage = 'Some error message';
+    const customErrorSignature = keccak256(Buffer.from('CustomError(string)')).slice(0, 10); // 0x8d6ea8be
+    const encodedMessage = new AbiCoder().encode(['string'], [decodedMessage]).replace('0x', '');
+    const encodedCustomError = customErrorSignature + encodedMessage;
+
+    web3Mock.onPost('contracts/call', { ...transaction, estimate: true }).reply(400, {
+      _status: {
+        messages: [
+          {
+            message: 'CONTRACT_REVERT_EXECUTED',
+            detail: decodedMessage,
+            data: encodedCustomError,
+          },
+        ],
+      },
+    });
+
+    const result: any = await ethImpl.estimateGas(transaction, id);
+
+    expect(result.data).to.equal(encodedCustomError);
+    expect(result.message).to.equal(`execution reverted: ${decodedMessage}`);
+  });
+
+  it('should eth_estimateGas with contract revert for contract call and generic revert error', async function () {
+    const decodedMessage = 'Some error message';
+    const defaultErrorSignature = keccak256(Buffer.from('Error(string)')).slice(0, 10); // 0x08c379a0
+    const encodedMessage = new AbiCoder().encode(['string'], [decodedMessage]).replace('0x', '');
+    const encodedGenericError = defaultErrorSignature + encodedMessage;
+
+    web3Mock.onPost('contracts/call', { ...transaction, estimate: true }).reply(400, {
+      _status: {
+        messages: [
+          {
+            message: 'CONTRACT_REVERT_EXECUTED',
+            detail: decodedMessage,
+            data: encodedGenericError,
+          },
+        ],
+      },
+    });
+
+    const result: any = await ethImpl.estimateGas(transaction, id);
+
+    expect(result.data).to.equal(encodedGenericError);
+    expect(result.message).to.equal(`execution reverted: ${decodedMessage}`);
+  });
+
   it('should eth_estimateGas handles a 501 unimplemented response from the mirror node correctly by returning default gas', async function () {
     web3Mock.onPost('contracts/call', { ...transaction, estimate: true }).reply(501, {
       _status: {

diff --git a/packages/relay/tests/lib/formatters.spec.ts b/packages/relay/tests/lib/formatters.spec.ts
@@ -44,6 +44,7 @@ import {
 } from '../../src/formatters';
 import constants from '../../src/lib/constants';
 import { BigNumber as BN } from 'bignumber.js';
+import { AbiCoder, keccak256 } from 'ethers';
 
 describe('Formatters', () => {
   describe('formatRequestIdMessage', () => {
@@ -640,6 +641,17 @@ describe('Formatters', () => {
           '0x08c379a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000000';
         expect(decodeErrorMessage(hexErrorMessage)).to.equal('');
       });
+
+      it('should return empty string for custom error message without parameters', () => {
+        expect(decodeErrorMessage('0x858d70bd')).to.equal('');
+      });
+
+      it('should return the message of custom error with string parameter', () => {
+        const signature = keccak256(Buffer.from('CustomError(string)')).slice(0, 10); // 0x8d6ea8be
+        const message = new AbiCoder().encode(['string'], ['Some error message']).replace('0x', '');
+        const hexErrorMessage = signature + message;
+        expect(decodeErrorMessage(hexErrorMessage)).to.equal('Some error message');
+      });
     });
   });