Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

There are two reasons to remove the code(as you see):
The calculated nBits will be replaced by the following GetNextWorkRequired(.) function. So calculating difficulty(nBits) in UpdateTime() function is meaningless. The main chain does not matter. But testnet will exec twice.
UpdateTime() function should remain single role for updating timestamp item of block.

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging passed.

I've already reviewed this and it looks good to me, but I want to get a second pair of eyes on it since I'm not as familiar with the miner code. @deadalnix @schancel

If updating the time has an impact on something, then it is correct for the function updating time to also update that thing. The alternative is for the function to leave the block template in an invalid state, which bring zero benefit - or if it does, it needs to be justified.

@deadalnix @jasonb The following picture shows UpdateTime() function calling.

if (pindexPrev != chainActive.Tip() || (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - nStart > 5)) {
    ...
    pblocktemplate = BlockAssembler(config, Params()).CreateNewBlock(scriptDummy);
    ...
}

Ok, so I was traveling but now I'm back and I can expand a bit more on the reasoning here.

First, you are correct pointing out that the nBits is computed twice in that specific case and that the computation is somewhat redundant. This shows that there is a probably design problem somewhere. You are correct so far.

On the other hand, the reasoning you make to justify the change assume that the caller of UpdateTime behaves in a specific way. This is really bad as it makes the code fragile. The reasoning made is that it is okay to put a giant bear trap somewhere in the garden because nobody is walking that path anyway. Eventually, someone is going to change something and fall into the trap. In that specific case, the trap is that the UpdateTime function can render invalid some valid template that is passed to it. It is not acceptable.

So lesson n°1 here: Do not make assumption about the caller behavior in some piece of code with a public API. If it is possible to use that code wrong, it *WILL* be used wrong, it's just a matter of time.

Or to quote John Carmack:

I noticed that each time PVS-Studio was updated, it found something in our codebase with the new rules. This seems to imply that if you have a large enough codebase, any class of error that is syntactically legal probably exists there. In a large project, code quality is every bit as statistical as physical material properties – flaws exist all over the place, you can only hope to minimize the impact they have on your users.

See http://www.gamasutra.com/view/news/128836/InDepth_Static_Code_Analysis.php for more.

So what's the problem here ? The problem is that updating the time and updating the nBits fields do not seem to be orthogonal operations. One has impact on the other. So rather than trying to make them orthogonal - as you propose to do, you should embrace the fact they aren't. A way to do this is to change the UpdateTime function into something like UpdateTimeAndTarget and do both in there.Call site can then be cleaned up, nBits and timestamp can be made private fields in the template structure, with a getter for each, and UpdateTimeAndTarget can become a member function that set both. This would create an API such as it become very difficult to make a mistake there, as well as make it clear who's responsibility it is to compute the difficulty. That's what comes to mind right now, I'm not sure of the implications of such an approach, it may have downside, but this is the general direction such a fix should go into.