Exception: Amount precision is too high for currency.

lukeholder commented 7 years ago

Using the Đồng (VND) currency, which has zero decimal places (zero minor units), returns an exception: "Amount precision is too high for currency." inside the ->getAmount() method.

I believe this commit is causing the error:

https://github.com/thephpleague/omnipay-common/pull/70/commits/529b18fe6b3196b4606b28548cb97ee913eaa914

The relevant line of code changed in the above commit is:

$decimal_count = strlen(substr(strrchr(sprintf('%.8g', $amount), '.'), 1));

with $amount of VND 103500000, (an int of 103500000) returns 6 as the $decimal_count, which is obviously incorrect. This is due to the result of sprintf('%.8g', $amount) being 41035000001.035e+8 which has 6 chars to the right of the decimal.

After restoring the original line:

$decimal_count = strlen(substr(strrchr((string)$amount, '.'), 1));

with $amount of VND 103500000, (an int of 103500000) returns 0 as the $decimal_count, which is the correct number of decimal places, and the transaction succeeds.

I don't quite understand the rationale for the above commit, hoping that experienced omnipay minds can assist.

/cc @judgej

Example 1:

$amount = 100000.3333;
echo "original amount: ".$amount;
echo "<br>";
echo "sprintf formatted amount: ".sprintf('%.8g', $amount);
echo "<br>";
echo "new result: ".strlen(substr(strrchr(sprintf('%.8g', $amount), '.'), 1));
echo "<br>";
echo "original result: ".strlen(substr(strrchr((string)$amount, '.'), 1));

results in:

original amount: 100000.3333
sprintf formatted amount: 100000.33
new result: 2
original result: 4

Example 2:

$amount = 103500000;
echo "original amount: ".$amount;
echo "<br>";
echo "sprintf formatted amount: ".sprintf('%.8g', $amount);
echo "<br>";
echo "new result: ".strlen(substr(strrchr(sprintf('%.8g', $amount), '.'), 1));
echo "<br>";
echo "original result: ".strlen(substr(strrchr((string)$amount, '.'), 1));

results in:


original amount: 103500000
sprintf formatted amount: 1.035e+8
new result: 6
original result: 0

judgej commented 7 years ago

Sorry - only just spotted this.

The original commits came from issues with different decimal accuracies being set for different PHP installations, meaning a simple cast to (string) gave different results on different installations. For example, some sites may give 1.66 and others may give 1.659999999997 The sprintf() was meant to bypass that and make sure the accuracy (decimal digits to which a string representation of a float is rounded to) was the same for all installs.

Of course, nothing should ever go via a float in the first place, but we are stuck with it as legacy (for Omnipay 2.x at least).