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
arcpatch-D3012
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6045
Build 10149: Bitcoin ABC Buildbot (legacy)
Build 10148: arc lint + arc unit

Event Timeline

markblundeberg retitled this revision from Backport GetDifficulty logic from Core to Backport current GetDifficulty logic from Core.May 10 2019, 04:27
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

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

This revision is now accepted and ready to land.May 28 2019, 22:20