Skip to content
This repository has been archived by the owner on Sep 5, 2020. It is now read-only.

Adds an opt out option to the node auto updater #1450

Merged
merged 31 commits into from
Nov 28, 2016
Merged

Conversation

frozeman
Copy link
Contributor

@frozeman frozeman commented Nov 22, 2016

This adds an opt out option where it saves the skipped node version in a skippedNodeVersion.json file. As long as the current to be updated node version matches the entry in the file it wont ask for update, otherwise popup the question window, which shows details about the update node.

TODO: we need to add md5 hash verification to ethereum client binaries updater, which sits now at:
https://github.com/ethereum/ethereum-client-binaries

screen shot 2016-11-22 at 17 47 00

Please all make sure to test this extensively.

  • This should involve, removing the clientBinaries.json from the mist data folder.
  • changing the file clientBinaries.json a bit (change on version number etc.) to trigger a update request
  • remove binaries and see if they update properly
  • ideally remove your global geth instance too
  • test the skipping, it will be stored in skippedNodeVersion.json in the mist data folder
  • downgrade a version and test the successful upgrade process.
    etc..

You can use http-server npm package to start a server inside any folder. Start one in the mist folder so you can serve the clientBinaries.json by yourself and make changes to it to test.

You just need to then change the line 58, of the modules/clientBinaryManager.js, for testing

@mention-bot
Copy link

@frozeman, thanks for your PR! By analyzing the history of the files in this pull request, we identified @alexvandesande, @luclu and @hiddentao to be potential reviewers.

@luclu
Copy link
Contributor

luclu commented Nov 22, 2016

I created a PR for md5 checksums: ethereum/ethereum-client-binaries#6

@luclu
Copy link
Contributor

luclu commented Nov 22, 2016

Notes / TODO (ongoing):

  • pressing "skip" on the initial startup (no downloaded nodes) will still download the binary
  • pressing "check for Ethereum node updates" while no updates available won't give any feedback to user, "... (Last checked ${time} ago)" could be added to the menu-item after checked
  • the popup looks neat!
  • "md5" key/value not recognised
  • Fabian still didn't install or configure eslint in his editor... dude... 🍤
  • code is currently locked in on geth, should be client agnostic

@evertonfraga
Copy link
Member

Still testing. some findings:

✅ From a clean install I managed to upgrade geth from 1.4.17 > 1.4.18 > 1.5.2 successfully.

⚠️ if Geth has no executable permissions, the splash gets stuck on that step. I know it's rather unlikely, but we can handle this better.

❌ If it's the first run (any nodes downloaded), the user will be prompted to "update". In case the user closes/skips the window, it cannot move forward, as they don't have any node. The user will only be able to start mist properly if it deletes skippedNodeVersion.json and accept the update request.

@alexvandesande
Copy link
Collaborator

Was about to make a PR to this PR but ended up committing the most important change (a9745a0) here.

Anyway, now both "update available window" have the same basic structure, removed some redundant code and language and standardized it to the look and feel of our windows and buttons:

screen shot 2016-11-24 at 18 30 32

screen shot 2016-11-24 at 18 30 33

@frozeman
Copy link
Contributor Author

frozeman commented Nov 25, 2016

@evertonfraga for the first time thats an issue, true. will fix. (Tho, i thought this shouldn't happen, as the of line: https://github.com/ethereum/mist/blob/nodeDownloaderFix/modules/clientBinaryManager.js#L88)

But if you skipped update, you can click the main menu and click "check for node updates". This will erase the skippedVersion file and check for updates again.

@alexvandesande looks good. Please add a third button, skip for now. Or do we want to do this behaviour only if the user closed the update window?

@luclu
Copy link
Contributor

luclu commented Nov 25, 2016

@alexvandesande I feel like the URL would look better if not wrapped

@evertonfraga
Copy link
Member

evertonfraga commented Nov 25, 2016

Well, I've ironed up my test environment and probably I've messed with some json file on the first tests — sorry for the false negative. Today i did an intensive batch of checks and everything worked properly.

  • All the skips working OK.
  • All updates worked OK.
  • Force node update OK.
  • skippedNodeVersion.json working OK.

I've been struggling to test on windows, though. For an undetected reason, I can't parse any json from remote server anymore. So got is not working, even on old Mist versions. Of course it's a problem of my setup, so maybe if @luclu could test on windows while I make this work again, I'd be glad.

@alexvandesande
Copy link
Collaborator

@luclu I increased the width 30px so it won't break like that 😜

@frozeman frozeman merged commit 3dee67e into develop Nov 28, 2016
@frozeman frozeman deleted the nodeDownloaderFix branch November 28, 2016 12:42
@evertonfraga evertonfraga modified the milestone: 0.8.8 Dec 5, 2016
@lock
Copy link

lock bot commented Mar 31, 2018

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked and limited conversation to collaborators Mar 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants