From 79c0700bfa52321cbbb3ac3b2dfb90e28c8a5ccd Mon Sep 17 00:00:00 2001 From: Luis Mastrangelo Date: Thu, 21 Mar 2024 18:00:54 +0100 Subject: [PATCH 01/11] Fix address type Signed-off-by: Luis Mastrangelo --- packages/relay/src/index.ts | 2 +- packages/relay/src/lib/eth.ts | 2 +- .../src/lib/services/ethService/ethCommonService/index.ts | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/relay/src/index.ts b/packages/relay/src/index.ts index b0027a6200..b6b6c8150e 100644 --- a/packages/relay/src/index.ts +++ b/packages/relay/src/index.ts @@ -89,7 +89,7 @@ export interface Eth { blockHash: string | null, fromBlock: string | null, toBlock: string | null, - address: string | null, + address: string | string[] | null, topics: any[] | null, requestId?: string, ): Promise; diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts index 81353b6ffd..4c960851ec 100644 --- a/packages/relay/src/lib/eth.ts +++ b/packages/relay/src/lib/eth.ts @@ -2287,7 +2287,7 @@ export class EthImpl implements Eth { blockHash: string | null, fromBlock: string | 'latest', toBlock: string | 'latest', - address: string | [string] | null, + address: string | string[] | null, topics: any[] | null, requestIdPrefix?: string, ): Promise { diff --git a/packages/relay/src/lib/services/ethService/ethCommonService/index.ts b/packages/relay/src/lib/services/ethService/ethCommonService/index.ts index 3b490ed98b..95174eae9a 100644 --- a/packages/relay/src/lib/services/ethService/ethCommonService/index.ts +++ b/packages/relay/src/lib/services/ethService/ethCommonService/index.ts @@ -268,7 +268,7 @@ export class CommonService implements ICommonService { } } - public async getLogsByAddress(address: string | [string], params: any, requestIdPrefix) { + public async getLogsByAddress(address: string | string[], params: any, requestIdPrefix) { const addresses = Array.isArray(address) ? address : [address]; const logPromises = addresses.map((addr) => this.mirrorNodeClient.getContractResultsLogsByAddress(addr, params, undefined, requestIdPrefix), @@ -283,7 +283,7 @@ export class CommonService implements ICommonService { return logs; } - public async getLogsWithParams(address: string | [string] | null, params, requestIdPrefix?: string): Promise { + public async getLogsWithParams(address: string | string[] | null, params, requestIdPrefix?: string): Promise { const EMPTY_RESPONSE = []; let logResults; @@ -321,7 +321,7 @@ export class CommonService implements ICommonService { blockHash: string | null, fromBlock: string | 'latest', toBlock: string | 'latest', - address: string | [string] | null, + address: string | string[] | null, topics: any[] | null, requestIdPrefix?: string, ): Promise { From cd4805ef45d836663365bf4863200ed9050f173f Mon Sep 17 00:00:00 2001 From: Luis Mastrangelo Date: Thu, 21 Mar 2024 21:23:33 +0100 Subject: [PATCH 02/11] Remove block range limit when filtering with a single address Signed-off-by: Luis Mastrangelo --- .../ethService/ethCommonService/index.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/relay/src/lib/services/ethService/ethCommonService/index.ts b/packages/relay/src/lib/services/ethService/ethCommonService/index.ts index 95174eae9a..480c0f4193 100644 --- a/packages/relay/src/lib/services/ethService/ethCommonService/index.ts +++ b/packages/relay/src/lib/services/ethService/ethCommonService/index.ts @@ -99,12 +99,8 @@ export class CommonService implements ICommonService { fromBlock: string, toBlock: string, requestIdPrefix?: string, + address?: string | string[] | null, ) { - const blockRangeLimit = - process.env.TEST === 'true' - ? constants.DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT - : Number(process.env.ETH_GET_LOGS_BLOCK_RANGE_LIMIT); - if (this.blockTagIsLatestOrPending(toBlock)) { toBlock = CommonService.blockLatest; } @@ -141,7 +137,14 @@ export class CommonService implements ICommonService { if (fromBlockNum > toBlockNum) { return false; - } else if (toBlockNum - fromBlockNum > blockRangeLimit) { + } + + const blockRangeLimit = + process.env.TEST === 'true' + ? constants.DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT + : Number(process.env.ETH_GET_LOGS_BLOCK_RANGE_LIMIT); + const isSingleAddress = Array.isArray(address) ? address.length === 1 : address !== ''; + if (!isSingleAddress || toBlockNum - fromBlockNum > blockRangeLimit) { throw predefined.RANGE_TOO_LARGE(blockRangeLimit); } } @@ -332,7 +335,9 @@ export class CommonService implements ICommonService { if (!(await this.validateBlockHashAndAddTimestampToParams(params, blockHash, requestIdPrefix))) { return EMPTY_RESPONSE; } - } else if (!(await this.validateBlockRangeAndAddTimestampToParams(params, fromBlock, toBlock, requestIdPrefix))) { + } else if ( + !(await this.validateBlockRangeAndAddTimestampToParams(params, fromBlock, toBlock, requestIdPrefix, address)) + ) { return EMPTY_RESPONSE; } From f7d1e72eb65e889a753abfaa7430eacd4eee8525 Mon Sep 17 00:00:00 2001 From: Luis Mastrangelo Date: Thu, 21 Mar 2024 23:34:20 +0100 Subject: [PATCH 03/11] Fix single address condition Signed-off-by: Luis Mastrangelo --- .../src/lib/services/ethService/ethCommonService/index.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/relay/src/lib/services/ethService/ethCommonService/index.ts b/packages/relay/src/lib/services/ethService/ethCommonService/index.ts index 480c0f4193..d7b983b196 100644 --- a/packages/relay/src/lib/services/ethService/ethCommonService/index.ts +++ b/packages/relay/src/lib/services/ethService/ethCommonService/index.ts @@ -143,8 +143,10 @@ export class CommonService implements ICommonService { process.env.TEST === 'true' ? constants.DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT : Number(process.env.ETH_GET_LOGS_BLOCK_RANGE_LIMIT); - const isSingleAddress = Array.isArray(address) ? address.length === 1 : address !== ''; - if (!isSingleAddress || toBlockNum - fromBlockNum > blockRangeLimit) { + const isSingleAddress = Array.isArray(address) + ? address.length === 1 + : typeof address === 'string' && address !== ''; + if (!isSingleAddress && toBlockNum - fromBlockNum > blockRangeLimit) { throw predefined.RANGE_TOO_LARGE(blockRangeLimit); } } From a31d83b960f607132501a8b20dff35ca0f6af0f8 Mon Sep 17 00:00:00 2001 From: Luis Mastrangelo Date: Thu, 21 Mar 2024 23:35:12 +0100 Subject: [PATCH 04/11] Add unit tests Signed-off-by: Luis Mastrangelo --- .../relay/tests/lib/eth/eth_getLogs.spec.ts | 84 ++++++++++++++----- 1 file changed, 63 insertions(+), 21 deletions(-) diff --git a/packages/relay/tests/lib/eth/eth_getLogs.spec.ts b/packages/relay/tests/lib/eth/eth_getLogs.spec.ts index 3d0b12f65a..d82970a80c 100644 --- a/packages/relay/tests/lib/eth/eth_getLogs.spec.ts +++ b/packages/relay/tests/lib/eth/eth_getLogs.spec.ts @@ -281,6 +281,46 @@ describe('@ethGetLogs using MirrorNode', async function () { expectLogData3(result[2]); }); + [CONTRACT_ADDRESS_1, [CONTRACT_ADDRESS_1]].forEach((address) => { + console.log(address); + it(`single address filter \`${JSON.stringify(address)}\` and large block range`, async function () { + const filteredLogs = { + logs: [DEFAULT_LOGS.logs[0], DEFAULT_LOGS.logs[1], DEFAULT_LOGS.logs[2]], + }; + restMock.onGet(CONTRACTS_LOGS_WITH_FILTER).reply(200, filteredLogs); + for (const log of filteredLogs.logs) { + restMock.onGet(`contracts/${log.address}`).reply(200, DEFAULT_CONTRACT); + } + + const fromBlock = { + ...DEFAULT_BLOCK, + number: 1, + }; + const toBlock = { + ...DEFAULT_BLOCK, + number: 1003, + }; + + const blockBeyondMaximumRange = { + ...DEFAULT_BLOCK, + number: 1007, + }; + + restMock.onGet(BLOCKS_LIMIT_ORDER_URL).reply(200, { blocks: [blockBeyondMaximumRange] }); + restMock.onGet('blocks/1').reply(200, fromBlock); + restMock.onGet('blocks/1003').reply(200, toBlock); + + const result = await ethImpl.getLogs(null, '0x1', '0x3eb', address, null); + + expect(result).to.exist; + + expect(result.length).to.eq(3); + expectLogData1(result[0]); + expectLogData2(result[1]); + expectLogData3(result[2]); + }); + }); + it('multiple addresses filter', async function () { const filteredLogsAddress1 = { logs: [DEFAULT_LOGS.logs[0], DEFAULT_LOGS.logs[1], DEFAULT_LOGS.logs[2]], @@ -439,27 +479,29 @@ describe('@ethGetLogs using MirrorNode', async function () { expectLogData1(result[0]); }); - it('when block range is too large', async function () { - const fromBlock = { - ...DEFAULT_BLOCK, - number: 1, - }; - const toBlock = { - ...DEFAULT_BLOCK, - number: 1003, - }; - - const blockBeyondMaximumRange = { - ...DEFAULT_BLOCK, - number: 1007, - }; - - restMock.onGet(BLOCKS_LIMIT_ORDER_URL).reply(200, { blocks: [blockBeyondMaximumRange] }); - restMock.onGet('blocks/1').reply(200, fromBlock); - restMock.onGet('blocks/1003').reply(200, toBlock); - - await ethGetLogsFailing(ethImpl, [null, '0x1', '0x3eb', null, null], (error) => { - expect(error.message).to.equal('Exceeded maximum block range: 1000'); + [null, [], [CONTRACT_ADDRESS_1, CONTRACT_ADDRESS_2]].forEach((address) => { + it(`when block range is too large with address \`${JSON.stringify(address)}\``, async function () { + const fromBlock = { + ...DEFAULT_BLOCK, + number: 1, + }; + const toBlock = { + ...DEFAULT_BLOCK, + number: 1003, + }; + + const blockBeyondMaximumRange = { + ...DEFAULT_BLOCK, + number: 1007, + }; + + restMock.onGet(BLOCKS_LIMIT_ORDER_URL).reply(200, { blocks: [blockBeyondMaximumRange] }); + restMock.onGet('blocks/1').reply(200, fromBlock); + restMock.onGet('blocks/1003').reply(200, toBlock); + + await ethGetLogsFailing(ethImpl, [null, '0x1', '0x3eb', address, null], (error) => { + expect(error.message).to.equal('Exceeded maximum block range: 1000'); + }); }); }); From b7df814dc011f1a7c387680834cc2bec7c112540 Mon Sep 17 00:00:00 2001 From: Luis Mastrangelo Date: Fri, 22 Mar 2024 15:57:37 +0100 Subject: [PATCH 05/11] Add acceptance test for large block range with a single address Signed-off-by: Luis Mastrangelo --- .../tests/acceptance/rpc_batch1.spec.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/server/tests/acceptance/rpc_batch1.spec.ts b/packages/server/tests/acceptance/rpc_batch1.spec.ts index 2660bb95b9..fdfb3d460b 100644 --- a/packages/server/tests/acceptance/rpc_batch1.spec.ts +++ b/packages/server/tests/acceptance/rpc_batch1.spec.ts @@ -267,6 +267,25 @@ describe('@api-batch-1 RPC Server Acceptance Tests', function () { } }); + it('should be able to use `address` param with a large block range', async () => { + //when we pass only address, it defaults to the latest block + const logs = await relay.call( + RelayCalls.ETH_ENDPOINTS.ETH_GET_LOGS, + [ + { + fromBlock: numberTo0x(latestBlock - constants.DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT - 10), + address: contractAddress, + }, + ], + requestId, + ); + expect(logs.length).to.be.greaterThan(0); + + for (const i in logs) { + expect(logs[i].address).to.equal(contractAddress); + } + }); + it('should be able to use `address` param with multiple addresses', async () => { const logs = await relay.call( RelayCalls.ETH_ENDPOINTS.ETH_GET_LOGS, From f70eb4d30c25958aa29a93c08eb367bf89da0418 Mon Sep 17 00:00:00 2001 From: Luis Mastrangelo Date: Fri, 22 Mar 2024 17:01:06 +0100 Subject: [PATCH 06/11] Fix acceptance test Signed-off-by: Luis Mastrangelo --- .../tests/acceptance/rpc_batch1.spec.ts | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/server/tests/acceptance/rpc_batch1.spec.ts b/packages/server/tests/acceptance/rpc_batch1.spec.ts index fdfb3d460b..7a0ba2870b 100644 --- a/packages/server/tests/acceptance/rpc_batch1.spec.ts +++ b/packages/server/tests/acceptance/rpc_batch1.spec.ts @@ -268,21 +268,27 @@ describe('@api-batch-1 RPC Server Acceptance Tests', function () { }); it('should be able to use `address` param with a large block range', async () => { - //when we pass only address, it defaults to the latest block - const logs = await relay.call( - RelayCalls.ETH_ENDPOINTS.ETH_GET_LOGS, - [ - { - fromBlock: numberTo0x(latestBlock - constants.DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT - 10), - address: contractAddress, - }, - ], - requestId, - ); - expect(logs.length).to.be.greaterThan(0); + const blockRangeLimit = constants.DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT; + constants.DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT = 10; + try { + //when we pass only address, it defaults to the latest block + const logs = await relay.call( + RelayCalls.ETH_ENDPOINTS.ETH_GET_LOGS, + [ + { + fromBlock: numberTo0x(latestBlock - constants.DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT - 1), + address: contractAddress, + }, + ], + requestId, + ); + expect(logs.length).to.be.greaterThan(0); - for (const i in logs) { - expect(logs[i].address).to.equal(contractAddress); + for (const i in logs) { + expect(logs[i].address).to.equal(contractAddress); + } + } finally { + constants.DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT = blockRangeLimit; } }); From 0aca2633a6a24dec24400b0fcb8d4d602e5824a8 Mon Sep 17 00:00:00 2001 From: Luis Mastrangelo Date: Fri, 22 Mar 2024 17:45:13 +0100 Subject: [PATCH 07/11] Update `eth_getLogs`'s openrpc docs summary and restrictions Signed-off-by: Luis Mastrangelo --- docs/openrpc.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/openrpc.json b/docs/openrpc.json index 39b711fb57..daf4656922 100644 --- a/docs/openrpc.json +++ b/docs/openrpc.json @@ -401,7 +401,8 @@ }, { "name": "eth_getLogs", - "summary": "Returns an array of all logs matching filter with given id.", + "summary": "Returns an array of all logs matching a given filter object.", + "description": "The block range filter, _i.e._, `fromBlock` and `toBlock` arguments, cannot be larger than `ETH_GET_LOGS_BLOCK_RANGE_LIMIT` (defaults to `1000`). However, when `address` represents a single address, either a `string` or an array with a single element, this restriction is lifted.", "params": [ { "name": "Filter", From cdb7d7c99fe913cdfb1bab5d096f7560b0a3f17d Mon Sep 17 00:00:00 2001 From: Luis Mastrangelo Date: Mon, 25 Mar 2024 20:00:52 +0100 Subject: [PATCH 08/11] Remove unneeded `console.log` Signed-off-by: Luis Mastrangelo --- packages/relay/tests/lib/eth/eth_getLogs.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/relay/tests/lib/eth/eth_getLogs.spec.ts b/packages/relay/tests/lib/eth/eth_getLogs.spec.ts index d82970a80c..107c8f2e23 100644 --- a/packages/relay/tests/lib/eth/eth_getLogs.spec.ts +++ b/packages/relay/tests/lib/eth/eth_getLogs.spec.ts @@ -282,7 +282,6 @@ describe('@ethGetLogs using MirrorNode', async function () { }); [CONTRACT_ADDRESS_1, [CONTRACT_ADDRESS_1]].forEach((address) => { - console.log(address); it(`single address filter \`${JSON.stringify(address)}\` and large block range`, async function () { const filteredLogs = { logs: [DEFAULT_LOGS.logs[0], DEFAULT_LOGS.logs[1], DEFAULT_LOGS.logs[2]], From bf60052de28eed2b91ee9781890664eeea28afd0 Mon Sep 17 00:00:00 2001 From: Luis Mastrangelo Date: Wed, 27 Mar 2024 22:55:17 +0100 Subject: [PATCH 09/11] Remove `env.TEST` usage as suggested by Fredy Signed-off-by: Luis Mastrangelo --- .../lib/services/ethService/ethCommonService/index.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/relay/src/lib/services/ethService/ethCommonService/index.ts b/packages/relay/src/lib/services/ethService/ethCommonService/index.ts index d7b983b196..2abcc1dbda 100644 --- a/packages/relay/src/lib/services/ethService/ethCommonService/index.ts +++ b/packages/relay/src/lib/services/ethService/ethCommonService/index.ts @@ -74,6 +74,10 @@ export class CommonService implements ICommonService { 'ETH_BLOCK_NUMBER_CACHE_TTL_MS_DEFAULT', ); + private getLogsBlockRangeLimit() { + return parseNumericEnvVar('ETH_GET_LOGS_BLOCK_RANGE_LIMIT', 'DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT'); + } + constructor(mirrorNodeClient: MirrorNodeClient, logger: Logger, cacheService: CacheService) { this.mirrorNodeClient = mirrorNodeClient; this.logger = logger; @@ -139,10 +143,9 @@ export class CommonService implements ICommonService { return false; } - const blockRangeLimit = - process.env.TEST === 'true' - ? constants.DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT - : Number(process.env.ETH_GET_LOGS_BLOCK_RANGE_LIMIT); + const blockRangeLimit = this.getLogsBlockRangeLimit(); + // Increasing it to more then one address may degrade mirror node performance + // when addresses contains many log events. const isSingleAddress = Array.isArray(address) ? address.length === 1 : typeof address === 'string' && address !== ''; From e3ad2a5dcf8737047a8f59a2ec4f7c0bd98e516e Mon Sep 17 00:00:00 2001 From: Luis Mastrangelo Date: Wed, 27 Mar 2024 22:56:26 +0100 Subject: [PATCH 10/11] Improve test titles to use `it should` format as suggested by Fredy Signed-off-by: Luis Mastrangelo --- packages/relay/tests/lib/eth/eth_getLogs.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/relay/tests/lib/eth/eth_getLogs.spec.ts b/packages/relay/tests/lib/eth/eth_getLogs.spec.ts index 107c8f2e23..bee6ff763f 100644 --- a/packages/relay/tests/lib/eth/eth_getLogs.spec.ts +++ b/packages/relay/tests/lib/eth/eth_getLogs.spec.ts @@ -282,7 +282,7 @@ describe('@ethGetLogs using MirrorNode', async function () { }); [CONTRACT_ADDRESS_1, [CONTRACT_ADDRESS_1]].forEach((address) => { - it(`single address filter \`${JSON.stringify(address)}\` and large block range`, async function () { + it(`should filter logs by \`${JSON.stringify(address)}\` with a large block range`, async function () { const filteredLogs = { logs: [DEFAULT_LOGS.logs[0], DEFAULT_LOGS.logs[1], DEFAULT_LOGS.logs[2]], }; @@ -479,7 +479,7 @@ describe('@ethGetLogs using MirrorNode', async function () { }); [null, [], [CONTRACT_ADDRESS_1, CONTRACT_ADDRESS_2]].forEach((address) => { - it(`when block range is too large with address \`${JSON.stringify(address)}\``, async function () { + it(`should fail when block range is too large for address(es) \`${JSON.stringify(address)}\``, async function () { const fromBlock = { ...DEFAULT_BLOCK, number: 1, From f341324e5a9ac865dee9f3ba3685843b62443a2b Mon Sep 17 00:00:00 2001 From: Luis Mastrangelo Date: Wed, 27 Mar 2024 23:13:02 +0100 Subject: [PATCH 11/11] Include more details in the openrpc `eth_getLogs` description Signed-off-by: Luis Mastrangelo --- docs/openrpc.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/openrpc.json b/docs/openrpc.json index daf4656922..e00d6301f7 100644 --- a/docs/openrpc.json +++ b/docs/openrpc.json @@ -402,7 +402,7 @@ { "name": "eth_getLogs", "summary": "Returns an array of all logs matching a given filter object.", - "description": "The block range filter, _i.e._, `fromBlock` and `toBlock` arguments, cannot be larger than `ETH_GET_LOGS_BLOCK_RANGE_LIMIT` (defaults to `1000`). However, when `address` represents a single address, either a `string` or an array with a single element, this restriction is lifted.", + "description": "The block range filter, _i.e._, `fromBlock` and `toBlock` arguments, cannot be larger than `ETH_GET_LOGS_BLOCK_RANGE_LIMIT` (defaults to `1000`). However, when `address` represents a single address, either a `string` or an array with a single element, this restriction is lifted.\n\nWhen the logs for an individual address exceeds the Mirror Node page limit `MIRROR_NODE_CONTRACT_RESULTS_LOGS_PG_MAX` (defaults to `200`) an error `-32011` _Mirror Node pagination count range too large_ is returned.", "params": [ { "name": "Filter",