Page MenuHomePhabricator
Differential D3014

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

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

Details

Reviewers
jasonbcox
deadalnix
Fabien
Group Reviewers
Restricted Project
Commits
rSTAGING36645ddefd90: Merge #10095: refactor: Move GetDifficulty out of `rpc/server.h`
rABC36645ddefd90: Merge #10095: refactor: Move GetDifficulty out of `rpc/server.h`
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

Event Timeline

markblundeberg created this revision.May 10 2019, 05:19
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 10 2019, 05:19
Herald added a reviewer: deadalnix. · View Herald Transcript
teamcity added a comment.May 10 2019, 05:20

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

markblundeberg added a child revision: D3012: Backport current GetDifficulty logic (& tests) from Core.May 10 2019, 05:24
Harbormaster completed remote builds in B5794: Diff 8583.May 10 2019, 05:34
teamcity added a comment.May 10 2019, 05:34

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

markblundeberg added a child revision: D3021: Merge #12151: rpc: Remove cs_main lock from blockToJSON and blockheaderToJSON.May 10 2019, 16:01
markblundeberg edited the summary of this revision. (Show Details)May 10 2019, 16:03
markblundeberg removed a child revision: D3012: Backport current GetDifficulty logic (& tests) from Core.May 10 2019, 16:11
markblundeberg updated this revision to Diff 8642.May 13 2019, 19:46

rebase

teamcity added a comment.May 13 2019, 19:47

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

Harbormaster completed remote builds in B5837: Diff 8642.May 13 2019, 20:00
teamcity added a comment.May 13 2019, 20:00

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

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 updated this revision to Diff 8658.May 14 2019, 17:37
markblundeberg marked 2 inline comments as done.
markblundeberg edited the summary of this revision. (Show Details)

update per Fabien comments

Harbormaster failed remote builds in B5847: Diff 8658!May 14 2019, 17:50
teamcity added a comment.May 14 2019, 17:50

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

markblundeberg updated this revision to Diff 8668.May 14 2019, 22:47

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

teamcity added a comment.May 14 2019, 22:49

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

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
Harbormaster completed remote builds in B5856: Diff 8668.May 14 2019, 23:02
teamcity added a comment.May 14 2019, 23:02

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

markblundeberg requested review of this revision.May 14 2019, 23:07
markblundeberg edited the test plan for this revision. (Show Details)
In D3014#71749, @deadalnix wrote:

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
markblundeberg updated this revision to Diff 8732.May 19 2019, 17:17

rebase

teamcity added a comment.May 19 2019, 17:18

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

markblundeberg added a child revision: D3070: Remove config argument from blockToJSON.May 19 2019, 17:26
teamcity added a comment.May 19 2019, 17:32

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

Harbormaster completed remote builds in B5894: Diff 8732.May 19 2019, 17:32
markblundeberg mentioned this in D3073: [RPC] Serialize in getblock without cs_main.May 20 2019, 13:54
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)
In D3014#72496, @deadalnix wrote:

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
Closed by commit rABC36645ddefd90: Merge #10095: refactor: Move GetDifficulty out of `rpc/server.h` (authored by Wladimir J. van der Laan <laanwj@gmail.com>, committed by Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com>). · Explain WhyMay 20 2019, 22:47
This revision was automatically updated to reflect the committed changes.
Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> added a commit: rSTAGING36645ddefd90: Merge #10095: refactor: Move GetDifficulty out of `rpc/server.h`.Sep 18 2019, 23:37

Revision Contents
Changeset List

PathSize
src/
1 line
3 lines
rpc/
18 lines
2 lines
1 line

Diff 8732

View Options

src/init.cpp

Loading...
View Options

src/rest.cpp

Loading...
View Options

src/rpc/blockchain.h

Loading...
View Options

src/rpc/blockchain.cpp

Loading...
View Options

src/rpc/server.h

Loading...