-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: Add trace filter API #12123
feat: Add trace filter API #12123
Conversation
All checks have completed ❌ Failed Test / Test (itest-api) (pull_request) |
…ake sure we stick to lookback limits
Co-authored-by: Rod Vagg <rod@vagg.org>
@snissn I was just going looking for source of the reference implementation of this but I've noticed this isn't in go-ethereum, only in "openethereum and erigon". So with one of those deprecated we only have erigon. I know little of the eth ecosystem, but in how widespread use is erigon, and how common is this call? Are people begging for this in Filecoin? And do we want to be signing up to be supporting even more methods than go-ethereum does when we have a fraction of the ability to support them? |
And btw the behaviour of default values seems to mostly be in here: https://github.com/ledgerwatch/erigon/blob/5ee7f351f5361ac1070b28f1f25455b6d00905af/turbo/jsonrpc/trace_filtering.go#L286-L316 Plus a bunch more behavioural things in From default |
…FromString and EthTraceFilter functions.
This is an expensive API as Rodd's profiling shows and I think it's hard to guesstimate the default value of allowed traces for a single call without knowing usage patterns etc. Bets to keep a sane value of say 500 and then depend on user feedback to change it when we do get feedback. RPC providers can always change the default for specific use cases/users they need to support. |
Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: Rod Vagg <rod@vagg.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: Rod Vagg <rod@vagg.org>
ci passes and merging to master has no conflicts ✔️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, nice work.
Feel free to merge but please either squash merge here with the appropriate description, or squash your commits into logical units with good descriptions locally first. Your choice (aiming for fewer and more descriptive commits).
Proposed Changes
Add json RPC support for trace_filter to lotus
This code uses a default value of
latest
for FromBlock and ToBlock if either are not provided. These are used to iterate through blocks and a filter is applied.The spec lists FromBlock and ToBlock as optional but i think in practice most use cases include will them. I'm not sure how to implement this function without a block range because of the way the data is organized.
TODO:
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>
fix: mempool: Introduce a cache for valid signatures
PR type
: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, testarea
, e.g. api, chain, state, mempool, multisig, networking, paych, proving, sealing, wallet, depsTesting