bitcoin/bitcoin — pull request #3982 by davecgh (2014-03-30 19:54:52+00:00)
Title: Decouple addrFrom and nNonce fields in version message read

The current code assumes that if the receive buffer for a version message is not empty after the addrMe field, it must contain the addrFrom field and the nNonce field. This pull request moves the nNonce read into its own statement after checking if the receive buffer is empty. This decouples the optionality of the fields so that each can be optional as opposed to both being required when addrFrom is present.

Note that there is an alternative way to approach this that I was unsure would be preferred, so I chose the approach in the pull request after discussing with @sipa who indicated the each read should be probably be separate.

The alternative approach comes from looking at commit 18c0fa97d0408a3ee8e4cb39c08156f7667f99ac which indicates that the three fields of addrFrom, nNonce, and strSubVersion were added in protocol version 106. In that case, perhaps all three reads should be under a single `if (!vRecv.empty())` check which indicates that all three must be present if the receive buffer is not empty after the addrMe field which already existed in prior protocol versions.

In either case, since this code defines the protocol spec, I think the handling should be one of the two approaches versus the current approach where two of the fields that were introduced in protocol version 106 are coupled where the third one is not.

If the alternative approach is preferred, I can update the pull request accordingly.