Let's just increase the version number and be done with this discussion. It's a small benefit, but it simplifies things and it's trivial to do. I don't understand how a policy of requiring version increases could limit future extensions: after the version/verack exchange, the protocol version is negotiated between peers, and there is no need for anything optional anymore. Note thay this is just about parsing, not about relaying - you should still relay parts of a message you haven't parsed. But that doesn't apply to the version message anyway, which is the only place where this matters. -- Pieter On 20 Jun 2013 12:38, "Turkey Breast" wrote: > I don't get why this is such a contentious change? > > Before I was able to use asserts to check the expected length of length of > messages per protocol version, I could pass in dumb iterators that just > parse the byte stream and I could serialize and deserialize a message to > check the parser is correct (in debug mode). > > This 'simple' change causes all that behaviour to be lost. You can no > longer just use iterators but must know the remaining length (or if you use > std::distance, you can only use specific std containers - not just anything > with an iterator and an operator++). You cannot check the deserialization > process by serializing the deserialized message and comparing it to the > original data (because the bool is always present in the serializer). > > It's a bit stupid you call it buggy code when this behaviour has never > been present in Bitcoin. The BIP doesn't introduce any unwanted > side-effects and is a trivial reasonable change. > > If you want optional fields then the proper way to do it, is to either set > a flag in the Services field of the "version" message to indicate different > formats for messages (i.e use this template structure for a message, not > that one), introduce a new message (if the changes are big), to > approve/improve Stefan's BIP 32 for custom services or to have a value in > the byte stream indicating which fields are present (maybe a bitfield or > so). > > Using a quirk of an implementation is just bad form and sloppy coding. > Optional fields should have their own mechanism that allows them to remain > as optional fields between protocol version upgrades. > > The bitcoind software can probably be improved too, by checking that the > length of the version message is consistent for the protocol version given > by the connected node. Right now it makes no assumptions based on that > which is a mistake (new clients can broadcast older version messages that > don't have all the fields required). Probably the software should penalise > hosts which do that. > > What's the big deal to update the protocol version number from 70001 to > 70002? It's not like we'll run out of integers. The field has now gone from > optional to required now anyway - that's a behaviour change. It'd be good > to enforce that. I see this as a bug. > > ------------------------------ > *From:* Mike Hearn > *To:* Pieter Wuille > *Cc:* Bitcoin Dev ; Tamas > Blummer > *Sent:* Thursday, June 20, 2013 11:17 AM > *Subject:* Re: [Bitcoin-development] Missing fRelayTxes in version > > There's no problem, but there's no benefit either. It also locks us in to > a potentially problematic guarantee - what if in future we want to have, > say, two optional new pieces of data in two different messages. We don't > want to require that if version > X then you have to implement all features > up to and including that point. > > Essentially the number of fields in a message is like a little version > number, just for that message. It adds flexibility to keep it that way, and > there's no downside, seeing as that bridge was already crossed and people > with parsers that can't handle it need to fix their code anyway. > > So I have a slight preference for keeping things the way they are, it > keeps things flexible for future and costs nothing. > > > > On Thu, Jun 20, 2013 at 11:06 AM, Pieter Wuille wrote: > > On Thu, Jun 20, 2013 at 09:36:40AM +0200, Mike Hearn wrote: > > Sure but why not do that when there's an actual new field to add? Does > > anyone have a proposal for a feature that needs a new version field at > the > > moment? There's no point changing the protocol now unless there's > actually > > a new field to add. > > > > Anyway I still don't see why anyone cares about this issue. The Bitcoin > > protocol does not and never has required that all messages have a fixed > > number of fields per version. Any parser written on the assumption it did > > was just buggy. Look at how tx messages are relayed for the most obvious > > example of that pattern in action - it's actually the raw byte stream > > that's stored and relayed to ensure that fields added in new versions > > aren't dropped during round-tripping. Old versions are supposed to > preserve > > fields from the future. > > Actually, that is not the same issue. What is being argued for here is that > the version in the version message itself should indicate which fields are > present, so a parser doesn't need to look at the length of the message. > That > seems like a minor but very reasonable request to me, and it's trivial to > do. > That doesn't mean that you may receive versions higher than what you know > of, > and thus messages with fields you don't know about. That doesn't matter, > you > can just ignore them. > > I see no problem with raising the protocol version number to indicate > "all fields up to fRelayTxes are required, if the announced nVersion is > above N". > In fact, I believe (though haven't checked) all previous additions to the > version > message were accompanied with a protocol version (then: client version) > increase > as well. > > -- > Pieter > > > > > ------------------------------------------------------------------------------ > This SF.net email is sponsored by Windows: > > Build for Windows Store. > > http://p.sf.net/sfu/windows-dev2dev > _______________________________________________ > Bitcoin-development mailing list > Bitcoin-development@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/bitcoin-development > > > > > ------------------------------------------------------------------------------ > This SF.net email is sponsored by Windows: > > Build for Windows Store. > > http://p.sf.net/sfu/windows-dev2dev > _______________________________________________ > Bitcoin-development mailing list > Bitcoin-development@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/bitcoin-development > >