diff options
Diffstat (limited to 'src')
63 files changed, 610 insertions, 361 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index 52bd4f1621..2616eb8638 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2,6 +2,10 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. +# Pattern rule to print variables, e.g. make print-top_srcdir +print-%: + @echo $* = $($*) + DIST_SUBDIRS = secp256k1 univalue AM_LDFLAGS = $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) $(SANITIZER_LDFLAGS) diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index 3d41d203d3..969f0ca411 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -78,6 +78,7 @@ QT_MOC_CPP = \ qt/moc_transactiondesc.cpp \ qt/moc_transactiondescdialog.cpp \ qt/moc_transactionfilterproxy.cpp \ + qt/moc_transactionoverviewwidget.cpp \ qt/moc_transactiontablemodel.cpp \ qt/moc_transactionview.cpp \ qt/moc_utilitydialog.cpp \ @@ -151,6 +152,7 @@ BITCOIN_QT_H = \ qt/transactiondesc.h \ qt/transactiondescdialog.h \ qt/transactionfilterproxy.h \ + qt/transactionoverviewwidget.h \ qt/transactionrecord.h \ qt/transactiontablemodel.h \ qt/transactionview.h \ diff --git a/src/bench/checkqueue.cpp b/src/bench/checkqueue.cpp index ffa772d8c1..d7b8c1badc 100644 --- a/src/bench/checkqueue.cpp +++ b/src/bench/checkqueue.cpp @@ -10,8 +10,6 @@ #include <random.h> #include <util/system.h> -#include <boost/thread/thread.hpp> - #include <vector> static const size_t BATCHES = 101; @@ -44,12 +42,9 @@ static void CCheckQueueSpeedPrevectorJob(benchmark::Bench& bench) void swap(PrevectorJob& x){p.swap(x.p);}; }; CCheckQueue<PrevectorJob> queue {QUEUE_BATCH_SIZE}; - boost::thread_group tg; // The main thread should be counted to prevent thread oversubscription, and // to decrease the variance of benchmark results. - for (auto x = 0; x < GetNumCores() - 1; ++x) { - tg.create_thread([&]{queue.Thread();}); - } + queue.StartWorkerThreads(GetNumCores() - 1); // create all the data once, then submit copies in the benchmark. FastRandomContext insecure_rand(true); @@ -70,8 +65,7 @@ static void CCheckQueueSpeedPrevectorJob(benchmark::Bench& bench) // it is done explicitly here for clarity control.Wait(); }); - tg.interrupt_all(); - tg.join_all(); + queue.StopWorkerThreads(); ECC_Stop(); } BENCHMARK(CCheckQueueSpeedPrevectorJob); diff --git a/src/bench/rpc_blockchain.cpp b/src/bench/rpc_blockchain.cpp index 4b45264a3c..45ed9f60dc 100644 --- a/src/bench/rpc_blockchain.cpp +++ b/src/bench/rpc_blockchain.cpp @@ -7,12 +7,15 @@ #include <rpc/blockchain.h> #include <streams.h> +#include <test/util/setup_common.h> #include <validation.h> #include <univalue.h> static void BlockToJsonVerbose(benchmark::Bench& bench) { + TestingSetup test_setup{}; + CDataStream stream(benchmark::data::block413567, SER_NETWORK, PROTOCOL_VERSION); char a = '\0'; stream.write(&a, 1); // Prevent compaction diff --git a/src/checkqueue.h b/src/checkqueue.h index e3faa1dec0..4ceeb3600a 100644 --- a/src/checkqueue.h +++ b/src/checkqueue.h @@ -6,13 +6,12 @@ #define BITCOIN_CHECKQUEUE_H #include <sync.h> +#include <tinyformat.h> +#include <util/threadnames.h> #include <algorithm> #include <vector> -#include <boost/thread/condition_variable.hpp> -#include <boost/thread/mutex.hpp> - template <typename T> class CCheckQueueControl; @@ -31,61 +30,64 @@ class CCheckQueue { private: //! Mutex to protect the inner state - boost::mutex mutex; + Mutex m_mutex; //! Worker threads block on this when out of work - boost::condition_variable condWorker; + std::condition_variable m_worker_cv; //! Master thread blocks on this when out of work - boost::condition_variable condMaster; + std::condition_variable m_master_cv; //! The queue of elements to be processed. //! As the order of booleans doesn't matter, it is used as a LIFO (stack) - std::vector<T> queue; + std::vector<T> queue GUARDED_BY(m_mutex); //! The number of workers (including the master) that are idle. - int nIdle; + int nIdle GUARDED_BY(m_mutex){0}; //! The total number of workers (including the master). - int nTotal; + int nTotal GUARDED_BY(m_mutex){0}; //! The temporary evaluation result. - bool fAllOk; + bool fAllOk GUARDED_BY(m_mutex){true}; /** * Number of verifications that haven't completed yet. * This includes elements that are no longer queued, but still in the * worker's own batches. */ - unsigned int nTodo; + unsigned int nTodo GUARDED_BY(m_mutex){0}; //! The maximum number of elements to be processed in one batch - unsigned int nBatchSize; + const unsigned int nBatchSize; + + std::vector<std::thread> m_worker_threads; + bool m_request_stop GUARDED_BY(m_mutex){false}; /** Internal function that does bulk of the verification work. */ - bool Loop(bool fMaster = false) + bool Loop(bool fMaster) { - boost::condition_variable& cond = fMaster ? condMaster : condWorker; + std::condition_variable& cond = fMaster ? m_master_cv : m_worker_cv; std::vector<T> vChecks; vChecks.reserve(nBatchSize); unsigned int nNow = 0; bool fOk = true; do { { - boost::unique_lock<boost::mutex> lock(mutex); + WAIT_LOCK(m_mutex, lock); // first do the clean-up of the previous loop run (allowing us to do it in the same critsect) if (nNow) { fAllOk &= fOk; nTodo -= nNow; if (nTodo == 0 && !fMaster) // We processed the last element; inform the master it can exit and return the result - condMaster.notify_one(); + m_master_cv.notify_one(); } else { // first iteration nTotal++; } // logically, the do loop starts here - while (queue.empty()) { + while (queue.empty() && !m_request_stop) { if (fMaster && nTodo == 0) { nTotal--; bool fRet = fAllOk; @@ -98,6 +100,10 @@ private: cond.wait(lock); // wait nIdle--; } + if (m_request_stop) { + return false; + } + // Decide how many work units to process now. // * Do not try to do everything at once, but aim for increasingly smaller batches so // all workers finish approximately simultaneously. @@ -106,7 +112,7 @@ private: nNow = std::max(1U, std::min(nBatchSize, (unsigned int)queue.size() / (nTotal + nIdle + 1))); vChecks.resize(nNow); for (unsigned int i = 0; i < nNow; i++) { - // We want the lock on the mutex to be as short as possible, so swap jobs from the global + // We want the lock on the m_mutex to be as short as possible, so swap jobs from the global // queue to the local batch vector instead of copying. vChecks[i].swap(queue.back()); queue.pop_back(); @@ -124,40 +130,68 @@ private: public: //! Mutex to ensure only one concurrent CCheckQueueControl - boost::mutex ControlMutex; + Mutex m_control_mutex; //! Create a new check queue - explicit CCheckQueue(unsigned int nBatchSizeIn) : nIdle(0), nTotal(0), fAllOk(true), nTodo(0), nBatchSize(nBatchSizeIn) {} + explicit CCheckQueue(unsigned int nBatchSizeIn) + : nBatchSize(nBatchSizeIn) + { + } - //! Worker thread - void Thread() + //! Create a pool of new worker threads. + void StartWorkerThreads(const int threads_num) { - Loop(); + { + LOCK(m_mutex); + nIdle = 0; + nTotal = 0; + fAllOk = true; + } + assert(m_worker_threads.empty()); + for (int n = 0; n < threads_num; ++n) { + m_worker_threads.emplace_back([this, n]() { + util::ThreadRename(strprintf("scriptch.%i", n)); + Loop(false /* worker thread */); + }); + } } //! Wait until execution finishes, and return whether all evaluations were successful. bool Wait() { - return Loop(true); + return Loop(true /* master thread */); } //! Add a batch of checks to the queue void Add(std::vector<T>& vChecks) { - boost::unique_lock<boost::mutex> lock(mutex); + LOCK(m_mutex); for (T& check : vChecks) { queue.push_back(T()); check.swap(queue.back()); } nTodo += vChecks.size(); if (vChecks.size() == 1) - condWorker.notify_one(); + m_worker_cv.notify_one(); else if (vChecks.size() > 1) - condWorker.notify_all(); + m_worker_cv.notify_all(); + } + + //! Stop all of the worker threads. + void StopWorkerThreads() + { + WITH_LOCK(m_mutex, m_request_stop = true); + m_worker_cv.notify_all(); + for (std::thread& t : m_worker_threads) { + t.join(); + } + m_worker_threads.clear(); + WITH_LOCK(m_mutex, m_request_stop = false); } ~CCheckQueue() { + assert(m_worker_threads.empty()); } }; @@ -181,7 +215,7 @@ public: { // passed queue is supposed to be unused, or nullptr if (pqueue != nullptr) { - ENTER_CRITICAL_SECTION(pqueue->ControlMutex); + ENTER_CRITICAL_SECTION(pqueue->m_control_mutex); } } @@ -205,7 +239,7 @@ public: if (!fDone) Wait(); if (pqueue != nullptr) { - LEAVE_CRITICAL_SECTION(pqueue->ControlMutex); + LEAVE_CRITICAL_SECTION(pqueue->m_control_mutex); } } }; diff --git a/src/fs.cpp b/src/fs.cpp index 7eb0f5ca9f..4f20ca4d28 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -31,6 +31,12 @@ FILE *fopen(const fs::path& p, const char *mode) #endif } +fs::path AbsPathJoin(const fs::path& base, const fs::path& path) +{ + assert(base.is_absolute()); + return fs::absolute(path, base); +} + #ifndef WIN32 static std::string GetErrorReason() @@ -21,6 +21,17 @@ namespace fs = boost::filesystem; namespace fsbridge { FILE *fopen(const fs::path& p, const char *mode); + /** + * Helper function for joining two paths + * + * @param[in] base Base path + * @param[in] path Path to combine with base + * @returns path unchanged if it is an absolute path, otherwise returns base joined with path. Returns base unchanged if path is empty. + * @pre Base path must be absolute + * @post Returned path will always be absolute + */ + fs::path AbsPathJoin(const fs::path& base, const fs::path& path); + class FileLock { public: diff --git a/src/init.cpp b/src/init.cpp index 09eb76eaee..64bbf8988d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -223,6 +223,7 @@ void Shutdown(NodeContext& node) if (g_load_block.joinable()) g_load_block.join(); threadGroup.interrupt_all(); threadGroup.join_all(); + StopScriptCheckWorkerThreads(); // After the threads that potentially access these pointers have been stopped, // destruct and reset all to nullptr. @@ -447,7 +448,7 @@ void SetupServerArgs(NodeContext& node) argsman.AddArg("-maxtimeadjustment", strprintf("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by peers forward or backward by this amount. (default: %u seconds)", DEFAULT_MAX_TIME_ADJUSTMENT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxuploadtarget=<n>", strprintf("Tries to keep outbound traffic under the given target (in MiB per 24h). Limit does not apply to peers with 'download' permission. 0 = no limit (default: %d)", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-onion=<ip:port>", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-onlynet=<net>", "Make outgoing connections only through network <net> (ipv4, ipv6 or onion). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-onlynet=<net>", "Make outgoing connections only through network <net> (ipv4, ipv6 or onion). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks. Warning: if it is used with ipv4 or ipv6 but not onion and the -onion or -proxy option is set, then outbound onion connections will still be made; use -noonion or -onion=0 to disable outbound onion connections in this case.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -1334,9 +1335,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA LogPrintf("Script verification uses %d additional threads\n", script_threads); if (script_threads >= 1) { g_parallel_script_checks = true; - for (int i = 0; i < script_threads; ++i) { - threadGroup.create_thread([i]() { return ThreadScriptCheck(i); }); - } + StartScriptCheckWorkerThreads(script_threads); } assert(!node.scheduler); diff --git a/src/key_io.cpp b/src/key_io.cpp index a270ede864..e27673fd16 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -12,6 +12,9 @@ #include <assert.h> #include <string.h> +/// Maximum witness length for Bech32 addresses. +static constexpr std::size_t BECH32_WITNESS_PROG_MAX_LEN = 40; + namespace { class DestinationEncoder { @@ -65,10 +68,11 @@ public: std::string operator()(const CNoDestination& no) const { return {}; } }; -CTxDestination DecodeDestination(const std::string& str, const CChainParams& params) +CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, std::string& error_str) { std::vector<unsigned char> data; uint160 hash; + error_str = ""; if (DecodeBase58Check(str, data, 21)) { // base58-encoded Bitcoin addresses. // Public-key-hash-addresses have version 0 (or 111 testnet). @@ -85,10 +89,21 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par std::copy(data.begin() + script_prefix.size(), data.end(), hash.begin()); return ScriptHash(hash); } + + // Set potential error message. + // This message may be changed if the address can also be interpreted as a Bech32 address. + error_str = "Invalid prefix for Base58-encoded address"; } data.clear(); auto bech = bech32::Decode(str); - if (bech.second.size() > 0 && bech.first == params.Bech32HRP()) { + if (bech.second.size() > 0) { + error_str = ""; + + if (bech.first != params.Bech32HRP()) { + error_str = "Invalid prefix for Bech32 address"; + return CNoDestination(); + } + // Bech32 decoding int version = bech.second[0]; // The first 5 bit symbol is the witness version (0-16) // The rest of the symbols are converted witness program bytes. @@ -109,11 +124,21 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par return scriptid; } } + + error_str = "Invalid Bech32 v0 address data size"; + return CNoDestination(); + } + + if (version > 16) { + error_str = "Invalid Bech32 address witness version"; return CNoDestination(); } - if (version > 16 || data.size() < 2 || data.size() > 40) { + + if (data.size() < 2 || data.size() > BECH32_WITNESS_PROG_MAX_LEN) { + error_str = "Invalid Bech32 address data size"; return CNoDestination(); } + WitnessUnknown unk; unk.version = version; std::copy(data.begin(), data.end(), unk.program); @@ -121,6 +146,10 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par return unk; } } + + // Set error message if address can't be interpreted as Base58 or Bech32. + if (error_str.empty()) error_str = "Invalid address format"; + return CNoDestination(); } } // namespace @@ -208,14 +237,21 @@ std::string EncodeDestination(const CTxDestination& dest) return std::visit(DestinationEncoder(Params()), dest); } +CTxDestination DecodeDestination(const std::string& str, std::string& error_msg) +{ + return DecodeDestination(str, Params(), error_msg); +} + CTxDestination DecodeDestination(const std::string& str) { - return DecodeDestination(str, Params()); + std::string error_msg; + return DecodeDestination(str, error_msg); } bool IsValidDestinationString(const std::string& str, const CChainParams& params) { - return IsValidDestination(DecodeDestination(str, params)); + std::string error_msg; + return IsValidDestination(DecodeDestination(str, params, error_msg)); } bool IsValidDestinationString(const std::string& str) diff --git a/src/key_io.h b/src/key_io.h index d80c08f49c..bd81f7847e 100644 --- a/src/key_io.h +++ b/src/key_io.h @@ -23,6 +23,7 @@ std::string EncodeExtPubKey(const CExtPubKey& extpubkey); std::string EncodeDestination(const CTxDestination& dest); CTxDestination DecodeDestination(const std::string& str); +CTxDestination DecodeDestination(const std::string& str, std::string& error_msg); bool IsValidDestinationString(const std::string& str); bool IsValidDestinationString(const std::string& str, const CChainParams& params); diff --git a/src/net.cpp b/src/net.cpp index 4f74bbede4..4759287c36 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -35,6 +35,7 @@ #include <algorithm> #include <cstdint> +#include <functional> #include <unordered_map> #include <math.h> @@ -1216,37 +1217,47 @@ void CConnman::NotifyNumConnectionsChanged() } } -void CConnman::InactivityCheck(CNode *pnode) const +bool CConnman::InactivityCheck(const CNode& node) const { - int64_t nTime = GetSystemTimeInSeconds(); - if (nTime - pnode->nTimeConnected > m_peer_connect_timeout) - { - if (pnode->nLastRecv == 0 || pnode->nLastSend == 0) - { - LogPrint(BCLog::NET, "socket no message in first %i seconds, %d %d from %d\n", m_peer_connect_timeout, pnode->nLastRecv != 0, pnode->nLastSend != 0, pnode->GetId()); - pnode->fDisconnect = true; - } - else if (nTime - pnode->nLastSend > TIMEOUT_INTERVAL) - { - LogPrintf("socket sending timeout: %is\n", nTime - pnode->nLastSend); - pnode->fDisconnect = true; - } - else if (nTime - pnode->nLastRecv > TIMEOUT_INTERVAL) - { - LogPrintf("socket receive timeout: %is\n", nTime - pnode->nLastRecv); - pnode->fDisconnect = true; - } - else if (pnode->nPingNonceSent && pnode->m_ping_start.load() + std::chrono::seconds{TIMEOUT_INTERVAL} < GetTime<std::chrono::microseconds>()) - { - LogPrintf("ping timeout: %fs\n", 0.000001 * count_microseconds(GetTime<std::chrono::microseconds>() - pnode->m_ping_start.load())); - pnode->fDisconnect = true; - } - else if (!pnode->fSuccessfullyConnected) - { - LogPrint(BCLog::NET, "version handshake timeout from %d\n", pnode->GetId()); - pnode->fDisconnect = true; - } + // Use non-mockable system time (otherwise these timers will pop when we + // use setmocktime in the tests). + int64_t now = GetSystemTimeInSeconds(); + + if (now <= node.nTimeConnected + m_peer_connect_timeout) { + // Only run inactivity checks if the peer has been connected longer + // than m_peer_connect_timeout. + return false; } + + if (node.nLastRecv == 0 || node.nLastSend == 0) { + LogPrint(BCLog::NET, "socket no message in first %i seconds, %d %d from %d\n", m_peer_connect_timeout, node.nLastRecv != 0, node.nLastSend != 0, node.GetId()); + return true; + } + + if (now > node.nLastSend + TIMEOUT_INTERVAL) { + LogPrintf("socket sending timeout: %is\n", now - node.nLastSend); + return true; + } + + if (now > node.nLastRecv + TIMEOUT_INTERVAL) { + LogPrintf("socket receive timeout: %is\n", now - node.nLastRecv); + return true; + } + + if (node.nPingNonceSent && node.m_ping_start.load() + std::chrono::seconds{TIMEOUT_INTERVAL} < GetTime<std::chrono::microseconds>()) { + // We use mockable time for ping timeouts. This means that setmocktime + // may cause pings to time out for peers that have been connected for + // longer than m_peer_connect_timeout. + LogPrintf("ping timeout: %fs\n", 0.000001 * count_microseconds(GetTime<std::chrono::microseconds>() - node.m_ping_start.load())); + return true; + } + + if (!node.fSuccessfullyConnected) { + LogPrint(BCLog::NET, "version handshake timeout from %d\n", node.GetId()); + return true; + } + + return false; } bool CConnman::GenerateSelectSet(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set) @@ -1522,7 +1533,7 @@ void CConnman::SocketHandler() if (bytes_sent) RecordBytesSent(bytes_sent); } - InactivityCheck(pnode); + if (InactivityCheck(*pnode)) pnode->fDisconnect = true; } { LOCK(cs_vNodes); @@ -1044,7 +1044,8 @@ private: void AcceptConnection(const ListenSocket& hListenSocket); void DisconnectNodes(); void NotifyNumConnectionsChanged(); - void InactivityCheck(CNode *pnode) const; + /** Return true if the peer is inactive and should be disconnected. */ + bool InactivityCheck(const CNode& node) const; bool GenerateSelectSet(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set); void SocketEvents(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set); void SocketHandler(); diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index f2a49e5a76..ccc0a6828c 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -30,6 +30,7 @@ #include <qt/macdockiconhandler.h> #endif +#include <functional> #include <chain.h> #include <chainparams.h> #include <interfaces/handler.h> @@ -845,7 +846,7 @@ void BitcoinGUI::showDebugWindowActivateConsole() void BitcoinGUI::showHelpMessageClicked() { - helpMessageDialog->show(); + GUIUtil::bringToFront(helpMessageDialog); } #ifdef ENABLE_WALLET diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index a2f46c339b..56813b2d19 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -19,6 +19,7 @@ #include <validation.h> #include <stdint.h> +#include <functional> #include <QDebug> #include <QThread> diff --git a/src/qt/forms/createwalletdialog.ui b/src/qt/forms/createwalletdialog.ui index 0b33c2cb8d..881869a46c 100644 --- a/src/qt/forms/createwalletdialog.ui +++ b/src/qt/forms/createwalletdialog.ui @@ -7,140 +7,135 @@ <x>0</x> <y>0</y> <width>364</width> - <height>213</height> + <height>249</height> </rect> </property> <property name="windowTitle"> <string>Create Wallet</string> </property> - <widget class="QDialogButtonBox" name="buttonBox"> - <property name="geometry"> - <rect> - <x>10</x> - <y>170</y> - <width>341</width> - <height>32</height> - </rect> - </property> - <property name="orientation"> - <enum>Qt::Horizontal</enum> - </property> - <property name="standardButtons"> - <set>QDialogButtonBox::Cancel|QDialogButtonBox::Ok</set> - </property> - </widget> - <widget class="QLineEdit" name="wallet_name_line_edit"> - <property name="geometry"> - <rect> - <x>120</x> - <y>20</y> - <width>231</width> - <height>24</height> - </rect> - </property> - <property name="placeholderText"> - <string>Wallet</string> - </property> - </widget> - <widget class="QLabel" name="label"> - <property name="geometry"> - <rect> - <x>20</x> - <y>20</y> - <width>101</width> - <height>21</height> - </rect> - </property> - <property name="text"> - <string>Wallet Name</string> - </property> - </widget> - <widget class="QCheckBox" name="encrypt_wallet_checkbox"> - <property name="geometry"> - <rect> - <x>20</x> - <y>50</y> - <width>220</width> - <height>22</height> - </rect> - </property> - <property name="toolTip"> - <string>Encrypt the wallet. The wallet will be encrypted with a passphrase of your choice.</string> - </property> - <property name="text"> - <string>Encrypt Wallet</string> - </property> - <property name="checked"> - <bool>false</bool> - </property> - </widget> - <widget class="QLabel" name="advanced_options_label"> - <property name="geometry"> - <rect> - <x>20</x> - <y>90</y> - <width>220</width> - <height>21</height> - </rect> - </property> - <property name="styleSheet"> - <string notr="true">font-weight:bold;</string> - </property> - <property name="text"> - <string>Advanced options</string> - </property> - </widget> - <widget class="QCheckBox" name="disable_privkeys_checkbox"> - <property name="enabled"> - <bool>true</bool> - </property> - <property name="geometry"> - <rect> - <x>20</x> - <y>115</y> - <width>220</width> - <height>22</height> - </rect> - </property> - <property name="toolTip"> - <string>Disable private keys for this wallet. Wallets with private keys disabled will have no private keys and cannot have an HD seed or imported private keys. This is ideal for watch-only wallets.</string> - </property> - <property name="text"> - <string>Disable Private Keys</string> - </property> - </widget> - <widget class="QCheckBox" name="blank_wallet_checkbox"> - <property name="geometry"> - <rect> - <x>20</x> - <y>135</y> - <width>220</width> - <height>22</height> - </rect> - </property> - <property name="toolTip"> - <string>Make a blank wallet. Blank wallets do not initially have private keys or scripts. Private keys and addresses can be imported, or an HD seed can be set, at a later time.</string> - </property> - <property name="text"> - <string>Make Blank Wallet</string> - </property> - </widget> - <widget class="QCheckBox" name="descriptor_checkbox"> - <property name="geometry"> - <rect> - <x>20</x> - <y>155</y> - <width>220</width> - <height>22</height> - </rect> - </property> - <property name="toolTip"> - <string>Use descriptors for scriptPubKey management</string> - </property> - <property name="text"> - <string>Descriptor Wallet</string> - </property> - </widget> + <property name="sizeGripEnabled"> + <bool>true</bool> + </property> + <layout class="QVBoxLayout" name="verticalLayout"> + <item> + <layout class="QHBoxLayout" name="horizontalLayout"> + <item> + <widget class="QLabel" name="wallet_name_label"> + <property name="text"> + <string>Wallet Name</string> + </property> + </widget> + </item> + <item> + <widget class="QLineEdit" name="wallet_name_line_edit"> + <property name="minimumSize"> + <size> + <width>262</width> + <height>0</height> + </size> + </property> + <property name="placeholderText"> + <string>Wallet</string> + </property> + </widget> + </item> + </layout> + </item> + <item> + <widget class="QCheckBox" name="encrypt_wallet_checkbox"> + <property name="toolTip"> + <string>Encrypt the wallet. The wallet will be encrypted with a passphrase of your choice.</string> + </property> + <property name="text"> + <string>Encrypt Wallet</string> + </property> + <property name="checked"> + <bool>false</bool> + </property> + </widget> + </item> + <item> + <spacer name="verticalSpacer_1"> + <property name="orientation"> + <enum>Qt::Vertical</enum> + </property> + <property name="sizeType"> + <enum>QSizePolicy::Fixed</enum> + </property> + <property name="sizeHint" stdset="0"> + <size> + <width>20</width> + <height>8</height> + </size> + </property> + </spacer> + </item> + <item> + <widget class="QGroupBox" name="groupBox"> + <property name="title"> + <string>Advanced Options</string> + </property> + <layout class="QVBoxLayout" name="verticalLayout_groupbox"> + <item> + <widget class="QCheckBox" name="disable_privkeys_checkbox"> + <property name="enabled"> + <bool>true</bool> + </property> + <property name="toolTip"> + <string>Disable private keys for this wallet. Wallets with private keys disabled will have no private keys and cannot have an HD seed or imported private keys. This is ideal for watch-only wallets.</string> + </property> + <property name="text"> + <string>Disable Private Keys</string> + </property> + </widget> + </item> + <item> + <widget class="QCheckBox" name="blank_wallet_checkbox"> + <property name="toolTip"> + <string>Make a blank wallet. Blank wallets do not initially have private keys or scripts. Private keys and addresses can be imported, or an HD seed can be set, at a later time.</string> + </property> + <property name="text"> + <string>Make Blank Wallet</string> + </property> + </widget> + </item> + <item> + <widget class="QCheckBox" name="descriptor_checkbox"> + <property name="toolTip"> + <string>Use descriptors for scriptPubKey management</string> + </property> + <property name="text"> + <string>Descriptor Wallet</string> + </property> + </widget> + </item> + </layout> + </widget> + </item> + <item> + <spacer name="verticalSpacer_2"> + <property name="orientation"> + <enum>Qt::Vertical</enum> + </property> + <property name="sizeHint" stdset="0"> + <size> + <width>20</width> + <height>0</height> + </size> + </property> + </spacer> + </item> + <item> + <widget class="QDialogButtonBox" name="buttonBox"> + <property name="orientation"> + <enum>Qt::Horizontal</enum> + </property> + <property name="standardButtons"> + <set>QDialogButtonBox::Cancel|QDialogButtonBox::Ok</set> + </property> + </widget> + </item> + </layout> </widget> <tabstops> <tabstop>wallet_name_line_edit</tabstop> diff --git a/src/qt/forms/overviewpage.ui b/src/qt/forms/overviewpage.ui index 4d3f90c484..ee9d4a113c 100644 --- a/src/qt/forms/overviewpage.ui +++ b/src/qt/forms/overviewpage.ui @@ -504,7 +504,7 @@ </layout> </item> <item> - <widget class="QListView" name="listTransactions"> + <widget class="TransactionOverviewWidget" name="listTransactions"> <property name="styleSheet"> <string notr="true">QListView { background: transparent; }</string> </property> @@ -517,9 +517,15 @@ <property name="horizontalScrollBarPolicy"> <enum>Qt::ScrollBarAlwaysOff</enum> </property> + <property name="sizeAdjustPolicy"> + <enum>QAbstractScrollArea::AdjustToContents</enum> + </property> <property name="selectionMode"> <enum>QAbstractItemView::NoSelection</enum> </property> + <property name="uniformItemSizes"> + <bool>true</bool> + </property> </widget> </item> </layout> @@ -544,6 +550,13 @@ </item> </layout> </widget> + <customwidgets> + <customwidget> + <class>TransactionOverviewWidget</class> + <extends>QListView</extends> + <header>qt/transactionoverviewwidget.h</header> + </customwidget> + </customwidgets> <resources/> <connections/> </ui> diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index cca4dce624..bc542a0833 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -12,6 +12,7 @@ #include <qt/optionsmodel.h> #include <qt/platformstyle.h> #include <qt/transactionfilterproxy.h> +#include <qt/transactionoverviewwidget.h> #include <qt/transactiontablemodel.h> #include <qt/walletmodel.h> @@ -21,6 +22,9 @@ #include <QPainter> #include <QStatusTipEvent> +#include <algorithm> +#include <map> + #define DECORATION_SIZE 54 #define NUM_ITEMS 5 @@ -34,7 +38,7 @@ public: QAbstractItemDelegate(parent), unit(BitcoinUnits::BTC), platformStyle(_platformStyle) { - + connect(this, &TxViewDelegate::width_changed, this, &TxViewDelegate::sizeHintChanged); } inline void paint(QPainter *painter, const QStyleOptionViewItem &option, @@ -67,13 +71,15 @@ public: painter->setPen(foreground); QRect boundingRect; - painter->drawText(addressRect, Qt::AlignLeft|Qt::AlignVCenter, address, &boundingRect); + painter->drawText(addressRect, Qt::AlignLeft | Qt::AlignVCenter, address, &boundingRect); + int address_rect_min_width = boundingRect.width(); if (index.data(TransactionTableModel::WatchonlyRole).toBool()) { QIcon iconWatchonly = qvariant_cast<QIcon>(index.data(TransactionTableModel::WatchonlyDecorationRole)); QRect watchonlyRect(boundingRect.right() + 5, mainRect.top()+ypad+halfheight, 16, halfheight); iconWatchonly.paint(painter, watchonlyRect); + address_rect_min_width += 5 + watchonlyRect.width(); } if(amount < 0) @@ -94,23 +100,42 @@ public: { amountText = QString("[") + amountText + QString("]"); } - painter->drawText(amountRect, Qt::AlignRight|Qt::AlignVCenter, amountText); + + QRect amount_bounding_rect; + painter->drawText(amountRect, Qt::AlignRight | Qt::AlignVCenter, amountText, &amount_bounding_rect); painter->setPen(option.palette.color(QPalette::Text)); - painter->drawText(amountRect, Qt::AlignLeft|Qt::AlignVCenter, GUIUtil::dateTimeStr(date)); + QRect date_bounding_rect; + painter->drawText(amountRect, Qt::AlignLeft | Qt::AlignVCenter, GUIUtil::dateTimeStr(date), &date_bounding_rect); + + const int minimum_width = std::max(address_rect_min_width, amount_bounding_rect.width() + date_bounding_rect.width()); + const auto search = m_minimum_width.find(index.row()); + if (search == m_minimum_width.end() || search->second != minimum_width) { + m_minimum_width[index.row()] = minimum_width; + Q_EMIT width_changed(index); + } painter->restore(); } inline QSize sizeHint(const QStyleOptionViewItem &option, const QModelIndex &index) const override { - return QSize(DECORATION_SIZE, DECORATION_SIZE); + const auto search = m_minimum_width.find(index.row()); + const int minimum_text_width = search == m_minimum_width.end() ? 0 : search->second; + return {DECORATION_SIZE + 8 + minimum_text_width, DECORATION_SIZE}; } int unit; - const PlatformStyle *platformStyle; +Q_SIGNALS: + //! An intermediate signal for emitting from the `paint() const` member function. + void width_changed(const QModelIndex& index) const; + +private: + const PlatformStyle* platformStyle; + mutable std::map<int, int> m_minimum_width; }; + #include <qt/overviewpage.moc> OverviewPage::OverviewPage(const PlatformStyle *platformStyle, QWidget *parent) : @@ -126,7 +151,6 @@ OverviewPage::OverviewPage(const PlatformStyle *platformStyle, QWidget *parent) // use a SingleColorIcon for the "out of sync warning" icon QIcon icon = platformStyle->SingleColorIcon(":/icons/warning"); - icon.addPixmap(icon.pixmap(QSize(64,64), QIcon::Normal), QIcon::Disabled); // also set the disabled icon because we are using a disabled QPushButton to work around missing HiDPI support of QLabel (https://bugreports.qt.io/browse/QTBUG-42503) ui->labelTransactionsStatus->setIcon(icon); ui->labelWalletStatus->setIcon(icon); @@ -136,7 +160,7 @@ OverviewPage::OverviewPage(const PlatformStyle *platformStyle, QWidget *parent) ui->listTransactions->setMinimumHeight(NUM_ITEMS * (DECORATION_SIZE + 2)); ui->listTransactions->setAttribute(Qt::WA_MacShowFocusRect, false); - connect(ui->listTransactions, &QListView::clicked, this, &OverviewPage::handleTransactionClicked); + connect(ui->listTransactions, &TransactionOverviewWidget::clicked, this, &OverviewPage::handleTransactionClicked); // start with displaying the "out of sync" warnings showOutOfSyncWarning(true); diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp index 5796a6ef56..2292c01d6a 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -18,6 +18,8 @@ #include <util/system.h> #include <util/translation.h> +#include <functional> + #include <QApplication> #include <QCloseEvent> #include <QPainter> diff --git a/src/qt/transactionoverviewwidget.h b/src/qt/transactionoverviewwidget.h new file mode 100644 index 0000000000..2bdead7bc4 --- /dev/null +++ b/src/qt/transactionoverviewwidget.h @@ -0,0 +1,41 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_QT_TRANSACTIONOVERVIEWWIDGET_H +#define BITCOIN_QT_TRANSACTIONOVERVIEWWIDGET_H + +#include <qt/transactiontablemodel.h> + +#include <QListView> +#include <QSize> +#include <QSizePolicy> + +QT_BEGIN_NAMESPACE +class QShowEvent; +class QWidget; +QT_END_NAMESPACE + +class TransactionOverviewWidget : public QListView +{ + Q_OBJECT + +public: + explicit TransactionOverviewWidget(QWidget* parent = nullptr) : QListView(parent) {} + + QSize sizeHint() const override + { + return {sizeHintForColumn(TransactionTableModel::ToAddress), QListView::sizeHint().height()}; + } + +protected: + void showEvent(QShowEvent* event) override + { + Q_UNUSED(event); + QSizePolicy sp = sizePolicy(); + sp.setHorizontalPolicy(QSizePolicy::Minimum); + setSizePolicy(sp); + } +}; + +#endif // BITCOIN_QT_TRANSACTIONOVERVIEWWIDGET_H diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 05b250eea9..e6d483364c 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -19,6 +19,7 @@ #include <uint256.h> #include <algorithm> +#include <functional> #include <QColor> #include <QDateTime> diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index 2edb7eff8a..02b3c62867 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -106,9 +106,24 @@ void WalletFrame::setCurrentWallet(WalletModel* wallet_model) { if (mapWalletViews.count(wallet_model) == 0) return; + // Stop the effect of hidden widgets on the size hint of the shown one in QStackedWidget. + WalletView* view_about_to_hide = currentWalletView(); + if (view_about_to_hide) { + QSizePolicy sp = view_about_to_hide->sizePolicy(); + sp.setHorizontalPolicy(QSizePolicy::Ignored); + view_about_to_hide->setSizePolicy(sp); + } + WalletView *walletView = mapWalletViews.value(wallet_model); - walletStack->setCurrentWidget(walletView); assert(walletView); + + // Set or restore the default QSizePolicy which could be set to QSizePolicy::Ignored previously. + QSizePolicy sp = walletView->sizePolicy(); + sp.setHorizontalPolicy(QSizePolicy::Preferred); + walletView->setSizePolicy(sp); + walletView->updateGeometry(); + + walletStack->setCurrentWidget(walletView); walletView->updateEncryptionStatus(); } diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index cad472b43b..58427b42cc 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -29,6 +29,7 @@ #include <wallet/wallet.h> // for CRecipient #include <stdint.h> +#include <functional> #include <QDebug> #include <QMessageBox> @@ -514,6 +515,13 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) questionString.append("</td><td>"); questionString.append(BitcoinUnits::formatHtmlWithUnit(getOptionsModel()->getDisplayUnit(), new_fee)); questionString.append("</td></tr></table>"); + + // Display warning in the "Confirm fee bump" window if the "Coin Control Features" option is enabled + if (getOptionsModel()->getCoinControlFeatures()) { + questionString.append("<br><br>"); + questionString.append(tr("Warning: This may pay the additional fee by reducing change outputs or adding inputs, when necessary. It may add a new change output if one does not already exist. These changes may potentially leak privacy.")); + } + SendConfirmationDialog confirmationDialog(tr("Confirm fee bump"), questionString); confirmationDialog.exec(); QMessageBox::StandardButton retval = static_cast<QMessageBox::StandardButton>(confirmationDialog.result()); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 3f97f6f3d9..7a336c1ad6 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2399,10 +2399,10 @@ static RPCHelpMan dumptxoutset() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - fs::path path = fs::absolute(request.params[0].get_str(), GetDataDir()); + const fs::path path = fsbridge::AbsPathJoin(GetDataDir(), request.params[0].get_str()); // Write to a temporary path and then move into `path` on completion // to avoid confusion due to an interruption. - fs::path temppath = fs::absolute(request.params[0].get_str() + ".incomplete", GetDataDir()); + const fs::path temppath = fsbridge::AbsPathJoin(GetDataDir(), request.params[0].get_str() + ".incomplete"); if (fs::exists(path)) { throw JSONRPCError( diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index b3102a236d..215e48ca26 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -39,13 +39,14 @@ static RPCHelpMan validateaddress() RPCResult{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "isvalid", "If the address is valid or not. If not, this is the only property returned."}, + {RPCResult::Type::BOOL, "isvalid", "If the address is valid or not"}, {RPCResult::Type::STR, "address", "The bitcoin address validated"}, {RPCResult::Type::STR_HEX, "scriptPubKey", "The hex-encoded scriptPubKey generated by the address"}, {RPCResult::Type::BOOL, "isscript", "If the key is a script"}, {RPCResult::Type::BOOL, "iswitness", "If the address is a witness address"}, {RPCResult::Type::NUM, "witness_version", /* optional */ true, "The version number of the witness program"}, {RPCResult::Type::STR_HEX, "witness_program", /* optional */ true, "The hex value of the witness program"}, + {RPCResult::Type::STR, "error", /* optional */ true, "Error message, if any"}, } }, RPCExamples{ @@ -54,13 +55,14 @@ static RPCHelpMan validateaddress() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - CTxDestination dest = DecodeDestination(request.params[0].get_str()); - bool isValid = IsValidDestination(dest); + std::string error_msg; + CTxDestination dest = DecodeDestination(request.params[0].get_str(), error_msg); + const bool isValid = IsValidDestination(dest); + CHECK_NONFATAL(isValid == error_msg.empty()); UniValue ret(UniValue::VOBJ); ret.pushKV("isvalid", isValid); - if (isValid) - { + if (isValid) { std::string currentAddress = EncodeDestination(dest); ret.pushKV("address", currentAddress); @@ -69,7 +71,10 @@ static RPCHelpMan validateaddress() UniValue detail = DescribeAddress(dest); ret.pushKVs(detail); + } else { + ret.pushKV("error", error_msg); } + return ret; }, }; diff --git a/src/scheduler.cpp b/src/scheduler.cpp index 7c361bf26f..b3ee23f139 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -7,6 +7,7 @@ #include <random.h> #include <assert.h> +#include <functional> #include <utility> CScheduler::CScheduler() diff --git a/src/sync.cpp b/src/sync.cpp index acfbe8fe29..a2b62c2286 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -13,8 +13,6 @@ #include <util/strencodings.h> #include <util/threadnames.h> -#include <boost/thread/mutex.hpp> - #include <map> #include <mutex> #include <set> @@ -224,7 +222,6 @@ template void EnterCritical(const char*, const char*, int, Mutex*, bool); template void EnterCritical(const char*, const char*, int, RecursiveMutex*, bool); template void EnterCritical(const char*, const char*, int, std::mutex*, bool); template void EnterCritical(const char*, const char*, int, std::recursive_mutex*, bool); -template void EnterCritical(const char*, const char*, int, boost::mutex*, bool); void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) { diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index 5dbf07b420..76d6727044 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -148,10 +148,7 @@ typedef CCheckQueue<FrozenCleanupCheck> FrozenCleanup_Queue; static void Correct_Queue_range(std::vector<size_t> range) { auto small_queue = MakeUnique<Correct_Queue>(QUEUE_BATCH_SIZE); - boost::thread_group tg; - for (auto x = 0; x < SCRIPT_CHECK_THREADS; ++x) { - tg.create_thread([&]{small_queue->Thread();}); - } + small_queue->StartWorkerThreads(SCRIPT_CHECK_THREADS); // Make vChecks here to save on malloc (this test can be slow...) std::vector<FakeCheckCheckCompletion> vChecks; for (const size_t i : range) { @@ -168,8 +165,7 @@ static void Correct_Queue_range(std::vector<size_t> range) BOOST_REQUIRE_EQUAL(FakeCheckCheckCompletion::n_calls, i); } } - tg.interrupt_all(); - tg.join_all(); + small_queue->StopWorkerThreads(); } /** Test that 0 checks is correct @@ -212,11 +208,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Correct_Random) BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure) { auto fail_queue = MakeUnique<Failing_Queue>(QUEUE_BATCH_SIZE); - - boost::thread_group tg; - for (auto x = 0; x < SCRIPT_CHECK_THREADS; ++x) { - tg.create_thread([&]{fail_queue->Thread();}); - } + fail_queue->StartWorkerThreads(SCRIPT_CHECK_THREADS); for (size_t i = 0; i < 1001; ++i) { CCheckQueueControl<FailingCheck> control(fail_queue.get()); @@ -237,18 +229,14 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure) BOOST_REQUIRE(success); } } - tg.interrupt_all(); - tg.join_all(); + fail_queue->StopWorkerThreads(); } // Test that a block validation which fails does not interfere with // future blocks, ie, the bad state is cleared. BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure) { auto fail_queue = MakeUnique<Failing_Queue>(QUEUE_BATCH_SIZE); - boost::thread_group tg; - for (auto x = 0; x < SCRIPT_CHECK_THREADS; ++x) { - tg.create_thread([&]{fail_queue->Thread();}); - } + fail_queue->StartWorkerThreads(SCRIPT_CHECK_THREADS); for (auto times = 0; times < 10; ++times) { for (const bool end_fails : {true, false}) { @@ -263,8 +251,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure) BOOST_REQUIRE(r != end_fails); } } - tg.interrupt_all(); - tg.join_all(); + fail_queue->StopWorkerThreads(); } // Test that unique checks are actually all called individually, rather than @@ -273,11 +260,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure) BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck) { auto queue = MakeUnique<Unique_Queue>(QUEUE_BATCH_SIZE); - boost::thread_group tg; - for (auto x = 0; x < SCRIPT_CHECK_THREADS; ++x) { - tg.create_thread([&]{queue->Thread();}); - - } + queue->StartWorkerThreads(SCRIPT_CHECK_THREADS); size_t COUNT = 100000; size_t total = COUNT; @@ -300,8 +283,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck) } BOOST_REQUIRE(r); } - tg.interrupt_all(); - tg.join_all(); + queue->StopWorkerThreads(); } @@ -313,10 +295,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck) BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory) { auto queue = MakeUnique<Memory_Queue>(QUEUE_BATCH_SIZE); - boost::thread_group tg; - for (auto x = 0; x < SCRIPT_CHECK_THREADS; ++x) { - tg.create_thread([&]{queue->Thread();}); - } + queue->StartWorkerThreads(SCRIPT_CHECK_THREADS); for (size_t i = 0; i < 1000; ++i) { size_t total = i; { @@ -335,8 +314,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory) } BOOST_REQUIRE_EQUAL(MemoryCheck::fake_allocated_memory, 0U); } - tg.interrupt_all(); - tg.join_all(); + queue->StopWorkerThreads(); } // Test that a new verification cannot occur until all checks @@ -344,11 +322,8 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory) BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup) { auto queue = MakeUnique<FrozenCleanup_Queue>(QUEUE_BATCH_SIZE); - boost::thread_group tg; bool fails = false; - for (auto x = 0; x < SCRIPT_CHECK_THREADS; ++x) { - tg.create_thread([&]{queue->Thread();}); - } + queue->StartWorkerThreads(SCRIPT_CHECK_THREADS); std::thread t0([&]() { CCheckQueueControl<FrozenCleanupCheck> control(queue.get()); std::vector<FrozenCleanupCheck> vChecks(1); @@ -367,7 +342,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup) } // Try to get control of the queue a bunch of times for (auto x = 0; x < 100 && !fails; ++x) { - fails = queue->ControlMutex.try_lock(); + fails = queue->m_control_mutex.try_lock(); } { // Unfreeze (we need lock n case of spurious wakeup) @@ -378,9 +353,8 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup) FrozenCleanupCheck::cv.notify_one(); // Wait for control to finish t0.join(); - tg.interrupt_all(); - tg.join_all(); BOOST_REQUIRE(!fails); + queue->StopWorkerThreads(); } @@ -431,7 +405,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks) cv.wait(l, [&](){return has_lock;}); bool fails = false; for (auto x = 0; x < 100 && !fails; ++x) { - fails = queue->ControlMutex.try_lock(); + fails = queue->m_control_mutex.try_lock(); } has_tried = true; cv.notify_one(); @@ -445,4 +419,3 @@ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks) } } BOOST_AUTO_TEST_SUITE_END() - diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp index d02c3613ba..ec487aa3ff 100644 --- a/src/test/fs_tests.cpp +++ b/src/test/fs_tests.cpp @@ -52,6 +52,23 @@ BOOST_AUTO_TEST_CASE(fsbridge_fstream) file >> input_buffer; BOOST_CHECK_EQUAL(input_buffer, "bitcoin"); } + { + // Join an absolute path and a relative path. + fs::path p = fsbridge::AbsPathJoin(tmpfolder, "fs_tests_₿_🏃"); + BOOST_CHECK(p.is_absolute()); + BOOST_CHECK_EQUAL(tmpfile1, p); + } + { + // Join two absolute paths. + fs::path p = fsbridge::AbsPathJoin(tmpfile1, tmpfile2); + BOOST_CHECK(p.is_absolute()); + BOOST_CHECK_EQUAL(tmpfile2, p); + } + { + // Ensure joining with empty paths does not add trailing path components. + BOOST_CHECK_EQUAL(tmpfile1, fsbridge::AbsPathJoin(tmpfile1, "")); + BOOST_CHECK_EQUAL(tmpfile1, fsbridge::AbsPathJoin(tmpfile1, {})); + } } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/banman.cpp b/src/test/fuzz/banman.cpp index e703fa39c1..e0715f3e29 100644 --- a/src/test/fuzz/banman.cpp +++ b/src/test/fuzz/banman.cpp @@ -26,7 +26,7 @@ int64_t ConsumeBanTimeOffset(FuzzedDataProvider& fuzzed_data_provider) noexcept void initialize_banman() { - InitializeFuzzingContext(); + static const auto testing_setup = MakeFuzzingContext<>(); } FUZZ_TARGET_INIT(banman, initialize_banman) diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 12ef4b203f..8ece94d771 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -36,9 +36,7 @@ bool operator==(const Coin& a, const Coin& b) void initialize_coins_view() { - static const ECCVerifyHandle ecc_verify_handle; - ECC_Start(); - SelectParams(CBaseChainParams::REGTEST); + static const auto testing_setup = MakeFuzzingContext<const TestingSetup>(); } FUZZ_TARGET_INIT(coins_view, initialize_coins_view) diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 7950213bd5..71b4b00116 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -17,7 +17,7 @@ void initialize_connman() { - InitializeFuzzingContext(); + static const auto testing_setup = MakeFuzzingContext<>(); } FUZZ_TARGET_INIT(connman, initialize_connman) diff --git a/src/test/fuzz/crypto_chacha20_poly1305_aead.cpp b/src/test/fuzz/crypto_chacha20_poly1305_aead.cpp index 1f122082b2..0e1c44cded 100644 --- a/src/test/fuzz/crypto_chacha20_poly1305_aead.cpp +++ b/src/test/fuzz/crypto_chacha20_poly1305_aead.cpp @@ -45,18 +45,24 @@ FUZZ_TARGET(crypto_chacha20_poly1305_aead) assert(ok); }, [&] { + if (AdditionOverflow(seqnr_payload, static_cast<uint64_t>(1))) { + return; + } seqnr_payload += 1; aad_pos += CHACHA20_POLY1305_AEAD_AAD_LEN; if (aad_pos + CHACHA20_POLY1305_AEAD_AAD_LEN > CHACHA20_ROUND_OUTPUT) { aad_pos = 0; + if (AdditionOverflow(seqnr_aad, static_cast<uint64_t>(1))) { + return; + } seqnr_aad += 1; } }, [&] { - seqnr_payload = fuzzed_data_provider.ConsumeIntegral<int>(); + seqnr_payload = fuzzed_data_provider.ConsumeIntegral<uint64_t>(); }, [&] { - seqnr_aad = fuzzed_data_provider.ConsumeIntegral<int>(); + seqnr_aad = fuzzed_data_provider.ConsumeIntegral<uint64_t>(); }, [&] { is_encrypt = fuzzed_data_provider.ConsumeBool(); diff --git a/src/test/fuzz/data_stream.cpp b/src/test/fuzz/data_stream.cpp index 28fc528ceb..f3b6e6af04 100644 --- a/src/test/fuzz/data_stream.cpp +++ b/src/test/fuzz/data_stream.cpp @@ -13,7 +13,7 @@ void initialize_data_stream_addr_man() { - InitializeFuzzingContext(); + static const auto testing_setup = MakeFuzzingContext<>(); } FUZZ_TARGET_INIT(data_stream_addr_man, initialize_data_stream_addr_man) diff --git a/src/test/fuzz/load_external_block_file.cpp b/src/test/fuzz/load_external_block_file.cpp index c428a86631..207ee586bc 100644 --- a/src/test/fuzz/load_external_block_file.cpp +++ b/src/test/fuzz/load_external_block_file.cpp @@ -15,7 +15,7 @@ void initialize_load_external_block_file() { - InitializeFuzzingContext(); + static const auto testing_setup = MakeFuzzingContext<const TestingSetup>(); } FUZZ_TARGET_INIT(load_external_block_file, initialize_load_external_block_file) diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp index 31b99600ef..21dca4eb05 100644 --- a/src/test/fuzz/net.cpp +++ b/src/test/fuzz/net.cpp @@ -22,7 +22,7 @@ void initialize_net() { - static const BasicTestingSetup basic_testing_setup; + static const auto testing_setup = MakeFuzzingContext<>(CBaseChainParams::MAIN); } FUZZ_TARGET_INIT(net, initialize_net) diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp index 0393491e4b..fff893fb3f 100644 --- a/src/test/fuzz/policy_estimator.cpp +++ b/src/test/fuzz/policy_estimator.cpp @@ -16,7 +16,7 @@ void initialize_policy_estimator() { - InitializeFuzzingContext(); + static const auto testing_setup = MakeFuzzingContext<>(); } FUZZ_TARGET_INIT(policy_estimator, initialize_policy_estimator) diff --git a/src/test/fuzz/policy_estimator_io.cpp b/src/test/fuzz/policy_estimator_io.cpp index 8fa52143d8..73242870a0 100644 --- a/src/test/fuzz/policy_estimator_io.cpp +++ b/src/test/fuzz/policy_estimator_io.cpp @@ -12,7 +12,7 @@ void initialize_policy_estimator_io() { - InitializeFuzzingContext(); + static const auto testing_setup = MakeFuzzingContext<>(); } FUZZ_TARGET_INIT(policy_estimator_io, initialize_policy_estimator_io) diff --git a/src/test/fuzz/pow.cpp b/src/test/fuzz/pow.cpp index 02beb6eb37..c4348495bf 100644 --- a/src/test/fuzz/pow.cpp +++ b/src/test/fuzz/pow.cpp @@ -43,7 +43,10 @@ FUZZ_TARGET_INIT(pow, initialize_pow) current_block.nHeight = current_height; } if (fuzzed_data_provider.ConsumeBool()) { - current_block.nTime = fixed_time + current_height * consensus_params.nPowTargetSpacing; + const uint32_t seconds = current_height * consensus_params.nPowTargetSpacing; + if (!AdditionOverflow(fixed_time, seconds)) { + current_block.nTime = fixed_time + seconds; + } } if (fuzzed_data_provider.ConsumeBool()) { current_block.nBits = fixed_bits; diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index e9aeef1d32..5d6a33d7c2 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -38,14 +38,8 @@ const TestingSetup* g_setup; void initialize_process_message() { - static TestingSetup setup{ - CBaseChainParams::REGTEST, - { - "-nodebuglogfile", - }, - }; - g_setup = &setup; - + static const auto testing_setup = MakeFuzzingContext<const TestingSetup>(); + g_setup = testing_setup.get(); for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { MineBlock(g_setup->m_node, CScript() << OP_TRUE); } diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index a54a3d5be1..d0d0e19694 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -17,18 +17,14 @@ #include <validation.h> #include <validationinterface.h> +namespace { const TestingSetup* g_setup; +} // namespace void initialize_process_messages() { - static TestingSetup setup{ - CBaseChainParams::REGTEST, - { - "-nodebuglogfile", - }, - }; - g_setup = &setup; - + static const auto testing_setup = MakeFuzzingContext<const TestingSetup>(); + g_setup = testing_setup.get(); for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { MineBlock(g_setup->m_node, CScript() << OP_TRUE); } diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp index d883426c81..7fadf36f98 100644 --- a/src/test/fuzz/script.cpp +++ b/src/test/fuzz/script.cpp @@ -154,13 +154,13 @@ FUZZ_TARGET_INIT(script, initialize_script) { WitnessUnknown witness_unknown_1{}; - witness_unknown_1.version = fuzzed_data_provider.ConsumeIntegral<int>(); + witness_unknown_1.version = fuzzed_data_provider.ConsumeIntegral<uint32_t>(); const std::vector<uint8_t> witness_unknown_program_1 = fuzzed_data_provider.ConsumeBytes<uint8_t>(40); witness_unknown_1.length = witness_unknown_program_1.size(); std::copy(witness_unknown_program_1.begin(), witness_unknown_program_1.end(), witness_unknown_1.program); WitnessUnknown witness_unknown_2{}; - witness_unknown_2.version = fuzzed_data_provider.ConsumeIntegral<int>(); + witness_unknown_2.version = fuzzed_data_provider.ConsumeIntegral<uint32_t>(); const std::vector<uint8_t> witness_unknown_program_2 = fuzzed_data_provider.ConsumeBytes<uint8_t>(40); witness_unknown_2.length = witness_unknown_program_2.size(); std::copy(witness_unknown_program_2.begin(), witness_unknown_program_2.end(), witness_unknown_2.program); diff --git a/src/test/fuzz/signet.cpp b/src/test/fuzz/signet.cpp index 541322d484..83effec064 100644 --- a/src/test/fuzz/signet.cpp +++ b/src/test/fuzz/signet.cpp @@ -17,7 +17,7 @@ void initialize_signet() { - InitializeFuzzingContext(CBaseChainParams::SIGNET); + static const auto testing_setup = MakeFuzzingContext<>(CBaseChainParams::SIGNET); } FUZZ_TARGET_INIT(signet, initialize_signet) diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index b7cf395e76..5b8eacf051 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -27,6 +27,7 @@ #include <txmempool.h> #include <uint256.h> #include <util/time.h> +#include <util/vector.h> #include <version.h> #include <algorithm> @@ -195,7 +196,7 @@ template <typename WeakEnumType, size_t size> }, [&] { WitnessUnknown witness_unknown{}; - witness_unknown.version = fuzzed_data_provider.ConsumeIntegral<int>(); + witness_unknown.version = fuzzed_data_provider.ConsumeIntegral<uint32_t>(); const std::vector<uint8_t> witness_unknown_program_1 = fuzzed_data_provider.ConsumeBytes<uint8_t>(40); witness_unknown.length = witness_unknown_program_1.size(); std::copy(witness_unknown_program_1.begin(), witness_unknown_program_1.end(), witness_unknown.program); @@ -338,9 +339,17 @@ inline void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, cons } } -inline void InitializeFuzzingContext(const std::string& chain_name = CBaseChainParams::REGTEST) +template <class T = const BasicTestingSetup> +std::unique_ptr<T> MakeFuzzingContext(const std::string& chain_name = CBaseChainParams::REGTEST, const std::vector<const char*>& extra_args = {}) { - static const BasicTestingSetup basic_testing_setup{chain_name, {"-nodebuglogfile"}}; + // Prepend default arguments for fuzzing + const std::vector<const char*> arguments = Cat( + { + "-nodebuglogfile", + }, + extra_args); + + return MakeUnique<T>(chain_name, arguments); } class FuzzedFileProvider diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp index 2e5a7549b7..21162356b8 100644 --- a/src/test/scheduler_tests.cpp +++ b/src/test/scheduler_tests.cpp @@ -9,6 +9,7 @@ #include <boost/test/unit_test.hpp> #include <boost/thread/thread.hpp> +#include <functional> #include <mutex> BOOST_AUTO_TEST_SUITE(scheduler_tests) diff --git a/src/test/sync_tests.cpp b/src/test/sync_tests.cpp index 71275f69d9..3e4d1dac9e 100644 --- a/src/test/sync_tests.cpp +++ b/src/test/sync_tests.cpp @@ -6,7 +6,6 @@ #include <test/util/setup_common.h> #include <boost/test/unit_test.hpp> -#include <boost/thread/mutex.hpp> #include <mutex> @@ -110,11 +109,6 @@ BOOST_AUTO_TEST_CASE(double_lock_mutex) TestDoubleLock<Mutex>(true /* should throw */); } -BOOST_AUTO_TEST_CASE(double_lock_boost_mutex) -{ - TestDoubleLock<boost::mutex>(true /* should throw */); -} - BOOST_AUTO_TEST_CASE(double_lock_recursive_mutex) { TestDoubleLock<RecursiveMutex>(false /* should not throw */); diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 5b35ed6976..1eb30c68d6 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -24,6 +24,7 @@ #include <util/strencodings.h> #include <validation.h> +#include <functional> #include <map> #include <string> @@ -428,12 +429,10 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) // check all inputs concurrently, with the cache PrecomputedTransactionData txdata(tx); - boost::thread_group threadGroup; CCheckQueue<CScriptCheck> scriptcheckqueue(128); CCheckQueueControl<CScriptCheck> control(&scriptcheckqueue); - for (int i=0; i<20; i++) - threadGroup.create_thread(std::bind(&CCheckQueue<CScriptCheck>::Thread, std::ref(scriptcheckqueue))); + scriptcheckqueue.StartWorkerThreads(20); std::vector<Coin> coins; for(uint32_t i = 0; i < mtx.vin.size(); i++) { @@ -455,9 +454,7 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) bool controlCheck = control.Wait(); assert(controlCheck); - - threadGroup.interrupt_all(); - threadGroup.join_all(); + scriptcheckqueue.StopWorkerThreads(); } SignatureData CombineSignatures(const CMutableTransaction& input1, const CMutableTransaction& input2, const CTransactionRef tx) diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 738f414cd0..8a9694f55b 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -143,9 +143,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve // Start script-checking threads. Set g_parallel_script_checks to true so they are used. constexpr int script_check_threads = 2; - for (int i = 0; i < script_check_threads; ++i) { - threadGroup.create_thread([i]() { return ThreadScriptCheck(i); }); - } + StartScriptCheckWorkerThreads(script_check_threads); g_parallel_script_checks = true; } @@ -154,6 +152,7 @@ ChainTestingSetup::~ChainTestingSetup() if (m_node.scheduler) m_node.scheduler->stop(); threadGroup.interrupt_all(); threadGroup.join_all(); + StopScriptCheckWorkerThreads(); GetMainSignals().FlushBackgroundCallbacks(); GetMainSignals().UnregisterBackgroundSignalScheduler(); m_node.connman.reset(); diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp index 90ee9422ba..908ad35e1b 100644 --- a/src/torcontrol.cpp +++ b/src/torcontrol.cpp @@ -15,10 +15,11 @@ #include <util/strencodings.h> #include <util/system.h> -#include <vector> #include <deque> +#include <functional> #include <set> #include <stdlib.h> +#include <vector> #include <boost/signals2/signal.hpp> #include <boost/algorithm/string/split.hpp> diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 9ae7b921b3..470e665844 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -618,6 +618,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const if (GetRand(m_check_ratio) >= 1) return; + AssertLockHeld(::cs_main); LOCK(cs); LogPrint(BCLog::MEMPOOL, "Checking mempool with %u transactions and %u inputs\n", (unsigned int)mapTx.size(), (unsigned int)mapNextTx.size()); diff --git a/src/txmempool.h b/src/txmempool.h index 9a1aa9bc2b..0a9cd81ff5 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -602,7 +602,7 @@ public: * all inputs are in the mapNextTx array). If sanity-checking is turned off, * check does nothing. */ - void check(const CCoinsViewCache *pcoins) const; + void check(const CCoinsViewCache *pcoins) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); // addUnchecked must updated state for all ancestors of a given transaction, // to track size/count of descendant transactions. First version of diff --git a/src/util/system.cpp b/src/util/system.cpp index 917d37ccc7..d1fb921642 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -398,7 +398,7 @@ bool ArgsManager::GetSettingsPath(fs::path* filepath, bool temp) const } if (filepath) { std::string settings = GetArg("-settings", BITCOIN_SETTINGS_FILENAME); - *filepath = fs::absolute(temp ? settings + ".tmp" : settings, GetDataDir(/* net_specific= */ true)); + *filepath = fsbridge::AbsPathJoin(GetDataDir(/* net_specific= */ true), temp ? settings + ".tmp" : settings); } return true; } @@ -1311,7 +1311,7 @@ fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific) if (path.is_absolute()) { return path; } - return fs::absolute(path, GetDataDir(net_specific)); + return fsbridge::AbsPathJoin(GetDataDir(net_specific), path); } void ScheduleBatchPriority() diff --git a/src/validation.cpp b/src/validation.cpp index a22c75a686..eeb99f0f0a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1763,9 +1763,14 @@ static bool WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValidationSt static CCheckQueue<CScriptCheck> scriptcheckqueue(128); -void ThreadScriptCheck(int worker_num) { - util::ThreadRename(strprintf("scriptch.%i", worker_num)); - scriptcheckqueue.Thread(); +void StartScriptCheckWorkerThreads(int threads_num) +{ + scriptcheckqueue.StartWorkerThreads(threads_num); +} + +void StopScriptCheckWorkerThreads() +{ + scriptcheckqueue.StopWorkerThreads(); } VersionBitsCache versionbitscache GUARDED_BY(cs_main); diff --git a/src/validation.h b/src/validation.h index 1b4e40cd53..d3fbabe1a2 100644 --- a/src/validation.h +++ b/src/validation.h @@ -149,8 +149,10 @@ void LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi bool LoadGenesisBlock(const CChainParams& chainparams); /** Unload database information */ void UnloadBlockIndex(CTxMemPool* mempool, ChainstateManager& chainman); -/** Run an instance of the script checking thread */ -void ThreadScriptCheck(int worker_num); +/** Run instances of script checking worker threads */ +void StartScriptCheckWorkerThreads(int threads_num); +/** Stop all of the script checking worker threads */ +void StopScriptCheckWorkerThreads(); /** * Return transaction from the block at block_index. * If block_index is not provided, fall back to mempool. diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 6ed48593fb..c0d107bf39 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -723,6 +723,23 @@ bool BerkeleyBatch::TxnAbort() return (ret == 0); } +bool BerkeleyDatabaseSanityCheck() +{ + int major, minor; + DbEnv::version(&major, &minor, nullptr); + + /* If the major version differs, or the minor version of library is *older* + * than the header that was compiled against, flag an error. + */ + if (major != DB_VERSION_MAJOR || minor < DB_VERSION_MINOR) { + LogPrintf("BerkeleyDB database version conflict: header version is %d.%d, library version is %d.%d\n", + DB_VERSION_MAJOR, DB_VERSION_MINOR, major, minor); + return false; + } + + return true; +} + std::string BerkeleyDatabaseVersion() { return DbEnv::version(nullptr, nullptr, nullptr); diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index bf1617d67f..a8209587d7 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -223,6 +223,10 @@ public: std::string BerkeleyDatabaseVersion(); +/** Perform sanity check of runtime BDB version versus linked BDB version. + */ +bool BerkeleyDatabaseSanityCheck(); + //! Return object giving access to Berkeley database at specified path. std::unique_ptr<BerkeleyDatabase> MakeBerkeleyDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 085dde1026..0d2be64dfb 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -86,6 +86,11 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const bool WalletInit::ParameterInteraction() const { +#ifdef USE_BDB + if (!BerkeleyDatabaseSanityCheck()) { + return InitError(Untranslated("A version conflict was detected between the run-time BerkeleyDB library and the one used during compilation.")); + } +#endif if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { for (const std::string& wallet : gArgs.GetArgs("-wallet")) { LogPrintf("%s: parameter interaction: -disablewallet -> ignoring -wallet=%s\n", __func__, wallet); diff --git a/src/wallet/ismine.h b/src/wallet/ismine.h index 5cdd7dff80..38ed7e7770 100644 --- a/src/wallet/ismine.h +++ b/src/wallet/ismine.h @@ -14,7 +14,27 @@ class CWallet; class CScript; -/** IsMine() return codes */ +/** + * IsMine() return codes, which depend on ScriptPubKeyMan implementation. + * Not every ScriptPubKeyMan covers all types, please refer to + * https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.21.0.md#ismine-semantics + * for better understanding. + * + * For LegacyScriptPubKeyMan, + * ISMINE_NO: the scriptPubKey is not in the wallet; + * ISMINE_WATCH_ONLY: the scriptPubKey has been imported into the wallet; + * ISMINE_SPENDABLE: the scriptPubKey corresponds to an address owned by the wallet user (can spend with the private key); + * ISMINE_USED: the scriptPubKey corresponds to a used address owned by the wallet user; + * ISMINE_ALL: all ISMINE flags except for USED; + * ISMINE_ALL_USED: all ISMINE flags including USED; + * ISMINE_ENUM_ELEMENTS: the number of isminetype enum elements. + * + * For DescriptorScriptPubKeyMan and future ScriptPubKeyMan, + * ISMINE_NO: the scriptPubKey is not in the wallet; + * ISMINE_SPENDABLE: the scriptPubKey matches a scriptPubKey in the wallet. + * ISMINE_USED: the scriptPubKey corresponds to a used address owned by the wallet user. + * + */ enum isminetype : unsigned int { ISMINE_NO = 0, diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 036fd4956f..30832f983b 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -62,7 +62,7 @@ bool VerifyWallets(interfaces::Chain& chain) std::set<fs::path> wallet_paths; for (const auto& wallet_file : gArgs.GetArgs("-wallet")) { - const fs::path path = fs::absolute(wallet_file, GetWalletDir()); + const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), wallet_file); if (!wallet_paths.insert(path).second) { chain.initWarning(strprintf(_("Ignoring duplicate -wallet %s."), wallet_file)); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index e3631d5c6b..ec51ba0453 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3460,10 +3460,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) CWallet* const pwallet = wallet.get(); if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !want_psbt) { - if (!pwallet->chain().rpcEnableDeprecated("bumpfee")) { - throw JSONRPCError(RPC_METHOD_DEPRECATED, "Using bumpfee with wallets that have private keys disabled is deprecated. Use psbtbumpfee instead or restart bitcoind with -deprecatedrpc=bumpfee. This functionality will be removed in 0.22"); - } - want_psbt = true; + throw JSONRPCError(RPC_WALLET_ERROR, "bumpfee is not available with wallets that have private keys disabled. Use psbtbumpfee instead."); } RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VOBJ}); @@ -3834,13 +3831,19 @@ RPCHelpMan getaddressinfo() LOCK(pwallet->cs_wallet); - UniValue ret(UniValue::VOBJ); - CTxDestination dest = DecodeDestination(request.params[0].get_str()); + std::string error_msg; + CTxDestination dest = DecodeDestination(request.params[0].get_str(), error_msg); + // Make sure the destination is valid if (!IsValidDestination(dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address"); + // Set generic error message in case 'DecodeDestination' didn't set it + if (error_msg.empty()) error_msg = "Invalid address"; + + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error_msg); } + UniValue ret(UniValue::VOBJ); + std::string currentAddress = EncodeDestination(dest); ret.pushKV("address", currentAddress); diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index aa7f1c83d8..f035a70a20 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <fs.h> +#include <univalue.h> #include <util/check.h> #include <util/system.h> @@ -37,6 +38,9 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainNam InitWalletDirTestingSetup::~InitWalletDirTestingSetup() { + gArgs.LockSettings([&](util::Settings& settings) { + settings.forced_settings.erase("walletdir"); + }); fs::current_path(m_cwd); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 918946f9e7..723552860a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3780,7 +3780,7 @@ std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, cons // 2. Path to an existing directory. // 3. Path to a symlink to a directory. // 4. For backwards compatibility, the name of a data file in -walletdir. - const fs::path& wallet_path = fs::absolute(name, GetWalletDir()); + const fs::path wallet_path = fsbridge::AbsPathJoin(GetWalletDir(), name); fs::file_type path_type = fs::symlink_status(wallet_path).type(); if (!(path_type == fs::file_not_found || path_type == fs::directory_file || (path_type == fs::symlink_file && fs::is_directory(wallet_path)) || diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index a1bb7343f4..bc90491a2c 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -105,7 +105,7 @@ static void WalletShowInfo(CWallet* wallet_instance) bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command, const std::string& name) { - fs::path path = fs::absolute(name, GetWalletDir()); + const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), name); if (args.IsArgSet("-format") && command != "createfromdump") { tfm::format(std::cerr, "The -format option can only be used with the \"createfromdump\" command.\n"); diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index c0207f9dd6..168ba841c8 100644 --- a/src/zmq/zmqpublishnotifier.cpp +++ b/src/zmq/zmqpublishnotifier.cpp @@ -17,6 +17,7 @@ #include <cstdarg> #include <cstddef> #include <map> +#include <optional> #include <string> #include <utility> @@ -227,50 +228,43 @@ bool CZMQPublishRawTransactionNotifier::NotifyTransaction(const CTransaction &tr return SendZmqMessage(MSG_RAWTX, &(*ss.begin()), ss.size()); } +// Helper function to send a 'sequence' topic message with the following structure: +// <32-byte hash> | <1-byte label> | <8-byte LE sequence> (optional) +static bool SendSequenceMsg(CZMQAbstractPublishNotifier& notifier, uint256 hash, char label, std::optional<uint64_t> sequence = {}) +{ + unsigned char data[sizeof(hash) + sizeof(label) + sizeof(uint64_t)]; + for (unsigned int i = 0; i < sizeof(hash); ++i) { + data[sizeof(hash) - 1 - i] = hash.begin()[i]; + } + data[sizeof(hash)] = label; + if (sequence) WriteLE64(data + sizeof(hash) + sizeof(label), *sequence); + return notifier.SendZmqMessage(MSG_SEQUENCE, data, sequence ? sizeof(data) : sizeof(hash) + sizeof(label)); +} -// TODO: Dedup this code to take label char, log string bool CZMQPublishSequenceNotifier::NotifyBlockConnect(const CBlockIndex *pindex) { uint256 hash = pindex->GetBlockHash(); LogPrint(BCLog::ZMQ, "zmq: Publish sequence block connect %s to %s\n", hash.GetHex(), this->address); - char data[sizeof(uint256)+1]; - for (unsigned int i = 0; i < sizeof(uint256); i++) - data[sizeof(uint256) - 1 - i] = hash.begin()[i]; - data[sizeof(data) - 1] = 'C'; // Block (C)onnect - return SendZmqMessage(MSG_SEQUENCE, data, sizeof(data)); + return SendSequenceMsg(*this, hash, /* Block (C)onnect */ 'C'); } bool CZMQPublishSequenceNotifier::NotifyBlockDisconnect(const CBlockIndex *pindex) { uint256 hash = pindex->GetBlockHash(); LogPrint(BCLog::ZMQ, "zmq: Publish sequence block disconnect %s to %s\n", hash.GetHex(), this->address); - char data[sizeof(uint256)+1]; - for (unsigned int i = 0; i < sizeof(uint256); i++) - data[sizeof(uint256) - 1 - i] = hash.begin()[i]; - data[sizeof(data) - 1] = 'D'; // Block (D)isconnect - return SendZmqMessage(MSG_SEQUENCE, data, sizeof(data)); + return SendSequenceMsg(*this, hash, /* Block (D)isconnect */ 'D'); } bool CZMQPublishSequenceNotifier::NotifyTransactionAcceptance(const CTransaction &transaction, uint64_t mempool_sequence) { uint256 hash = transaction.GetHash(); LogPrint(BCLog::ZMQ, "zmq: Publish hashtx mempool acceptance %s to %s\n", hash.GetHex(), this->address); - unsigned char data[sizeof(uint256)+sizeof(mempool_sequence)+1]; - for (unsigned int i = 0; i < sizeof(uint256); i++) - data[sizeof(uint256) - 1 - i] = hash.begin()[i]; - data[sizeof(uint256)] = 'A'; // Mempool (A)cceptance - WriteLE64(data+sizeof(uint256)+1, mempool_sequence); - return SendZmqMessage(MSG_SEQUENCE, data, sizeof(data)); + return SendSequenceMsg(*this, hash, /* Mempool (A)cceptance */ 'A', mempool_sequence); } bool CZMQPublishSequenceNotifier::NotifyTransactionRemoval(const CTransaction &transaction, uint64_t mempool_sequence) { uint256 hash = transaction.GetHash(); LogPrint(BCLog::ZMQ, "zmq: Publish hashtx mempool removal %s to %s\n", hash.GetHex(), this->address); - unsigned char data[sizeof(uint256)+sizeof(mempool_sequence)+1]; - for (unsigned int i = 0; i < sizeof(uint256); i++) - data[sizeof(uint256) - 1 - i] = hash.begin()[i]; - data[sizeof(uint256)] = 'R'; // Mempool (R)emoval - WriteLE64(data+sizeof(uint256)+1, mempool_sequence); - return SendZmqMessage(MSG_SEQUENCE, data, sizeof(data)); + return SendSequenceMsg(*this, hash, /* Mempool (R)emoval */ 'R', mempool_sequence); } |