From d760f1696cd948af4ade4454965f310d58940ed8 Mon Sep 17 00:00:00 2001 From: Andrea Maria Piana Date: Thu, 6 Dec 2018 11:53:45 +0100 Subject: [PATCH] Add mailservers confirmations & use peer count for online status We now check that we are only connected to some `peers` instead of using `NetInfo` from `react-native`. This is because it has been reported to be quite flaky at times, not reporting online status after sleeping, and for privacy concerns (on ios it pings `apple.com`, on desktop `google.com`). Adds a new banner `Wallet Offline` and change `Connecting to peers` to `Chat offline`. A message will be marked as `Sent` only if it made it to the mailserver you are connected to, which will increase the guarantees that we can make about a message (if you see it as sent, it has reached at least a mailserver), this has the consequence that: - If you are not connected to any mailserver or the mailserver is non responsive/down, and you send a message, it will be marked as `Not sent`, although it might have been actually made it in the network. Probably this is something that we would like to communicate to the user through UX (i.e. tick if made it to at least a peer, double tick if it made to a mailserver ) Currently I have only enabled this feature in nightlies & devs, I would give it a run and see how we feel about it. --- .env | 1 + .env.e2e | 1 + .env.jenkins | 1 + .env.nightly | 1 + .env.nightly.staging.fleet | 1 + .../status/ethereum/module/StatusModule.java | 20 +++++++++++++++++++ .../react-native-status/desktop/rctstatus.cpp | 10 ++++++++++ .../react-native-status/desktop/rctstatus.h | 1 + .../ios/RCTStatus/RCTStatus.m | 10 ++++++++++ src/status_im/chat/models/message.cljs | 4 ++-- src/status_im/mailserver/core.cljs | 17 ++++++++++++++++ src/status_im/native_module/core.cljs | 2 ++ src/status_im/native_module/impl/module.cljs | 4 ++++ src/status_im/node/core.cljs | 19 +++++++++--------- .../ui/components/connectivity/view.cljs | 4 +++- .../ui/screens/chat/input/send_button.cljs | 18 ++++++++--------- .../ui/screens/desktop/main/chat/views.cljs | 11 +++++----- src/status_im/utils/config.cljs | 1 + test/cljs/status_im/test/mailserver/core.cljs | 4 ++-- translations/en.json | 3 ++- 20 files changed, 103 insertions(+), 30 deletions(-) diff --git a/.env b/.env index 51a1ec153a..4cb3730438 100644 --- a/.env +++ b/.env @@ -16,3 +16,4 @@ HARDWALLET_ENABLED=0 PFS_ENCRYPTION_ENABLED=0 DEV_BUILD=1 ERC20_CONTRACT_WARNINGS=1 +MAILSERVER_CONFIRMATIONS_ENABLED=1 diff --git a/.env.e2e b/.env.e2e index 53d2e1a0bc..27df101b4f 100644 --- a/.env.e2e +++ b/.env.e2e @@ -12,3 +12,4 @@ PAIRING_ENABLED=1 EXTENSIONS=1 PFS_ENCRYPTION_ENABLED=0 ERC20_CONTRACT_WARNINGS=1 +MAILSERVER_CONFIRMATIONS_ENABLED=1 diff --git a/.env.jenkins b/.env.jenkins index 584e1fad24..214d61510f 100644 --- a/.env.jenkins +++ b/.env.jenkins @@ -15,3 +15,4 @@ EXTENSIONS=1 PFS_ENCRYPTION_ENABLED=0 PAIRING_ENABLED=1 ERC20_CONTRACT_WARNINGS=1 +MAILSERVER_CONFIRMATIONS_ENABLED=1 diff --git a/.env.nightly b/.env.nightly index edc0696e6e..bd91eae6d8 100644 --- a/.env.nightly +++ b/.env.nightly @@ -14,3 +14,4 @@ MAINNET_WARNING_ENABLED=1 EXTENSIONS=1 PFS_ENCRYPTION_ENABLED=0 ERC20_CONTRACT_WARNINGS=1 +MAILSERVER_CONFIRMATIONS_ENABLED=1 diff --git a/.env.nightly.staging.fleet b/.env.nightly.staging.fleet index 61f28668cb..cf78e74aec 100644 --- a/.env.nightly.staging.fleet +++ b/.env.nightly.staging.fleet @@ -13,3 +13,4 @@ MAINNET_WARNING_ENABLED=1 EXTENSIONS=1 PFS_ENCRYPTION_ENABLED=0 ERC20_CONTRACT_WARNINGS=1 +MAILSERVER_CONFIRMATIONS_ENABLED=1 diff --git a/modules/react-native-status/android/src/main/java/im/status/ethereum/module/StatusModule.java b/modules/react-native-status/android/src/main/java/im/status/ethereum/module/StatusModule.java index 86a63c1228..c6dd2681f1 100644 --- a/modules/react-native-status/android/src/main/java/im/status/ethereum/module/StatusModule.java +++ b/modules/react-native-status/android/src/main/java/im/status/ethereum/module/StatusModule.java @@ -791,6 +791,26 @@ class StatusModule extends ReactContextBaseJavaModule implements LifecycleEventL StatusThreadPoolExecutor.getInstance().execute(r); } + @ReactMethod + public void updateMailservers(final String enodes, final Callback callback) { + Log.d(TAG, "updateMailservers"); + if (!checkAvailability()) { + callback.invoke(false); + return; + } + + Runnable r = new Runnable() { + @Override + public void run() { + String res = Statusgo.UpdateMailservers(enodes); + + callback.invoke(res); + } + }; + + StatusThreadPoolExecutor.getInstance().execute(r); + } + @Override public @Nullable Map getConstants() { diff --git a/modules/react-native-status/desktop/rctstatus.cpp b/modules/react-native-status/desktop/rctstatus.cpp index bf865e60db..4f5c17634a 100644 --- a/modules/react-native-status/desktop/rctstatus.cpp +++ b/modules/react-native-status/desktop/rctstatus.cpp @@ -319,3 +319,13 @@ void RCTStatus::logStatusGoResult(const char* methodName, const char* result) qCWarning(RCTSTATUS) << methodName << "- error:" << qUtf8Printable(error); } } + +void RCTStatus::updateMailservers(QString enodes, double callbackId) { + Q_D(RCTStatus); + qCDebug(RCTSTATUS) << "::updateMailservers call - callbackId:" << callbackId; + QtConcurrent::run([&](QString enodes, double callbackId) { + const char* result = UpdateMailservers(enodes.toUtf8().data()); + logStatusGoResult("::updateMailservers UpdateMailservers", result); + d->bridge->invokePromiseCallback(callbackId, QVariantList{result}); + }, enodes, callbackId); +} diff --git a/modules/react-native-status/desktop/rctstatus.h b/modules/react-native-status/desktop/rctstatus.h index 0c425dda45..5f5c20ce8e 100644 --- a/modules/react-native-status/desktop/rctstatus.h +++ b/modules/react-native-status/desktop/rctstatus.h @@ -48,6 +48,7 @@ public: Q_INVOKABLE void extractGroupMembershipSignatures(QString signatures, double callbackId); Q_INVOKABLE void enableInstallation(QString installationId, double callbackId); Q_INVOKABLE void disableInstallation(QString installationId, double callbackId); + Q_INVOKABLE void updateMailservers(QString enodes, double callbackId); Q_INVOKABLE void setAdjustResize(); Q_INVOKABLE void setAdjustPan(); diff --git a/modules/react-native-status/ios/RCTStatus/RCTStatus.m b/modules/react-native-status/ios/RCTStatus/RCTStatus.m index c24d93e110..7dc3e6d06b 100644 --- a/modules/react-native-status/ios/RCTStatus/RCTStatus.m +++ b/modules/react-native-status/ios/RCTStatus/RCTStatus.m @@ -206,6 +206,16 @@ RCT_EXPORT_METHOD(addPeer:(NSString *)enode #endif } +//////////////////////////////////////////////////////////////////// updateMailservers +RCT_EXPORT_METHOD(updateMailservers:(NSString *)enodes + callback:(RCTResponseSenderBlock)callback) { + char * result = UpdateMailservers((char *) [enodes UTF8String]); + callback(@[[NSString stringWithUTF8String: result]]); +#if DEBUG + NSLog(@"UpdateMailservers() method called"); +#endif +} + //////////////////////////////////////////////////////////////////// recoverAccount RCT_EXPORT_METHOD(recoverAccount:(NSString *)passphrase password:(NSString *)password diff --git a/src/status_im/chat/models/message.cljs b/src/status_im/chat/models/message.cljs index ac39e4e8ac..2352051bb0 100644 --- a/src/status_im/chat/models/message.cljs +++ b/src/status_im/chat/models/message.cljs @@ -316,8 +316,8 @@ ;;;; Send message (fx/defn send - [{{:keys [network-status]} :db :as cofx} chat-id message-id send-record] - (if (= network-status :offline) + [{{:keys [peers-count]} :db :as cofx} chat-id message-id send-record] + (if (zero? peers-count) {:dispatch-later [{:ms 10000 :dispatch [:message/update-message-status chat-id message-id :not-sent]}]} (protocol/send send-record chat-id (assoc cofx :message-id message-id)))) diff --git a/src/status_im/mailserver/core.cljs b/src/status_im/mailserver/core.cljs index 87e8d1be9d..fb79af0dc0 100644 --- a/src/status_im/mailserver/core.cljs +++ b/src/status_im/mailserver/core.cljs @@ -127,6 +127,15 @@ (response-handler #(log/debug "mailserver: add-peer success" %) #(log/error "mailserver: add-peer error" %)))) +;; We now wait for a confirmation from the mailserver before marking the message +;; as sent. + +(defn update-mailservers! [enodes] + (status/update-mailservers + (.stringify js/JSON (clj->js enodes)) + (response-handler #(log/debug "mailserver: update-mailservers success" %) + #(log/error "mailserver: update-mailservers error" %)))) + (defn remove-peer! [enode] (let [args {:jsonrpc "2.0" :id 2 @@ -147,6 +156,11 @@ (fn [enode] (remove-peer! enode))) +(re-frame/reg-fx + :mailserver/update-mailservers + (fn [enodes] + (update-mailservers! enodes))) + (defn mark-trusted-peer! [web3 enode] (.markTrustedPeer (transport.utils/shh web3) enode @@ -198,6 +212,9 @@ (update-mailserver-state :connecting) (update :mailserver/connection-checks inc)) :mailserver/add-peer address + ;; Any message sent before this takes effect will not be marked as sent + ;; probably we should improve the UX so that is more transparent to the user + :mailserver/update-mailservers [address] :utils/dispatch-later [{:ms connection-timeout :dispatch [:mailserver/check-connection-timeout]}]} (when-not (or sym-key-id generating-sym-key?) diff --git a/src/status_im/native_module/core.cljs b/src/status_im/native_module/core.cljs index 5d6656035d..0d44e51203 100644 --- a/src/status_im/native_module/core.cljs +++ b/src/status_im/native_module/core.cljs @@ -73,3 +73,5 @@ (def enable-installation native-module/enable-installation) (def disable-installation native-module/disable-installation) + +(def update-mailservers native-module/update-mailservers) diff --git a/src/status_im/native_module/impl/module.cljs b/src/status_im/native_module/impl/module.cljs index 10a0279c2a..d10314966d 100644 --- a/src/status_im/native_module/impl/module.cljs +++ b/src/status_im/native_module/impl/module.cljs @@ -163,3 +163,7 @@ (defn is24Hour [] (when status (.-is24Hour status))) + +(defn update-mailservers [enodes on-result] + (when status + (call-module #(.updateMailservers status enodes on-result)))) diff --git a/src/status_im/node/core.cljs b/src/status_im/node/core.cljs index 47f270495f..ae1b47c98f 100644 --- a/src/status_im/node/core.cljs +++ b/src/status_im/node/core.cljs @@ -100,18 +100,19 @@ :RendezvousNodes rendezvous-nodes}) :always - (assoc :WhisperConfig {:Enabled true - :LightClient true - :MinimumPoW 0.001 - :EnableNTPSync true} - :RequireTopics (get-topics network) - :InstallationID installation-id - :PFSEnabled (or config/pfs-encryption-enabled? + (assoc :WhisperConfig {:Enabled true + :LightClient true + :MinimumPoW 0.001 + :EnableNTPSync true} + :RequireTopics (get-topics network) + :InstallationID installation-id + :MailServerConfirmations config/mailserver-confirmations-enabled? + :PFSEnabled (or config/pfs-encryption-enabled? ;; We don't check dev-mode? here as ;; otherwise we would have to restart the node ;; when the user enables it - config/group-chats-enabled? - (config/pairing-enabled? true))) + config/group-chats-enabled? + (config/pairing-enabled? true))) (and config/bootnodes-settings-enabled? diff --git a/src/status_im/ui/components/connectivity/view.cljs b/src/status_im/ui/components/connectivity/view.cljs index 09607896d7..83d6cc0f2a 100644 --- a/src/status_im/ui/components/connectivity/view.cljs +++ b/src/status_im/ui/components/connectivity/view.cljs @@ -39,7 +39,9 @@ view-id [:get :view-id] window-width [:dimensions/window-width]] (when-let [label (cond - offline? :t/offline + (and offline? + disconnected?) :t/offline + offline? :t/wallet-offline disconnected? :t/disconnected mailserver-connection-error? :t/mailserver-reconnect mailserver-request-error? :t/mailserver-request-error-status diff --git a/src/status_im/ui/screens/chat/input/send_button.cljs b/src/status_im/ui/screens/chat/input/send_button.cljs index 8eccbb7db8..b4b49a325a 100644 --- a/src/status_im/ui/screens/chat/input/send_button.cljs +++ b/src/status_im/ui/screens/chat/input/send_button.cljs @@ -14,22 +14,22 @@ (animation/timing spin-value {:toValue to-spin-value :duration 300}))))) -(defn sendable? [input-text offline? login-processing?] +(defn sendable? [input-text disconnected? login-processing?] (let [trimmed (string/trim input-text)] (not (or (string/blank? trimmed) (= trimmed "/") - offline? - login-processing?)))) + login-processing? + disconnected?)))) (defview send-button-view [] - (letsubs [{:keys [command-completion]} [:chats/selected-chat-command] - {:keys [input-text]} [:chats/current-chat] - offline? [:offline?] - spin-value (animation/create-value 1) - login-processing? [:get-in [:accounts/login :processing]]] + (letsubs [{:keys [command-completion]} [:chats/selected-chat-command] + {:keys [input-text seq-arg-input-text]} [:chats/current-chat] + disconnected? [:disconnected?] + login-processing? [:get-in [:accounts/login :processing]] + spin-value (animation/create-value 1)] {:component-did-update (send-button-view-on-update {:spin-value spin-value :command-completion command-completion})} - (when (and (sendable? input-text offline? login-processing?) + (when (and (sendable? input-text disconnected? login-processing?) (or (not command-completion) (#{:complete :less-than-needed} command-completion))) [react/touchable-highlight {:on-press #(re-frame/dispatch [:chat.ui/send-current-message])} diff --git a/src/status_im/ui/screens/desktop/main/chat/views.cljs b/src/status_im/ui/screens/desktop/main/chat/views.cljs index 63f8006e9a..9b6544da5d 100644 --- a/src/status_im/ui/screens/desktop/main/chat/views.cljs +++ b/src/status_im/ui/screens/desktop/main/chat/views.cljs @@ -238,11 +238,10 @@ :current-public-key current-public-key)]))]] [connectivity/error-view]]))) -(views/defview send-button [inp-ref network-status] +(views/defview send-button [inp-ref disconnected?] (views/letsubs [{:keys [input-text]} [:chats/current-chat]] (let [empty? (= "" input-text) - offline? (= :offline network-status) - inactive? (or empty? offline?)] + inactive? (or empty? disconnected?)] [react/touchable-highlight {:style styles/send-button :disabled inactive? :on-press (fn [] @@ -284,7 +283,7 @@ (views/defview chat-text-input [chat-id input-text] (views/letsubs [inp-ref (atom nil) - network-status [:network-status]] + disconnected? [:disconnected?]] {:component-will-update (fn [e [_ new-chat-id new-input-text]] (let [[_ old-chat-id] (.. e -props -argv)] @@ -305,7 +304,7 @@ :default-value input-text :on-content-size-change #(set-container-height-fn (.-height (.-contentSize (.-nativeEvent %)))) :submit-shortcut {:key "Enter"} - :on-submit-editing #(when (= :online network-status) + :on-submit-editing #(when-not disconnected? (.clear @inp-ref) (.focus @inp-ref) (re-frame/dispatch [:chat.ui/send-current-message])) @@ -313,7 +312,7 @@ (let [native-event (.-nativeEvent e) text (.-text native-event)] (re-frame/dispatch [:chat.ui/set-chat-input-text text])))}] - [send-button inp-ref network-status]]))) + [send-button inp-ref disconnected?]]))) (views/defview chat-view [] (views/letsubs [{:keys [input-text chat-id] :as current-chat} [:chats/current-chat]] diff --git a/src/status_im/utils/config.cljs b/src/status_im/utils/config.cljs index 349db7347a..bd1ae352ee 100644 --- a/src/status_im/utils/config.cljs +++ b/src/status_im/utils/config.cljs @@ -21,6 +21,7 @@ (defn pairing-enabled? [dev-mode?] (and (enabled? (get-config :PAIRING_ENABLED "0")) (or dev-mode? platform/desktop?))) +(def mailserver-confirmations-enabled? (enabled? (get-config :MAILSERVER_CONFIRMATIONS_ENABLED))) (def mainnet-warning-enabled? (enabled? (get-config :MAINNET_WARNING_ENABLED 0))) (def pfs-encryption-enabled? (enabled? (get-config :PFS_ENCRYPTION_ENABLED "0"))) (def in-app-notifications-enabled? (enabled? (get-config :IN_APP_NOTIFICATIONS_ENABLED 0))) diff --git a/test/cljs/status_im/test/mailserver/core.cljs b/test/cljs/status_im/test/mailserver/core.cljs index 47f096a172..bf9e5f7629 100644 --- a/test/cljs/status_im/test/mailserver/core.cljs +++ b/test/cljs/status_im/test/mailserver/core.cljs @@ -321,13 +321,13 @@ (testing "Mailserver disconnected, sym-key exists" (let [result (peers-summary-change-result true false true)] (is (= (into #{} (keys result)) - #{:db :mailserver/add-peer :utils/dispatch-later})) + #{:db :mailserver/add-peer :utils/dispatch-later :mailserver/update-mailservers})) (is (= (get-in result [:db :mailserver/state]) :connecting)))) (testing "Mailserver disconnected, sym-key doesn't exists (unlikely situation in practice)" (let [result (peers-summary-change-result false false true)] (is (= (into #{} (keys result)) - #{:db :mailserver/add-peer :utils/dispatch-later :shh/generate-sym-key-from-password})) + #{:db :mailserver/add-peer :utils/dispatch-later :shh/generate-sym-key-from-password :mailserver/update-mailservers})) (is (= (get-in result [:db :mailserver/state]) :connecting)))) (testing "Mailserver isn't concerned by peer summary changes" diff --git a/translations/en.json b/translations/en.json index 849e24b7a2..34df1d285d 100644 --- a/translations/en.json +++ b/translations/en.json @@ -165,7 +165,8 @@ "datetime-yesterday": "yesterday", "create-new-account": "Create new account", "are-you-sure?": "Are you sure?", - "disconnected": "Connecting to peers...", + "disconnected": "Chat offline", + "wallet-offline": "Wallet offline", "sign-in-to-status": "Sign in to Status", "leave-group-chat-confirmation": "Are you sure you want to leave this group?", "dapp-profile": "ÐApp profile",