From ef728cd10045fee9a73a734259baf370cd080ed5 Mon Sep 17 00:00:00 2001 From: Steven Wroblewski Date: Thu, 1 Oct 2026 10:17:03 +0200 Subject: [PATCH] =?UTF-8?q?Speicherstand=20mit=20Pr=C3=BCfsumme=20und=20Pr?= =?UTF-8?q?=C3=BCfung=20beim=20Laden?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Snapshots tragen jetzt eine Prüfsumme (Format 2). Beim Wiederherstellen werden Zustände abgelehnt, die die Regeln nie erzeugen: zu große Takte, mehr als vier Spieler online, doppelte Schlüssel, unmögliche Wartezeiten. World::load ersetzt den öffentlichen Aufruf von set_all_offline. Die festgeschriebenen Werte sind bewusst angepasst und um einen Offline-Spieler und das Kartenformat ergänzt. Co-Authored-By: Claude Haiku 4.5 --- core/src/replay.rs | 5 +- core/src/snapshot.rs | 82 ++++++++++++++++++++--- core/src/world.rs | 8 ++- core/tests/golden.rs | 107 ++++++++++++++++++++++++++--- core/tests/snapshot.rs | 149 +++++++++++++++++++++++++++++++++++++++-- core/tests/world.rs | 8 --- 6 files changed, 324 insertions(+), 35 deletions(-) diff --git a/core/src/replay.rs b/core/src/replay.rs index 96f8f8a..c0243d2 100644 --- a/core/src/replay.rs +++ b/core/src/replay.rs @@ -9,8 +9,9 @@ use crate::snapshot::SnapshotError; use crate::world::World; const MAGIC: [u8; 4] = *b"GRWR"; -/// Version of the recording format. -pub const RECORDING_VERSION: u32 = 1; +/// Version of the recording format. Raise it whenever the encoded recording, +/// including the snapshot inside it, changes. +pub const RECORDING_VERSION: u32 = 2; /// Reason a recording could not be read or replayed. #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/core/src/snapshot.rs b/core/src/snapshot.rs index 42a4b6b..7fcef94 100644 --- a/core/src/snapshot.rs +++ b/core/src/snapshot.rs @@ -1,18 +1,26 @@ //! Complete, versioned encoding of a world. Saves, joining players and state //! checksums all use it. +//! +//! Layout: the marker `GRWS`, the format version, the encoded state, and an +//! FNV-1a checksum over everything before it. use std::fmt; use crate::codec::{DecodeError, Reader, Writer}; use crate::hash::fnv1a64; use crate::map::Map; -use crate::player::{Player, MAX_KNOWN_PLAYERS}; -use crate::world::World; +use crate::player::{Player, MAX_KNOWN_PLAYERS, MAX_ONLINE_PLAYERS}; +use crate::world::{World, DIG_COOLDOWN_TICKS}; const MAGIC: [u8; 4] = *b"GRWS"; /// Version of the snapshot format. Raise it whenever the encoded state /// changes. -pub const SNAPSHOT_VERSION: u32 = 1; +pub const SNAPSHOT_VERSION: u32 = 2; +/// Length of the checksum at the end of a snapshot. +const CHECKSUM_BYTES: usize = 8; +/// Highest tick a restored world may have. No real session gets near it, +/// and adding to it can never overflow. +pub const MAX_RESTORABLE_TICK: u64 = u64::MAX / 2; /// Reason a snapshot could not be restored. #[derive(Clone, Debug, PartialEq, Eq)] @@ -21,8 +29,13 @@ pub enum SnapshotError { NotASnapshot, /// The snapshot was written by an incompatible version of the game. UnsupportedVersion(u32), + /// The checksum does not match: the bytes changed after they were + /// written. + ChecksumMismatch, /// The snapshot is damaged. Corrupt(DecodeError), + /// The bytes decode, but describe a state the rules can never produce. + Invalid(&'static str), } impl fmt::Display for SnapshotError { @@ -32,7 +45,9 @@ impl fmt::Display for SnapshotError { SnapshotError::UnsupportedVersion(version) => { write!(f, "unsupported snapshot version {version}") } + SnapshotError::ChecksumMismatch => write!(f, "snapshot checksum does not match"), SnapshotError::Corrupt(error) => write!(f, "damaged snapshot: {error}"), + SnapshotError::Invalid(what) => write!(f, "impossible state in snapshot: {what}"), } } } @@ -65,16 +80,20 @@ impl World { } /// Encodes the complete world behind a header that names format and - /// version. + /// version, followed by a checksum. pub fn to_snapshot(&self) -> Vec { let mut w = Writer::new(); w.raw(&MAGIC); w.u32(SNAPSHOT_VERSION); self.encode_state(&mut w); - w.into_bytes() + let mut bytes = w.into_bytes(); + let checksum = fnv1a64(&bytes); + bytes.extend_from_slice(&checksum.to_le_bytes()); + bytes } - /// Restores a world from bytes written by [`World::to_snapshot`]. + /// Restores a world exactly as it was encoded, including which players + /// were online. Joining players receive the host's world this way. pub fn from_snapshot(bytes: &[u8]) -> Result { let mut r = Reader::new(bytes); let magic = r @@ -87,6 +106,17 @@ impl World { if version != SNAPSHOT_VERSION { return Err(SnapshotError::UnsupportedVersion(version)); } + if r.remaining() < CHECKSUM_BYTES { + return Err(DecodeError::UnexpectedEnd.into()); + } + let (body, trailer) = bytes.split_at(bytes.len() - CHECKSUM_BYTES); + let mut checksum = [0u8; CHECKSUM_BYTES]; + checksum.copy_from_slice(trailer); + if fnv1a64(body) != u64::from_le_bytes(checksum) { + return Err(SnapshotError::ChecksumMismatch); + } + + let mut r = Reader::new(&body[MAGIC.len() + 4..]); let seed = r.u64()?; let tick = r.u64()?; let map = Map::decode(&mut r)?; @@ -99,11 +129,47 @@ impl World { players.push(Player::decode(&mut r)?); } r.finish()?; - Ok(World { + + let world = World { seed, tick, map, players, - }) + }; + world.check_restorable()?; + Ok(world) + } + + /// Restores a saved world for a new session. Like + /// [`World::from_snapshot`], but every player starts offline, because + /// nobody has joined the new session yet. + pub fn load(bytes: &[u8]) -> Result { + let mut world = World::from_snapshot(bytes)?; + world.set_all_offline(); + Ok(world) + } + + /// Rejects states that the rules can never produce, so that a damaged or + /// crafted snapshot fails here instead of breaking a later tick. + fn check_restorable(&self) -> Result<(), SnapshotError> { + if self.tick > MAX_RESTORABLE_TICK { + return Err(SnapshotError::Invalid("tick")); + } + let online = self.players.iter().filter(|player| player.online).count(); + if online > MAX_ONLINE_PLAYERS { + return Err(SnapshotError::Invalid("online players")); + } + for (index, player) in self.players.iter().enumerate() { + if self.players[..index] + .iter() + .any(|other| other.key == player.key) + { + return Err(SnapshotError::Invalid("duplicate player key")); + } + if player.next_action_tick > self.tick + DIG_COOLDOWN_TICKS { + return Err(SnapshotError::Invalid("next action tick")); + } + } + Ok(()) } } diff --git a/core/src/world.rs b/core/src/world.rs index 2df6b2e..f89584c 100644 --- a/core/src/world.rs +++ b/core/src/world.rs @@ -95,9 +95,11 @@ impl World { .map(|index| PlayerId(index as u8)) } - /// Marks every player as offline. Used after loading a save from disk, - /// when nobody has joined yet. - pub fn set_all_offline(&mut self) { + /// Marks every player as offline. Only [`World::load`] uses it: a loaded + /// save starts a new session, which nobody has joined yet. Called during + /// a running session on one machine, it would split the worlds of the + /// players apart. + pub(crate) fn set_all_offline(&mut self) { for player in &mut self.players { player.online = false; } diff --git a/core/tests/golden.rs b/core/tests/golden.rs index 7c462a3..5f272ad 100644 --- a/core/tests/golden.rs +++ b/core/tests/golden.rs @@ -1,15 +1,16 @@ //! Pinned values for everything that must not change unnoticed. //! -//! Every other test compares two computations inside the same build, so a +//! Most other tests compare two computations inside the same build, so a //! change that affects both sides alike stays green. This file compares //! against values recorded once. If one of them fails, rules or encodings //! changed, on purpose or by accident. If saves or recordings are affected, //! raise the version constant named in the failure message, then update the //! value here deliberately. +use grovewright_core::codec::Writer; use grovewright_core::command::Command; use grovewright_core::hash::fnv1a64; -use grovewright_core::map::Pos; +use grovewright_core::map::{Map, Pos, Terrain, Tile}; use grovewright_core::mapgen::outcrop_origin; use grovewright_core::player::PlayerId; use grovewright_core::replay::Recording; @@ -99,13 +100,55 @@ fn frozen_script(world: &World) -> Vec> { ticks } -/// Covers the map generator, the map encoding (terrain numbers, byte order of -/// width and height) and the snapshot header. +/// The commands of a second session of 30 ticks that ends with one player +/// offline, so that the encoding of offline players is pinned as well. +/// +/// Never change or extend this script either. +fn frozen_offline_script(world: &World) -> Vec> { + let origin = outcrop_origin(world.map()); + let scripted = [ + ( + 0, + Command::Join { + key: [7; 16], + name: "Linus".to_owned(), + }, + ), + ( + 1, + Command::Join { + key: [8; 16], + name: "Margaret".to_owned(), + }, + ), + ( + 3, + Command::Dig { + player: PlayerId(1), + pos: origin, + }, + ), + ( + 10, + Command::Leave { + player: PlayerId(0), + }, + ), + ]; + let mut ticks = vec![Vec::new(); 30]; + for (tick, command) in scripted { + ticks[tick].push(command); + } + ticks +} + +/// Covers the map generator, the map encoding and the snapshot header and +/// checksum. #[test] fn the_starting_snapshot_is_unchanged() { let actual = fnv1a64(&World::new(SEED).to_snapshot()); assert_eq!( - actual, 0x1b78_d996_1586_fbf1, + actual, 0x6812_937b_f2b2_19ae, "the starting world or the snapshot format changed; if saves are \ affected, raise SNAPSHOT_VERSION in core/src/snapshot.rs; then \ update this value" @@ -129,6 +172,22 @@ fn the_frozen_session_ends_in_the_same_state() { ); } +/// Covers the encoding of a player who is offline. +#[test] +fn the_frozen_offline_session_ends_in_the_same_state() { + let mut world = World::new(SEED); + for commands in frozen_offline_script(&world) { + world.step(&commands); + } + assert_eq!( + world.state_hash(), + 0x904f_10e1_7c52_8be4, + "the rules or the snapshot format changed; old recordings no longer \ + verify; if saves are affected, raise SNAPSHOT_VERSION in \ + core/src/snapshot.rs; then update this value" + ); +} + /// Covers the recording header and layout and the wire format of commands. #[test] fn the_recording_format_is_unchanged() { @@ -140,8 +199,40 @@ fn the_recording_format_is_unchanged() { } assert_eq!( fnv1a64(&recording.to_bytes(&world)), - 0x2312_5a7d_d8ef_5ae0, - "the recording or command format changed; raise RECORDING_VERSION in \ - core/src/replay.rs; then update this value" + 0x1c24_8cc7_6bc8_846a, + "the recording or command format changed; if other tests in this \ + file fail as well, deal with those first, because a recording \ + contains a snapshot and an end state; otherwise raise \ + RECORDING_VERSION in core/src/replay.rs; then update this value" + ); +} + +/// Covers the byte layout of a map: width before height, little endian, +/// terrain numbers and damage. The map is not square, so a swap of width +/// and height shows. +#[test] +fn the_map_encoding_is_unchanged() { + let mut map = Map::filled(3, 2, Terrain::Ground); + map.set(Pos::new(1, 0), Tile::new(Terrain::Bedrock)); + map.set(Pos::new(0, 1), Tile::new(Terrain::HardRock)); + map.set( + Pos::new(2, 1), + Tile { + terrain: Terrain::SoftRock, + damage: 1, + }, + ); + let mut w = Writer::new(); + map.encode(&mut w); + assert_eq!( + w.into_bytes(), + vec![ + 3, 0, 0, 0, // width + 2, 0, 0, 0, // height + 0, 0, 3, 0, 0, 0, // row 0: ground, bedrock, ground + 2, 0, 0, 0, 1, 1, // row 1: hard rock, ground, soft rock hit once + ], + "the map encoding changed; raise SNAPSHOT_VERSION in \ + core/src/snapshot.rs; then update these bytes" ); } diff --git a/core/tests/snapshot.rs b/core/tests/snapshot.rs index 204075a..2403ebd 100644 --- a/core/tests/snapshot.rs +++ b/core/tests/snapshot.rs @@ -1,10 +1,11 @@ -use grovewright_core::codec::DecodeError; +use grovewright_core::codec::{DecodeError, Writer}; use grovewright_core::command::Command; -use grovewright_core::map::Pos; +use grovewright_core::hash::fnv1a64; +use grovewright_core::map::{Map, Pos, Terrain}; use grovewright_core::mapgen::outcrop_origin; -use grovewright_core::player::PlayerId; -use grovewright_core::snapshot::{SnapshotError, SNAPSHOT_VERSION}; -use grovewright_core::world::World; +use grovewright_core::player::{Player, PlayerId, MAX_ONLINE_PLAYERS}; +use grovewright_core::snapshot::{SnapshotError, MAX_RESTORABLE_TICK, SNAPSHOT_VERSION}; +use grovewright_core::world::{Event, World, DIG_COOLDOWN_TICKS}; const ADA: PlayerId = PlayerId(0); @@ -27,6 +28,29 @@ fn played_world() -> World { world } +/// Builds snapshot bytes by hand: header, state and checksum. This lets tests +/// describe states that the rules would never produce. +fn snapshot_of(seed: u64, tick: u64, map: &Map, players: &[Player]) -> Vec { + let mut w = Writer::new(); + w.raw(b"GRWS"); + w.u32(SNAPSHOT_VERSION); + w.u64(seed); + w.u64(tick); + map.encode(&mut w); + w.u8(players.len() as u8); + for player in players { + player.encode(&mut w); + } + let mut bytes = w.into_bytes(); + let checksum = fnv1a64(&bytes); + bytes.extend_from_slice(&checksum.to_le_bytes()); + bytes +} + +fn player(key: u8) -> Player { + Player::new([key; 16], "P") +} + #[test] fn a_snapshot_restores_an_identical_world() { let world = played_world(); @@ -35,6 +59,13 @@ fn a_snapshot_restores_an_identical_world() { assert_eq!(restored.state_hash(), world.state_hash()); } +#[test] +fn a_snapshot_is_header_then_state_then_checksum() { + let world = played_world(); + let by_hand = snapshot_of(world.seed(), world.tick(), world.map(), world.players()); + assert_eq!(world.to_snapshot(), by_hand); +} + #[test] fn the_state_hash_covers_every_part_of_the_state() { let base = played_world(); @@ -81,6 +112,35 @@ fn a_restored_world_continues_exactly_like_the_original() { assert_eq!(original, restored); } +#[test] +fn from_snapshot_keeps_online_players_online() { + let restored = World::from_snapshot(&played_world().to_snapshot()).unwrap(); + assert!(restored.player(ADA).unwrap().online); +} + +#[test] +fn loading_a_save_starts_with_every_player_offline() { + let world = played_world(); + let loaded = World::load(&world.to_snapshot()).unwrap(); + assert!(loaded.players().iter().all(|player| !player.online)); + assert_eq!(loaded.tick(), world.tick()); + assert_eq!(loaded.map(), world.map()); + assert_eq!( + loaded.player(ADA).unwrap().inventory, + world.player(ADA).unwrap().inventory + ); +} + +#[test] +fn a_returning_player_can_join_a_loaded_world() { + let mut loaded = World::load(&played_world().to_snapshot()).unwrap(); + let events = loaded.step(&[Command::Join { + key: [3; 16], + name: "Ada".to_owned(), + }]); + assert_eq!(events, vec![Event::PlayerJoined { player: ADA }]); +} + #[test] fn bytes_that_are_not_a_snapshot_are_rejected() { assert_eq!(World::from_snapshot(b""), Err(SnapshotError::NotASnapshot)); @@ -104,18 +164,95 @@ fn snapshots_of_another_version_are_rejected() { ); } +#[test] +fn any_changed_byte_is_detected() { + let bytes = played_world().to_snapshot(); + for index in [8, bytes.len() / 2, bytes.len() - 9, bytes.len() - 1] { + let mut changed = bytes.clone(); + changed[index] ^= 0x01; + assert_eq!( + World::from_snapshot(&changed), + Err(SnapshotError::ChecksumMismatch), + "byte {index}" + ); + } +} + #[test] fn cut_off_and_padded_snapshots_are_rejected() { let bytes = played_world().to_snapshot(); assert_eq!( World::from_snapshot(&bytes[..bytes.len() - 1]), - Err(SnapshotError::Corrupt(DecodeError::UnexpectedEnd)) + Err(SnapshotError::ChecksumMismatch) ); let mut padded = bytes.clone(); padded.push(0); assert_eq!( World::from_snapshot(&padded), + Err(SnapshotError::ChecksumMismatch) + ); + + assert_eq!( + World::from_snapshot(&bytes[..10]), + Err(SnapshotError::Corrupt(DecodeError::UnexpectedEnd)) + ); +} + +#[test] +fn damaged_content_behind_a_valid_checksum_is_rejected() { + let world = played_world(); + let mut bytes = snapshot_of(world.seed(), world.tick(), world.map(), world.players()); + bytes.truncate(bytes.len() - 8); + bytes.push(0); + let checksum = fnv1a64(&bytes); + bytes.extend_from_slice(&checksum.to_le_bytes()); + assert_eq!( + World::from_snapshot(&bytes), Err(SnapshotError::Corrupt(DecodeError::TrailingBytes)) ); } + +#[test] +fn states_the_rules_never_produce_are_rejected() { + let map = Map::filled(4, 3, Terrain::Ground); + let too_many_online: Vec = (0..=MAX_ONLINE_PLAYERS as u8).map(player).collect(); + let mut offline_twin = player(1); + offline_twin.online = false; + let mut waiting_too_long = player(1); + waiting_too_long.next_action_tick = 5 + DIG_COOLDOWN_TICKS + 1; + + let cases = [ + (snapshot_of(1, MAX_RESTORABLE_TICK + 1, &map, &[]), "tick"), + (snapshot_of(1, 5, &map, &too_many_online), "online players"), + ( + snapshot_of(1, 5, &map, &[player(1), offline_twin]), + "duplicate player key", + ), + ( + snapshot_of(1, 5, &map, &[waiting_too_long]), + "next action tick", + ), + ]; + for (bytes, what) in cases { + assert_eq!( + World::from_snapshot(&bytes), + Err(SnapshotError::Invalid(what)), + "{what}" + ); + assert_eq!( + World::load(&bytes), + Err(SnapshotError::Invalid(what)), + "{what}" + ); + } +} + +#[test] +fn states_right_at_the_limits_are_accepted() { + let map = Map::filled(4, 3, Terrain::Ground); + let mut players: Vec = (0..MAX_ONLINE_PLAYERS as u8).map(player).collect(); + players[0].next_action_tick = MAX_RESTORABLE_TICK + DIG_COOLDOWN_TICKS; + let bytes = snapshot_of(1, MAX_RESTORABLE_TICK, &map, &players); + assert!(World::from_snapshot(&bytes).is_ok()); +} diff --git a/core/tests/world.rs b/core/tests/world.rs index b839f5d..46c9dd4 100644 --- a/core/tests/world.rs +++ b/core/tests/world.rs @@ -176,14 +176,6 @@ fn the_world_remembers_a_limited_number_of_players() { assert_eq!(world.players().len(), MAX_KNOWN_PLAYERS); } -#[test] -fn set_all_offline_marks_every_player_offline() { - let mut world = World::new(1); - world.step(&[join(1), join(2)]); - world.set_all_offline(); - assert!(world.players().iter().all(|player| !player.online)); -} - #[test] fn three_hits_break_soft_rock_and_give_one_stone() { let (mut world, id, rock) = world_with_player();