Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#28577: net: raise V1_PREFIX_LEN from 12 to 16
Browse files Browse the repository at this point in the history
ba2e5bf net: raise V1_PREFIX_LEN from 12 to 16 (Pieter Wuille)

Pull request description:

  A "version" message in the V1 protocol starts with a fixed 16 bytes:
  * The 4-byte network magic
  * The 12-byte command string: "version" plus 5 0x00 bytes

  The current code detects incoming V1 connections by just looking at the first 12 bytes (matching an [earlier version](bitcoin/bips#1496) of BIP324), but 16 bytes is more precise. This isn't an observable difference right now, as a 12 byte prefix ought to be negligible already, but it may become observable with future extensions to the protocol, so make the code match the specification.

ACKs for top commit:
  achow101:
    ACK ba2e5bf
  theStack:
    re-ACK ba2e5bf
  mzumsande:
    Code review ACK ba2e5bf

Tree-SHA512: 64876b03613bd1c5dda82f4ca1b367014365f9ae4cfa30f45c5758a563c68cbea81a98d02ba616c264674c204517aac8b7de94da10f32e77b56267a43710c651
  • Loading branch information
achow101 committed Oct 4, 2023
2 parents 8369467 + ba2e5bf commit 5b44784
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
10 changes: 5 additions & 5 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1098,10 +1098,10 @@ void V2Transport::ProcessReceivedMaybeV1Bytes() noexcept
AssertLockNotHeld(m_send_mutex);
Assume(m_recv_state == RecvState::KEY_MAYBE_V1);
// We still have to determine if this is a v1 or v2 connection. The bytes being received could
// be the beginning of either a v1 packet (network magic + "version\x00"), or of a v2 public
// key. BIP324 specifies that a mismatch with this 12-byte string should trigger sending of the
// key.
std::array<uint8_t, V1_PREFIX_LEN> v1_prefix = {0, 0, 0, 0, 'v', 'e', 'r', 's', 'i', 'o', 'n', 0};
// be the beginning of either a v1 packet (network magic + "version\x00\x00\x00\x00\x00"), or
// of a v2 public key. BIP324 specifies that a mismatch with this 16-byte string should trigger
// sending of the key.
std::array<uint8_t, V1_PREFIX_LEN> v1_prefix = {0, 0, 0, 0, 'v', 'e', 'r', 's', 'i', 'o', 'n', 0, 0, 0, 0, 0};
std::copy(std::begin(Params().MessageStart()), std::end(Params().MessageStart()), v1_prefix.begin());
Assume(m_recv_buffer.size() <= v1_prefix.size());
if (!std::equal(m_recv_buffer.begin(), m_recv_buffer.end(), v1_prefix.begin())) {
Expand Down Expand Up @@ -1295,7 +1295,7 @@ size_t V2Transport::GetMaxBytesToProcess() noexcept
// receive buffer.
Assume(m_recv_buffer.size() <= V1_PREFIX_LEN);
// As long as we're not sure if this is a v1 or v2 connection, don't receive more than what
// is strictly necessary to distinguish the two (12 bytes). If we permitted more than
// is strictly necessary to distinguish the two (16 bytes). If we permitted more than
// the v1 header size (24 bytes), we may not be able to feed the already-received bytes
// back into the m_v1_fallback V1 transport.
return V1_PREFIX_LEN - m_recv_buffer.size();
Expand Down
2 changes: 1 addition & 1 deletion src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ class V2Transport final : public Transport

/** The length of the V1 prefix to match bytes initially received by responders with to
* determine if their peer is speaking V1 or V2. */
static constexpr size_t V1_PREFIX_LEN = 12;
static constexpr size_t V1_PREFIX_LEN = 16;

// The sender side and receiver side of V2Transport are state machines that are transitioned
// through, based on what has been received. The receive state corresponds to the contents of,
Expand Down

0 comments on commit 5b44784

Please sign in to comment.