Solidus: Wrong charges on Stripe when using three digits subunit.

Created on 31 Jul 2020  ยท  10Comments  ยท  Source: solidusio/solidus

In our system the prices are defined with three digits subunit. and the charges on Stripe are 10 times higher.

Solidus Version:
2.10.0

To Reproduce
Our Money configuration has the following:

Spree.config do |config|
  config.currency = 'CHF'
end

Money::Currency.table[:chf].merge!(subunit_to_unit: 1000, smallest_denomination: 50)

to override the default behaviour.

Given an amount of 60.20 CHF, when a Payment is captured or authorized (see https://github.com/solidusio/solidus/blob/66af3b67dfe39535ed539aa36c907b014ff3f8e5/core/app/models/spree/payment/processing.rb#L51 and https://github.com/solidusio/solidus/blob/66af3b67dfe39535ed539aa36c907b014ff3f8e5/core/app/models/spree/payment/processing.rb#L174) the method cents is called which returns 60200 instead of the expected 6020, necessary for the Stripe charge.

Additional context

Our current patch consists of:

  • defining a new method in Spree::Money
def two_digits_cents
  ((cents / currency.subunit_to_unit.to_f) * 100).to_i
end
  • overriding the methods listed above to use money.two_digits_cents instead of money.money.cents.

I'd love to open a PR with a fix, but I am not sure if this approach is correct. If you confirm me that it is, I'll proceed with a PR.

Most helpful comment

If it is not urgent, yes, I'd be glad to open a PR. Not this week, though. ๐Ÿ‘

All 10 comments

@coorasse I'm not very familiar with this aspect of Solidus so I may be wrong, but I suspect the problem stems from the subunit_to_unit configuration, which represents the proportion between the subunit (cents) and the unit (franc). When you set it to 1000, you tell Money that a franc is made of 1000 cents, which means 60.20 CHF will be (incorrectly) multiplied by 1000 and converted into 60200 cents.

Can you clarify why you set that value to 1000? Perhaps there's a better way to achieve the same goal.

We configured it like that because the shop administrator wants to set prices using 3 digits.

@coorasse do you know the business requirement behind that choice? As far as I can see, for a product to cost 60.203 CHF doesn't mean anything to a customer if the smallest unit is 0.01 CHF.

In fact, look at what happens with your fix:

cents = 60203 # 60.203 CHF
subunit_to_unit = 1000

((cents / subunit_to_unit.to_f) * 100).to_i # => 6020 (60.20 CHF)

This means that, even though the price of the product is 60.203 CHF, you're charging the customer 60.20 CHF (which, off the top of my head, may also cause accounting problems in the long run).

I think that's the root of the problem here. Increasing subunit_to_unit to 1000 allows you to specify such granular prices, but it's a hack that I suspect may create weird problems such as this.

If you could share a bit behind why you need such precision in the prices, we can probably find another solution.

The client needs to set them with three digits. I cannot share too much information, but since the amounts bought are high, it really makes a difference for them to also be able to change the third digit. I can think about the gas price for example.

Next to that, in this business case, we have also the option smallest_denomination: 50, which rounds the total always to 0.05 cents since you cannot invoice with a higher detail (as you also correctly pointed out).

So the example you reported, is absolutely true in general, but it does not affect us because you cannot have a total of 60.203 CHF: only single item prices. If you buy an item for 60.203, the cart total will display 60.20 anyway.

That was also my doubt: can we change it in the Payment model? Is there a case where the payment needs to be with cents precision? (so 60203?)
In the case of Stripe, specifically, you can only submit two digits, so 6020 => 60.20CHF would be any way better than 60203 => 602.30 CHF ๐Ÿ˜†

Still, at the moment, if you configure Solidus with any currency which has more than two digits for cents, Stripe charges are wrong. https://github.com/RubyMoney/money/blob/master/config/currency_iso.json#L223
So maybe we can ignore our specific case and generalise the issue?

Got it, the use case makes sense!

With that said, I don't know if this is a general issue: I think if you create a Stripe charge in a currency that has more than two digits for cents, Stripe will compute the final amount correctly. For instance, I expect that a charge with { "amount": 12345, "currency": "BHD" } would be converted into BHD 12.345. The reason it's not working in your case is that Swiss francs use two digits, but you're sending three.

In your specific situation, one problem I see is that Solidus may not mark the order as paid if the order total is smaller than the payment total because of the rounding that takes place in your custom #two_digit_cents method.

Have you tried the Money.infinite_precision option? I wasn't familiar with it, but it looks like it will allow you to specify as many digits as you want after the decimal separator without impacting the conversion between unit and subunit. I would try that or some other solution that has to do with the rounding configuration rather than the actual currency configuration.

Keep me posted on how it goes!

Still, at the moment, if you configure Solidus with any currency which has more than two digits for cents, Stripe charges are wrong. https://github.com/RubyMoney/money/blob/master/config/currency_iso.json#L223

@coorasse with Payment Intents at the moment the number if digits is hardcoded, see https://github.com/solidusio/solidus_stripe/blob/master/app/controllers/solidus_stripe/intents_controller.rb#L54. I guess this may not a problem for your specific scenario, as you said that the order will be charged up to 2 decimals anyways, but it may need to be addressed for other situations.

@spaghetticode yeah, I think we need to fix that to use current_order.currency.subunit_to_unit, otherwise it won't work for currencies where the unit is made up by more or fewer than 100 sub-units.

Thanks for the hint @aldesantis! Here are my findings so far:

  • configuring Money.infinite_precision = true seems a better option than playing around with subunit_to_unit and smallest_denomination options.
  • ๐Ÿ†• The partial to render the input for the money always use precision: 2 by default https://github.com/solidusio/solidus/blob/master/backend/app/views/spree/admin/shared/_number_with_currency.html.erb#L10 (that was the same also before. I just noticed that now,maybe it should be a different issue)
  • The calls to money.money.cents I mentioned above returns now a BigDecimal instead of an Integer, therefore I had to explicitly do money.cents.to_i to avoid errors when calling Stripe. (by the way: the call money.money.cents should be replaced with money.cents anyway.
  • You are right, the payment gateway takes care already of 2 or 3 digits precision currencies.

So, to conclude, I think:

  • Money.infinite_precision = true is the configuration I was looking for. Thanks!
  • When configuring it, I still had to patch Solidus and explicitly call cents.to_i (PR needed?)
  • The partial I mentioned above does not support more digits. PR needed?

Thanks for the help so far!

@coorasse thanks for the tests, I think the issues you found should be addressed โ€” I'm pretty sure you're not the only store needing greater-than-usual precision for product prices.

Would you have some time to submit your patches as PRs to the core? If not, I can also try to find some time this week!

If it is not urgent, yes, I'd be glad to open a PR. Not this week, though. ๐Ÿ‘

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ericsaupe picture ericsaupe  ยท  4Comments

jhawthorn picture jhawthorn  ยท  4Comments

kennyadsl picture kennyadsl  ยท  3Comments

brchristian picture brchristian  ยท  5Comments

seand7565 picture seand7565  ยท  5Comments