Skip to content

Commit 7c41c62

Browse files
quiet-nodequiet-node
authored
chore: cherry pick feat: enhanced eth_getLogs with timestamp range validation and new error handling (#3431) to release/0.64 (#3434)
feat: enhanced eth_getLogs with timestamp range validation and new error handling (#3431) * fix: returned empty array for eth_getLogs if toBlock is not found * feat: added new predefined error TIMESTAMP_RANGE_TOO_LARGE * fix: added validation to ensure the timestamp range between fromBlock and toBlock is valid * chore: added comments to clarify the logic * test: added unit test * test: added acceptance test * chore: updated a unique JsonRPC error code for TIMESTAMP_RANGE_TOO_LARGE * chore: updated 7 timestamp validation * chore: explained the logic behind MISSING_FROM_BLOCK_PARAM error * feat: added a dedicated validateBlockRange for eth_newFilter * fix: threw INVALID_BLOCK_RANGE if fromBlockNum is greater than toBlockNum --------- Signed-off-by: Logan Nguyen <[email protected]>
1 parent 214f1ef commit 7c41c62

File tree

7 files changed

+188
-24
lines changed

7 files changed

+188
-24
lines changed

‎packages/relay/src/lib/errors/JsonRpcError.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,11 @@ export const predefined = {
133133
code: -32000,
134134
message: `Exceeded maximum block range: ${blockRange}`,
135135
}),
136+
TIMESTAMP_RANGE_TOO_LARGE: (fromBlock: string, fromTimestamp: number, toBlock: string, toTimestamp: number) =>
137+
new JsonRpcError({
138+
code: -32004,
139+
message: `The provided fromBlock and toBlock contain timestamps that exceed the maximum allowed duration of 7 days (604800 seconds): fromBlock: ${fromBlock} (${fromTimestamp}), toBlock: ${toBlock} (${toTimestamp})`,
140+
}),
136141
REQUEST_BEYOND_HEAD_BLOCK: (requested: number, latest: number) =>
137142
new JsonRpcError({
138143
code: -32000,

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2767,6 +2767,37 @@ export class EthImpl implements Eth {
27672767
return await this.getAcccountNonceFromContractResult(address, blockNum, requestDetails);
27682768
}
27692769

2770+
/**
2771+
* Retrieves logs based on the provided parameters.
2772+
*
2773+
* The function handles log retrieval as follows:
2774+
*
2775+
* - Using `blockHash`:
2776+
* - If `blockHash` is provided, logs are retrieved based on the timestamp of the block associated with the `blockHash`.
2777+
*
2778+
* - Without `blockHash`:
2779+
*
2780+
* - If only `fromBlock` is provided:
2781+
* - Logs are retrieved from `fromBlock` to the latest block.
2782+
* - If `fromBlock` does not exist, an empty array is returned.
2783+
*
2784+
* - If only `toBlock` is provided:
2785+
* - A predefined error `MISSING_FROM_BLOCK_PARAM` is thrown because `fromBlock` is required.
2786+
*
2787+
* - If both `fromBlock` and `toBlock` are provided:
2788+
* - Logs are retrieved from `fromBlock` to `toBlock`.
2789+
* - If `toBlock` does not exist, an empty array is returned.
2790+
* - If the timestamp range between `fromBlock` and `toBlock` exceeds 7 days, a predefined error `TIMESTAMP_RANGE_TOO_LARGE` is thrown.
2791+
*
2792+
* @param {string | null} blockHash - The block hash to prioritize log retrieval.
2793+
* @param {string | 'latest'} fromBlock - The starting block for log retrieval.
2794+
* @param {string | 'latest'} toBlock - The ending block for log retrieval.
2795+
* @param {string | string[] | null} address - The address(es) to filter logs by.
2796+
* @param {any[] | null} topics - The topics to filter logs by.
2797+
* @param {RequestDetails} requestDetails - The details of the request.
2798+
* @returns {Promise<Log[]>} - A promise that resolves to an array of logs or an empty array if no logs are found.
2799+
* @throws {Error} Throws specific errors like `MISSING_FROM_BLOCK_PARAM` or `TIMESTAMP_RANGE_TOO_LARGE` when applicable.
2800+
*/
27702801
async getLogs(
27712802
blockHash: string | null,
27722803
fromBlock: string | 'latest',

‎packages/relay/src/lib/services/ethService/ethCommonService/index.ts‎

Lines changed: 86 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services'
2222
import * as _ from 'lodash';
2323
import { Logger } from 'pino';
2424

25-
import { numberTo0x, parseNumericEnvVar, toHash32 } from '../../../../formatters';
25+
import { numberTo0x, parseNumericEnvVar, prepend0x, toHash32 } from '../../../../formatters';
2626
import { MirrorNodeClient } from '../../../clients';
2727
import constants from '../../../constants';
2828
import { JsonRpcError, predefined } from '../../../errors/JsonRpcError';
@@ -78,6 +78,9 @@ export class CommonService implements ICommonService {
7878
'ETH_BLOCK_NUMBER_CACHE_TTL_MS_DEFAULT',
7979
);
8080

81+
// Maximum allowed timestamp range for mirror node requests' timestamp parameter is 7 days (604800 seconds)
82+
private readonly maxTimestampParamRange = 604800; // 7 days
83+
8184
private getLogsBlockRangeLimit() {
8285
return parseNumericEnvVar('ETH_GET_LOGS_BLOCK_RANGE_LIMIT', 'DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT');
8386
}
@@ -111,13 +114,16 @@ export class CommonService implements ICommonService {
111114
) {
112115
if (this.blockTagIsLatestOrPending(toBlock)) {
113116
toBlock = CommonService.blockLatest;
114-
}
115-
116-
const latestBlockNumber: string = await this.getLatestBlockNumber(requestDetails);
117-
118-
// toBlock is a number and is less than the current block number and fromBlock is not defined
119-
if (Number(toBlock) < Number(latestBlockNumber) && !fromBlock) {
120-
throw predefined.MISSING_FROM_BLOCK_PARAM;
117+
} else {
118+
const latestBlockNumber: string = await this.getLatestBlockNumber(requestDetails);
119+
120+
// - When `fromBlock` is not explicitly provided, it defaults to `latest`.
121+
// - Then if `toBlock` equals `latestBlockNumber`, it means both `toBlock` and `fromBlock` essentially refer to the latest block, so the `MISSING_FROM_BLOCK_PARAM` error is not necessary.
122+
// - If `toBlock` is explicitly provided and does not equals to `latestBlockNumber`, it establishes a solid upper bound.
123+
// - If `fromBlock` is missing, indicating the absence of a lower bound, throw the `MISSING_FROM_BLOCK_PARAM` error.
124+
if (Number(toBlock) !== Number(latestBlockNumber) && !fromBlock) {
125+
throw predefined.MISSING_FROM_BLOCK_PARAM;
126+
}
121127
}
122128

123129
if (this.blockTagIsLatestOrPending(fromBlock)) {
@@ -140,13 +146,34 @@ export class CommonService implements ICommonService {
140146
} else {
141147
fromBlockNum = parseInt(fromBlockResponse.number);
142148
const toBlockResponse = await this.getHistoricalBlockResponse(requestDetails, toBlock, true);
143-
if (toBlockResponse != null) {
144-
params.timestamp.push(`lte:${toBlockResponse.timestamp.to}`);
145-
toBlockNum = parseInt(toBlockResponse.number);
149+
150+
/**
151+
* If `toBlock` is not provided, the `lte` field cannot be set,
152+
* resulting in a request to the Mirror Node that includes only the `gte` parameter.
153+
* Such requests will be rejected, hence causing the whole request to fail.
154+
* Return false to handle this gracefully and return an empty response to end client.
155+
*/
156+
if (!toBlockResponse) {
157+
return false;
158+
}
159+
160+
params.timestamp.push(`lte:${toBlockResponse.timestamp.to}`);
161+
toBlockNum = parseInt(toBlockResponse.number);
162+
163+
// Validate timestamp range for Mirror Node requests (maximum: 7 days or 604,800 seconds) to prevent exceeding the limit,
164+
// as requests with timestamp parameters beyond 7 days are rejected by the Mirror Node.
165+
const timestampDiff = toBlockResponse.timestamp.to - fromBlockResponse.timestamp.from;
166+
if (timestampDiff > this.maxTimestampParamRange) {
167+
throw predefined.TIMESTAMP_RANGE_TOO_LARGE(
168+
prepend0x(fromBlockNum.toString(16)),
169+
fromBlockResponse.timestamp.from,
170+
prepend0x(toBlockNum.toString(16)),
171+
toBlockResponse.timestamp.to,
172+
);
146173
}
147174

148175
if (fromBlockNum > toBlockNum) {
149-
return false;
176+
throw predefined.INVALID_BLOCK_RANGE;
150177
}
151178

152179
const blockRangeLimit = this.getLogsBlockRangeLimit();
@@ -163,6 +190,53 @@ export class CommonService implements ICommonService {
163190
return true;
164191
}
165192

193+
public async validateBlockRange(fromBlock: string, toBlock: string, requestDetails: RequestDetails) {
194+
let fromBlockNumber: any = null;
195+
let toBlockNumber: any = null;
196+
197+
if (this.blockTagIsLatestOrPending(toBlock)) {
198+
toBlock = CommonService.blockLatest;
199+
} else {
200+
toBlockNumber = Number(toBlock);
201+
202+
const latestBlockNumber: string = await this.getLatestBlockNumber(requestDetails);
203+
204+
// - When `fromBlock` is not explicitly provided, it defaults to `latest`.
205+
// - Then if `toBlock` equals `latestBlockNumber`, it means both `toBlock` and `fromBlock` essentially refer to the latest block, so the `MISSING_FROM_BLOCK_PARAM` error is not necessary.
206+
// - If `toBlock` is explicitly provided and does not equals to `latestBlockNumber`, it establishes a solid upper bound.
207+
// - If `fromBlock` is missing, indicating the absence of a lower bound, throw the `MISSING_FROM_BLOCK_PARAM` error.
208+
if (Number(toBlock) !== Number(latestBlockNumber) && !fromBlock) {
209+
throw predefined.MISSING_FROM_BLOCK_PARAM;
210+
}
211+
}
212+
213+
if (this.blockTagIsLatestOrPending(fromBlock)) {
214+
fromBlock = CommonService.blockLatest;
215+
} else {
216+
fromBlockNumber = Number(fromBlock);
217+
}
218+
219+
// If either or both fromBlockNumber and toBlockNumber are not set, it means fromBlock and/or toBlock is set to latest, involve MN to retrieve their block number.
220+
if (!fromBlockNumber || !toBlockNumber) {
221+
const fromBlockResponse = await this.getHistoricalBlockResponse(requestDetails, fromBlock, true);
222+
const toBlockResponse = await this.getHistoricalBlockResponse(requestDetails, toBlock, true);
223+
224+
if (fromBlockResponse) {
225+
fromBlockNumber = parseInt(fromBlockResponse.number);
226+
}
227+
228+
if (toBlockResponse) {
229+
toBlockNumber = parseInt(toBlockResponse.number);
230+
}
231+
}
232+
233+
if (fromBlockNumber > toBlockNumber) {
234+
throw predefined.INVALID_BLOCK_RANGE;
235+
}
236+
237+
return true;
238+
}
239+
166240
/**
167241
* returns the block response
168242
* otherwise return undefined.

‎packages/relay/src/lib/services/ethService/ethFilterService/index.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,7 @@ export class FilterService implements IFilterService {
134134
try {
135135
FilterService.requireFiltersEnabled();
136136

137-
if (
138-
!(await this.common.validateBlockRangeAndAddTimestampToParams({}, fromBlock, toBlock, requestDetails, address))
139-
) {
137+
if (!(await this.common.validateBlockRange(fromBlock, toBlock, requestDetails))) {
140138
throw predefined.INVALID_BLOCK_RANGE;
141139
}
142140

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

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ import {
4545
} from '../../helpers';
4646
import {
4747
BLOCK_HASH,
48+
BLOCK_NUMBER_2,
49+
BLOCK_NUMBER_3,
4850
BLOCKS_LIMIT_ORDER_URL,
4951
CONTRACT_ADDRESS_1,
5052
CONTRACT_ADDRESS_2,
@@ -430,7 +432,7 @@ describe('@ethGetLogs using MirrorNode', async function () {
430432
expect(result).to.be.empty;
431433
});
432434

433-
it('with non-existing toBlock filter', async function () {
435+
it('should return empty response if toBlock is not existed', async function () {
434436
const filteredLogs = {
435437
logs: [DEFAULT_LOGS.logs[0]],
436438
};
@@ -446,7 +448,7 @@ describe('@ethGetLogs using MirrorNode', async function () {
446448
const result = await ethImpl.getLogs(null, '0x5', '0x10', null, null, requestDetails);
447449

448450
expect(result).to.exist;
449-
expectLogData1(result[0]);
451+
expect(result).to.be.empty;
450452
});
451453

452454
it('when fromBlock > toBlock', async function () {
@@ -462,10 +464,10 @@ describe('@ethGetLogs using MirrorNode', async function () {
462464
restMock.onGet(BLOCKS_LIMIT_ORDER_URL).reply(200, { blocks: [latestBlock] });
463465
restMock.onGet('blocks/16').reply(200, fromBlock);
464466
restMock.onGet('blocks/5').reply(200, DEFAULT_BLOCK);
465-
const result = await ethImpl.getLogs(null, '0x10', '0x5', null, null, requestDetails);
466467

467-
expect(result).to.exist;
468-
expect(result).to.be.empty;
468+
await expect(ethImpl.getLogs(null, '0x10', '0x5', null, null, requestDetails)).to.be.rejectedWith(
469+
predefined.INVALID_BLOCK_RANGE.message,
470+
);
469471
});
470472

471473
it('with only toBlock', async function () {
@@ -608,4 +610,40 @@ describe('@ethGetLogs using MirrorNode', async function () {
608610
expect(result.length).to.eq(0);
609611
expect(result).to.deep.equal([]);
610612
});
613+
614+
it('Should throw TIMESTAMP_RANGE_TOO_LARGE predefined error if timestamp range between fromBlock and toBlock exceed the maximum allowed duration of 7 days', async () => {
615+
const mockedFromTimeStamp = 1651560389;
616+
const mockedToTimeStamp = mockedFromTimeStamp + 604800 * 2 + 1; // 7 days (604800 seconds) and 1 second greater than mockedFromTimeStamp
617+
618+
restMock.onGet(BLOCKS_LIMIT_ORDER_URL).reply(200, { blocks: [latestBlock] });
619+
restMock.onGet(`blocks/${BLOCK_NUMBER_2}`).reply(200, {
620+
...DEFAULT_BLOCK,
621+
timestamp: { ...DEFAULT_BLOCK.timestamp, from: mockedFromTimeStamp.toString() },
622+
number: BLOCK_NUMBER_2,
623+
});
624+
625+
restMock.onGet(`blocks/${BLOCK_NUMBER_3}`).reply(200, {
626+
...DEFAULT_BLOCK,
627+
timestamp: { ...DEFAULT_BLOCK.timestamp, to: mockedToTimeStamp.toString() },
628+
number: BLOCK_NUMBER_3,
629+
});
630+
631+
await expect(
632+
ethImpl.getLogs(
633+
null,
634+
BLOCK_NUMBER_2.toString(16),
635+
BLOCK_NUMBER_3.toString(16),
636+
ethers.ZeroAddress,
637+
DEFAULT_LOG_TOPICS,
638+
requestDetails,
639+
),
640+
).to.be.rejectedWith(
641+
predefined.TIMESTAMP_RANGE_TOO_LARGE(
642+
`0x${BLOCK_NUMBER_2.toString(16)}`,
643+
mockedFromTimeStamp,
644+
`0x${BLOCK_NUMBER_3.toString(16)}`,
645+
mockedToTimeStamp,
646+
).message,
647+
);
648+
});
611649
});

‎packages/relay/tests/lib/services/eth/filter.spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ describe('Filter API Test Suite', async function () {
274274
});
275275

276276
it('validates fromBlock and toBlock', async function () {
277-
// fromBlock is larger than toBlock
277+
// reject if fromBlock is larger than toBlock
278278
await RelayAssertions.assertRejection(
279279
predefined.INVALID_BLOCK_RANGE,
280280
filterService.newFilter,
@@ -290,13 +290,13 @@ describe('Filter API Test Suite', async function () {
290290
['latest', blockNumberHexes[1400], requestDetails],
291291
);
292292

293-
// block range is too large
293+
// reject when no fromBlock is provided
294294
await RelayAssertions.assertRejection(
295-
predefined.RANGE_TOO_LARGE(1000),
295+
predefined.MISSING_FROM_BLOCK_PARAM,
296296
filterService.newFilter,
297297
true,
298298
filterService,
299-
[blockNumberHexes[5], blockNumberHexes[2000], requestDetails],
299+
[null, blockNumberHexes[1400], requestDetails],
300300
);
301301

302302
// block range is valid

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,24 @@ describe('@api-batch-1 RPC Server Acceptance Tests', function () {
317317
}
318318
});
319319

320+
it('should return empty logs if `toBlock` is not found', async () => {
321+
const notExistedLog = latestBlock + 99;
322+
323+
const logs = await relay.call(
324+
RelayCalls.ETH_ENDPOINTS.ETH_GET_LOGS,
325+
[
326+
{
327+
fromBlock: log0Block.blockNumber,
328+
toBlock: `0x${notExistedLog.toString(16)}`,
329+
address: [contractAddress, contractAddress2],
330+
},
331+
],
332+
requestIdPrefix,
333+
);
334+
335+
expect(logs.length).to.eq(0);
336+
});
337+
320338
it('should be able to use `address` param', async () => {
321339
//when we pass only address, it defaults to the latest block
322340
const logs = await relay.call(

0 commit comments

Comments
 (0)