eth_getLogs - add support for multiple addresses #719
Conversation
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Codecov ReportBase: 72.53% // Head: 72.74% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #719 +/- ##
==========================================
+ Coverage 72.53% 72.74% +0.21%
==========================================
Files 16 16
Lines 1227 1244 +17
Branches 224 229 +5
==========================================
+ Hits 890 905 +15
- Misses 279 280 +1
- Partials 58 59 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
packages/relay/src/lib/eth.ts
Outdated
| params.timestamp = [`gte:${blockResponse.timestamp.from}`, `lte:${blockResponse.timestamp.to}`]; | ||
| } else { | ||
| params.timestamp = []; | ||
| // Use the `toBlock` if it is the only passed tag, if not utilize the `fromBlock` |
There was a problem hiding this comment.
I can't quite get what this means. However I think if toBlock is passed without fromBlock and the block number/tag is not latest/pending, we can either return an error (Alchemy) or an empty response (Infura). In the case of Infura I think it returns an empty response, because if not passed the fromBlock defaults to latest and it becomes fromBlock > toBlock, which results to logs not being found.
There was a problem hiding this comment.
Let's return an error like Alchemy but make sure the error notes that it's due to a missing fromBlock when a toBlock was provided
There was a problem hiding this comment.
For context:
When toBlock param value is the tag "latest" or "pending" -> returns logs for latest block
When toBlock param is hex number >= "latest" block number -> returns logs for latest block
When toBlock param is hex number < "latest" block number -> {"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"One of the blocks specified in filter (fromBlock, toBlock or blockHash) cannot be found."}}
Unfortunately the 2 seconds per block creation time in hedera may sometimes be a problem here.
Nana-EC
requested review from
dimitrovmaksim,
georgi-l95,
david-bakin-sl and
lukelee-sl
# Conflicts: # packages/server/tests/acceptance/index.spec.ts
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
# Conflicts: # packages/relay/src/lib/eth.ts
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
packages/relay/src/lib/eth.ts
Outdated
| } else { | ||
| const blockRangeLimit = Number(process.env.ETH_GET_LOGS_BLOCK_RANGE_LIMIT) || constants.DEFAULT_ETH_GET_LOGS_BLOCK_RANGE_LIMIT; | ||
| // toBlock is not specified or is `latest` or `pending` | ||
| if (EthImpl.blockTagIsLatestOrPending(toBlock)) { |
There was a problem hiding this comment.
Can't we combine this if with the else if, for example something like if (!EthImpl.blockTagIsLatestOrPending(toBlock) && toBlockNumber < latestBlockNumber, imo it is unnecessary to assign "latest" to the toBlock if it already is either latest or pending
There was a problem hiding this comment.
It could also be null in this case.
There was a problem hiding this comment.
The name blockTagIsLatestOrPending does not suggest it checks for null, and personally I don't think it should.
There was a problem hiding this comment.
Maybe it's better to handle this here, https://github.com/hashgraph/hedera-json-rpc-relay/blob/main/packages/server/src/server.ts#L409-L410 instead of || null to be || "latest"
packages/relay/src/lib/eth.ts
Outdated
|
|
||
| // Set default blockTag if fromBlock is not specified | ||
| if (!fromBlock) { | ||
| toBlock = "latest"; |
There was a problem hiding this comment.
Shouldn't this be also fromBlock?
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
|
Kudos, SonarCloud Quality Gate passed!
|
Description:
eth_getLogsresults by multiple addresses. It is now possible to pass an array value for theaddressparam.eth_getLogsby reducing the function complexity.Related issue(s):
Fixes #
Notes for reviewer:
Checklist