Comfy-Org / Comfy-Org/comfy-multi-player

Malformed stamp counters break deterministic LWW ordering

Open
#149 1 comment 0 reactions 1 assignee Claimed by @skishore23 View on GitHub
bug
Dominant language
TypeScript
Stars
5
Forks
1
Avg merge
12h 22m
Merged PRs (30d)
121

Description

At reviewed SHA `811d6a267b34f1eba2ed0969dd24ccdda565371b`, malformed Lamport counters are accepted and make same-op-set projection depend on arrival order.

- `stamp[0]` is coerced with `Number(...)`, so a string such as `"not-a-number"` becomes `NaN`.
- the comparator then reports both `compare(NaN, 2)` and `compare(2, NaN)` as newer (`1`), breaking antisymmetry.
- an independent two-order reproduction ended with different widget values while every op reported `applied`.

Evidence and reproduction

Reviewed SHA: `811d6a267b34f1eba2ed0969dd24ccdda565371b`.

Exact source locations:

- envelope validation stops after kind and `op_id`; it does not validate `stamp`, `base_version`, or `actor`: [`src/applier.ts:385-401`](https://github.com/Comfy-Org/comfy-multi-player/blob/811d6a267b34f1eba2ed0969dd24ccdda565371b/src/applier.ts#L385-L401)
- `stampKey` accepts any array of length at least two and coerces the first member: [`src/stamps.ts:54-59`](https://github.com/Comfy-Org/comfy-multi-player/blob/811d6a267b34f1eba2ed0969dd24ccdda565371b/src/stamps.ts#L54-L59)
- `compareStampKeys` assumes the numeric member is orderable: [`src/stamps.ts:42-46`](https://github.com/Comfy-Org/comfy-multi-player/blob/811d6a267b34f1eba2ed0969dd24ccdda565371b/src/stamps.ts#L42-L46)
- `set_widget` uses this result as its LWW gate: [`src/applier.ts:895-903`](https://github.com/Comfy-Org/comfy-multi-player/blob/811d6a267b34f1eba2ed0969dd24ccdda565371b/src/applier.ts#L895-L903)

Independent verification seeded two documents from one Yjs snapshot and applied the same malformed and valid `set_widget` ops in opposite orders:

- malformed then valid: final value `V`, stored stamp counter `2`
- valid then malformed: final value `M`, stored malformed counter serialized as `null`

Both operations returned `applied` in both replicas. This violates deterministic convergence and the total-order premise of the LWW gate. Open issue #15 concerns stamp authority but does not cover malformed numeric coercion, `NaN`, comparator antisymmetry, or this arrival-order divergence.

Suggested root fix: validate the stamp tuple at the wire envelope before digesting or entering a transaction. Require a finite valid Lamport counter plus the contractually valid actor and reject malformed input without mutation. Add a two-arrival-order regression that asserts identical projection and rejection.

Draft-head check: open drafts #123 (`38e6ec6fb02a68d676855dd64d271bea96edc3cd`) and #138 (`ca2a3ec25bcc0d6243a8346d1f8ed53f606cd243`) do not address this path.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.