Gringotts: A complete payment library for Elixir and Phoenix Framework

Absolutely disallow. Consuming or returning floats should be a hard error, absolutely not allowed as it will eventually break.

Such services should be disallowed. Anyone who uses a float to transfer monetary values is not just recklessness, but also malicious. Storing a number as a string allows lossless conversion and that is fine, same with fixed point, integers, ratio’s, etc, but storing it as a float should always be absolutely disallowed.

2 Likes

We would all easily agree that touching floating point arithmetic would be a very bad thing. But as best I can tell, simply converting from a float to a Decimal would not appear to introduce any additional precision errors.

That assumption is based upon my reading of IEE 754, consulting the Erlang and Decimal docs and running millions of empirical tests in the last hour or so round tripping Float -> Decimal -> Float (using :random.uniform/0 so not conclusive as yet). I also reviewed how Poison parses numbers and it ultimately uses String.to_float/1 which calls :erlang.binary_to_float/1 which is the same code path I used in my testing.

But I’m not a mathematician therefore I’m not claiming I have a definitive understanding whatsoever. However I definitely want to get to a clear and specific understanding here so I can make the best informed decisions on what I need to do in the lib given the goals I have expressed for it in the readme.

I’d be very happy to be directed to the relevant literature on the simple “conversion” path since all content I can find in the last hour have been about arithmetic about which we have no dispute.

Of course this doesn’t change your second challenge around some very popular external services that deliver exchange rates in json number format (and therefore reduced to a Float by json decoders).

2 Likes

I think the problem is not allowing the conversion per-se, but by allowing the conversion from floats then some application developers will not realize that they are dealing with an imprecise format and the potential for hard-to-detect errors. So in a way the library could be seen as encouraging bad practices. As an alternative new/2 could raise an error describing the issue when called with a float.

2 Likes

I think the problem is not allowing the conversion per-se, but by allowing the conversion from floats then some application developers will not realize that they are dealing with an imprecise format

Its good point of course.

And after writing a much longer answer with questions I realise all my issues are not about Money.new/2 but are about doing arithmetic (interest rate calcs, exchange rate calcs) which don’t involve Money.new/2, just conversion of float to Decimal in order to become a multiplicand.

I’ll deprecate Money.new(currency, Float) in the next version and remove it in ex_money 2.0.

Great conversation, thanks to you all.

3 Likes

Yes, yes it would. :wink:

Say you get a number from json or so, not a string, a number, and it is like 8.10, well 8.10 is not perfectly representable so you’d probably get a 8.0999999999999999* decimal, and this is just one of the most simple examples.

Never even accept float!

For note, 8.099999999999999 will (with enough 9’s) be rendered to the screen or so as 8.1, but will still internally have lots of 9’s that will mess up calculations. You cannot perfectly represent Base 10 floating point in Base 2 floating point, it is impossible for many numbers.

3 Likes

While it isn’t on the topic of floats and their evil use within the realm of money, I’d like to mention that I’m the maintainer of the Braintree library for Elixir.

I’ve personally never changed payment gateways during a project and don’t see a pressing need for an all encompassing payment gateway, but I know other people coming to the language will be looking for it. Best of luck!

Please reach out or feel free to lift the XML handling from the Braintree library.

4 Likes

CURRENT STATUS: we have merged the implementation in the dev branch; we’ll be releasing a new version in a day or two.

PR# MERGED: https://github.com/aviabird/gringotts/pull/71
ISSUE# https://github.com/aviabird/gringotts/issues/62

1 Like

I’ve released ex_money version 2.0.0 which no longer supports a float as a parameter to the factory method Money.new/2 (it will return an error or raise on the ! method).

I have also added Money.from_float/2 which will accept a float parameter. The intention is to make it really clear to a developer that they are using something out of the ordinary. As a precaution to ensure that ex_money doesn’t introduce any additional imprecision it will return an error if the supplied float has more than 15 digits of precision.

(not really Gringotts related I know but I want to reflect the decisions I took based upon the solid feedback from the community)

8 Likes

Hi @pkrawat1 and team, I would like to reinforce the point above.

Using a GenServer when you don’t need one is an anti-pattern and it will actually have severe applications on the system performance, as you put all operations besides a single process which will become a bottleneck. We cover this in detail in Adopting Elixir and there is a thread about this particular topic in the forum.

Even if you may end-up using the worker for configuration, I would ask you to revisit that, because it should not be the place of libraries to impose a stateful configuration mechanism. Simply ask the users to pass the configuration every time the gateway is called. If the users of your library need to store this configuration somewhere, they can easily do so by using an Agent, application config, GenServer, etc.

11 Likes

Thanks for your feedback! yes, we realised it after @michalmuskala’s feedback!

We have been working on it and merged the PR 2 days back.

We have got rid of the worker and Gringotts does not start any application now.

4 Likes

Apologies for my at least - but probably more - 2 days late feedback. :slight_smile:

3 Likes

It’s our pleasure that we even got your attention and more importantly your feedback. We are thankful!

We are planning on adding as many as 22 new gateways in next 2 months would be exciting to have 30 to 32 gateways by June.

7 Likes

Decimal v1.5.0 has just been released with similar change: passing floats to Decimal.new is soft deprecated (no warnings emitted yet) and there is now a new function to do that explicitly: Decimal.from_float.

Thanks everyone for feedback in this thread!

3 Likes

Definitely a great move there! :smiley:

A question, does the Decimal.from_float also accept an accuracy in it? So you can tell it to only accept to 2 decimal places or so (and in what base)?

A question, does the Decimal.from_float also accept an accuracy in it?

No, it just accepts the float to convert, see: https://hexdocs.pm/decimal/Decimal.html#from_float/1.

1 Like

Release update 1.1.0

We are super excited about the release of gringotts 1.1.0.

Added

  • api: Introduces a Money protocol (#71)
  • core: Introduces Response.t (#119)
  • development: Adds a useful mix task gringotts.new (#78)
  • docs: Adds changelog, contributing guide (#117)

Changed

We are very thankful for all the awesome and super useful feedback given by you all :+1::+1::+1:

What’s next?

We are working on adding more gateways, we are expecting to have 18-20 new gateways by mid of May.

PS: We are working on something cool internally apart from gringotts and expect it to present it to you all by mid of May. :wink:

6 Likes

I noticed this library hasn’t had any commits since Oct 25, 2018, and that the demo https://gringottspay.herokuapp.com/ is no longer live. Is this no longer in development?

1 Like

@jpinnix we are taking any PR’s and issues and fixing them. Also planning some more gateways this summer. Let us know if you have any questions.

6 Likes

We may have to start using liveness advisories in the Elixir community like they do on some Clojure projects like: https://github.com/candera/causatum#liveness-advisory

2 Likes

What would be nice would be for such advisories to be in the code, so that scripts could automatically run from time to time and update curated lists of live libraries. It would solve the usual problem that especially new users have concerning which libraries are nice and current!

1 Like