Page MenuHomePhabricator

Implement simple moving average over work difficulty adjustement algorithm.

Authored by deadalnix on Oct 14 2017, 09:39.



As per title. Hopefully this is much simpler than alternative, but more complex alternative do not perform suffisciently better to be worth it.

Test Plan
make check

Added unit tests for it.

Diff Detail

rABC Bitcoin ABC
Lint Not Applicable
Tests Not Applicable

Event Timeline

Unknown Object (User) added a subscriber: Unknown Object (User).Oct 14 2017, 18:10

Could someone give a quick, 3 sentence more detailed explanation of the algorithm? I read something about average of top 3 blocks?

Math and algorithm look good to me

242 ↗(On Diff #1552)

It's not obvious to me why nHeight is compared the old DifficultyAdjustmentInterval

Maybe a short comment to explain?

244 ↗(On Diff #1552)

Redundant "suitable". Should be:

// Get the last suitable block of the difficulty interval.
254 ↗(On Diff #1552)

Rephrase as:

// Compute the target based on time and work done during the interval.

Add bounding as suggested by @Mengerian and update tests for it.

173 ↗(On Diff #1565)

There are (patholigcal) cases where this assert could fail if MTP is not used. Since the condition is protected against anyway by the bounds you added immediately below, I suggest we remove this assert.

360 ↗(On Diff #1565)

I believe the "in seconds" part is referring to the return value. It would make more sense as a second sentence. e.g. "Returned value is in seconds."

161 ↗(On Diff #1565)

I assume there is no chance that these are from different chains due to other mechanisms?

173 ↗(On Diff #1565)

Blocks for this case should be rejected by the network. However, if they did get through, what would be the outcome of this assert triggering?

176 ↗(On Diff #1565)

144 * params.nPowerTargetSpacing would be the ideal value to find -- this caps it at twice that? If so, should we use 2 * (pindexLast->nHeight - pindexFirst->nHeight) * params.nPowerTargetSpacing ?

178 ↗(On Diff #1565)

Similar question

188 ↗(On Diff #1565)

Since this is uint, I assume -work on this class returns the two's complement? The implementation on base_uint class looks like it to me, but it's been a long time since dealing with this.

254 ↗(On Diff #1565)

This should always be the n-2 block assuming ordered timestamps?

31 ↗(On Diff #1565)


33 ↗(On Diff #1565)

Does this patch leave this function unused besides tests?

87 ↗(On Diff #1565)

Why is this change necessary? I don't see any changes which currently affect this.

269 ↗(On Diff #1565)

Why was >> 10 chosen?

275 ↗(On Diff #1565)

How was this value obtained?

290 ↗(On Diff #1565)

Ditto for >> 4

173 ↗(On Diff #1565)

Node crash.

188 ↗(On Diff #1565)

Yes, -x return the 2's complement.

254 ↗(On Diff #1565)

This is n-1 most of the time.

33 ↗(On Diff #1565)

It is required to verify history.

87 ↗(On Diff #1565)

It was invalid, but in practice doesn't change the result of the test.

269 ↗(On Diff #1565)

Just picked a very small value to check it move slowly.

275 ↗(On Diff #1565)

Run the code. It ensure that someone can test their implementation and see if they end up with the same number.

290 ↗(On Diff #1565)

Just checking a reasonable range for variation.

This revision is now accepted and ready to land.Oct 30 2017, 16:37
This revision was automatically updated to reflect the committed changes.