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

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

Details

Reviewers

bytesofman

Group Reviewers

Restricted Project

Commits

rABC56b8bdd42fb4: [electrum] show a warning when a BIP21 URI sets a significant amount in the…

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 Diff	electrum-tests
Build 53497: arc lint + arc unit

Event Timeline

PiRK created this revision.Feb 6 2024, 11:31

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 6 2024, 11:31

PiRK requested review of this revision.Feb 6 2024, 11:31

Harbormaster completed remote builds in B26956: Diff 45002.Feb 6 2024, 11:33

PiRK added inline comments.Feb 6 2024, 11:41

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).

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

In D15396#346394, @bytesofman wrote:

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)