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

The float64 issue... again #144

Open
treeder opened this issue Feb 20, 2020 · 4 comments
Open

The float64 issue... again #144

treeder opened this issue Feb 20, 2020 · 4 comments

Comments

@treeder
Copy link
Contributor

treeder commented Feb 20, 2020

floats are just outright rejected as of this commit: a94ed0a

Which makes it annoying for JSON users.

Two ideas to make this a bit smoother.

  1. Allow whole numbers to pass through, eg: if f == math.Trunc(f)
  2. Put more info into the error that rejects float64's, such as if you are parsing JSON, use decoder.UseNumber()
@treeder treeder assigned jmank88 and unassigned jmank88 Feb 20, 2020
@treeder
Copy link
Contributor Author

treeder commented Feb 20, 2020

@jmank88

@treeder
Copy link
Contributor Author

treeder commented Feb 20, 2020

Actually, it appears that it already did the same thing as 1 here. Why wasn't that acceptable?

@jmank88
Copy link
Contributor

jmank88 commented Feb 20, 2020

Actually, it appears that it already did the same thing as 1 here. Why wasn't that acceptable?

Information can be lost beforehand that the trunc check won't catch. Even a whole number integer could actually change into a different whole number integer (e.g. 10000000000000001->10000000000000000). The PR (#135) shows some examples.

It would be convenient to still accept float64 in some narrow cases to accommodate users that already have a float64 (and no lossy conversion happening first to 'correct'), but that ofc simultaneously exposes other users to accidental bugs, so asking the few users already stuck with float64s to convert explicitly seems like a minimal burden.

The doc says:

// Note: The encoding/json package uses float64 for numbers by default, which is inaccurate
// for many web3 types, and unsupported here. The json.Decoder method UseNumber() will
// switch to using json.Number instead, which is accurate (full precision, backed by the
// original string) and supported here.

And the current error is: floating point numbers are not valid in web3 - please use an integer or string instead (including big.Int and json.Number)

How about adding this?

-floating point numbers are not valid in web3 - please use an integer or string instead (including big.Int and json.Number)
+floating point numbers are not valid in web3 - please use an integer or string instead (including big.Int and json.Number) - see json.Decoder.UseNumber()

@treeder
Copy link
Contributor Author

treeder commented Sep 17, 2020

I think we could resolve all these issues using shopspring/decimal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants