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

util, rpc, gui: Changes for snapshotdownload and add feature sync from zero #2093

Merged
merged 5 commits into from
Apr 11, 2021

Conversation

iFoggz
Copy link
Member

@iFoggz iFoggz commented Apr 3, 2021

During the Forking there was a lot of snapshot downloads. From logs of my server there was many from the same ips (redownloads) this is bandwidth intensive and also increase the CDN monthly bill. These changes will improve on that.

This is complete however will be reviewing it and possibly making any changes and/or changes requested. I've tested Syncfromzero and tested the mandatory check.

Snapshotdownload changes:

  1. Check to see if there is a new mandatory version of the wallet available for download.
  • This is important in case a user does not notice update notifications or has them disabled.
  • This will check for an update of a mandatory version and inform the user and will not allow the download of the snapshot to avoid wasted resources and time.

Add Sync from zero feature:

  1. Add Arg argument for -syncfromzero
  2. Add gui menu option for syncfromzero
  3. Functions added
  • Add a function in Upgrade to handle the request for -syncfromzero. Function keeps the code neater and separated in a sense.
  • Add a function in UpgradeQt to handle gui menu option so we let the user know the result of the task before the wallet is shutdown.

Other changes:

  1. Fixed bug that resulted in client_message_out always being empty
  2. Do not allow -snapshotdownload and -syncfromzero to be called in conjunction
  3. Add unique scheduler function to support the bug fix since the scheduler does not need a reply from Version information from github. Keeps it more simple ex. []{}
  4. add bool to CheckForLatestVersion to determine if this call from snapshot or syncfromzero.
  • Changes to LogPrintf to log the correct function calling this
  • Add check to see if snapshotdownload was request and new mandatory is available
  • Add bool to CleanupBlockchainData to determine is syncfromzero or snapshotdownload was called (logging purpose)

@iFoggz
Copy link
Member Author

iFoggz commented Apr 3, 2021

Wording i'm sure could be adjusted as well but left it as is for comments

Copy link
Contributor

@RoboticMind RoboticMind left a comment

Choose a reason for hiding this comment

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

Glad to see something like this! I think there's some changes that can be made to the wording to help make it a little bit better.

Mostly looked at those messages to the user, so there may be other changes needed in the code itself

src/qt/bitcoingui.cpp Outdated Show resolved Hide resolved
src/qt/upgradeqt.cpp Outdated Show resolved Hide resolved
src/gridcoinresearchd.cpp Outdated Show resolved Hide resolved
src/qt/upgradeqt.cpp Outdated Show resolved Hide resolved
src/gridcoin/upgrade.cpp Outdated Show resolved Hide resolved
@iFoggz
Copy link
Member Author

iFoggz commented Apr 3, 2021

@RoboticMind Appreciate the input on the wording. once cycy and jim review those mentions i'll commit and push so we all can agree on what best wording is. :)

src/gridcoin/upgrade.cpp Outdated Show resolved Hide resolved
src/gridcoin/upgrade.cpp Outdated Show resolved Hide resolved
src/gridcoinresearchd.cpp Outdated Show resolved Hide resolved
src/qt/bitcoingui.cpp Outdated Show resolved Hide resolved
src/qt/upgradeqt.cpp Outdated Show resolved Hide resolved
src/qt/upgradeqt.cpp Outdated Show resolved Hide resolved
@iFoggz iFoggz force-pushed the snapshotchanges branch from 10be804 to a82160d Compare April 4, 2021 23:57
@iFoggz
Copy link
Member Author

iFoggz commented Apr 4, 2021

I believe I got everything I did add instructions for removing the files in event of failure

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.

"Sync from zero" is a rather informal term that the technical people in this community are familiar with, but it doesn't seem like it will be very intuitive to an average user. I think "sync from genesis" is a little better, but maybe "erase blockchain data" is a more meaningful and accurate label for this feature since there is no syncing directly involved. Thoughts?

Going to do some testing.

src/gridcoin/upgrade.cpp Outdated Show resolved Hide resolved
std::cout << _("Latest Version github data response:") << std::endl;
std::cout << VersionResponse << std::endl;

throw std::runtime_error(_("Failed to download snapshot as mandatory client is available for download."));
Copy link
Member

Choose a reason for hiding this comment

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

This message doesn't match the message printed to stdout. Can we pull the translation once and use it for both places?

Copy link
Member Author

Choose a reason for hiding this comment

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

this line is not in the qt side of things. could you elabarote where it doesn't match?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I meant the line above:

std::cout << _("Unable to download a snapshot, as the wallet has detected that a new mandatory version is available for install. The mandatory upgrade must be installed before the snapshot can be downloaded and applied.") << std::endl;

Since the message passed to the exception is translated as well, why not use the above in both places? That way, someone will get the full message in the log if they launch without a console (like Windows shortcuts), and it reduces work for translators.

Copy link
Member Author

Choose a reason for hiding this comment

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

i agree with doing that, there is a couple other places with same messages translated twice so i'm gonna whip something up singly that will address them all.

Copy link
Member

Choose a reason for hiding this comment

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

Did you still want to reuse ResetBlockchainMessages(UpdateAvailable) for the exception message to avoid a redundant translation?

src/qt/bitcoin.cpp Show resolved Hide resolved
src/qt/upgradeqt.cpp Outdated Show resolved Hide resolved
src/qt/bitcoingui.cpp Outdated Show resolved Hide resolved
src/gridcoin/upgrade.cpp Outdated Show resolved Hide resolved
@iFoggz
Copy link
Member Author

iFoggz commented Apr 7, 2021

"Sync from zero" is a rather informal term that the technical people in this community are familiar with, but it doesn't seem like it will be very intuitive to an average user. I think "sync from genesis" is a little better, but maybe "erase blockchain data" is a more meaningful and accurate label for this feature since there is no syncing directly involved. Thoughts?

Going to do some testing.

this still to be addressed.

Copy link
Contributor

@RoboticMind RoboticMind left a comment

Choose a reason for hiding this comment

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

Have some more grammatical/wording suggestions after looking over it again now after some of the changes


Msg.setIcon(QMessageBox::Question);
Msg.setText(tr("Do you want to delete blockchain data and sync from zero?"));
Msg.setInformativeText(tr("Warning: After the blockchain data is deleted the wallet will shutdown and when restarted will begin syncing from zero. Your balance will temporarily show as 0 GRC while syncing."));
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a missing comma here

Suggested change
Msg.setInformativeText(tr("Warning: After the blockchain data is deleted the wallet will shutdown and when restarted will begin syncing from zero. Your balance will temporarily show as 0 GRC while syncing."));
Msg.setInformativeText(tr("Warning: After the blockchain data is deleted, the wallet will shutdown and when restarted will begin syncing from zero. Your balance will temporarily show as 0 GRC while syncing."));

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

src/qt/upgradeqt.cpp Outdated Show resolved Hide resolved
CTxDB().Close();

if (Syncfromzero.SyncFromZero(app))
LogPrintf("Syncfromzero: Success!");
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of the logs message start with Syncfromzero: while others say Sync from zero: . Probably should pick one and stay with it. Maybe one of the two or even Sync From Zero: or SyncFromZero:

Copy link
Member Author

Choose a reason for hiding this comment

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

all logging is syncfromzero: its the message pop ups in qt that are spaced.

src/gridcoin/upgrade.cpp Outdated Show resolved Hide resolved
src/gridcoin/upgrade.cpp Outdated Show resolved Hide resolved
@cyrossignol
Copy link
Member

"Sync from zero" is a rather informal term that the technical people in this community are familiar with, but it doesn't seem like it will be very intuitive to an average user. I think "sync from genesis" is a little better, but maybe "erase blockchain data" is a more meaningful and accurate label for this feature since there is no syncing directly involved. Thoughts?

this still to be addressed.

"Reset blockchain data" is another option.

@jamescowens
Copy link
Member

I like "reset blockchain data".

@RoboticMind
Copy link
Contributor

I agree that's a good option. I think I've even seen something similarly named in other cryptocurrency wallets. That's why I put it in #2033 as a potential way to name it

@iFoggz
Copy link
Member Author

iFoggz commented Apr 11, 2021

hopefully i didnt miss anything. make sure its all good to go before i start tests again from changes.

@jamescowens jamescowens self-requested a review April 11, 2021 15:08
Copy link
Member

@jamescowens jamescowens left a comment

Choose a reason for hiding this comment

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

utACK. Time to test.

@jamescowens jamescowens requested a review from cyrossignol April 11, 2021 15:09
@jamescowens jamescowens merged commit 076e5b6 into gridcoin-community:development Apr 11, 2021
@jamescowens jamescowens changed the title Changes for snapshotdownload and add feature sync from zero util, rpc, gui: Changes for snapshotdownload and add feature sync from zero Jul 30, 2021
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.

Add "Sync From Zero" Menu Option & Startup Option
4 participants