[qa] Don’t set unknown rpcserialversion #9322

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:Mf1612-qaSerial changing 2 files +5 −3
  1. MarcoFalke commented at 1:18 pm on December 11, 2016: member

    When merged, this will merge #9292 as well.

    I created a new pull, so 0.13.2 isn’t blocked by this.

  2. Complain when unknown rpcserialversion is specified 80d073c9bc
  3. [qa] Don't set unknown rpcserialversion fa615d39b5
  4. MarcoFalke added the label RPC/REST/ZMQ on Dec 11, 2016
  5. MarcoFalke added the label Tests on Dec 11, 2016
  6. MarcoFalke added this to the milestone 0.13.2 on Dec 11, 2016
  7. luke-jr commented at 2:25 pm on December 11, 2016: member
    It looks intentional and desirable to allow setting future serialisation versions. Especially something like 9999 to mean “use the very latest even if not the default”.
  8. sipa commented at 6:56 pm on December 11, 2016: member
    @luke-jr Makes sense, but I was assuming that’s what the default was.
  9. gmaxwell commented at 6:59 am on December 13, 2016: contributor
    ACK
  10. in src/init.cpp: in fa615d39b5
    987@@ -988,6 +988,9 @@ bool AppInitParameterInteraction()
    988     if (GetArg("-rpcserialversion", DEFAULT_RPC_SERIALIZE_VERSION) < 0)
    989         return InitError("rpcserialversion must be non-negative.");
    990 
    991+    if (GetArg("-rpcserialversion", DEFAULT_RPC_SERIALIZE_VERSION) > 1)
    


    laanwj commented at 11:19 am on December 13, 2016:
    Maybe define a constant for the max valid RPC serialize version?

    MarcoFalke commented at 11:19 am on December 14, 2016:

    Which would imply the default is not always the max. It is not clear from the comments if this is desired.

    Edit: Anyway, fixing this nit is irrelevant for backport. Either we merge/backport this and bikeshed later or we close the pull based on luke-jr’s rationale.

  11. MarcoFalke added the label Needs backport on Dec 13, 2016
  12. laanwj approved
  13. laanwj merged this on Dec 15, 2016
  14. laanwj closed this on Dec 15, 2016

  15. laanwj referenced this in commit c9e00591cd on Dec 15, 2016
  16. MarcoFalke deleted the branch on Dec 15, 2016
  17. MarcoFalke removed the label Needs backport on Dec 15, 2016
  18. MarcoFalke referenced this in commit 49a612f347 on Dec 15, 2016
  19. DrahtBot locked this on Sep 8, 2021

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: 2024-12-19 03:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me