p2p: apply Core's message size and count limits, and bound queued sends per peer

[bkeroack]

Problem

Several P2P paths took no notice of the limits Bitcoin Core applies to a single message, or to what is queued for one peer. Core references are to src/ at current master.

Queued sends and getdata. A peer's outbound queue held 256 messages and counted no bytes. handle_getdata read, deserialized and queued every block a request named, with a try_send whose failure it ignored: whatever did not fit was read, dropped, and still charged to -maxuploadtarget, and what did fit stayed in memory for as long as the peer did not read it. The write loop awaited each writer.send with nothing else in the select!, so while a write was parked, neither the disconnect signal (disconnectnode, a ban) nor the ping timeout reached the peer.

Core stops serving a peer once the bytes queued to it pass -maxsendbuffer (DEFAULT_MAXSENDBUFFER, 1,000,000 bytes, net.h:99; fPauseSend, net.cpp:1670). ProcessGetData breaks on it (net_processing.cpp:2534, 2556) and keeps the rest in m_getdata_requests (:4172); ProcessMessages takes no further message from the peer while that backlog is non-empty or the buffer is full (:5153, :5157). A peer whose socket has taken no bytes for TIMEOUT_INTERVAL (20 minutes, net.h:59) is dropped (net.cpp:2036-2041, "socket sending timeout"), and m_last_send moves on every partial send (net.cpp:1644).

Message limits. rust-bitcoin decodes vectors of any length the frame can hold. satd checked no counts, and accepted v1 frames up to 32 MiB (net/connection.rs), allocating the announced payload before reading any of it and keeping the receive buffer at its largest size for the life of the connection. Core:

• MAX_PROTOCOL_MESSAGE_LENGTH = 4,000,000; a header announcing more ends the connection (net.h:65, net.cpp:760-765). The receive buffer grows at most 256 KiB ahead of the bytes received (net.cpp:781-782; v2 MAX_RESERVE_AHEAD, net.cpp:1327-1357).
• inv / getdata over MAX_INV_SZ (50,000) are Misbehaving (net_processing.cpp:126, 4040, 4131).
• headers over MAX_HEADERS_RESULTS (2000) is Misbehaving, checked before the headers are read (net_processing.h:50, net_processing.cpp:4740).
• addr / addrv2 over MAX_ADDR_TO_SEND (1000) is Misbehaving, after SetupAddressRelay (net_processing.cpp:190, 5635).
• A notfound over MAX_PEER_TX_ANNOUNCEMENTS + MAX_BLOCKS_IN_TRANSIT_PER_PEER (5016) entries is ignored (net_processing.cpp:5066).

Peer text. The user agent was not bounded and was stored, logged at INFO and reported in getpeerinfo.subver as sent, line breaks and escape sequences included; unknown message types were logged as sent. Core reads the user agent as LIMITED_STRING(strSubVer, MAX_SUBVERSION_LENGTH) (net_processing.cpp:3636), which throws for a longer one (serialize.h:617-628): the exception is caught (net_processing.cpp:5195), the version is dropped, and the peer stays without a handshake. It keeps the user agent only as SanitizeString(strSubVer) (:3637; util/strencodings.cpp:23-40), and sanitizes message types in its log lines.

BIP 157. satd silently ignored getcfilters / getcfheaders / getcfcheckpt requests with an unsupported filter type (or any request while it serves no filters), an unknown stop hash, a start above the stop, or too long a range. Core's PrepareBlockFilterRequest disconnects the peer for each (net_processing.cpp:3262-3310; fDisconnect, no misbehaviour).

Fix

• net/send_queue.rs (new): PeerSender replaces the peer's mpsc::Sender and counts the v1 wire size of each queued message; the write loop takes it off after writing. SendQueue holds the count, the unserved getdata entries (Core's m_getdata_requests), and whether the write loop may read the peer's next message.
• getdata is served as ProcessGetData does: entries go to the backlog and are served, oldest first, until the queue passes 1,000,000 bytes or has no free slot. The write loop asks for more (NetEvent::GetDataResume) once the queue is back under the limit. From the moment it hands a getdata over until the backlog is empty, and while the queue is over the limit, the write loop leaves the peer's next message unread, as ProcessMessages does; that keeps answers in order and the backlog to one request. Handing over a getdata and asking for more both wake the manager's drain, so a peer fetching blocks is served at once rather than once per 500 ms maintenance tick. -maxuploadtarget is charged only for a block that was queued.
• Every write in the write loop (send_watched) also watches the disconnect signal, and gives up when the socket has taken no bytes for 20 minutes. v1 and v2 writes count sent bytes as the socket takes them (write_counted), which is what that watch reads.
• v1 frames are capped at 4,000,000 bytes. v1 and v2 receive buffers grow at most 256 KiB ahead of the bytes received, and a v1 buffer grown past 64 KiB is released after its message. The decoded payload is moved out rather than cloned.
• The count limits above, each with Core's reaction. Misbehaving maps to satd's existing add_ban_score(id, 100, …), the same mapping used for the other Misbehaving cases in manager.rs.
• A version with a user agent over 256 bytes is ignored and the handshake keeps waiting, so the existing -peertimeout handshake deadline ends the connection, as in Core; a well-formed version after it is accepted. net/limits.rs (new) has Core's SanitizeString, applied to the stored user agent (and so to the "Peer connected" line and getpeerinfo.subver) and to message types in log lines.
• prepare_block_filter_request implements PrepareBlockFilterRequest for all three BIP 157 messages, and disconnects where it does. A build without block-filter-index disconnects any such request, since it serves no filter type.

Tests

Unit tests (node):

• net::connection::bounds_tests: a_frame_over_four_million_bytes_is_refused, the_payload_buffer_grows_with_the_bytes_received, the_receive_buffer_is_released_after_a_large_message, a_send_counts_its_bytes_as_the_socket_takes_them.
• net::v2transport::bounds_tests: a_packet_buffer_grows_with_the_bytes_received, a_send_counts_its_bytes_as_the_socket_takes_them.
• net::manager::p2pbounds_tests: inv_and_getdata_past_max_inv_sz_are_misbehaviour, headers_past_max_headers_results_are_misbehaviour, addr_and_addrv2_past_max_addr_to_send_are_misbehaviour and notfound_past_the_limit_is_ignored (each with the at-the-limit case as the must-succeed half); getdata_stops_serving_past_the_send_buffer, getdata_resumes_as_the_queue_drains; peer_user_agent_is_sanitized, version_with_an_oversized_user_agent_is_ignored; write loop: a_parked_write_ends_on_disconnect, a_write_that_makes_no_progress_times_out (paused clock), reading_waits_for_a_getdata_to_be_served, the_loop_asks_for_more_once_the_queue_drains; BIP 157: bip157::{a_valid_request_is_served, bad_requests_disconnect, a_request_to_a_node_not_serving_filters_disconnects, a_range_past_the_limit_disconnects, an_off_chain_stop_hash}.
• net::send_queue::tests and net::limits::tests for the new modules.

Integration (satd/tests/p2p_limits.rs, new target, against a live regtest node): a_frame_over_four_million_bytes_ends_the_connection, an_inv_over_max_inv_sz_ends_the_connection, a_large_getdata_is_served_in_full_as_the_peer_reads (one 50,000-entry getdata, about 12 MB of blocks, read 3 s late: every block arrives, in order).

The existing BIP 157 regtests that asserted the silent drop now assert the disconnect, edited in place and renamed: test_p2p_disconnect_when_peer_serve_disabled, test_p2p_disconnect_invalid_filter_type, test_p2p_disconnect_oversized_range, test_p2p_disconnect_unknown_stop_hash, and the 2001-header case in test_p2p_getcfheaders_accepts_2000_headers.

Fails without the fix: yes. The new unit tests were run on the unfixed tree first (the test files only, plus the two receive-buffer constants they name): 15 failed, each on its own assertion, for example

• inv_and_getdata_past_max_inv_sz_are_misbehaviour: "inv of 50001 entries disconnects"
• headers_past_max_headers_results_are_misbehaviour: "2001 headers disconnect"
• addr_and_addrv2_past_max_addr_to_send_are_misbehaviour: "case 0: 1001 addresses disconnect"
• notfound_past_the_limit_is_ignored: "notfound of 5017 entries: 0 heights still in flight"
• getdata_stops_serving_past_the_send_buffer: "30 blocks, 3007814 bytes queued at once; the bound is 1000000 plus one block"
• a_frame_over_four_million_bytes_is_refused: the 4,000,001-byte frame was accepted
• the_payload_buffer_grows_with_the_bytes_received: "asked for 4000000 bytes at once"
• the_receive_buffer_is_released_after_a_large_message: "kept 1000024 bytes after a 1 MB message"
• a_send_counts_its_bytes_as_the_socket_takes_them (v1): "3900024 sent, 3900024 in whole messages"
• peer_user_agent_is_sanitized, version_with_an_oversized_user_agent_is_ignored ("an oversized user agent must not be answered, got … Version"), and the BIP 157 bad_requests_disconnect, a_request_to_a_node_not_serving_filters_disconnects, an_off_chain_stop_hash ("a header satd never validated is not a block it would serve").

The guards were then removed one at a time in a single perturbed build (read gate, resume request, the send-buffer check in serve_getdata, the disconnect branch and the timeout in send_watched), and the named tests failed: reading_waits_for_a_getdata_to_be_served, the_loop_asks_for_more_once_the_queue_drains, getdata_stops_serving_past_the_send_buffer, getdata_resumes_as_the_queue_drains, a_parked_write_ends_on_disconnect ("the write loop ends while its write is parked: Elapsed"), a_write_that_makes_no_progress_times_out ("a write with no progress ends: Elapsed"). The integration tests were not run against the unfixed tree.

Notes

• Misbehaving is mapped to add_ban_score(id, 100, …), as it already is elsewhere in manager.rs, so it currently bans the address. Core's ladder (discourage, and no punishment for manual, local and inbound-onion peers) is a separate change.
• -maxsendbuffer stays an unsupported Core option (warned and skipped, as before); Core's default of 1,000,000 bytes applies.
• BIP 157, a stop hash off the active chain: Core serves one its BlockRequestAllowed admits (a validated block within 30 days of the best header; net_processing.cpp:1951). satd keeps filters for the active chain only, so it cannot serve that request; it ignores it without a disconnect, as before. Anything else off the active chain, such as a header-only or never-validated block, now disconnects. The work-equivalent-age half of BlockRequestAllowed is not applied, so satd ignores slightly more such requests than Core would refuse.
• A version whose user agent is not valid UTF-8 still fails rust-bitcoin's decode and is skipped, where Core would accept and sanitize it. Unchanged here.
• satd answers a peer's ping directly on the peer's task, so a pong can still go out ahead of messages already queued to the peer. Unchanged here.
• contrib/core-functional/inventory.toml's note for p2p_addrv2_relay.py ("L83 an oversized addrv2 is not disconnected") describes the old behaviour. The row is skip and was not re-measured here.

Release notes

satd now applies Bitcoin Core's limits to single P2P messages: a frame over 4,000,000 bytes ends the connection, and an inv or getdata over 50,000 entries, headers over 2000 or addr/addrv2 over 1000 is treated as misbehaviour. Blocks a peer asks for with getdata are served as it reads them, with about 1 MB queued per peer as in Core's default -maxsendbuffer, and a peer whose connection has taken no data for 20 minutes is disconnected; disconnectnode and bans now take effect on a peer that has stopped reading. A peer's user agent is limited to 256 bytes and sanitized before it is logged or reported in getpeerinfo. BIP 157 filter requests that Core refuses (unknown filter type, unknown stop hash, bad range, or any request while filters are not served) now disconnect the peer instead of being ignored.

🤖 Generated with Claude Code