Re: [Bitcoin-development] Missing fRelayTxes in version

[Pieter Wuille <pieter.wuille@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 6744 bytes --]

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" <turkeybreast@yahoo.com> 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 <mike@plan99.net>
> *To:* Pieter Wuille <pieter.wuille@gmail.com>
> *Cc:* Bitcoin Dev <bitcoin-development@lists.sourceforge.net>; Tamas
> Blummer <tamas@bitsofproof.com>
> *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 <pieter.wuille@gmail.com>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
>
>

[-- Attachment #2: Type: text/html, Size: 11057 bytes --]