This pull request removes the dependency of serialize.h on PROTOCOL_VERSION, and makes this parameter required instead of implicit. This is much saner in my opinion, as it makes the places where changing a version number can have an influence obvious.
This builds upon #1109, but since it may be more controversial, it's a separate pull request.
It is unfortunate that every usage site gets larger, with this patch.
Yes, I did it that way because it exposed all places where implicit versions where being used. In many places, this implicit value must obey some hidden rules. For example, GetHash() needs to be done using a constant version number, or everything would break, but used PROTOCOL_VERSION formerly. If we ever do a protocol change that adds fields to blocks or transactions, this has to be thought through thoroughly, and many places where PROTOCOL_VERSION is used, will need to be changed to pNode->nVersion, for example. With this change, those places become obvious.
Or put otherwise: i think using PROTOCOL_VERSION is a bad default anyway, because as soon as the version number becomes relevant, every piece of code that uses the default constructor will have to be changed anyway.
Larger, but also more explicit and less magic. I like it.
New diagram: https://dev.visucore.com/bitcoin/includes_saneserial.svg
I also expect that making the version parameter explicit and passing it through prevents many of the "argument unused" warnings currently emitted in the headers.
I doubt that; they don't become magically used because they're explicit instead of implicit.
In addition to standard code separation, this change opens the door
to fixing several include inter-dependencies.
This commit removes the dependency of serialize.h on PROTOCOL_VERSION,
and makes this parameter required instead of implicit. This is much saner,
as it makes the places where changing a version number can have an
influence obvious.