Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect 100x amount in payment notification when currency is HUF #3234

Open
openoms opened this issue Apr 29, 2024 · 5 comments
Open

Incorrect 100x amount in payment notification when currency is HUF #3234

openoms opened this issue Apr 29, 2024 · 5 comments

Comments

@openoms
Copy link
Collaborator

openoms commented Apr 29, 2024

Describe the bug
The amount is shown as 100x in the notification when using HUF as the display currency.

To Reproduce
Steps to reproduce the behavior:
The padding in the notification should be the same as in the history with two digits accuracy.

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
Notification showing +364 506Ft
image
The correct amount history is
Notification showing +3654.06Ft
image

Smartphone (please complete the following information):

  • Device: Pixel5
  • OS: GraphenOS
  • Version 2.2.263

Additional context
Add any other context about the problem here.

@openoms openoms changed the title Incorrect padding in payment notification Incorrect padding in payment notification when currency symbol is after the amount Apr 29, 2024
@amitamrutiya
Copy link

Hey @openoms, I want to work on this issue. I think it's a good one to explore in this repository.

@thevaibhav-dixit
Copy link

@openoms is there any trace for this ?

@openoms
Copy link
Collaborator Author

openoms commented Apr 30, 2024

shared in DM with @thevaibhav-dixit .

Turns out HUF is special as it does not have a minor exponent https://leejo.github.io/2015/08/02/a_lot_of_huf/ :

        HUF : {
            exponent: 0,
            iso_alpha_code: "HUF",
            iso_numeric_code: "348",
            locale: EnBy,
            minor_units: 5,
            name: "Hungarian Forint",
            symbol: "Ft",
            symbol_first: false,
        },

It is causing the problem in the notifications, but not in the transaction history.

@openoms openoms changed the title Incorrect padding in payment notification when currency symbol is after the amount Incorrect 100x amount in payment notification when currency is HUF Apr 30, 2024
@openoms
Copy link
Collaborator Author

openoms commented Dec 17, 2024

The issue is likely related to the difference between the libraries used (or how they are handled) in the backend (https://github.com/galoymoney/blink) vs this repo for the mobile app.

Excerpt from an example notification trace from the backend :

app.handle_transaction_occurred_event

TransactionOccurred { transaction_type: LightningReceipt, settlement_amount: TransactionAmount { minor_units: 16000, currency: Crypto(Currency { code: "BTC", exponent: 8, locale: EnUs, minor_units: 100000000, name: "Bitcoin", symbol: "₿", symbol_first: true }) }, display_amount: Some(TransactionAmount { minor_units: 366519, currency: Iso(Currency { iso_alpha_code: "HUF", exponent: 0, iso_numeric_code: "348", locale: EnBy, minor_units: 5, name: "Hungarian Forint", symbol: "Ft", symbol_first: false }) }) }

@mlori
Copy link

mlori commented Dec 18, 2024

The final text of the txNotification is presented based on this metadata:

    minor_units: 366519, 
    currency: Iso(Currency {
      iso_alpha_code: "HUF", 
      exponent: 0, 
      iso_numeric_code: "348", 
      locale: EnBy, 
      minor_units: 5, 
      name: "Hungarian Forint", 
      symbol: "Ft", 
      symbol_first: false
    })
  }

I asume the text prestended in the push notifcation is renderred on backend side. While the text presented in tx history and other places is renderred on UI side.
Is it possible to override the above setup for HUF in the backend library, with a custom formatter?
Options:

  1. On backend side set exponent = 2, this implies, the minor_units should be 500 (so it will be expressed in fillér) - In this case i think no UI side change is required.
  2. keep exponent = 0 and round the amount so minor_units = 3665.
    Drawback of 2.: we lost the decimals and In this case the UI side has to be adapted too.
    somewhere around this point: https://github.com/GaloyMoney/blink-mobile/blob/main/app/hooks/use-display-currency.ts#L146

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants