diff --git a/ci/dash/test_integrationtests.sh b/ci/dash/test_integrationtests.sh index b4b0304b2ad8..a5973a56bad9 100755 --- a/ci/dash/test_integrationtests.sh +++ b/ci/dash/test_integrationtests.sh @@ -44,7 +44,7 @@ EXTRA_ARGS="--dashd-arg=-socketevents=$SOCKETEVENTS" set +e # shellcheck disable=SC2086 -LD_LIBRARY_PATH="$DEPENDS_DIR/$HOST/lib" ./test/functional/test_runner.py --ci --attempts=3 --ansi --combinedlogslen=4000 --timeout-factor="${TEST_RUNNER_TIMEOUT_FACTOR}" ${TEST_RUNNER_EXTRA} --failfast --nocleanup --tmpdir="$(pwd)/testdatadirs" $PASS_ARGS $EXTRA_ARGS +LD_LIBRARY_PATH="$DEPENDS_DIR/$HOST/lib" ./test/functional/test_runner.py --ci --attempts=3 --ansi --combinedlogslen=99999999 --timeout-factor="${TEST_RUNNER_TIMEOUT_FACTOR}" ${TEST_RUNNER_EXTRA} --failfast --nocleanup --tmpdir="$(pwd)/testdatadirs" $PASS_ARGS $EXTRA_ARGS RESULT=$? set -e diff --git a/configure.ac b/configure.ac index 3798c0222614..a4d68750b89f 100644 --- a/configure.ac +++ b/configure.ac @@ -1586,7 +1586,7 @@ if test "$use_natpmp" != "no"; then CPPFLAGS="$TEMP_CPPFLAGS" fi -if test "$build_bitcoin_wallet$build_bitcoin_cli$build_bitcoin_tx$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench$enable_fuzz_binary" = "nononononononono"; then +if test "$build_bitcoin_wallet$build_bitcoin_cli$build_bitcoin_tx$build_bitcoin_util$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench$enable_fuzz_binary" = "nonononononononono"; then use_boost=no else use_boost=yes diff --git a/contrib/debian/copyright b/contrib/debian/copyright index af24c9c258e5..d2d1c4394131 100644 --- a/contrib/debian/copyright +++ b/contrib/debian/copyright @@ -17,14 +17,6 @@ Copyright: 2010-2011, Jonas Smedegaard 2011, Matt Corallo License: GPL-2+ -Files: src/secp256k1/build-aux/m4/ax_jni_include_dir.m4 -Copyright: 2008 Don Anderson -License: GNU-All-permissive-License - -Files: src/secp256k1/build-aux/m4/ax_prog_cc_for_build.m4 -Copyright: 2008 Paolo Bonzini -License: GNU-All-permissive-License - Files: src/qt/res/icons/proxy.png src/qt/res/src/proxy.svg @@ -56,12 +48,6 @@ License: Expat TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. -License: GNU-All-permissive-License - Copying and distribution of this file, with or without modification, are - permitted in any medium without royalty provided the copyright notice - and this notice are preserved. This file is offered as-is, without any - warranty. - License: GPL-2+ This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the diff --git a/doc/build-windows.md b/doc/build-windows.md index 873d8d67af62..ec2fe1dbed07 100644 --- a/doc/build-windows.md +++ b/doc/build-windows.md @@ -6,7 +6,7 @@ Below are some notes on how to build Dash Core for Windows. The options known to work for building Dash Core on Windows are: * On Linux, using the [Mingw-w64](https://www.mingw-w64.org/) cross compiler tool chain. -* On Windows, using [Windows Subsystem for Linux (WSL)](https://docs.microsoft.com/windows/wsl/about) and Mingw-w64. +* On Windows, using [Windows Subsystem for Linux (WSL)](https://learn.microsoft.com/en-us/windows/wsl/about) and Mingw-w64. Other options which may work, but which have not been extensively tested are (please contribute instructions): @@ -18,7 +18,7 @@ package meets the minimum required `g++` version specified in [dependencies.md]( Installing Windows Subsystem for Linux --------------------------------------- -Follow the upstream installation instructions, available [here](https://docs.microsoft.com/windows/wsl/install-win10). +Follow the upstream installation instructions, available [here](https://learn.microsoft.com/en-us/windows/wsl/install). Cross-compilation ------------------- @@ -34,7 +34,7 @@ Note that for WSL the Dash Core source path MUST be somewhere in the default mou example /usr/src/dash, AND not under /mnt/d/. If this is not the case the dependency autoconf scripts will fail. This means you cannot use a directory that is located directly on the host Windows file system to perform the build. -Additional WSL Note: WSL support for [launching Win32 applications](https://docs.microsoft.com/en-us/archive/blogs/wsl/windows-and-ubuntu-interoperability#launching-win32-applications-from-within-wsl) +Additional WSL Note: WSL support for [launching Win32 applications](https://learn.microsoft.com/en-us/archive/blogs/wsl/windows-and-ubuntu-interoperability#launching-win32-applications-from-within-wsl) results in `Autoconf` configure scripts being able to execute Windows Portable Executable files. This can cause unexpected behaviour during the build, such as Win32 error dialogs for missing libraries. The recommended approach is to temporarily disable WSL support for Win32 applications. diff --git a/doc/fuzzing.md b/doc/fuzzing.md index c8a121341c5b..2bd7461889f3 100644 --- a/doc/fuzzing.md +++ b/doc/fuzzing.md @@ -279,8 +279,8 @@ $ sudo apt-get install libtool libtool-bin wget automake autoconf bison gdb ``` At this point, you must install the .NET core. The process differs, depending on your Linux distribution. -See [this link](https://docs.microsoft.com/en-us/dotnet/core/install/linux) for details. -On ubuntu 20.04, the following should work: +See [this link](https://learn.microsoft.com/en-us/dotnet/core/install/linux) for details. +On Ubuntu 20.04, the following should work: ```sh $ wget -q https://packages.microsoft.com/config/ubuntu/20.04/packages-microsoft-prod.deb diff --git a/doc/managing-wallets.md b/doc/managing-wallets.md index 72ae306eb376..7096b6286249 100644 --- a/doc/managing-wallets.md +++ b/doc/managing-wallets.md @@ -90,7 +90,7 @@ In the RPC, the destination parameter must include the name of the file. Otherwi $ dash-cli -rpcwallet="wallet-01" backupwallet /home/node01/Backups/backup-01.dat ``` -In the GUI, the wallet is selected in the `Wallet` drop-down list in the upper right corner. If this list is not present, the wallet can be loaded in `File` ->`Open wallet` if necessary. Then, the backup can be done in `File` -> `Backup Wallet...`. +In the GUI, the wallet is selected in the `Wallet` drop-down list in the upper right corner. If this list is not present, the wallet can be loaded in `File` ->`Open Wallet` if necessary. Then, the backup can be done in `File` -> `Backup Wallet…`. This backup file can be stored on one or multiple offline devices, which must be reliable enough to work in an emergency and be malware free. Backup files can be regularly tested to avoid problems in the future. @@ -110,7 +110,7 @@ Non-HD wallets must be backed up every 1000 keys used since the previous backup, ### 1.6 Restoring the Wallet From a Backup -To restore a wallet, the `restorewallet` RPC must be used. +To restore a wallet, the `restorewallet` RPC or the `Restore Wallet` GUI menu item (`File` -> `Restore Wallet…`) must be used. ``` $ dash-cli restorewallet "restored-wallet" /home/node01/Backups/backup-01.dat @@ -122,4 +122,4 @@ After that, `getwalletinfo` can be used to check if the wallet has been fully re $ dash-cli -rpcwallet="restored-wallet" getwalletinfo ``` -The restored wallet can also be loaded in the GUI via `File` ->`Open wallet`. \ No newline at end of file +The restored wallet can also be loaded in the GUI via `File` ->`Open wallet`. diff --git a/src/.clang-tidy b/src/.clang-tidy index 76978140ecd7..9e518d1fbf26 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -7,6 +7,7 @@ modernize-use-default-member-init, modernize-use-nullptr, performance-for-range-copy, performance-move-const-arg, +performance-no-automatic-move, performance-unnecessary-copy-initialization, readability-const-return-type, readability-redundant-declaration, @@ -19,6 +20,7 @@ misc-unused-using-decls, modernize-use-default-member-init, modernize-use-nullptr, performance-move-const-arg, +performance-no-automatic-move, performance-unnecessary-copy-initialization, readability-redundant-declaration, readability-redundant-string-init, diff --git a/src/addrman.cpp b/src/addrman.cpp index 7959ec2b74ca..e353d457264e 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -1195,7 +1195,7 @@ std::pair AddrManImpl::SelectTriedCollision() { LOCK(cs); Check(); - const auto ret = SelectTriedCollision_(); + auto ret = SelectTriedCollision_(); Check(); return ret; } @@ -1204,7 +1204,7 @@ std::pair AddrManImpl::Select(bool new_only, std::optiona { LOCK(cs); Check(); - const auto addrRet = Select_(new_only, network); + auto addrRet = Select_(new_only, network); Check(); return addrRet; } @@ -1213,7 +1213,7 @@ std::vector AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, { LOCK(cs); Check(); - const auto addresses = GetAddr_(max_addresses, max_pct, network); + auto addresses = GetAddr_(max_addresses, max_pct, network); Check(); return addresses; } diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index c8018e7b9aa7..fe6bbb6a9ecb 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -852,7 +852,7 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co UniValue valReply(UniValue::VSTR); if (!valReply.read(response.body)) throw std::runtime_error("couldn't parse reply from server"); - const UniValue reply = rh->ProcessReply(valReply); + UniValue reply = rh->ProcessReply(valReply); if (reply.empty()) throw std::runtime_error("expected reply to have result, error and id properties"); diff --git a/src/bitcoin-util.cpp b/src/bitcoin-util.cpp index 41982a128457..15ed5f3a558f 100644 --- a/src/bitcoin-util.cpp +++ b/src/bitcoin-util.cpp @@ -77,13 +77,12 @@ static int AppInitUtil(ArgsManager& args, int argc, char* argv[]) return CONTINUE_EXECUTION; } -static void grind_task(uint32_t nBits, CBlockHeader& header_orig, uint32_t offset, uint32_t step, std::atomic& found) +static void grind_task(uint32_t nBits, CBlockHeader header, uint32_t offset, uint32_t step, std::atomic& found, uint32_t& proposed_nonce) { arith_uint256 target; bool neg, over; target.SetCompact(nBits, &neg, &over); if (target == 0 || neg || over) return; - CBlockHeader header = header_orig; // working copy header.nNonce = offset; uint32_t finish = std::numeric_limits::max() - step; @@ -94,7 +93,7 @@ static void grind_task(uint32_t nBits, CBlockHeader& header_orig, uint32_t offse do { if (UintToArith256(header.GetHash()) <= target) { if (!found.exchange(true)) { - header_orig.nNonce = header.nNonce; + proposed_nonce = header.nNonce; } return; } @@ -118,16 +117,19 @@ static int Grind(const std::vector& args, std::string& strPrint) uint32_t nBits = header.nBits; std::atomic found{false}; + uint32_t proposed_nonce{}; std::vector threads; int n_tasks = std::max(1u, std::thread::hardware_concurrency()); for (int i = 0; i < n_tasks; ++i) { - threads.emplace_back( grind_task, nBits, std::ref(header), i, n_tasks, std::ref(found) ); + threads.emplace_back(grind_task, nBits, header, i, n_tasks, std::ref(found), std::ref(proposed_nonce)); } for (auto& t : threads) { t.join(); } - if (!found) { + if (found) { + header.nNonce = proposed_nonce; + } else { strPrint = "Could not satisfy difficulty target"; return EXIT_FAILURE; } diff --git a/src/instantsend/instantsend.cpp b/src/instantsend/instantsend.cpp index 495eefd68119..62a1278413b1 100644 --- a/src/instantsend/instantsend.cpp +++ b/src/instantsend/instantsend.cpp @@ -144,7 +144,7 @@ std::variant CInstantSendManager::Proc } uint256 hashBlock{}; - const auto tx = GetTransaction(nullptr, &mempool, islock->txid, Params().GetConsensus(), hashBlock); + auto tx = GetTransaction(nullptr, &mempool, islock->txid, Params().GetConsensus(), hashBlock); const bool found_transaction{tx != nullptr}; // we ignore failure here as we must be able to propagate the lock even if we don't have the TX locally std::optional minedHeight = GetBlockHeight(hashBlock); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 19ebf9d6f3bf..34f89047d2ad 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -261,8 +261,8 @@ class Chain { public: virtual ~Notifications() {} - virtual void transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime, uint64_t mempool_sequence) {} - virtual void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) {} + virtual void transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) {} + virtual void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {} virtual void blockConnected(const CBlock& block, int height) {} virtual void blockDisconnected(const CBlock& block, int height) {} virtual void updatedBlockTip() {} diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 851848a60d5d..748922c8433c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -773,9 +773,9 @@ class PeerManagerImpl final : public PeerManager bool MaybeSendGetHeaders(CNode& pfrom, const std::string& msg_type, const CBlockLocator& locator, Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); /** Potentially fetch blocks from this peer upon receipt of a new headers tip */ - void HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex* pindexLast); + void HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex& last_header); /** Update peer state based on received headers message */ - void UpdatePeerStateForReceivedHeaders(CNode& pfrom, const CBlockIndex *pindexLast, bool received_new_header, bool may_have_more_headers); + void UpdatePeerStateForReceivedHeaders(CNode& pfrom, const CBlockIndex& last_header, bool received_new_header, bool may_have_more_headers); void SendBlockTransactions(CNode& pfrom, const CBlock& block, const BlockTransactionsRequest& req) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); @@ -3136,22 +3136,21 @@ bool PeerManagerImpl::MaybeSendGetHeaders(CNode& pfrom, const std::string& msg_t } /* - * Given a new headers tip ending in pindexLast, potentially request blocks towards that tip. + * Given a new headers tip ending in last_header, potentially request blocks towards that tip. * We require that the given tip have at least as much work as our tip, and for * our current tip to be "close to synced" (see CanDirectFetch()). */ -void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex* pindexLast) +void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex& last_header) { const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); LOCK(cs_main); CNodeState *nodestate = State(pfrom.GetId()); - if (CanDirectFetch() && pindexLast->IsValid(BLOCK_VALID_TREE) && m_chainman.ActiveChain().Tip()->nChainWork <= pindexLast->nChainWork) { - + if (CanDirectFetch() && last_header.IsValid(BLOCK_VALID_TREE) && m_chainman.ActiveChain().Tip()->nChainWork <= last_header.nChainWork) { std::vector vToFetch; - const CBlockIndex *pindexWalk = pindexLast; - // Calculate all the blocks we'd need to switch to pindexLast, up to a limit. + const CBlockIndex* pindexWalk{&last_header}; + // Calculate all the blocks we'd need to switch to last_header, up to a limit. while (pindexWalk && !m_chainman.ActiveChain().Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) && !IsBlockRequested(pindexWalk->GetBlockHash())) { @@ -3166,8 +3165,8 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c // direct fetch and rely on parallel download instead. if (!m_chainman.ActiveChain().Contains(pindexWalk)) { LogPrint(BCLog::NET, "Large reorg, won't direct fetch to %s (%d)\n", - pindexLast->GetBlockHash().ToString(), - pindexLast->nHeight); + last_header.GetBlockHash().ToString(), + last_header.nHeight); } else { std::vector vGetData; // Download as much as possible, from earliest to latest. @@ -3183,14 +3182,15 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c } if (vGetData.size() > 1) { LogPrint(BCLog::NET, "Downloading blocks toward %s (%d) via headers direct fetch\n", - pindexLast->GetBlockHash().ToString(), pindexLast->nHeight); + last_header.GetBlockHash().ToString(), + last_header.nHeight); } if (vGetData.size() > 0) { if (!m_ignore_incoming_txs && nodestate->m_provides_cmpctblocks && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && - pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) { + last_header.pprev->IsValid(BLOCK_VALID_CHAIN)) { // In any case, we want to download using a compact block, not a regular one vGetData[0] = CInv(MSG_CMPCT_BLOCK, vGetData[0].hash); } @@ -3201,12 +3201,12 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c } /** - * Given receipt of headers from a peer ending in pindexLast, along with + * Given receipt of headers from a peer ending in last_header, along with * whether that header was new and whether the headers message was full, * update the state we keep for the peer. */ void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, - const CBlockIndex *pindexLast, bool received_new_header, bool may_have_more_headers) + const CBlockIndex& last_header, bool received_new_header, bool may_have_more_headers) { LOCK(cs_main); CNodeState *nodestate = State(pfrom.GetId()); @@ -3215,14 +3215,13 @@ void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, } nodestate->nUnconnectingHeaders = 0; - assert(pindexLast); - UpdateBlockAvailability(pfrom.GetId(), pindexLast->GetBlockHash()); + UpdateBlockAvailability(pfrom.GetId(), last_header.GetBlockHash()); // From here, pindexBestKnownBlock should be guaranteed to be non-null, // because it is set in UpdateBlockAvailability. Some nullptr checks // are still present, however, as belt-and-suspenders. - if (received_new_header && pindexLast->nChainWork > m_chainman.ActiveChain().Tip()->nChainWork) { + if (received_new_header && last_header.nChainWork > m_chainman.ActiveChain().Tip()->nChainWork) { nodestate->m_last_block_announcement = GetTime(); } @@ -3319,10 +3318,10 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, } } - UpdatePeerStateForReceivedHeaders(pfrom, pindexLast, received_new_header, nCount == GetHeadersLimit(pfrom, uses_compressed)); + UpdatePeerStateForReceivedHeaders(pfrom, *pindexLast, received_new_header, nCount == GetHeadersLimit(pfrom, uses_compressed)); // Consider immediately downloading blocks. - HeadersDirectFetchBlocks(pfrom, peer, pindexLast); + HeadersDirectFetchBlocks(pfrom, peer, *pindexLast); return; } diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index d3e6283eb5a9..7fd65de80a9f 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -909,11 +909,11 @@ class NotificationsProxy : public CValidationInterface virtual ~NotificationsProxy() = default; void TransactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime, uint64_t mempool_sequence) override { - m_notifications->transactionAddedToMempool(tx, nAcceptTime, mempool_sequence); + m_notifications->transactionAddedToMempool(tx, nAcceptTime); } void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override { - m_notifications->transactionRemovedFromMempool(tx, reason, mempool_sequence); + m_notifications->transactionRemovedFromMempool(tx, reason); } void BlockConnected(const std::shared_ptr& block, const CBlockIndex* index) override { @@ -1303,7 +1303,7 @@ class ChainImpl : public Chain if (!m_node.mempool) return; LOCK2(::cs_main, m_node.mempool->cs); for (const CTxMemPoolEntry& entry : m_node.mempool->mapTx) { - notifications.transactionAddedToMempool(entry.GetSharedTx(), /* nAcceptTime = */ 0, /* mempool_sequence = */ 0); + notifications.transactionAddedToMempool(entry.GetSharedTx(), /*nAcceptTime=*/0); } } bool hasAssumedValidChain() override diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 9aae2d7e0c96..af14af07e2b3 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -533,7 +533,7 @@ UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) const if (request.mode == JSONRPCRequest::GET_HELP || !IsValidNumArgs(request.params.size())) { throw std::runtime_error(ToString()); } - const UniValue ret = m_fun(*this, request); + UniValue ret = m_fun(*this, request); if (gArgs.GetBoolArg("-rpcdoccheck", DEFAULT_RPC_DOC_CHECK)) { CHECK_NONFATAL(std::any_of(m_results.m_results.begin(), m_results.m_results.end(), [&ret](const RPCResult& res) { return res.MatchesType(ret); })); } diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 86395c08c313..aa8185cc5205 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -201,7 +201,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool) for (int i = 0; i < num_out; ++i) { tx_mut.vout.emplace_back(amount_out, CScript() << OP_RETURN); } - const auto tx = MakeTransactionRef(tx_mut); + auto tx = MakeTransactionRef(tx_mut); // Restore previously removed outpoints for (const auto& in : tx->vin) { Assert(outpoints_rbf.insert(in.prevout).second); diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index f4d12c41fce4..17709d9f72f4 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -73,7 +73,7 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage) for (auto& in : tx_mut.vin) { outpoints.push_back(in.prevout); } - const auto new_tx = MakeTransactionRef(tx_mut); + auto new_tx = MakeTransactionRef(tx_mut); // add newly constructed transaction to outpoints for (uint32_t i = 0; i < num_out; i++) { outpoints.emplace_back(new_tx->GetHash(), i); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index cd999067b278..f1915033efd6 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -496,7 +496,7 @@ CBlock TestChainSetup::CreateAndProcessBlock( chainstate = &Assert(m_node.chainman)->ActiveChainstate(); } - auto block = this->CreateBlock(txns, scriptPubKey, *chainstate); + CBlock block = this->CreateBlock(txns, scriptPubKey, *chainstate); std::shared_ptr shared_pblock = std::make_shared(block); Assert(m_node.chainman)->ProcessNewBlock(shared_pblock, true, nullptr); diff --git a/src/validation.cpp b/src/validation.cpp index 95629b462cf1..a3bc8ef0fea5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1400,7 +1400,7 @@ MempoolAcceptResult AcceptToMemoryPool(CChainState& active_chainstate, const CTr std::vector coins_to_uncache; auto args = MemPoolAccept::ATMPArgs::SingleAccept(chainparams, accept_time, bypass_limits, coins_to_uncache, test_accept); - const MempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptSingleTransaction(tx, args); + MempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptSingleTransaction(tx, args); if (result.m_result_type != MempoolAcceptResult::ResultType::VALID || test_accept) { if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) { LogPrint(BCLog::MEMPOOL, "%s: %s %s (%s)\n", __func__, tx->GetHash().ToString(), result.m_state.GetRejectReason(), result.m_state.GetDebugMessage()); @@ -1429,7 +1429,7 @@ PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTx std::vector coins_to_uncache; const CChainParams& chainparams = active_chainstate.m_params; - const auto result = [&]() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { + auto result = [&]() EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); if (test_accept) { auto args = MemPoolAccept::ATMPArgs::PackageTestAccept(chainparams, GetTime(), coins_to_uncache); diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index 2ab5fb449b42..6d91c1b76c06 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -75,7 +75,7 @@ std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& reques std::string wallet_name; if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) { - const std::shared_ptr pwallet = GetWallet(context, wallet_name); + std::shared_ptr pwallet = GetWallet(context, wallet_name); if (!pwallet) throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded"); return pwallet; } diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 7fc43fa5ef2a..635bd0c61686 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -534,6 +534,14 @@ static RPCHelpMan loadwallet() bilingual_str error; std::vector warnings; std::optional load_on_start = request.params[1].isNull() ? std::nullopt : std::optional(request.params[1].get_bool()); + + { + LOCK(context.wallets_mutex); + if (std::any_of(context.wallets.begin(), context.wallets.end(), [&name](const auto& wallet) { return wallet->GetName() == name; })) { + throw JSONRPCError(RPC_WALLET_ALREADY_LOADED, "Wallet \"" + name + "\" is already loaded."); + } + } + std::shared_ptr const wallet = LoadWallet(context, name, load_on_start, options, status, error, warnings); HandleWalletError(wallet, status, error); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 2d567548412f..098c53024889 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -953,7 +953,7 @@ BOOST_AUTO_TEST_CASE(minimum_inputs_test) /*tx_noinputs_size=*/0, /*avoid_partial=*/false, }; - const auto result = SelectCoins(*wallet, available_coins, target, coin_control, coin_selection_params); + auto result = SelectCoins(*wallet, available_coins, target, coin_control, coin_selection_params); BOOST_REQUIRE(result); // Should consume only the first two coins (9 + 16) >= 25 and account correctly diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index cf398e4df3fd..4e3885b845e2 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -1556,7 +1556,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestChain100Setup) mtx.vin.clear(); mtx.vin.push_back(CTxIn(tx_id_to_spend, 0)); - wallet.transactionAddedToMempool(MakeTransactionRef(mtx), 0, 0); + wallet.transactionAddedToMempool(MakeTransactionRef(mtx), 0); const uint256& good_tx_id = mtx.GetHash(); { @@ -1577,7 +1577,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestChain100Setup) static_cast(wallet.GetDatabase()).m_pass = false; mtx.vin.clear(); mtx.vin.push_back(CTxIn(good_tx_id, 0)); - BOOST_CHECK_EXCEPTION(wallet.transactionAddedToMempool(MakeTransactionRef(mtx), 0, 0), + BOOST_CHECK_EXCEPTION(wallet.transactionAddedToMempool(MakeTransactionRef(mtx), 0), std::runtime_error, HasReason("DB error adding transaction to wallet, write failed")); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3132c36766d6..a50371e459f3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -259,7 +259,7 @@ std::shared_ptr LoadWalletInternal(WalletContext& context, const std::s } context.chain->initMessage(_("Loading wallet…").translated); - const std::shared_ptr wallet = CWallet::Create(context, name, std::move(database), options.create_flags, error, warnings); + std::shared_ptr wallet = CWallet::Create(context, name, std::move(database), options.create_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet loading failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_LOAD; @@ -343,7 +343,7 @@ std::shared_ptr CreateWallet(WalletContext& context, const std::string& // Make the wallet context.chain->initMessage(_("Loading wallet…").translated); - const std::shared_ptr wallet = CWallet::Create(context, name, std::move(database), wallet_creation_flags, error, warnings); + std::shared_ptr wallet = CWallet::Create(context, name, std::move(database), wallet_creation_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet creation failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_CREATE; @@ -1329,7 +1329,7 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, const SyncTxState& sta fAnonymizableTallyCachedNonDenom = false; } -void CWallet::transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime, uint64_t mempool_sequence) { +void CWallet::transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) { LOCK(cs_wallet); WalletBatch batch(GetDatabase()); SyncTransaction(tx, TxStateInMempool{}, batch); @@ -1340,7 +1340,7 @@ void CWallet::transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcce } } -void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) { +void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) { if (reason != MemPoolRemovalReason::CONFLICT) { LOCK(cs_wallet); auto it = mapWallet.find(tx->GetHash()); @@ -1391,7 +1391,7 @@ void CWallet::blockConnected(const CBlock& block, int height) WalletBatch batch(GetDatabase()); for (size_t index = 0; index < block.vtx.size(); index++) { SyncTransaction(block.vtx[index], TxStateConfirmed{block_hash, height, static_cast(index)}, batch); - transactionRemovedFromMempool(block.vtx[index], MemPoolRemovalReason::BLOCK, 0 /* mempool_sequence */); + transactionRemovedFromMempool(block.vtx[index], MemPoolRemovalReason::BLOCK); } // reset cache to make sure no longer immature coins are included @@ -2927,7 +2927,7 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri const auto start{SteadyClock::now()}; // TODO: Can't use std::make_shared because we need a custom deleter but // should be possible to use std::allocate_shared. - const std::shared_ptr walletInstance(new CWallet(chain, coinjoin_loader, name, args, std::move(database)), ReleaseWallet); + std::shared_ptr walletInstance(new CWallet(chain, coinjoin_loader, name, args, std::move(database)), ReleaseWallet); // TODO: refactor this condition: validation of error looks like workaround if (!walletInstance->AutoBackupWallet(fs::PathFromString(walletFile), error, warnings) && !error.original.empty()) { return nullptr; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0ac939954c42..cc53f9028265 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -643,7 +643,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati */ CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true, bool rescanning_old_block = false); bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - void transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime, uint64_t mempool_sequence) override; + void transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) override; void blockConnected(const CBlock& block, int height) override; void blockDisconnected(const CBlock& block, int height) override; void updatedBlockTip() override; @@ -665,7 +665,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati uint256 last_failed_block; }; ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional max_height, const WalletRescanReserver& reserver, bool fUpdate, const bool save_progress); - void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override; + void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) override; void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void ResendWalletTransactions(); diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 541751768d63..bc01efbe5278 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -299,16 +299,12 @@ def wallet_file(name): assert_raises_rpc_error(-18, "Wallet file verification failed. Failed to load database path '{}'. Path does not exist.".format(path), self.nodes[0].loadwallet, 'wallets') # Fail to load duplicate wallets - path = os.path.join(self.options.tmpdir, "node0", self.chain, "wallets", "w1", self.wallet_data_filename) - if self.options.descriptors: - assert_raises_rpc_error(-4, f"Wallet file verification failed. SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another instance of {self.config['environment']['PACKAGE_NAME']}?", self.nodes[0].loadwallet, wallet_names[0]) - else: - assert_raises_rpc_error(-35, "Wallet file verification failed. Refusing to load database. Data file '{}' is already loaded.".format(path), self.nodes[0].loadwallet, wallet_names[0]) - - # Fail to load duplicate wallets by different ways (directory and filepath) + assert_raises_rpc_error(-35, "Wallet \"w1\" is already loaded.", self.nodes[0].loadwallet, wallet_names[0]) if not self.options.descriptors: - path = os.path.join(self.options.tmpdir, "node0", self.chain, "wallets", self.wallet_data_filename) - assert_raises_rpc_error(-35, "Wallet file verification failed. Refusing to load database. Data file '{}' is already loaded.".format(path), self.nodes[0].loadwallet, self.wallet_data_filename) + # This tests the default wallet that BDB makes, so SQLite wallet doesn't need to test this + # Fail to load duplicate wallets by different ways (directory and filepath) + path = os.path.join(self.options.tmpdir, "node0", self.chain, "wallets", "wallet.dat") + assert_raises_rpc_error(-35, "Wallet file verification failed. Refusing to load database. Data file '{}' is already loaded.".format(path), self.nodes[0].loadwallet, 'wallet.dat') # Only BDB doesn't open duplicate wallet files. SQLite does not have this limitation. While this may be desired in the future, it is not necessary # Fail to load if one wallet is a copy of another @@ -320,6 +316,11 @@ def wallet_file(name): # Fail to load if wallet file is a symlink assert_raises_rpc_error(-4, "Wallet file verification failed. Invalid -wallet path 'w8_symlink'", self.nodes[0].loadwallet, 'w8_symlink') + # Fail to load if a directory is specified that doesn't contain a wallet + os.mkdir(wallet_dir('empty_wallet_dir')) + path = os.path.join(self.options.tmpdir, "node0", self.chain, "wallets", "empty_wallet_dir") + assert_raises_rpc_error(-18, "Wallet file verification failed. Failed to load database path '{}'. Data is not in recognized format.".format(path), self.nodes[0].loadwallet, 'empty_wallet_dir') + self.log.info("Test dynamic wallet creation.") # Fail to create a wallet if it already exists. @@ -345,11 +346,6 @@ def wallet_file(name): assert new_wallet_name in self.nodes[0].listwallets() - # Fail to load if a directory is specified that doesn't contain a wallet - os.mkdir(wallet_dir('empty_wallet_dir')) - path = os.path.join(self.options.tmpdir, "node0", self.chain, "wallets", "empty_wallet_dir") - assert_raises_rpc_error(-18, "Wallet file verification failed. Failed to load database path '{}'. Data is not in recognized format.".format(path), self.nodes[0].loadwallet, 'empty_wallet_dir') - self.log.info("Test dynamic wallet unloading") # Test `unloadwallet` errors