-
Notifications
You must be signed in to change notification settings - Fork 21k
eth/filters: add timestamp to derived logs #31887
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
Conversation
This will be huge when it gets merged 🔥 |
I don't really like the idea of adding more redundant fields to the logs. The receipt itself already holds blockhash and blocknumber so the logs having them kinda unnecessarily duplicates the data. Now we are adding the timestamp as well. I think the better approach would be to rework our json api s.th. the whole receipt is always returned and you don't need these unnecessary fields in the log anymore. Not opposed to merging this, just my 2cents around the spec edit: also Txindex and Txhash |
The idea is to exactly avoid the extra roundtrip for fetching the block. While processing the logs we will anyway have to fetch at least the header and might as well return this data when there is clear demand for it.
I'm not sure I understand the alternative. If you mean return the whole receipt when people query for logs: it still doesn't contain the timestamp. |
Thanks for opening this @s1na - having It would avoid the extra call to |
TODO: The log timestamp is not available in the |
would have been great to see a discussion about returning the field as hex or decimal was had, Reth uses hex, Geth now uses Decimal |
Damn, this is an oversight! It should be returned as hex. |
The block timestamp field is now added to the logs returned by eth_getLogs.
Implements ethereum/execution-apis#639.