Details

Reviewers

deadalnix
jasonbcox
Fabien

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGING0264e4a57d73: Merge #10858: [RPC] Add "warnings" field to getblockchaininfo and unify…
rSTAGING2e5a9ba1e9cf: Merge #10858: [RPC] Add "warnings" field to getblockchaininfo and unify…
rABC0264e4a57d73: Merge #10858: [RPC] Add "warnings" field to getblockchaininfo and unify…
rABC2e5a9ba1e9cf: Merge #10858: [RPC] Add "warnings" field to getblockchaininfo and unify…

Summary

395cef7 Change getmininginfo errors field to warnings (Andrew Chow)
8502b20 Unify help text for GetWarnings output in get*info RPCs (Andrew Chow)
f77f0e4 Add warnings field to getblockchaininfo (Andrew Chow)

Pull request description:

The `getblockchaininfo` output does not contain the `warnings` field which the `getinfo`, `getmininginfo`, and `getnetworkinfo` RPCs have. It should have it as the errors pertain to the blockchain. This PR adds that field.

This PR also unifies the help text for the `warnings` field and its output position so that all of the `get*info` commands are consistent.

`getnetworkinfo`'s `errors` field is named `warnings`. I did not change this even though it is inconsistent since this naming has been in use for a long time.

Tree-SHA512: 385ab6acfee67fc8816f4d51ab2bd7a623264c7973906dfbab0a171f199e9db16fde19093a5bc3dfbdd4ff5f19d2186b646eb6b3bae0a4d7c9add43650a4a9d9

Backport of PR10858
https://github.com/bitcoin/bitcoin/commit/9a8e9167f2636fdc2fc047dfed1747920b0f047f?diff=unified#diff-a0c8f511d90e83aa9b5857e819ced344R1165

Completes T567

Test Plan

make check
test_runner.py
getblockchaininfo should also produce a "warnings" field
getmininginfo should have a "warnings" field in place of its "errors" field during regular use
getmininginfo should behave as previously (i.e. have an "errors" field) when running bitcoind -deprecatedrpc=getmininginfo

Diff Detail

Repository

rABC Bitcoin ABC

Branch

BackportPR10858

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 5284
Build 8630: Bitcoin ABC Buildbot (legacy)
Build 8629: arc lint + arc unit

Event Timeline

nakihito created this revision.Mar 22 2019, 23:34

Owners added a reviewer: Restricted Owners Package.Mar 22 2019, 23:34

Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 22 2019, 23:34

Herald added a subscriber: schancel. · View Herald Transcript

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

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

Harbormaster completed remote builds in B5282: Diff 7802.Mar 22 2019, 23:49

nakihito edited the test plan for this revision. (Show Details)Mar 22 2019, 23:58

nakihito mentioned this in D2731: [rpc] getblockchaininfo: add size_on_disk, prune_target_size, automatic_pruning.Mar 23 2019, 00:00

Fixed indentation in getblockchaininfo help.

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

Harbormaster completed remote builds in B5284: Diff 7805.Mar 23 2019, 00:39

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

nakihito mentioned this in D2732: [rpc] getblockchaininfo: add size_on_disk, prune_target_size, automatic_pruning.Mar 23 2019, 02:05

The title and summary of this diff are highly confusing. I know it was just a copy from the PR description, but please update all mentions of errors to warnings to match the change in decision they made in the PR (see comments of the original PR).

src/rpc/blockchain.cpp
1238	If you did these indents yourself, please revert them. If it was the linter, leave as-is.
src/rpc/mining.cpp
267	These behavior change needs a mention in release notes.

This revision now requires changes to proceed.Mar 25 2019, 17:53

nakihito retitled this revision from Merge #10858: [RPC] Add "errors" field to getblockchaininfo and unify "errors" field in get*info RPCs to Merge #10858: [RPC] Add "warnings" field to getblockchaininfo and unify "warnings" field in get*info RPCs.Mar 25 2019, 18:50

nakihito edited the summary of this revision. (Show Details)

nakihito added inline comments.Mar 25 2019, 19:26

src/rpc/blockchain.cpp
1238	A similar change was requested by Fabien in a different diff (see https://reviews.bitcoinabc.org/D2711?id=7751#inline-16561).

Updated release notes with new behavoir.

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

Harbormaster completed remote builds in B5293: Diff 7821.Mar 25 2019, 21:24

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

Fabien added inline comments.Mar 25 2019, 21:52

src/rpc/blockchain.cpp
1238	This is a good idea to fix it, but this time it's not part of the original PR (it was the last time). You'd better separate it from this diff to keep this backport closer to the PR, this make it easier to review.

nakihito marked 2 inline comments as done.Mar 25 2019, 22:06

Removed indentation fix in getblockchain help.

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

nakihito mentioned this in D2737: Adjusted indentation in getblockchaininfo's help text to properly align..Mar 25 2019, 22:19

nakihito added inline comments.Mar 25 2019, 22:20

src/rpc/blockchain.cpp
1238	Separated the indentation changes to here: https://reviews.bitcoinabc.org/D2737

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

Harbormaster completed remote builds in B5297: Diff 7826.Mar 25 2019, 22:22

Fabien added a child revision: D2737: Adjusted indentation in getblockchaininfo's help text to properly align..Mar 26 2019, 08:13

The deprecation feature is missing from the test plan.

This revision now requires changes to proceed.Mar 26 2019, 09:27

Fabien added a child revision: D2732: [rpc] getblockchaininfo: add size_on_disk, prune_target_size, automatic_pruning.Mar 26 2019, 10:05

In D2730#64163, @Fabien wrote:

The deprecation feature is missing from the test plan.

Sorry, I didn't specify that case in the test plan. Updated test plan to be more specific.

nakihito edited the test plan for this revision. (Show Details)Mar 26 2019, 20:08

jasonbcox accepted this revision.Mar 26 2019, 21:34

jasonbcox added inline comments.

doc/release-notes.md
14 ↗	(On Diff #7826)	Nit: the release notes will render better if you use backticks ` rather than single quotes ' since this file is Markdown. For example, look how backticks render on Github: https://github.com/Bitcoin-ABC/bitcoin-abc/blob/master/doc/release-notes.md