Tested on main branch locally commit 285d9ad

Summary

When a peer sends protocol messages (HEADERS, GETHEADERS, ADDR, INV) before completing the VERSION/VERACK handshake, the node silently ignores the message instead of disconnecting the
misbehaving peer. This is inconsistent with other protocol violations (e.g., sending a non-VERSION as the first message correctly triggers disconnect).

This allows misbehaving or malicious peers to occupy inbound connection slots indefinitely without completing the handshake.

Environment

• OS: Linux 6.17.0-19-generic
• unicity-node: regtest
• Deterministic: reproduces 100% of the time

Steps to Reproduce

Manual reproduction

1. Start a node
   ./build/bin/unicityd --regtest --listen --port=29590 &

2. Connect with a raw socket, send VERSION but NOT VERACK, then send HEADERS

python3 -c "
import socket, struct, hashlib, time

MAGIC = 0x4B7C2E91  # regtest

def checksum(p):
    return hashlib.sha256(hashlib.sha256(p).digest()).digest()[:4]

def msg(cmd, payload=b''):
    return (struct.pack('<I', MAGIC) +
            cmd.encode().ljust(12, b'\x00') +
            struct.pack('<I', len(payload)) +
            checksum(payload) + payload)

def version_payload():
    p = struct.pack('<i', 1) + struct.pack('<Q', 1) + struct.pack('<q', int(time.time()))
    p += struct.pack('<Q', 1) + b'\x00'*10 + b'\xff\xff' + bytes([127,0,0,1]) + struct.pack('>H', 29590)
    p += struct.pack('<Q', 0) + b'\x00'*16 + struct.pack('>H', 0)
    p += struct.pack('<Q', 42)
    ua = b'/Test/'
    p += bytes([len(ua)]) + ua + struct.pack('<i', 0)
    return p

s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.settimeout(5)
s.connect(('127.0.0.1', 29590))

# Send VERSION (starts handshake)
s.sendall(msg('version', version_payload()))
time.sleep(0.5)
s.recv(4096)  # receive node's VERSION

# Send HEADERS before VERACK (protocol violation!)
s.sendall(msg('headers', b'\x00'))
time.sleep(2)

# Check if still connected
try:
    s.sendall(b'')  # probe
    print('STILL CONNECTED — BUG: node should have disconnected')
except:
    print('DISCONNECTED — correct behavior')
s.close()
"

Automated reproduction

A comprehensive test is available

bug_pre_verack_no_disconnect.py

cd test/functional
copy test there
python3 bug_pre_verack_no_disconnect.py

The test:

1. Verifies baseline: non-VERSION as first message correctly disconnects (peer.cpp:618-622)
2. Tests 4 message types sent before VERACK: HEADERS, GETHEADERS, ADDR, INV
3. Each should trigger disconnect but instead the message is silently ignored

Example output:

=== Baseline: Non-VERSION as first message (should disconnect) ===
BASELINE OK: Non-VERSION first message caused disconnect

=== Pre-VERACK message tests (all should disconnect) ===
Test: HEADERS before VERACK
BUG CONFIRMED: Node did NOT disconnect. Message silently ignored.
Test: GETHEADERS before VERACK
BUG CONFIRMED: Node did NOT disconnect. Message silently ignored.
Test: ADDR before VERACK
BUG CONFIRMED: Node did NOT disconnect. Message silently ignored.
Test: INV before VERACK
BUG CONFIRMED: Node did NOT disconnect. Message silently ignored.

BUGS CONFIRMED: 4 message type(s) silently ignored pre-VERACK

Root Cause

File: src/network/peer.cpp:724-728

} else {
// Only process non-handshake messages after handshake is complete
if (!successfully_connected_) {
LOG_NET_DEBUG("received {} before handshake complete from peer={}, ignoring", command, id_);
return; // ← BUG: should call post_disconnect() before returning
}
// Pass to handler
if (message_handler_) {
message_handler_(shared_from_this(), std::move(msg));
}
}

The catch-all handler for non-handshake messages received before successfully_connected_ (set after VERACK) logs at DEBUG level and returns without disconnecting.

Inconsistency with other handlers

┌──────────────────────────────┬───────────────────────────┬───────────────────┬──────────┐
│ Violation │ Code location │ Behavior │ Correct? │
├──────────────────────────────┼───────────────────────────┼───────────────────┼──────────┤
│ Non-VERSION as first message │ peer.cpp:618-622 │ post_disconnect() │ Yes │
├──────────────────────────────┼───────────────────────────┼───────────────────┼──────────┤
│ VERACK with payload │ message.cpp (deser fails) │ Disconnect │ Yes │
├──────────────────────────────┼───────────────────────────┼───────────────────┼──────────┤
│ HEADERS before VERACK │ peer.cpp:726 │ Silent ignore │ No │
├──────────────────────────────┼───────────────────────────┼───────────────────┼──────────┤
│ GETHEADERS before VERACK │ peer.cpp:726 │ Silent ignore │ No │
├──────────────────────────────┼───────────────────────────┼───────────────────┼──────────┤
│ ADDR before VERACK │ peer.cpp:726 │ Silent ignore │ No │
├──────────────────────────────┼───────────────────────────┼───────────────────┼──────────┤
│ INV before VERACK │ peer.cpp:726 │ Silent ignore │ No │
└──────────────────────────────┴───────────────────────────┴───────────────────┴──────────┘

Stub integration test

The integration test at test/integration-network/dos/pre_verack_message_tests.cpp:57-84 claims to test this behavior but is a stub — it uses CHECK(true) and INFO("Pre-VERACK HEADERS would
trigger instant disconnect") without actually injecting a pre-VERACK message. This gives false confidence that the behavior is tested.

Proposed Fix

Priority 1: Disconnect on pre-VERACK messages (one-line fix)

File: src/network/peer.cpp:726-728

• LOG_NET_DEBUG("received {} before handshake complete from peer={}, ignoring", command, id_);
• return;

• LOG_NET_ERROR("received {} before handshake complete from peer={}, disconnecting (protocol violation)", command, id_);
• post_disconnect();
• return;

Priority 2: Fix the stub integration test

File: test/integration-network/dos/pre_verack_message_tests.cpp:57-84

Either implement a real test that injects a pre-VERACK message and verifies disconnect, or remove the stub to avoid false confidence.

Impact

• Security: Misbehaving peers can occupy inbound connection slots (MAX_INBOUND_PER_NETGROUP = 4 per /16 subnet) indefinitely without completing the handshake. This could contribute to
  eclipse attacks by exhausting slots that legitimate peers need.
• Protocol compliance: Inconsistent with Bitcoin Core behavior, which disconnects peers that send protocol messages before handshake completion.
• False test coverage: The stub integration test passes without actually testing the behavior, masking the bug.