test forks with real ProcessWithParent

[ebuchman]

#191

go test -test.run ShorterFork fails, as expected from the issue.

• properly test issue by using real ProcessWithParent in chain_manager_test.go
• fix issue by fixing CalculateTD
• port the other tests to use real ProcessWithParent

Note this is more than just a "use BlockInfo() to get the TD" problem since the parent might not be on the canonical chain, in which case writeBlockInfo will have never been called, unless we want to store block info for every block we receive from peers, but that seems messy.

The proper solution afaik is to build out a working blockchain tree using the link struct (which tracks the td). Otherwise there's too much pressure on the blockpool/peers. I will submit a PR for that after a get a chance to rebase later today. It is not quite done but will be good for review.

[ebuchman]

Just realized this test doesn't properly capture the problem because of the fake ProcessWithParent in the test file. I will attempt to address that in this PR and then submit a new one for tracking forks.

[ebuchman]

Ok I believe this is all done. Now we can actually test real chains with real block processing rules :D

[tgerring]

I don't see function parameter n used... should makechain be passed n instead of 5?

[ebuchman]

Yes good point, thanks @tgerring . That infrastructure needs a little more work for more convenient testing of forks starting elsewhere besides the genesis block. Will be especially useful if/when we implement some basic checkpointing.

Also, @obscuren , I believe TestChain needs a mutex lock, as it can be called either by a miner or a peer and might trigger a race condition on access to workingChain. Same probably goes for InsertChain. But there is no mutex field in the ChainManager. Should I add one? Or perhaps figure out an alternative method to synchronize using channels?

[ebuchman]

Ok that's much cleaner. Can test forks from arbitrary heights, and easy to run many trials per Test.

[ebuchman]

The plot thickens! Valid forks still fail in the wild.

The tests were sharing a db, so they mask the fact that state's don't persist through ProcessWithParent. Since we don't call state.Sync() until the blockchain's TD is exceeded, we lose state's in between calls to ProcessWithParen, and so get invalid merkle root errors as a result.

The tests capture that now using two memDbs and flushing the states created on one to point to the other (and thus have empty tries), simulating what happens when blocks come in from peers.

Fix coming.

[obscuren]

Hold of on the fix. This has changed significantly and resulted in much easier code. This shouldn't be a problem any longer.

What happens now is that a chain is no longer checked but the block vs parent instead. The block is inserted regardless of the outcome of the entire chain.

[obscuren]

Closing this issue since it has completely and utterly changed