This is just to ensure we have a height that is >= that the interval? (I'm asking because a height and an interval wouldn't normally make sense to compare)

suffiscient -> sufficient

What does sufficient mean in this context?

Could we directly iterate backwards over blocks? I assume this performs an O(N) lookup for each iteration of this loop?

I'm missing something. Why do we start with blockCount 3 if we want at least 5?

Is this just defensive programming? How can this ever be true if pindexLast is the descendant of pindexFast?

If it's defensive programming, could you please call that out?

So we find two blocks that took longer to produce than 13 blocks *should* have taken? This could be 5 blocks, or it could be something > 13?

If yes to both, please add a comment as to why 13 blocks.

This all makes sense. I'll look up these GetCompact/SetCompact functions in a bit. I assume they return/set the number of bits required to represent the target?

Why did this change? Is it because it's continually calculated now?

Comment says 60s, but looks like every 600s?

stay > stays

My 2¢: I really hate using i++ in an index operation.

witha bogous -> with a bogus

So we produce a bunch of blocks every 60s, then a handful of blocks every 10 minutes, then a single block 100 minutes in the future, and then some more 10 minute blocks?

Should we assert that it has no impact?

quicly => quickly

etablish -> establish

emiting -> emitting

sckewed -> skewed

topmost -> top most

Compte the a target -> Compute the target

assert will crash while pindex->nHeight<3. Should we change this line into:

if (pindex->nHeight < 3) return NULL;

No.

Yes.

This ensure we have enough history to use the algorithm. This should never run as we have enough history today.

It doesn't quite work because of the GetSuitableBlock logic. This also use a skiplist so it is log(n). Probably good enough.

Because of GetSuitableBlock, 3 may be enough to get there.

There is no guarantee that timestamps are ordered. There are good reasons for this, that are outside of the scope of this diff.

13 is to make sure we have enough history and because it has no common factor with 2016.

They return a 32bits representation, losing some precision in the process, like floating points.

It's correct, but doesn't change much in practice because nBits is constant.

The pattern fit timestamp manipulation. There is one block in there that'll have a negative duration compared to the previous one.

The difficulty should get harder, not stay the same (because previous blocks are coming too fast).

No complement. The formula is obviously incorrect using the negative.

Both actualDuration and work are const.

Nitpick, also trying out review...

It should be noted that this is a slight approximation.

The exact calculation would be:

return actualDuration * ( (-(work * params.nPowTargetSpacing / actualDuration)) / (work * params.nPowTargetSpacing));

Because the "W" in the formula above represents the work per block, so we substitute

W = (Hash Rate) * (Target time) = (work over interval) * (target time / interval duration)

into

Target = -W / W

(where "-W" is the 256-bit 2's complement simplification of (2^256 - W)

In practice, this makes a very small difference, since it's off-by-one of a large 256 bit number.

This condition is unnecessary and can be removed

This is beautiful.

{10,20,30,40,50,60,70,80,90,100,110,65} can happen so this is protecting against underflow in the subtraction below.

In this comment, which target is "the most stable value"?

This looks like an unrelated change.

Style-nit: Below, inline comments are using //, but here you're using /*. Consider choosing one and sticking with it. Personally, I prefer /** */ is for docblocks and // for inline comments, so I prefer the style you use below, but I don't care too much as long as you're consistent.

+1

This just repeats the docblock above. Consider removing it.

Why do swaps rather than just ifs and return the right one? The swaps themselves don't seem important here.

grammar-nit: s/ensure/ensures

grammar-nit: s/do/does

grammar-nit: s/to/too

I don't love asserts for input validation, only for catching programming errors. Why not just return a "reasonable" value here? Like the previous work, for example?

I prefer this // style for comments inside of a function.

grammar-nit: s/ensure/ensures
grammar-nit: remove the comma after "value"

grammar-nit: s/avoid/avoids

grammar-nit: s/swing/swings
grammer-nit: s/and to overreact/and overreactions

There is no previous work is pindexPrev is null.