fix(call_log): take a synced call's direction from its creator, not the direction fields

src/client/app_state.rs

@@ -3055,8 +3055,15 @@ impl Client {
             return;
         }
 
-        if crate::features::call_log::dispatch_call_log_mutation(&self.core.event_bus, m, full_sync)
-        {
+        // A call's direction is its creator compared against this account; the
+        // predicate is only consulted once the mutation is known to be a call
+        // log, so the other mutation kinds do not pay for the snapshot.
+        if crate::features::call_log::dispatch_call_log_mutation(
+            &self.core.event_bus,
+            m,
+            full_sync,
+            |jid| self.is_own_jid(jid),
+        ) {
             return;
         }
 

src/features/call_log.rs

@@ -6,23 +6,39 @@
 //!
 //! # The index carries what the record can leave out
 //!
-//! The index is `["call_log", callCreatorJid, callId, fromMe]`, not the bare
+//! The index is `["call_log", callCreatorJid, callId, direction]`, not the bare
 //! literal `schemas::CALL_LOG.index_parts` declares. WA Web builds it as
 //! `JSON.stringify([action, ...indexArgs])` (`WAWebSyncdActionUtils.buildIndex`)
-//! and `getCallLogMutation` passes `indexArgs: [d, p, m]` — the call creator, the
-//! call id, and whether this account placed the call.
+//! and `getCallLogMutation` passes `indexArgs: [d, p, m]` — the call creator,
+//! the call id, and a direction flag whose meaning is not agreed on (below).
 //!
 //! That matters because the *record*'s `callCreatorJid` is optional and WA Web
 //! leaves it unset for calls it did not receive one for, while the index's is
 //! filled in either way — `d == null && (d = fromMe ? me : peerJid)`. A consumer
 //! given only the record cannot always say who the call was with.
 //!
-//! # `is_incoming` does not mean what it says
+//! # The direction fields disagree with each other, so neither is read
 //!
-//! WA Web writes the record with `isIncoming: n.fromMe`, so the field carries
-//! "this account placed the call" — the opposite of its name. The index's
-//! `fromMe` is the same value under an honest name, which is why
-//! [`CallLogSync::from_me`] comes from there and is the field to trust.
+//! Two fields claim to carry the call's direction, and which one is honest
+//! depends on who wrote the mutation:
+//!
+//! - WA Web's own writer sets both from its local `fromMe`: `indexArgs: [d, p,
+//!   m]` with `m = n.fromMe ? "1" : "0"`, and `isIncoming: n.fromMe`. Its
+//!   `isIncoming` is therefore inverted against its own name.
+//! - Mutations authored by the phone carry them the other way round, literally
+//!   as `isIncoming` — a call the account placed arrives as `0`/`false`.
+//!
+//! Since app state fans a companion's mutations out to every other device, both
+//! flavors reach this client, and no fixed reading of either field is right for
+//! both.
+//!
+//! WA Web's reader sidesteps the whole thing, and so does this module:
+//! `generateCallLogFromCallSyncRecord` destructures the record without ever
+//! touching `isIncoming`, and takes direction from
+//! `getCallLogTargetDetails`, which is `fromMe: isMeAccount(callCreatorWid)` —
+//! the call creator compared against this account. [`CallLogSync::from_me`] is
+//! derived the same way. That is also why the creator is worth having in the
+//! index: it is what the direction is computed from.
 
 use crate::appstate_sync::Mutation;
 use wacore::appstate::schemas;
@@ -32,10 +48,15 @@ use waproto::whatsapp as wa;
 
 /// Dispatch inbound call-log mutations synced from the primary device.
 /// Returns `true` if handled, `false` if the mutation is not a call log.
+///
+/// `is_own_jid` decides the call's direction and is consulted only once the
+/// mutation is known to be a call log, so the app-state path pays nothing for
+/// it on the mutations it is not.
 pub(crate) fn dispatch_call_log_mutation(
     event_bus: &wacore::types::events::CoreEventBus,
     m: &Mutation,
     full_sync: bool,
+    is_own_jid: impl FnOnce(&Jid) -> bool,
 ) -> bool {
     if m.operation != wa::syncd_mutation::SyncdOperation::Set
         || m.index.first().map(String::as_str) != Some(schemas::CALL_LOG.name)
@@ -53,16 +74,9 @@ pub(crate) fn dispatch_call_log_mutation(
         log::warn!("Skipping call_log mutation: missing call id in index");
         return true;
     };
-    // WA Web writes "1"/"0" (`m = n.fromMe ? "1" : "0"`). Anything else is a
-    // shape we do not know how to read, and guessing would mislabel the call.
-    let from_me = match m.index.get(3).map(String::as_str) {
-        Some("1") => true,
-        Some("0") => false,
-        other => {
-            log::warn!("Skipping call_log mutation: unreadable fromMe {other:?} in index");
-            return true;
-        }
-    };
+    // Direction comes from the creator; see the module docs for why not from
+    // either field that claims to carry it.
+    let from_me = is_own_jid(&call_creator_jid);
 
     // The mutation's own time, which is metadata rather than the call's: WA Web
     // measures it against the pairing timestamp to drop records that predate the
@@ -141,11 +155,31 @@ mod tests {
         }
     }
 
+    /// This account, as either identity: a creator matching one of these is a
+    /// call we placed. Stands in for `Client::is_own_jid`, which compares a JID
+    /// against the device's own PN and LID.
+    const OWN_PN: &str = "5511888880000@s.whatsapp.net";
+    const OWN_LID: &str = "111122223333444@lid";
+    /// Whoever we were talking to. Not us, under either identity.
+    const PEER: &str = "5511999990000@s.whatsapp.net";
+
+    fn is_own(jid: &Jid) -> bool {
+        matches!(jid.user.as_str(), "5511888880000" | "111122223333444")
+    }
+
     fn dispatch(mutation: &Mutation, full_sync: bool) -> (bool, Vec<Arc<Event>>) {
+        dispatch_as(mutation, full_sync, is_own)
+    }
+
+    fn dispatch_as(
+        mutation: &Mutation,
+        full_sync: bool,
+        is_own_jid: impl FnOnce(&Jid) -> bool,
+    ) -> (bool, Vec<Arc<Event>>) {
         let bus = CoreEventBus::new();
         let recorder = Arc::new(Recorder::default());
         bus.subscribe_handler(recorder.clone()).detach();
-        let handled = dispatch_call_log_mutation(&bus, mutation, full_sync);
+        let handled = dispatch_call_log_mutation(&bus, mutation, full_sync, is_own_jid);
         let events = recorder.events.lock().unwrap().clone();
         (handled, events)
     }
@@ -165,8 +199,12 @@ mod tests {
         }
     }
 
+    /// A call this account placed: the creator is us, which is what the
+    /// direction is read from. The fourth part is the writers' disputed field
+    /// and is deliberately set to the value that would give the wrong answer if
+    /// anything still read it as `fromMe`.
     fn full_index() -> [&'static str; 4] {
-        ["call_log", "5511999990000@s.whatsapp.net", "call-42", "1"]
+        ["call_log", OWN_PN, "call-42", "0"]
     }
 
     #[test]
@@ -206,46 +244,82 @@ mod tests {
         let Event::CallLogSync(update) = events[0].as_ref() else {
             panic!("expected CallLogSync event");
         };
-        assert_eq!(
-            update.call_creator_jid.to_string(),
-            "5511999990000@s.whatsapp.net"
-        );
+        assert_eq!(update.call_creator_jid.to_string(), OWN_PN);
         assert_eq!(update.call_id, "call-42");
-        assert!(
-            update.from_me,
-            "the index says this account placed the call"
-        );
         assert_eq!(update.timestamp.timestamp_millis(), 1_700_000_000_000);
     }
 
-    /// WA Web writes the record's `isIncoming` from its local `fromMe`, so the
-    /// two disagree by name and agree by value. A consumer trusting the field
-    /// name would file every call backwards; `from_me` is the honest one. Both
-    /// directions, so neither a constant nor the record can stand in for it.
+    /// The regression: a call this account placed reads as `from_me`.
+    ///
+    /// Both of the fields that claim to carry direction are set the way the
+    /// phone sends them for an outbound call — index `"0"`, record
+    /// `is_incoming: false` — because that combination is what made this read
+    /// backwards. Direction comes from the creator, so it survives them.
+    #[test]
+    fn a_call_this_account_placed_is_from_me() {
+        for creator in [OWN_PN, OWN_LID] {
+            let record = wa::CallLogRecord {
+                is_incoming: Some(false),
+                ..Default::default()
+            };
+            let index = ["call_log", creator, "call-7", "0"];
+            let (_, events) = dispatch(&call_log_mutation(&index, Some(record)), false);
+
+            let Event::CallLogSync(update) = events[0].as_ref() else {
+                panic!("expected CallLogSync event");
+            };
+            assert!(
+                update.from_me,
+                "{creator} is this account, so it placed the call"
+            );
+        }
+    }
+
+    /// The other direction, and the failure case for the fix: a call somebody
+    /// else placed must not become ours just because the direction fields say
+    /// so. WA Web's writer sets both to `fromMe`, so an inbound call it logged
+    /// carries index `"0"` and `is_incoming: false` — identical to the outbound
+    /// fixture above, and told apart only by the creator.
     #[test]
-    fn from_me_comes_from_the_index_not_the_record() {
-        for (index_from_me, record_is_incoming, expected) in
-            [("0", Some(true), false), ("1", Some(false), true)]
-        {
+    fn a_call_the_peer_placed_is_not_from_me() {
+        for (index_part, record_is_incoming) in [("0", Some(false)), ("1", Some(true))] {
             let record = wa::CallLogRecord {
                 is_incoming: record_is_incoming,
                 ..Default::default()
             };
-            let index = [
-                "call_log",
-                "5511999990000@s.whatsapp.net",
-                "call-7",
-                index_from_me,
-            ];
+            let index = ["call_log", PEER, "call-7", index_part];
             let (_, events) = dispatch(&call_log_mutation(&index, Some(record)), false);
 
             let Event::CallLogSync(update) = events[0].as_ref() else {
                 panic!("expected CallLogSync event");
             };
-            assert_eq!(
-                update.from_me, expected,
-                "the index is authoritative, and it says fromMe={index_from_me}"
+            assert!(
+                !update.from_me,
+                "the creator is the peer, whatever index[3]={index_part} claims"
+            );
+        }
+    }
+
+    /// The fourth index part is no longer read, so a value we cannot parse is
+    /// no longer a reason to drop the call: everything the event carries comes
+    /// from parts we did read.
+    #[test]
+    fn an_unreadable_direction_part_no_longer_drops_the_call() {
+        for index in [
+            ["call_log", OWN_PN, "call-42", "yes"],
+            ["call_log", OWN_PN, "call-42", ""],
+        ] {
+            let (handled, events) = dispatch(
+                &call_log_mutation(&index, Some(wa::CallLogRecord::default())),
+                false,
             );
+
+            assert!(handled);
+            assert_eq!(events.len(), 1, "{index:?} is still a usable call log");
+            let Event::CallLogSync(update) = events[0].as_ref() else {
+                panic!("expected CallLogSync event");
+            };
+            assert!(update.from_me);
         }
     }
 
@@ -259,14 +333,13 @@ mod tests {
 
     /// An index we cannot read is still ours — handing it on would only offer it
     /// to dispatchers keyed on other indexes — but it cannot be turned into an
-    /// event a consumer could trust.
+    /// event a consumer could trust. Only the parts the event is built from:
+    /// without a creator there is no direction either.
     #[test]
     fn an_unreadable_index_is_claimed_without_event() {
         for index in [
             &["call_log"][..],
-            &["call_log", "5511999990000@s.whatsapp.net"][..],
-            &["call_log", "5511999990000@s.whatsapp.net", "call-42"][..],
-            &["call_log", "5511999990000@s.whatsapp.net", "call-42", "yes"][..],
+            &["call_log", PEER][..],
             &["call_log", "not a jid", "call-42", "1"][..],
         ] {
             let (handled, events) = dispatch(

src/features/contacts.rs

@@ -231,14 +231,7 @@ impl<'a> Contacts<'a> {
         }
 
         // Skip own JID: server never responds when tctoken is sent for self
-        let is_own_jid = {
-            let snap = self.client.persistence_manager.get_device_snapshot();
-            snap.pn.as_ref().is_some_and(|pn| pn.is_same_user_as(jid))
-                || snap
-                    .lid
-                    .as_ref()
-                    .is_some_and(|lid| lid.is_same_user_as(jid))
-        };
+        let is_own_jid = self.client.is_own_jid(jid);
         if !jid.is_group()
             && !jid.is_newsletter()
             && !jid.is_bot()

src/send/tctoken_lifecycle.rs

@@ -1,19 +1,27 @@
 use super::*;
 
+/// Whether `jid` addresses this account, given its two identities.
+///
+/// Both, because a JID reaches us as a phone number or a LID depending on the
+/// thread's migration state, and only one of the two ever matches.
+///
+/// Addressing mode is part of the answer, not noise: WA Web's `isMeAccount`
+/// keys on `isSameAccountAndAddressingMode`, so a peer LID whose digits happen
+/// to spell our phone number is not us. `is_same_user_as` compares only the
+/// user and would say it is — the same looseness `is_same_chat_as` exists to
+/// guard, and the same rule `is_self_dm_recipient` already follows.
+pub(crate) fn is_own_identity(own_pn: Option<&Jid>, own_lid: Option<&Jid>, jid: &Jid) -> bool {
+    own_pn.is_some_and(|pn| pn.is_same_chat_as(jid))
+        || own_lid.is_some_and(|lid| lid.is_same_chat_as(jid))
+}
+
 impl Client {
     /// Whether `jid` is our own account (PN or LID). The privacy-token paths
     /// never attach to or issue for ourselves; a single source of truth keeps
     /// the message and call paths from drifting apart.
-    fn is_own_jid(&self, jid: &Jid) -> bool {
+    pub(crate) fn is_own_jid(&self, jid: &Jid) -> bool {
         let snapshot = self.persistence_manager.get_device_snapshot();
-        snapshot
-            .pn
-            .as_ref()
-            .is_some_and(|pn| pn.is_same_user_as(jid))
-            || snapshot
-                .lid
-                .as_ref()
-                .is_some_and(|lid| lid.is_same_user_as(jid))
+        is_own_identity(snapshot.pn.as_ref(), snapshot.lid.as_ref(), jid)
     }
 
     /// Look up and include a privacy token in outgoing 1:1 message stanza nodes.
@@ -412,10 +420,42 @@ impl Client {
 
 #[cfg(test)]
 mod tests {
+    use super::is_own_identity;
     use crate::test_utils::create_test_client;
     use wacore::store::traits::TcTokenEntry;
     use wacore_binary::{Jid, Server};
 
+    /// Self-detection keys on the addressing mode, not just the digits. A LID
+    /// is an assigned number in its own namespace, so one can spell a phone
+    /// number that belongs to somebody else — and a user-only comparison would
+    /// hand that peer our own identity: no privacy token where one is due, and
+    /// a call they placed filed as one we placed.
+    #[test]
+    fn a_peer_addressed_in_the_other_namespace_is_not_us() {
+        let own_pn = Jid::new("5511888880000", Server::Pn);
+        let own_lid = Jid::new("111122223333444", Server::Lid);
+
+        // Each identity matches itself, device suffix and all.
+        assert!(is_own_identity(Some(&own_pn), Some(&own_lid), &own_pn));
+        assert!(is_own_identity(Some(&own_pn), Some(&own_lid), &own_lid));
+        assert!(is_own_identity(
+            Some(&own_pn),
+            Some(&own_lid),
+            &own_pn.with_device(12)
+        ));
+
+        // Same digits, other namespace: a different account, both ways round.
+        let peer_lid = Jid::new("5511888880000", Server::Lid);
+        let peer_pn = Jid::new("111122223333444", Server::Pn);
+        assert!(!is_own_identity(Some(&own_pn), Some(&own_lid), &peer_lid));
+        assert!(!is_own_identity(Some(&own_pn), Some(&own_lid), &peer_pn));
+
+        // And with no LID known, our PN's digits in the LID namespace are still
+        // not us — the case `lid_recipient_without_own_lid_is_not_self_dm`
+        // pins for self-DM detection, which follows the same rule.
+        assert!(!is_own_identity(Some(&own_pn), None, &peer_lid));
+    }
+
     #[tokio::test]
     async fn record_sender_timestamp_creates_byteless_placeholder() {
         let client = create_test_client().await;

wacore/src/types/events.rs

@@ -2415,12 +2415,14 @@ pub struct CallLogSync {
     /// The call's identifier, from the mutation's index (the same value
     /// `record.call_id` carries when the record carries one).
     pub call_id: String,
-    /// Whether *this account* placed the call.
+    /// Whether *this account* placed the call, from
+    /// [`call_creator_jid`](Self::call_creator_jid) compared against this
+    /// account.
     ///
-    /// Read this rather than `record.is_incoming`, which despite its name holds
-    /// the same thing rather than its opposite: WA Web writes the record with
-    /// `isIncoming: fromMe`, so a consumer taking the field at its word files
-    /// every call backwards.
+    /// Read this rather than `record.is_incoming`, which is not reliable in
+    /// either direction: it means the opposite of its name in mutations WA Web
+    /// wrote and exactly its name in ones the phone wrote, so a consumer taking
+    /// it at its word files some calls backwards.
     pub from_me: bool,
     /// When the mutation was written, not when the call happened — the call's
     /// own time is `record.start_time`.