eth/filters: avoid block body retrieval when no matching logs

[s1na]

This PR supersedes https://github.com/ethereum/go-ethereum/pull/23655. It applies the trick only to the eth backend.

It changes the logic of GetLogs, which doesn't fill in log metadata anymore. Both GetLogs callers check for this variant and fill in the details when necessary (One is in checkMatches, the other in here https://github.com/ethereum/go-ethereum/blob/d12b1a91cd9423f83bf77dbe363164797549ff15/eth/filters/filter_system.go#L421

NOTE: the field logIndex has changed its meaning

[s1na]

I tested the result against master and the result seems correct. Benchmark results:

------------ master -----------
Requesting logs for block range [13000000, 13002000]
Filtering logs took 18713
Requesting logs for block range [13002000, 13004000]
Filtering logs took 15218
Requesting logs for block range [13004000, 13006000]
Filtering logs took 15882
Requesting logs for block range [13006000, 13008000]
Filtering logs took 18950
Requesting logs for block range [13008000, 13010000]
Filtering logs took 14798

------------------ PR. ----------------
Requesting logs for block range [13000000, 13002000]
Filtering logs took 15515
Requesting logs for block range [13002000, 13004000]
Filtering logs took 12520
Requesting logs for block range [13004000, 13006000]
Filtering logs took 13296
Requesting logs for block range [13006000, 13008000]
Filtering logs took 16063
Requesting logs for block range [13008000, 13010000]
Filtering logs took 12268

[holiman]

This got bitrotted by the log filter cache PR, needs a rebase.

[holiman]

This got bitrotted by the log filter cache PR, needs a rebase.

Ping @s1na , want to give this another push?

[holiman]

Ping @s1na , want to give this another push?

Ping again

[s1na]

~~@holiman @fjl PTAL~~

Edit: Ah wait, I think its better to cache the derived logs.

[s1na]

Yeah this idea doesn't seem to play nice with the cache. I see 2 options, none good:

• Cache underived logs: which means we have to fetch block every time
• But we can't easily cache derived logs either after we've done filtering at which point we have the data to fill in

[fjl]

We can put the *types.Block into the cache entry as well, along with the logs slice.

[s1na]

We can put the *types.Block into the cache entry as well, along with the logs slice.

Now that you say it, blocks are cached in blockchain. It's fine.

[fjl]

I think what needs to happen here, is that the cache should contain the un-derived logs along with the *types.Block pointer for the block the logs were created in. That way, we can always derive them later for any query.

We can also consider mutating the cache entry with the derived results, if it is possible.

[s1na]

We discussed this, a plan of action:

• Cache a flattened list of logs. In this case we need to partially derive fields (everyone except for txhash).
• Also cache block body atomic.Pointer[types.Body] (note body not block). instantiating block has extra costs.
• Upon seeing first matching log, fetch body and cache it.
• Note: this PR is calculating LogIndex wrong, it should be index of log in block.

[s1na]

Since light client can't follow the chain I couldn't test the log filters on a live node. But with the testcase I caught an issue that has been there for a while. Basically the block range of a filter wasn't enforced. That's now fixed in the PR.

[s1na]

@fjl ping 👉