Page MenuHomePhabricator

Merge #14146: wallet: Remove trailing separators from -walletdir arg
ClosedPublic

Authored by jasonbcox on Jan 17 2020, 18:44.

Details

Summary

2d471636eb9160ab51b08e491e3f003f57adbc36 wallet: Remove trailing separators from -walletdir arg (Pierre Rochard)
ea3009ee942188750480ca6cc273b2b91cf77ded wallet: Add walletdir arg unit tests (Pierre Rochard)

Pull request description:

If a user passes in a path with a trailing separator as the `walletdir`, multiple BerkeleyEnvironments may be created in the same directory which can lead to data corruption.

Discovered while reviewing https://github.com/bitcoin/bitcoin/pull/12493#issuecomment-417147646

Tree-SHA512: f2bbf1749d904fd3f326b88f2ead58c8386034355910906d7faea155d518642e9cd4ceb3cae272f2d9d8feb61f126523e1c97502799d24e4315bb53e49fd7c09

Backport of Core PR14146
https://github.com/bitcoin/bitcoin/pull/14146/files

Test Plan

ninja check
Autotools build on CI

Diff Detail

Repository
rABC Bitcoin ABC
Branch
pr14146
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 9030
Build 16022: Default Diff Build & Tests
Build 16021: arc lint + arc unit

Event Timeline

deadalnix requested changes to this revision.Jan 17 2020, 22:09
deadalnix added inline comments.
src/wallet/test/init_test_fixture.cpp
8

Why?

src/wallet/test/init_tests.cpp
5

Why?

18

So I added a BOOST_CHECK(false); because this seemed suspicious that this would run. As it turns out, the cmake build indeed doesn't run this - but make does.

This revision now requires changes to proceed.Jan 17 2020, 22:09
src/wallet/test/init_tests.cpp
18

Very good catch. Fixed in D5008

jasonbcox added inline comments.
src/wallet/test/init_test_fixture.cpp
8

Needed for gArgs. I imagine out of order backports elsewhere is the cause, because Core adds this later in PR16278.

src/wallet/test/init_tests.cpp
5

Original PR has calls like bool result = g_wallet_init_interface.Verify();
In our codebase, we need bool result = g_wallet_init_interface.Verify(Params()); the include is for Params()

jasonbcox marked 2 inline comments as done.

Rebase on D5008

This revision is now accepted and ready to land.Jan 19 2020, 15:07