From b510488f112160d0bd7e645390ccc365f92f793d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Thor=C3=A9n?= Date: Thu, 3 Aug 2017 16:51:36 +0200 Subject: [PATCH] fix #1050 Introduce a money namespace that solves a bunch of issues related to significant numbers being preserved and converted correctly. --- .re-natal | 3 +- externs/externs.js | 12 +++++-- package-lock.json | 29 ++++++++++------- package.json | 1 + src/status_im/components/drawer/view.cljs | 5 +-- src/status_im/js_dependencies.cljs | 1 + .../screens/transaction_details.cljs | 7 +++-- .../transactions/views/list_item.cljs | 6 ++-- src/status_im/utils/money.cljs | 31 +++++++++++++++++++ test/cljs/status_im/test/runner.cljs | 6 ++-- test/cljs/status_im/test/utils/money.cljs | 12 +++++++ 11 files changed, 88 insertions(+), 25 deletions(-) create mode 100644 src/status_im/utils/money.cljs create mode 100644 test/cljs/status_im/test/utils/money.cljs diff --git a/.re-natal b/.re-natal index 075c6ec798..504a3efe7c 100644 --- a/.re-natal +++ b/.re-natal @@ -41,7 +41,8 @@ "nfc-react-native", "react-native-http-bridge", "emojilib", - "react-native-mapbox-gl" + "react-native-mapbox-gl", + "bignumber.js" ], "imageDirs": [ "images" diff --git a/externs/externs.js b/externs/externs.js index fa6c455b7a..546a2be7e1 100644 --- a/externs/externs.js +++ b/externs/externs.js @@ -9,8 +9,8 @@ var TopLevel = { "alert" : function () {}, "Animated" : function () {}, "Array" : function () {}, -"AutoGrowingTextInput" : function () {}, "awesome-phonenumber" : function () {}, +"BigNumber" : function () {}, "blur" : function () {}, "call" : function () {}, "callJail" : function () {}, @@ -23,6 +23,7 @@ var TopLevel = { "clearInterval" : function () {}, "clearStorageAPIs" : function () {}, "clearTimeout" : function () {}, +"clearWatch" : function () {}, "Clipboard" : function () {}, "cloneWithRows" : function () {}, "close" : function () {}, @@ -50,12 +51,14 @@ var TopLevel = { "digest" : function () {}, "Dimensions" : function () {}, "discardTransaction" : function () {}, +"dividedBy" : function () {}, "dy" : function () {}, "encrypt" : function () {}, "ENC_DEC" : function () {}, "end" : function () {}, "endCoordinates" : function () {}, "Error" : function () {}, +"ErrorUtils" : function () {}, "eth" : function () {}, "event" : function () {}, "fallbacks" : function () {}, @@ -74,6 +77,7 @@ var TopLevel = { "getAll" : function () {}, "getBlock" : function () {}, "getCardId" : function () {}, +"getCurrentPosition" : function () {}, "getExample" : function () {}, "getInitialOrientation" : function () {}, "getLayout" : function () {}, @@ -143,7 +147,7 @@ var TopLevel = { "ReactNative" : function () {}, "readFile" : function () {}, "readTag" : function () {}, -"realm-class" : function () {}, +"realm" : function () {}, "recoverAccount" : function () {}, "registerComponent" : function () {}, "reload" : function () {}, @@ -167,6 +171,8 @@ var TopLevel = { "sendWeb3Request" : function () {}, "sequence" : function () {}, "set" : function () {}, +"setAccessToken" : function () {}, +"setGlobalHandler" : function () {}, "setInterval" : function () {}, "setNativeProps" : function () {}, "setSoftInputMode" : function () {}, @@ -219,8 +225,8 @@ var TopLevel = { "ValueXY" : function () {}, "View" : function () {}, "vy" : function () {}, +"watchPosition" : function () {}, "Web3" : function () {}, -"web3" : function () {}, "width" : function () {}, "window" : function () {}, "write" : function () {}, diff --git a/package-lock.json b/package-lock.json index de4fad18ac..3b48a3e932 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4,11 +4,6 @@ "lockfileVersion": 1, "requires": true, "dependencies": { - "Base64": { - "version": "0.2.1", - "resolved": "https://registry.npmjs.org/Base64/-/Base64-0.2.1.tgz", - "integrity": "sha1-ujpCMHCOGGcFBl5mur3Uw1z2ACg=" - }, "abbrev": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.0.tgz", @@ -968,6 +963,11 @@ "resolved": "https://registry.npmjs.org/base-64/-/base-64-0.1.0.tgz", "integrity": "sha1-eAqZyE59YAJgNhURxId2E78k9rs=" }, + "Base64": { + "version": "0.2.1", + "resolved": "https://registry.npmjs.org/Base64/-/Base64-0.2.1.tgz", + "integrity": "sha1-ujpCMHCOGGcFBl5mur3Uw1z2ACg=" + }, "base64-js": { "version": "0.0.8", "resolved": "https://registry.npmjs.org/base64-js/-/base64-js-0.0.8.tgz", @@ -1008,7 +1008,9 @@ "integrity": "sha1-5tXqjF2tABMEpwsiY4RH9pyy+Ak=" }, "bignumber.js": { - "version": "git+https://github.com/debris/bignumber.js.git#94d7146671b9719e00a09c29b01a691bc85048c2" + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-4.0.2.tgz", + "integrity": "sha1-LR3DfuWWiGfs6pC22k0W5oYI0h0=" }, "bindings": { "version": "1.3.0", @@ -5813,6 +5815,11 @@ } } }, + "string_decoder": { + "version": "0.10.31", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", + "integrity": "sha1-YuIDvEF2bGwoyfyEMB2rHFMQ+pQ=" + }, "string-range": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/string-range/-/string-range-1.2.2.tgz", @@ -5833,11 +5840,6 @@ "resolved": "https://registry.npmjs.org/string.fromcodepoint/-/string.fromcodepoint-0.2.1.tgz", "integrity": "sha1-jZeDM8C8klOPUPOD5IiPPlYZ1lM=" }, - "string_decoder": { - "version": "0.10.31", - "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-0.10.31.tgz", - "integrity": "sha1-YuIDvEF2bGwoyfyEMB2rHFMQ+pQ=" - }, "stringstream": { "version": "0.0.5", "resolved": "https://registry.npmjs.org/stringstream/-/stringstream-0.0.5.tgz", @@ -6265,6 +6267,11 @@ "utf8": "2.1.2", "xhr2": "0.1.4", "xmlhttprequest": "1.8.0" + }, + "dependencies": { + "bignumber.js": { + "version": "git+https://github.com/debris/bignumber.js.git#94d7146671b9719e00a09c29b01a691bc85048c2" + } } }, "whatwg-fetch": { diff --git a/package.json b/package.json index a49217a068..2676789da8 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "babel-plugin-transform-regenerator": "6.20.0", "babel-preset-react-native": "1.9.0", "babel-register": "6.18.0", + "bignumber.js": "^4.0.2", "browserify-zlib": "^0.1.4", "buffer": "^3.6.0", "chance": "1.0.4", diff --git a/src/status_im/components/drawer/view.cljs b/src/status_im/components/drawer/view.cljs index 65610d02ca..c894f1bab5 100644 --- a/src/status_im/components/drawer/view.cljs +++ b/src/status_im/components/drawer/view.cljs @@ -27,7 +27,8 @@ [status-im.utils.gfycat.core :as gfycat] [status-im.utils.listview :as lw] [status-im.utils.platform :as platform] - [status-im.utils.utils :as utils])) + [status-im.utils.utils :as utils] + [status-im.utils.money :as money])) (defonce drawer-atom (atom nil)) (defn open-drawer [] (.openDrawer @drawer-atom)) @@ -95,7 +96,7 @@ (defview transaction-list-item [{:keys [to value timestamp] :as transaction}] [recipient [:contact-by-address to]] - (let [eth-value (.fromWei js/Web3.prototype value "ether") + (let [eth-value (str (money/wei->ether value)) value (i18n/label-number eth-value) recipient-name (or (:name recipient) to)] [touchable-highlight {:on-press #(rf/dispatch [:navigate-to-modal :transaction-details transaction])} diff --git a/src/status_im/js_dependencies.cljs b/src/status_im/js_dependencies.cljs index 952dacdcb3..6f89109948 100644 --- a/src/status_im/js_dependencies.cljs +++ b/src/status_im/js_dependencies.cljs @@ -7,3 +7,4 @@ (def homoglyph-finder (js/require "homoglyph-finder")) (def identicon-js (js/require "identicon.js")) (def Web3 (js/require "web3")) +(def BigNumber (js/require "bignumber.js")) diff --git a/src/status_im/transactions/screens/transaction_details.cljs b/src/status_im/transactions/screens/transaction_details.cljs index 858c9b1436..085b1e6f97 100644 --- a/src/status_im/transactions/screens/transaction_details.cljs +++ b/src/status_im/transactions/screens/transaction_details.cljs @@ -12,7 +12,8 @@ [status-im.transactions.styles.screens :as st] [status-im.transactions.views.list-item :as transactions-list-item] [status-im.transactions.views.password-form :as password-form] - [status-im.utils.platform :as platform])) + [status-im.utils.platform :as platform] + [status-im.utils.money :as money])) (defn toolbar-view [] [toolbar/toolbar @@ -40,8 +41,8 @@ [current-account [:get-current-account] recipient [:contact-by-address to]] (let [recipient-name (or (:name recipient) to (i18n/label :t/contract-creation)) - gas-price (.fromWei js/Web3.prototype gas-price "ether") - fee-value (* gas gas-price) + gas-price' (money/wei->ether gas-price) + fee-value (money/fee-value gas gas-price') estimated-fee (str fee-value " ETH")] [rn/view st/details-container [detail-item (i18n/label :t/to) recipient-name true] diff --git a/src/status_im/transactions/views/list_item.cljs b/src/status_im/transactions/views/list_item.cljs index 952fdeddbb..ea6438d03c 100644 --- a/src/status_im/transactions/views/list_item.cljs +++ b/src/status_im/transactions/views/list_item.cljs @@ -4,7 +4,8 @@ [status-im.components.chat-icon.screen :as chat-icon] [status-im.components.react :as rn] [status-im.i18n :as i18n] - [status-im.transactions.styles.list-item :as st])) + [status-im.transactions.styles.list-item :as st] + [status-im.utils.money :as money])) (defview item-image [contact] [rn/view {:style st/item-photo} @@ -30,8 +31,7 @@ (defview view [{:keys [to value id] :as transaction} on-deny] [recipient [:contact-by-address to]] - (let [bignumber (.toBigNumber js/Web3.prototype value) - eth-value (str (.fromWei js/Web3.prototype bignumber "ether")) + (let [eth-value (str (money/wei->ether value)) value-str (str (i18n/label-number eth-value) " ETH") recipient-name (or (:name recipient) to (i18n/label :t/contract-creation))] [rn/view {:style st/item} diff --git a/src/status_im/utils/money.cljs b/src/status_im/utils/money.cljs new file mode 100644 index 0000000000..9ccff7e643 --- /dev/null +++ b/src/status_im/utils/money.cljs @@ -0,0 +1,31 @@ +(ns status-im.utils.money + (:require [status-im.js-dependencies :as dependencies])) + +;; The BigNumber version included in web3 sometimes hangs when dividing large +;; numbers Hence we want to use these functions instead of fromWei etc, which +;; come bundled with web3. See +;; https://github.com/MikeMcl/bignumber.js/issues/120 for this regression being +;; introduced in some JS environments. It is fixed in the MikeMcl/bignumber.js +;; repo, but not in the web3 BigNumber fork: +;; https://github.com/ethereum/web3.js/issues/877 +;; +;; Additionally, while it is possible to use the BigNumber constructor without +;; stringifying the number, this only works up to some 15 significant digits: +;; https://github.com/MikeMcl/bignumber.js/issues/120 +;; +;; Lastly, notice the bad rounding for native Javascript numbers above 17 digits +;; that may result in errors earlier up the call chain. Ideally all money-related +;; sensitive functions should be moved into this namespace to check for such +;; matters: +;; (str 111122223333441239) => "111122223333441230" + +(defn bignumber [n] + (dependencies/BigNumber. (str n))) + +(def ether-unit-value (bignumber "1000000000000000000")) + +(defn wei->ether [n] + (.dividedBy (bignumber n) ether-unit-value)) + +(defn fee-value [gas gas-price] + (.times (bignumber gas) (bignumber gas-price))) diff --git a/test/cljs/status_im/test/runner.cljs b/test/cljs/status_im/test/runner.cljs index 295c0bd919..954ec0a833 100644 --- a/test/cljs/status_im/test/runner.cljs +++ b/test/cljs/status_im/test/runner.cljs @@ -3,7 +3,8 @@ [status-im.test.contacts.handlers] [status-im.test.chat.models.input] [status-im.test.handlers] - [status-im.test.utils.utils])) + [status-im.test.utils.utils] + [status-im.test.utils.money])) (enable-console-print!) @@ -16,4 +17,5 @@ (doo-tests 'status-im.test.contacts.handlers 'status-im.test.chat.models.input 'status-im.test.handlers - 'status-im.test.utils.utils) + 'status-im.test.utils.utils + 'status-im.test.utils.money) diff --git a/test/cljs/status_im/test/utils/money.cljs b/test/cljs/status_im/test/utils/money.cljs new file mode 100644 index 0000000000..ce152c4acd --- /dev/null +++ b/test/cljs/status_im/test/utils/money.cljs @@ -0,0 +1,12 @@ +(ns status-im.test.utils.money + (:require [cljs.test :refer-macros [deftest is testing]] + [status-im.utils.money :as money])) + +(deftest wei->ether + (testing "Numeric input, 15 significant digits" + (is (= (str (money/wei->ether 111122223333444000)) + "0.111122223333444"))) + (testing "String input, 18 significant digits" + (is (= (str (money/wei->ether "111122223333441239")) + "0.111122223333441239")))) +