Include vout when copying transaction ID from coin selection

[meshcollider]

Fixes #432

I think it makes sense to just add the vout to the existing function because I can't imagine a situation where a user in the coin selection dialog would want just the transaction ID rather than the specific outpoint, and they can just delete it from the end anyway.

[laanwj]

ACK c4ea11c
FWIW we used to append the vout to the id when copying from the normal transaction list as well, a long time ago, but it always got a lot of flak from confused users. But I think it's fine here, coin control is an advanced feature anyway.

[jarolrod]

Concept ACK

Can we do something to document this change in the text of the related context menu action? If the text states copy transaction ID then a user will expect ONLY the txid. We should set the correct expectations of what the action will do.

maybe the following?

@@ -55,7 +55,7 @@ CoinControlDialog::CoinControlDialog(CCoinControl& coin_control, WalletModel* _m
     contextMenu->addAction(tr("&Copy address"), this, &CoinControlDialog::copyAddress);
     contextMenu->addAction(tr("Copy &label"), this, &CoinControlDialog::copyLabel);
     contextMenu->addAction(tr("Copy &amount"), this, &CoinControlDialog::copyAmount);
-    copyTransactionHashAction = contextMenu->addAction(tr("Copy transaction &ID"), this, &CoinControlDialog::copyTransactionHash);
+    copyTransactionHashAction = contextMenu->addAction(tr("Copy transaction &ID/vout"), this, &CoinControlDialog::copyTransactionHash);
     contextMenu->addSeparator();
     lockAction = contextMenu->addAction(tr("L&ock unspent"), this, &CoinControlDialog::lockCoin);
     unlockAction = contextMenu->addAction(tr("&Unlock unspent"), this, &CoinControlDialog::unlockCoin);

[meshcollider]

Modified the context menu text as per @jarolrod's suggestion

[shaavan]

Concept ACK

This PR adds the functionality of copying output index and transaction ID as a context menu option from the Coin Selection dialog box. I was able to successfully test this PR on Ubuntu 20.04 (Using Qt version 5.12.8).

The Difference with Master:

Master	PR
9f4cc49b1294299ae9bba26e5b48b82a032222f8c46d05e455602b298ff8bcba	9f4cc49b1294299ae9bba26e5b48b82a032222f8c46d05e455602b298ff8bcba:1

The changes in functions’ names aptly represent the change in functionality. I have mentioned some minor nits that shall be looked into.

Btw, I was just curious. Would it be beneficial to apply this functionality to the context menu action under the transaction tab?

[promag]

Code review ACK 949e700.

@laanwj I think we can have both actions copy-txid and copy-outpoint.

Left some nits.

[meshcollider]

Thanks for the reviews, will address the nits shortly.

Would it be beneficial to apply this functionality to the context menu action under the transaction tab?

IMO no, because unlike the coin selection which deals with UTXOs, transactions can have multiple outputs and most non-technical users don't understand the idea of outputs anyway, so it would just be confusing.

[meshcollider]

Addressed nits (and rebased)

[kristapsk]

ACK 10c6929

[promag]

Anything against having both actions?

[meshcollider]

@promag seems a bit more GUI clutter for no purpose. It's a manual action touching the clipboard so it can't interfere with any automation, so the user can always just manually delete the :n on the end if they don't want it. IMO txid alone is useless in this situation because this is a specific outpoint selection dialog.

[hebasto]

Concept ACK.

[hebasto]

ACK 10c6929, tested on Linux Mint 20.2 (Qt 5.12.8).

[shaavan]

ACK 10c6929

Changes since my last review:

• copyTransactionOutpointAction has been renamed to m_copy_transaction_outpoint_action.
• Comment has been aptly updated to match the change in functionality.
• The copyTransactionOutpoint function has been expanded to make it more clear.
• PR has been rebased on the current master.
• Formatting has now been corrected.

Difference with Master (Screenshot):

Master	PR