Page MenuHomePhabricator

Backport current GetDifficulty logic (& tests) from Core
ClosedPublic

Authored by markblundeberg on May 10 2019, 04:27.

Details

Summary

Current GetDifficulty has a divergent history from core, time to patch
it up.

We missed a couple backports:
PR9612 https://github.com/bitcoin/bitcoin/pull/9612/files
PR10095 https://github.com/bitcoin/bitcoin/pull/10095/files

Then in D619, we simplified the logic of GetDifficulty in a nice way.

Core would later do the same:
PR13288 https://github.com/bitcoin/bitcoin/pull/13288/files
PR14135 https://github.com/bitcoin/bitcoin/pull/14135/files
But they had also added some tests in the meantime:
PR11748 https://github.com/bitcoin/bitcoin/pull/11748/files

This diff brings back basically into sync with core:

  • get tests
  • remove GetDifficultyFromBits function that isn't used elsewhere
  • and most importantly -- have less code ownership

It can't be simply done with revert-backport-backport-backport-backport so
I'm doing a snapshot update.

Depends on D3021

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
patch
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 5792
Build 9646: Bitcoin ABC Buildbot (legacy)
Build 9645: arc lint + arc unit

Event Timeline

markblundeberg created this revision.May 10 2019, 04:27
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 10 2019, 04:27
markblundeberg retitled this revision from Backport GetDifficulty logic from Core to Backport current GetDifficulty logic from Core.May 10 2019, 04:27
markblundeberg planned changes to this revision.May 10 2019, 05:00
markblundeberg retitled this revision from Backport current GetDifficulty logic from Core to Backport current GetDifficulty logic (& tests) from Core.
markblundeberg edited the summary of this revision. (Show Details)

rebase onto 10095 backport

markblundeberg edited the summary of this revision. (Show Details)May 10 2019, 14:43
markblundeberg planned changes to this revision.May 10 2019, 15:18
schancel accepted this revision.May 12 2019, 02:29
schancel added inline comments.
src/rpc/blockchain.cpp
50 ↗(On Diff #8599)

uint32_t nbits = blockindex->nBits; and leave the other two things alone.

src/rpc/blockchain.h
18 ↗(On Diff #8599)

`Get the required difficulty of the next block w/r/t to the given block index.

This revision is now accepted and ready to land.May 12 2019, 02:29
markblundeberg added inline comments.May 12 2019, 04:55
src/rpc/blockchain.cpp
50 ↗(On Diff #8599)

I agree but just want to get this backported for now.

jasonbcox requested changes to this revision.May 23 2019, 21:51
jasonbcox added inline comments.
src/rpc/blockchain.cpp
46 ↗(On Diff #8599)

Fix comment style:

/**
 * First line...
 */
50 ↗(On Diff #8599)

Also agree. Taking ownership of this code is not wise if we can avoid it.

52 ↗(On Diff #8599)

casting style should be: double(...) like a function call

src/test/blockchain_tests.cpp
3 ↗(On Diff #8599)

use the C++ lib: cstdlib

6 ↗(On Diff #8599)

Ordering of these includes needs to be sections like so:

  1. Header of this source file
  2. Project headers
  3. Test project headers
  4. 3rd party dependencies (boost, etc; such that each 3rd party dep gets its own section)
  5. system libs
10 ↗(On Diff #8599)

comment style

33 ↗(On Diff #8599)

comment style

This revision now requires changes to proceed.May 23 2019, 21:51
markblundeberg marked 9 inline comments as done.

rebase ; address comments

deadalnix accepted this revision.May 24 2019, 17:19
jasonbcox accepted this revision.May 28 2019, 22:20
This revision is now accepted and ready to land.May 28 2019, 22:20

rebase before land