[qa] Don't set unknown rpcserialversion #9322

Merged
merged 2 commits into from Dec 15, 2016

Projects

None yet

5 participants

@MarcoFalke
Member

When merged, this will merge #9292 as well.

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

@MarcoFalke MarcoFalke added this to the 0.13.2 milestone Dec 11, 2016
@luke-jr
Member
luke-jr commented Dec 11, 2016

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".

@sipa
Member
sipa commented Dec 11, 2016

@luke-jr Makes sense, but I was assuming that's what the default was.

@gmaxwell
Member

ACK

@laanwj
laanwj approved these changes Dec 14, 2016 View changes
@@ -988,6 +988,9 @@ bool AppInitParameterInteraction()
if (GetArg("-rpcserialversion", DEFAULT_RPC_SERIALIZE_VERSION) < 0)
return InitError("rpcserialversion must be non-negative.");
+ if (GetArg("-rpcserialversion", DEFAULT_RPC_SERIALIZE_VERSION) > 1)
@laanwj
laanwj Dec 14, 2016 Member

Maybe define a constant for the max valid RPC serialize version?

@MarcoFalke
MarcoFalke Dec 14, 2016 edited Member

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.

@laanwj laanwj merged commit fa615d3 into bitcoin:master Dec 15, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@laanwj laanwj added a commit that referenced this pull request Dec 15, 2016
@laanwj laanwj Merge #9322: [qa] Don't set unknown rpcserialversion
fa615d3 [qa] Don't set unknown rpcserialversion (MarcoFalke)
80d073c Complain when unknown rpcserialversion is specified (Pieter Wuille)
c9e0059
@MarcoFalke MarcoFalke deleted the MarcoFalke:Mf1612-qaSerial branch Dec 15, 2016
@MarcoFalke MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Dec 15, 2016
@MarcoFalke MarcoFalke [qa] Don't set unknown rpcserialversion
Github-Pull: #9322
Rebased-From: fa615d3
49a612f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment