Page MenuHomePhabricator

Merge #10095: refactor: Move GetDifficulty out of `rpc/server.h`
ClosedPublic

Authored by markblundeberg on May 10 2019, 05:19.

Details

Summary

backport PR10095 https://github.com/bitcoin/bitcoin/pull/10095/files
f885b67 refactor: Make rest.cpp dependency on *toJSON in blockchain.cpp explicit (Wladimir J. van der Laan)
8d8f28d refactor: Move RPCNotifyBlockChange out of rpc/server.h (Wladimir J. van der Laan)
e6dcfee refactor: Move GetDifficulty out of rpc/server.h (Wladimir J. van der Laan)

Made slightly awkward due to semiduplicated effort / out of order
backports; some tiny cleanup changes made too.

(Also supersedes PR9612, which added that comment in front of GetDifficulty.)

Depends on D3013

Test Plan

make check
test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markblundeberg created this revision.May 10 2019, 05:19
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 10 2019, 05:19
markblundeberg edited the summary of this revision. (Show Details)May 10 2019, 16:03
Fabien requested changes to this revision.May 14 2019, 10:19
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/rpc/blockchain.h
18 ↗(On Diff #8642)

Nit: if wrt means with respect to, then the following to is a duplicate.

27 ↗(On Diff #8642)

The last parameter is not named, maybe add pindex ?

This revision now requires changes to proceed.May 14 2019, 10:19
markblundeberg added inline comments.May 14 2019, 17:35
src/rpc/blockchain.h
27 ↗(On Diff #8642)

Hmm I'm OK with these small changes, as this backport series will bring us up-to-date with Core.

markblundeberg marked 2 inline comments as done.
markblundeberg edited the summary of this revision. (Show Details)

update per Fabien comments

rebase for retest (same random rpc_listtransactions.py failure occured earlier in D2750)

deadalnix requested changes to this revision.May 14 2019, 22:57

Please write a proper test plan.

This revision now requires changes to proceed.May 14 2019, 22:57
markblundeberg requested review of this revision.May 14 2019, 23:07
markblundeberg edited the test plan for this revision. (Show Details)

Please write a proper test plan.

OK indeed, I think make check is clearer as it compiles a specific (large) group of things, whereas just 'compile' might be interpreted as only compiling bitcoind.

Fabien accepted this revision.May 16 2019, 14:26
deadalnix requested changes to this revision.May 20 2019, 17:15

The test place doesn't cover RPC tests, however, this is clearly changing RPC tests.

This revision now requires changes to proceed.May 20 2019, 17:15
markblundeberg requested review of this revision.May 20 2019, 17:37
markblundeberg edited the test plan for this revision. (Show Details)

The test place doesn't cover RPC tests, however, this is clearly changing RPC tests.

OK, I don't see how compile-only refactoring like this changes behaviour, but I've added to test plan.

deadalnix accepted this revision.May 20 2019, 20:18
This revision is now accepted and ready to land.May 20 2019, 20:18