Details

Reviewers

jasonbcox
schancel
deadalnix

Group Reviewers

Restricted Project

Commits

rSTAGINGf0c3148e5a72: Backport current GetDifficulty logic (& tests) from Core
rABCf0c3148e5a72: Backport current GetDifficulty logic (& tests) from Core

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

backport-getdiff

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 5805
Build 9672: Bitcoin ABC Buildbot (legacy)
Build 9671: 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

Herald added a reviewer: deadalnix. · View Herald Transcript

markblundeberg retitled this revision from Backport GetDifficulty logic from Core to Backport current GetDifficulty logic from Core.May 10 2019, 04:27

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5792: Diff 8581.May 10 2019, 04:41

markblundeberg planned changes to this revision.May 10 2019, 05:00

rebase onto 10095 backport

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5795: Diff 8584.May 10 2019, 05:39

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

rebase onto D3021

markblundeberg removed a parent revision: D3014: Merge #10095: refactor: Move GetDifficulty out of `rpc/server.h`.May 10 2019, 16:11

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

markblundeberg mentioned this in D3021: Merge #12151: rpc: Remove cs_main lock from blockToJSON and blockheaderToJSON.May 10 2019, 16:12

Harbormaster completed remote builds in B5805: Diff 8599.May 10 2019, 16:25

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

schancel accepted this revision.May 12 2019, 02:29

schancel added inline comments.

src/rpc/blockchain.cpp
50	`uint32_t nbits = blockindex->nBits;` and leave the other two things alone.
src/rpc/blockchain.h
18	`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	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	Fix comment style: /** * First line... */
50	Also agree. Taking ownership of this code is not wise if we can avoid it.
52	casting style should be: `double(...)` like a function call
src/test/blockchain_tests.cpp
3	use the C++ lib: `cstdlib`
6	Ordering of these includes needs to be sections like so: Header of this source file Project headers Test project headers 3rd party dependencies (boost, etc; such that each 3rd party dep gets its own section) system libs
10	comment style
33	comment style