Skip to content

gui: Add option to show warnings on wallet close, add warning for swaps #9715

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

f321x
Copy link
Member

@f321x f321x commented Apr 8, 2025

Allows to register a potential warning shown when the user tries to close the wallet GUI.
A callback: Callable[[], Optional[str]] can be registered using Abstract_Wallet.register_closing_warning_callback(callback).

On QT wallet close the callbacks will get called, if the callback returns a str it is presented to the user like this (str is the bottom message):
Screenshot_20250409_102511

TODO: implement for QML if this gets merged

@f321x f321x force-pushed the warning_on_close branch from c281d38 to e202ec5 Compare April 9, 2025 08:27
@accumulator
Copy link
Member

On Android this is not really a good option. There's no guarantee the app goes through a confirm-close sequence, as the app can be destroyed at any time by the OS. The current confirm-close is there, and it will save the wallet file, but one can easily close the app through other means.

Ideally the app should run an android background service to monitor chain and lightning events, so the UI can pop in and out of existence, but that would require a big overhaul..

@f321x f321x force-pushed the warning_on_close branch from e202ec5 to 110e52e Compare April 9, 2025 10:40
@f321x f321x force-pushed the warning_on_close branch 3 times, most recently from ab8fa9e to 0c8a786 Compare April 11, 2025 16:26
@ecdsa ecdsa added this to the 4.6.0 milestone Apr 22, 2025
@f321x f321x force-pushed the warning_on_close branch 2 times, most recently from f441e7a to 4f5fca3 Compare May 6, 2025 15:14
@ecdsa
Copy link
Member

ecdsa commented May 7, 2025

it would be nice if we could avoid adding a new persisted dict.
it should be possible to reconstruct what we need from the already existing swap manager data.

@f321x f321x force-pushed the warning_on_close branch 2 times, most recently from 907f230 to d931825 Compare May 7, 2025 12:57
@f321x f321x force-pushed the warning_on_close branch from d931825 to 60e4dca Compare May 7, 2025 12:59
@f321x
Copy link
Member Author

f321x commented May 7, 2025

I changed it to a more elegant solution which registers a callback that on wallet close will check if there are swaps with unconfirmed funding transaction. This way there is clear separation between swap and gui code and no persisted dict is required.

@@ -1279,6 +1279,15 @@ def get_group_id_for_payment_hash(self, payment_hash):
if swap:
return swap.spending_txid if swap.is_reverse else swap.funding_txid

def has_pending_swaps(self) -> bool:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be better to return a list of pending swaps, then the GUI can adapt its message depending on the direction of the swap(s)

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

Successfully merging this pull request may close these issues.

3 participants