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

[GUI][Bug] Reset custom change address #1551

Merged

Conversation

random-zebra
Copy link

First commit fixes a bug where the btnChangeAddress in SendWidget was deactivated, despite a custom address still being saved (e.g. saving a valid address, and then trying to change it to an invalid one).

Second commit addresses a feature request: being able to reset only the change address (without resetting the whole coin control) directly from within the dialog.
With this PR, when a custom change address is set, the text of the "CANCEL" button in SendChangeAddressDialog is changed to "RESET" and, when clicked, resets the default change address to CNoDestination (which means that a new change address will be created during the send operation).
The upright "X" button retains its original function of just closing the dialog.
Closes #1526

@random-zebra random-zebra added GUI Bug Usability Needs Backport Placeholder tag for anything needing a backport to prior version branches labels Apr 23, 2020
@random-zebra random-zebra added this to the 4.1.0 milestone Apr 23, 2020
@random-zebra random-zebra self-assigned this Apr 23, 2020
@Fuzzbawls
Copy link
Collaborator

I think we should take an extra precaution with regards to an invalid address being entered by resetting the change address. this will prevent a previously saved (valid) change address from persisting.

Index: src/qt/pivx/send.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/qt/pivx/send.cpp	(revision 4763c2c08d2f80b96027eda4ab4c2a7642d14e08)
+++ src/qt/pivx/send.cpp	(date 1587713628926)
@@ -601,6 +601,7 @@
                 ui->btnChangeAddress->setActive(true);
             } else {
                 inform(tr("Invalid change address"));
+                QMetaObject::invokeMethod(dialog, "reset", Qt::DirectConnection);
             }
         }
     }

@random-zebra random-zebra force-pushed the 202004_GUI_reset_changeaddress branch from f822974 to b00b79c Compare April 24, 2020 13:48
@random-zebra
Copy link
Author

Added a commit to move the address validation inside SendChangeAddressDialog.
The reason is twofold: first, it addresses the concern raised by @Fuzzbawls about a previously (valid) address being persisted when entering an invalid one. Second, it offers better UX, removing the need for an extra click, in case of errors during copy-paste.

Fuzzbawls
Fuzzbawls previously approved these changes Apr 25, 2020
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

ACK b00b79c

@Fuzzbawls Fuzzbawls requested a review from furszy April 27, 2020 00:05
furszy
furszy previously approved these changes Apr 27, 2020
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

Left a minor comment. Tested ACK b00b79c

src/qt/pivx/sendchangeaddressdialog.cpp Outdated Show resolved Hide resolved
@random-zebra random-zebra dismissed stale reviews from furszy and Fuzzbawls via e03b881 April 27, 2020 09:48
@random-zebra random-zebra force-pushed the 202004_GUI_reset_changeaddress branch from b00b79c to e03b881 Compare April 27, 2020 09:48
@random-zebra
Copy link
Author

Error message changed to "Invalid address" / squashed into last commit.

@random-zebra random-zebra force-pushed the 202004_GUI_reset_changeaddress branch from e03b881 to 581fab6 Compare April 27, 2020 18:08
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

ACK 581fab6

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

re-ACK 581fab6

@Fuzzbawls Fuzzbawls merged commit 8b25a1e into PIVX-Project:master Apr 28, 2020
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request May 5, 2020
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request May 5, 2020
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request May 5, 2020
Fuzzbawls added a commit that referenced this pull request May 5, 2020
b97530a [GUI][Bug] Don't clear address label during send address validation (Fuzzbawls)
323ba4e [GUI][Bug] Fix editing of CS address labels (Fuzzbawls)
6fc5a2d [GUI] Re translate welcome wizard on every language selection. (furszy)
977d2ce BugFix welcome wizard not storing & loading the selected language. (furszy)
e15062a BugFix invalid language mapping. (furszy)
512708b [GUI] Update translations from Transifex for 4.1 (Fuzzbawls)
1f652be [GUI][Bug] Fix "Select all" / "Unselect all" logic in coincontrol (random-zebra)
c379e4b Removing warrows.dev dead seeder (furszy)
f65d370 [GUI] Reconnect CS owner address edit-label action (Fuzzbawls)
9ae6b2e Update copyright headers for files changed in 2020 (Fuzzbawls)
9ed3b99 [Trivial] Fix wording: "must be above" to "must be at least" (random-zebra)
da791e0 [GUI] Custom fee, min amount validation. (furszy)
15c5ddb [GUI] SendCustomFeeDialog: prevent user from saving insane fees Github-Pull: #1576 Rebased-From: d063a7f (random-zebra)
d0a9274 [GUI] Do not create new SettingsMultisendWidget (furszy)
00a3590 [QA][Bug] Shorter wallet_basic.py functional test Github-Pull: #1580 Rebased-From: df9fc17 (random-zebra)
0a15bba [Qt] fix transaction details output-index to reflect vout index (random-zebra)
908413d [Cleanup][GUI] remove unneeded loop in decomposeTransaction Github-Pull: #1578 Rebased-From: a5d3e8d (random-zebra)
bcd2461 [Trivial] Hide CWallet::MultiSend() call in main Github-Pull: #1577 Rebased-From: c67fcec (random-zebra)
e0e6ad2 [RPC] Disable/Hide multisend Github-Pull: #1577 Rebased-From: f50918e (random-zebra)
abc09b1 [GUI] Hide MultiSend in the options Github-Pull: #1577 Rebased-From: 7ac2aee (random-zebra)
a0a9fe7 [BUG] CustomFeeDialog: proper reset on clearAll Github-Pull: #1574 Rebased-From: a10c0f7 (random-zebra)
f849742 [Trivial] CustomFeeDialog: brace on newline Github-Pull: #1574 Rebased-From: 2a47123 (random-zebra)
fa9a688 [GUI] Update the record time when wtx.nTimeSmart changes Github-Pull: #1565 Rebased-From: 0c581fb (random-zebra)
f7ddff0 [Refactor] Encapsulate ComputeTimeSmart in CWalletTx Github-Pull: #1565 Rebased-From: b1dc5d3 (random-zebra)
da90bf4 [Cleanup] Remove unneeded GetComputedTxTime Github-Pull: #1565 Rebased-From: db99c41 (random-zebra)
7968c6d [Bug] Fix nTimeSmart computation (random-zebra)
b4e40a4 [Bug][Wallet] Fix CWallet::GetMinimumFee improperly set to feePerKb (random-zebra)
6fc725e [UX] SendChangeAddressDialog: validate address on save Github-Pull: #1551 Rebased-From: 581fab6 (random-zebra)
5709870 [GUI] Reset change address from within SendChangeAddressDialog Github-Pull: #1551 Rebased-From: ca99eaa (random-zebra)
08db3df [Bug][GUI] Send: Deactivate btnChangeAddress only when addr is not set Github-Pull: #1551 Rebased-From: 15b56a2 (random-zebra)
8b5dcf0 [Doc] Note that v3 onion addresses are not supported (Fuzzbawls)
d4247b0 [GUI] CoinControlDialog remove duplicate esc button (furszy)
48a9113 [GUI] CoinControl: mark delegated after setting checked state Github-Pull: #1543 Rebased-From: 1c1ee43 (random-zebra)
9599424 [GUI] MnWizard: validate IP (random-zebra)
d352817 [GUI][Bug] MnWizard: Fix validators for masternode alias and port (random-zebra)
c3a1611 [QA] Avoid printing to console during cache creation (random-zebra)
4a9f6c1 BugFix: cleanup invalid IsMasternodeReward method in OutPoint primitive. (furszy)
16bf3f3 Update hard coded seeds (Fuzzbawls)
28300e6 Update seed scripts from upstream (Fuzzbawls)
1dd592a [Qt] Don't translate dummy strings in mnrow (Fuzzbawls)
05ffd66 [Build] Disable apt-cacher for Windows WSL gitian setup (Fuzzbawls)
0e45518 [Trivial] jump line after function styling. (furszy)
22e0a1f [Trivial] Fix warning in mousePressEvent. (furszy)
b2701cc [GUI] Warn about change address not belonging to the wallet. (furszy)
6f81368 [GUI] Dark theme, inactive icons color fix. (furszy)
0a9fa8d [GUI] Add more room to contacts dropdown Github-Pull: #1425 Rebased-From: 452a461 (random-zebra)
93929a2 [GUI] Don't log to console by default. (furszy)

Pull request description:

  This brings the `4.1` branch up to date with all the merged PRs intended for the 4.1.0 release

ACKs for top commit:
  furszy:
    utACK b97530a
  random-zebra:
    utACK b97530a

Tree-SHA512: 25774120b34a6ba46598df0bcedd591e6b4435c96204a8b24df8b01060a1b3c364db06e44554068135541a9b2053c8e64f323a015b760b07e5e7dc8f9eaa880d
@random-zebra random-zebra removed the Needs Backport Placeholder tag for anything needing a backport to prior version branches label May 6, 2020
@random-zebra random-zebra deleted the 202004_GUI_reset_changeaddress branch September 24, 2020 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Unable to return Change Address to default
3 participants