fix(setu-vlc): don't report causally-equal clocks as concurrent - #40
Open
Alicepoltora wants to merge 1 commit into
Open
fix(setu-vlc): don't report causally-equal clocks as concurrent#40Alicepoltora wants to merge 1 commit into
Alicepoltora wants to merge 1 commit into
Conversation
VectorClock::is_concurrent decided concurrency with a structural
`self != other` on the inner HashMap, while happens_before treats
missing nodes as implicit zeros. Clocks representing the same causal
state but differing by an explicit zero entry ({a:0} vs {}, or
{n:3} vs {n:3,m:0}) were therefore wrongly reported as concurrent,
which can cause spurious conflict detection in the consensus layer.
Reimplement is_concurrent by scanning the union of nodes with
implicit-zero semantics: concurrent iff one clock is strictly
greater at some node and strictly less at another. Add regression
tests for the explicit-zero cases plus divergent/ordered/empty.
Fixes AdvaitaLabs#39.
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.
Fixes #39.
Problem
VectorClock::is_concurrentdecided concurrency with!self.happens_before(other) && !other.happens_before(self) && self != other. Theself != otherterm is a structuralHashMapcomparison, whereashappens_beforecorrectly treats a missing node as an implicit0. Two clocks representing the same causal state but differing only by an explicit zero entry were therefore reported as concurrent:Such explicit zeros are produced by the crate's own API (
with_node,set(_, 0),reset_node,VLCSnapshot::for_node), so this is reachable in normal use and can cause spurious conflict detection in the consensus/causality layers.VLCSnapshot::is_concurrentdelegates here and inherits the fix.Fix
Reimplement
is_concurrentby scanning the union of node ids with implicit-zero semantics: two clocks are concurrent iff one is strictly greater at some node and strictly less at another. This no longer depends on structural equality, so causally-equal clocks are correctly treated as equal (not concurrent). No other behavior changes — ordered and genuinely-divergent pairs are classified exactly as before.Tests
Adds regression tests: explicit-zero equal cases (
{a:0}vs{},{n:3}vs{n:3,m:0}) now returnfalse; divergent pairs stilltrue; ordered and empty pairsfalse; symmetry check. Verified locally:cargo test -p setu-vlcgreen,cargo clippyclean, added coderustfmt-clean.Notes
Only the
is_concurrentbody and a#[cfg(test)]module are touched; no public signatures change. Complementary to the test-coverage PR #38 (they can be rebased in either order).