Skip to content

engine: deserialized states bypass the zone-change occurrence allocator — turn_zone_change_index defaults to 0 on restore #7066

Description

@matthewevans

Surfaced while proving the occurrence-exactness invariant of
filter_already_collected_trigger_events_from (crates/engine/src/game/triggers.rs).
Pre-existing — this predates #7058 and is not caused by it. Escalated rather than
fixed: a restore-time index reconciliation is a materially larger change with its own
migration story. Anchors are at 84a4d01e63fd712936e58c9ab4e6834dc66e9e19.

The gap

ZoneChangeRecord::turn_zone_change_index is the engine's occurrence key for zone
changes. In live play restrictions::record_zone_change (restrictions.rs:606-621)
is its sole allocator: it reads state.zone_changes_this_turn.len(), stamps the
record, and pushes, so distinct occurrences within a turn always carry distinct
indices.

Deserialized states bypass that allocator entirely. GameEvent derives
Deserialize (types/events.rs:717), and
PersistedGameState::into_game_state (types/game_state.rs:9024, called from
crates/engine-wasm/src/lib.rs:46-47) reconstructs ZoneChanged values directly
into live buffers
:

Buffer Anchor (types/game_state.rs)
deferred_entry_events :13318
pending_trigger_event_batch :13508
current_trigger_events :14998
stack_trigger_event_batches :15011
current_trigger_event :14858
PendingZoneChangeDelivery.delivery_events :3309
deferred_events :3797, :4617
the ledger itself (zone_changes_this_turn) :14377

Every one is #[serde(default)], so an absent turn_zone_change_index
silently deserializes to 0. Indices are copied from the payload and never
re-derived against zone_changes_this_turn.len().

Neither restore arm re-checks the index:

9024:    pub fn into_game_state(self) -> GameState {
9025:        let mut state = match self {
9026:            Self::Raw(state) => {
9027:                let mut state = *state;
9028:                crate::game::precast_copy_shortcut::normalize_untrusted_restore(&mut state);
9029:                state
9030:            }
9031:            Self::Trusted(envelope) => (*envelope).into_game_state(),
9032:        };
9035:        state.migrate_transient_loop_sequence();

The Raw arm (:9026-9030) runs only normalize_untrusted_restore;
migrate_transient_loop_sequence (:9035) sits outside the match and so applies
to both arms. Neither reconciles the index — so this applies to the Trusted arm
too.

Consequence

A wire-loaded state can hold two ZoneChanged values from distinct occurrences
that both carry index 0. Once that happens they are byte-identical, and the queued
witness in filter_already_collected_trigger_events_from (triggers.rs:8348-8378)
consumes by full-equality position() — so one occurrence can be suppressed without
ownership, which is exactly what CR 603.2c sentence 2 forbids ("it can trigger
repeatedly if one event contains multiple occurrences").

The same stale index also feeds batched_zone_change_already_collected
(triggers.rs:1682-1706, read at :1694) and its writer
record_batched_zone_change_collected (:1708-1723, :1720), plus the
ledger-subscript consumers in types/ability.rs and the ledger-population consumers
QuantityRef::ZoneChangeCountThisTurn / ZoneChangeAggregateThisTurn
(game/quantity.rs:4125-4165).

Related (not a duplicate)

Same class as the known TriggerIndex zone-desync / deserialization lead: after a
wire load, state.battlefield and objects[*].zone are likewise never reconciled;
containment for that one shipped in trigger_index::candidates_for_event. That
defect is recorded only as a local project note — searches over --state all for
TriggerIndex, zone desync, desync, trigger index and deserialization trigger were run three times independently and found no tracked issue for it — so
this is filed as its own issue rather than as a comment there.

Scope ruling applied in the meantime

The occurrence-exactness invariant is documented as holding for live play
states produced by the engine's own execution, where record_zone_change is the sole
allocator. Deserialized states are a separate trust boundary, and the doc contract on
filter_already_collected_trigger_events_from says so explicitly rather than
implying the proof covers restored states.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions