Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: eth: Add support for blockHash param in eth_getLogs #10782

Merged
merged 1 commit into from
May 12, 2023

Conversation

fridrik01
Copy link
Contributor

@fridrik01 fridrik01 commented Apr 28, 2023

Fixes: #10781

Context
When working on #10780 I noticed we currently did not support the blockHash param for the eth_getLogs rpc method. If it was specified by user he got confusing results (fromBlock/toBlock were both 0). This PR therefore adds this feature as described in the eth docs

Test plan

Lets consider the tipset 2848101 (hex: 0x2B7565) and get the logs for only that tipset (using fromBlock/toBlock). Since the log output it long lets just count the lines and compute the sha1sum:

curl -s http://localhost:1234/rpc/v1 -X POST -H "Content-Type: application/json" --data '{"method":"eth_getLogs","params":[{"fromBlock": "0x2B7565", "toBlock": "0x2B7565"}],"id":1,"jsonrpc":"2.0"}'|jq|wc -l
231

curl -s http://localhost:1234/rpc/v1 -X POST -H "Content-Type: application/json" --data '{"method":"eth_getLogs","params":[{"fromBlock": "0x2B7565", "toBlock": "0x2B7565"}],"id":1,"jsonrpc":"2.0"}'|jq|sha1sum
50cbd75fd8e9059441d91173c53372237f5f3600  -

Now lets get the same tipset by specifying its blockHash and observe that we get the same results (since using blockHash is the same as specifying fromBlock=toBlock of the tipset):

curl -s http://localhost:1234/rpc/v1 -X POST -H "Content-Type: application/json" --data '{"method":"eth_getLogs","params":[{"blockHash": "0xe8179500429f4c485015a9253e9ee7270a1d3f773fc33ea21d2f007e14623980"}],"id":1,"jsonrpc":"2.0"}'|jq|wc -l
231

curl -s http://localhost:1234/rpc/v1 -X POST -H "Content-Type: application/json" --data '{"method":"eth_getLogs","params":[{"blockHash": "0xe8179500429f4c485015a9253e9ee7270a1d3f773fc33ea21d2f007e14623980"}],"id":1,"jsonrpc":"2.0"}'|jq|sha1sum 
50cbd75fd8e9059441d91173c53372237f5f3600  -

@fridrik01 fridrik01 marked this pull request as ready for review April 28, 2023 10:29
@fridrik01 fridrik01 requested a review from a team as a code owner April 28, 2023 10:29
@Stebalien
Copy link
Member

I think we need to handle this in

// TODO: derive a tipset hash from eth hash - might need to push this down into the EventFilterManager

(we may not have the block yet, or it may be on another fork)

I think we need to set tipsetCid:

tipsetCid cid.Cid

Which will be matched here:

if f.tipsetCid != cid.Undef {
tsCid, err := te.Cid()
if err != nil {
return false
}
return f.tipsetCid.Equals(tsCid)
}

@fridrik01 fridrik01 force-pushed the 10781-add-blockhash-param-to-ethgetLogs branch from 5cbf745 to 4ca30ab Compare May 10, 2023 20:43
@arajasek arajasek merged commit 1dd24fe into master May 12, 2023
93 checks passed
@arajasek arajasek deleted the 10781-add-blockhash-param-to-ethgetLogs branch May 12, 2023 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The ETH Rpc eth_getLogs is missing supports for blockHash param
3 participants