Skip to content

setu-vlc: VectorClock::is_concurrent flags causally-equal clocks as concurrent when they differ only by explicit zero entries #39

Description

@Alicepoltora

Summary

VectorClock::is_concurrent decides concurrency with !self.happens_before(other) && !other.happens_before(self) && self != other. The self != other term uses the derived PartialEq on the inner HashMap, which is structural. But happens_before correctly treats missing nodes as implicit zeros. As a result, two clocks that represent the same causal state — differing only by an explicit 0 entry — are neither ordered nor structurally equal, so they are wrongly reported as concurrent.

Reproduction

use setu_vlc::VectorClock;

// {a:0} (via with_node) and {} (via new) are the same causal state: nothing happened.
let seeded = VectorClock::with_node("a".to_string()); // {a:0}
let empty  = VectorClock::new();                      // {}

assert!(!seeded.happens_before(&empty));
assert!(!empty.happens_before(&seeded));
assert!(seeded.is_concurrent(&empty)); // <-- returns true, but they are equal, not concurrent

// Same class of problem: an explicit zero on an otherwise-equal clock.
let mut x = VectorClock::new(); x.set("n", 3);
let mut y = VectorClock::new(); y.set("n", 3); y.set("m", 0); // m:0 is implicit-equal
assert_eq!(x, y);              // fails: structurally different
assert!(x.is_concurrent(&y)); // <-- returns true

Both is_concurrent assertions currently pass, demonstrating the bug (verified against main).

Why it matters

Explicit zero entries arise through the crate's own API — with_node() inserts 0, and set(node, 0) / reset_node() produce them — so this is reachable in normal use (e.g. VLCSnapshot::for_node). Vector clocks that are causally identical being classified as concurrent can trigger spurious conflict detection in the consensus/causality layers that build on this crate. VLCSnapshot::is_concurrent delegates here, so it inherits the same behavior.

Root cause

Concurrency is defined in terms of structural equality (self != other) instead of causal equality. happens_before already normalizes implicit zeros; is_concurrent does not.

Suggested fix

Define concurrency without relying on structural equality — e.g. the standard definition: a and b are concurrent iff there exist nodes i, j with a[i] > b[i] and a[j] < b[j] (using implicit-zero get). Equivalently, drop the self != other term and treat causally-equal clocks as not concurrent via an implicit-zero equality check. Happy to send a PR with the fix plus regression tests if the maintainers agree on the intended semantics.

Environment

  • Crate: crates/setu-vlc, main @ current HEAD
  • rustc stable

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions