From 54086b56042831c26ffdc5e73dfb9a557483ca87 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 7 Sep 2026 17:27:07 +0200 Subject: [PATCH] Stop the rate limiter from being its own amplifier The control-message pacing added two commits ago logged one WARN line per refused packet, which is a thousand formatted strings and a thousand stdout writes per second for a client sending a thousand -- the exact cost the limiter was added to bound, moved from the disk write to the log. One line per burst now, re-armed when the peer behaves. Co-Authored-By: Claude Opus 5 --- src/net/server_runtime.gd | 13 ++++++++- tests/unit/test_server_authority.gd | 45 +++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/net/server_runtime.gd b/src/net/server_runtime.gd index e0786be..bbe0edd 100644 --- a/src/net/server_runtime.gd +++ b/src/net/server_runtime.gd @@ -38,6 +38,11 @@ var _snapshot_phase: int = 0 var _tick: int = 0 ## Tick each peer was last allowed a control message on. var _peer_control_tick: Dictionary[int, int] = {} +## Peers already told off for going too fast. Without this the refusal logs one +## line per packet, which turns the limiter into the amplification it exists to +## prevent -- a client sending a thousand a second would get a thousand lines +## formatted and written. One line per burst, cleared when the peer behaves. +var _peer_control_warned: Dictionary[int, bool] = {} func _ready() -> void: @@ -205,6 +210,7 @@ func _forget_peer(peer_id: int) -> void: peer_accounts.erase(peer_id) peer_characters.erase(peer_id) _peer_control_tick.erase(peer_id) + _peer_control_warned.erase(peer_id) _broadcast_roster() @@ -427,9 +433,14 @@ func instance_of(peer_id: int) -> Instance: func _accept_control(peer_id: int) -> bool: var last: int = _peer_control_tick.get(peer_id, -CONTROL_COOLDOWN_TICKS) if _tick - last < CONTROL_COOLDOWN_TICKS: - GameLog.warn("server", "peer %d control message dropped, too fast" % peer_id) + if not _peer_control_warned.get(peer_id, false): + _peer_control_warned[peer_id] = true + GameLog.warn("server", + "peer %d is sending control messages faster than %d ticks apart" + % [peer_id, CONTROL_COOLDOWN_TICKS]) return false _peer_control_tick[peer_id] = _tick + _peer_control_warned.erase(peer_id) return true diff --git a/tests/unit/test_server_authority.gd b/tests/unit/test_server_authority.gd index 92d4fa5..5f7a382 100644 --- a/tests/unit/test_server_authority.gd +++ b/tests/unit/test_server_authority.gd @@ -306,3 +306,48 @@ func test_clamping_the_muzzle_does_not_swallow_the_shot() -> void: # a blocked muzzle falls back to the shooter's centre, it does not cancel. assert_eq(int(_fire_flush_against_the_wall(6)["fired"]), 7, "six Doubleshots is seven projectiles, wall or no wall") + + +# --- Control-message pacing ------------------------------------------------- +# Selecting or creating a character rewrites the whole store to disk, re-places +# the player and broadcasts a roster to everyone, all off one small reliable +# packet. The pacing below is what stops a client turning that into a synchronous +# disk write it controls the rate of. + +func _runtime() -> ServerRuntime: + # Not parented: _ready() would open a lobby and load a store, and neither is + # involved in deciding whether a message arrived too soon. + return autofree(ServerRuntime.new()) as ServerRuntime + + +func test_control_messages_are_paced_per_peer() -> void: + var srv := _runtime() + assert_true(srv._accept_control(PEER), "the first one is always allowed") + assert_false(srv._accept_control(PEER), "the second on the same tick is not") + srv._tick += ServerRuntime.CONTROL_COOLDOWN_TICKS - 1 + assert_false(srv._accept_control(PEER), "one tick short is still too soon") + srv._tick += 1 + assert_true(srv._accept_control(PEER), "and the cooldown does expire") + + +func test_one_peer_flooding_does_not_gag_another() -> void: + var srv := _runtime() + assert_true(srv._accept_control(1)) + assert_false(srv._accept_control(1)) + assert_true(srv._accept_control(2), "pacing is per peer, not global") + + +func test_a_refused_flood_is_logged_once_and_not_once_per_packet() -> void: + # The limiter must not become the amplification it exists to prevent: a + # client sending a thousand a second would otherwise have the server format + # and write a thousand lines. + var srv := _runtime() + assert_true(srv._accept_control(PEER)) + for _i in 500: + assert_false(srv._accept_control(PEER)) + assert_true(srv._peer_control_warned.get(PEER, false), + "the first refusal of a burst is worth saying") + srv._tick += ServerRuntime.CONTROL_COOLDOWN_TICKS + assert_true(srv._accept_control(PEER)) + assert_false(srv._peer_control_warned.has(PEER), + "behaving again re-arms the warning for the next burst")