[#123] Vector clock verifications too strict and should not reject valid events

Changelog: fix

Author: markharding

packages/rs-core/src/client.rs

@@ -510,6 +510,10 @@ impl PolycentricClient {
             }
         }
 
+        // Content is valid once its signature and signer authorization check
+        // out. The vector clock still has to be structurally sound, but a
+        // referenced event we haven't synced is a gap to backfill, not grounds
+        // to reject — so the missing list is allowed to be non-empty here.
         crate::vector_clock::verify_vector_clock(
             &self.event_store,
             vc,
@@ -1149,7 +1153,7 @@ mod tests {
     }
 
     #[test]
-    fn rejects_event_referencing_unseen_co_signer_event() {
+    fn keeps_content_event_referencing_unsynced_event() {
         let mut client = PolycentricClient::new();
         let a = keypair(1);
         let b = keypair(2);
@@ -1162,13 +1166,16 @@ mod tests {
             vec![b.public.clone()],
         );
 
-        // B claims to have observed A at seq=7 in collection 2 — we have none.
+        // B's event references A at seq=7 in collection 2, which we haven't
+        // synced. The event is still valid — the gap is ours to fetch.
         let event = sign_event(&b, &identity, 2, 1, 1, vec![7, 1], dummy_post_digest());
-        assert_invalid_contains(client.validate_event(&event, &[]), "unseen event");
+        client
+            .validate_event(&event, &[])
+            .expect("content event with an unsynced referenced event should validate");
     }
 
     #[test]
-    fn validates_event_with_observed_co_signer_event_present() {
+    fn validates_event_when_referenced_event_present() {
         let mut client = PolycentricClient::new();
         let a = keypair(1);
         let b = keypair(2);

packages/rs-core/src/identity.rs

@@ -79,6 +79,9 @@ impl IdentityDirectory {
                             && e.content == current.content)
                 })
                 .filter(|e| match &e.vc {
+                    // Chain building needs the full causal history: only accept
+                    // a candidate whose vector clock verifies and references no
+                    // unsynced events.
                     Some(vc) => vector_clock::verify_vector_clock(
                         store,
                         vc,
@@ -88,7 +91,8 @@ impl IdentityDirectory {
                         &e.signer,
                         e.sequence,
                     )
-                    .is_ok(),
+                    .map(|missing| missing.is_empty())
+                    .unwrap_or(false),
                     None => true,
                 })
                 .collect();

packages/rs-core/src/vector_clock.rs

@@ -1,12 +1,19 @@
 use polycentric_common::{
     error::CoreError,
-    models::protos_v2::{Identity, PublicKey, VectorClock},
+    models::protos_v2::{EventKey, Identity, PublicKey, VectorClock},
 };
 
 use crate::store::event_store::EventStore;
 
-/// Structural check against the identity doc + every non-self non-zero
-/// entry must reference an event we've seen from that co-signer.
+/// Check a vector clock against the identity doc.
+///
+/// Returns `Err` if the clock is structurally invalid (wrong length, unknown
+/// signer, or a self entry that doesn't match the event's sequence).
+///
+/// Returns `Ok(missing)` with the referenced events from the identity's other
+/// keys that we haven't synced. A missing event does not make the referencing
+/// event invalid — it's just an event to fetch. Building the identity chain
+/// needs `missing` empty; content readers keep the event either way.
 pub fn verify_vector_clock(
     store: &EventStore,
     vc: &VectorClock,
@@ -15,9 +22,10 @@ pub fn verify_vector_clock(
     collection: i32,
     signer: &PublicKey,
     expected_self_sequence: u64,
-) -> Result<(), CoreError> {
+) -> Result<Vec<EventKey>, CoreError> {
     let self_position = vc.get_signer_position(doc, signer, expected_self_sequence)?;
     let dedup = doc.deduplicated_keys();
+    let mut missing = Vec::new();
     for (pos, &observed) in vc.sequence.iter().enumerate() {
         if pos == self_position || observed == 0 {
             continue;
@@ -27,11 +35,13 @@ pub fn verify_vector_clock(
             .by_identity_collection_signer(identity, collection, other.key_type, &other.key, 0)
             .any(|(k, _)| k.sequence == observed);
         if !seen {
-            return Err(CoreError::InvalidEvent(format!(
-                "vector_clock references unseen event from co-signer at sequence {}",
-                observed
-            )));
+            missing.push(EventKey {
+                collection,
+                identity: identity.to_string(),
+                signed_by: Some(other.clone()),
+                sequence: observed,
+            });
         }
     }
-    Ok(())
+    Ok(missing)
 }