all: derive log fields only after filtering #23655
Conversation
s1na
requested review from
gballet,
holiman,
karalabe,
MariusVanDerWijden,
rjl493456442 and
zsfelfoldi
as code owners
| @@ -716,7 +716,7 @@ func deriveLogFields(receipts []*receiptLogs, hash common.Hash, number uint64, t | |||
| // ReadLogs retrieves the logs for all transactions in a block. The log fields | |||
| // are populated with metadata. In case the receipts or the block body | |||
| // are not found, a nil is returned. | |||
| func ReadLogs(db ethdb.Reader, hash common.Hash, number uint64) [][]*types.Log { | |||
| func ReadLogs(db ethdb.Reader, hash common.Hash, number uint64, fn func([]*types.Log) []*types.Log) []*types.Log { | |||
There was a problem hiding this comment.
Perhaps replace fn with filterFn, and document it.
Also, is it required to be func([]*types.Log) []*types.Log, or could it be simplified (without any meaningful loss of performance) into func(*types.Log) bool ?
If so, I think that would be cleaner.
Then you could also define allLogsFilter := func(*types.Log) bool { return true } , maybe, if that's common. Or, let nil mean "no filter", which IMO is even nicer from api perspective
| flatLogs = append(flatLogs, l...) | ||
| } | ||
| filtered := fn(flatLogs) | ||
| if len(filtered) > 0 { |
There was a problem hiding this comment.
This clause can be de-indented, if you check the return-case first.
if len(filtered) == 0{
return nil
}
|
We discussed this today. GetLogsFiltered is not a good idea, we should instead add the filter function parameter to GetLogs. Also, GetLogs should take the block number as argument because both of its call sites use it like |
|
WTF |
This PR includes Optim2 mentioned here: #23147 (comment)
We want to avoid deriving log fields like tx hash which require reading the block body from disk unless there are actually matching logs in that block.
Not super happy with the implementation. I added a new method to Backend
GetLogsFilteredonly to avoid breaking other places which useGetLogslike LES. Might re-write if I think of a better solution