status-im / status-mobile

a free (libre) open source, mobile OS for Ethereum
https://status.app
Mozilla Public License 2.0
3.91k stars 984 forks source link

Error while storing wallet `raw-balances` #17896

Closed ulisesmac closed 12 months ago

ulisesmac commented 1 year ago

Bug Report

Problem

~The utils.number/parse-int function is returning 0 or the default value given when we attempt to parse very long numbers, such as wallet raw-balance values, e.g. 150 000 SNTs would be represented as 150000000000000000000000, and:~

(parse-int "150000000000000000000000")
;; => 0

~The reason is we are checking if the number is an integer using clojure.core/integer?, which says no.~

Update:

While requesting and storing the :raw-balance for the wallet in app-db, utils.number/parse-int has been used, that function returns a 0 when the number is too large (e.g. 150 000 SNT, would be "150000000000000000000000").

For wallet balances, we should always use big number functions (please check the ns utils.money).

This issue consists of updating the raw-balance stored to properly use big numbers.

Expected behavior

Properly store the raw balances, test with 150,000 received and other smaller and bigger amounts.

Actual behavior

If we receive a big raw balance, the total balance in the account cards in the wallet page report $0.00

ulisesmac commented 1 year ago

@ilmotta Maybe you are interested on this. An easy solution would be to just use number? but I'm wondering why it wasn't the first approach

ulisesmac commented 1 year ago

Interesting and funny that:

(number? ##NaN)
;; => true
ulisesmac commented 1 year ago

And besides the docstring of NaN? (https://cljs.github.io/api/cljs.core/NaNQMARK)

Returns true if num is NaN, else false

(NaN? ##NaN)
(NaN? "Hola")
(NaN? "0x")
;; All of them => true

(NaN? 10)
(NaN? "0x0")
(NaN? "0x1")
;; All of them => false

Maybe due to the implementation based on JS:

(defn ^boolean NaN?
  [val]
  (js/isNaN val))
ilmotta commented 1 year ago

Interesting @ulisesmac! What's going on is that very big numbers can't be represented as integers in the computer without a special big int implementation.

The function utils.number/parse-int deals only with the max value of 2147483647.

In the part of the code dealing with wallet and token values, we should always use bignums, which currently live in utils.money, otherwise they overflow, etc.

ilmotta commented 1 year ago

The bug to me is in the calling code, which should use proper bignums, not in the function utils.number/parse-int, which will never work with large numbers such as the ones we use to represent token values.

ulisesmac commented 1 year ago

Thanks @ilmotta !!!

I agree, as you said we should use utils.money/bignumber when receiving the raw-balances. I'll update the issue to properly solve it