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

Added option for disabling watch-only notification #8133

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

myxmaster
Copy link
Contributor

@myxmaster myxmaster commented Jan 9, 2023

This fixes #2835.

I added a checkbox in the warning dialog to disable the warning, also added the option in Preferences -> Misc to toggle it.

warning-dialog

preferences

@SomberNight
Copy link
Member

I think there is a real risk of a casual user creating a wallet from an address and not realising it's watch-only. They might have previously selected "don't show again" for another wallet, in which case they would not get a warning anymore.

I suppose if you use a watch-only wallet on a regular basis it might become a nuisance to always get the popup... I imagine that's your motivation, right? Maybe we could add the "don't show" option on a per-wallet basis (save into wallet file instead of config?).

Even then, people might forget that a wallet is watching-only. :/

And re exposing the option in the Preferences, that seems overkill. People who would hide the warning in the first place would not likely re-enable it later IMO.

checkbox.stateChanged.connect(partial(checkboxStateChanged, result=result))
self.show_warning(msg, title=_('Watch-only wallet'), checkbox=checkbox)
if result.isChecked == Qt.Checked:
self.config.set_key('disable_watch-only_warning', True, save=True)
Copy link
Member

Choose a reason for hiding this comment

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

Nit but I would prefer not using dashes in config keys. atm IIRC they all use snake-case, and we should keep it consistent. Just convert any - to _.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1

@myxmaster
Copy link
Contributor Author

I thought about that and found it is not a serious problem because worst thing that happens is: You cannot send. And then you very likely remember why :)

Also: I had the idea to add "Watch-only Wallet" in the title bar of the main window. That way people who don't remember turning off watch-only-warning can still see this anytime.

@SomberNight
Copy link
Member

I thought about that and found it is not a serious problem because worst thing that happens is: You cannot send. And then you very likely remember why :)

Some people create imported wallets from random addresses they find on forums/etc, to monitor their history. If they mistakenly hand those out to receive on, the money is lost to them (as the address belongs to someone else).

Also: I had the idea to add "Watch-only Wallet" in the title bar of the main window.

That's already there:

extra.append(_('watching only'))

@myxmaster
Copy link
Contributor Author

Maybe we could add the "don't show" option on a per-wallet basis (save into wallet file instead of config?).

You are right, that's a little bit more safe.

Some people create imported wallets from random addresses they find on forums/etc, to monitor their history. If they mistakenly hand those out to receive on, the money is lost to them (as the address belongs to someone else).

Ok, that's true. What about a warning message in case a user tries to create a payment request for a watch-only wallet? Would you then feel comfortable adding this feature? If not, please comment/close issue #2835.

That's already there:

extra.append(_('watching only'))

Uhm yeah, didn't even check, sorry :)

@ecdsa
Copy link
Member

ecdsa commented Jan 11, 2023

The popup is also a protection against some attacks. There have been reports of users having a watching-only wallet maliciously installed on their computer. It is a very cheap attack, requires no programming skills. Someone with unauthorized access to a computer would certainly disable that popup too.

Uhm yeah, didn't even check, sorry :)

No, it is not that you did not check, but that you did not see it.
The title bar thing is not visible enough, this is why we have this popup.

@myxmaster
Copy link
Contributor Author

No, it is not that you did not check, but that you did not see it.

Indeed, good point. I still think that it can be annoying for advanced users to always quit that popup.

Another idea:
Receive Tab and Addresses Tab could start with a highly visible warning, something like in this mockup:

mockup

That way people who use Electrum for checking balances of cold wallets can turn off the annoying popup at app start, but inattentive users won't get scammed so easily.

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.

Watching-only Notification on Wallet Open
3 participants