eth/filters: subscribe history logs

[jsvisa]

This is the second part of #15063

[s1na]

I have some misgivings about how we handle block ranges right now. IMO block ranges specifically for the live mode are not necessary. I.e. when you subscribe you get the logs for all new blocks. To make my case web3.js only lets you specify FromBlock (no ToBlock) and ethers.js doesn't support any of those parameters. These are 2 of the biggest web3 libraries.

My suggestion is we can simplify this greatly by: only allowing FromBlock and only for specifying historical blocks. I.e. FromBlock should be either a number below head block or safe or finalized. Later on we can allow ToBlock (for historical range queries).

[jsvisa]

@s1na lgtm, and as we discussed offline, seems we have more work to do, I'll post them here(in case I forget).

• support block range subscription;
• support server active close connection;

[fjl]

I like @s1na's idea to not allow toBlock for subscription queries. The problem with using our subscription model is that there is no way for the server to signal the end of a subscription. We can fix that by changing how subscriptions work, but I think that will be a lot of work for everyone to support. Let's try the simple solution for now.

[s1na]

Took me a minute to get this, but smart trick I love it!

[s1na]

Reality check: It's more complicated.

• Reorg happens in a future block that rangeLogsAsync hasn't processed yet. In that case we don't really care about the event since when rangeLogsAsync reaches that point it will go on the right chain by itself.
• We can keep track of how far we are in delivery by checking the block number of incoming logs.
• However it's harder to say where is the point of reorg, because removed logs are sent in the reverse order (from more recent block backwards).
• What we can do is compare as they come in to see if they pass the blocks for which we've already delivered logs.
• At this point we have to stop rangeAsyncLogs immediately. Or hmm there's a question what should be done if it is mid-block (with some logs remaining in that block).
• Then the new subscription which sends removed logs will also send the replacement logs of the new chain. So we can just tune into that.

[s1na]

I don't understand this condition: log.BlockNumber <= delivered. Why did you add this?

[jsvisa]

Ensure that all logs of the same block are sent out, avoid sending only a subset of logs.

[jsvisa]

Reorg maybe happened more than once between from and to, so I think we need to check for each <-reorgLogsCh message.

[jsvisa]

I was wondering if we should distinguish between two different Reorgs and one Reorg with sequence data(but the received logs maybe not continuous, due to query filter) 🤔

[jsvisa]

I came up with a scenario:

1. from, to is 1 -> 10;
2. delivered is 6;
3. A Reorg occurred between 5-9, assume the logs are huge, split into the following logs:
   1. Removed 5-7
   2. Removed 8-9
   3. Replaced 5-7
   4. Replaced 8-9
4. We are receiving a log from <-reorgLogsCh, here we set the reorgBlock to 5, and send all removed logs between 5-6 to the subscriber;
5. Then we recv a log from <-logChan, because reorg is detected, so no logs were sent to the subscriber;
6. Next we may recv <-reorgLogsCh or errChan, if the first channel comes first(the following ii, iii, iv), everything works fine; else if the latter comes first, we miss the following new logs(Replaced 5-7 and 8-9).

[s1na]

Yes you're right that considering 2 different reorgs adds another layer of complexity. I have not considered that in the changes I made.

Regarding the scenario. Yes I think we should disable the errChan breaking out of the loop if we're in the middle of a reorg processing. But I also don't know how to detect the end of a reorg yet.

[holiman]

I think histLogs is a bit wonky, is the benefit of reducing historicalLogs really worth it?

[holiman]

on input, you take a ctx. But the ctx is lost, afaict you don't check for ctx timeouts or cancel. Shouldn't you? What is the lifecycle here?

[jsvisa]

As the ctx will be canceled when the subscription goroutine returns, so we instantiated a new background context to control the workflow.

The lifecycle is as below:

1. do historical logs fetch and push;
2. do live logs push;
3. terminate if the subscription was canceled or the push channel was broken;

[holiman]

This seems a bit strange. Once reorged i sset, it never becomes unset again. It will discard all histLogs when it's in that mode. Seems to me that this big switch has thee different modes:

• reorged mode, where it discards historic logs
• liveOnly mode
• "normal" non-reorged mode

It makes it a bit complicated to follow, IMO

[jsvisa]

Once reorged i sset, it never becomes unset again

Here we only need to handle the case when our historical delivery process is catching up with the live blocks.

liveOnly mode

Yeah, We did have another switch liveOnly, which was set after all historical logs were delivered: https://github.com/ethereum/go-ethereum/blob/633c7ac67a6de35e000bdc72d51f1054f4e743b5/eth/filters/api.go#L373-L375

[s1na]

Seems to me that this big switch has thee different modes:

Actually I managed to bring it down to 2 modes (674a888). Now only setting liveOnly mode. This will be set:

• either when a reorg happens to discard historical logs
• or when historical logs have finished

But my patch has a problem. In the "reorg" mode, we only send removed log notifications up to the delivered point. So if we finish historical processing (say head = 10 so delivered = 10). Then after emitting block 11, we get a reorg from block 8. We will only delivered the removed logs up to 10.

[s1na]

Ok I pushed another commit which should fix the problem above and IMO the code is more readable now.

That said I noticed the tests fail every once in a while. I believe the edge case they're catching is the following:

• Blockchain will first write the new (reorged) blocks to db and then send removed/new logs. This can cause the history processor to go accidentally on the new chain before we realize there's a reorg happening.
• So we return oldBlock1, oldBlock2, newBlock3, removeOldBlock3, newBlock3, newBlock4. I.e. the removed logs don't match the ones we emitted for that height.

If my hunch is right about the issue, it will be a nasty one to fix.

[s1na]

I've pushed a fix for this. The subscription now tracks a number of block hashes that it has delivered logs for. This map is used to avoid sending removed logs for a block we haven't delivered as well as sending duplicate logs.

[MariusVanDerWijden]

Do we need to deref the log here to copy it?
e.g. log := *log
Otherwise we just copy the pointer if I understand correctly

[fjl]

log := data.(*types.Log)
n.callback(log)
n.logs = append(n.logs, log)

[fjl]

func (n *mockNotifier) Done() {
   close(n.done)
}

and call Done from FilterAPI.

[fjl]

defer notifier.Done()

[cong08]

why not merge it ?

[cong08]

why not merge it ?

@karalabe @rjl493456442