As per title. Hopefully this is much simpler than alternative, but more complex alternative do not perform suffisciently better to be worth it.
Details
Details
make check
Added unit tests for it.
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- newpow
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 995 Build 995: arc lint + arc unit
Event Timeline
Comment Actions
Could someone give a quick, 3 sentence more detailed explanation of the algorithm? I read something about average of top 3 blocks?
Comment Actions
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. |
| src/pow.cpp | ||
|---|---|---|
| 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. |
| src/chain.h | ||
|---|---|---|
| 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." |
| src/pow.cpp | ||
| 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? |
| src/pow.h | ||
| 31 ↗ | (On Diff #1565) | adjustment |
| 33 ↗ | (On Diff #1565) | Does this patch leave this function unused besides tests? |
| src/test/pow_tests.cpp | ||
| 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 |
| src/pow.cpp | ||
|---|---|---|
| 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. |
| src/pow.h | ||
| 33 ↗ | (On Diff #1565) | It is required to verify history. |
| src/test/pow_tests.cpp | ||
| 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. |