Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

throw errors on dangerous behaviours #9032

Merged
merged 1 commit into from
Sep 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions src/status_im/multiaccounts/update/core.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
[status-im.transport.message.contact :as message.contact]
[status-im.transport.message.protocol :as protocol]
[status-im.utils.fx :as fx]
[status-im.utils.types :as types]))
[status-im.utils.types :as types]
[taoensso.timbre :as log]))

(fx/defn multiaccount-update-message [{:keys [db] :as cofx}]
(let [multiaccount (:multiaccount db)
Expand Down Expand Up @@ -53,11 +54,15 @@
:params ["multiaccount" (types/serialize new-multiaccount)]
:on-success on-success}]}
{:keys [name photo-path prefered-name]} new-multiaccount-fields]
(if (or name photo-path prefered-name)
(fx/merge cofx
fx
(send-multiaccount-update))
fx)))
(if (empty? current-multiaccount)
;; NOTE: this should never happen, but if it does this is a critical error
;; and it is better to crash than risk having an unstable state
(throw (js/Error. "Please shake the phone to report this error and restart the app. multiaccount is currently empty, which means something went wrong when trying to update it with"))
(if (or name photo-path prefered-name)
(fx/merge cofx
fx
(send-multiaccount-update))
fx))))

(fx/defn clean-seed-phrase
"A helper function that removes seed phrase from storage."
Expand All @@ -72,8 +77,12 @@
settings
{:keys [on-success] :or {on-success #()}}]
(let [new-multiaccount (assoc multiaccount :settings settings)]
{:db (assoc db :multiaccount new-multiaccount)
::json-rpc/call
[{:method "settings_saveConfig"
:params ["multiaccount" (types/serialize new-multiaccount)]
:on-success on-success}]}))
(if (empty? multiaccount)
;; NOTE: this should never happen, but if it does this is a critical error
;; and it is better to crash than risk having an unstable state
(throw (js/Error. "Please shake the phone to report this error and restart the app. multiaccount is currently empty, which means something went wrong when trying to update settings"))
{:db (assoc db :multiaccount new-multiaccount)
::json-rpc/call
[{:method "settings_saveConfig"
:params ["multiaccount" (types/serialize new-multiaccount)]
:on-success on-success}]})))
2 changes: 1 addition & 1 deletion src/status_im/utils/fx.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
(if (get merged-fx k)
(if (mergeable-keys k)
(update merged-fx k into v)
(do (log/error "Merging fx with common-key: " k v)
(do (log/error "Merging fx with common-key: " k v (get merged-fx k))
(reduced {:merging-fx-with-common-keys k})))
(assoc merged-fx k v))))
fx
Expand Down
2 changes: 1 addition & 1 deletion test/cljs/status_im/test/models/bootnode.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
{:random-id-generator (constantly "some-id")
:db {:bootnodes/manage new-bootnode
:network "mainnet_rpc"
:multiaccount {}}})]
:multiaccount {:not-empty "would throw an error if was empty"}}})]
(is (= expected (get-in actual [:db :multiaccount :bootnodes])))))
(testing "adding an existing bootnode"
(let [new-bootnode {:id {:value "a"}
Expand Down
8 changes: 6 additions & 2 deletions test/cljs/status_im/test/multiaccounts/update/core.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@
[status-im.multiaccounts.update.core :as multiaccounts.update]))

(deftest test-multiaccount-update
(let [efx (multiaccounts.update/multiaccount-update {:db {:multiaccount {}}} nil {})
;;TODO this test case actually shows that we are doing a needless rpc call when
;;there is no changes, but it is an edge case that shouldn't really happen
(let [efx (multiaccounts.update/multiaccount-update
{:db {:multiaccount {:not-empty "would throw an error if was empty"}}}
nil {})
json-rpc (into #{} (map :method (::json-rpc/call efx)))]
(is (json-rpc "settings_saveConfig"))
(is (= (get-in efx [:db :multiaccount]) {}))))
(is (= (get-in efx [:db :multiaccount]) {:not-empty "would throw an error if was empty"}))))

(deftest test-clean-seed-phrase
(let [efx (multiaccounts.update/clean-seed-phrase {:db {:multiaccount {:mnemonic "lalalala"}}})
Expand Down