dex-services icon indicating copy to clipboard operation
dex-services copied to clipboard

Determine JSON type for amounts

Open nlordell opened this issue 5 years ago • 1 comments

Currently we have several kinds of amounts and they have various return types.

  • atoms=true which are large integer values. Using JSON numerical types cause issues with common parsers (such as the ECMAScript JSON.parse parser) which can only handle precision up to a double, which is not enough for our case.
  • atoms=false which are fractional values in base units.

Currently the way we format numbers depends on the route:

  • markets/:market amounts are always JSON numbers, despite the fact that if atoms=true we can run into precision issues
  • markets/:market/estimate-buy-amount/:sell-amount-in-quote amounts are always JSON strings

We should probably decide whether or not base units (atoms=false) should be JSON numbers or remain strings and make the different routes consistent with their types.

Maybe the FE team could weigh in with their thoughts @anxolin @Velenir @W3stside

For some more context, see the discussion in #1111

Some options:

  1. Use JSON strings for everything:
// atoms=false
{ sellAmoutInBase: "1000000000000000000", ...}
// atoms=true
{ sellAmoutInBase: "1.0", ...}
  1. Use JSON strings for atoms and JSON numbers for base units
// atoms=false
{ sellAmoutInBase: "1000000000000000000", ...}
// atoms=true
{ sellAmoutInBase: 1.0, ...}
  1. Use tagged enums for amounts
// atoms=false
{ sellAmoutInBase: { unit: "atoms", value: "1000000000000000000" }, ...}
// atoms=true
{ sellAmoutInBase: { unit: "baseunit", value: 1.0 }, ...}

nlordell avatar Aug 03 '20 09:08 nlordell

Sorry @nlordell I missed this.

I like option 1. Floats should be avoided. With strings we have virtually unlimited precision and virtually unlimited big numbers We just need to make sure that if you change an endpoint, it doesn't affect us. So let us know if you deploy in staging, although it's good to test before that, so ideally would be good if u can run the web pointing to your new URL

anxolin avatar Aug 21 '20 15:08 anxolin