chore: Cherry pick #2377 on 0.46 release (#2385)

fix: wrong gas estimation due to invalid tx object (#2377) * fix: wrong gas estimation due to invalid tx object optimization chore: fix typo test: add unit test fix logic delete input if not used * add simple fix * remove unused import * revert changes * fix acceptance tests * add unit tests --------- Signed-off-by: georgi-l95 <[email protected]> Signed-off-by: Alfredo Gutierrez <[email protected]> Co-authored-by: Georgi Lazarov <[email protected]>
hashgraph · Apr 22, 2024 · 76c45e1 · 76c45e1
1 parent 353b62c
commit 76c45e1
Show file tree

Hide file tree

Showing 5 changed files with 172 additions and 3 deletions.
diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts
@@ -576,6 +576,7 @@ export class EthImpl implements Eth {
 
       if (contractCallResponse?.result) {
         gas = prepend0x(trimPrecedingZeros(contractCallResponse.result));
+        this.logger.info(`${requestIdPrefix} Returning gas: ${gas}`);
       }
     } catch (e: any) {
       this.logger.error(
@@ -624,9 +625,8 @@ export class EthImpl implements Eth {
         // Handle Contract Call or Contract Create
         gas = this.defaultGas;
       }
+      this.logger.error(`${requestIdPrefix} Returning predefined gas: ${gas}`);
     }
-    this.logger.error(`${requestIdPrefix} Returning predefined gas: ${gas}`);
-
     return gas;
   }
 
@@ -644,9 +644,19 @@ export class EthImpl implements Eth {
     if (transaction.gas) {
       transaction.gas = parseInt(transaction.gas);
     }
+
+    if (transaction.data && transaction.input) {
+      throw predefined.INVALID_ARGUMENTS('Cannot accept both input and data fields. Use only one.');
+    }
+
     // Support either data or input. https://ethereum.github.io/execution-apis/api-documentation/ lists input but many EVM tools still use data.
+    // We chose in the mirror node to use data field as the correct one, however for us to be able to support all tools,
+    // we have to modify transaction object, so that it complies with the mirror node.
+    // That means that, if input field is passed, but data is not, we have to copy one value to the other.
+    // For optimization purposes, we can rid of the input property or replace it with empty string.
     if (transaction.input && transaction.data === undefined) {
       transaction.data = transaction.input;
+      delete transaction.input;
     }
   }
 

diff --git a/packages/relay/tests/lib/eth/eth_call.spec.ts b/packages/relay/tests/lib/eth/eth_call.spec.ts
@@ -880,4 +880,74 @@ describe('@ethCall Eth Call spec', async function () {
       expect(result).to.equal('0x');
     });
   });
+
+  describe('contractCallFormat', () => {
+    it('should format transaction value to tiny bar integer', () => {
+      const transaction = {
+        value: '0x2540BE400',
+      };
+
+      ethImpl.contractCallFormat(transaction);
+      expect(transaction.value).to.equal(1);
+    });
+
+    it('should parse gasPrice to integer', () => {
+      const transaction = {
+        gasPrice: '1000000000',
+      };
+
+      ethImpl.contractCallFormat(transaction);
+
+      expect(transaction.gasPrice).to.equal(1000000000);
+    });
+
+    it('should parse gas to integer', () => {
+      const transaction = {
+        gas: '50000',
+      };
+
+      ethImpl.contractCallFormat(transaction);
+
+      expect(transaction.gas).to.equal(50000);
+    });
+
+    it('should throw an error if both input and data fields are provided', () => {
+      const transaction = {
+        input: 'input data',
+        data: 'some data',
+      };
+      try {
+        ethImpl.contractCallFormat(transaction);
+      } catch (error) {
+        expect(error.code).eq(-32000);
+        expect(error.message).eq('Invalid arguments: Cannot accept both input and data fields. Use only one.');
+      }
+    });
+
+    it('should copy input to data if input is provided but data is not', () => {
+      const transaction = {
+        input: 'input data',
+      };
+
+      ethImpl.contractCallFormat(transaction);
+
+      // @ts-ignore
+      expect(transaction.data).to.equal('input data');
+      expect(transaction.input).to.be.undefined;
+    });
+
+    it('should not modify transaction if input and data fields are not provided', () => {
+      const transaction = {
+        value: '0x2540BE400',
+        gasPrice: '1000000000',
+        gas: '50000',
+      };
+
+      ethImpl.contractCallFormat(transaction);
+
+      expect(transaction.value).to.equal(1);
+      expect(transaction.gasPrice).to.equal(1000000000);
+      expect(transaction.gas).to.equal(50000);
+    });
+  });
 });
diff --git a/packages/relay/tests/lib/eth/eth_estimateGas.spec.ts b/packages/relay/tests/lib/eth/eth_estimateGas.spec.ts
@@ -29,7 +29,7 @@ import constants from '../../../src/lib/constants';
 import { SDKClient } from '../../../src/lib/clients';
 import { numberTo0x } from '../../../dist/formatters';
 import { DEFAULT_NETWORK_FEES, NO_TRANSACTIONS, ONE_TINYBAR_IN_WEI_HEX, RECEIVER_ADDRESS } from './eth-config';
-import { JsonRpcError } from '../../../src/lib/errors/JsonRpcError';
+import { JsonRpcError, predefined } from '../../../src/lib/errors/JsonRpcError';
 import { generateEthTestEnv } from './eth-helpers';
 
 dotenv.config({ path: path.resolve(__dirname, '../test.env') });
@@ -232,6 +232,21 @@ describe('@ethEstimateGas Estimate Gas spec', async function () {
     );
   });
 
+  it('should eth_estimateGas for contract create with input field and absent data field', async () => {
+    const gasEstimation = 1357410;
+    const callData = {
+      input:
+        '0x81cb089c285e5ee3a7353704fb114955037443af85e5ee3a7353704fb114955037443af85e5ee3a7353704fb114955037443af85e5ee3a7353704fb114955037443af',
+      from: '0x81cb089c285e5ee3a7353704fb114955037443af',
+      to: null,
+      value: '0x0',
+    };
+    web3Mock.onPost('contracts/call', { ...callData, estimate: true }).reply(200, { result: `0x14b662` });
+
+    const gas = await ethImpl.estimateGas({ ...callData }, null);
+    expect((gas as string).toLowerCase()).to.equal(numberTo0x(gasEstimation).toLowerCase());
+  });
+
   it('should eth_estimateGas transfer with invalid value', async function () {
     const result = await ethImpl.estimateGas(
       {
@@ -370,4 +385,21 @@ describe('@ethEstimateGas Estimate Gas spec', async function () {
     expect(transaction.gasPrice).to.eq(1000000);
     expect(transaction.gas).to.eq(14250000);
   });
+
+  it('should throw on estimateGas precheck', async function () {
+    const transaction = {
+      from: '0x05fba803be258049a27b820088bab1cad2058871',
+      data: '0x',
+      input: '0x',
+      value: '0xA186B8E9800',
+      gasPrice: '0xF4240',
+      gas: '0xd97010',
+    };
+
+    try {
+      ethImpl.contractCallFormat(transaction);
+    } catch (error) {
+      expect(error).to.equal(predefined.INVALID_ARGUMENTS('Cannot accept both input and data fields. Use only one.'));
+    }
+  });
 });
diff --git a/packages/server/tests/acceptance/rpc_batch2.spec.ts b/packages/server/tests/acceptance/rpc_batch2.spec.ts
@@ -356,6 +356,46 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
       expect(res).to.not.be.equal('0x');
       expect(res).to.not.be.equal('0x0');
     });
+
+    it('should execute "eth_estimateGas" with input as 0x instead of data', async function () {
+      const res = await relay.call(
+        RelayCalls.ETH_ENDPOINTS.ETH_ESTIMATE_GAS,
+        [
+          {
+            from: '0x114f60009ee6b84861c0cdae8829751e517bc4d7',
+            to: '0xae410f34f7487e2cd03396499cebb09b79f45d6e',
+            value: '0xa688906bd8b00000',
+            gas: '0xd97010',
+            input: '0x',
+          },
+        ],
+        requestId,
+      );
+      expect(res).to.contain('0x');
+      expect(res).to.not.be.equal('0x');
+      expect(res).to.not.be.equal('0x0');
+    });
+
+    it('should throw on "eth_estimateGas" with both input and data fields present in the txObject', async function () {
+      try {
+        await relay.call(
+          RelayCalls.ETH_ENDPOINTS.ETH_ESTIMATE_GAS,
+          [
+            {
+              from: '0x114f60009ee6b84861c0cdae8829751e517bc4d7',
+              to: '0xae410f34f7487e2cd03396499cebb09b79f45d6e',
+              value: '0xa688906bd8b00000',
+              gas: '0xd97010',
+              input: '0x',
+              data: '0x',
+            },
+          ],
+          requestId,
+        );
+      } catch (e) {
+        expect(e).to.equal(predefined.INVALID_ARGUMENTS('Cannot accept both input and data fields. Use only one.'));
+      }
+    });
   });
 
   describe('eth_gasPrice', async function () {

diff --git a/packages/server/tests/acceptance/rpc_batch3.spec.ts b/packages/server/tests/acceptance/rpc_batch3.spec.ts
@@ -324,6 +324,23 @@ describe('@api-batch-3 RPC Server Acceptance Tests', function () {
       await Assertions.assertPredefinedRpcError(errorType, relay.call, false, relay, args);
     });
 
+    it('should fail to execute "eth_call" with both data and input fields', async function () {
+      const callData = {
+        from: accounts[0].address,
+        to: evmAddress,
+        data: BASIC_CONTRACT_PING_CALL_DATA,
+      };
+
+      // deploymentBlockNumber to HEX
+      const block = numberTo0x(deploymentBlockNumber);
+      try {
+        await relay.call(RelayCall.ETH_ENDPOINTS.ETH_CALL, [callData, { blockNumber: block }], requestId);
+      } catch (error) {
+        expect(error.code).eq(-32000);
+        expect(error.message).eq('Invalid arguments: Cannot accept both input and data fields. Use only one.');
+      }
+    });
+
     it('should fail to execute "eth_call" with wrong block number object', async function () {
       const callData = {
         from: accounts[0].address,