Valid refund rejected due to float comparison

Version v5.13.16

Environment Docker

Checklist

  • Can you replicate the issue on our v5 demo site https://demo.invoiceninja.com or Invoice Ninja? Yes
  • Have you searched existing issues? Yes
  • Have you inspected the logs in storage/logs/laravel.log for any errors? Tes

Describe the bug

ValidRefundableRequest::checkTotalRefundableAmount() sums refund amounts with collect()->sum() and compares the result to the refundable balance using with a plain > operator, both are float operations. When the invoices being refunded sum, in decimal, to exactly the payment’s remaining balance, float rounding can push the computed total a fraction of a cent over the target, and a valid full refund gets rejected.

var_dump(11.85 + 18.35 > 30.20); // bool(true)

11.85 + 18.35 evaluates to 30.200000000000003 in IEEE-754 double precision, not 30.2. The check at ValidRefundableRequest.php#L146-179 is affected.

Steps To Reproduce

Payment: amount = 30.20, refunded = 0.00
Refund request:

{
  "id": "PAYMENT_ID",
  "invoices": [
    {"invoice_id": "INV_1", "amount": 11.85},
    {"invoice_id": "INV_2", "amount": 18.35}
  ]
}

422 Unprocessable Content, {"errors":{"id":["texts.max_refundable_payment"]}}.

We hit it on multiple different payments/amounts in production (another example: 11.26 + 17.76 against a 29.02 payment, same failure).

Minimal standalone repro on 3v4l (copy-pasted checkTotalRefundableAmount() body, with Payment/ctrans() stubbed out):
https://3v4l.org/Tl3AY

Expected Behavior

The refund succeeds, the requested total (11.85 + 18.35 = 30.20) exactly matches the payment’s refundable balance (30.20 - 0.00 refunded), so it should not be treated as exceeding the maximum.

Suggested fix

You already have App\Utils\BcMath (BcMath.php) with exactly the primitives this needs, sum() and greaterThan(). This validation rule just doesn’t use it yet:

use App\Utils\BcMath;

// ...
if (count($request_invoices) > 0) {
    $total_refund_requested = (float) BcMath::sum(array_column($request_invoices, 'amount'), 2);
} elseif (array_key_exists('amount', $this->input)) {
    $total_refund_requested = $this->input['amount'];
}

// ...

if (BcMath::greaterThan($total_refund_requested, $max_total_refundable, 2)) {
    $this->error_msg = ctrans('texts.max_refundable_payment', [
        'max_refundable' => $max_total_refundable,
    ]);
    return false;
}

Happy to open a PR if useful.

Hi,

Thanks for reporting this! GitHub is a better place to track it, if you decide to create a PR please create it against the v5-develop branch.

1 Like

Thank you for your fast reply.
Github issue + PR created:

1 Like

Awesome, thank you!!