Handling Forks and a Working Blockchain Tree

[ebuchman]

So it appears, and in some of my testing, that any new fork will just take over (note the ShorterFork test is missing in chain_manager_test.go). I suspect this is being worked on, but here's a summary:

When ProcessWithParent is called, it sums the difficulty by calling sm.CalculateTD:
https://github.com/ethereum/go-ethereum/blob/poc8/chain/block_manager.go#L258

But this function simply adds to the difficulty of the chain head, rather than the parent of the block being processed:
https://github.com/ethereum/go-ethereum/blob/poc8/chain/block_manager.go#L298

chain_manager.go has a similar function (that is never called) that would do this properly, but it depends on writeBlockInfo having been called in the past to get the current TD of that block from leveldb:
https://github.com/ethereum/go-ethereum/blob/poc8/chain/chain_manager.go#L232

Of course if the parent of the block isn't on canonical, that's a problem. It seems we need a more comprehensive way of dealing with extensions/forks of the chain.

I suspect this is being worked on. But I took a stab at it yesterday by adding a workingTree map[string]*link to the ChainManager to replace the transient workingChain. Chains are added to the tree through TestChain if all ProcessWithParent passes. ImportChain will then establish the new canonical chain either by doing nothing (ie TestChain extends a non-dominant fork), adding blocks to the current head (standard), or causing a re-org (ie. TestChain extended a fork that now has most TD, where TD is tracked in the links objects).

The main benefit is that we can persist forks/branches in memory rather than having to get them in totality from the peers/blockpool every time, and we can track their dynamic TD.

I would have to rebase the code and cleanup a bit, but would this be something you'd be interested in seeing in a PR, and possibly for discussion?

[obscuren]

Thanks for pointing out. That's indeed an error. Could you submit your test as a PR so I can have a proper look at it?

This should be relatively easy to fix because the TD for each block is available by called BlockInfo on a the chain which should give you some additional information about the block including the TD.

[ebuchman]

the key was to fix CalculateTD and to check the workingChain in BlockInfo. I will close this (discuss in PR if necessary), but probably open another issue for general fork handling.