P2P protocol version message incorrectly defined.

[evoskuil]

The described addr_recv services and addr_trans services are not properly part of the version message.

[harding]

@evoskuil could you provide more information about what you mean by "not properly part of"? If I recall correctly, when I wrote that section, they appeared to be part of all the version messages I inspected on the network and they were part of the Bitcoin [Core] code since 2009.

[evoskuil]

I just meant that these do not show up on the wire for any version messages from any nodes on mainnet, nor will any node accept them.

[evoskuil]

Also notice they aren't documented here: https://en.bitcoin.it/wiki/Protocol_documentation#version

[laanwj]

This is the code in net.cpp that sends the version message:
https://github.com/bitcoin/bitcoin/blob/master/src/net.cpp#L504

    PushMessage(NetMsgType::VERSION, PROTOCOL_VERSION, (uint64_t)nLocalServices, nTime, addrYou, addrMe,
                nLocalHostNonce, strSubVersion, nBestHeight, ::fRelayTxes);

It does send two CAddress structures: addrYou and addrMe after the timestamp.
A CAddress is serialized as:

uint64_t services
uint8_t ip[16];
uint16_t port;

So from what I can see, the documentation is correct.

Also notice they aren't documented here: https://en.bitcoin.it/wiki/Protocol_documentation#version

It doesn't expand the subfields there, but addr_recv and addr_from are there.

[laanwj]

It doesn't look conditional, in CAddress::SerializationOp: https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L301

[evoskuil]

Sorry, my mistake... my address deserialization is capturing the services fields which I didn't notice.