eth_getLogs not handling non int block params

[Nana-EC]

Description

Currently the eth_getLogs endpoint parses the fromBlock and toBlock without consideration for non int values.
This results in the block time frames not being honoured

See eth.getLogs where parseInt(toBlock) and parseInt(fromBlock) are used.

These should apply the blockNumberOrTag logic used by the other methods

Steps to reproduce

1. curl -X POST -H 'Content-Type: application/json' -d '{"jsonrpc":"2.0","id":"2","method":"eth_getLogs","params":[{ "fromBlock": "0x0", "toBlock": "latest", "addresss": "0x0000000000000000000000000000000002dC46Db"}]}' http://localhost:7546
2. Results in a blocks?block.number=lte:NaN&block.number=gte:0&order=asc 400 status

Additional context

No response

Hedera network

previewnet

Version

v0.6.0

Operating system

No response

[Nana-EC]

Fix will like take the form of updating eth.getLogs()
before

      if (toBlock) {
        // @ts-ignore
        filters.push(`lte:${parseInt(toBlock)}`);
        order = constants.ORDER.DESC;
      }
      if (fromBlock) {
        // @ts-ignore
        filters.push(`gte:${parseInt(fromBlock)}`);
        order = constants.ORDER.ASC;
      }

after

      if (toBlock) {
        const blockNumber = await this.translateBlockTag(toBlock, requestId);
        // @ts-ignore
        filters.push(`lte:${blockNumber}`);
        order = constants.ORDER.DESC;
      }
      if (fromBlock) {
        const blockNumber = await this.translateBlockTag(fromBlock, requestId);
        // @ts-ignore
        filters.push(`gte:${blockNumber}`);
        order = constants.ORDER.ASC;
      }

[dimitrovmaksim]

Wouldn't translateBlockTag potentially add more requests to the mirror node? I was wondering if we could do it another way. So earliest is 0 this one should be easy, and latest and pending should be the last block at the time of the query(?)

In the case of fromBlock being latest or pending, doesn't this mean that we should only take the last block, e.g. query the blocks by desc order and limit to 1?

In the case of toBlock being latest or pending, this should mean we want to take all blocks until the last one, so wouldn't just not setting lte: cover that?

[Nana-EC]

Wouldn't translateBlockTag potentially add more requests to the mirror node? I was wondering if we could do it another way. So earliest is 0 this one should be easy, and latest and pending should be the last block at the time of the query(?)

In the case of fromBlock being latest or pending, doesn't this mean that we should only take the last block, e.g. query the blocks by desc order and limit to 1?

In the case of toBlock being latest or pending, this should mean we want to take all blocks until the last one, so wouldn't just not setting lte: cover that?

@dimitrovmaksim actually made more progress on this, below is where i got to.
Challenge is the mirror node rejects time ranges greater than 7 days.
We should decide what we want to do, potentially return an error highlighting this with a suggestion of the potential blocks they could make requests for.

Also i think most of your suggestions are already captured in this.getHistoricalBlockResponse() logic

      if (toBlock) {
        const blockResponse = await this.getHistoricalBlockResponse(toBlock, true, requestId);
        // @ts-ignore
        params.timestamp = [`lte:${blockResponse?.timestamp?.to}`];
        order = constants.ORDER.DESC;
      }
      if (fromBlock) {
        const blockResponse = await this.getHistoricalBlockResponse(fromBlock, undefined, requestId);
        // @ts-ignore
        if (params.timestamp) {
          params.timestamp.push(`gte:${blockResponse?.timestamp?.from}`);
        } else {
          params.timestamp = [`gte:${blockResponse?.timestamp?.from}`];
        }
        order = constants.ORDER.ASC;
      }

[dimitrovmaksim]

Many JSON-RPC providers limit the block range for eth_getLogs, bsc is 5000, Infura I think is 2000, so I guess its fine to have a limit and return an error if the range exceeds the limit.

BSC: curl https://bsc-dataseed.binance.org/ -X POST -H "Content-Type: application/json" --data '{"method":"eth_getLogs","params":[{"fromBlock": "0x1", "toBlock": "latest"}],"id":1,"jsonrpc":"2.0"}' = > {"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"exceed maximum block range: 5000"}}%

Wouldn't this [lte:${blockResponse?.timestamp?.to}] result in lte:undefined if there's no blockResponse? which will result in {"_status":{"messages":[{"message":"Invalid parameter: timestamp"}]}}.

Also this implementation, If I understand correctly, gets the block passed as either fromBlock or toBlock and uses their timestamps for the logs request, so for example if I send a toBlock that is bigger than the latest existing block, the lte timestamp will be undefined and the logs query will probably fail. In this case it's probably better to get a range of blocks after all and use the first and last blocks timestamps.
Sending an eth_getLogs() with a toBlock number greater than the latest block number on the BSC network returns logs for all blocks up to the latest (if the range does not exceed the limit)

IMO, it would probably be the better if the mirror node could handle the block tags and build a where clause accordingly, instead of the relay performing multiple requests.

[Nana-EC]

@dimitrovmaksim good points.

Let's start with a configureable limit of 500 and the error message

Regarding the toBlock, for the mean time as a band aid, let's

• also ask for the latest block and get that timestamp
• compare that to the toBlock response and take the min of the timestamps.

Also open a ticket against mirror node with your suggestion that we can adopt at a later time

[dimitrovmaksim]

To sum it up:

1. Default limit is 500 blocks, but can be configurable (max limit ?)
2. Use getHistoricalBlockResponse to fetch the block by either number or tag
3. If toBlock is null or getHistoricalBlockResponse for toBlock returns Not Found => fetch latest instead (we have to do a check for the blocks range limit)
4. If fromBlock is not passed, assume it's 0
5. if fromBlock is passed, but getHistoricalBlockResponse returns Not Found return an empty Logs array
6. Check if toBlock - fromBlock is less than or equal to the set limit
7. Use the timestamps from the from and to Blocks to fetch the logs

The only issue here is that getHistoricalBlockResponse throws a Not Found error if a block is not found instead of returning an empty response/undefined