aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/Makefile.test.include2
-rw-r--r--src/bench/wallet_balance.cpp3
-rw-r--r--src/interfaces/chain.cpp44
-rw-r--r--src/interfaces/chain.h2
-rw-r--r--src/interfaces/wallet.cpp5
-rw-r--r--src/interfaces/wallet.h7
-rw-r--r--src/qt/walletcontroller.cpp4
-rw-r--r--src/qt/walletmodel.cpp17
-rw-r--r--src/test/README.md29
-rw-r--r--src/test/main.cpp18
-rw-r--r--src/validationinterface.cpp18
-rw-r--r--src/validationinterface.h12
-rw-r--r--src/wallet/test/wallet_test_fixture.cpp3
-rw-r--r--src/wallet/test/wallet_test_fixture.h1
-rw-r--r--src/wallet/wallet.cpp16
-rw-r--r--src/wallet/wallet.h5
16 files changed, 110 insertions, 76 deletions
diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index 059876bec8..93c5973a21 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -1011,7 +1011,7 @@ endif
%.cpp.test: %.cpp
@echo Running tests: `cat $< | grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1` from $<
- $(AM_V_at)$(TEST_BINARY) --catch_system_errors=no -l test_suite -t "`cat $< | grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1`" > $<.log 2>&1 || (cat $<.log && false)
+ $(AM_V_at)$(TEST_BINARY) --catch_system_errors=no -l test_suite -t "`cat $< | grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1`" -- DEBUG_LOG_OUT > $<.log 2>&1 || (cat $<.log && false)
%.json.h: %.json
@$(MKDIR_P) $(@D)
diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp
index 62568a9da5..0381369218 100644
--- a/src/bench/wallet_balance.cpp
+++ b/src/bench/wallet_balance.cpp
@@ -23,9 +23,8 @@ static void WalletBalance(benchmark::State& state, const bool set_dirty, const b
wallet.SetupLegacyScriptPubKeyMan();
bool first_run;
if (wallet.LoadWallet(first_run) != DBErrors::LOAD_OK) assert(false);
- wallet.handleNotifications();
}
-
+ auto handler = chain->handleNotifications({ &wallet, [](CWallet*) {} });
const Optional<std::string> address_mine{add_mine ? Optional<std::string>{getnewaddress(wallet)} : nullopt};
if (add_watchonly) importaddress(wallet, ADDRESS_WATCHONLY);
diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
index 775a89f4cf..0b3cd08e22 100644
--- a/src/interfaces/chain.cpp
+++ b/src/interfaces/chain.cpp
@@ -148,22 +148,12 @@ class LockImpl : public Chain::Lock, public UniqueLock<RecursiveMutex>
using UniqueLock::UniqueLock;
};
-class NotificationsHandlerImpl : public Handler, CValidationInterface
+class NotificationsProxy : public CValidationInterface
{
public:
- explicit NotificationsHandlerImpl(Chain& chain, Chain::Notifications& notifications)
- : m_chain(chain), m_notifications(&notifications)
- {
- RegisterValidationInterface(this);
- }
- ~NotificationsHandlerImpl() override { disconnect(); }
- void disconnect() override
- {
- if (m_notifications) {
- m_notifications = nullptr;
- UnregisterValidationInterface(this);
- }
- }
+ explicit NotificationsProxy(std::shared_ptr<Chain::Notifications> notifications)
+ : m_notifications(std::move(notifications)) {}
+ virtual ~NotificationsProxy() = default;
void TransactionAddedToMempool(const CTransactionRef& tx) override
{
m_notifications->transactionAddedToMempool(tx);
@@ -185,8 +175,26 @@ public:
m_notifications->updatedBlockTip();
}
void ChainStateFlushed(const CBlockLocator& locator) override { m_notifications->chainStateFlushed(locator); }
- Chain& m_chain;
- Chain::Notifications* m_notifications;
+ std::shared_ptr<Chain::Notifications> m_notifications;
+};
+
+class NotificationsHandlerImpl : public Handler
+{
+public:
+ explicit NotificationsHandlerImpl(std::shared_ptr<Chain::Notifications> notifications)
+ : m_proxy(std::make_shared<NotificationsProxy>(std::move(notifications)))
+ {
+ RegisterSharedValidationInterface(m_proxy);
+ }
+ ~NotificationsHandlerImpl() override { disconnect(); }
+ void disconnect() override
+ {
+ if (m_proxy) {
+ UnregisterSharedValidationInterface(m_proxy);
+ m_proxy.reset();
+ }
+ }
+ std::shared_ptr<NotificationsProxy> m_proxy;
};
class RpcHandlerImpl : public Handler
@@ -342,9 +350,9 @@ public:
{
::uiInterface.ShowProgress(title, progress, resume_possible);
}
- std::unique_ptr<Handler> handleNotifications(Notifications& notifications) override
+ std::unique_ptr<Handler> handleNotifications(std::shared_ptr<Notifications> notifications) override
{
- return MakeUnique<NotificationsHandlerImpl>(*this, notifications);
+ return MakeUnique<NotificationsHandlerImpl>(std::move(notifications));
}
void waitForNotificationsIfTipChanged(const uint256& old_tip) override
{
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index caefa87e11..e1bc9bbbf3 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -229,7 +229,7 @@ public:
};
//! Register handler for notifications.
- virtual std::unique_ptr<Handler> handleNotifications(Notifications& notifications) = 0;
+ virtual std::unique_ptr<Handler> handleNotifications(std::shared_ptr<Notifications> notifications) = 0;
//! Wait for pending notifications to be processed unless block hash points to the current
//! chain tip.
diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp
index bb6bb4923f..645a8709d2 100644
--- a/src/interfaces/wallet.cpp
+++ b/src/interfaces/wallet.cpp
@@ -368,16 +368,17 @@ public:
}
return result;
}
- bool tryGetBalances(WalletBalances& balances, int& num_blocks) override
+ bool tryGetBalances(WalletBalances& balances, int& num_blocks, bool force, int cached_num_blocks) override
{
auto locked_chain = m_wallet->chain().lock(true /* try_lock */);
if (!locked_chain) return false;
+ num_blocks = locked_chain->getHeight().get_value_or(-1);
+ if (!force && num_blocks == cached_num_blocks) return false;
TRY_LOCK(m_wallet->cs_wallet, locked_wallet);
if (!locked_wallet) {
return false;
}
balances = getBalances();
- num_blocks = locked_chain->getHeight().get_value_or(-1);
return true;
}
CAmount getBalance() override { return m_wallet->GetBalance().m_mine_trusted; }
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index 0e551b0a96..dbb0912230 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -201,8 +201,11 @@ public:
//! Get balances.
virtual WalletBalances getBalances() = 0;
- //! Get balances if possible without blocking.
- virtual bool tryGetBalances(WalletBalances& balances, int& num_blocks) = 0;
+ //! Get balances if possible without waiting for chain and wallet locks.
+ virtual bool tryGetBalances(WalletBalances& balances,
+ int& num_blocks,
+ bool force,
+ int cached_num_blocks) = 0;
//! Get balance.
virtual CAmount getBalance() = 0;
diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp
index 233c0ab6be..88c694567e 100644
--- a/src/qt/walletcontroller.cpp
+++ b/src/qt/walletcontroller.cpp
@@ -116,7 +116,7 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wal
const bool called = QMetaObject::invokeMethod(wallet_model, "startPollBalance");
assert(called);
- connect(wallet_model, &WalletModel::unload, [this, wallet_model] {
+ connect(wallet_model, &WalletModel::unload, this, [this, wallet_model] {
// Defer removeAndDeleteWallet when no modal widget is active.
// TODO: remove this workaround by removing usage of QDiallog::exec.
if (QApplication::activeModalWidget()) {
@@ -128,7 +128,7 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wal
} else {
removeAndDeleteWallet(wallet_model);
}
- });
+ }, Qt::QueuedConnection);
// Re-emit coinsSent signal from wallet model.
connect(wallet_model, &WalletModel::coinsSent, this, &WalletController::coinsSent);
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index 41876f7119..608797d6ad 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -78,21 +78,18 @@ void WalletModel::pollBalanceChanged()
// rescan.
interfaces::WalletBalances new_balances;
int numBlocks = -1;
- if (!m_wallet->tryGetBalances(new_balances, numBlocks)) {
+ if (!m_wallet->tryGetBalances(new_balances, numBlocks, fForceCheckBalanceChanged, cachedNumBlocks)) {
return;
}
- if(fForceCheckBalanceChanged || numBlocks != cachedNumBlocks)
- {
- fForceCheckBalanceChanged = false;
+ fForceCheckBalanceChanged = false;
- // Balance and number of transactions might have changed
- cachedNumBlocks = numBlocks;
+ // Balance and number of transactions might have changed
+ cachedNumBlocks = numBlocks;
- checkBalanceChanged(new_balances);
- if(transactionTableModel)
- transactionTableModel->updateConfirmations();
- }
+ checkBalanceChanged(new_balances);
+ if(transactionTableModel)
+ transactionTableModel->updateConfirmations();
}
void WalletModel::checkBalanceChanged(const interfaces::WalletBalances& new_balances)
diff --git a/src/test/README.md b/src/test/README.md
index 731720f654..57cda26d7c 100644
--- a/src/test/README.md
+++ b/src/test/README.md
@@ -17,26 +17,31 @@ and tests weren't explicitly disabled.
After configuring, they can be run with `make check`.
-To run the bitcoind tests manually, launch `src/test/test_bitcoin`. To recompile
+To run the unit tests manually, launch `src/test/test_bitcoin`. To recompile
after a test file was modified, run `make` and then run the test again. If you
modify a non-test file, use `make -C src/test` to recompile only what's needed
-to run the bitcoind tests.
+to run the unit tests.
-To add more bitcoind tests, add `BOOST_AUTO_TEST_CASE` functions to the existing
+To add more unit tests, add `BOOST_AUTO_TEST_CASE` functions to the existing
.cpp files in the `test/` directory or add new .cpp files that
implement new `BOOST_AUTO_TEST_SUITE` sections.
-To run the bitcoin-qt tests manually, launch `src/qt/test/test_bitcoin-qt`
+To run the GUI unit tests manually, launch `src/qt/test/test_bitcoin-qt`
-To add more bitcoin-qt tests, add them to the `src/qt/test/` directory and
+To add more GUI unit tests, add them to the `src/qt/test/` directory and
the `src/qt/test/test_main.cpp` file.
### Running individual tests
-test_bitcoin has some built-in command-line arguments; for
-example, to run just the getarg_tests verbosely:
+`test_bitcoin` has some built-in command-line arguments; for
+example, to run just the `getarg_tests` verbosely:
- test_bitcoin --log_level=all --run_test=getarg_tests
+ test_bitcoin --log_level=all --run_test=getarg_tests -- DEBUG_LOG_OUT
+
+`log_level` controls the verbosity of the test framework, which logs when a
+test case is entered, for example. The `DEBUG_LOG_OUT` after the two dashes
+redirects the debug log, which would normally go to a file in the test datadir
+(`BasicTestingSetup::m_path_root`), to the standard terminal output.
... or to run just the doubledash test:
@@ -56,11 +61,15 @@ see `uint256_tests.cpp`.
### Logging and debugging in unit tests
+`make check` will write to a log file `foo_tests.cpp.log` and display this file
+on failure. For running individual tests verbosely, refer to the section
+[above](#running-individual-tests).
+
To write to logs from unit tests you need to use specific message methods
provided by Boost. The simplest is `BOOST_TEST_MESSAGE`.
-For debugging you can launch the test_bitcoin executable with `gdb`or `lldb` and
-start debugging, just like you would with bitcoind:
+For debugging you can launch the `test_bitcoin` executable with `gdb`or `lldb` and
+start debugging, just like you would with any other program:
```bash
gdb src/test/test_bitcoin
diff --git a/src/test/main.cpp b/src/test/main.cpp
index e6529949e2..f32243d1d3 100644
--- a/src/test/main.cpp
+++ b/src/test/main.cpp
@@ -11,12 +11,16 @@
#include <test/util/setup_common.h>
-/** Redirect debug log to boost log */
+#include <iostream>
+
+/** Redirect debug log to unit_test.log files */
const std::function<void(const std::string&)> G_TEST_LOG_FUN = [](const std::string& s) {
- if (s.back() == '\n') {
- // boost will insert the new line
- BOOST_TEST_MESSAGE(s.substr(0, s.size() - 1));
- } else {
- BOOST_TEST_MESSAGE(s);
- }
+ static const bool should_log{std::any_of(
+ &boost::unit_test::framework::master_test_suite().argv[1],
+ &boost::unit_test::framework::master_test_suite().argv[boost::unit_test::framework::master_test_suite().argc],
+ [](const char* arg) {
+ return std::string{"DEBUG_LOG_OUT"} == arg;
+ })};
+ if (!should_log) return;
+ std::cout << s;
};
diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index c895904b19..f9f61e8a09 100644
--- a/src/validationinterface.cpp
+++ b/src/validationinterface.cpp
@@ -75,8 +75,10 @@ CMainSignals& GetMainSignals()
return g_signals;
}
-void RegisterValidationInterface(CValidationInterface* pwalletIn) {
- ValidationInterfaceConnections& conns = g_signals.m_internals->m_connMainSignals[pwalletIn];
+void RegisterSharedValidationInterface(std::shared_ptr<CValidationInterface> pwalletIn) {
+ // Each connection captures pwalletIn to ensure that each callback is
+ // executed before pwalletIn is destroyed. For more details see #18338.
+ ValidationInterfaceConnections& conns = g_signals.m_internals->m_connMainSignals[pwalletIn.get()];
conns.UpdatedBlockTip = g_signals.m_internals->UpdatedBlockTip.connect(std::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3));
conns.TransactionAddedToMempool = g_signals.m_internals->TransactionAddedToMempool.connect(std::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, std::placeholders::_1));
conns.BlockConnected = g_signals.m_internals->BlockConnected.connect(std::bind(&CValidationInterface::BlockConnected, pwalletIn, std::placeholders::_1, std::placeholders::_2));
@@ -87,6 +89,18 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) {
conns.NewPoWValidBlock = g_signals.m_internals->NewPoWValidBlock.connect(std::bind(&CValidationInterface::NewPoWValidBlock, pwalletIn, std::placeholders::_1, std::placeholders::_2));
}
+void RegisterValidationInterface(CValidationInterface* callbacks)
+{
+ // Create a shared_ptr with a no-op deleter - CValidationInterface lifecycle
+ // is managed by the caller.
+ RegisterSharedValidationInterface({callbacks, [](CValidationInterface*){}});
+}
+
+void UnregisterSharedValidationInterface(std::shared_ptr<CValidationInterface> callbacks)
+{
+ UnregisterValidationInterface(callbacks.get());
+}
+
void UnregisterValidationInterface(CValidationInterface* pwalletIn) {
if (g_signals.m_internals) {
g_signals.m_internals->m_connMainSignals.erase(pwalletIn);
diff --git a/src/validationinterface.h b/src/validationinterface.h
index 5707422635..f9a359b7ad 100644
--- a/src/validationinterface.h
+++ b/src/validationinterface.h
@@ -30,6 +30,14 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn);
void UnregisterValidationInterface(CValidationInterface* pwalletIn);
/** Unregister all wallets from core */
void UnregisterAllValidationInterfaces();
+
+// Alternate registration functions that release a shared_ptr after the last
+// notification is sent. These are useful for race-free cleanup, since
+// unregistration is nonblocking and can return before the last notification is
+// processed.
+void RegisterSharedValidationInterface(std::shared_ptr<CValidationInterface> callbacks);
+void UnregisterSharedValidationInterface(std::shared_ptr<CValidationInterface> callbacks);
+
/**
* Pushes a function to callback onto the notification queue, guaranteeing any
* callbacks generated prior to now are finished when the function is called.
@@ -163,7 +171,7 @@ protected:
* Notifies listeners that a block which builds directly on our current tip
* has been received and connected to the headers tree, though not validated yet */
virtual void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& block) {};
- friend void ::RegisterValidationInterface(CValidationInterface*);
+ friend void ::RegisterSharedValidationInterface(std::shared_ptr<CValidationInterface>);
friend void ::UnregisterValidationInterface(CValidationInterface*);
friend void ::UnregisterAllValidationInterfaces();
};
@@ -173,7 +181,7 @@ class CMainSignals {
private:
std::unique_ptr<MainSignalsInstance> m_internals;
- friend void ::RegisterValidationInterface(CValidationInterface*);
+ friend void ::RegisterSharedValidationInterface(std::shared_ptr<CValidationInterface>);
friend void ::UnregisterValidationInterface(CValidationInterface*);
friend void ::UnregisterAllValidationInterfaces();
friend void ::CallFunctionInValidationInterfaceQueue(std::function<void ()> func);
diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp
index ba0843f352..b9e714946d 100644
--- a/src/wallet/test/wallet_test_fixture.cpp
+++ b/src/wallet/test/wallet_test_fixture.cpp
@@ -10,7 +10,6 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName)
{
bool fFirstRun;
m_wallet.LoadWallet(fFirstRun);
- m_wallet.handleNotifications();
-
+ m_chain_notifications_handler = m_chain->handleNotifications({ &m_wallet, [](CWallet*) {} });
m_chain_client->registerRpcs();
}
diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h
index 4e4129fb2c..81d8a60b8a 100644
--- a/src/wallet/test/wallet_test_fixture.h
+++ b/src/wallet/test/wallet_test_fixture.h
@@ -23,6 +23,7 @@ struct WalletTestingSetup: public TestingSetup {
std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node);
std::unique_ptr<interfaces::ChainClient> m_chain_client = interfaces::MakeWalletClient(*m_chain, {});
CWallet m_wallet;
+ std::unique_ptr<interfaces::Handler> m_chain_notifications_handler;
};
#endif // BITCOIN_WALLET_TEST_WALLET_TEST_FIXTURE_H
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 9a972febab..98f308f927 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -62,8 +62,10 @@ bool AddWallet(const std::shared_ptr<CWallet>& wallet)
bool RemoveWallet(const std::shared_ptr<CWallet>& wallet)
{
- LOCK(cs_wallets);
assert(wallet);
+ // Unregister with the validation interface which also drops shared ponters.
+ wallet->m_chain_notifications_handler.reset();
+ LOCK(cs_wallets);
std::vector<std::shared_ptr<CWallet>>::iterator i = std::find(vpwallets.begin(), vpwallets.end(), wallet);
if (i == vpwallets.end()) return false;
vpwallets.erase(i);
@@ -105,13 +107,9 @@ static std::set<std::string> g_unloading_wallet_set;
// Custom deleter for shared_ptr<CWallet>.
static void ReleaseWallet(CWallet* wallet)
{
- // Unregister and delete the wallet right after BlockUntilSyncedToCurrentChain
- // so that it's in sync with the current chainstate.
const std::string name = wallet->GetName();
wallet->WalletLogPrintf("Releasing wallet\n");
- wallet->BlockUntilSyncedToCurrentChain();
wallet->Flush();
- wallet->m_chain_notifications_handler.reset();
delete wallet;
// Wallet is now released, notify UnloadWallet, if any.
{
@@ -137,6 +135,7 @@ void UnloadWallet(std::shared_ptr<CWallet>&& wallet)
// Notify the unload intent so that all remaining shared pointers are
// released.
wallet->NotifyUnload();
+
// Time to ditch our shared_ptr and wait for ReleaseWallet call.
wallet.reset();
{
@@ -4092,7 +4091,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
}
// Register with the validation interface. It's ok to do this after rescan since we're still holding locked_chain.
- walletInstance->handleNotifications();
+ walletInstance->m_chain_notifications_handler = walletInstance->chain().handleNotifications(walletInstance);
walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST));
@@ -4105,11 +4104,6 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
return walletInstance;
}
-void CWallet::handleNotifications()
-{
- m_chain_notifications_handler = m_chain->handleNotifications(*this);
-}
-
void CWallet::postInitProcess()
{
auto locked_chain = chain().lock();
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 9a83c84fc4..fe59773488 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -606,7 +606,7 @@ class WalletRescanReserver; //forward declarations for ScanForWalletTransactions
/**
* A CWallet maintains a set of transactions and balances, and provides the ability to create new transactions.
*/
-class CWallet final : public WalletStorage, private interfaces::Chain::Notifications
+class CWallet final : public WalletStorage, public interfaces::Chain::Notifications
{
private:
CKeyingMaterial vMasterKey GUARDED_BY(cs_wallet);
@@ -782,9 +782,6 @@ public:
/** Registered interfaces::Chain::Notifications handler. */
std::unique_ptr<interfaces::Handler> m_chain_notifications_handler;
- /** Register the wallet for chain notifications */
- void handleNotifications();
-
/** Interface for accessing chain state. */
interfaces::Chain& chain() const { assert(m_chain); return *m_chain; }
generated by cgit v1.2.3 (git 2.26.2) at 2024-08-25 02:57:23 +0000