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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

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 ↗	(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: 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 ↗	(On Diff #8599)	comment style
33 ↗	(On Diff #8599)	comment style