OpenBazaar / openbazaar-go

OpenBazaar 2.0 Server Daemon in Go
MIT License
991 stars 284 forks source link

Investigate scientific notation in stringified amounts from the client #1911

Open placer14 opened 4 years ago

placer14 commented 4 years ago

When a client handles a number with sufficiently large precision, it will serialize it into scientific notation. We need to investigate how to improve the amout handling to support this format as it is deserialized.

From @rmisio:

When a number is beyond a certain threshold JSON seriaizes it in scientific notation. OB-go is not handling that format. A listing with bigPrice: "1.890987463000000000000000001e+27" results in the error invalid item price amount.

Edit: FWIW, this was made a launch blocker based on: https://github.com/OpenBazaar/openbazaar-go/issues/1911#issuecomment-563396136

rmisio commented 4 years ago

There may be more to the puzzle than I thought.

I save a listing with a bigPrice of 1.23456789e+26 and the server returns a 500 error with invalid item price amount, but if I refresh the listing, the price was updated.

So... I think maybe the server is properly handling the scientific notation at least enough to update the listing.

Another erroneous thing is the price is never updated in the index, it still has the old price leading to mismatch between the listing price and the one in the index (/ob/listings/:peerId).

Expectations:

hoffmabc commented 4 years ago

This is a necessary fix, so we need to have a plan for addressing this.

rmisio commented 4 years ago

Just for some extra context here. When a number is above a certain threshold (not sure what that is other than it's pretty high), JSON will serialize it into scientific notation:

JSON.stringify({ charlie: 94949494949494949494949494949 }); // "{"charlie":9.494949494949496e+28}"

So basically that behavior must be outlined in the JSON spec.

The bigNumber.js lib used by the desktop and Haven adheres to the spec, so when serializing a bigNumber above the threshold, it will serialize it in scientific notation (albeit as a string: "9.494949494949496e+28" instead of 9.494949494949496e+28).

I'm not sure if we could customize this on the client. It's maybe possible we can override the toJSON method to not have it go into scientific notation. But, that's not really advisable IMHO because we'd be violating the JSON spec.

Since the server is accepting JSON here, ideally IMHO, they could honor the JSON spec (I do understand this is a gray area because technically string based numbers are not covered in the JSON spec, but ideally string numbers would mimic the behavior of native numbers).

hoffmabc commented 4 years ago

Most likely will require a function to deal with the conversion. - @placer14