Fix message ordering for 1-1 chats

src/status_im/chat/console.cljs

@@ -1,7 +1,7 @@
 (ns status-im.chat.console
   (:require [status-im.ui.components.styles :refer [default-chat-color]]
             [status-im.utils.random :as random]
-            [status-im.constants :as const]
+            [status-im.constants :as constants]
             [status-im.i18n :as i18n]
             [clojure.string :as string]))
 
@@ -9,29 +9,31 @@
                         :or   {message-id (random/id)}}]
   {:message-id   message-id
    :outgoing     false
-   :chat-id      const/console-chat-id
+   :chat-id      constants/console-chat-id
-   :from         const/console-chat-id
+   :from         constants/console-chat-id
    :to           "me"
    :content      content
    :content-type content-type})
 
 (def chat
-  {:chat-id      const/console-chat-id
+  {:chat-id               constants/console-chat-id
-   :name         (string/capitalize const/console-chat-id)
+   :name                  (string/capitalize constants/console-chat-id)
    :color                 default-chat-color
    :group-chat            false
    :is-active             true
    :unremovable?          true
    :timestamp             (.getTime (js/Date.))
-   :photo-path   const/console-chat-id
+   :photo-path            constants/console-chat-id
-   :contacts     [{:identity         const/console-chat-id
+   :contacts              [{:identity         constants/console-chat-id
                             :text-color       "#FFFFFF"
-                   :background-color "#AB7967"}]})
+                            :background-color "#AB7967"}]
+   :last-to-clock-value   0
+   :last-from-clock-value 0})
 
 (def contact
-  {:whisper-identity const/console-chat-id
+  {:whisper-identity constants/console-chat-id
-   :name             (string/capitalize const/console-chat-id)
+   :name             (string/capitalize constants/console-chat-id)
-   :photo-path       const/console-chat-id
+   :photo-path       constants/console-chat-id
    :dapp?            true
    :unremovable?     true
    :bot-url          "local://console-bot"

src/status_im/chat/handlers.cljs

@@ -79,7 +79,9 @@
                    :group-chat            true
                    :public?               true
                    :is-active             true
-                   :timestamp   (random/timestamp)}]
+                   :timestamp             (random/timestamp)
+                   :last-to-clock-value   0
+                   :last-from-clock-value 0}]
       (merge
        (when-not exists?
          {:db (assoc-in db [:chats (:chat-id chat)] chat)
@@ -138,7 +140,9 @@
      :group-admin           current-public-key
      :is-active             true
      :timestamp             (random/timestamp)
-     :contacts    selected-contacts'}))
+     :contacts              selected-contacts'
+     :last-to-clock-value   0
+     :last-from-clock-value 0}))
 
 (handlers/register-handler-fx
   :create-new-group-chat-and-open

src/status_im/chat/models.cljs

@@ -21,7 +21,9 @@
      :group-chat            false
      :is-active             true
      :timestamp             now
-     :contacts   [{:identity chat-id}]}))
+     :contacts              [{:identity chat-id}]
+     :last-to-clock-value   0
+     :last-from-clock-value 0}))
 
 (defn add-chat
   "Adds new chat to db & realm, if the chat with same id already exists, justs restores it"

src/status_im/chat/models/message.cljs

@@ -25,7 +25,7 @@
     (:ref (get available-commands-responses response-name))))
 
 (defn add-message-to-db
-  [db chat-id {:keys [message-id clock-value] :as message} current-chat?]
+  [db chat-id {:keys [message-id from-clock-value to-clock-value] :as message} current-chat?]
   (let [prepared-message (cond-> (assoc message
                                         :chat-id    chat-id
                                         :appearing? true)
@@ -33,13 +33,27 @@
                            (assoc :appearing? false))]
     (cond-> (-> db
                 (update-in [:chats chat-id :messages] assoc message-id prepared-message)
-                (update-in [:chats chat-id :last-clock-value] (fnil max 0) clock-value))
+                (update-in [:chats chat-id :last-from-clock-value] max from-clock-value)
+                (update-in [:chats chat-id :last-to-clock-value] max to-clock-value))
       (not current-chat?)
       (update-in [:chats chat-id :unviewed-messages] (fnil conj #{}) message-id))))
 
+;; We start with [0 0] ([from-clock-value to-clock-value]) for each participant of 1-1 chat (local perspective on each device).
+;; Now for sending, we always increment the to-clock-value and include it in message payload being sent (so only to-clock-value is present in network message).
+;; Locally, the sent message always replicates the latest-from-clock-value of the chat.
+;; Upon receiving message, receiver reads the to-clock-value of the received message and sets that to be the from-clock-value locally
+;; (this will be also the new latest-from-clock-value of the chat), to-clock-value for the message is the latest-to-clock-value of the 1-1 chat`.
+
+;; All this ensures, that there will be no [from-clock-value to-clock-value] duplicate in chat message on each device + the local order will appear consistent,
+;; even if it’s possible it won’t be the same on both devices (for example user A sends 5 messages, during the sending,
+;; he receives the message from user B, so his local order will be A1, A2, B, A3, A4, A5, but his messages will take a long time to reach user B,
+;; for some reason, so user B will see it as B, A1, A2, A3, A4, A5).
+;; I don’t think that’s very problematic and I don’t think we can do much about it without single source of truth where order received messages are serialised
+;; and definite order is established (server), it is the case even in the current implementation.
+
 (defn receive
   [{:keys [db now] :as cofx}
-   {:keys [from group-id chat-id content-type content message-id timestamp clock-value]
+   {:keys [from group-id chat-id content-type content message-id timestamp from-clock-value to-clock-value]
     :as   message}]
   (let [{:keys [current-chat-id view-id
                 access-scope->commands-responses] :contacts/keys [contacts]} db
@@ -51,16 +65,16 @@
                                         (chat-model/upsert-chat cofx {:chat-id chat-identifier
                                                                       :group-chat (boolean group-id)})
                                         (chat-model/add-chat cofx chat-identifier))
-        chat             (get-in fx [:db :chats chat-identifier])
+        {:keys [last-from-clock-value
+                last-to-clock-value]} (get-in fx [:db :chats chat-identifier])
         command-request?              (= content-type constants/content-type-command-request)
         command                       (:command content)
         enriched-message              (cond-> (assoc message
                                                      :chat-id          chat-identifier
                                                      :timestamp        (or timestamp now)
                                                      :show?            true
-                                        :clock-value (clocks-utils/receive
+                                                     :to-clock-value   last-to-clock-value
-                                                      clock-value
+                                                     :from-clock-value (or to-clock-value (inc last-from-clock-value)))
-                                                      (:last-clock-value chat)))
                                         public-key
                                         (assoc :user-statuses {public-key (if current-chat? :seen :received)})
                                         (and command command-request?)
@@ -140,7 +154,7 @@
 (defn- generate-message
   [{:keys [network-status]} chat-id message]
   (assoc (select-keys message [:from :message-id])
-         :payload (cond-> (select-keys message [:content :content-type :clock-value :timestamp :show?])
+         :payload (cond-> (select-keys message [:content :content-type :to-clock-value :timestamp :show?])
                     (= :offline network-status)
                     (assoc :show? false))))
 
@@ -158,14 +172,17 @@
         (cond
           (and group-chat (not public?))
           (let [{:keys [public-key private-key]} (get chats chat-id)]
-            {:send-group-message (assoc options
+            {:send-group-message (-> options
-                                        :group-id chat-id
+                                     (update-in [:message :payload] dissoc :to-clock-value)
+                                     (assoc :group-id chat-id
                                             :keypair {:public  public-key
-                                                  :private private-key})})
+                                                      :private private-key}))})
 
           (and group-chat public?)
-          {:send-public-group-message (assoc options :group-id chat-id
+          {:send-public-group-message (-> options
-                                             :username (get-in accounts [current-account-id :name]))}
+                                          (update-in [:message :payload] dissoc :to-clock-value)
+                                          (assoc :group-id chat-id
+                                                 :username (get-in accounts [current-account-id :name])))}
 
           :else
           (merge {:send-message (assoc-in options [:message :to] chat-id)}
@@ -175,7 +192,7 @@
 
 (defn- prepare-message [params chat now random-id]
   (let [{:keys [chat-id identity message-text]} params
-        {:keys [group-chat public? last-clock-value]} chat
+        {:keys [group-chat public? last-from-clock-value last-to-clock-value]} chat
         message {:message-id       random-id
                  :chat-id          chat-id
                  :content          message-text
@@ -183,7 +200,8 @@
                  :content-type     constants/text-content-type
                  :outgoing         true
                  :timestamp        now
-                 :clock-value  (clocks-utils/send last-clock-value)
+                 :from-clock-value last-from-clock-value
+                 :to-clock-value   (inc last-to-clock-value)
                  :show?            true}]
     (cond-> message
       (not group-chat)
@@ -208,7 +226,7 @@
     (merge fx (send cofx params'))))
 
 (defn- prepare-command
-  [identity chat-id now clock-value
+  [identity chat-id now last-from-clock-value last-to-clock-value
    {request-params  :params
     request-command :command
     :keys           [prefill prefillBotDb]
@@ -244,7 +262,8 @@
      :to-message       to-message
      :type             (:type command)
      :has-handler      (:has-handler command)
-     :clock-value  (clocks-utils/send clock-value)
+     :to-clock-value   (inc last-to-clock-value)
+     :from-clock-value last-from-clock-value
      :show?            true}))
 
 (defn send-command
@@ -254,11 +273,12 @@
           :as   content} :command
          chat-id         :chat-id} params
         request          (:request handler-data)
-        last-clock-value (get-in chats [chat-id :last-clock-value])
+        {:keys [last-to-clock-value
+                last-from-clock-value]} (get chats chat-id)
         hidden-params    (->> (:params command)
                               (filter :hidden)
                               (map :name))
-        command'         (prepare-command current-public-key chat-id now last-clock-value request content)
+        command'         (prepare-command current-public-key chat-id now last-to-clock-value last-from-clock-value request content)
         params'          (assoc params :command command')
         fx               (-> (chat-model/upsert-chat cofx {:chat-id chat-id})
                              (update :db add-message-to-db chat-id command' true)

src/status_im/chat/subs.cljs

@@ -90,10 +90,11 @@
 
 (defn message-datemark-groups
   "Transforms map of messages into sequence of `[datemark messages]` tuples, where
-  messages with particular datemark are sorted according to their `:clock-value` and
+  messages with particular datemark are sorted according to their clock-values and
-  tuples themeselves are sorted according to the highest `:clock-value` in the messages."
+  tuples themeselves are sorted according to the highest clock-values in the messages."
   [id->messages]
-  (let [datemark->messages (transduce (comp (map second)
+  (let [clock-sorter (juxt :from-clock-value :to-clock-value)
+        datemark->messages (transduce (comp (map second)
                                             (filter :show?)
                                             (map (fn [{:keys [timestamp] :as msg}]
                                                    (assoc msg :datemark (time/day-relative timestamp)))))
@@ -103,8 +104,9 @@
                                       id->messages)]
     (->> datemark->messages
          (map (fn [[datemark messages]]
-                [datemark (sort-by :clock-value > messages)]))
+                [datemark (->> messages (sort-by clock-sorter) reverse)]))
-         (sort-by (comp :clock-value first second) >))))
+         (sort-by (comp clock-sorter first second))
+         reverse)))
 
 (reg-sub
   :get-chat-message-datemark-groups

src/status_im/data_store/realm/chats.cljs

@@ -7,10 +7,12 @@
   (:refer-clojure :exclude [exists?]))
 
 (defn- normalize-chat [{:keys [chat-id] :as chat}]
-  (let [last-message (messages/get-last-message chat-id)]
+  (let [last-to-clock-value (messages/get-last-clock-value chat-id :to-clock-value)
+        last-from-clock-value (messages/get-last-clock-value chat-id :from-clock-value)]
     (-> chat
         (realm/fix-map->vec :contacts)
-        (assoc :last-clock-value (or (:clock-value last-message) 0)))))
+        (assoc :last-to-clock-value (or last-to-clock-value 0))
+        (assoc :last-from-clock-value (or last-from-clock-value 0)))))
 
 (defn get-all-active
   []

src/status_im/data_store/realm/messages.cljs

@@ -52,11 +52,12 @@
       (realm/page from (+ from number-of-messages))
       realm/js-object->clj))
 
-(defn get-last-message
+(defn get-last-clock-value
-  [chat-id]
+  [chat-id clock-prop]
   (-> (realm/get-by-field @realm/account-realm :message :chat-id chat-id)
-      (realm/sorted :clock-value :desc)
+      (realm/sorted clock-prop :desc)
-      (realm/single-clj)))
+      (realm/single-clj)
+      (get clock-prop)))
 
 (defn get-unviewed
   [current-public-key]

src/status_im/data_store/realm/schemas/account/core.cljs

@@ -20,7 +20,8 @@
             [status-im.data-store.realm.schemas.account.v19.core :as v19]
             [status-im.data-store.realm.schemas.account.v20.core :as v20]
             [status-im.data-store.realm.schemas.account.v21.core :as v21]
-            [status-im.data-store.realm.schemas.account.v22.core :as v22]))
+            [status-im.data-store.realm.schemas.account.v22.core :as v22]
+            [status-im.data-store.realm.schemas.account.v23.core :as v23]))
 
 
 ;; TODO(oskarth): Add failing test if directory vXX exists but isn't in schemas.
@@ -91,5 +92,7 @@
                :migration     v21/migration}
               {:schema        v22/schema
                :schemaVersion 22
-               :migration     v22/migration}])
+               :migration     v22/migration}
-
+              {:schema        v23/schema
+               :schemaVersion 23
+               :migration     v23/migration}])

src/status_im/data_store/realm/schemas/account/v23/core.cljs

@@ -0,0 +1,64 @@
+(ns status-im.data-store.realm.schemas.account.v23.core
+  (:require [status-im.data-store.realm.schemas.account.v22.chat :as chat]
+            [status-im.data-store.realm.schemas.account.v1.chat-contact :as chat-contact]
+            [status-im.data-store.realm.schemas.account.v19.contact :as contact]
+            [status-im.data-store.realm.schemas.account.v20.discover :as discover]
+            [status-im.data-store.realm.schemas.account.v23.message :as message]
+            [status-im.data-store.realm.schemas.account.v12.pending-message :as pending-message]
+            [status-im.data-store.realm.schemas.account.v1.processed-message :as processed-message]
+            [status-im.data-store.realm.schemas.account.v19.request :as request]
+            [status-im.data-store.realm.schemas.account.v19.user-status :as user-status]
+            [status-im.data-store.realm.schemas.account.v5.contact-group :as contact-group]
+            [status-im.data-store.realm.schemas.account.v5.group-contact :as group-contact]
+            [status-im.data-store.realm.schemas.account.v8.local-storage :as local-storage]
+            [status-im.data-store.realm.schemas.account.v21.browser :as browser]
+            [goog.object :as object]
+            [taoensso.timbre :as log]
+            [cljs.reader :as reader]
+            [clojure.string :as string]))
+
+(def schema [chat/schema
+             chat-contact/schema
+             contact/schema
+             discover/schema
+             message/schema
+             pending-message/schema
+             processed-message/schema
+             request/schema
+             user-status/schema
+             contact-group/schema
+             group-contact/schema
+             local-storage/schema
+             browser/schema])
+
+(defn update-new-message [new-realm message-id to-clock-value from-clock-value]
+  (when-let [message (some-> new-realm
+                             (.objects "message")
+                             (.filtered (str "message-id = \"" message-id "\""))
+                             (aget 0))]
+    (aset message "to-clock-value" to-clock-value)
+    (aset message "from-clock-value" from-clock-value)))
+
+(defn update-chat-messages [old-realm new-realm chat-id]
+  (let [from-clock-value (atom 0)
+        to-clock-value   (atom 0)]
+    (some-> old-realm
+            (.objects "message")
+            (.filtered (str "chat-id = \"" chat-id "\""))
+            (.sorted "clock-value" false)
+            (.map (fn [message _ _]
+                    (let [message-id (object/get message "message-id")
+                          outgoing?  (boolean (object/get message "outgoing"))]
+                      (if outgoing?
+                        (update-new-message new-realm message-id (swap! to-clock-value inc) @from-clock-value)
+                        (update-new-message new-realm message-id @to-clock-value (swap! from-clock-value inc)))))))))
+
+(defn update-chats [old-realm new-realm]
+  (some-> new-realm
+          (.objects "chat")
+          (.map (fn [chat _ _]
+                  (update-chat-messages old-realm new-realm (object/get chat "chat-id"))))))
+
+(defn migration [old-realm new-realm]
+  (log/debug "migrating v23 account database: " old-realm new-realm)
+  (update-chats old-realm new-realm))

src/status_im/data_store/realm/schemas/account/v23/message.cljs

@@ -0,0 +1,32 @@
+(ns status-im.data-store.realm.schemas.account.v23.message)
+
+(def schema {:name       :message
+             :primaryKey :message-id
+             :properties {:message-id       :string
+                          :from             :string
+                          :to               {:type     :string
+                                             :optional true}
+                          :group-id         {:type     :string
+                                             :optional true}
+                          :content          :string ; TODO make it ArrayBuffer
+                          :content-type     :string
+                          :username         {:type     :string
+                                             :optional true}
+                          :timestamp        :int
+                          :chat-id          {:type    :string
+                                             :indexed true}
+                          :outgoing         :bool
+                          :retry-count      {:type    :int
+                                             :default 0}
+                          :message-type     {:type     :string
+                                             :optional true}
+                          :message-status   {:type     :string
+                                             :optional true}
+                          :user-statuses    {:type       :list
+                                             :objectType :user-status}
+                          :from-clock-value {:type    :int
+                                             :default 0}
+                          :to-clock-value   {:type    :int
+                                             :default 0}
+                          :show?            {:type    :bool
+                                             :default true}}})

src/status_im/protocol/handlers.cljs

@@ -470,7 +470,7 @@
               route-fx (case type
                          (:message
                           :group-message
-                          :public-group-message) {:dispatch [:pre-received-message (transform-protocol-message message)]}
+                          :public-group-message) {:dispatch [:chat-received-message/add (transform-protocol-message message)]}
                          :pending                (cond-> {::pending-messages-save message}
                                                    chat-message
                                                    (assoc :dispatch