feat: Add trace filter API #12123
feat: Add trace filter API #12123
Conversation
|
All checks have completed ❌ Failed Test / Test (itest-api) (pull_request) |
| fromBlock, err := a.EthBlockNumberFromString(ctx, filter.FromBlock) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("cannot parse fromBlock: %w", err) | ||
| } | ||
|
|
||
| toBlock, err := a.EthBlockNumberFromString(ctx, filter.ToBlock) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("cannot parse toBlock: %w", err) | ||
| } |
There was a problem hiding this comment.
since these are specified to be optional in the eth docs, can we find out what their default behaviour is if you don't specify them?
There was a problem hiding this comment.
I am not finding too much clarity yet. I have found this reference https://github.com/openethereum/parity-ethereum/blob/55c90d4016505317034e3e98f699af07f5404b63/ethcore/src/client/client.rs#L2052 which leads to a call that implies that from/to block is not null: https://github.com/openethereum/parity-ethereum/blob/55c90d4016505317034e3e98f699af07f5404b63/ethcore/src/client/client.rs#L1249
I think a really reasonable default value for FromBlock / ToBlock being empty is to use a default value of "latest"
implemented here: f74fd60
There was a problem hiding this comment.
my read of https://github.com/ledgerwatch/erigon/blob/4bcfa59f7ebdab9888e13f2fb8344c694a589ea8/turbo/jsonrpc/trace_filtering.go#L315-L326 is that from defaults to 0 and to defaults to head. But 0 seems a little silly to me, so both being latest is probably ok I guess.
node/impl/full/eth.go
Outdated
| results = append(results, &txTrace) | ||
|
|
||
| // If Count is specified, limit the results | ||
| if filter.Count > 0 && len(results) >= filter.Count { |
There was a problem hiding this comment.
I think we're going to need a maximum length for results regardless of Count because this could allow users to fetch unreasonably large results.
You can see an example of this in the Events.MaxFilterResults configuration option and how it gets used in the various components that need it (grep for MaxFilterResults); but the thing we shouldn't copy from that is that it just truncates your results silently:
lotus/chain/events/filter/event.go
Lines 141 to 144 in 1e8bc10
It would be great to find out what an ethereum implementation uses for a max, and what they do when they hit max, and try and copy that.
node/impl/full/eth.go
Outdated
| for _, blockTrace := range blockTraces { | ||
| if matchFilterCriteria(blockTrace, filter) { | ||
| traceCounter++ | ||
| if traceCounter <= filter.After { |
There was a problem hiding this comment.
does this mean After is 1-based? so if I say "after 1" then I should expect the 2nd and beyond?
| // Trace filter should find the transaction | ||
| traces, err := client.EthTraceFilter(ctx, filter) | ||
| require.NoError(t, err) | ||
| require.NotNil(t, traces) |
There was a problem hiding this comment.
can we assert anything about the trace itself? or is it impenetrable garbage for our purposes here?
…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.
chain/types/ethtypes/eth_types.go
Outdated
| // Optional, default: nil. | ||
| // The JSON decoding must treat a string as equivalent to an array with one value, for example | ||
| // "0x8888f1f195afa192cfee86069858" must be decoded as [ "0x8888f1f195afa192cfee86069858" ] | ||
| FromAddress *[]string `json:"fromAddress,omitempty"` |
There was a problem hiding this comment.
these two slices don't need to be pointers, I don't think you gain anything from having an additional layer of nil and needing to dereference these. But, there is a EthAddressList type you could use instead of []string here, which does the special single vs array json decoding for you, so you should probably swap them out for that. See EthFilterSpec and EthSubscriptionParams which have the same comment about decoding.
Co-authored-by: Rod Vagg <rod@vagg.org>
Proposed Changes
Add json RPC support for trace_filter to lotus
This code assumes that FromBlock and ToBlock and both 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 signaturesPR 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