From b6e515618bc26590a8a71fc06c1ed3b84575088c Mon Sep 17 00:00:00 2001 From: Roman Volosovskyi Date: Mon, 3 Dec 2018 17:34:26 +0200 Subject: [PATCH] [#6956] upgradable `message-id` Implementation: 1. `transport.utils/message-id` function is called only in three places now and accepts `from` and `raw_payload` as parameters. ID is calculated as `sha3(from + raw_payload)`. 2. This means that for wrapped private group chat message the raw payload of `GroupMembershipUpdate` is used. --- src/status_im/chat/models/message.cljs | 29 +-- .../realm/schemas/account/migrations.cljs | 178 +++++++++--------- src/status_im/events.cljs | 4 +- src/status_im/group_chats/core.cljs | 50 ++--- src/status_im/transport/impl/receive.cljs | 4 +- src/status_im/transport/message/contact.cljs | 2 - src/status_im/transport/message/protocol.cljs | 12 +- src/status_im/transport/utils.cljs | 8 +- .../cljs/status_im/test/group_chats/core.cljs | 8 +- 9 files changed, 154 insertions(+), 141 deletions(-) diff --git a/src/status_im/chat/models/message.cljs b/src/status_im/chat/models/message.cljs index 881c6f7ef5..0d18eea8b4 100644 --- a/src/status_im/chat/models/message.cljs +++ b/src/status_im/chat/models/message.cljs @@ -28,7 +28,8 @@ [status-im.ui.components.react :as react] [status-im.utils.fx :as fx] [taoensso.timbre :as log] - [status-im.data-store.messages :as messages-store])) + [status-im.data-store.messages :as messages-store] + [status-im.transport.message.transit :as transit])) (defn- wrap-group-message "Wrap a group message in a membership update" @@ -305,7 +306,7 @@ :message-type :system-message :content-type constants/content-type-status}] (assoc message - :message-id (transport.utils/message-id message) + :message-id (transport.utils/system-message-id message) :old-message-id "system"))) (defn group-message? [{:keys [message-type]}] @@ -318,11 +319,7 @@ (if (= network-status :offline) {:dispatch-later [{:ms 10000 :dispatch [:message/update-message-status chat-id message-id :not-sent]}]} - (let [wrapped-record (if (= (:message-type send-record) :group-user-message) - (wrap-group-message cofx chat-id send-record) - send-record)] - - (protocol/send wrapped-record chat-id cofx)))) + (protocol/send send-record chat-id (assoc cofx :message-id message-id)))) (defn add-message-type [message {:keys [chat-id group-chat public?]}] (cond-> message @@ -335,10 +332,14 @@ (def ^:private transport-keys [:content :content-type :message-type :clock-value :timestamp]) -(fx/defn upsert-and-send [{:keys [now] :as cofx} {:keys [chat-id] :as message}] +(fx/defn upsert-and-send [{:keys [now] :as cofx} {:keys [chat-id from] :as message}] (let [send-record (protocol/map->Message (select-keys message transport-keys)) old-message-id (transport.utils/old-message-id send-record) - message-id (transport.utils/message-id message) + wrapped-record (if (= (:message-type send-record) :group-user-message) + (wrap-group-message cofx chat-id send-record) + send-record) + raw-payload (transport.utils/from-utf8 (transit/serialize wrapped-record)) + message-id (transport.utils/message-id from raw-payload) message-with-id (assoc message :message-id message-id :old-message-id old-message-id)] @@ -348,7 +349,7 @@ :timestamp now}) (add-message false message-with-id true) (add-own-status chat-id message-id :sending) - (send chat-id message-id send-record)))) + (send chat-id message-id wrapped-record)))) (fx/defn send-push-notification [cofx fcm-token status] (when (and fcm-token (= status :sent)) @@ -374,9 +375,13 @@ send-record (-> message (select-keys transport-keys) (update :message-type keyword) - protocol/map->Message)] + protocol/map->Message) + + wrapped-record (if (= (:message-type send-record) :group-user-message) + (wrap-group-message cofx chat-id send-record) + send-record)] (fx/merge cofx - (send chat-id message-id send-record) + (send chat-id message-id wrapped-record) (update-message-status chat-id message-id :sending)))) (fx/defn remove-message-from-group diff --git a/src/status_im/data_store/realm/schemas/account/migrations.cljs b/src/status_im/data_store/realm/schemas/account/migrations.cljs index 1dadc6ca82..b80f7d8fd5 100644 --- a/src/status_im/data_store/realm/schemas/account/migrations.cljs +++ b/src/status_im/data_store/realm/schemas/account/migrations.cljs @@ -4,9 +4,10 @@ [status-im.chat.models.message-content :as message-content] [status-im.transport.utils :as transport.utils] [cljs.tools.reader.edn :as edn] - [status-im.js-dependencies :as dependencies] [clojure.string :as string] - [cljs.tools.reader.edn :as edn])) + [status-im.constants :as constants] + [cognitect.transit :as transit] + [status-im.js-dependencies :as dependencies])) (defn v1 [old-realm new-realm] (log/debug "migrating v1 account database: " old-realm new-realm)) @@ -162,77 +163,9 @@ (defn v24 [old-realm new-realm] (log/debug "migrating v24 account database")) -(defn v25 [old-realm new-realm] - (log/debug "migrating v25 account database") - (let [new-messages (.objects new-realm "message") - user-statuses (.objects new-realm "user-status") - old-ids->new-ids (volatile! {}) - updated-messages-ids (volatile! #{}) - updated-message-statuses-ids (volatile! #{}) - messages-to-be-deleted (volatile! []) - statuses-to-be-deleted (volatile! [])] - (dotimes [i (.-length new-messages)] - (let [message (aget new-messages i) - message-id (aget message "message-id") - from (aget message "from") - chat-id (:chat-id (edn/read-string (aget message "content"))) - clock-value (aget message "clock-value") - new-message-id (transport.utils/message-id - {:from from - :chat-id chat-id - :clock-value clock-value})] - (vswap! old-ids->new-ids assoc message-id new-message-id))) - - (dotimes [i (.-length new-messages)] - (let [message (aget new-messages i) - old-message-id (aget message "message-id") - content (edn/read-string (aget message "content")) - response-to (:response-to content) - new-message-id (get @old-ids->new-ids old-message-id)] - (if (contains? @updated-messages-ids new-message-id) - (vswap! messages-to-be-deleted conj message) - (do - (vswap! updated-messages-ids conj new-message-id) - (aset message "message-id" new-message-id) - (when (and response-to (get @old-ids->new-ids response-to)) - (let [new-content (assoc content :response-to - (get @old-ids->new-ids response-to))] - (aset message "content" (prn-str new-content)))))))) - - (doseq [message @messages-to-be-deleted] - (.delete new-realm message)) - - (dotimes [i (.-length user-statuses)] - (let [user-status (aget user-statuses i) - message-id (aget user-status "message-id") - new-message-id (get @old-ids->new-ids message-id) - public-key (aget user-status "public-key") - new-status-id (str new-message-id "-" public-key)] - (if (contains? @updated-message-statuses-ids new-status-id) - (vswap! statuses-to-be-deleted conj user-status) - (do - (vswap! updated-message-statuses-ids conj new-status-id) - (aset user-status "status-id" new-status-id) - (aset user-status "message-id" new-message-id))))) - - (doseq [status @statuses-to-be-deleted] - (.delete new-realm status)))) +(defn v25 [old-realm new-realm]) (defn v26 [old-realm new-realm] - (let [user-statuses (.objects new-realm "user-status")] - (dotimes [i (.-length user-statuses)] - (let [user-status (aget user-statuses i) - status-id (aget user-status "message-id") - message-id (aget user-status "message-id") - public-key (aget user-status "public-key") - new-status-id (str message-id "-" public-key)] - (when (and (= "-" (last status-id))) - (if (.objectForPrimaryKey - new-realm - "user-status" - new-status-id) - (.delete new-realm user-status) - (aset user-status "status-id" new-status-id)))))) (let [chats (.objects new-realm "chat")] (dotimes [i (.-length chats)] (let [chat (aget chats i) @@ -244,32 +177,103 @@ (.-length))] (aset chat "unviewed-messages-count" user-statuses-count))))) +;; Message record's interface was +;; copied from status-im.transport.message.protocol +;; to ensure that any further changes to this record will not +;; affect migrations (defrecord Message [content content-type message-type clock-value timestamp]) -(defn sha3 [s] - (.sha3 dependencies/Web3.prototype s)) - (defn replace-ns [str-message] (string/replace-first str-message "status-im.data-store.realm.schemas.account.migrations" "status-im.transport.message.protocol")) +(defn sha3 [s] + (.sha3 dependencies/Web3.prototype s)) + (defn old-message-id + "Calculates the same `message-id` as was used in `0.9.31`" [message] (sha3 (replace-ns (pr-str message)))) -(defn v27 [old-realm new-realm] - (let [messages (.objects new-realm "message")] +;; The code below copied from status-im.transport.message.transit +;; in order to make sure that future changes will not have any impact +;; on migrations +(defn- new->legacy-command-data [{:keys [command-path params] :as content}] + (get {["send" #{:personal-chats}] [{:command-ref ["transactor" :command 83 "send"] + :command "send" + :bot "transactor" + :command-scope-bitmask 83} + constants/content-type-command] + ["request" #{:personal-chats}] [{:command-ref ["transactor" :command 83 "request"] + :request-command-ref ["transactor" :command 83 "send"] + :command "request" + :request-command "send" + :bot "transactor" + :command-scope-bitmask 83 + :prefill [(get params :asset) + (get params :amount)]} + constants/content-type-command-request]} + command-path)) + +(deftype MessageHandler [] + Object + (tag [this v] "c4") + (rep [this {:keys [content content-type message-type clock-value timestamp]}] + (condp = content-type + constants/content-type-text ;; append new content add the end, still pass content the old way at the old index + #js [(:text content) content-type message-type clock-value timestamp content] + constants/content-type-command ;; handle command compatibility issues + (let [[legacy-content legacy-content-type] (new->legacy-command-data content)] + #js [(merge content legacy-content) (or legacy-content-type content-type) message-type clock-value timestamp]) + ;; no need for legacy conversions for rest of the content types + #js [content content-type message-type clock-value timestamp]))) + +(def writer (transit/writer :json + {:handlers + {Message (MessageHandler.)}})) + +(defn serialize + "Serializes a record implementing the StatusMessage protocol using the custom writers" + [o] + (transit/write writer o)) + +(defn raw-payload + [message] + (transport.utils/from-utf8 (serialize message))) + +(defn v27 [old-ream new-realm] + (let [messages (.objects new-realm "message") + user-statuses (.objects new-realm "user-status") + old-ids->new-ids (volatile! {})] (dotimes [i (.-length messages)] - (let [js-message (aget messages i) - message {:content (edn/read-string - (aget js-message "content")) - :content-type (aget js-message "content-type") - :message-type (keyword - (aget js-message "message-type")) - :clock-value (aget js-message "clock-value") - :timestamp (aget js-message "timestamp")} - message-record (map->Message message) - old-message-id (old-message-id message-record)] - (aset js-message "old-message-id" old-message-id))))) + (let [message (aget messages i) + prev-message-id (aget message "message-id") + content (-> (aget message "content") + edn/read-string + (dissoc :should-collapse? :metadata :render-recipe)) + content-type (aget message "content-type") + message-type (keyword + (aget message "message-type")) + clock-value (aget message "clock-value") + from (aget message "from") + timestamp (aget message "timestamp") + message-record (Message. content content-type message-type + clock-value timestamp) + old-message-id (old-message-id message-record) + raw-payload (raw-payload message-record) + message-id (transport.utils/message-id from raw-payload)] + (vswap! old-ids->new-ids assoc prev-message-id message-id) + (aset message "message-id" message-id) + (aset message "old-message-id" old-message-id))) + + (dotimes [i (.-length user-statuses)] + (let [user-status (aget user-statuses i) + message-id (aget user-status "message-id") + new-message-id (get @old-ids->new-ids message-id) + public-key (aget user-status "public-key") + new-status-id (str new-message-id "-" public-key)] + (when (contains? @old-ids->new-ids message-id) + (aset user-status "status-id" new-status-id) + (aset user-status "message-id" new-message-id)))))) diff --git a/src/status_im/events.cljs b/src/status_im/events.cljs index 8cf43bb952..d85178aa50 100644 --- a/src/status_im/events.cljs +++ b/src/status_im/events.cljs @@ -1082,8 +1082,8 @@ (handlers/register-handler-fx :group-chats.callback/extract-signature-success - (fn [cofx [_ group-update sender-signature]] - (group-chats/handle-membership-update cofx group-update sender-signature))) + (fn [cofx [_ group-update raw-payload sender-signature]] + (group-chats/handle-membership-update cofx group-update raw-payload sender-signature))) ;; profile module diff --git a/src/status_im/group_chats/core.cljs b/src/status_im/group_chats/core.cljs index 1f3c6da2c1..a8c281b56e 100644 --- a/src/status_im/group_chats/core.cljs +++ b/src/status_im/group_chats/core.cljs @@ -17,7 +17,8 @@ [status-im.transport.message.group-chat :as message.group-chat] [status-im.utils.fx :as fx] [status-im.chat.models :as models.chat] - [status-im.accounts.db :as accounts.db])) + [status-im.accounts.db :as accounts.db] + [status-im.transport.message.transit :as transit])) ;; Description of the flow: ;; the flow is complicated a bit by 2 asynchronous call to status-go, which might make the logic a bit more opaque. @@ -99,28 +100,27 @@ "Send a membership update to all participants but the sender" ([cofx payload chat-id] (send-membership-update cofx payload chat-id nil)) - ([cofx payload chat-id removed-members] + ([{:keys [message-id] :as cofx} payload chat-id removed-members] (let [members (clojure.set/union (get-in cofx [:db :chats chat-id :contacts]) removed-members) {:keys [web3]} (:db cofx) current-public-key (accounts.db/current-public-key cofx)] (fx/merge cofx - {:shh/send-group-message {:web3 web3 - :src current-public-key - :dsts members - :success-event [:transport/message-sent - chat-id - (transport.utils/message-id (assoc (:message payload) - :chat-id chat-id - :from current-public-key)) - :group-user-message] - :payload payload}})))) + {:shh/send-group-message + {:web3 web3 + :src current-public-key + :dsts members + :success-event [:transport/message-sent + chat-id + message-id + :group-user-message] + :payload payload}})))) (fx/defn handle-membership-update-received "Extract signatures in status-go and act if successful" - [cofx membership-update signature] - {:group-chats/extract-membership-signature [[membership-update signature]]}) + [cofx membership-update signature raw-payload] + {:group-chats/extract-membership-signature [[membership-update raw-payload signature]]}) (defn chat->group-update "Transform a chat in a GroupMembershipUpdate" @@ -147,19 +147,21 @@ (defn handle-extract-signature-response "Callback to dispatch on extract signature response" - [payload sender-signature response-js] + [payload raw-payload sender-signature response-js] (let [{:keys [error identities]} (parse-response response-js)] (if error (re-frame/dispatch [:group-chats.callback/extract-signature-failed error]) - (re-frame/dispatch [:group-chats.callback/extract-signature-success (add-identities payload identities) sender-signature])))) + (re-frame/dispatch [:group-chats.callback/extract-signature-success + (add-identities payload identities) raw-payload sender-signature])))) (defn sign-membership [{:keys [chat-id events] :as payload}] (native-module/sign-group-membership (signature-material chat-id events) (partial handle-sign-response payload))) -(defn extract-membership-signature [payload sender] - (native-module/extract-group-membership-signatures (signature-pairs payload) - (partial handle-extract-signature-response payload sender))) +(defn extract-membership-signature [payload raw-payload sender] + (native-module/extract-group-membership-signatures + (signature-pairs payload) + (partial handle-extract-signature-response payload raw-payload sender))) (defn- members-added-event [last-clock-value members] {:type "members-added" @@ -404,6 +406,7 @@ [cofx {:keys [chat-id message membership-updates] :as membership-update} + raw-payload sender-signature] (let [dev-mode? (get-in cofx [:db :account/account :dev-mode?])] (when (and config/group-chats-enabled? @@ -426,7 +429,8 @@ ;; don't allow anything but group messages (instance? protocol/Message message) (= :group-user-message (:message-type message))) - (protocol/receive message chat-id sender-signature nil %))))))) + (protocol/receive message chat-id sender-signature nil + (assoc % :js-obj #js {:payload raw-payload})))))))) (defn handle-sign-success "Upsert chat and send signed payload to group members" @@ -435,7 +439,7 @@ updated-chat (update old-chat :membership-updates conj signed-events) my-public-key (accounts.db/current-public-key cofx) group-update (chat->group-update chat-id updated-chat) - new-group-fx (handle-membership-update group-update my-public-key) + new-group-fx (handle-membership-update group-update nil my-public-key) ;; We need to send to users who have been removed as well recipients (clojure.set/union (:contacts old-chat) @@ -453,5 +457,5 @@ (re-frame/reg-fx :group-chats/extract-membership-signature (fn [signatures] - (doseq [[payload sender] signatures] - (extract-membership-signature payload sender)))) + (doseq [[payload raw-payload sender] signatures] + (extract-membership-signature payload raw-payload sender)))) diff --git a/src/status_im/transport/impl/receive.cljs b/src/status_im/transport/impl/receive.cljs index 4b56b31c52..fc886d1ffb 100644 --- a/src/status_im/transport/impl/receive.cljs +++ b/src/status_im/transport/impl/receive.cljs @@ -9,8 +9,8 @@ (extend-type transport.group-chat/GroupMembershipUpdate protocol/StatusMessage - (receive [this _ signature _ cofx] - (group-chats/handle-membership-update-received cofx this signature))) + (receive [this _ signature _ {:keys [js-obj] :as cofx}] + (group-chats/handle-membership-update-received cofx this signature (.-payload js-obj)))) (extend-type transport.contact/ContactRequest protocol/StatusMessage diff --git a/src/status_im/transport/message/contact.cljs b/src/status_im/transport/message/contact.cljs index 9e4fa38e5e..7874500c53 100644 --- a/src/status_im/transport/message/contact.cljs +++ b/src/status_im/transport/message/contact.cljs @@ -1,8 +1,6 @@ (ns ^{:doc "Contact request and update API"} status-im.transport.message.contact (:require [cljs.spec.alpha :as spec] - [status-im.data-store.transport :as transport-store] - [status-im.transport.db :as transport.db] [status-im.transport.message.protocol :as protocol] [status-im.utils.fx :as fx])) diff --git a/src/status_im/transport/message/protocol.cljs b/src/status_im/transport/message/protocol.cljs index 5f388d227f..5599d8887d 100644 --- a/src/status_im/transport/message/protocol.cljs +++ b/src/status_im/transport/message/protocol.cljs @@ -80,17 +80,14 @@ (defrecord Message [content content-type message-type clock-value timestamp] StatusMessage - (send [this chat-id cofx] + (send [this chat-id {:keys [message-id] :as cofx}] (let [dev-mode? (get-in cofx [:db :account/account :dev-mode?]) current-public-key (accounts.db/current-public-key cofx) params {:chat-id chat-id :payload this :success-event [:transport/message-sent chat-id - (transport.utils/message-id - {:from current-public-key - :chat-id chat-id - :clock-value clock-value}) + message-id message-type]}] (case message-type :public-group-user-message @@ -118,9 +115,8 @@ [(assoc (into {} this) :old-message-id (transport.utils/old-message-id this) :message-id (transport.utils/message-id - {:chat-id (:chat-id content) - :from signature - :clock-value clock-value}) + signature + (.-payload (:js-obj cofx))) :chat-id chat-id :from signature :js-obj (:js-obj cofx))]}) diff --git a/src/status_im/transport/utils.cljs b/src/status_im/transport/utils.cljs index 37044b4c9d..6e3b286b91 100644 --- a/src/status_im/transport/utils.cljs +++ b/src/status_im/transport/utils.cljs @@ -21,10 +21,14 @@ [message] (sha3 (pr-str message))) +(defn system-message-id + [{:keys [from chat-id clock-value]}] + (sha3 (str from chat-id clock-value))) + (defn message-id "Get a message-id" - [{:keys [from chat-id clock-value] :as m}] - (sha3 (str from chat-id clock-value))) + [from raw-payload] + (sha3 (str from raw-payload))) (defn get-topic "Get the topic of a group chat or public chat from the chat-id" diff --git a/test/cljs/status_im/test/group_chats/core.cljs b/test/cljs/status_im/test/group_chats/core.cljs index 4e9a848088..8c316af1f5 100644 --- a/test/cljs/status_im/test/group_chats/core.cljs +++ b/test/cljs/status_im/test/group_chats/core.cljs @@ -32,7 +32,7 @@ (with-redefs [config/group-chats-enabled? true] (testing "a brand new chat" (let [actual (-> - (group-chats/handle-membership-update {:now 0 :db {}} initial-message admin) + (group-chats/handle-membership-update {:now 0 :db {}} initial-message "payload" admin) :db :chats (get chat-id))] @@ -66,6 +66,7 @@ (group-chats/handle-membership-update {:now 0 :db {}} (assoc initial-message :chat-id bad-chat-id) + "payload" admin) :db :chats @@ -74,10 +75,10 @@ (is (not actual))))) (testing "an already existing chat" (let [cofx (assoc - (group-chats/handle-membership-update {:now 0 :db {}} initial-message admin) + (group-chats/handle-membership-update {:now 0 :db {}} initial-message "payload" admin) :now 0)] (testing "the message has already been received" - (let [actual (group-chats/handle-membership-update cofx initial-message admin)] + (let [actual (group-chats/handle-membership-update cofx initial-message "payload" admin)] (testing "it noops" (is (= (get-in cofx [:db :chats chat-id]) @@ -105,6 +106,7 @@ {:type "name-changed" :clock-value 13 :name "new-name"}]}]} + "payload" member-3) actual-chat (get-in actual [:db :chats chat-id])] (testing "the chat is updated"