[RPC] Add "errors" field to getblockchaininfo and unify "errors" field in get*info RPCs #10858

pull achow101 wants to merge 3 commits into bitcoin:master from achow101:getblockchaininfo-errors changing 5 files +15 −6
  1. achow101 commented at 11:14 PM on July 17, 2017: member

    The getblockchaininfo output does not contain the errors 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 errors 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.

  2. fanquake added the label RPC/REST/ZMQ on Jul 18, 2017
  3. laanwj commented at 6:34 AM on July 18, 2017: member

    IMO, errors does not need to be duplicated on every get*info call. It's fine to have it on only one.

    Also, "warnings" is a better name. None of the messages are really errors, all of them are advisory. Errors result in a node abort. This is why the old name (on the deprecated getinfo) is "errors" and the new one (on getnetworkinfo) is named "warnings". (having it on getmininginfo is weird, though, I didn't even know that)

  4. achow101 force-pushed on Jul 18, 2017
  5. achow101 force-pushed on Jul 18, 2017
  6. achow101 commented at 8:18 PM on July 18, 2017: member

    I have changed the field to be warnings instead of errors. However I left getmininginfo's errors field as is so that we don't break api compatibility.

  7. achow101 force-pushed on Aug 15, 2017
  8. achow101 force-pushed on Aug 15, 2017
  9. jnewbery commented at 7:48 PM on September 1, 2017: member

    I think we can be more aggressive in improving and harmonizing RPCs, as long as we give plenty of warning and time for clients to upgrade. How about changing getmininginfo to return a warning field in the next version (currently 0.16), but add a command line argument to have it return the information in an errors field. Then remove that option in the following version.

  10. achow101 force-pushed on Sep 19, 2017
  11. achow101 commented at 2:03 AM on September 19, 2017: member

    Rebased. @jnewbery I don't like the idea of adding special command line args for dictating whether something is shown in a specific RPC.

  12. jnewbery commented at 1:41 PM on September 19, 2017: member

    @achow101 - do you disagree with changing RPCs in general? I think we need to have some way of evolving the API over releases otherwise mistakes made in the past are set in stone forever.

    If you do agree that the API needs to change over releases, can you suggest a better, safer way of doing it than making the versioning configurable with a command line option? We could obviously add a version path to the RPC endpoint like many REST APIs implement, but I think that's overkill at this stage (and I don't know of anyone who's volunteered implementing it)

  13. achow101 commented at 1:55 PM on September 19, 2017: member

    @jnewbery I agree that the API should change. However I don't think that we should give each deprecated thing its own command line argument to enable or disable it. I think we could do some sort of versioning with different endpoints like what multiwallet does with multiple wallets. So we could add a /deprecated/ endpoint for which any deprecated behavior would exist behind before being removed in the future. If you want to access deprecated stuff, you can use that endpoint, and ideally, non-deprecated stuff would also work behind that endpoint as well.

  14. jnewbery commented at 2:16 PM on September 19, 2017: member

    we could add a /deprecated/ endpoint for which any deprecated behavior would exist behind before being removed in the future. If you want to access deprecated stuff, you can use that endpoint, and ideally, non-deprecated stuff would also work behind that endpoint as well.

    I don't think that's ideal. The user would then:

    1. upgrade, find that an RPC that they use is deprecated and then have to update their client software to access the new endpoint. Updating client software is more difficult than updating a Bitcoin Core command line option, especially for businesses where the client software might be maintained by a different team.
    2. then continue using the /deprecated endpoint for everything, because why wouldn't they? At that point we've lost all benefit of having a staged end-of-life for deprecated RPCs.
  15. achow101 commented at 2:24 PM on September 19, 2017: member

    @jnewbery Ok, then.. How about a -deprecated=<rpc> option? So instead of a switch for every deprecated thing, the user can specify that some or all RPCs should use the deprecated behavior.

  16. jnewbery commented at 3:30 PM on September 19, 2017: member

    How about a -deprecated=<rpc> option?

    Yes, sounds sensible. I think -deprecatedrpcmethod is probably a bit more descriptive. We could just rename the new option from #11031 .

  17. achow101 force-pushed on Sep 26, 2017
  18. achow101 commented at 4:05 PM on September 26, 2017: member

    Rebased onto #11031 to use -deprecatedrpc for showing errors instead of warnings for getmininginfo. I wasn't sure what to do with the help text, so I just left the errors field there with a deprecation warning.

  19. achow101 force-pushed on Sep 26, 2017
  20. achow101 force-pushed on Sep 26, 2017
  21. jnewbery commented at 4:33 PM on September 26, 2017: member

    p2p-versionbits-warning.py requires update to use ['warnings'] instead of ['errors']. Otherwise looks good to me.

  22. achow101 force-pushed on Sep 26, 2017
  23. achow101 commented at 5:43 PM on September 26, 2017: member

    Fixed p2p-versionbits-warning.py

  24. achow101 force-pushed on Sep 26, 2017
  25. Add warnings field to getblockchaininfo f77f0e4825
  26. Unify help text for GetWarnings output in get*info RPCs 8502b20852
  27. Change getmininginfo errors field to warnings
    Changes the errors field to warnings. To maintain compatibility,
the errors field is deprecated and enabled by starting bitcoind with
-deprecatedrpc=getmininginfo
    395cef7601
  28. achow101 force-pushed on Sep 27, 2017
  29. achow101 commented at 3:27 PM on September 27, 2017: member

    Rebased to master

  30. jnewbery commented at 8:26 PM on September 27, 2017: member

    Tested ACK 395cef7601479b97f5794b0c98067c859f00fc7f

  31. in src/rpc/blockchain.cpp:1166 in 395cef7601 
    1162 | @@ -1162,6 +1163,7 @@ UniValue getblockchaininfo(const JSONRPCRequest& request)
1163 |              "        }\n"
1164 |              "     }\n"
1165 |              "  }\n"
1166 | +            "  \"warnings\" : \"...\",         (string) any network and blockchain warnings.\n"
    jonasschnelli commented at 3:12 AM on September 28, 2017:

    nit: I think here is one whitespace missing between text warning[...]and description ((string)[...])

  32. jonasschnelli commented at 3:34 AM on September 28, 2017: contributor

    utACK 395cef7601479b97f5794b0c98067c859f00fc7f

  33. laanwj merged this on Sep 28, 2017
  34. laanwj closed this on Sep 28, 2017
  35. laanwj referenced this in commit 9a8e9167f2 on Sep 28, 2017
  36. laanwj referenced this in commit 3843780fd8 on Feb 8, 2018
  37. PastaPastaPasta referenced this in commit 39bbe50937 on Dec 22, 2019
  38. PastaPastaPasta referenced this in commit 284ebb6bc4 on Jan 2, 2020
  39. PastaPastaPasta referenced this in commit c1299bff49 on Jan 4, 2020
  40. PastaPastaPasta referenced this in commit f0a3c23356 on Jan 12, 2020
  41. PastaPastaPasta referenced this in commit 8e03d839e4 on Jan 12, 2020
  42. PastaPastaPasta referenced this in commit 03bc16162f on Jan 12, 2020
  43. PastaPastaPasta referenced this in commit 5aa0c923ff on Jan 12, 2020
  44. PastaPastaPasta referenced this in commit a287d84b82 on Jan 12, 2020
  45. PastaPastaPasta referenced this in commit 168e93162f on Jun 9, 2020
  46. PastaPastaPasta referenced this in commit 7e04a9a3b1 on Jun 9, 2020
  47. PastaPastaPasta referenced this in commit 8254d30d21 on Jun 10, 2020
  48. PastaPastaPasta referenced this in commit 55cc7d8f89 on Jun 10, 2020
  49. PastaPastaPasta referenced this in commit da0042972b on Jun 11, 2020
  50. PastaPastaPasta referenced this in commit 1b7e6b45af on Jun 11, 2020
  51. ckti referenced this in commit fcc9cac35d on Mar 28, 2021
  52. random-zebra referenced this in commit 2d50b6e3b9 on Jun 9, 2021
  53. gades referenced this in commit e40d0d3a08 on Jun 26, 2021
  54. DrahtBot locked this on Sep 8, 2021
Contributors
achow101laanwjjnewberyjonasschnelli
Labels
RPC/REST/ZMQ
Linked (view graph)
#11412 Simplify GetWarnings()#12336 Remove deprecated rpc options
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 15:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me