Skip to content

Commit 084547f

Browse files
quiet-nodequiet-node
authored
fix: fixed requests throw error when both data and input fields are present in transaction object (#2551)
* fix: fixed callData mistakenly accepts call.value Signed-off-by: Logan Nguyen <[email protected]> * fix: fixed requests throw error when both data and input fields are present in transaction object Signed-off-by: Logan Nguyen <[email protected]> --------- Signed-off-by: Logan Nguyen <[email protected]>
1 parent d0a1e25 commit 084547f

File tree

5 files changed

+78
-69
lines changed

5 files changed

+78
-69
lines changed

‎packages/relay/src/lib/eth.ts‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -561,16 +561,19 @@ export class EthImpl implements Eth {
561561
_blockParam: string | null,
562562
requestIdPrefix?: string,
563563
): Promise<string | JsonRpcError> {
564-
this.logger.trace(
565-
`${requestIdPrefix} estimateGas(transaction=${JSON.stringify(transaction)}, _blockParam=${_blockParam})`,
566-
);
564+
const callData = transaction.data ? transaction.data : transaction.input;
565+
const callDataSize = callData ? callData.length : 0;
567566

568-
if (transaction?.data?.length >= constants.FUNCTION_SELECTOR_CHAR_LENGTH) {
567+
if (callDataSize >= constants.FUNCTION_SELECTOR_CHAR_LENGTH) {
569568
this.ethExecutionsCounter
570-
.labels(EthImpl.ethEstimateGas, transaction.data.substring(0, constants.FUNCTION_SELECTOR_CHAR_LENGTH))
569+
.labels(EthImpl.ethEstimateGas, callData.substring(0, constants.FUNCTION_SELECTOR_CHAR_LENGTH))
571570
.inc();
572571
}
573572

573+
this.logger.trace(
574+
`${requestIdPrefix} estimateGas(transaction=${JSON.stringify(transaction)}, _blockParam=${_blockParam})`,
575+
);
576+
574577
this.contractCallFormat(transaction);
575578
let gas = EthImpl.gasTxBaseCost;
576579
try {
@@ -653,16 +656,14 @@ export class EthImpl implements Eth {
653656
transaction.gas = parseInt(transaction.gas);
654657
}
655658

656-
if (transaction.data && transaction.input) {
657-
throw predefined.INVALID_ARGUMENTS('Cannot accept both input and data fields. Use only one.');
658-
}
659-
660659
// Support either data or input. https://ethereum.github.io/execution-apis/api-documentation/ lists input but many EVM tools still use data.
661660
// We chose in the mirror node to use data field as the correct one, however for us to be able to support all tools,
662661
// we have to modify transaction object, so that it complies with the mirror node.
663-
// That means that, if input field is passed, but data is not, we have to copy one value to the other.
664-
// For optimization purposes, we can rid of the input property or replace it with empty string.
665-
if (transaction.input && transaction.data === undefined) {
662+
// That means that, if input field is passed, but data is not, we have to copy value of input to the data to comply with mirror node.
663+
// The second scenario occurs when both the data and input fields are present but hold different values.
664+
// In this case, the value in the input field should be the one used for consensus based on this resource https://github.com/ethereum/execution-apis/blob/main/tests/eth_call/call-contract.io
665+
// Eventually, for optimization purposes, we can rid of the input property or replace it with empty string.
666+
if ((transaction.input && transaction.data === undefined) || (transaction.input && transaction.data)) {
666667
transaction.data = transaction.input;
667668
delete transaction.input;
668669
}
@@ -1549,7 +1550,7 @@ export class EthImpl implements Eth {
15491550
* @param blockParam
15501551
*/
15511552
async call(call: any, blockParam: string | object | null, requestIdPrefix?: string): Promise<string | JsonRpcError> {
1552-
const callData = call.data ? call.data : call.value;
1553+
const callData = call.data ? call.data : call.input;
15531554
// log request
15541555
this.logger.trace(
15551556
`${requestIdPrefix} call({to=${call.to}, from=${call.from}, data=${callData}, gas=${call.gas}, ...}, blockParam=${blockParam})`,
@@ -1558,9 +1559,9 @@ export class EthImpl implements Eth {
15581559
const callDataSize = callData ? callData.length : 0;
15591560
this.logger.trace(`${requestIdPrefix} call data size: ${callDataSize}, gas: ${call.gas}`);
15601561
// metrics for selector
1561-
if (call.data?.length >= constants.FUNCTION_SELECTOR_CHAR_LENGTH)
1562+
if (callDataSize >= constants.FUNCTION_SELECTOR_CHAR_LENGTH)
15621563
this.ethExecutionsCounter
1563-
.labels(EthImpl.ethCall, call.data.substring(0, constants.FUNCTION_SELECTOR_CHAR_LENGTH))
1564+
.labels(EthImpl.ethCall, callData.substring(0, constants.FUNCTION_SELECTOR_CHAR_LENGTH))
15641565
.inc();
15651566

15661567
const blockNumberOrTag = await this.extractBlockNumberOrTag(blockParam, requestIdPrefix);

‎packages/relay/tests/lib/eth/eth_call.spec.ts‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -817,17 +817,26 @@ describe('@ethCall Eth Call spec', async function () {
817817
expect(transaction.gas).to.equal(50000);
818818
});
819819

820-
it('should throw an error if both input and data fields are provided', () => {
820+
it('should accepts both input and data fields but copy value of input field to data field', () => {
821+
const inputValue = 'input value';
822+
const dataValue = 'data value';
821823
const transaction = {
822-
input: 'input data',
823-
data: 'some data',
824+
input: inputValue,
825+
data: dataValue,
824826
};
825-
try {
826-
ethImpl.contractCallFormat(transaction);
827-
} catch (error) {
828-
expect(error.code).eq(-32000);
829-
expect(error.message).eq('Invalid arguments: Cannot accept both input and data fields. Use only one.');
830-
}
827+
ethImpl.contractCallFormat(transaction);
828+
expect(transaction.data).to.eq(inputValue);
829+
expect(transaction.data).to.not.eq(dataValue);
830+
expect(transaction.input).to.be.undefined;
831+
});
832+
833+
it('should not modify transaction if only data field is present', () => {
834+
const dataValue = 'data value';
835+
const transaction = {
836+
data: dataValue,
837+
};
838+
ethImpl.contractCallFormat(transaction);
839+
expect(transaction.data).to.eq(dataValue);
831840
});
832841

833842
it('should copy input to data if input is provided but data is not', () => {

‎packages/relay/tests/lib/eth/eth_estimateGas.spec.ts‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -449,21 +449,24 @@ describe('@ethEstimateGas Estimate Gas spec', async function () {
449449
expect(transaction.gas).to.eq(14250000);
450450
});
451451

452-
it('should throw on estimateGas precheck', async function () {
452+
it('should accepts both input and data fields but copy value of input field to data field', () => {
453+
const inputValue = 'input value';
454+
const dataValue = 'data value';
453455
const transaction = {
454456
from: '0x05fba803be258049a27b820088bab1cad2058871',
455-
data: '0x',
456-
input: '0x',
457+
input: inputValue,
458+
data: dataValue,
457459
value: '0xA186B8E9800',
458460
gasPrice: '0xF4240',
459461
gas: '0xd97010',
460462
};
461463

462-
try {
463-
ethImpl.contractCallFormat(transaction);
464-
} catch (error) {
465-
expect(error.code).to.equal(-32000);
466-
expect(error.message).to.equal('Invalid arguments: Cannot accept both input and data fields. Use only one.');
467-
}
464+
ethImpl.contractCallFormat(transaction);
465+
expect(transaction.data).to.eq(inputValue);
466+
expect(transaction.data).to.not.eq(dataValue);
467+
expect(transaction.input).to.be.undefined;
468+
expect(transaction.value).to.eq(1110);
469+
expect(transaction.gasPrice).to.eq(1000000);
470+
expect(transaction.gas).to.eq(14250000);
468471
});
469472
});

‎packages/server/tests/acceptance/rpc_batch2.spec.ts‎

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -391,25 +391,24 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
391391
expect(res).to.not.be.equal('0x0');
392392
});
393393

394-
it('should throw on "eth_estimateGas" with both input and data fields present in the txObject', async function () {
395-
try {
396-
await relay.call(
397-
RelayCalls.ETH_ENDPOINTS.ETH_ESTIMATE_GAS,
398-
[
399-
{
400-
from: '0x114f60009ee6b84861c0cdae8829751e517bc4d7',
401-
to: '0xae410f34f7487e2cd03396499cebb09b79f45d6e',
402-
value: '0xa688906bd8b00000',
403-
gas: '0xd97010',
404-
input: '0x',
405-
data: '0x',
406-
},
407-
],
408-
requestId,
409-
);
410-
} catch (e) {
411-
expect(e).to.equal(predefined.INVALID_ARGUMENTS('Cannot accept both input and data fields. Use only one.'));
412-
}
394+
it('should execute "eth_estimateGas" with both input and data fields present in the txObject', async function () {
395+
const res = await relay.call(
396+
RelayCalls.ETH_ENDPOINTS.ETH_ESTIMATE_GAS,
397+
[
398+
{
399+
from: '0x114f60009ee6b84861c0cdae8829751e517bc4d7',
400+
to: '0xae410f34f7487e2cd03396499cebb09b79f45d6e',
401+
value: '0xa688906bd8b00000',
402+
gas: '0xd97010',
403+
input: '0x',
404+
data: '0x',
405+
},
406+
],
407+
requestId,
408+
);
409+
expect(res).to.contain('0x');
410+
expect(res).to.not.be.equal('0x');
411+
expect(res).to.not.be.equal('0x0');
413412
});
414413
});
415414

‎packages/server/tests/acceptance/rpc_batch3.spec.ts‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,20 @@ describe('@api-batch-3 RPC Server Acceptance Tests', function () {
294294
expect(res).to.eq(BASIC_CONTRACT_PING_RESULT);
295295
});
296296

297+
it('should execute "eth_call" with both data and input fields', async function () {
298+
const callData = {
299+
from: accounts[0].address,
300+
to: basicContractAddress,
301+
data: BASIC_CONTRACT_PING_CALL_DATA,
302+
input: BASIC_CONTRACT_PING_CALL_DATA,
303+
};
304+
305+
// deploymentBlockNumber to HEX
306+
const block = numberTo0x(deploymentBlockNumber);
307+
const res = await relay.call(RelayCall.ETH_ENDPOINTS.ETH_CALL, [callData, { blockNumber: block }], requestId);
308+
expect(res).to.eq(BASIC_CONTRACT_PING_RESULT);
309+
});
310+
297311
it('should fail to execute "eth_call" with wrong block tag', async function () {
298312
const callData = {
299313
from: accounts[0].address,
@@ -333,23 +347,6 @@ describe('@api-batch-3 RPC Server Acceptance Tests', function () {
333347
await Assertions.assertPredefinedRpcError(errorType, relay.call, false, relay, args);
334348
});
335349

336-
it('should fail to execute "eth_call" with both data and input fields', async function () {
337-
const callData = {
338-
from: accounts[0].address,
339-
to: basicContractAddress,
340-
data: BASIC_CONTRACT_PING_CALL_DATA,
341-
};
342-
343-
// deploymentBlockNumber to HEX
344-
const block = numberTo0x(deploymentBlockNumber);
345-
try {
346-
await relay.call(RelayCall.ETH_ENDPOINTS.ETH_CALL, [callData, { blockNumber: block }], requestId);
347-
} catch (error) {
348-
expect(error.code).eq(-32000);
349-
expect(error.message).eq('Invalid arguments: Cannot accept both input and data fields. Use only one.');
350-
}
351-
});
352-
353350
it('should fail to execute "eth_call" with wrong block number object', async function () {
354351
const callData = {
355352
from: accounts[0].address,

0 commit comments

Comments
 (0)