Page MenuHomePhabricator

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

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

Details

Reviewers
jasonbcox
deadalnix
Fabien
Group Reviewers
Restricted Project
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

Diff Detail

Repository
rABC Bitcoin ABC
Branch
arcpatch-D3014
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 5856
Build 9772: Bitcoin ABC Teamcity Staging
Build 9771: arc lint + arc unit

Event Timeline

markblundeberg created this revision.Fri, May 10, 05:19
Herald added a reviewer: Restricted Project. · View Herald TranscriptFri, May 10, 05:19
markblundeberg edited the summary of this revision. (Show Details)Fri, May 10, 16:03
Fabien requested changes to this revision.Tue, May 14, 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.Tue, May 14, 10:19
markblundeberg added inline comments.Tue, May 14, 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.Tue, May 14, 22:57

Please write a proper test plan.

This revision now requires changes to proceed.Tue, May 14, 22:57
markblundeberg requested review of this revision.Tue, May 14, 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.Thu, May 16, 14:26