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

wallet: Fix backupwallet for multiwallets #13667

Merged
merged 1 commit into from Aug 7, 2018

Conversation

Projects
None yet
7 participants
@domob1812
Copy link
Contributor

domob1812 commented Jul 15, 2018

backupwallet was broken for multiwallets in their own directories (i.e. something like DATADIR/wallets/mywallet/wallet.dat). In this case, the backup would use DATADIR/wallets/wallet.dat as source file and not take the specific wallet's directory into account.

This led to either an error during the backup (if the wrong source file was not present) or would silently back up the wrong wallet; especially the latter behaviour can be quite bad for users.

wallet: Fix backupwallet for multiwallets
backupwallet was broken for multiwallets in their own directories
(i.e. something like DATADIR/wallets/mywallet/wallet.dat).  In this
case, the backup would use DATADIR/wallets/wallet.dat as source file
and not take the specific wallet's directory into account.

This led to either an error during the backup (if the wrong source
file was not present) or would silently back up the wrong wallet;
especially the latter behaviour can be quite bad for users.

@domob1812 domob1812 force-pushed the domob1812:fix-wallet-backup branch from 737ad09 to a1a998c Jul 15, 2018

@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented Jul 15, 2018

Would this require backport to the 0.16 branch?

@promag

This comment has been minimized.

Copy link
Member

promag commented Jul 15, 2018

utACK a1a998c. No travis?

@domob1812

This comment has been minimized.

Copy link
Contributor

domob1812 commented Jul 15, 2018

I think that 0.16 does not yet support multiwallets in their own subdirectories, does it? Then it would not require a backport.

@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented Jul 15, 2018

Thanks, just wanted to ask for clarity.

domob1812 added a commit to xaya/xaya that referenced this pull request Jul 16, 2018

Fix backupwallet with multi wallets.
This is a backport of the upstream fix
bitcoin/bitcoin#13667 to 1.0.  It fixes
#48 for the branch.
@DrahtBot

This comment has been minimized.

Copy link
Contributor

DrahtBot commented Jul 16, 2018

No more conflicts as of last run.
@domob1812

This comment has been minimized.

Copy link
Contributor

domob1812 commented Aug 5, 2018

Is there anything I need to do here?

@MarcoFalke MarcoFalke added this to the 0.17.0 milestone Aug 5, 2018

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Aug 7, 2018

utACK a1a998c

@laanwj laanwj merged commit a1a998c into bitcoin:master Aug 7, 2018

laanwj added a commit that referenced this pull request Aug 7, 2018

Merge #13667: wallet: Fix backupwallet for multiwallets
a1a998c wallet: Fix backupwallet for multiwallets (Daniel Kraft)

Pull request description:

  `backupwallet` was broken for multiwallets in their own directories (i.e. something like `DATADIR/wallets/mywallet/wallet.dat`).  In this case, the backup would use `DATADIR/wallets/wallet.dat` as source file and not take the specific wallet's directory into account.

  This led to either an error during the backup (if the wrong source file was not present) or would silently back up the wrong wallet; especially the latter behaviour can be quite bad for users.

Tree-SHA512: 7efe2450ca047e40719fcc7cc211ed94699056020ac737cada7b59e8240298675960570c45079add424d0aab520437d5050d956acd695a9c2452dd4317b4d2c4

@domob1812 domob1812 deleted the domob1812:fix-wallet-backup branch Aug 7, 2018

@Hary2511

This comment has been minimized.

Copy link

Hary2511 commented Aug 7, 2018

What should I do here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment