eth, eth/downloader: better remote head tracking

[karalabe]

Currently the downloader has a bad bug: it caches the known head of a remote peer when it connects and never updates that head (inside the downloader). This means that if after the initial sync cycle a new cycle is initiated against a long connected peer, it will start syncing assuming that ooold head, resulting in a ton of duplicate blocks being pulled, imported and ignored.

The solution is to replace the cached head with a callback (:trollface:) that can fetch the actual current head. This is somewhat complicated by the fact that in the eth protocol handler we were a bit liberal in how we set the current head. The PR also fixes this by only ever updating the head header of a remote peer when receiving a propagated block, but even then only setting it to propagated block - 1. This is good for a variety of reasons:

• Peers only propagate blocks they think are valid (header, PoW), but they may turn out to not be. However if the remote peer thinks block N might be valid, then block N-1 surely is. So by setting the head to N-1 instead of N, it is surely a correct hash/difficulty.
• By tracking HEAD - 1 instead of the potentially real head we have a nice side effect that sync cycles are only spawned if there's a gap of two blocks between our chain and a remote chain. This works around races between propagated/announced blocks and sync cycle startups.
• Having the head 1 lower than the potentially true one doesn't affect the downloader sync wise, as it will fetch headers until no more are returned, not capped by any field at all.
• Also having a 1 gap shouldn't affect syncing at all, since propagated blocks will fill it in and the fetcher should also cleanly handle < 7 gaps in the chain.

[robotally]

Vote	Count	Reviewers
👍	1	@fjl
👎	0

Updated: Mon Aug 8 08:22:25 UTC 2016

[hdiedrich]

Are you sure re "Peers only propagate blocks they think are valid (header, PoW), but they may turn out to not be. However if the remote peer thinks block N might be valid, then block N-1 surely is. So by setting the head to N-1 instead of N, it is surely a correct hash/difficulty." -- the hash/TD should be a correct tuple per se for N, just as for N-1, just N-1 has a higher likeliness to be accepted by anyone beyond the peer itself?

[karalabe]

The propagation logic currently is that if you get a block over the network, you validate it (header, PoW, chain progression etc), and if it seems valid, you propagate it forward (to logN peers) without actually importing it. After propagating you import it, and announce to everyone else. Hence when you propagate a block, you haven't yet imported it; whereas propagation requires that you successfully validated it, which also entails having it's parent block in your chain already.

[fjl]

Code looks 👍 but I haven't run this yet.

[obscuren]

👍