Skip to content

Commit

Permalink
fix: strictly throw errors if immature records found
Browse files Browse the repository at this point in the history 
Signed-off-by: Logan Nguyen <[email protected]>
  • Loading branch information
@quiet-node
quiet-node committed Jan 10, 2025
1 parent 8ddc557 commit 256a28b
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 59 deletions.
44 changes: 37 additions & 7 deletions packages/relay/src/lib/eth.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1937,6 +1937,8 @@ export class EthImpl implements Eth {

if (!contractResults[0]) return null;

this.handleImmatureContractResultRecord(contractResults[0], requestDetails);

const resolvedToAddress = await this.resolveEvmAddress(contractResults[0].to, requestDetails);
const resolvedFromAddress = await this.resolveEvmAddress(contractResults[0].from, requestDetails, [
constants.TYPE_ACCOUNT,
Expand Down Expand Up @@ -2231,11 +2233,7 @@ export class EthImpl implements Eth {
return this.createTransactionFromLog(syntheticLogs[0]);
}

if (!contractResult.block_number || (!contractResult.transaction_index && contractResult.transaction_index !== 0)) {
this.logger.warn(
`${requestIdPrefix} getTransactionByHash(hash=${hash}) mirror-node returned status 200 with missing properties in contract_results - block_number==${contractResult.block_number} and transaction_index==${contractResult.transaction_index}`,
);
}
this.handleImmatureContractResultRecord(contractResult, requestDetails);

const fromAddress = await this.resolveEvmAddress(contractResult.from, requestDetails, [constants.TYPE_ACCOUNT]);
const toAddress = await this.resolveEvmAddress(contractResult.to, requestDetails);
Expand Down Expand Up @@ -2329,6 +2327,8 @@ export class EthImpl implements Eth {
);
return receipt;
} else {
this.handleImmatureContractResultRecord(receiptResponse, requestDetails);

const effectiveGas = await this.getCurrentGasPriceForBlock(receiptResponse.block_hash, requestDetails);
// support stricter go-eth client which requires the transaction hash property on logs
const logs = receiptResponse.logs.map((log) => {
Expand All @@ -2341,7 +2341,7 @@ export class EthImpl implements Eth {
removed: false,
topics: log.topics,
transactionHash: toHash32(receiptResponse.hash),
transactionIndex: nullableNumberTo0x(receiptResponse.transaction_index),
transactionIndex: numberTo0x(receiptResponse.transaction_index),
});
});

Expand All @@ -2357,7 +2357,7 @@ export class EthImpl implements Eth {
logs: logs,
logsBloom: receiptResponse.bloom === EthImpl.emptyHex ? EthImpl.emptyBloom : receiptResponse.bloom,
transactionHash: toHash32(receiptResponse.hash),
transactionIndex: nullableNumberTo0x(receiptResponse.transaction_index),
transactionIndex: numberTo0x(receiptResponse.transaction_index),
effectiveGasPrice: effectiveGas,
root: receiptResponse.root || constants.DEFAULT_ROOT_HASH,
status: receiptResponse.status,
Expand Down Expand Up @@ -2570,6 +2570,8 @@ export class EthImpl implements Eth {
// prepare transactionArray
let transactionArray: any[] = [];
for (const contractResult of contractResults) {
this.handleImmatureContractResultRecord(contractResult, requestDetails);

// there are several hedera-specific validations that occur right before entering the evm
// if a transaction has reverted there, we should not include that tx in the block response
if (Utils.isRevertedDueToHederaSpecificValidation(contractResult)) {
Expand Down Expand Up @@ -2839,4 +2841,32 @@ export class EthImpl implements Eth {
const exchangeRateInCents = currentNetworkExchangeRate.cent_equivalent / currentNetworkExchangeRate.hbar_equivalent;
return exchangeRateInCents;
}

/**
* Checks if a contract result record is immature by validating required fields.
* An immature record can be characterized by:
* - `transaction_index` being null/undefined
* - `block_number` being null/undefined
* - `block_hash` being '0x' (empty hex)
*
* @param {any} record - The contract result record to validate
* @param {RequestDetails} requestDetails - Details used for logging and tracking the request
* @throws {Error} If the record is missing required fields
*/
private handleImmatureContractResultRecord(record: any, requestDetails: RequestDetails) {
if (record.transaction_index == null || record.block_number == null || record.block_hash === EthImpl.emptyHex) {
if (this.logger.isLevelEnabled('debug')) {
this.logger.debug(

Check warning on line 2859 in packages/relay/src/lib/eth.ts

View check run for this annotation

Codecov / codecov/patch

packages/relay/src/lib/eth.ts#L2859 

Added line #L2859 was not covered by tests
`${
requestDetails.formattedRequestId
} Contract result is missing required fields: block_number, transaction_index, or block_hash is an empty hex (0x). contractResult=${JSON.stringify(
record,
)}`,
);
}
throw predefined.INTERNAL_ERROR(
`The contract result response from the remote Mirror Node server is missing required fields. `,
);
}
}
}
9 changes: 7 additions & 2 deletions packages/relay/src/lib/services/ethService/ethCommonService/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,12 @@ export class CommonService implements ICommonService {

const logs: Log[] = [];
for (const log of logResults) {
if (log.block_number == null || log.index == null || log.block_hash === EthImpl.emptyHex) {
if (
log.transaction_index == null ||
log.block_number == null ||
log.index == null ||
log.block_hash === EthImpl.emptyHex
) {
if (this.logger.isLevelEnabled('debug')) {
this.logger.debug(
`${
Expand All @@ -371,7 +376,7 @@ export class CommonService implements ICommonService {
removed: false,
topics: log.topics,
transactionHash: toHash32(log.transaction_hash),
transactionIndex: nullableNumberTo0x(log.transaction_index),
transactionIndex: numberTo0x(log.transaction_index),
}),
);
}
Expand Down
33 changes: 17 additions & 16 deletions packages/relay/tests/lib/eth/eth_getLogs.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,17 @@
*
*/

import MockAdapter from 'axios-mock-adapter';
import { expect, use } from 'chai';
import sinon from 'sinon';
import chaiAsPromised from 'chai-as-promised';
import { ethers } from 'ethers';
import sinon from 'sinon';

import { Eth } from '../../../src';
import { SDKClient } from '../../../src/lib/clients';
import { CacheService } from '../../../src/lib/services/cacheService/cacheService';
import HAPIService from '../../../src/lib/services/hapiService/hapiService';
import { RequestDetails } from '../../../src/lib/types';
import {
defaultDetailedContractResults,
defaultDetailedContractResults2,
Expand All @@ -35,7 +43,6 @@ import {
overrideEnvsInMochaDescribe,
withOverriddenEnvsInMochaTest,
} from '../../helpers';
import { SDKClient } from '../../../src/lib/clients';
import {
BLOCK_HASH,
BLOCKS_LIMIT_ORDER_URL,
Expand All @@ -56,13 +63,7 @@ import {
DEFAULT_NULL_LOG_TOPICS,
NOT_FOUND_RES,
} from './eth-config';
import { ethers } from 'ethers';
import { generateEthTestEnv } from './eth-helpers';
import { RequestDetails } from '../../../src/lib/types';
import MockAdapter from 'axios-mock-adapter';
import HAPIService from '../../../src/lib/services/hapiService/hapiService';
import { Eth } from '../../../src';
import { CacheService } from '../../../src/lib/services/cacheService/cacheService';

use(chaiAsPromised);

Expand Down Expand Up @@ -174,7 +175,7 @@ describe('@ethGetLogs using MirrorNode', async function () {
expectLogData(result[3], filteredLogs.logs[3], defaultDetailedContractResults3);
});

it('no filters but undefined transaction_index', async function () {
it('should throw an error if transaction_index is falsy', async function () {
const filteredLogs = {
logs: [
{ ...DEFAULT_LOGS.logs[0], transaction_index: undefined },
Expand All @@ -189,13 +190,13 @@ describe('@ethGetLogs using MirrorNode', async function () {
restMock.onGet(`contracts/${log.address}`).reply(200, { ...DEFAULT_CONTRACT, contract_id: `0.0.105${index}` });
});

const result = await ethImpl.getLogs(null, 'latest', 'latest', null, null, requestDetails);
expect(result).to.exist;

expect(result.length).to.eq(4);
result.forEach((log, _index) => {
expect(log.transactionIndex).to.be.null;
});
try {
await ethImpl.getLogs(null, 'latest', 'latest', null, null, requestDetails);
expect.fail('should have thrown an error');
} catch (error) {
expect(error).to.exist;
expect(error.message).to.include('The log entry from the remote Mirror Node server is missing required fields.');
}
});

withOverriddenEnvsInMochaTest({ MIRROR_NODE_LIMIT_PARAM: '2' }, () => {
Expand Down
58 changes: 35 additions & 23 deletions packages/relay/tests/lib/eth/eth_getTransactionByHash.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@

import { expect, use } from 'chai';
import chaiAsPromised from 'chai-as-promised';
import { Transaction, Transaction2930, Transaction1559 } from '../../../src/lib/model';

import { Transaction, Transaction1559, Transaction2930 } from '../../../src/lib/model';
import { RequestDetails } from '../../../src/lib/types';
import RelayAssertions from '../../assertions';
import { defaultDetailedContractResultByHash, defaultFromLongZeroAddress } from '../../helpers';
import {
DEFAULT_DETAILED_CONTRACT_RESULT_BY_HASH_REVERTED,
DEFAULT_TRANSACTION,
Expand All @@ -30,14 +33,13 @@ import {
EMPTY_LOGS_RESPONSE,
NO_TRANSACTIONS,
} from './eth-config';
import { defaultDetailedContractResultByHash, defaultFromLongZeroAddress } from '../../helpers';
import { generateEthTestEnv } from './eth-helpers';
import { RequestDetails } from '../../../src/lib/types';

use(chaiAsPromised);

describe('@ethGetTransactionByHash eth_getTransactionByHash tests', async function () {
let { restMock, ethImpl } = generateEthTestEnv();
this.timeout(100000);
const { restMock, ethImpl } = generateEthTestEnv();
const from = '0x00000000000000000000000000000000000003f7';
const evm_address = '0xc37f417fa09933335240fca72dd257bfbde9c275';
const contractResultMock = {
Expand Down Expand Up @@ -217,37 +219,46 @@ describe('@ethGetTransactionByHash eth_getTransactionByHash tests', async functi
if (result) expect(result.v).to.eq('0x0');
});

it('handles transactions with undefined transaction_index', async function () {
it('should throw an error if transaction_index is falsy', async function () {
const detailedResultsWithNullNullableValues = {
...defaultDetailedContractResultByHash,
transaction_index: undefined,
};
const uniqueTxHash = '0x14aad7b827375d12d73af57b6a3e84353645fd31305ea58ff52dda53ec640534';

restMock.onGet(`contracts/results/${uniqueTxHash}`).reply(200, detailedResultsWithNullNullableValues);
const result = await ethImpl.getTransactionByHash(uniqueTxHash, requestDetails);
expect(result).to.not.be.null;

expect(result).to.exist;
if (result) expect(result.transactionIndex).to.be.null;
try {
await ethImpl.getTransactionByHash(uniqueTxHash, requestDetails);
expect.fail('should have thrown an error');
} catch (error) {
expect(error).to.exist;
expect(error.message).to.include(
'The contract result response from the remote Mirror Node server is missing required fields.',
);
}
});

it('handles transactions with undefined block_number', async function () {
it('should throw an error if block_number is falsy', async function () {
const detailedResultsWithNullNullableValues = {
...defaultDetailedContractResultByHash,
block_number: undefined,
};
const uniqueTxHash = '0x14aad7b827375d12d73af57b6a3e84353645fd31305ea58ff52dda53ec640511';

restMock.onGet(`contracts/results/${uniqueTxHash}`).reply(200, detailedResultsWithNullNullableValues);
const result = await ethImpl.getTransactionByHash(uniqueTxHash, requestDetails);
expect(result).to.not.be.null;

expect(result).to.exist;
if (result) expect(result.blockNumber).to.be.null;
try {
await ethImpl.getTransactionByHash(uniqueTxHash, requestDetails);
expect.fail('should have thrown an error');
} catch (error) {
expect(error).to.exist;
expect(error.message).to.include(
'The contract result response from the remote Mirror Node server is missing required fields.',
);
}
});

it('handles transactions with undefined transaction_index and block_number', async function () {
it('should throw an error if transaction_index and block_number are falsy', async function () {
const detailedResultsWithNullNullableValues = {
...defaultDetailedContractResultByHash,
block_number: undefined,
Expand All @@ -257,13 +268,14 @@ describe('@ethGetTransactionByHash eth_getTransactionByHash tests', async functi
const uniqueTxHash = '0x14aad7b827375d12d73af57b6a3e84353645fd31305ea58ff52d1a53ec640511';

restMock.onGet(`contracts/results/${uniqueTxHash}`).reply(200, detailedResultsWithNullNullableValues);
const result = await ethImpl.getTransactionByHash(uniqueTxHash, requestDetails);
expect(result).to.not.be.null;

expect(result).to.exist;
if (result) {
expect(result.blockNumber).to.be.null;
expect(result.transactionIndex).to.be.null;
try {
await ethImpl.getTransactionByHash(uniqueTxHash, requestDetails);
expect.fail('should have thrown an error');
} catch (error) {
expect(error).to.exist;
expect(error.message).to.include(
'The contract result response from the remote Mirror Node server is missing required fields.',
);
}
});

Expand Down
27 changes: 16 additions & 11 deletions packages/relay/tests/lib/eth/eth_getTransactionReceipt.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,22 @@
*/

import { expect, use } from 'chai';
import sinon, { createSandbox } from 'sinon';
import chaiAsPromised from 'chai-as-promised';
import { EthImpl } from '../../../src/lib/eth';
import sinon, { createSandbox } from 'sinon';

import constants from '../../../src/lib/constants';
import { EthImpl } from '../../../src/lib/eth';
import { RequestDetails } from '../../../src/lib/types';
import RelayAssertions from '../../assertions';
import { DEFAULT_BLOCK, EMPTY_LOGS_RESPONSE } from './eth-config';
import { defaultErrorMessageHex } from '../../helpers';
import { DEFAULT_BLOCK, EMPTY_LOGS_RESPONSE } from './eth-config';
import { generateEthTestEnv } from './eth-helpers';
import { RequestDetails } from '../../../src/lib/types';

use(chaiAsPromised);

describe('@ethGetTransactionReceipt eth_getTransactionReceipt tests', async function () {
this.timeout(10000);
let { restMock, ethImpl, cacheService } = generateEthTestEnv();
const { restMock, ethImpl, cacheService } = generateEthTestEnv();
let sandbox: sinon.SinonSandbox;

const requestDetails = new RequestDetails({ requestId: 'eth_getTransactionReceiptTest', ipAddress: '0.0.0.0' });
Expand Down Expand Up @@ -291,7 +292,7 @@ describe('@ethGetTransactionReceipt eth_getTransactionReceipt tests', async func
expect(receipt.gasUsed).to.eq('0x0');
});

it('handles missing transaction index', async function () {
it('should throw an error if transaction index is falsy', async function () {
// fake unique hash so request dont re-use the cached value but the mock defined
const uniqueTxHash = '0x17cad7b827375d12d73af57b6a3e84353645fd31305ea58ff52dda53ec640533';

Expand All @@ -306,12 +307,16 @@ describe('@ethGetTransactionReceipt eth_getTransactionReceipt tests', async func
evm_address: contractEvmAddress,
});
stubBlockAndFeesFunc(sandbox);
const receipt = await ethImpl.getTransactionReceipt(uniqueTxHash, requestDetails);

expect(receipt).to.exist;

expect(receipt.logs[0].transactionIndex).to.eq(null);
expect(receipt.transactionIndex).to.eq(null);
try {
await ethImpl.getTransactionReceipt(uniqueTxHash, requestDetails);
expect.fail('should have thrown an error');
} catch (error) {
expect(error).to.exist;
expect(error.message).to.include(
'The contract result response from the remote Mirror Node server is missing required fields.',
);
}
});

it('valid receipt on cache match', async function () {
Expand Down

0 comments on commit 256a28b

Please sign in to comment.