Skip to content

Commit e4ea5c3

Browse files
committed
wallet, bench: Move commonly used functions to their own file and fix a bug
(cherry picked from commit 9598089)
1 parent 13bce82 commit e4ea5c3

6 files changed

Lines changed: 82 additions & 67 deletions

File tree

src/Makefile.test_util.include

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,8 @@ libtest_util_a_SOURCES = \
4646
test/util/validation.cpp \
4747
test/util/wallet.cpp \
4848
$(TEST_UTIL_H)
49+
50+
if ENABLE_WALLET
51+
libtest_util_a_SOURCES += \
52+
wallet/test/load_util.cpp
53+
endif

src/bench/wallet_balance.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,14 @@
88
#include <test/util/mining.h>
99
#include <test/util/setup_common.h>
1010
#include <test/util/wallet.h>
11+
#include <wallet/test/util.h>
1112
#include <validationinterface.h>
1213
#include <wallet/receive.h>
1314
#include <wallet/wallet.h>
1415

1516
#include <optional>
1617

17-
using wallet::CreateMockWalletDatabase;
18-
using wallet::CWallet;
19-
using wallet::DBErrors;
20-
using wallet::WALLET_FLAG_DESCRIPTORS;
21-
18+
namespace wallet {
2219
static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const bool add_mine, const uint32_t epoch_iters)
2320
{
2421
const auto test_setup = MakeNoLogFileContext<const TestingSetup>();
@@ -29,7 +26,6 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b
2926
LOCK(wallet.cs_wallet);
3027
wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
3128
wallet.SetupDescriptorScriptPubKeyMans("", "");
32-
if (wallet.LoadWallet() != DBErrors::LOAD_OK) assert(false);
3329
}
3430
auto handler = test_setup->m_node.chain->handleNotifications({&wallet, [](CWallet*) {}});
3531

@@ -59,3 +55,4 @@ BENCHMARK(WalletBalanceDirty, benchmark::PriorityLevel::HIGH);
5955
BENCHMARK(WalletBalanceClean, benchmark::PriorityLevel::HIGH);
6056
BENCHMARK(WalletBalanceMine, benchmark::PriorityLevel::HIGH);
6157
BENCHMARK(WalletBalanceWatch, benchmark::PriorityLevel::HIGH);
58+
} // namespace wallet

src/bench/wallet_loading.cpp

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <test/util/mining.h>
99
#include <test/util/setup_common.h>
1010
#include <test/util/wallet.h>
11+
#include <wallet/test/util.h>
1112
#include <util/translation.h>
1213
#include <validationinterface.h>
1314
#include <wallet/context.h>
@@ -16,33 +17,7 @@
1617

1718
#include <optional>
1819

19-
using wallet::CWallet;
20-
using wallet::DatabaseFormat;
21-
using wallet::DatabaseOptions;
22-
using wallet::TxStateInactive;
23-
using wallet::WALLET_FLAG_DESCRIPTORS;
24-
using wallet::WalletContext;
25-
using wallet::WalletDatabase;
26-
27-
static std::shared_ptr<CWallet> BenchLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, DatabaseOptions& options)
28-
{
29-
bilingual_str error;
30-
std::vector<bilingual_str> warnings;
31-
auto wallet = CWallet::Create(context, "", std::move(database), options.create_flags, error, warnings);
32-
NotifyWalletLoaded(context, wallet);
33-
if (context.chain) {
34-
wallet->postInitProcess();
35-
}
36-
return wallet;
37-
}
38-
39-
static void BenchUnloadWallet(std::shared_ptr<CWallet>&& wallet)
40-
{
41-
SyncWithValidationInterfaceQueue();
42-
wallet->m_chain_notifications_handler.reset();
43-
UnloadWallet(std::move(wallet));
44-
}
45-
20+
namespace wallet {
4621
static void AddTx(CWallet& wallet)
4722
{
4823
CMutableTransaction mtx;
@@ -95,7 +70,7 @@ static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet)
9570
options.require_format = DatabaseFormat::SQLITE;
9671
}
9772
auto database = CreateMockWalletDatabase(options);
98-
auto wallet = BenchLoadWallet(std::move(database), context, options);
73+
auto wallet = TestLoadWallet(std::move(database), context, options.create_flags);
9974

10075
// Generate a bunch of transactions and addresses to put into the wallet
10176
for (int i = 0; i < 1000; ++i) {
@@ -105,14 +80,14 @@ static void WalletLoading(benchmark::Bench& bench, bool legacy_wallet)
10580
database = DuplicateMockDatabase(wallet->GetDatabase(), options);
10681

10782
// reload the wallet for the actual benchmark
108-
BenchUnloadWallet(std::move(wallet));
83+
TestUnloadWallet(context, std::move(wallet));
10984

11085
bench.epochs(5).run([&] {
111-
wallet = BenchLoadWallet(std::move(database), context, options);
86+
wallet = TestLoadWallet(std::move(database), context, options.create_flags);
11287

11388
// Cleanup
11489
database = DuplicateMockDatabase(wallet->GetDatabase(), options);
115-
BenchUnloadWallet(std::move(wallet));
90+
TestUnloadWallet(context, std::move(wallet));
11691
});
11792
}
11893

@@ -125,3 +100,4 @@ BENCHMARK(WalletLoadingLegacy, benchmark::PriorityLevel::HIGH);
125100
static void WalletLoadingDescriptors(benchmark::Bench& bench) { WalletLoading(bench, /*legacy_wallet=*/false); }
126101
BENCHMARK(WalletLoadingDescriptors, benchmark::PriorityLevel::HIGH);
127102
#endif
103+
} // namespace wallet

src/wallet/test/load_util.cpp

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright (c) 2021 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include <wallet/test/util.h>
6+
7+
#include <sync.h>
8+
#include <validationinterface.h>
9+
#include <wallet/context.h>
10+
#include <wallet/wallet.h>
11+
#include <wallet/walletdb.h>
12+
13+
#include <memory>
14+
#include <optional>
15+
#include <vector>
16+
17+
namespace wallet {
18+
std::shared_ptr<CWallet> TestLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, uint64_t create_flags)
19+
{
20+
bilingual_str error;
21+
std::vector<bilingual_str> warnings;
22+
auto wallet = CWallet::Create(context, "", std::move(database), create_flags, error, warnings);
23+
if (context.coinjoin_loader) {
24+
// TODO: see CreateWalletWithoutChain
25+
AddWallet(context, wallet);
26+
}
27+
NotifyWalletLoaded(context, wallet);
28+
if (context.chain) {
29+
wallet->postInitProcess();
30+
}
31+
return wallet;
32+
}
33+
34+
std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context)
35+
{
36+
DatabaseOptions options;
37+
options.create_flags = WALLET_FLAG_DESCRIPTORS;
38+
DatabaseStatus status;
39+
bilingual_str error;
40+
auto database = MakeWalletDatabase("", options, status, error);
41+
return TestLoadWallet(std::move(database), context, options.create_flags);
42+
}
43+
44+
void TestUnloadWallet(WalletContext& context, std::shared_ptr<CWallet>&& wallet)
45+
{
46+
std::vector<bilingual_str> warnings;
47+
SyncWithValidationInterfaceQueue();
48+
wallet->m_chain_notifications_handler.reset();
49+
if (context.coinjoin_loader) {
50+
RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt, warnings);
51+
}
52+
UnloadWallet(std::move(wallet));
53+
}
54+
} // namespace wallet

src/wallet/test/util.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#ifndef BITCOIN_WALLET_TEST_UTIL_H
66
#define BITCOIN_WALLET_TEST_UTIL_H
77

8+
#include <cstdint>
89
#include <memory>
910

1011
class ArgsManager;
@@ -19,8 +20,13 @@ class Loader;
1920

2021
namespace wallet {
2122
class CWallet;
23+
class WalletDatabase;
24+
struct WalletContext;
2225

2326
std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, CChain& cchain, ArgsManager& args, const CKey& key);
27+
std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context);
28+
std::shared_ptr<CWallet> TestLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, uint64_t create_flags);
29+
void TestUnloadWallet(WalletContext& context, std::shared_ptr<CWallet>&& wallet);
2430
} // namespace wallet
2531

2632
#endif // BITCOIN_WALLET_TEST_UTIL_H

src/wallet/test/wallet_tests.cpp

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -51,32 +51,6 @@ static_assert(DEFAULT_TRANSACTION_MINFEE >= DEFAULT_MIN_RELAY_TX_FEE, "wallet mi
5151

5252
BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup)
5353

54-
static std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context)
55-
{
56-
DatabaseOptions options;
57-
options.create_flags = WALLET_FLAG_DESCRIPTORS;
58-
DatabaseStatus status;
59-
bilingual_str error;
60-
std::vector<bilingual_str> warnings;
61-
auto database = MakeWalletDatabase("", options, status, error);
62-
auto wallet = CWallet::Create(context, "", std::move(database), options.create_flags, error, warnings);
63-
if (context.coinjoin_loader) {
64-
// TODO: see CreateWalletWithoutChain
65-
AddWallet(context, wallet);
66-
}
67-
NotifyWalletLoaded(context, wallet);
68-
return wallet;
69-
}
70-
71-
static void TestUnloadWallet(WalletContext& context, std::shared_ptr<CWallet>&& wallet)
72-
{
73-
std::vector<bilingual_str> warnings;
74-
SyncWithValidationInterfaceQueue();
75-
wallet->m_chain_notifications_handler.reset();
76-
RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt, warnings);
77-
UnloadWallet(std::move(wallet));
78-
}
79-
8054
static CMutableTransaction TestSimpleSpend(const CTransaction& from, uint32_t index, const CKey& key, const CScript& pubkey)
8155
{
8256
CMutableTransaction mtx;
@@ -820,8 +794,9 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup)
820794
// being blocked
821795
wallet = TestLoadWallet(context);
822796
BOOST_CHECK(rescan_completed);
823-
// AddToWallet events for block_tx and mempool_tx
824-
BOOST_CHECK_EQUAL(addtx_count, 2);
797+
// Loading will also ask for current mempool transactions
798+
// AddToWallet events for block_tx and mempool_tx (x2)
799+
BOOST_CHECK_EQUAL(addtx_count, 3);
825800
{
826801
LOCK(wallet->cs_wallet);
827802
BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_tx.GetHash()), 1U);
@@ -835,7 +810,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup)
835810
SyncWithValidationInterfaceQueue();
836811
// AddToWallet events for block_tx and mempool_tx events are counted a
837812
// second time as the notification queue is processed
838-
BOOST_CHECK_EQUAL(addtx_count, 4);
813+
BOOST_CHECK_EQUAL(addtx_count, 5);
839814

840815
TestUnloadWallet(context, std::move(wallet));
841816

@@ -857,7 +832,9 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup)
857832
SyncWithValidationInterfaceQueue();
858833
});
859834
wallet = TestLoadWallet(context);
860-
BOOST_CHECK_EQUAL(addtx_count, 2);
835+
// Since mempool transactions are requested at the end of loading, there will
836+
// be 2 additional AddToWallet calls, one from the previous test, and a duplicate for mempool_tx
837+
BOOST_CHECK_EQUAL(addtx_count, 2 + 2);
861838
{
862839
LOCK(wallet->cs_wallet);
863840
BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_tx.GetHash()), 1U);

0 commit comments

Comments
 (0)