mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-11 08:23:45 +00:00
fix(multiplayer): server no longer rate-limits a backlog it caused itself
Closes task 5.10's three recording gaps, and the gap-closing found a real input-loss bug. Replay log: a failed write now ends the log permanently instead of desyncing every later record's framing; close() is called from _exit_tree with a summary, since the RefCounted destructor closes it implicitly but never says whether the log is complete; rejected packets are recorded with their reason in the kind byte (framing unchanged, FORMAT_VERSION 2 so "no rejects" differs from "this build never recorded them"). Recording is capped at 8 per peer per window - uncapped, the diagnostic is a remote disk-fill amplifier, since the attacker picks the packet rate. Uncapped totals live on MatchSim and survive the peer's disconnect. The bug: a 2s host stall has the client sending at 60Hz throughout, and ENet delivers that whole backlog in the first window after resume - 70 of an honest client's packets rejected as "rate limit exceeded". Redundancy does not cover it, because the dropped packets are contiguous: 0 of 70 rescued, and 82 of 923 sequences (8.88%, ~1.4s of input) never reached the server, against 0.00% with no stall. Every prediction gate passed. Fixed by granting each already-tracked peer a capped, two-window packet grace when the server detects its own wall-clock stall. Rate-limit rejects 70 -> 0, sequences missing 8.88% -> 0.00%, seq-guard rejects 9 -> 0. Controls on the unfixed build lost 4.34/7.52/7.86%. All three abuse roles still disconnect and no flood induced a stall, so the grace cannot be farmed. Also corrects an earlier wrong conclusion: the reviewer's free-flight p95 0.688 is real and reproduces on two processes with 0.0% snapshot loss. The plain --role=client drive fails the 0.5 free-flight bound in 3 of 8 runs because that drive is mostly a contact test - the harness comment already said so - leaving a cohort as small as 12 samples. Near-surface error is genuinely several times open-air error, so the calibrated bound now belongs to --exercise-free-flight alone and the plain role asserts the always-well-sampled all-cohort percentiles at 1.2/2.0, printing the free-flight numbers as reported-not-asserted. 6/6 plain runs pass where 3/7 failed; tightening to 0.3 still fails. tools/replay_dump.gd reads a log back: counts by kind, plus how much of the input sequence stream reached the server once redundancy is counted.
This commit is contained in:
@@ -108,6 +108,76 @@ func test_a_truncated_log_yields_its_intact_records() -> void:
|
||||
DirAccess.remove_absolute(ProjectSettings.globalize_path(path))
|
||||
|
||||
|
||||
func test_rejected_packets_are_recorded_and_distinguishable_by_reason() -> void:
|
||||
# The log exists to answer "my input did nothing", and the packets that
|
||||
# explain that are exactly the ones the server threw away. Recording them
|
||||
# is only useful if the reason survives too, so a reader can tell a
|
||||
# malformed packet from one the rate limiter dropped.
|
||||
var path := _temp_path("rejects")
|
||||
var log_writer = ReplayLogScript.new()
|
||||
log_writer.open_for_write(path)
|
||||
var malformed := PackedByteArray([0x01, 0x02])
|
||||
var flooded := PackedByteArray([0x09, 0x08, 0x07])
|
||||
var far_future := PackedByteArray([0x11, 0x22, 0x33, 0x44])
|
||||
log_writer.record_rejected_input(ReplayLogScript.RecordKind.REJECTED_MALFORMED, 10, 5, malformed)
|
||||
log_writer.record_rejected_input(ReplayLogScript.RecordKind.REJECTED_RATE_LIMIT, 11, 5, flooded)
|
||||
log_writer.record_rejected_input(ReplayLogScript.RecordKind.REJECTED_SEQ_GUARD, 12, 6, far_future)
|
||||
log_writer.record_input(13, 5, PackedByteArray([0xAA]))
|
||||
log_writer.close()
|
||||
|
||||
var records: Array = ReplayLogScript.read_all(path).get("records", [])
|
||||
assert_eq(records.size(), 4, "rejects and accepted input share one ordered stream")
|
||||
assert_eq(records[0]["kind"], ReplayLogScript.RecordKind.REJECTED_MALFORMED, "malformed reason survives")
|
||||
assert_eq(records[0]["payload"], malformed, "and so do the bytes that caused it")
|
||||
assert_eq(records[1]["kind"], ReplayLogScript.RecordKind.REJECTED_RATE_LIMIT, "rate-limit reason survives")
|
||||
assert_eq(records[1]["payload"], flooded, "with its own payload")
|
||||
assert_eq(records[2]["kind"], ReplayLogScript.RecordKind.REJECTED_SEQ_GUARD, "seq-guard reason survives")
|
||||
assert_eq(records[2]["peer_id"], 6, "attributed to the peer that sent it")
|
||||
assert_eq(records[3]["kind"], ReplayLogScript.RecordKind.INPUT, "an accepted packet is still its own kind")
|
||||
DirAccess.remove_absolute(ProjectSettings.globalize_path(path))
|
||||
|
||||
|
||||
func test_a_failed_write_stops_the_log_instead_of_corrupting_it() -> void:
|
||||
# A half-written record desyncs the framing of everything after it, turning
|
||||
# "the disk filled up" into "the file is garbage". Forcing a real ENOSPC is
|
||||
# out of scope for a unit test, so the failure is injected directly — this
|
||||
# covers the guard and the accounting, not the detection, which is a
|
||||
# get_error() check on the real FileAccess.
|
||||
var path := _temp_path("writefail")
|
||||
var log_writer = ReplayLogScript.new()
|
||||
log_writer.open_for_write(path)
|
||||
log_writer.record_input(1, 1, PackedByteArray([1, 2, 3, 4]))
|
||||
log_writer.write_failed = true
|
||||
log_writer.record_input(2, 1, PackedByteArray([5, 6, 7, 8]))
|
||||
log_writer.record_snapshot(3, PackedByteArray([9]))
|
||||
assert_eq(log_writer.records_written, 1, "nothing is written after a failure")
|
||||
log_writer.close()
|
||||
|
||||
var records: Array = ReplayLogScript.read_all(path).get("records", [])
|
||||
assert_eq(records.size(), 1, "what was written before the failure is still readable")
|
||||
assert_eq(records[0]["payload"], PackedByteArray([1, 2, 3, 4]), "and intact")
|
||||
DirAccess.remove_absolute(ProjectSettings.globalize_path(path))
|
||||
|
||||
|
||||
func test_an_oversized_payload_is_dropped_without_breaking_later_records() -> void:
|
||||
# `length` is a u16. Truncating an over-64KB payload to fit would leave the
|
||||
# reader parsing the payload's own tail as the next record header.
|
||||
var path := _temp_path("oversized")
|
||||
var log_writer = ReplayLogScript.new()
|
||||
log_writer.open_for_write(path)
|
||||
var oversized := PackedByteArray()
|
||||
oversized.resize(0x10000)
|
||||
log_writer.record_input(1, 1, oversized)
|
||||
assert_eq(log_writer.records_dropped, 1, "the oversized record is counted as dropped")
|
||||
log_writer.record_input(2, 1, PackedByteArray([7, 7]))
|
||||
log_writer.close()
|
||||
|
||||
var records: Array = ReplayLogScript.read_all(path).get("records", [])
|
||||
assert_eq(records.size(), 1, "only the well-sized record is present")
|
||||
assert_eq(records[0]["tick"], 2, "and it is the one that came after the drop, correctly framed")
|
||||
DirAccess.remove_absolute(ProjectSettings.globalize_path(path))
|
||||
|
||||
|
||||
func test_writing_to_an_unopened_log_is_a_no_op() -> void:
|
||||
# --replay-log is optional, so every record_* call happens behind a null
|
||||
# check in production — but the class must not corrupt or crash if that
|
||||
|
||||
Reference in New Issue
Block a user