Mask values on Transactions View #708

pablomartin4btc · 2023-02-06T03:20:16Z

Currently the mask values option (Settings menu->Mask values) hides the wallet balances shown on the Overview page including the recent transactions list from the right panel but it doesn't hide the amounts from the transaction view.

This enhancement has been mentioned on PR #701 as a desirable follow-up.

First approach was to hide the amounts on the transactions view when mask values option is checked:

But later on as reviewer furszy recommended, I've disabled the Transaction tab directly and switch to the Overview tab if the mask values option is set, check the new screenshots in the comment below.

DrahtBot · 2023-02-06T03:20:20Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	furszy, hernanmarino, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

pablomartin4btc · 2023-02-06T03:42:57Z

This requires PR #701 in order to work as it is - back to draft!

furszy

Code ACK 0369d8b

src/qt/overviewpage.cpp

furszy

Need to adapt BitcoinUnits::formatWithPrivacy function to accept negative amounts. Otherwise, when the wallet make a send, the assertion crashes the software.

pablomartin4btc · 2023-02-28T14:42:48Z

Need to adapt BitcoinUnits::formatWithPrivacy function to accept negative amounts. Otherwise, when the wallet make a send, the assertion crashes the software.

Fixed it, thanks.

furszy · 2023-02-28T16:18:51Z

Thinking further about this, if the "privacy" option is enabled, the transactions screen has no usage, thus shouldn't be accessible for the user.

In the current form, the screen is showing all the spends and receives ordered by date..
It presents the addresses where the user sent coins and the addresses where the user received them, so.. anyone could easily lookup them in an explorer and see how much money the wallet contains. Hiding amounts is not enough.

We could either block the view when the privacy option is enabled, or could create an empty screen image to replace the transactions list when privacy is enabled.

pablomartin4btc · 2023-02-28T18:42:38Z

Thinking further about this, if the "privacy" option is enabled, the transactions screen has no usage, thus shouldn't be accessible for the user.

We could either block the view when the privacy option is enabled, or could create an empty screen image to replace the transactions list when privacy is enabled.

Yeah, I realised what you are saying as soon as I fixed the negative amount issue looking at the type transactions, date-time, addresses columns, the red foreground colour due to the sent transaction type, etc., but the intention of this masking as explained in the issue #652 is to prevent the "user vulnerability to shoulder surfing".

Another approach could be to convert all columns to "privacy" mode or to disable the "Transactions" (button/ Tab view/ "historyAction").

pablomartin4btc · 2023-02-28T22:30:37Z

Disabling the Transaction tab when the user sets the mask value option:

Also, if you are already in the Transaction tab, switch/ change focus to the overview page as shown here:

hernanmarino

Tested ACK d41037b . I like the approach of hiding the Transaction table completely, I see no point in showing an empty table .

src/qt/bitcoinunits.cpp

hernanmarino

After more thorough testing I found the following issue, I think this pre-opened windows should be hidden or closed.

pablomartin4btc · 2023-03-02T16:34:29Z

After more thorough testing I found the following issue, I think this pre-opened windows should be hidden or closed.

Good catch! I'll give it a go... cheers.

pablomartin4btc · 2023-03-04T07:35:35Z

Updates:

Cleanup from previous approach.
Closing all dialogs with transaction details that were opened from the transactions view when mask values is selected.

hernanmarino

Tested ACK 5fb3fac

furszy

Code review ACK 4d0d15b
left two details.

src/qt/transactionview.h

furszy

ACK 4492de1

hernanmarino

ACK 4492de1

achow101 · 2023-03-13T17:37:19Z

ACK 4492de1

RandyMcMillan · 2023-05-27T17:37:35Z

ACK 4492de1

post merge ACK

Great job!

#701 (comment)

e26e665 gui: fix crash on selecting "Mask values" in transaction view (Sebastian Falbesoner) Pull request description: This PR fixes a crash bug that can be caused with the following steps: - change to the "Transactions" view - right-click on an arbitrary transaction -> "Show transaction details" - close the transaction detail window again - select menu item "Settings" -> "Mask values" The problem is that the list of opened dialogs, tracked in the member variable `m_opened_dialogs` (introduced in #708, commit 4492de1), is only ever appended with newly opened transaction detail dialog pointers, but never removed. This leads to dangling pointers in the list, and if the "Mask values" menu item is selected, a crash is caused in the course of trying to close the opened transaction detail dialogs (see `closeOpenedDialogs()` method). Fix this by removing a pointer of the list if the corresponding widget is destroyed. ACKs for top commit: achow101: ACK e26e665 pablomartin4btc: tACK e26e665 furszy: utACK e26e665 hebasto: ACK e26e665, tested on Ubuntu 22.04. Tree-SHA512: 37885c22abae0ab065b4878bae46fd362f41b09609d081fd59e26bb05474f427b98771ee73f5480526afaef04e016c5ba62c956e0e85a57b6a0f44a905b68a83

pablomartin4btc changed the title ~~qt: mask values on transactions view~~ Mask values on Transactions View Feb 6, 2023

pablomartin4btc marked this pull request as draft February 6, 2023 03:43

pablomartin4btc mentioned this pull request Feb 8, 2023

Persist "mask values" to prevent shoulder surfing #652

Closed

pablomartin4btc force-pushed the mask-values-on-transaction-table branch 2 times, most recently from 220a00d to 0369d8b Compare February 10, 2023 14:29

pablomartin4btc marked this pull request as ready for review February 10, 2023 15:47

furszy reviewed Feb 10, 2023

View reviewed changes

src/qt/overviewpage.cpp Outdated Show resolved Hide resolved

pablomartin4btc force-pushed the mask-values-on-transaction-table branch from 0369d8b to 3a8bc93 Compare February 28, 2023 04:33

pablomartin4btc requested a review from furszy February 28, 2023 04:33

pablomartin4btc force-pushed the mask-values-on-transaction-table branch from 3a8bc93 to 2634584 Compare February 28, 2023 04:35

furszy suggested changes Feb 28, 2023

View reviewed changes

pablomartin4btc force-pushed the mask-values-on-transaction-table branch from 2634584 to ee25124 Compare February 28, 2023 14:41

pablomartin4btc requested a review from furszy February 28, 2023 14:43

pablomartin4btc force-pushed the mask-values-on-transaction-table branch from ee25124 to d41037b Compare February 28, 2023 22:32

hernanmarino reviewed Mar 1, 2023

View reviewed changes

furszy reviewed Mar 1, 2023

View reviewed changes

src/qt/bitcoinunits.cpp Outdated Show resolved Hide resolved

DrahtBot requested a review from furszy March 1, 2023 18:47

hernanmarino reviewed Mar 2, 2023

View reviewed changes

pablomartin4btc force-pushed the mask-values-on-transaction-table branch 2 times, most recently from 94f104c to 786a26a Compare March 4, 2023 07:26

pablomartin4btc force-pushed the mask-values-on-transaction-table branch from 786a26a to 5fb3fac Compare March 4, 2023 07:39

hernanmarino approved these changes Mar 6, 2023

View reviewed changes

pablomartin4btc force-pushed the mask-values-on-transaction-table branch from 5fb3fac to 4d0d15b Compare March 7, 2023 03:44

furszy reviewed Mar 10, 2023

View reviewed changes

src/qt/transactionview.h Outdated Show resolved Hide resolved

src/qt/transactionview.h Outdated Show resolved Hide resolved

DrahtBot requested review from hernanmarino and removed request for hernanmarino March 10, 2023 15:03

qt: mask values on transactions view

4492de1

pablomartin4btc force-pushed the mask-values-on-transaction-table branch from 4d0d15b to 4492de1 Compare March 10, 2023 17:52

furszy approved these changes Mar 10, 2023

View reviewed changes

DrahtBot requested review from hernanmarino and removed request for hernanmarino March 10, 2023 18:05

hernanmarino approved these changes Mar 13, 2023

View reviewed changes

hebasto added the Needs release notes label Mar 14, 2023

hebasto merged commit 460e394 into bitcoin-core:master Mar 14, 2023

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 14, 2023

Merge bitcoin-core/gui#708: Mask values on Transactions View

3449b9b

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 15, 2023

Merge bitcoin-core/gui#708: Mask values on Transactions View

f4af722

theStack mentioned this pull request Oct 30, 2023

Fix crash on selecting "Mask values" in transaction view #774

Merged

hebasto removed the Needs release notes label Apr 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mask values on Transactions View #708

Mask values on Transactions View #708

pablomartin4btc commented Feb 6, 2023 •

edited by hebasto

DrahtBot commented Feb 6, 2023 •

edited

pablomartin4btc commented Feb 6, 2023

furszy left a comment

furszy left a comment

pablomartin4btc commented Feb 28, 2023 •

edited

furszy commented Feb 28, 2023 •

edited

pablomartin4btc commented Feb 28, 2023 •

edited

pablomartin4btc commented Feb 28, 2023

hernanmarino left a comment

hernanmarino left a comment •

edited

pablomartin4btc commented Mar 2, 2023

pablomartin4btc commented Mar 4, 2023

hernanmarino left a comment

furszy left a comment

furszy left a comment

hernanmarino left a comment

achow101 commented Mar 13, 2023

RandyMcMillan commented May 27, 2023

Mask values on Transactions View #708

Mask values on Transactions View #708

Conversation

pablomartin4btc commented Feb 6, 2023 • edited by hebasto

DrahtBot commented Feb 6, 2023 • edited

Reviews

pablomartin4btc commented Feb 6, 2023

furszy left a comment

Choose a reason for hiding this comment

furszy left a comment

Choose a reason for hiding this comment

pablomartin4btc commented Feb 28, 2023 • edited

furszy commented Feb 28, 2023 • edited

pablomartin4btc commented Feb 28, 2023 • edited

pablomartin4btc commented Feb 28, 2023

hernanmarino left a comment

Choose a reason for hiding this comment

hernanmarino left a comment • edited

Choose a reason for hiding this comment

pablomartin4btc commented Mar 2, 2023

pablomartin4btc commented Mar 4, 2023

hernanmarino left a comment

Choose a reason for hiding this comment

furszy left a comment

Choose a reason for hiding this comment

furszy left a comment

Choose a reason for hiding this comment

hernanmarino left a comment

Choose a reason for hiding this comment

achow101 commented Mar 13, 2023

RandyMcMillan commented May 27, 2023

pablomartin4btc commented Feb 6, 2023 •

edited by hebasto

DrahtBot commented Feb 6, 2023 •

edited

pablomartin4btc commented Feb 28, 2023 •

edited

furszy commented Feb 28, 2023 •

edited

pablomartin4btc commented Feb 28, 2023 •

edited

hernanmarino left a comment •

edited