Page MenuHomePhabricator

Merge #10858: [RPC] Add "warnings" field to getblockchaininfo and unify "warnings" field in get*info RPCs
ClosedPublic

Authored by nakihito on Mar 22 2019, 23:34.

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
nakihito edited the test plan for this revision. (Show Details)Mar 22 2019, 23:58
nakihito updated this revision to Diff 7805.Mar 23 2019, 00:24
nakihito edited the test plan for this revision. (Show Details)

Fixed indentation in getblockchaininfo help.

jasonbcox requested changes to this revision.Mar 25 2019, 17:53

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 ↗(On Diff #7805)

If you did these indents yourself, please revert them. If it was the linter, leave as-is.

src/rpc/mining.cpp
267 ↗(On Diff #7805)

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 ↗(On Diff #7805)

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

nakihito updated this revision to Diff 7821.Mar 25 2019, 21:09
nakihito marked an inline comment as done.

Updated release notes with new behavoir.

Fabien added inline comments.Mar 25 2019, 21:52
src/rpc/blockchain.cpp
1238 ↗(On Diff #7805)

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
nakihito updated this revision to Diff 7826.Mar 25 2019, 22:07

Removed indentation fix in getblockchain help.

nakihito added inline comments.Mar 25 2019, 22:20
src/rpc/blockchain.cpp
1238 ↗(On Diff #7805)

Separated the indentation changes to here: https://reviews.bitcoinabc.org/D2737

Fabien requested changes to this revision.Tue, Mar 26, 09:27

The deprecation feature is missing from the test plan.

This revision now requires changes to proceed.Tue, Mar 26, 09:27
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)Tue, Mar 26, 20:08
jasonbcox accepted this revision.Tue, Mar 26, 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

nakihito updated this revision to Diff 7839.Tue, Mar 26, 21:42

Changed single quotes to ` in release notes.

nakihito updated this revision to Diff 7840.Tue, Mar 26, 22:06

Forgot to add `s to warnings.

Fabien accepted this revision.Thu, Mar 28, 08:42
This revision is now accepted and ready to land.Thu, Mar 28, 08:42