Paths

Table of Contentst

Backup and restore the wallet's HD Master Seed to recover lost wallet files.
AbandonedPublic
Actions

Authored by Mengerian on Apr 3 2018, 05:58.

Details

Reviewers

deadalnix
rocks

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Summary

This commit adds the ability to backup and restore wallets using the HD Master Seed defined in BIP 32.

Please feel free to provide any feedback or comments. I always wanted this functionality in the reference client and thought others would find it useful as well. I tried to make this version a fully functional change that minimizes impact to the rest of the client, but if other approaches would be better, etc. please let me know.

This change enables wallets to be saved offline using a short key instead of requiring a prior wallet.dat file to restore. The ability to backup and restore from a key is available in other clients, but not in Bitcoin core clients. Bitcoin core exports the HD Master Extended Key in ‘dumpwallet’ and this key can be used to manually recover addresses, however the core client requires the HD Master Seed to function and this is only found in the wallet.dat file itself.

To implement recovery this commit adds a new “Encrypt Wallet Advanced” menu option and dialog window. Recovery during wallet encryption was the least disruptive method because normal wallet encryption already generates a new HD Master Seed and inserts it into the wallet to use from that point forward. The recovery process implemented instead inserts a manually entered HD Master Seed to use instead of the auto generated seed. Once the recovered seed is added a simple ‘-rescan=1’ scans the blockchain and repopulates the wallet.

The Encrypted Wallet Advanced dialog offers three options:

Standard wallet encryption – This option encrypts the wallet with a new auto generated seed, this is identical to normal wallet encryption.
Recover wallet – This option allows the user to enter a backup seed to recover the wallet and then encrypts the wallet as normal
Manually generate seed – This option allows the user to roll dice to create a new seed which guarantees full entropy and then encrypts the wallet as normal

The dialog window provides user instructions depending on the options selected. Keys are exported and imported in base58 format. The -rescan command was also modified to optionally force rescanning from the genesis block. This was needed because by default -rescan only scans from the time the wallet.dat file was created, which for newly created wallet.dat files would skip all prior addresses trying to be recovered.

Below are files changed or added along with a description of the changes. I also added comments in the code where I thought it would help.

Changes by file

New dialog window and methods

A src/qt/encryptwalletadvanceddialog.cpp
A src/qt/encryptwalletadvanceddialog.h
A src/qt/forms/encryptwalletadvanceddialog.ui

Update wallet routines to encrypt using user entered backup seed
Update ‘-rescan’ to have ‘-rescan=1’ force a rescan from the genesis block

M src/wallet/wallet.cpp
M src/wallet/wallet.h

Export the HD Master Seed in ‘dumpwallet’

M src/wallet/rpcdump.cpp

Add “Encrypt Wallet Advanced” menu option and associated plumbing

M src/qt/bitcoingui.cpp
M src/qt/bitcoingui.h
M src/qt/walletframe.cpp
M src/qt/walletframe.h
M src/qt/walletview.cpp
M src/qt/walletview.h

Pass recovery seed from dialog window to wallet encryption routines

M src/qt/walletmodel.cpp
M src/qt/walletmodel.h

New file additions to make

M contrib/bitcoin-qt.pro
M src/Makefile.qt.include
M src/qt/CMakeLists.txt

Test Plan

On testnet, use an existing wallet and backup seed with 'dumpwallet'
Delete wallet.dat file
Restart bitcoin-qt
Select menu option 'Encrypt Wallet Advanced'
Select recover wallet option #2
Enter seed from dump wallet file
Exit bitcoin-qt
Restart with '-rescan=1'
Client should scan from the genesis block and repopulate all address transactions

Diff Detail

Repository

rABC Bitcoin ABC

Branch

hdrestore

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 2229
Build 2601: Bitcoin ABC Buildbot (legacy)
Build 2600: arc lint + arc unit

Event Timeline

rocks created this revision.Apr 3 2018, 05:58

Owners added a reviewer: Restricted Owners Package.Apr 3 2018, 05:58

Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 3 2018, 05:58

rocks edited the summary of this revision. (Show Details)Apr 3 2018, 06:00

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging passed.

Harbormaster completed remote builds in B2229: Diff 3377.Apr 3 2018, 06:51

It seems like this is doing many things at once and comes with no tests. This needs to be split up and the relevant parts should see tests added.

For instance, there are change in the rescan mechanism that seems fairly independent from the rest of the changes.

Lastly, please use clang-format 4.0 for formatting.

This revision now requires changes to proceed.Apr 5 2018, 15:05

Sure, thanks for the comments. I have a few questions and will base another pass on that.

Are there any preferred or required mechanisms to implement unit tests? Or can any approach be used as long as some unit testing is added?

Regarding splitting it up, how fine grained does a commit need to be?
- The rescan changes are <1% of the changes and easy to separate
- The dump wallet changes are also easy to separate.
- However the new dialog box, menu option and connecting them are >90% of the line changes and need to be done at the same time, or parts would be implemented that are not accessible (for example a dialog box with no ability to access it). Another approach might be to implement this as a new RPC command first, and only add the dialog box in a subsequent part.

If this is redone as a series of 5 separate commits (for example), would all need to be submitted at the same time, or is it better to submit them one at a time with an understanding on what the final goal is?

Lastly, is there interest in having this feature? i.e. would you like to see this worked on and added? Or is it a waste of time from your perspective and unnecessary.

In D1237#24578, @rocks wrote:

Sure, thanks for the comments. I have a few questions and will base another pass on that.

Are there any preferred or required mechanisms to implement unit tests? Or can any approach be used as long as some unit testing is added?

Regarding splitting it up, how fine grained does a commit need to be?

The rescan changes are <1% of the changes and easy to separate

The dump wallet changes are also easy to separate.

However the new dialog box, menu option and connecting them are >90% of the line changes and need to be done at the same time, or parts would be implemented that are not accessible (for example a dialog box with no ability to access it). Another approach might be to implement this as a new RPC command first, and only add the dialog box in a subsequent part.

If this is redone as a series of 5 separate commits (for example), would all need to be submitted at the same time, or is it better to submit them one at a time with an understanding on what the final goal is?

Lastly, is there interest in having this feature? i.e. would you like to see this worked on and added? Or is it a waste of time from your perspective and unnecessary.

Unit testing is preferred. If unit tests are not feasible, then integration/functional tests are the next best bet.

Finer grained the better. The rescan changes being so small will be easy to review. Some larger diffs (like the dialog box) are inevitable, but keep in mind that they become much easier to review when other changes (like rescan) are not included with the diff.

You can do either. If you're going to submit them all at once, just make sure to submit stacked diffs properly via arc diff HEAD~ or something similar (assuming the diff you want to submit is at the top of the stack). If this proves too annoying for you, feel free to submit one-by-one.

Mengerian commandeered this revision.Oct 13 2023, 14:35

Mengerian abandoned this revision.

Mengerian added a reviewer: rocks.