core: sorted reorg insertion order for proper head header updating

[karalabe]

Currently when we're doing reorgs, the latest block is correctly updated to whatever it truly is, but the latest header lags behind until the next block is imported.

A repro log from the included test (plotting CurrentHeader, the CurrentBlock is correct):

Head #2 e64ad2cb… (td 262144)
Insert #3 e64ad2cb…->ccf0772d… (diff 131072), new head #3 ccf0772d… (td 393216)
Insert #3 e64ad2cb…->80b16434… (diff 131072), new head #3 ccf0772d… (td 393216)

Head #3 ccf0772d… (td 393216)
Insert #4 ccf0772d…->042594ee… (diff 131072), new head #4 042594ee… (td 524288)
Insert #4 ccf0772d…->7d6a286f… (diff 131072), new head #4 7d6a286f… (td 524288)

Head #4 7d6a286f… (td 524288)
Insert #5 042594ee…->79d450b6… (diff 131072), new head #4 042594ee… (td 524288) <- !!!
Insert #5 042594ee…->886d2342… (diff 131072), new head #5 886d2342… (td 655360)

Head #5 886d2342… (td 655360)
Insert #6 79d450b6…->e735a486… (diff 131072), new head #5 79d450b6… (td 655360) <- !!!
Insert #6 79d450b6…->7a19c5d7… (diff 131072), new head #5 79d450b6… (td 655360) <- !!!

Head #5 79d450b6… (td 655360)
Insert #7 e735a486…->374dd593… (diff 131072), new head #7 374dd593… (td 917504)
Insert #7 e735a486…->305bc466… (diff 131072), new head #7 305bc466… (td 917504)

This caused issues on the light clients where they got left behind a few blocks (seemingly not seeing that new content is available), or specifically on Rinkeby we've seen the faucet import blocks 2-by-2, not sequentially, which we couldn't explain until now.

The bug was a combination of lazy code and a subsequent refactor:

• The core/BlockChain has an insert() method, which originally updated our chain head to whatever it was given (e.g. insert(block 2) would update head to 2, irrelevant what block we're at currently). The reorg code assumed this behavior blindly, and when inserting a chain of newly-became-canonical blocks, it inserted them in reverse order for some arbitrary lazy reason "Order does not matter. Last block will be written in ImportChain itself which creates the new head properly" and then topped the latest block on top. Internally reorging a new chain with blocks 3-4-5 meant that we set head to 5-4-3-5, arriving to a correct head at the end. This worked correctly in its current form although during execution the internal state could be considered invalid.
• Then 2 years ago when implementing fast sync, the blockchain and headerchain notions were split apart and the head block was tracked separately from the head header. Since the head header could then be further out than the head block, insert was modified to only change the latest header if the block being inserted is not canonical yet.

The combo effect of the above two ideas is that calling insert on 5-4-3-5 will set the latest header to 5-4-3, but will not set it to 5 again, since it sees that 5 is already a canonical header. The fix is kind of obvious: don't reorg in a weird reverse order, rather insert the blocks in their proper sequence. This would result in a reorg calling insert on 3-4-5-5. This will properly set the latest block to 3-4-5-5 and will set the latest header to 3-4-5.

Note, I'm unsure about the design idea behind inserting the last block twice, and that should definitely warrant exploration from a performance perspective, but that's out of the scope of this PR and I'l like to have this PR be small and quickly fix a bug. We can explore performance issues in a followup PR.

[holiman]

LGTM