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: Implement consolidateunspent wizard #2125

Conversation

jamescowens
Copy link
Member

@jamescowens jamescowens commented May 5, 2021

This implements a three step wizard that leads the user through the process of selecting inputs, selecting a destination, and
then reviewing the overall transaction.

Select Inputs:

The select inputs screen uses similar code to that in coincontroldialog to support the consolidate button there. Pointers to the coincontrol data structures constructed in sendcoinsdialog are passed into both coincontrol and the consolidateunspentwizard to faciliate using the underlying machinery in a unified manner. This is possible because both coincontrol and onsolidateunspendwizard are called with the sendcoinsdialog and are modal.

Note that there is a stop sign and the next button is disabled if more than 600 outputs are selected. The next button is also disabled if less than 2 outputs are selected (as it makes no sense to consolidate when there is no consolidation achievable based on the selection). If a filter operation is applied based on the filter criteria, and that criteria would result in more than 600 inputs being selected, the selection is reduced to 600 inputs and a warning triangle is presented.

Select Destination Address:

If all of the inputs selected in the prior page are from one address, then that address will already be preselected (but allow the opportunity for it to be changed by the user prior to pressing next). If the inputs selected correspond to more than one address, an address will NOT be pre-selected, and the user will be required to pick an address to proceed. The next button will be disabled until a valid address is selected for the destination.

Confirmation:

The final screen presents the details of the intended transaction for review by the user. When the "Finish" button is pressed, the transaction to send is filled in in the send dialog sreen, and the user can review the details again if desired and press the send button to send.

NOTE: I know the appearance needs to be fine-tuned. I am not necessarily the best at making pretty things UI-wise, but the basics are here. I also think we can move the Consolidate Wizard button out of the coincontrol frame, so that it is visible whether or not the advanced coin control is enabled in the config.

This further addresses #1984.

@jamescowens jamescowens self-assigned this May 5, 2021
@jamescowens jamescowens added this to the Ingrid milestone May 5, 2021
@jamescowens jamescowens force-pushed the ui_consolidate_unspent_wizard branch from 99ac71a to e17906a Compare May 6, 2021 01:22
@jamescowens jamescowens changed the title Implement consolidateunspent wizard gui: Implement consolidateunspent wizard May 6, 2021
@jamescowens jamescowens force-pushed the ui_consolidate_unspent_wizard branch from e17906a to 2e4e722 Compare May 6, 2021 03:15
@jamescowens
Copy link
Member Author

jamescowens commented May 6, 2021

For some devilish reason the table widget on the select destination page shows numbers for the headers instead of the column labels. I have no idea why. The similar table widget in the consolidate button in coincontrol displays correctly. I am having @iFoggz see if he can find what I must be missing. I am marking it ready for review anyway.

@cyrossignol I am having you review this, since you were looking at the coincontrol code earlier and also just put up a complimentary PR. I think we should show the consolidate wizard button irrespective of the new "toggle" mode you implemented in #2126. We may want to favor this wizard in place of the consolidate button in coincontrol itself.

Since this wizard actually works with the underlying data structures in the coin control/send dialog, it can be used instead of, or partially or completely (pressing finish), and then go to coin control to make further adjustments, although this wizard exposes the subset of coin control functionality needed just for consolidation purposes.

@jamescowens jamescowens marked this pull request as ready for review May 6, 2021 03:18
@jamescowens jamescowens force-pushed the ui_consolidate_unspent_wizard branch from 2e4e722 to b0feb61 Compare May 6, 2021 04:12
@jamescowens
Copy link
Member Author

The header problem is fixed. @iFoggz found it. I was using ui->addressTableWidget->clear(), instead of ui->addressTableWidget->clearContents() in SetAddressList!

Copy link
Member

@cyrossignol cyrossignol left a comment

Choose a reason for hiding this comment

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

Some initial comments to start. Will circle back when I have time to do more thorough testing.

src/qt/coincontroldialog.h Outdated Show resolved Hide resolved
src/qt/consolidateunspentdialog.h Outdated Show resolved Hide resolved
src/qt/coincontroldialog.cpp Show resolved Hide resolved
src/qt/consolidateunspentwizardselectdestinationpage.cpp Outdated Show resolved Hide resolved
src/qt/consolidateunspentwizardselectinputspage.cpp Outdated Show resolved Hide resolved
src/qt/sendcoinsdialog.cpp Show resolved Hide resolved
@jamescowens jamescowens force-pushed the ui_consolidate_unspent_wizard branch 3 times, most recently from d2ba0fb to 1234333 Compare May 13, 2021 02:11
@jamescowens
Copy link
Member Author

jamescowens commented May 13, 2021

I think this now works the way I had intended. If you cancel out of the wizard after having selected inputs, you will now see they are correctly set in the coin control label fields (this was not working quite correctly before), and you can go to coincontrol and make further adjustments (this WAS working correctly). If you go back to the wizard, state is maintained, and you can pick up where you left off there if you choose.

The select destination address page in the wizard is now a bit smarter than the consolidate button in coincontrol. I think I need to retrofit the improvements from the select destination page to there.

@jamescowens jamescowens force-pushed the ui_consolidate_unspent_wizard branch from 1234333 to d6a5af7 Compare May 14, 2021 16:41
This implements a three step wizard that leads the user through
the process of selecting inputs, selecting a destination, and
then reviewing the overall transaction.

Select Inputs:

The select inputs screen uses similar code to that in coincontroldialog
to support the consolidate button there. Pointers to the coincontrol
data structures constructed in sendcoinsdialog are passed into
both coincontrol and the consolidateunspentwizard to faciliate using
the underlying machinery in a unified manner. This is possible because
both coincontrol and consolidateunspendwizard are called with the
sendcoinsdialog and are modal.

Note that there is a stop sign and the next button is disabled if more than
600 outputs are selected. The next button is also disabled if less than 2
outputs are selected (as it makes no sense to consolidate when there is no
consolidation achievable based on the selection). If a filter operation
is applied based on the filter criteria, and that criteria would result in
more than 600 inputs being selected, the selection is reduced to 600 inputs
and a warning triangle is presented.

Select Destination Address:

If all of the inputs selected in the prior page are from one address, then
that address will already be preselected (but allow the opportunity for it
to be changed by the user prior to pressing next). If the inputs selected
correspond to more than one address, an address will NOT be pre-selected,
and the user will be required to pick an address to proceed. The next button
will be disabled until a valid address is selected for the destination.

Confirmation:

The final screen presents the details of the intended transaction for review
by the user. When the "Finish" button is pressed, the transaction to send
is filled in in the send dialog sreen, and the user can review the details
again if desired and press the send button to send.
This corrects the mapping of change to the CoinControlChangeLabel
field.
@jamescowens jamescowens merged commit 453a632 into gridcoin-community:development May 16, 2021
@jamescowens jamescowens deleted the ui_consolidate_unspent_wizard branch May 23, 2021 00:23
jamescowens added a commit to jamescowens/Gridcoin-Research that referenced this pull request Aug 1, 2021
Added
 - util, rpc. gui: Changes for snapshotdownload and add feature sync from zero gridcoin-community#2093 (@iFoggz)
 - gui: Implement GUI version of consolidateunspent (coin control part) gridcoin-community#2111 (@jamescowens)
 - gui: Implement consolidateunspent wizard gridcoin-community#2125 (@jamescowens)
 - qt: Add antialiasing to traffic graph widget gridcoin-community#2150 (@barton2526)
 - util: Port of ArgsManager and a significant subset of src/util gridcoin-community#2146 (@jamescowens)
 - doc: add issue templates for bug reports and feature requests gridcoin-community#2147 (@Pythonix)
 - gui, rpc: Implement dynamic stakesplitting control, settings changes via rpc, and dynamic changes to sidestaking via rpc gridcoin-community#2164 (@jamescowens)
 - rpc: Implement getblocksbatch gridcoin-community#2205 (@jamescowens)
 - voting, rpc, gui: Implement demand loading of historical poll by poll id and AVW calculation gridcoin-community#2210 (@jamescowens)
 - gui: Show GUI error dialog if command line parsing fails gridcoin-community#2218 (@jamescowens)
 - gui: Implement close confirmation. gridcoin-community#2216 (@denravonska)
 - build: Use -fstack-reuse=none gridcoin-community#2232 (@barton2526)

Changed
 - doc: Update build doc gridcoin-community#2078 (@iFoggz)
 - gui: Normalize button and input control appearance gridcoin-community#2096 (@cyrossignol)
 - consensus: Implement GetMinimumRequiredConnectionsForStaking gridcoin-community#2097 (@jamescowens)
 - refactor: move CTransaction to primitives gridcoin-community#2006 (@div72)
 - consensus, refactor, test: Merkle gridcoin-community#2094 (@div72)
 - gui: Update diagnostics gridcoin-community#2095 (@jamescowens)
 - gui: Refresh UI styles and sidebar/statusbar design gridcoin-community#2102 (@cyrossignol)
 - gui: Set standard base Qt style on Windows and macOS gridcoin-community#2114 (@cyrossignol)
 - build, refactor: bump to C++17 gridcoin-community#2113 (@div72)
 - util, rpc, gui: Implement GetMaxInputsForConsolidationTxn() gridcoin-community#2119 (@jamescowens)
 - gui: Refresh overview page design gridcoin-community#2117 (@cyrossignol)
 - depends: change boost mirror gridcoin-community#2122 (@div72)
 - refactor: small cleanup gridcoin-community#2123 (@div72)
 - build: Update depends Qt recipe to version 5.12.10 gridcoin-community#2129 (@cyrossignol)
 - build: Bump Codespell to 2.0.0 gridcoin-community#2135 (@barton2526)
 - gui: Refresh "send coins" page design gridcoin-community#2126 (@cyrossignol)
 - gui: Optimize locks to improve responsiveness gridcoin-community#2137 (@cyrossignol)
 - gui: Refresh "receive payment" page design gridcoin-community#2138 (@cyrossignol)
 - gui: Add empty placeholder to recent transactions list gridcoin-community#2140 (@cyrossignol)
 - gui: Refresh transaction history page design gridcoin-community#2143 (@cyrossignol)
 - gui: Refresh address book page design gridcoin-community#2145 (@cyrossignol)
 - doc: Update http to https where possible gridcoin-community#2148 (@barton2526)
 - depends: Update dependencies gridcoin-community#2153 (@barton2526)
 - depends: Bump python to 3.6 gridcoin-community#2159 (@barton2526)
 - test: Update cppcheck linter to c++17 gridcoin-community#2157 (@barton2526)
 - test: Drop Travis specific workarounds, Mention commit id in error, Fix typos, Update spellcheck ignore words gridcoin-community#2158 (@barton2526)
 - gui: Overhaul the voting UI gridcoin-community#2151 (@cyrossignol)
 - wallet: simplify nTimeSmart calculation gridcoin-community#2144 (@div72)
 - gui: Refresh checkbox and radio button styles gridcoin-community#2170 (@cyrossignol)
 - build: Bump libevent to 2.1.11 gridcoin-community#2172 (@barton2526)
 - build: Update native_mac_alias, Remove big sur patch file in qt recipe gridcoin-community#2173 (@barton2526)
 - docs: Misc Grammar gridcoin-community#2176 (@barton2526)
 - build: miniupnpc 2.2.2 gridcoin-community#2179 (@barton2526)
 - rpc: Refresh rainbymagnitude gridcoin-community#2163 (@jamescowens)
 - util: optimize HexStr gridcoin-community#2185 (@div72)
 - refactor: misc style changes gridcoin-community#2177 (@div72)
 - rpc: consolidatemsunspent changes. gridcoin-community#2136 (@iFoggz)
 - refactor: Replace "GlobalStatus" state management gridcoin-community#2183 (@cyrossignol)
 - rpc, util: Remove use of ArgsManager::NETWORK_ONLY for now gridcoin-community#2190 (@jamescowens)
 - doc: Replace hidden service with onion service, Capitalize "Tor" gridcoin-community#2193 (@barton2526)
 - gui: Update Qt Linguist localization files gridcoin-community#2192 (@cyrossignol)
 - script: Shell script cleanups gridcoin-community#2195 (@barton2526)
 - build: set minimum required Boost to 1.58.0 gridcoin-community#2194 (@barton2526)
 - build, util: Prevent execution for Windows versions less than Windows 7 gridcoin-community#2203 (@jamescowens)
 - build: Tweak NSIS Windows installer gridcoin-community#2204 (@jamescowens)
 - build: Add bison in depends gridcoin-community#2206 (@iFoggz)
 - build: macOS toolchain bump gridcoin-community#2207 (@div72)
 - doc: Update build-unix.md gridcoin-community#2212 (@springfielddatarecovery)
 - build: Bump minimum python version to 3.6, Remove python2 references gridcoin-community#2219 (@barton2526)
 - depends: Change openSSL source path to Github gridcoin-community#2237 (@barton2526)
 - doc: Fix typo in bug report template gridcoin-community#2243 (@jamescowens)
 - ci: fold depends output gridcoin-community#2244 (@div72)

Removed
 - wallet: remove dead hardcoded addnodes gridcoin-community#2116 (@sweede-se)
 - rpc: Remove readconfig gridcoin-community#2248 (@jamescowens)
 - rpc: Remove obsolete comparesnapshotaccrual RPC function gridcoin-community#2100 (@jamescowens)
 - rpc: Remove memorypool RPC Command gridcoin-community#2214 (@RoboticMind)
 - rpc: Remove deprecated RPC commands gridcoin-community#2101 (@jamescowens)
 - Remove CCT from README, add Discord gridcoin-community#2134 (@barton2526)
 - refactor: Remove obsolete pubsub method definitions gridcoin-community#2191 (@barton2526)
 - refactor: Remove msMiningErrorsIncluded & msMiningErrorsExcluded gridcoin-community#2215 (@RoboticMind)
 - qt: Remove obsolete topLevelWidget(), Remove obsolete QRegExpValidator gridcoin-community#2198 (@barton2526)
 - net: Drop support of the insecure miniUPnPc versions gridcoin-community#2178 (@barton2526)
 - log: remove deprecated db log category gridcoin-community#2201 (@barton2526)
 - doc: Remove CCT from README and release process docs gridcoin-community#2175 (@barton2526)
 - build: Remove travis references gridcoin-community#2156 (@barton2526)

Fixed
 - gui: Fix macOS and designer font sizes gridcoin-community#2098 (@cyrossignol)
 - gui: Have the TrafficGraphWidget respect the selected stylesheet. gridcoin-community#2107 (@jamescowens)
 - gui: Fix macOS display inconsistencies gridcoin-community#2106 (@cyrossignol)
 - gui: Fix RPC console auto-complete background color gridcoin-community#2108 (@cyrossignol)
 - gui: Avoid reloading redundant stylesheets gridcoin-community#2109 (@cyrossignol)
 - gui: Fix "no active beacon" status gridcoin-community#2110 (@cyrossignol)
 - gui: Fix dark theme link text color visibility gridcoin-community#2115 (@cyrossignol)
 - scraper, util, qt: Fix several deprecations and warnings gridcoin-community#2131 (@jamescowens)
 - gui: Fix duplicate time in GUIUtil::dateTimeStr() gridcoin-community#2139 (@cyrossignol)
 - gui: Fix debug console traffic graph legend colors gridcoin-community#2142 (@cyrossignol)
 - gui: Fix nomenclature gridcoin-community#2104 (@jamescowens)
 - doc: Fix Typos gridcoin-community#2149 (@barton2526)
 - doc: Fix "master" branch build status badge in readme gridcoin-community#2167 (@cyrossignol)
 - gui: Fix Inter font rendering on Windows with FreeType gridcoin-community#2169 (@cyrossignol)
 - gui: Fix assert on non-existent data directory and GUI datadir chooser corner case issues gridcoin-community#2174 (@jamescowens)
 - gui: Fix display artifact in poll loading indicator gridcoin-community#2180 (@cyrossignol)
 - rpc, logging: Minor fixes for sidestake logging gridcoin-community#2187 (@jamescowens)
 - gui: Fix fractional scaling for dialog sizes gridcoin-community#2189 (@cyrossignol)
 - doc: Random fixes gridcoin-community#2197 (@barton2526)
 - doc: getbalance should say GRC not "btc" gridcoin-community#2199 (@barton2526)
 - net: Add missing verification of IPv6 address in CNetAddr::GetIn6Addr¦ gridcoin-community#2200 (@barton2526)
 - doc: remove duplicate line from .gitignore gridcoin-community#2202 (@Pythonix)
 - util: Tweak exception handling in MilliTimer class to eliminate compiler warnings gridcoin-community#2233 (@jamescowens)
 - depends: patch missing include in qt gridcoin-community#2234 (@div72)
 - wallet, rpc: Check each input for IsMine() in GetAddressGroupings gridcoin-community#2242 (@jamescowens)
 - util, qt: Fix snapshot download gridcoin-community#2246 (@jamescowens)
 - gui: Fix Column Widths in RPC Console. Elide long strings in their center. Indent user agent. gridcoin-community#2241 (@barton2526)
 - qt: Fix crash during download snapshot on macOS gridcoin-community#2250 (@jamescowens)
 - qt: Don't allow to open the debug window during splashscreen & verification state gridcoin-community#2245 (@barton2526)
 - gui: Fix address book selected model record when editing gridcoin-community#2253 (@cyrossignol)
 - researcher: Check wallet status before beacon renewal gridcoin-community#2254 (@cyrossignol)
 - qt: Prevent pasting (no label) as label in consolidation transaction gridcoin-community#2255 (@jamescowens)
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.

2 participants