[fix 4466] improve peer-summary-change-fx

do not request messages from inbox everytime a peer is added or removed

Signed-off-by: Eric Dvorsak <eric@dvorsak.fr>

src/status_im/transport/core.cljs

@@ -34,7 +34,7 @@
                                 :shh/restore-sym-keys {:web3       web3
                                                        :transport  (:transport/chats db)
                                                        :on-success sym-key-added-callback}}
-                               (inbox/peers-summary-change-fx)))))
+                               (inbox/connect-to-mailserver)))))
 
 ;;TODO (yenda) remove once go implements persistence
 ;;Since symkeys are not persisted, we restore them via add sym-keys,

src/status_im/transport/inbox.cljs

@@ -73,7 +73,8 @@
                         (success-fn resp)
                         (error-fn err)))))
 
-(defn request-inbox-messages [web3 wnode topics to from sym-key-id success-fn error-fn]
+(defn request-inbox-messages
+  [web3 wnode topics to from sym-key-id success-fn error-fn]
   (let [opts (merge {:mailServerPeer wnode
                      :symKeyID       sym-key-id}
                     (when from {:from from})
@@ -136,34 +137,46 @@
    A connection-check is made after `connection timeout` is reached and
    mailserver-status is changed to error if it is not connected by then"
   [{:keys [db] :as cofx}]
-  (let [web3     (:web3 db)
+  (let [web3          (:web3 db)
-        wnode    (get-current-wnode-address db)]
+        wnode         (get-current-wnode-address db)
+        peers-summary (:peers-summary db)
+        connected?    (registered-peer? peers-summary wnode)]
     (when config/offline-inbox-enabled?
-      (handlers-macro/merge-fx cofx
+      (if connected?
-                               {::add-peer {:wnode wnode}
+        (handlers-macro/merge-fx cofx
-                                :utils/dispatch-later [{:ms connection-timeout
+                                 (update-mailserver-status :connected)
-                                                        :dispatch [:inbox/connection-check]}]}
+                                 (generate-mailserver-symkey))
-                               (update-mailserver-status :connecting)
+        (handlers-macro/merge-fx cofx
-                               (generate-mailserver-symkey)))))
+                                 {::add-peer {:wnode wnode}
+                                  :utils/dispatch-later [{:ms connection-timeout
+                                                          :dispatch [:inbox/connection-check]}]}
+                                 (update-mailserver-status :connecting)
+                                 (generate-mailserver-symkey))))))
 
 (defn peers-summary-change-fx
-  "Called when a peer summary signal is received or we want to try to connect
+  "There is only 2 summary changes that require offline inboxing action:
-   to mailserver ie. after login.
+  - mailserver disconnected: we try to reconnect
-   Marks the mailserver as trusted peer if it is already added to the list of
+  - mailserver connected: we mark the mailserver as trusted peer"
-   peers or connects to it if it's not the case"
+  [previous-summary {:keys [db] :as cofx}]
-  [{:keys [db] :as cofx}]
   (when (and (:account/account db)
              config/offline-inbox-enabled?)
-    (let [{:keys [peers-summary peers-count]} db]
+    (let [{:keys [peers-summary peers-count]} db
-      (if (zero? peers-count)
+          wnode                               (get-current-wnode-address db)
-        (update-mailserver-status :disconnected cofx)
+          mailserver-was-registered?          (registered-peer? previous-summary
-        (let [wnode (get-current-wnode-address db)]
+                                                                wnode)
-          (if (registered-peer? peers-summary wnode)
+          mailserver-is-registered?           (registered-peer? peers-summary
-            (handlers-macro/merge-fx cofx
+                                                                wnode)
-                                     {::mark-trusted-peer {:web3  (:web3 db)
+          mailserver-connected?               (and mailserver-is-registered?
-                                                           :wnode wnode}}
+                                                   (not mailserver-was-registered?))
-                                     (generate-mailserver-symkey))
+          mailserver-disconnected?            (and mailserver-was-registered?
-            (connect-to-mailserver cofx)))))))
+                                                   (not mailserver-is-registered?))]
+      (cond
+        mailserver-disconnected?
+        (connect-to-mailserver cofx)
+
+        mailserver-connected?
+        {::mark-trusted-peer {:web3  (:web3 db)
+                              :wnode wnode}}))))
 
 (defn get-topics
   [db topics discover?]

src/status_im/ui/screens/events.cljs

@@ -405,12 +405,13 @@
 (handlers/register-handler-fx
  :discovery/summary
  (fn [{:keys [db] :as cofx} [_ peers-summary]]
-   (let [peers-count (count peers-summary)]
+   (let [previous-summary (:peers-summary db)
+         peers-count      (count peers-summary)]
      (handlers-macro/merge-fx cofx
                               {:db (assoc db
                                           :peers-summary peers-summary
                                           :peers-count peers-count)}
-                              (inbox/peers-summary-change-fx)))))
+                              (inbox/peers-summary-change-fx previous-summary)))))
 
 (handlers/register-handler-fx
  :signal-event

test/cljs/status_im/test/transport/inbox.cljs

@@ -37,11 +37,10 @@
             :dispatch-later
             [{:ms 5000, :dispatch [:inbox/remove-fetching-notification]}]}))))
 
-(defn cofx-fixtures [sym-key peers-count registered-peer?]
+(defn cofx-fixtures [sym-key registered-peer?]
   {:db {:mailserver-status :connected
         :inbox/sym-key-id sym-key
         :network "mainnet_rpc"
-        :peers-count peers-count
         :peers-summary (if registered-peer?
                          [{:id "wnode-id"}]
                          [])
@@ -49,32 +48,32 @@
                           :settings {:wnode {:mainnet "mailserver-a"}}}
         :inbox/wnodes {:mainnet {"mailserver-a" {:address "enode://wnode-id@ip"}}}}})
 
-(defn peers-summary-change-fx-result [sym-key peers-count registered-peer?]
+(defn peers-summary-change-fx-result [sym-key registered-peer? registered-peer-before?]
-  (update-in (inbox/peers-summary-change-fx (cofx-fixtures sym-key peers-count registered-peer?))
+  (inbox/peers-summary-change-fx (if registered-peer-before?
-             [:db] dissoc :account/account :inbox/wnodes :peers-summary :peers-count))
+                                   [{:id "wnode-id"}]
+                                   [])
+                                 (cofx-fixtures sym-key
+                                                registered-peer?)))
 
 (deftest peers-summary-change-fx
-  (testing "Mailserver is connected and sym-key doesn't exists"
+  (testing "Mailserver connected"
-    (let [result (peers-summary-change-fx-result false 1 true)]
+    (let [result (peers-summary-change-fx-result false true false)]
       (is (= (into #{} (keys result))
-             #{:db :status-im.transport.inbox/mark-trusted-peer :shh/generate-sym-key-from-password}))))
+             #{:status-im.transport.inbox/mark-trusted-peer}))))
-  (testing "Mailserver is connected and sym-key exists"
+  (testing "Mailserver disconnected, sym-key exists"
-    (let [result (peers-summary-change-fx-result true 1 true)]
+    (let [result (peers-summary-change-fx-result true false true)]
-      (is (= (into #{} (keys result))
-             #{:db :status-im.transport.inbox/mark-trusted-peer}))))
-  (testing "Mailserver is not connected and sym-key doesn't exists"
-    (let [result (peers-summary-change-fx-result false 1 false)]
-      (is (= (into #{} (keys result))
-             #{:db :status-im.transport.inbox/add-peer :utils/dispatch-later :shh/generate-sym-key-from-password}))
-      (is (= (get-in result [:db :mailserver-status])
-             :connecting))))
-  (testing "Mailserver is not connected and sym-key exists"
-    (let [result (peers-summary-change-fx-result true 1 false)]
       (is (= (into #{} (keys result))
              #{:db :status-im.transport.inbox/add-peer :utils/dispatch-later}))
       (is (= (get-in result [:db :mailserver-status])
              :connecting))))
-  (testing "App is not connected to any peer"
+  (testing "Mailserver disconnected, sym-key doesn't exists (unlikely situation in practice)"
-    (is (= (get-in (peers-summary-change-fx-result true 0 true)
+    (let [result (peers-summary-change-fx-result false false true)]
-                   [:db :mailserver-status])
+      (is (= (into #{} (keys result))
-           :disconnected))))
+             #{:db :status-im.transport.inbox/add-peer :utils/dispatch-later  :shh/generate-sym-key-from-password}))
+      (is (= (get-in result [:db :mailserver-status])
+             :connecting))))
+  (testing "Mailserver isn't concerned by peer summary changes"
+    (is (= (into #{} (keys (peers-summary-change-fx-result true true true)))
+           #{}))
+    (is (= (into #{} (keys (peers-summary-change-fx-result true false false)))
+           #{}))))