dnum.from Incorrect number from String() converting to scientific notation
Closed this issue · 7 comments
in the from method
String(value) converts expoents greater than 21 or less than -6 to scientific notation, which makes it not pass the regex in the next line
I was trying something like multiply(price, 10 ** -8, 18)
It's not a big issue since divide(price, 10 ** Math.abs(expo), 18) should do the same
a possible solution could be something like this, I'm not sure of the implications or if this should be handled in the lib, if you want I can look further and open a pr
value = String(value);
if (value.includes('e')) value = Number(value).toFixed(20)
// or value = typeof value === 'number' ? value.toFixed(20) : String(value)
if (!value.match(NUM_RE)) {
throw new Error(`Incorrect number: ${value}`);
}It took some time today to figure what I was doing wrong here haha
Scientific notation is used if the radix is 10 and the number's magnitude (ignoring sign) is greater than or equal to 10^21 or less than 10-6
got here because that's how the pyth api is setup btw https://docs.pyth.network/pythnet-price-feeds/best-practices
Oh good catch! Yes we should definitely handle this better. I’ve been doing some tests and noticed that even toFixed() can produce scientific notation numbers:
(10 ** 20).toFixed(20) // "100000000000000000000.00000000000000000000"
(10 ** 21).toFixed(20) // "1e+21"From Number.toFixed:
If the absolute value of numObj is greater or equal to 1021, this method uses the same algorithm as Number.prototype.toString() and returns a string in exponential notation. toFixed() returns "Infinity", "NaN", or "-Infinity" if the value of numObj is non-finite.
I see two paths:
- We try to get it right, maybe by using something like https://github.com/shrpne/from-exponential which seems complete. It would make the library a little bit heavier but not by much.
- We detect the scientific notation and add a specific error message to let consumers know about the exact problem and how to solve it. An issue I see with this solution is that the error might not appear during the development phase but later.
Also I am thinking that it might be good to add some warning in the README about the limitations of using numbers as an input, compared to strings / bigints / dnums.
What do you think?
I think ethers throws a numeric underflow(?) well this lib users probably are already avoiding numbers so a warning on readme and custom error message may be enough
but if it's easily fixable I don't see why not, from-exponential is tinyy
looks like from-exponential loses precision 😢
Oh too bad, yes then I guess the simplest way to handle it would be a custom error for now. Let me know if you want to open a PR for it, otherwise I’ll do it later this week :)
@greg-schrammel I changed my mind and decided to use from-exponential rather than throwing an error. The reasoning is that Number is always prone to precision loss so it doesn’t change that.