Incorrect protocol version documented for version.relay.

[evoskuil]

Bounty: 12,000 bits / ~$40 USD

According to other bitcoin.org documentation and observation of the network, version.relay was not enabled in Bitcoin Core until protocol version 70002, but is documented as version 70001:

Added in protocol version 70001 as described by BIP37.

[evoskuil]

Review of the sources shows that version.relay is read but not written in version 0.8.x (70001) and it is read and written in version 0.9.0 (70002).

[Mirobit]

Agreed it doesn't work with version 70001.

[evoskuil]

To be clear, it half-works, which can be a bit confusing.

[wbnns]

@evoskuil @Mirobit Thanks for bringing this up - what do you all propose to resolve this issue?

[evoskuil]

@wbnns There is proper Bitcoin Protocol behavior and there is the Bitcoin Core implementation. Assuming you are documenting the former I would document the proper behavior according to the BIP. Even so it might be worth a note on the bug since it needs to be worked around for useful operation against these older nodes. I've found a few of these types of issues in various clients because libbitcoin has configurable min and max protocol levels.

[wbnns]

Bounty: 12,000 bits / ~$30 USD

[achow101]

The field is optional. It does not need to be set even if the protocol version is 70001 or higher.

[evoskuil]

@achow101 Agree, it being optional makes it unimportant that Bitcoin Core reads it in 70001 but doesn't write it until 70002. I had been operating in consideration of BIP60, which makes it mandatory. Since BIP37 is "final" and BIP60 is "draft", your interpretation is correct.

[evoskuil]

Just for the record I would point out that the version.relay field is not valid at any protocol level prior to 70001. I mention this because I do encounter the occasional peer that sets it at these lower levels.

[evoskuil]

Ugh, I just reminded myself again why this sucks. The relay field is added to the version message. It is not possible for a pair of peers to know the each other's versions before sending the version message (the second one could). So the BIP37 specification is a bug, as it breaks the handshake with any earlier version peer that properly validates the version message. There's no good way to handle it apart from going back and patching those older versions and any new version that implements an older (and perfectly relevant) protocol version.

[harding]

@evoskuil I suggest you re-open the issue. If this complication is making it hard for implementors to write software, it's something that should be documented IMO. (That's what the documentation is for, after all, not just helping people fall asleep at their desks because it's so boring to read. :-)

@wbnns if he does re-open, maybe you can remove the bounty so that this issue can be addressed with care rather than with haste? It's up to you whether @achow101 deserves the bounty for moving this issue forward (and then what does @evoskuil deserve for taking the time to report and carefully describe this issue? I hate this whole bounty thing).

[wbnns]

@harding Hey, not sure if you saw, but this was opened on August 17, 2016 and the Pull Request Welcome tag has been present for several months. I don't think there was anything hasty about adding a bounty the other day (over a year and a half after the issue was opened).

Whoever submits the pull request that resolves the issue gets the bounty. Also, no need to digress that you "hate" the bounty system (you already commented on #1606 which is sufficient). Let's keep things positive.

[harding]

@wbnns I agree comments about the bounty system should be left on #1606. I apologize for my digression.

[wbnns]

@harding Ok, cool, no worries - thanks man.

[evoskuil]

@harding Okay, will do.

This issue is so persistently annoying to P2P protocol development that I feel like revising BIP60 to retroactively invalidate version.relay altogether in BIP37 (as opposed to requiring version changes to be fixed-size, which doesn't resolve the issue).

I finally realized that it is likely Core support for sending relay was released a (protocol) version after reading relay for the simple reason that sending it to size-validating peers would cause the new implementation to be dropped during handshake, at least in the cases where the peer received the version message first. By incrementally deploying the change, once there was sufficient coverage of new nodes, the team would be intentionally isolating all size-validating pre-BIP37 nodes. This would force an upgrade to nodes which presumably have BIP37's bloom filtering enabled.

It's possible that the upgrade was just sloppy work, but it seems likely that the planned obsolescence was not seen as a negative. It's also possible that Core itself doesn't (or didn't) validate the size of the version message, in which case it would have been seen as benign. I've seen this same broken reasoning in the BIP151 proposal, which treats a totally invalid message type (i.e. prior to the handshake) as not breaking compatibility. This causes the same problem in that message-type-validating peers must drop the connection attempt before it can be downgraded.

Libbitcoin validates both message size and message type for all messages. If the protocol version does not support the message then the peer is dropped. It also supports configurable protocol version levels, able to emulate a peer of all versions back to 31402. This is done for historical/educational reasons, for easy development testing against older/specific peers, and as a matter of proper protocol implementation. All down-level peers should operate properly against any new peer. Protocol versions do not expire, and there are reasons to use lower level protocols even in new implementations.

So that's my rant :/. There's no good way to "fix" this since BIP37 itself is broken. Working around the bug means either never setting relay (and always getting tx flow) or making it impossible to connect to size-validating peers of protocol versions below 70001. Core requires a minimum peer protocol version of 31800 and Libbitcoin requires at least 31402. Allowing the extra byte, in violation of the original protocol, in modified down-level size-validating nodes, is the only option. Older implementations would need to be modified.