From a5d5ed3596ac380f63dd69991b1e3eb6af4ecf46 Mon Sep 17 00:00:00 2001 From: Goran Jovic Date: Fri, 20 Jul 2018 15:12:07 +0200 Subject: [PATCH] bug #5171 - fixed incorrect amount validation in wallet request Signed-off-by: Goran Jovic --- src/status_im/ui/screens/wallet/db.cljs | 14 +++-- .../ui/screens/wallet/request/events.cljs | 2 +- .../ui/screens/wallet/request/views.cljs | 2 +- test/cljs/status_im/test/runner.cljs | 4 +- .../status_im/test/ui/screens/wallet/db.cljs | 52 +++++++++++++++++++ 5 files changed, 66 insertions(+), 8 deletions(-) create mode 100644 test/cljs/status_im/test/ui/screens/wallet/db.cljs diff --git a/src/status_im/ui/screens/wallet/db.cljs b/src/status_im/ui/screens/wallet/db.cljs index 78420a9234..8c64f859a2 100644 --- a/src/status_im/ui/screens/wallet/db.cljs +++ b/src/status_im/ui/screens/wallet/db.cljs @@ -1,6 +1,5 @@ (ns status-im.ui.screens.wallet.db - (:require [clojure.string :as string] - [cljs.spec.alpha :as spec] + (:require [cljs.spec.alpha :as spec] [status-im.i18n :as i18n] status-im.ui.screens.wallet.request.db status-im.ui.screens.wallet.send.db @@ -16,9 +15,14 @@ ;; Placeholder namespace for wallet specs, which are a WIP depending on data ;; model we decide on for balances, prices, etc. -(defn- too-precise-amount? [amount decimals] - (let [amount-splited (string/split amount #"[.]")] - (and (= (count amount-splited) 2) (> (count (last amount-splited)) decimals)))) + +(defn- too-precise-amount? + "Checks if number has any extra digit beyond the allowed number of decimals. + It does so by checking the number against its rounded value." + [amount decimals] + (let [bn (money/bignumber amount)] + (not (.eq bn + (.round bn decimals))))) (defn parse-amount [amount decimals] (when-not (empty? amount) diff --git a/src/status_im/ui/screens/wallet/request/events.cljs b/src/status_im/ui/screens/wallet/request/events.cljs index af48893387..2a0495ffb6 100644 --- a/src/status_im/ui/screens/wallet/request/events.cljs +++ b/src/status_im/ui/screens/wallet/request/events.cljs @@ -35,7 +35,7 @@ (handlers/register-handler-fx :wallet.request/set-and-validate-amount (fn [{:keys [db]} [_ amount symbol decimals]] - (let [{:keys [value error]} (wallet-db/parse-amount amount symbol)] + (let [{:keys [value error]} (wallet-db/parse-amount amount decimals)] {:db (-> db (assoc-in [:wallet :request-transaction :amount] (money/formatted->internal value symbol decimals)) (assoc-in [:wallet :request-transaction :amount-text] amount) diff --git a/src/status_im/ui/screens/wallet/request/views.cljs b/src/status_im/ui/screens/wallet/request/views.cljs index 0454a0a774..ffc7bd1313 100644 --- a/src/status_im/ui/screens/wallet/request/views.cljs +++ b/src/status_im/ui/screens/wallet/request/views.cljs @@ -53,7 +53,7 @@ token]]] [bottom-buttons/bottom-buttons styles/bottom-buttons nil ;; Force a phantom button to ensure consistency with other transaction screens which define 2 buttons - [button/button {:disabled? (not (and to amount)) + [button/button {:disabled? (or amount-error (not (and to amount))) :on-press #(re-frame/dispatch [:wallet-send-request whisper-identity amount symbol decimals]) :text-style {:padding-horizontal 0} :accessibility-label :sent-request-button} diff --git a/test/cljs/status_im/test/runner.cljs b/test/cljs/status_im/test/runner.cljs index fa47a69d4f..e3a404a8fc 100644 --- a/test/cljs/status_im/test/runner.cljs +++ b/test/cljs/status_im/test/runner.cljs @@ -46,7 +46,8 @@ [status-im.test.utils.universal-links.core] [status-im.test.utils.http] [status-im.test.ui.screens.events] - [status-im.test.ui.screens.accounts.login.events])) + [status-im.test.ui.screens.accounts.login.events] + [status-im.test.ui.screens.wallet.db])) (enable-console-print!) @@ -103,4 +104,5 @@ 'status-im.test.utils.http 'status-im.test.ui.screens.events 'status-im.test.ui.screens.accounts.login.events + 'status-im.test.ui.screens.wallet.db 'status-im.test.browser.events) diff --git a/test/cljs/status_im/test/ui/screens/wallet/db.cljs b/test/cljs/status_im/test/ui/screens/wallet/db.cljs new file mode 100644 index 0000000000..29fb2ac34d --- /dev/null +++ b/test/cljs/status_im/test/ui/screens/wallet/db.cljs @@ -0,0 +1,52 @@ +(ns status-im.test.ui.screens.wallet.db + (:require [cljs.test :refer-macros [deftest is testing]] + [status-im.ui.screens.wallet.db :as wallet.db] + [status-im.utils.money :as money] + [status-im.i18n :as i18n])) + +(deftest test-too-precise-amount? + (testing "try both decimal and scientific or hex format" + (is (= false (wallet.db/too-precise-amount? "100" 2))) + (is (= false (wallet.db/too-precise-amount? "100" 0))) + (is (= true (wallet.db/too-precise-amount? "100.1" 0))) + (is (= false (wallet.db/too-precise-amount? "100.23" 2))) + (is (= true (wallet.db/too-precise-amount? "100.233" 2))) + (is (= true (wallet.db/too-precise-amount? "100.0000000000000000001" 18))) + (is (= false (wallet.db/too-precise-amount? "100.000000000000000001" 18))) + (is (= false (wallet.db/too-precise-amount? "1e-18" 18))) + (is (= true (wallet.db/too-precise-amount? "1e-19" 18))) + (is (= true (wallet.db/too-precise-amount? "0xa.a" 2))) ;; 0xa.a is 10.625 + (is (= false (wallet.db/too-precise-amount? "0xa.a" 3))) + (is (= false (wallet.db/too-precise-amount? "1000" 3))))) + +(defn- equal-results? [a b] + (and (= (:error a) (:error b)) + (or (and (nil? (:amount a)) + (nil? (:amount b))) + (.eq (:amount a) (:amount b))))) + +(deftest test-parse-amount + (testing "test amount parsing" + (is (equal-results? {:value (money/bignumber "100")} (wallet.db/parse-amount "100" 2))) + (is (equal-results? {:value (money/bignumber "100")} (wallet.db/parse-amount "100" 0))) + (is (equal-results? {:error (i18n/label :t/validation-amount-is-too-precise {:decimals 0}) + :value (money/bignumber "100.1")} (wallet.db/parse-amount "100.1" 0))) + (is (equal-results? {:value (money/bignumber "100.23")} (wallet.db/parse-amount "100.23" 2))) + (is (equal-results? {:error (i18n/label :t/validation-amount-is-too-precise {:decimals 2}) + :value (money/bignumber "100.233")} (wallet.db/parse-amount "100.233" 2))) + (is (equal-results? {:error (i18n/label :t/validation-amount-is-too-precise {:decimals 18}) + :value (money/bignumber "100.0000000000000000001")} + (wallet.db/parse-amount "100.0000000000000000001" 18))) + (is (equal-results? {:value (money/bignumber "100.000000000000000001")} + (wallet.db/parse-amount "100.000000000000000001" 18))) + (is (equal-results? {:value (money/bignumber "1e-18")} (wallet.db/parse-amount "1e-18" 18))) + (is (equal-results? {:error (i18n/label :t/validation-amount-is-too-precise {:decimals 18}) + :value (money/bignumber "1e-19")} (wallet.db/parse-amount "1e-19" 18))) + (is (equal-results? {:error (i18n/label :t/validation-amount-is-too-precise {:decimals 2}) + :value (money/bignumber "10.625")} (wallet.db/parse-amount "0xa.a" 2))) + (is (equal-results? {:value (money/bignumber "10.625")} (wallet.db/parse-amount "0xa.a" 3))) + (is (equal-results? {:error (i18n/label :t/validation-amount-invalid-number) + :value nil} (wallet.db/parse-amount "SOMETHING" 5))) + (is (nil? (wallet.db/parse-amount nil 5))) + (is (nil? (wallet.db/parse-amount "" 5))) + (is (equal-results? {:value (money/bignumber "1000")} (wallet.db/parse-amount "1000" 3)))))