Page MenuHomePhabricator

Implement simple moving average over work difficulty adjustement algorithm.
ClosedPublic

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

Details

Summary

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

Repository
rABC Bitcoin ABC
Branch
newpow
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 997
Build 997: arc lint + arc unit

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

src/pow.cpp
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.

src/pow.cpp
173

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.

src/chain.h
360

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."

src/pow.cpp
161

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

173

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

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

Similar question

188

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

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

src/pow.h
31

adjustment

33

Does this patch leave this function unused besides tests?

src/test/pow_tests.cpp
87

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

269

Why was >> 10 chosen?

275

How was this value obtained?

290

Ditto for >> 4

src/pow.cpp
173

Node crash.

188

Yes, -x return the 2's complement.

254

This is n-1 most of the time.

src/pow.h
33

It is required to verify history.

src/test/pow_tests.cpp
87

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

269

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

275

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

290

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.