Fixed an issue where payment amounts were incorrect due to rounding errors

[simonworkhouse]

This pull request fixes an issue where amounts were being calculated incorrectly due to the way that PHP handles casting floats to integers. The problem code exists both for calculating the amount that is to be paid and when validating that the paid amount matches the expected amount.

For example, the payment total is calculated using $amount = (int) ((float) SubjectReader::readAmount($buildSubject) * 100); and let's use 1268.10 as the return value of SubjectReader::readAmount($buildSubject). Evaluating (int) ((float) 1268.10 * 100) returns the result int(126809) which is one cent less than the actual order total.

Out of curiosity, I ran some basic tests to see how often this might occur, and for order totals ranging from $0.01 to $100,000.00 it occurred 587,200 times (PHP 7.4.3).

The fix was just to round the value prior to casting it to an integer, although I have left this pull request as a draft for now because that is not necessarily the ideal solution.

You may or may not want to implement an alternative solution which utilises PHP's arbitrary precision math functions, or perhaps something purpose built for handling money such as https://github.com/moneyphp/money.

[BlainMaguire]

I don't think that cast to float is necessary on the line in question.

As you may have gathered, multiplying by 100 and then casting to int is aiming to remove the decimal point altogether. This is since the API call on the backend to Simplify isn't using floats (for the reason you mention) but rather an int.

If the amount is in fact a float then doing == is probably not a good idea here.

Now I see that we do in fact validate that the Magento store currency matches the currency in Simplify which is good.

Multiplying by 100 works for a lot of popular currencies like USD or EUR. One thing to consider is that not all currencies actually have just two decimal places. Maybe a library is the best option to take a currency and give back the number of decimals to use for multiplication (or to pass as precision to round) to remove the decimal before making the call to pay.

To make things a little more complicated, I believe Magento has its own constants it uses for precision and these can be be overridden with plugins (at least in Magento 1) and I think this might be where the original readAmount might be getting it's number of decimals from.

[simonworkhouse]

@BlainMaguire I'd suggest that for now this solution is good enough to address the issue, but perhaps someone could look at improvements and adding different currency support in the future.

[simonworkhouse]

This issue recently had it's first birthday.