Merge #13667: wallet: Fix backupwallet for multiwallets
ClosedPublic
Actions

Authored by nakihito on Dec 13 2019, 19:05.

Details

Reviewers

deadalnix
Fabien
jasonbcox

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGING903f7859b4cb: Merge #13667: wallet: Fix backupwallet for multiwallets
rABC903f7859b4cb: Merge #13667: wallet: Fix backupwallet for multiwallets

Summary

a1a998cf24c0cf1232e44ec8eaf2ad6875ab5153 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

Backport of Core PR13667
https://github.com/bitcoin/bitcoin/pull/13667/

Test Plan

make check
test_runner.py

Diff Detail

Repository

rABC Bitcoin ABC

Branch

PR13667

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 8561
Build 15112: Default Diff Build & Tests
Build 15111: arc lint + arc unit

Event Timeline

nakihito created this revision.Dec 13 2019, 19:05

Owners added a reviewer: Restricted Owners Package.Dec 13 2019, 19:05

Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 13 2019, 19:05

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8555: Diff 14848.Dec 13 2019, 19:16

The backport LGTM. This should be accompanied by a comment in the release notes, as it's pretty important for users that backup multiwallets from nested directories.

This revision now requires changes to proceed.Dec 13 2019, 21:16

In D4723#113830, @jasonbcox wrote:

The backport LGTM. This should be accompanied by a comment in the release notes, as it's pretty important for users that backup multiwallets from nested directories.

Good point. I'll write something up.

Added release notes.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8561: Diff 14860.Dec 13 2019, 23:37

Re-worded release notes to be less wonky.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8562: Diff 14862.Dec 14 2019, 00:32

jasonbcox accepted this revision.Dec 14 2019, 00:37

This revision is now accepted and ready to land.Dec 14 2019, 00:37

Closed by commit rABC903f7859b4cb: Merge #13667: wallet: Fix backupwallet for multiwallets (authored by Wladimir J. van der Laan <laanwj@gmail.com>, committed by nakihito). · Explain WhyDec 14 2019, 00:53