Page MenuHomePhabricator

[electrum] show a warning when a BIP21 URI sets a significant amount in the send tab
ClosedPublic

Authored by PiRK on Feb 6 2024, 11:31.

Details

Summary

This is to prevent users from accidentaly sending large amounts after clicking on a payment URI. The warning makes them aware that they should pay attention.

The XEC threshold serves as a fallback in case the user didn't enable fiat conversions in the application. It can be adjusted in the future if the exchange rate changes significantly.

Test Plan

Does not trigger the warning:
ecash:qrh3ethkfms79tlcw7m736t38hp9kg5f7gycxeymme?amount=2999999.00

Triggers the warning if the fiat conversion is enabled:
ecash:qrh3ethkfms79tlcw7m736t38hp9kg5f7gycxeymme?amount=3500000.00
ecash:qrh3ethkfms79tlcw7m736t38hp9kg5f7gycxeymme?amount=2000000.00&addr=qrh3ethkfms79tlcw7m736t38hp9kg5f7gycxeymme&amount=1500000.00

Triggers the warning if the fiat conversion is disabled (after restarting the application to actually make the fiat conversion unavailable):
ecash:qrh3ethkfms79tlcw7m736t38hp9kg5f7gycxeymme?amount=3000000.00
ecash:qrh3ethkfms79tlcw7m736t38hp9kg5f7gycxeymme?amount=2000000.00&addr=qrh3ethkfms79tlcw7m736t38hp9kg5f7gycxeymme&amount=1000000.00

Diff Detail

Repository
rABC Bitcoin ABC
Branch
warn_bip21
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 26966
Build 53498: Build Diffelectrum-tests
Build 53497: arc lint + arc unit

Event Timeline

PiRK requested review of this revision.Feb 6 2024, 11:31
electrum/electrumabc/exchange_rate.py
569 ↗(On Diff #45002)

ccy or self.ccy uses the fact that bool(None) is False and the or operator in python returns the first non "falsy" operand.
See https://stackoverflow.com/a/47007761/4494781
So if ccy is not specified when calling echange_rate, we maintain the previous behavior of fetching the rate for self.ccy (the currently selected fiat currency).

bytesofman requested changes to this revision.Feb 6 2024, 13:44
bytesofman added a subscriber: bytesofman.

what's the behavior if the price API is down?

electrum/electrumabc_gui/qt/main_window.py
3024 ↗(On Diff #45002)

Can we show what amount is about to be sent in this warning msg?

This revision now requires changes to proceed.Feb 6 2024, 13:44

what's the behavior if the price API is down?

It depends. If it worked previously during that Electrum ABC session it will use the last know exchange rate. If it never worked since startup of the application, it will use the XEC threshold as fallback (self.fx.exchange_rate("USD") will return None).

electrum/electrumabc_gui/qt/main_window.py
3024 ↗(On Diff #45002)

Yes, will do. I will have to refactor that diff a bit so that the fiat conversion and warning message are in the same function and we don't need to compute the thing twice.

show the amount in the warning message. Note that if the fiat conversion is enabled, the displayed amount will be shown in the selected currency (the threshold is still checked in USD)

Examples with and without fiat conversion (with EUR selected as the fiat currency)

Screenshot from 2024-02-06 17-51-46.png (174×502 px, 28 KB)

Screenshot from 2024-02-06 17-52-21.png (155×502 px, 25 KB)

This revision is now accepted and ready to land.Feb 6 2024, 16:59