Repository navigation
Conversation
A change of peer p at counter c can depend on p only through (c-1)@p. A crafted dep on an older op of p made the import panic with "lamport is not continuous" and poisoned the document. See context/import-peer-id-reuse.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Frando
marked this pull request as ready for review
October 28, 2026 08:13
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A local commit depends on the doc's frontiers. The frontiers hold at most the latest op of the change's own peer. So a change
c@p(countercof peerp) can depend onponly through(c-1)@p. An imported update can claim any deps, though.If a change depends on an older op of its own peer,
importpanics. The import computes the change's lamport from that old op. The DAG then merges the change into the peer's last node, andAppDagasserts "lamport is not continuous" while it holds the op log lock. The doc is poisoned after that.Binary blocks store a dep on the own peer as one bit that means
(c-1)@p. So only JSON updates or crafted bytes can carry such a dep.Repro: peer 1 commits three changes. Export them with
export_json_updates_without_peer_compression. Set the deps of2@1to["0@1"]and import the JSON into a new doc:Fix
OpLog::check_own_peer_depsreturnsDecodeErrorfor any dep on the change's own peer other than(c-1)@p. The error names both ids, for example "Invalid change 2@1: it depends on 0@1". The check runs incheck_and_trim_known_part_in_arena, next to the skipped-counter check, before anything is changed. It covers these imports:import_batchA snapshot imported into an empty doc takes the fast path and skips the check. That does no harm: the DAG nodes come from the block metadata, so the merge that panics does not run.
Valid changes never have such a dep. The check is one pass over the deps of the new changes, with no allocation.
Tests
crates/loro/tests/import_own_peer_deps.rshas three tests:Each test checks that the doc keeps its version and value and stays usable. On main, the first test panics and the other two fail because the import succeeds.
This PR includes a patch changeset for
loro-crdt, because JSON import in JS hits the same panic.cargo test -p loro -p loro-internalpasses: 1548 passed, 0 failed, 27 ignored.