From: Pieter Wuille <pieter.wuille@gmail•com>
To: Turkey Breast <turkeybreast@yahoo•com>
Cc: Bitcoin Dev <bitcoin-development@lists•sourceforge.net>
Subject: Re: [Bitcoin-development] Missing fRelayTxes in version
Date: Thu, 20 Jun 2013 12:52:51 +0200 [thread overview]
Message-ID: <CAPg+sBiJvWckJYa-iw=qtjfj1PoY_bKhFD3sx6LqfUrFbWrGHA@mail.gmail.com> (raw)
In-Reply-To: <1371724625.50978.YahooMailNeo@web162706.mail.bf1.yahoo.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 --]
next prev parent reply other threads:[~2013-06-20 10:52 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-20 7:30 Tamas Blummer
2013-06-20 7:36 ` Mike Hearn
2013-06-20 8:17 ` Tamas Blummer
2013-06-20 8:31 ` Mike Hearn
2013-06-20 8:39 ` Tamas Blummer
2013-06-20 9:06 ` Pieter Wuille
2013-06-20 9:17 ` Mike Hearn
2013-06-20 10:37 ` Turkey Breast
2013-06-20 10:50 ` Mike Hearn
2013-06-20 10:52 ` Pieter Wuille [this message]
2013-06-20 10:58 ` Mike Hearn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPg+sBiJvWckJYa-iw=qtjfj1PoY_bKhFD3sx6LqfUrFbWrGHA@mail.gmail.com' \
--to=pieter.wuille@gmail$(echo .)com \
--cc=bitcoin-development@lists$(echo .)sourceforge.net \
--cc=turkeybreast@yahoo$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox