From 76c45e13c5c2fcc3a4ecc8f4214aa208f6313d4a Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Mon, 22 Apr 2024 16:46:22 -0600 Subject: [PATCH] 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 Signed-off-by: Alfredo Gutierrez Co-authored-by: Georgi Lazarov --- packages/relay/src/lib/eth.ts | 14 +++- packages/relay/tests/lib/eth/eth_call.spec.ts | 70 +++++++++++++++++++ .../tests/lib/eth/eth_estimateGas.spec.ts | 34 ++++++++- .../tests/acceptance/rpc_batch2.spec.ts | 40 +++++++++++ .../tests/acceptance/rpc_batch3.spec.ts | 17 +++++ 5 files changed, 172 insertions(+), 3 deletions(-) diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts index bc6f361da6..c4aac28e2f 100644 --- 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 index c2985d6492..3ba8958e9e 100644 --- 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 index 3fa8c0ab16..c7fb94b506 100644 --- 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 index f4a160cb62..8558224eb3 100644 --- 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 index b539705982..2c26504181 100644 --- 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,