feat: enhanced eth_getLogs with timestamp range validation and new er…

…ror handling (#3431) * fix: returned empty array for eth_getLogs if toBlock is not found Signed-off-by: Logan Nguyen <[email protected]> * feat: added new predefined error TIMESTAMP_RANGE_TOO_LARGE Signed-off-by: Logan Nguyen <[email protected]> * fix: added validation to ensure the timestamp range between fromBlock and toBlock is valid Signed-off-by: Logan Nguyen <[email protected]> * chore: added comments to clarify the logic Signed-off-by: Logan Nguyen <[email protected]> * test: added unit test Signed-off-by: Logan Nguyen <[email protected]> * test: added acceptance test Signed-off-by: Logan Nguyen <[email protected]> * chore: updated a unique JsonRPC error code for TIMESTAMP_RANGE_TOO_LARGE Signed-off-by: Logan Nguyen <[email protected]> * chore: updated 7 timestamp validation Signed-off-by: Logan Nguyen <[email protected]> * chore: explained the logic behind MISSING_FROM_BLOCK_PARAM error Signed-off-by: Logan Nguyen <[email protected]> * feat: added a dedicated validateBlockRange for eth_newFilter Signed-off-by: Logan Nguyen <[email protected]> * fix: threw INVALID_BLOCK_RANGE if fromBlockNum is greater than toBlockNum Signed-off-by: Logan Nguyen <[email protected]> --------- Signed-off-by: Logan Nguyen <[email protected]>
hashgraph · Jan 29, 2025 · 2c31a0d · 2c31a0d
1 parent 214f1ef
commit 2c31a0d
Show file tree

Hide file tree

Showing 7 changed files with 188 additions and 24 deletions.
diff --git a/packages/relay/src/lib/errors/JsonRpcError.ts b/packages/relay/src/lib/errors/JsonRpcError.ts
@@ -133,6 +133,11 @@ export const predefined = {
       code: -32000,
       message: `Exceeded maximum block range: ${blockRange}`,
     }),
+  TIMESTAMP_RANGE_TOO_LARGE: (fromBlock: string, fromTimestamp: number, toBlock: string, toTimestamp: number) =>
+    new JsonRpcError({
+      code: -32004,
+      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})`,
+    }),
   REQUEST_BEYOND_HEAD_BLOCK: (requested: number, latest: number) =>
     new JsonRpcError({
       code: -32000,

diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts
@@ -2767,6 +2767,37 @@ export class EthImpl implements Eth {
     return await this.getAcccountNonceFromContractResult(address, blockNum, requestDetails);
   }
 
+  /**
+   * Retrieves logs based on the provided parameters.
+   *
+   * The function handles log retrieval as follows:
+   *
+   * - Using `blockHash`:
+   *   - If `blockHash` is provided, logs are retrieved based on the timestamp of the block associated with the `blockHash`.
+   *
+   * - Without `blockHash`:
+   *
+   *   - If only `fromBlock` is provided:
+   *     - Logs are retrieved from `fromBlock` to the latest block.
+   *     - If `fromBlock` does not exist, an empty array is returned.
+   *
+   *   - If only `toBlock` is provided:
+   *     - A predefined error `MISSING_FROM_BLOCK_PARAM` is thrown because `fromBlock` is required.
+   *
+   *   - If both `fromBlock` and `toBlock` are provided:
+   *     - Logs are retrieved from `fromBlock` to `toBlock`.
+   *     - If `toBlock` does not exist, an empty array is returned.
+   *     - If the timestamp range between `fromBlock` and `toBlock` exceeds 7 days, a predefined error `TIMESTAMP_RANGE_TOO_LARGE` is thrown.
+   *
+   * @param {string | null} blockHash - The block hash to prioritize log retrieval.
+   * @param {string | 'latest'} fromBlock - The starting block for log retrieval.
+   * @param {string | 'latest'} toBlock - The ending block for log retrieval.
+   * @param {string | string[] | null} address - The address(es) to filter logs by.
+   * @param {any[] | null} topics - The topics to filter logs by.
+   * @param {RequestDetails} requestDetails - The details of the request.
+   * @returns {Promise<Log[]>} - A promise that resolves to an array of logs or an empty array if no logs are found.
+   * @throws {Error} Throws specific errors like `MISSING_FROM_BLOCK_PARAM` or `TIMESTAMP_RANGE_TOO_LARGE` when applicable.
+   */
   async getLogs(
     blockHash: string | null,
     fromBlock: string | 'latest',

diff --git a/packages/relay/src/lib/services/ethService/ethCommonService/index.ts b/packages/relay/src/lib/services/ethService/ethCommonService/index.ts
@@ -22,7 +22,7 @@ import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services'
 import * as _ from 'lodash';
 import { Logger } from 'pino';
 
-import { numberTo0x, parseNumericEnvVar, toHash32 } from '../../../../formatters';
+import { numberTo0x, parseNumericEnvVar, prepend0x, toHash32 } from '../../../../formatters';
 import { MirrorNodeClient } from '../../../clients';
 import constants from '../../../constants';
 import { JsonRpcError, predefined } from '../../../errors/JsonRpcError';
@@ -78,6 +78,9 @@ export class CommonService implements ICommonService {
     'ETH_BLOCK_NUMBER_CACHE_TTL_MS_DEFAULT',
   );
 
+  // Maximum allowed timestamp range for mirror node requests' timestamp parameter is 7 days (604800 seconds)
+  private readonly maxTimestampParamRange = 604800; // 7 days
+
   private getLogsBlockRangeLimit() {
     return parseNumericEnvVar('ETH_GET_LOGS_BLOCK_RANGE_LIMIT', 'DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT');
   }
@@ -111,13 +114,16 @@ export class CommonService implements ICommonService {
   ) {
     if (this.blockTagIsLatestOrPending(toBlock)) {
       toBlock = CommonService.blockLatest;
-    }
-
-    const latestBlockNumber: string = await this.getLatestBlockNumber(requestDetails);
-
-    // toBlock is a number and is less than the current block number and fromBlock is not defined
-    if (Number(toBlock) < Number(latestBlockNumber) && !fromBlock) {
-      throw predefined.MISSING_FROM_BLOCK_PARAM;
+    } else {
+      const latestBlockNumber: string = await this.getLatestBlockNumber(requestDetails);
+
+      // - When `fromBlock` is not explicitly provided, it defaults to `latest`.
+      // - 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.
+      // - If `toBlock` is explicitly provided and does not equals to `latestBlockNumber`, it establishes a solid upper bound.
+      // - If `fromBlock` is missing, indicating the absence of a lower bound, throw the `MISSING_FROM_BLOCK_PARAM` error.
+      if (Number(toBlock) !== Number(latestBlockNumber) && !fromBlock) {
+        throw predefined.MISSING_FROM_BLOCK_PARAM;
+      }
     }
 
     if (this.blockTagIsLatestOrPending(fromBlock)) {
@@ -140,13 +146,34 @@ export class CommonService implements ICommonService {
     } else {
       fromBlockNum = parseInt(fromBlockResponse.number);
       const toBlockResponse = await this.getHistoricalBlockResponse(requestDetails, toBlock, true);
-      if (toBlockResponse != null) {
-        params.timestamp.push(`lte:${toBlockResponse.timestamp.to}`);
-        toBlockNum = parseInt(toBlockResponse.number);
+
+      /**
+       * If `toBlock` is not provided, the `lte` field cannot be set,
+       * resulting in a request to the Mirror Node that includes only the `gte` parameter.
+       * Such requests will be rejected, hence causing the whole request to fail.
+       * Return false to handle this gracefully and return an empty response to end client.
+       */
+      if (!toBlockResponse) {
+        return false;
+      }
+
+      params.timestamp.push(`lte:${toBlockResponse.timestamp.to}`);
+      toBlockNum = parseInt(toBlockResponse.number);
+
+      // Validate timestamp range for Mirror Node requests (maximum: 7 days or 604,800 seconds) to prevent exceeding the limit,
+      // as requests with timestamp parameters beyond 7 days are rejected by the Mirror Node.
+      const timestampDiff = toBlockResponse.timestamp.to - fromBlockResponse.timestamp.from;
+      if (timestampDiff > this.maxTimestampParamRange) {
+        throw predefined.TIMESTAMP_RANGE_TOO_LARGE(
+          prepend0x(fromBlockNum.toString(16)),
+          fromBlockResponse.timestamp.from,
+          prepend0x(toBlockNum.toString(16)),
+          toBlockResponse.timestamp.to,
+        );
       }
 
       if (fromBlockNum > toBlockNum) {
-        return false;
+        throw predefined.INVALID_BLOCK_RANGE;
       }
 
       const blockRangeLimit = this.getLogsBlockRangeLimit();
@@ -163,6 +190,53 @@ export class CommonService implements ICommonService {
     return true;
   }
 
+  public async validateBlockRange(fromBlock: string, toBlock: string, requestDetails: RequestDetails) {
+    let fromBlockNumber: any = null;
+    let toBlockNumber: any = null;
+
+    if (this.blockTagIsLatestOrPending(toBlock)) {
+      toBlock = CommonService.blockLatest;
+    } else {
+      toBlockNumber = Number(toBlock);
+
+      const latestBlockNumber: string = await this.getLatestBlockNumber(requestDetails);
+
+      // - When `fromBlock` is not explicitly provided, it defaults to `latest`.
+      // - 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.
+      // - If `toBlock` is explicitly provided and does not equals to `latestBlockNumber`, it establishes a solid upper bound.
+      // - If `fromBlock` is missing, indicating the absence of a lower bound, throw the `MISSING_FROM_BLOCK_PARAM` error.
+      if (Number(toBlock) !== Number(latestBlockNumber) && !fromBlock) {
+        throw predefined.MISSING_FROM_BLOCK_PARAM;
+      }
+    }
+
+    if (this.blockTagIsLatestOrPending(fromBlock)) {
+      fromBlock = CommonService.blockLatest;
+    } else {
+      fromBlockNumber = Number(fromBlock);
+    }
+
+    // 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.
+    if (!fromBlockNumber || !toBlockNumber) {
+      const fromBlockResponse = await this.getHistoricalBlockResponse(requestDetails, fromBlock, true);
+      const toBlockResponse = await this.getHistoricalBlockResponse(requestDetails, toBlock, true);
+
+      if (fromBlockResponse) {
+        fromBlockNumber = parseInt(fromBlockResponse.number);
+      }
+
+      if (toBlockResponse) {
+        toBlockNumber = parseInt(toBlockResponse.number);
+      }
+    }
+
+    if (fromBlockNumber > toBlockNumber) {
+      throw predefined.INVALID_BLOCK_RANGE;
+    }
+
+    return true;
+  }
+
   /**
    * returns the block response
    * otherwise return undefined.

diff --git a/packages/relay/src/lib/services/ethService/ethFilterService/index.ts b/packages/relay/src/lib/services/ethService/ethFilterService/index.ts
@@ -134,9 +134,7 @@ export class FilterService implements IFilterService {
     try {
       FilterService.requireFiltersEnabled();
 
-      if (
-        !(await this.common.validateBlockRangeAndAddTimestampToParams({}, fromBlock, toBlock, requestDetails, address))
-      ) {
+      if (!(await this.common.validateBlockRange(fromBlock, toBlock, requestDetails))) {
         throw predefined.INVALID_BLOCK_RANGE;
       }
 

diff --git a/packages/relay/tests/lib/eth/eth_getLogs.spec.ts b/packages/relay/tests/lib/eth/eth_getLogs.spec.ts
@@ -45,6 +45,8 @@ import {
 } from '../../helpers';
 import {
   BLOCK_HASH,
+  BLOCK_NUMBER_2,
+  BLOCK_NUMBER_3,
   BLOCKS_LIMIT_ORDER_URL,
   CONTRACT_ADDRESS_1,
   CONTRACT_ADDRESS_2,
@@ -430,7 +432,7 @@ describe('@ethGetLogs using MirrorNode', async function () {
     expect(result).to.be.empty;
   });
 
-  it('with non-existing toBlock filter', async function () {
+  it('should return empty response if toBlock is not existed', async function () {
     const filteredLogs = {
       logs: [DEFAULT_LOGS.logs[0]],
     };
@@ -446,7 +448,7 @@ describe('@ethGetLogs using MirrorNode', async function () {
     const result = await ethImpl.getLogs(null, '0x5', '0x10', null, null, requestDetails);
 
     expect(result).to.exist;
-    expectLogData1(result[0]);
+    expect(result).to.be.empty;
   });
 
   it('when fromBlock > toBlock', async function () {
@@ -462,10 +464,10 @@ describe('@ethGetLogs using MirrorNode', async function () {
     restMock.onGet(BLOCKS_LIMIT_ORDER_URL).reply(200, { blocks: [latestBlock] });
     restMock.onGet('blocks/16').reply(200, fromBlock);
     restMock.onGet('blocks/5').reply(200, DEFAULT_BLOCK);
-    const result = await ethImpl.getLogs(null, '0x10', '0x5', null, null, requestDetails);
 
-    expect(result).to.exist;
-    expect(result).to.be.empty;
+    await expect(ethImpl.getLogs(null, '0x10', '0x5', null, null, requestDetails)).to.be.rejectedWith(
+      predefined.INVALID_BLOCK_RANGE.message,
+    );
   });
 
   it('with only toBlock', async function () {
@@ -608,4 +610,40 @@ describe('@ethGetLogs using MirrorNode', async function () {
     expect(result.length).to.eq(0);
     expect(result).to.deep.equal([]);
   });
+
+  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 () => {
+    const mockedFromTimeStamp = 1651560389;
+    const mockedToTimeStamp = mockedFromTimeStamp + 604800 * 2 + 1; // 7 days (604800 seconds) and 1 second greater than mockedFromTimeStamp
+
+    restMock.onGet(BLOCKS_LIMIT_ORDER_URL).reply(200, { blocks: [latestBlock] });
+    restMock.onGet(`blocks/${BLOCK_NUMBER_2}`).reply(200, {
+      ...DEFAULT_BLOCK,
+      timestamp: { ...DEFAULT_BLOCK.timestamp, from: mockedFromTimeStamp.toString() },
+      number: BLOCK_NUMBER_2,
+    });
+
+    restMock.onGet(`blocks/${BLOCK_NUMBER_3}`).reply(200, {
+      ...DEFAULT_BLOCK,
+      timestamp: { ...DEFAULT_BLOCK.timestamp, to: mockedToTimeStamp.toString() },
+      number: BLOCK_NUMBER_3,
+    });
+
+    await expect(
+      ethImpl.getLogs(
+        null,
+        BLOCK_NUMBER_2.toString(16),
+        BLOCK_NUMBER_3.toString(16),
+        ethers.ZeroAddress,
+        DEFAULT_LOG_TOPICS,
+        requestDetails,
+      ),
+    ).to.be.rejectedWith(
+      predefined.TIMESTAMP_RANGE_TOO_LARGE(
+        `0x${BLOCK_NUMBER_2.toString(16)}`,
+        mockedFromTimeStamp,
+        `0x${BLOCK_NUMBER_3.toString(16)}`,
+        mockedToTimeStamp,
+      ).message,
+    );
+  });
 });
diff --git a/packages/relay/tests/lib/services/eth/filter.spec.ts b/packages/relay/tests/lib/services/eth/filter.spec.ts
@@ -274,7 +274,7 @@ describe('Filter API Test Suite', async function () {
     });
 
     it('validates fromBlock and toBlock', async function () {
-      // fromBlock is larger than toBlock
+      // reject if fromBlock is larger than toBlock
       await RelayAssertions.assertRejection(
         predefined.INVALID_BLOCK_RANGE,
         filterService.newFilter,
@@ -290,13 +290,13 @@ describe('Filter API Test Suite', async function () {
         ['latest', blockNumberHexes[1400], requestDetails],
       );
 
-      // block range is too large
+      // reject when no fromBlock is provided
       await RelayAssertions.assertRejection(
-        predefined.RANGE_TOO_LARGE(1000),
+        predefined.MISSING_FROM_BLOCK_PARAM,
         filterService.newFilter,
         true,
         filterService,
-        [blockNumberHexes[5], blockNumberHexes[2000], requestDetails],
+        [null, blockNumberHexes[1400], requestDetails],
       );
 
       // block range is valid

diff --git a/packages/server/tests/acceptance/rpc_batch1.spec.ts b/packages/server/tests/acceptance/rpc_batch1.spec.ts
@@ -317,6 +317,24 @@ describe('@api-batch-1 RPC Server Acceptance Tests', function () {
         }
       });
 
+      it('should return empty logs if `toBlock` is not found', async () => {
+        const notExistedLog = latestBlock + 99;
+
+        const logs = await relay.call(
+          RelayCalls.ETH_ENDPOINTS.ETH_GET_LOGS,
+          [
+            {
+              fromBlock: log0Block.blockNumber,
+              toBlock: `0x${notExistedLog.toString(16)}`,
+              address: [contractAddress, contractAddress2],
+            },
+          ],
+          requestIdPrefix,
+        );
+
+        expect(logs.length).to.eq(0);
+      });
+
       it('should be able to use `address` param', async () => {
         //when we pass only address, it defaults to the latest block
         const logs = await relay.call(