Skip to content

Commit dbd7a91

Browse files
author
MarcoFalke
committed
Merge #19310: wallet: BerkeleyDatabase make BerkeleyDatabase::Create, CreateMock, and CreateDummy non-static functions
da7a83c Remove WalletDatabase::Create, CreateMock, and CreateDummy (Andrew Chow) d6045d0 scripted-diff: Replace WalletDatabase::Create* with CreateWalletDatabase (Andrew Chow) 45c08f8 Add Create*WalletDatabase functions (Andrew Chow) Pull request description: Instead of having `Create`, `CreateMock`, and `CreateDummy` being static functions in `BerkeleyDatabase`, move these to standalone functions in `walletdb.cpp`. This prepares us for having different `WalletDatabase` classes. Part of #18971. This was originally one commit but has been split into 3 to make it (hopefully) easier to review. ACKs for top commit: MarcoFalke: ACK da7a83c 🎂 ryanofsky: Code review ACK da7a83c. Easy review, nice scripted-diff Tree-SHA512: 1feb7cb3889168c555154bf3701a49095fd6b8cab911d44b7f7efbf6fcee2280ccb3d4afec8a83755b39a592ecd13b90a318faa655c321f87bdabdf1e2312327
2 parents c2bcb99 + da7a83c commit dbd7a91

15 files changed

+74
-65
lines changed

‎src/bench/coin_selection.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ static void CoinSelection(benchmark::State& state)
3131
{
3232
NodeContext node;
3333
auto chain = interfaces::MakeChain(node);
34-
CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
34+
CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
3535
wallet.SetupLegacyScriptPubKeyMan();
3636
std::vector<std::unique_ptr<CWalletTx>> wtxs;
3737
LOCK(wallet.cs_wallet);
@@ -65,7 +65,7 @@ static void CoinSelection(benchmark::State& state)
6565
typedef std::set<CInputCoin> CoinSet;
6666
static NodeContext testNode;
6767
static auto testChain = interfaces::MakeChain(testNode);
68-
static CWallet testWallet(testChain.get(), WalletLocation(), WalletDatabase::CreateDummy());
68+
static CWallet testWallet(testChain.get(), WalletLocation(), CreateDummyWalletDatabase());
6969
std::vector<std::unique_ptr<CWalletTx>> wtxn;
7070

7171
// Copied from src/wallet/test/coinselector_tests.cpp

‎src/bench/wallet_balance.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ static void WalletBalance(benchmark::State& state, const bool set_dirty, const b
2626

2727
NodeContext node;
2828
std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node);
29-
CWallet wallet{chain.get(), WalletLocation(), WalletDatabase::CreateMock()};
29+
CWallet wallet{chain.get(), WalletLocation(), CreateMockWalletDatabase()};
3030
{
3131
wallet.SetupLegacyScriptPubKeyMan();
3232
bool first_run;

‎src/qt/test/addressbooktests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ void EditAddressAndSubmit(
5959
void TestAddAddressesToSendBook(interfaces::Node& node)
6060
{
6161
TestChain100Setup test;
62-
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), WalletDatabase::CreateMock());
62+
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), CreateMockWalletDatabase());
6363
wallet->SetupLegacyScriptPubKeyMan();
6464
bool firstRun;
6565
wallet->LoadWallet(firstRun);

‎src/qt/test/wallettests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ void TestGUI(interfaces::Node& node)
140140
}
141141
node.context()->connman = std::move(test.m_node.connman);
142142
node.context()->mempool = std::move(test.m_node.mempool);
143-
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), WalletDatabase::CreateMock());
143+
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), CreateMockWalletDatabase());
144144
bool firstRun;
145145
wallet->LoadWallet(firstRun);
146146
{

‎src/wallet/bdb.h

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -120,25 +120,6 @@ class BerkeleyDatabase
120120
}
121121
}
122122

123-
/** Return object for accessing database at specified path. */
124-
static std::unique_ptr<BerkeleyDatabase> Create(const fs::path& path)
125-
{
126-
std::string filename;
127-
return MakeUnique<BerkeleyDatabase>(GetWalletEnv(path, filename), std::move(filename));
128-
}
129-
130-
/** Return object for accessing dummy database with no read/write capabilities. */
131-
static std::unique_ptr<BerkeleyDatabase> CreateDummy()
132-
{
133-
return MakeUnique<BerkeleyDatabase>();
134-
}
135-
136-
/** Return object for accessing temporary in-memory database. */
137-
static std::unique_ptr<BerkeleyDatabase> CreateMock()
138-
{
139-
return MakeUnique<BerkeleyDatabase>(std::make_shared<BerkeleyEnvironment>(), "");
140-
}
141-
142123
/** Rewrite the entire database on disk, with the exception of key pszSkip if non-zero
143124
*/
144125
bool Rewrite(const char* pszSkip=nullptr);

‎src/wallet/salvage.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ bool RecoverDatabaseFile(const fs::path& file_path)
116116
}
117117

118118
DbTxn* ptxn = env->TxnBegin();
119-
CWallet dummyWallet(nullptr, WalletLocation(), WalletDatabase::CreateDummy());
119+
CWallet dummyWallet(nullptr, WalletLocation(), CreateDummyWalletDatabase());
120120
for (KeyValPair& row : salvagedData)
121121
{
122122
/* Filter for only private key type KV pairs to be added to the salvaged wallet */

‎src/wallet/test/coinselector_tests.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ typedef std::set<CInputCoin> CoinSet;
2929
static std::vector<COutput> vCoins;
3030
static NodeContext testNode;
3131
static auto testChain = interfaces::MakeChain(testNode);
32-
static CWallet testWallet(testChain.get(), WalletLocation(), WalletDatabase::CreateDummy());
32+
static CWallet testWallet(testChain.get(), WalletLocation(), CreateDummyWalletDatabase());
3333
static CAmount balance = 0;
3434

3535
CoinEligibilityFilter filter_standard(1, 6, 0);
@@ -283,7 +283,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
283283
// Make sure that can use BnB when there are preset inputs
284284
empty_wallet();
285285
{
286-
std::unique_ptr<CWallet> wallet = MakeUnique<CWallet>(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock());
286+
std::unique_ptr<CWallet> wallet = MakeUnique<CWallet>(m_chain.get(), WalletLocation(), CreateMockWalletDatabase());
287287
bool firstRun;
288288
wallet->LoadWallet(firstRun);
289289
wallet->SetupLegacyScriptPubKeyMan();

‎src/wallet/test/ismine_tests.cpp

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
3535

3636
// P2PK compressed
3737
{
38-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
38+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
3939
keystore.SetupLegacyScriptPubKeyMan();
4040
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
4141
scriptPubKey = GetScriptForRawPubKey(pubkeys[0]);
@@ -52,7 +52,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
5252

5353
// P2PK uncompressed
5454
{
55-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
55+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
5656
keystore.SetupLegacyScriptPubKeyMan();
5757
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
5858
scriptPubKey = GetScriptForRawPubKey(uncompressedPubkey);
@@ -69,7 +69,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
6969

7070
// P2PKH compressed
7171
{
72-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
72+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
7373
keystore.SetupLegacyScriptPubKeyMan();
7474
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
7575
scriptPubKey = GetScriptForDestination(PKHash(pubkeys[0]));
@@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
8686

8787
// P2PKH uncompressed
8888
{
89-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
89+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
9090
keystore.SetupLegacyScriptPubKeyMan();
9191
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
9292
scriptPubKey = GetScriptForDestination(PKHash(uncompressedPubkey));
@@ -103,7 +103,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
103103

104104
// P2SH
105105
{
106-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
106+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
107107
keystore.SetupLegacyScriptPubKeyMan();
108108
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
109109

@@ -127,7 +127,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
127127

128128
// (P2PKH inside) P2SH inside P2SH (invalid)
129129
{
130-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
130+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
131131
keystore.SetupLegacyScriptPubKeyMan();
132132
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
133133

@@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
145145

146146
// (P2PKH inside) P2SH inside P2WSH (invalid)
147147
{
148-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
148+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
149149
keystore.SetupLegacyScriptPubKeyMan();
150150
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
151151

@@ -163,7 +163,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
163163

164164
// P2WPKH inside P2WSH (invalid)
165165
{
166-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
166+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
167167
keystore.SetupLegacyScriptPubKeyMan();
168168
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
169169

@@ -179,7 +179,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
179179

180180
// (P2PKH inside) P2WSH inside P2WSH (invalid)
181181
{
182-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
182+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
183183
keystore.SetupLegacyScriptPubKeyMan();
184184
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
185185

@@ -197,7 +197,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
197197

198198
// P2WPKH compressed
199199
{
200-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
200+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
201201
keystore.SetupLegacyScriptPubKeyMan();
202202
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
203203
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -212,7 +212,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
212212

213213
// P2WPKH uncompressed
214214
{
215-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
215+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
216216
keystore.SetupLegacyScriptPubKeyMan();
217217
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
218218
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
@@ -231,7 +231,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
231231

232232
// scriptPubKey multisig
233233
{
234-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
234+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
235235
keystore.SetupLegacyScriptPubKeyMan();
236236
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
237237

@@ -262,7 +262,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
262262

263263
// P2SH multisig
264264
{
265-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
265+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
266266
keystore.SetupLegacyScriptPubKeyMan();
267267
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
268268
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
@@ -283,7 +283,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
283283

284284
// P2WSH multisig with compressed keys
285285
{
286-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
286+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
287287
keystore.SetupLegacyScriptPubKeyMan();
288288
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
289289
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -309,7 +309,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
309309

310310
// P2WSH multisig with uncompressed key
311311
{
312-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
312+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
313313
keystore.SetupLegacyScriptPubKeyMan();
314314
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
315315
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
@@ -335,7 +335,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
335335

336336
// P2WSH multisig wrapped in P2SH
337337
{
338-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
338+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
339339
keystore.SetupLegacyScriptPubKeyMan();
340340
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
341341

@@ -362,7 +362,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
362362

363363
// OP_RETURN
364364
{
365-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
365+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
366366
keystore.SetupLegacyScriptPubKeyMan();
367367
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
368368
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -376,7 +376,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
376376

377377
// witness unspendable
378378
{
379-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
379+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
380380
keystore.SetupLegacyScriptPubKeyMan();
381381
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
382382
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -390,7 +390,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
390390

391391
// witness unknown
392392
{
393-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
393+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
394394
keystore.SetupLegacyScriptPubKeyMan();
395395
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
396396
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -404,7 +404,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
404404

405405
// Nonstandard
406406
{
407-
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
407+
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
408408
keystore.SetupLegacyScriptPubKeyMan();
409409
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
410410
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));

‎src/wallet/test/scriptpubkeyman_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ BOOST_AUTO_TEST_CASE(CanProvide)
1919
// Set up wallet and keyman variables.
2020
NodeContext node;
2121
std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node);
22-
CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
22+
CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
2323
LegacyScriptPubKeyMan& keyman = *wallet.GetOrCreateLegacyScriptPubKeyMan();
2424

2525
// Make a 1 of 2 multisig script

‎src/wallet/test/wallet_test_fixture.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
WalletTestingSetup::WalletTestingSetup(const std::string& chainName)
88
: TestingSetup(chainName),
9-
m_wallet(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock())
9+
m_wallet(m_chain.get(), WalletLocation(), CreateMockWalletDatabase())
1010
{
1111
bool fFirstRun;
1212
m_wallet.LoadWallet(fFirstRun);

0 commit comments

Comments
 (0)