Improvements to the open up transaction in third-party link action

jarolrod commented at 4:35 am on September 24, 2021: member

#4092 introduced the ability to open up a transaction in a block explorer. This improves the related code by simplifying the addition and connection of the action through an overloaded addAction function and prepends action description text to the host, “Show in”. The reason to add this text is to make it clear what the action does. It also creates a clearer mental correlation between a user doing the work to add the 3rd-party tx link and this new menu action popping up.

This updates the setting text so that “third-party” is hyphenated. It should be hyphenated because it is being used as a modifier of both “URL” and “transaction URLs”.

Additionally, this fixes #431 by ensuring that the seperator will be added before creating action.

Screenshots of visual changes:

Context menu actions

master	pr

Setting text (tooltip text containing usage of “third-party” is also properly hyphenated)

master	pr

qt, refactor: simplify third-party tx url action through overload

Simplify the creation, addition, and slot/signal connection of
a third part tx url context menu action by using an overloaded
addAction function.

9980f4aa5e

qt: improve text for open third-party tx url action

The text for an open third-party tx URL action
is improved by appending the host name with "Show in".
This makes it self-explanatory what the action will do.

a70a98075a

jarolrod added the label UI on Sep 24, 2021

qt: ensure seperator when adding third-party transaction links

This ensures that if we're going to add an action to open up
a transaction in a third-party link (block explorer) that it
is seperated into it's own section.

8177578b29

jarolrod commented at 7:13 am on September 24, 2021: member

Updated from a70a98075a0d258d41c1310553a2337538a1d80c -> 8177578b296adb82fb8ab6c64cd76b832ebcc132 (pr430.01 -> pr430.02, diff)

Changes: include new commit to fix #431

in src/qt/transactionview.cpp:238 in 8177578b29 outdated

237-                    connect(thirdPartyTxUrlAction, &QAction::triggered, [this, url] { openThirdPartyTxUrl(url); });
238+                        actions_created = true;
239+                    }
240+                    /*: Transactions table context menu action to show the
241+                        selected transaction in a third-party block explorer.
242+                        %1 is a stand-in argument for the URL of the explorer. */

shaavan commented at 1:12 pm on September 24, 2021:

Regarding the Translator’s comments. The beginning of the comment looks a little confusing. How about…

0                    /*: The context menu action of the Transaction table to show the
1                        selected transaction in a third-party block explorer.
2                        %1 is a stand-in argument for the URL of the explorer. */

jarolrod commented at 6:09 am on September 27, 2021:

I think this is better as is, will leave this unresolved for now.

shaavan commented at 1:16 pm on September 24, 2021: contributor

Concept ACK

This PR introduced the overloaded addAction function, which simplifies the code while keeping the functionality intact. The introduced translator’s comments also look good. I have a suggestion regarding them that I will come to in a minute. Finally, the added actions_created boolean allows solving the bug of separator not being added while also increasing the understandability of the code without additional comments.

qt: hyphenate usage of third-party modifier

Our usage of "third-party" should be hyphenated
as it is being used as a modifier of both "URL"
and "transaction URLs".

2ccde2f932

jarolrod commented at 6:12 am on September 27, 2021: member

Updated from 8177578b296adb82fb8ab6c64cd76b832ebcc132 -> 2ccde2f9329d2685563b09ff0830f0d5916f57d5 (pr430.02 -> pr430.03, diff)

Changes: included one more commit to address the proper hyphenation of “third-party”.

shaavan commented at 1:31 pm on September 27, 2021: contributor

The new commit looks good. I have tested the PR on Ubuntu 20.04 (using Qt version 5.12.8) with the recent commit, and the changes are reflected perfectly in the GUI.

Master	PR

stratospher commented at 8:58 pm on September 27, 2021: contributor

tested ACK 2ccde2f. In agreement with the PR’s objectives. The PR was tested and verified to be consistent with the proposed changes:

Adding the description “Show in” - gave a clearer idea of the menu action.
Hyphenating to “third-party”
Fixing #431 so that the separator between third-party link actions and normal actions are in different sections even when the first string in a Third party transaction URL is empty. ( Example from #431 : | | https://mempool.space/tx/%s | https://blockstream.info/tx/%s)

Notes:

9980f4a addAction() adds openThirdPartyTxUrl() to the menu’s list of actions and returns it directly. This simplifies the code as compared to the previous version where connect() was used.
a70a980 Adds the description “Show in” to get a clear picture that we want to show the transaction in a third party transaction URL.
8177578 Makes a separate section for third-party links Uses a boolean variable to identify the first non-empty string and then adds the separator instead of arbitrarily adding the separator after the first string in listUrls[].
2ccde2f Makes “Third party” -> “Third-party”

promag commented at 1:59 pm on September 28, 2021: contributor

Code view ACK 2ccde2f9329d2685563b09ff0830f0d5916f57d5.

hebasto commented at 10:01 am on September 29, 2021: member

Tested 2ccde2f9329d2685563b09ff0830f0d5916f57d5 on Linux Mint 20.2 (Qt 5.12.8) – #431 is fixed.

An idea for a follow up – introduce a warning message about possibility to break privacy while using external links. @prayank23 What do you think about such a warning?

hebasto approved

hebasto commented at 10:14 am on September 29, 2021: member

ACK 2ccde2f9329d2685563b09ff0830f0d5916f57d5

hebasto merged this on Sep 29, 2021

hebasto closed this on Sep 29, 2021

sidhujag referenced this in commit 9cceea94b7 on Sep 29, 2021

ghost commented at 10:27 pm on September 29, 2021: none

An idea for a follow up – introduce a warning message about possibility to break privacy while using external links.

Yes we can inform the user about privacy issues because using such block explorers when you are running your own node doesn’t make sense. Can add one line about using open source explorers with Bitcoin Core so URL will be localhost.

promag commented at 10:36 pm on September 29, 2021: contributor

The user might want to share the link with someone?

ghost commented at 8:42 pm on January 20, 2022: none

The user might want to share the link with someone?

Makes sense. Warning can be avoided in that case or sharing transaction id without explorer links is also good enough as the other person can check it in the explorer they prefer to use.

I was testing different things in GUI today to see if attackers can get IP for users and a fun experiment to share problems with users can be:

Create a DNS token: https://canarytokens.org/generate
Save https://randomchars.canarytokens.com/tx/%s in Bitcoin Core settings
Right click on any transaction
Check your email for IP address

Why would someone use a random explorer? Attackers can use several ways to convince newbies with features that are not available in other explorers. Example: https://bitcointalk.org/index.php?topic=2100392.0

bitcoin-core locked this on Jan 20, 2023

Improvements to the open up transaction in third-party link action #430