Consistent use of number wrappers / BigNumber

[nikulis]

(Thanks to @lisbakke's work and the discussion in #245)

Orderbook uses the num wrapper around precision decimals while the auth & public clients just pass on strings as-is.

Should the clients also use num (or BigNumber, etc.) for precision decimals? My initial thought was that it might be a bit heavy-handed, but if we anticipate that users want to do precision math with the data (which… I think they might), it could be a good proactive move in the name of safety. I guess it could be opt-in first, and later made default.

It would also be nice to find a place in docs to mention pitfalls of JS/IEEE-754 floating points for precision financial applications, which has come up in recent issues.

I realize that implementing this for the clients would be more complicated than for the orderbook, since the clients have more response formats to handle.

[rmm5t]

Should the clients also use num (or BigNumber, etc.) for precision decimals?

I vote no. The bulk of this library is mostly just a RESTful API wrapper. That wrapper should match the published GDAX API as closely as possible and only add conveniences when they make clear sense.

While it would be a nicety if the inputs to functions handled numeric objects and automatically performed a toString() before sending them off to the network, the nuance of this is likely too much of a burden.

Conversely, automatically converting the Strings from the network to some 3rd party numeric type (like bignumber.js) before returning them to the client only opens a can of worms. bignumber.js is a good choice, but I'm certain it will open up even more confusion from an Issue reporting perspective. I think we'd see an influx of more confused issues like this. It would also mean forcing a decimal library on all clients.

If someone is just using the gdax library to interface with the API endpoints, that's one thing. If they're doing more advanced analysis with orderbooks, prediction models, and other calculations, they're going to want to wrap the API objects in other objects for this analysis anyway and it's going to involve creating a bunch more convenience behaviors than just converting strings to a numeric type (at least my trading bot software is forced to take this approach). gdax-tt is another example of something that has to normalize these behaviors, and nothing the gdax client does can necessarily help it (unless the entire community settles on a one-true decimal library -- something the author of the top 4 most popular packages cannot even settle on).

<rant>At the end of the day, JavaScript really sucks at numbers and math. It's actually a disappointment that libraries like bignumber.js are even necessary, and it's even more of a disappointment that operators cannot be overridden to enhance libraries like bignumber.js. Other languages handle this much, much better.</rant>

[nikulis]

That wrapper should match the published GDAX API as closely as possible

Sounds perfectly reasonable to me. And, like you said, gdax-tt offers that higher functionality already. (I have a feeling that the line between what features belong here vs. gdax-tt might always be fuzzy.)

Should we ask the opposite question, then – would it make sense to take out num/BigWhatever from Orderbook so that it's consistent?

[nikulis]

And yes, I share the Number frustration. BigInt is in stage 3, and I'm not quite sure how I feel about it yet…

[rmm5t]

Should we ask the opposite question, then – would it make sense to take out num/BigWhatever from Orderbook so that it's consistent?

Fair question, but I fear there's little that can be done given the math involved with the current level 3 Orderbook. I know there's a desire to convert it to a level 2 order book, which would greatly simplify things, but would likely still require better number parsing for the purpose of keeping the order book sorted.

[fb55]

Very interesting question. In general this library already is in a weird state, as most of it is a thin wrapper around the API, but then there is also the orderbook sync class, which implements advanced functionality.

If we decide to do this, we should do it properly and also eg. parse dates. It's also not unthinkable to offer an option to opt of from this, if you want plain responses.

This prevents people from shooting themselves in the foot too easily, which can only be a win. Advanced users that might want to use their own big number library can use the opt out. At the same time, I would agree that this isn't necessarily what users expect.

Still not sure whether this is a great idea.

[fb55]

level 2 order book, which would greatly simplify things, but would likely still require better number parsing for the purpose of keeping the order book sorted.

Yes, very much the case

[fb55]

Curious if @CjS77 has any input as the maintainer of gdax-tt

[rmm5t]

In general this library already is in a weird state, as most of it is a thin wrapper around the API, but then there is also the orderbook sync class, which implements advanced functionality.

Good point. Perhaps OrderbookSync should be removed from this package and extracted to its own package? That would also make it easier to potentially support both Level 2 and Level 3 order book implementations without confusing the core gdax library.

[fb55]

Perhaps OrderbookSync should be removed from this package and extracted to its own package?

I think this leads into a discussion about what the general purpose of this library is. Imho this is the one-stop-shop for people that want to automate flows on GDAX (whether that's CLI tools or trading bots shouldn't matter). That would also include providing some mechanism to keep a book up-to-date. I'd say it would make sense to move the l3 book to its own package once we have a l2 book here.

[drewrothstein]

Hi, we are closing out PRs + Issues as this project is being archived.