(android) Fix wrong fiat unit on payment technical details (then) line#811
Open
PeterXMR wants to merge 1 commit into
Open
(android) Fix wrong fiat unit on payment technical details (then) line#811PeterXMR wants to merge 1 commit into
PeterXMR wants to merge 1 commit into
Conversation
…d fiat changes Issue ACINQ#535: in the "Technical details" page of a payment, the (then) fiat value used the user's currently-preferred fiat as the display unit, while the numeric amount was computed from the historical exchange rate (which is recorded in a possibly-different currency). Switching the preferred fiat after the payment was received therefore showed a wrong amount with a wrong-but-currently-preferred currency code. The historical fiat amount is only meaningful in the currency the user had configured at the time of the payment, which is preserved on ExchangeRate.BitcoinPriceRate.fiatCurrency. CsvWriter.convertToFiat already follows this convention; this change aligns the technical details screen with it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #535.
Summary
In the "Technical details" page of a payment, the
(then)fiat value used the user's currently-preferred fiat as the display unit, while the numeric amount was computed from the historical exchange rate (which is recorded in a possibly-different currency). After switching the preferred fiat the line therefore showed a wrong amount with a wrong-but-currently-preferred currency code — e.g. a 1 EUR incoming payment displayed as "1 PHP (then)" after switching the preferred fiat to PHP, instead of "1 EUR (then)".Change
(then)line now formats withrateThen.fiatCurrencyinstead of the user'sprefFiat. The(now)line is unchanged (it correctly uses the live rate for the preferred fiat).CsvWriter.convertToFiatin phoenix-shared, which readsexchangeRate.fiatCurrency.namedirectly off the historical rate.Tests
Added two JVM unit tests to AmountFormatterTest.kt that lock in the formatter contract the fix relies on:
then_fiat_uses_historical_rate_currency_not_preferred_fiatthen_fiat_label_does_not_follow_preferred_fiat_changeBoth pass under
./gradlew :phoenix-android:testDebugUnitTest --tests "fr.acinq.phoenix.utils.AmountConverterTest".Pre-existing build note (not blocking)
phoenix-android/src/test/.../LnurlAuthTest.ktdoes not compile on current master becauseCrypto.compact2derwas removed in bitcoin-kmp 0.30.0. I temporarily moved the file aside to run the new tests, then restored it untouched — this PR does not touch that file. The unit-test task will need the existing breakage resolved separately before CI can run.Test plan
:phoenix-android:assembleDebug)End-to-end verification on testnet3
Reproduced the original bug scenario against a real Phoenix wallet on Android Pixel 4a API 34 emulator, with a real Lightning channel funded via ACINQ's testnet3 LSP.
Setup:
5297b4f6…43a29a4a— 189,072 sat, confirmed at block 4,965,924.42061b2351d7…c04149c4b1f, opening channeld48a5c0fd8ae…d1b36654ef9. Net wallet balance: 187,505 sat (deposit − 1,000 sat service fee − 567 sat miner fee).Step 1 — Technical details before changing preferred fiat (EUR):
(now)and(then)match because the live fiat matches the recordedoriginalFiat.Step 2 — Switch preferred fiat to PHP, re-open Technical details:
The
(then)line stayed in EUR (the currency recorded at receive time), with the same numeric values as before — exactly the behavior described in #535. The(now)line correctly recomputed against the live PHP rate (8,772.42 PHP / 122.41 EUR ≈ 71.7 PHP/EUR, which matches current real-world rates, confirming the live and historical rate paths are both functioning).Without this fix the
(then)line would have re-labeled to "PHP" while continuing to use the EUR-historical rate as the multiplier, producing the "122.41 PHP" wrong-amount-wrong-unit output reported in the issue.Screenshots:

Before changing preferred fiat — both (now) and (then) lines show EUR.
After switching preferred fiat to PHP — (now) line correctly shows PHP, (then) line correctly stays in EUR with the same numeric value as before. This is exactly the behavior issue #535 asked for.
