From 52fb0e191478114b26b195f4c9b9e8f3bd5e3a40 Mon Sep 17 00:00:00 2001 From: Victor Yanev Date: Fri, 25 Oct 2024 11:32:50 +0300 Subject: [PATCH] fix: Incosistent handling of failed EVM transactions Signed-off-by: Victor Yanev --- packages/relay/src/lib/eth.ts | 22 +++++-- .../tests/lib/eth/eth_getBlockByHash.spec.ts | 62 ++++++++++++++++- .../lib/eth/eth_getBlockByNumber.spec.ts | 66 ++++++++++++++++++- 3 files changed, 142 insertions(+), 8 deletions(-) diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts index 8b7a1c424..936680c29 100644 --- a/packages/relay/src/lib/eth.ts +++ b/packages/relay/src/lib/eth.ts @@ -48,6 +48,7 @@ import { formatContractResult, formatTransactionIdWithoutQueryParams, getFunctionSelector, + hexToASCII, isHex, isValidEthereumAddress, nanOrNumberTo0x, @@ -55,6 +56,7 @@ import { numberTo0x, parseNumericEnvVar, prepend0x, + strip0x, toHash32, trimPrecedingZeros, weibarHexToTinyBarInt, @@ -326,10 +328,9 @@ export class EthImpl implements Eth { requestDetails: RequestDetails, ): Promise { const requestIdPrefix = requestDetails.formattedRequestId; - const maxResults = - ConfigService.get('TEST') - ? constants.DEFAULT_FEE_HISTORY_MAX_RESULTS - : Number(ConfigService.get('FEE_HISTORY_MAX_RESULTS')); + const maxResults = ConfigService.get('TEST') + ? constants.DEFAULT_FEE_HISTORY_MAX_RESULTS + : Number(ConfigService.get('FEE_HISTORY_MAX_RESULTS')); this.logger.trace( `${requestIdPrefix} feeHistory(blockCount=${blockCount}, newestBlock=${newestBlock}, rewardPercentiles=${rewardPercentiles})`, @@ -2297,9 +2298,20 @@ export class EthImpl implements Eth { throw predefined.MAX_BLOCK_SIZE(blockResponse.count); } + const isWrongNonce = (contractResult: { result: string; error_message: any }) => { + return ( + contractResult.result === constants.TRANSACTION_RESULT_STATUS.WRONG_NONCE || + hexToASCII(strip0x(contractResult.error_message ?? '')) === constants.TRANSACTION_RESULT_STATUS.WRONG_NONCE + ); + }; + // prepare transactionArray let transactionArray: any[] = []; for (const contractResult of contractResults) { + if (isWrongNonce(contractResult)) { + // skip wrong nonce transactions as they are not valid transactions which should be included in the block + continue; + } contractResult.from = await this.resolveEvmAddress(contractResult.from, requestDetails, [constants.TYPE_ACCOUNT]); contractResult.to = await this.resolveEvmAddress(contractResult.to, requestDetails); contractResult.chain_id = contractResult.chain_id || this.chain; @@ -2308,7 +2320,7 @@ export class EthImpl implements Eth { } transactionArray = this.populateSyntheticTransactions(showDetails, logs, transactionArray, requestDetails); - transactionArray = showDetails ? _.uniqBy(transactionArray, 'hash') : _.uniq(transactionArray); + transactionArray = showDetails ? transactionArray : _.uniq(transactionArray); const formattedReceipts: IReceiptRootHash[] = ReceiptsRootUtils.buildReceiptRootHashes( transactionArray.map((tx) => (showDetails ? tx.hash : tx)), diff --git a/packages/relay/tests/lib/eth/eth_getBlockByHash.spec.ts b/packages/relay/tests/lib/eth/eth_getBlockByHash.spec.ts index 23c6d3e3e..4bf8e04c6 100644 --- a/packages/relay/tests/lib/eth/eth_getBlockByHash.spec.ts +++ b/packages/relay/tests/lib/eth/eth_getBlockByHash.spec.ts @@ -32,7 +32,7 @@ import { } from '../../helpers'; import { SDKClient } from '../../../src/lib/clients'; import RelayAssertions from '../../assertions'; -import { numberTo0x } from '../../../dist/formatters'; +import { ASCIIToHex, numberTo0x, prepend0x } from '../../../dist/formatters'; import { ACCOUNT_WITHOUT_TRANSACTIONS, BLOCK_HASH, @@ -334,4 +334,64 @@ describe('@ethGetBlockByHash using MirrorNode', async function () { args, ); }); + + it('eth_getBlockByHash should skip transactions with wrong nonces when showDetails = false', async function () { + // mirror node request mocks + restMock.onGet(`blocks/${BLOCK_HASH}`).reply(200, DEFAULT_BLOCK); + restMock.onGet(CONTRACT_RESULTS_WITH_FILTER_URL).reply(200, { + results: [ + ...defaultContractResults.results, + { ...defaultContractResults.results[0], result: 'WRONG_NONCE' }, + { ...defaultContractResults.results[0], error_message: prepend0x(ASCIIToHex('WRONG_NONCE')) }, + ], + }); + restMock.onGet(CONTRACT_RESULTS_LOGS_WITH_FILTER_URL).reply(200, DEFAULT_ETH_GET_BLOCK_BY_LOGS); + + const showDetails = false; + const result = await ethImpl.getBlockByHash(BLOCK_HASH, showDetails, requestDetails); + + RelayAssertions.assertBlock( + result, + { + hash: BLOCK_HASH_TRIMMED, + gasUsed: TOTAL_GAS_USED, + number: BLOCK_NUMBER_HEX, + parentHash: BLOCK_HASH_PREV_TRIMMED, + timestamp: BLOCK_TIMESTAMP_HEX, + transactions: [CONTRACT_HASH_1, CONTRACT_HASH_2], // should not include the transaction with wrong nonce + receiptsRoot: DEFAULT_BLOCK_RECEIPTS_ROOT_HASH, + }, + showDetails, + ); + }); + + it('eth_getBlockByHash should skip transactions with wrong nonces when showDetails = true', async function () { + // mirror node request mocks + restMock.onGet(`blocks/${BLOCK_HASH}`).reply(200, DEFAULT_BLOCK); + restMock.onGet(CONTRACT_RESULTS_WITH_FILTER_URL).reply(200, { + results: [ + ...defaultContractResults.results, + { ...defaultContractResults.results[1], result: 'WRONG_NONCE' }, + { ...defaultContractResults.results[1], error_message: prepend0x(ASCIIToHex('WRONG_NONCE')) }, + ], + }); + restMock.onGet(CONTRACT_RESULTS_LOGS_WITH_FILTER_URL).reply(200, DEFAULT_ETH_GET_BLOCK_BY_LOGS); + + const showDetails = true; + const result = await ethImpl.getBlockByHash(BLOCK_HASH, showDetails, requestDetails); + + RelayAssertions.assertBlock( + result, + { + hash: BLOCK_HASH_TRIMMED, + gasUsed: TOTAL_GAS_USED, + number: BLOCK_NUMBER_HEX, + parentHash: BLOCK_HASH_PREV_TRIMMED, + timestamp: BLOCK_TIMESTAMP_HEX, + transactions: [CONTRACT_HASH_1, CONTRACT_HASH_2], // should not include the transaction with wrong nonce + receiptsRoot: DEFAULT_BLOCK_RECEIPTS_ROOT_HASH, + }, + showDetails, + ); + }); }); diff --git a/packages/relay/tests/lib/eth/eth_getBlockByNumber.spec.ts b/packages/relay/tests/lib/eth/eth_getBlockByNumber.spec.ts index 085526025..e1cb7cc41 100644 --- a/packages/relay/tests/lib/eth/eth_getBlockByNumber.spec.ts +++ b/packages/relay/tests/lib/eth/eth_getBlockByNumber.spec.ts @@ -22,7 +22,7 @@ import { expect, use } from 'chai'; import sinon from 'sinon'; import chaiAsPromised from 'chai-as-promised'; import { Logger } from 'pino'; -import { predefined } from '../../../src/lib/errors/JsonRpcError'; +import { predefined } from '../../../src'; import { EthImpl } from '../../../src/lib/eth'; import { blockLogsBloom, @@ -34,7 +34,7 @@ import { Block, Transaction } from '../../../src/lib/model'; import { MirrorNodeClient, SDKClient } from '../../../src/lib/clients'; import RelayAssertions from '../../assertions'; import constants from '../../../src/lib/constants'; -import { hashNumber, numberTo0x } from '../../../dist/formatters'; +import { ASCIIToHex, hashNumber, numberTo0x, prepend0x } from '../../../dist/formatters'; import { BLOCK_HASH, BLOCK_HASH_PREV_TRIMMED, @@ -566,4 +566,66 @@ describe('@ethGetBlockByNumber using MirrorNode', async function () { args, ); }); + + it('eth_getBlockByNumber should skip transactions with wrong nonces when showDetails = false', async function () { + // mirror node request mocks + restMock.onGet(`blocks/${BLOCK_NUMBER}`).reply(200, DEFAULT_BLOCK); + restMock.onGet(BLOCKS_LIMIT_ORDER_URL).reply(200, MOST_RECENT_BLOCK); + restMock.onGet(CONTRACT_RESULTS_WITH_FILTER_URL).reply(200, { + results: [ + ...defaultContractResults.results, + { ...defaultContractResults.results[0], result: 'WRONG_NONCE' }, + { ...defaultContractResults.results[0], error_message: prepend0x(ASCIIToHex('WRONG_NONCE')) }, + ], + }); + restMock.onGet(CONTRACT_RESULTS_LOGS_WITH_FILTER_URL).reply(200, DEFAULT_ETH_GET_BLOCK_BY_LOGS); + + const showDetails = false; + const result = await ethImpl.getBlockByNumber(numberTo0x(BLOCK_NUMBER), showDetails, requestDetails); + + RelayAssertions.assertBlock( + result, + { + hash: BLOCK_HASH_TRIMMED, + gasUsed: TOTAL_GAS_USED, + number: BLOCK_NUMBER_HEX, + parentHash: BLOCK_HASH_PREV_TRIMMED, + timestamp: BLOCK_TIMESTAMP_HEX, + transactions: [CONTRACT_HASH_1, CONTRACT_HASH_2], // should not include the transaction with wrong nonce + receiptsRoot: DEFAULT_BLOCK_RECEIPTS_ROOT_HASH, + }, + showDetails, + ); + }); + + it('eth_getBlockByNumber should skip transactions with wrong nonces when showDetails = true', async function () { + // mirror node request mocks + restMock.onGet(`blocks/${BLOCK_NUMBER}`).reply(200, DEFAULT_BLOCK); + restMock.onGet(BLOCKS_LIMIT_ORDER_URL).reply(200, MOST_RECENT_BLOCK); + restMock.onGet(CONTRACT_RESULTS_WITH_FILTER_URL).reply(200, { + results: [ + ...defaultContractResults.results, + { ...defaultContractResults.results[1], result: 'WRONG_NONCE' }, + { ...defaultContractResults.results[1], error_message: prepend0x(ASCIIToHex('WRONG_NONCE')) }, + ], + }); + restMock.onGet(CONTRACT_RESULTS_LOGS_WITH_FILTER_URL).reply(200, DEFAULT_ETH_GET_BLOCK_BY_LOGS); + + const showDetails = true; + const result = await ethImpl.getBlockByHash(numberTo0x(BLOCK_NUMBER), showDetails, requestDetails); + + RelayAssertions.assertBlock( + result, + { + hash: BLOCK_HASH_TRIMMED, + gasUsed: TOTAL_GAS_USED, + number: BLOCK_NUMBER_HEX, + parentHash: BLOCK_HASH_PREV_TRIMMED, + timestamp: BLOCK_TIMESTAMP_HEX, + transactions: [CONTRACT_HASH_1, CONTRACT_HASH_2], // should not include the transaction with wrong nonce + receiptsRoot: DEFAULT_BLOCK_RECEIPTS_ROOT_HASH, + }, + showDetails, + ); + }); });