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

walletdb: change migration script #480

Closed

Conversation

tynes
Copy link
Contributor

@tynes tynes commented Jul 23, 2020

This PR moves the migration out of the node boot process and instead uses a script to do the migration. Users giving feedback seemed to like doing the migration through a script instead of through stopping, pulling a new version and restarting the node.

@pinheadmz
Copy link
Member

pinheadmz commented Jul 27, 2020

I'm OK with this approach, from the developer call it sounds like any update where backing up the walletDB is prudent should not be done automatically, and we already discussed why rescanning automatically isn't a good idea either. I think this is good design practice for hsd going forward, although I'll also add here that in #396 the migration was executed automatically in updated software. In bcoin-org/bcoin#703 the user had to migrate the chainDB manually after updating bcoin, but before launching it again. If updated software is run without the migration, an assertion error stops the process: https://github.com/bcoin-org/bcoin/blob/master/lib/blockchain/chaindb.js#L63

So I think @tynes if you have the time what I think would be good here is this:

  • In walletDB, check for the migration flag this.db.has(layout.M.encode(0)) in walletdb.open() and log an error. This will be output to debug log on every single launch of hsd until the issue is fixed. That error should either specify "see changelog" or directly reference the script migrate/wallet0.js.

  • Add a description of the process (including how to back up the walletDB) in the CHANGELOG

  • In the root README, right after INSTALL and a section UPDATING and if nothing else, just add "always read the changelog before updating"

Another option would be test for the migrate flag and instead of loging an error, just stop hsd with process.exit() and throw a descriptive error message -- thought on this @turbomaze ?

Will test the actual migration script shortly

migrate/wallet0.js Outdated Show resolved Hide resolved
@pinheadmz
Copy link
Member

Hoping we can close this and rally around merging #415

@tynes
Copy link
Contributor Author

tynes commented Aug 10, 2020

Closing in favor of #415

@tynes tynes closed this Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants