diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e30687d774d5..25f3f506afef 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -512,9 +512,9 @@ jobs: uses: actions/cache/restore@v5 with: path: /cache/ctcache - key: ctcache-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'depends/packages/*') }}-linux64_multiprocess-${{ github.sha }} + key: ctcache-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'contrib/devtools/bitcoin-tidy/*', 'depends/packages/*') }}-linux64_multiprocess-${{ github.sha }} restore-keys: | - ctcache-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'depends/packages/*') }}-linux64_multiprocess- + ctcache-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'contrib/devtools/bitcoin-tidy/*', 'depends/packages/*') }}-linux64_multiprocess- - name: Run linters run: | @@ -530,7 +530,7 @@ jobs: uses: actions/cache/save@v5 with: path: /cache/ctcache - key: ctcache-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'depends/packages/*') }}-linux64_multiprocess-${{ github.sha }} + key: ctcache-${{ hashFiles('contrib/containers/ci/ci.Dockerfile', 'contrib/devtools/bitcoin-tidy/*', 'depends/packages/*') }}-linux64_multiprocess-${{ github.sha }} test-linux64: name: linux64-test diff --git a/ci/dash/lint-tidy.sh b/ci/dash/lint-tidy.sh index be2639e1d5ab..36df742b3ac4 100755 --- a/ci/dash/lint-tidy.sh +++ b/ci/dash/lint-tidy.sh @@ -43,6 +43,8 @@ fi # Setup ctcache for clang-tidy result caching export CTCACHE_SAVE_OUTPUT=1 +# The cache key is the preprocessed source: keep comments so that a NOLINT edit invalidates it +export CTCACHE_KEEP_COMMENTS=1 export CTCACHE_CLANG_TIDY="${CLANG_TIDY_BIN}" mkdir -p "${CTCACHE_DIR}" @@ -64,6 +66,11 @@ fi # Zero stats before run to get accurate statistics for this run only python3 "${CLANG_TIDY_CACHE_PY}" --zero-stats 2>&1 || true +TIDY_BUILD_DIR="${BASE_ROOT_DIR}/build-ci/tidy-build" +cmake -B "${TIDY_BUILD_DIR}" -DLLVM_DIR="$(llvm-config --cmakedir)" -DCMAKE_BUILD_TYPE=Release -S "${BASE_ROOT_DIR}/contrib/devtools/bitcoin-tidy" +cmake --build "${TIDY_BUILD_DIR}" "${MAKEJOBS}" +cmake --build "${TIDY_BUILD_DIR}" --target bitcoin-tidy-tests "${MAKEJOBS}" + cd "${BASE_ROOT_DIR}/build-ci/dashcore-${BUILD_TARGET}/src" # Filter out files by regex here, because regex may not be @@ -84,6 +91,7 @@ if ! ( run-clang-tidy \ -checks=clang-diagnostic-old-style-cast,google-readability-casting \ -warnings-as-errors=-clang-diagnostic-old-style-cast,-google-readability-casting \ -clang-tidy-binary="${CLANG_TIDY_CACHE}" \ + -load="${TIDY_BUILD_DIR}/libbitcoin-tidy.so" \ -p "${CAST_LINT_DB}" \ -quiet "${MAKEJOBS}" | \ python3 "${BASE_ROOT_DIR}/ci/dash/lint-cstyle-casts.py" filter --source-root "${BASE_ROOT_DIR}" | \ diff --git a/contrib/devtools/bitcoin-tidy/CMakeLists.txt b/contrib/devtools/bitcoin-tidy/CMakeLists.txt new file mode 100644 index 000000000000..35e60d1d87e2 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/CMakeLists.txt @@ -0,0 +1,56 @@ +cmake_minimum_required(VERSION 3.9) + +project(bitcoin-tidy VERSION 1.0.0 DESCRIPTION "clang-tidy checks for Bitcoin Core") + +include(GNUInstallDirs) + +set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED True) +set(CMAKE_CXX_EXTENSIONS False) + +# TODO: Figure out how to avoid the terminfo check +find_package(LLVM REQUIRED CONFIG) +find_program(CLANG_TIDY_EXE NAMES "clang-tidy-${LLVM_VERSION_MAJOR}" "clang-tidy" HINTS ${LLVM_TOOLS_BINARY_DIR}) +message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}") +message(STATUS "Found clang-tidy: ${CLANG_TIDY_EXE}") + +add_library(bitcoin-tidy MODULE bitcoin-tidy.cpp logprintf.cpp) +target_include_directories(bitcoin-tidy SYSTEM PRIVATE ${LLVM_INCLUDE_DIRS}) + +# Disable RTTI and exceptions as necessary +if (MSVC) + target_compile_options(bitcoin-tidy PRIVATE /GR-) +else() + target_compile_options(bitcoin-tidy PRIVATE -fno-rtti) + target_compile_options(bitcoin-tidy PRIVATE -fno-exceptions) +endif() + +if(CMAKE_HOST_APPLE) + # ld64 expects no undefined symbols by default + target_link_options(bitcoin-tidy PRIVATE -Wl,-flat_namespace) + target_link_options(bitcoin-tidy PRIVATE -Wl,-undefined -Wl,suppress) +endif() + +# Add warnings +if (MSVC) + target_compile_options(bitcoin-tidy PRIVATE /W4) +else() + target_compile_options(bitcoin-tidy PRIVATE -Wall) + target_compile_options(bitcoin-tidy PRIVATE -Wextra) +endif() + +if(CMAKE_VERSION VERSION_LESS 3.27) + set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "--load=${CMAKE_BINARY_DIR}/${CMAKE_SHARED_MODULE_PREFIX}bitcoin-tidy${CMAKE_SHARED_MODULE_SUFFIX}" "-checks=-*,bitcoin-*") +else() + # CLANG_TIDY_COMMAND supports generator expressions as of 3.27 + set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "--load=$" "-checks=-*,bitcoin-*") +endif() + +# Create a dummy library that runs clang-tidy tests as a side-effect of building +add_library(bitcoin-tidy-tests OBJECT EXCLUDE_FROM_ALL example_logprintf.cpp) +add_dependencies(bitcoin-tidy-tests bitcoin-tidy) + +set_target_properties(bitcoin-tidy-tests PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}") + + +install(TARGETS bitcoin-tidy LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/contrib/devtools/bitcoin-tidy/README b/contrib/devtools/bitcoin-tidy/README new file mode 100644 index 000000000000..c15e07c4ede5 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/README @@ -0,0 +1,11 @@ +# Bitcoin Tidy + +Example Usage: + +```bash +cmake -S . -B build -DLLVM_DIR=$(llvm-config --cmakedir) -DCMAKE_BUILD_TYPE=Release + +cmake --build build -j$(nproc) + +cmake --build build --target bitcoin-tidy-tests -j$(nproc) +``` diff --git a/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp b/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp new file mode 100644 index 000000000000..0f34d3779387 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp @@ -0,0 +1,22 @@ +// Copyright (c) 2023 Bitcoin Developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "logprintf.h" + +#include +#include + +class BitcoinModule final : public clang::tidy::ClangTidyModule +{ +public: + void addCheckFactories(clang::tidy::ClangTidyCheckFactories& CheckFactories) override + { + CheckFactories.registerCheck("bitcoin-unterminated-logprintf"); + } +}; + +static clang::tidy::ClangTidyModuleRegistry::Add + X("bitcoin-module", "Adds bitcoin checks."); + +volatile int BitcoinModuleAnchorSource = 0; diff --git a/contrib/devtools/bitcoin-tidy/example_logprintf.cpp b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp new file mode 100644 index 000000000000..a12a666c086d --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp @@ -0,0 +1,108 @@ +// Copyright (c) 2023 Bitcoin Developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +// Test for bitcoin-unterminated-logprintf + +enum LogFlags { + NONE +}; + +enum Level { + None +}; + +template +static inline void LogPrintf_(const std::string& logging_function, const std::string& source_file, const int source_line, const LogFlags flag, const Level level, const char* fmt, const Args&... args) +{ +} + +#define LogPrintLevel_(category, level, ...) LogPrintf_(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__) +#define LogPrintf(...) LogPrintLevel_(LogFlags::NONE, Level::None, __VA_ARGS__) + +#define LogPrint(category, ...) \ + do { \ + LogPrintf(__VA_ARGS__); \ + } while (0) + + +class CWallet +{ + std::string GetDisplayName() const + { + return "default wallet"; + } + +public: + template + void WalletLogPrintf(const char* fmt, Params... parameters) const + { + LogPrintf(("%s " + std::string{fmt}).c_str(), GetDisplayName(), parameters...); + }; +}; + +struct ScriptPubKeyMan +{ + std::string GetDisplayName() const + { + return "default wallet"; + } + + template + void WalletLogPrintf(const char* fmt, Params... parameters) const + { + LogPrintf(("%s " + std::string{fmt}).c_str(), GetDisplayName(), parameters...); + }; +}; + +void good_func() +{ + LogPrintf("hello world!\n"); +} +void good_func2() +{ + CWallet wallet; + wallet.WalletLogPrintf("hi\n"); + ScriptPubKeyMan spkm; + spkm.WalletLogPrintf("hi\n"); + + const CWallet& walletref = wallet; + walletref.WalletLogPrintf("hi\n"); + + auto* walletptr = new CWallet(); + walletptr->WalletLogPrintf("hi\n"); + delete walletptr; +} +void bad_func() +{ + LogPrintf("hello world!"); +} +void bad_func2() +{ + LogPrintf(""); +} +void bad_func3() +{ + // Ending in "..." has no special meaning. + LogPrintf("hello world!..."); +} +void bad_func4_ignored() +{ + LogPrintf("hello world!"); // NOLINT(bitcoin-unterminated-logprintf) +} +void bad_func5() +{ + CWallet wallet; + wallet.WalletLogPrintf("hi"); + ScriptPubKeyMan spkm; + spkm.WalletLogPrintf("hi"); + + const CWallet& walletref = wallet; + walletref.WalletLogPrintf("hi"); + + auto* walletptr = new CWallet(); + walletptr->WalletLogPrintf("hi"); + delete walletptr; +} diff --git a/contrib/devtools/bitcoin-tidy/logprintf.cpp b/contrib/devtools/bitcoin-tidy/logprintf.cpp new file mode 100644 index 000000000000..36beac28c86c --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/logprintf.cpp @@ -0,0 +1,60 @@ +// Copyright (c) 2023 Bitcoin Developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "logprintf.h" + +#include +#include + + +namespace { +AST_MATCHER(clang::StringLiteral, unterminated) +{ + size_t len = Node.getLength(); + if (len > 0 && Node.getCodeUnit(len - 1) == '\n') { + return false; + } + return true; +} +} // namespace + +namespace bitcoin { + +void LogPrintfCheck::registerMatchers(clang::ast_matchers::MatchFinder* finder) +{ + using namespace clang::ast_matchers; + + /* + Logprintf(..., ..., ..., ..., ..., "foo", ...) + */ + + finder->addMatcher( + callExpr( + callee(functionDecl(hasName("LogPrintf_"))), + hasArgument(5, stringLiteral(unterminated()).bind("logstring"))), + this); + + /* + auto walletptr = &wallet; + wallet.WalletLogPrintf("foo"); + wallet->WalletLogPrintf("foo"); + */ + finder->addMatcher( + cxxMemberCallExpr( + callee(cxxMethodDecl(hasName("WalletLogPrintf"))), + hasArgument(0, stringLiteral(unterminated()).bind("logstring"))), + this); +} + +void LogPrintfCheck::check(const clang::ast_matchers::MatchFinder::MatchResult& Result) +{ + if (const clang::StringLiteral* lit = Result.Nodes.getNodeAs("logstring")) { + const clang::ASTContext& ctx = *Result.Context; + const auto user_diag = diag(lit->getEndLoc(), "Unterminated format string used with LogPrintf"); + const auto& loc = lit->getLocationOfByte(lit->getByteLength(), *Result.SourceManager, ctx.getLangOpts(), ctx.getTargetInfo()); + user_diag << clang::FixItHint::CreateInsertion(loc, "\\n"); + } +} + +} // namespace bitcoin diff --git a/contrib/devtools/bitcoin-tidy/logprintf.h b/contrib/devtools/bitcoin-tidy/logprintf.h new file mode 100644 index 000000000000..db95dfe143e8 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/logprintf.h @@ -0,0 +1,29 @@ +// Copyright (c) 2023 Bitcoin Developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef LOGPRINTF_CHECK_H +#define LOGPRINTF_CHECK_H + +#include + +namespace bitcoin { + +// Warn about any use of LogPrintf that does not end with a newline. +class LogPrintfCheck final : public clang::tidy::ClangTidyCheck +{ +public: + LogPrintfCheck(clang::StringRef Name, clang::tidy::ClangTidyContext* Context) + : clang::tidy::ClangTidyCheck(Name, Context) {} + + bool isLanguageVersionSupported(const clang::LangOptions& LangOpts) const override + { + return LangOpts.CPlusPlus; + } + void registerMatchers(clang::ast_matchers::MatchFinder* Finder) override; + void check(const clang::ast_matchers::MatchFinder::MatchResult& Result) override; +}; + +} // namespace bitcoin + +#endif // LOGPRINTF_CHECK_H diff --git a/src/.clang-tidy b/src/.clang-tidy index d127a4a60f52..ce43637d0490 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -1,5 +1,6 @@ Checks: ' -*, +bitcoin-*, bugprone-argument-comment, bugprone-use-after-move, bugprone-lambda-function-name, diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index 46ecab50a1b9..e1c3c8ef896e 100644 --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -13,11 +13,12 @@ static void AddTx(const CTransactionRef& tx, const CAmount& nFee, CTxMemPool& po { int64_t nTime = 0; unsigned int nHeight = 1; + uint64_t sequence = 0; bool spendsCoinbase = false; unsigned int sigOps = 1; LockPoints lp; pool.addUnchecked(CTxMemPoolEntry( - tx, nFee, nTime, nHeight, + tx, nFee, nTime, nHeight, sequence, spendsCoinbase, sigOps, lp)); } diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index 827d36e60463..b073c61b5882 100644 --- a/src/bench/mempool_stress.cpp +++ b/src/bench/mempool_stress.cpp @@ -16,10 +16,11 @@ static void AddTx(const CTransactionRef& tx, CTxMemPool& pool) EXCLUSIVE_LOCKS_R { int64_t nTime = 0; unsigned int nHeight = 1; + uint64_t sequence = 0; bool spendsCoinbase = false; unsigned int sigOpCost = 4; LockPoints lp; - pool.addUnchecked(CTxMemPoolEntry(tx, 1000, nTime, nHeight, spendsCoinbase, sigOpCost, lp)); + pool.addUnchecked(CTxMemPoolEntry(tx, 1000, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); } struct Available { diff --git a/src/bench/rpc_mempool.cpp b/src/bench/rpc_mempool.cpp index 37fd8e203e1e..84f6e32ecbb3 100644 --- a/src/bench/rpc_mempool.cpp +++ b/src/bench/rpc_mempool.cpp @@ -16,7 +16,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) { LockPoints lp; - pool.addUnchecked(CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_height=*/1, /*spends_coinbase=*/false, /*sigops_count=*/1, lp)); + pool.addUnchecked(CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_count=*/1, lp)); } static void RpcMempool(benchmark::Bench& bench) diff --git a/src/chainlock/signing.cpp b/src/chainlock/signing.cpp index 121b164c760e..ba1802ad452b 100644 --- a/src/chainlock/signing.cpp +++ b/src/chainlock/signing.cpp @@ -149,7 +149,7 @@ void ChainLockSigner::TrySignChainTip() for (const auto& txid : *txids) { if (!m_clhandler.IsTxSafeForMining(txid) && !m_isman.IsLocked(txid)) { - LogPrint(BCLog::CHAINLOCKS, /* Continued */ + LogPrint(BCLog::CHAINLOCKS, "%s -- not signing block %s due to TX %s not being islocked and not old enough.\n", __func__, pindexWalk->GetBlockHash().ToString(), txid.ToString()); return; diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp index 3ba3b6bdbeba..58e0d0dbfec6 100644 --- a/src/coinjoin/client.cpp +++ b/src/coinjoin/client.cpp @@ -118,7 +118,7 @@ void CCoinJoinClientSession::ProcessMessage(CNode& peer, Chainstate& active_chai return; } - WalletCJLogPrint(m_wallet, "DSFINALTX -- txNew %s", txNew.ToString()); /* Continued */ + WalletCJLogPrint(m_wallet, "DSFINALTX -- txNew %s", txNew.ToString()); // NOLINT(bitcoin-unterminated-logprintf) // check to see if input is spent already? (and probably not confirmed) SignFinalTransaction(peer, active_chainstate, connman, mempool, txNew); @@ -479,7 +479,7 @@ bool CCoinJoinClientSession::SignFinalTransaction(CNode& peer, Chainstate& activ LOCK(cs_coinjoin); finalMutableTransaction = CMutableTransaction{finalTransactionNew}; - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- finalMutableTransaction=%s", __func__, finalMutableTransaction.ToString()); /* Continued */ + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- finalMutableTransaction=%s", __func__, finalMutableTransaction.ToString()); // NOLINT(bitcoin-unterminated-logprintf) // STEP 1: check final transaction general rules @@ -622,7 +622,7 @@ bool CCoinJoinClientSession::SignFinalTransaction(CNode& peer, Chainstate& activ } // push all of our signatures to the Masternode - WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- pushing signed inputs to the masternode, finalMutableTransaction=%s", __func__, finalMutableTransaction.ToString()); /* Continued */ + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- pushing signed inputs to the masternode, finalMutableTransaction=%s", __func__, finalMutableTransaction.ToString()); // NOLINT(bitcoin-unterminated-logprintf) CNetMsgMaker msgMaker(peer.GetCommonVersion()); connman.PushMessage(&peer, msgMaker.Make(NetMsgType::DSSIGNFINALTX, signed_inputs)); SetState(POOL_STATE_SIGNING); @@ -735,7 +735,7 @@ void CCoinJoinClientManager::AddPendingObservation(const std::vector& // would make them selectable again while a valid mixing transaction spending them // may already be in flight. Nothing more we can do about it here beyond making // the failure loud - the wallet database is broken. - LogPrintf("CCoinJoinClientManager::%s -- ERROR: failed to persist locks for %d successfully mixed input(s), " /* Continued */ + LogPrintf("CCoinJoinClientManager::%s -- ERROR: failed to persist locks for %d successfully mixed input(s), " "they will not survive a restart until this succeeds\n", __func__, outpoints.size()); } @@ -769,7 +769,7 @@ void CCoinJoinClientManager::LoadPendingObservations(wallet::WalletBatch& batch) // never becomes readable, and the retry runs for as long as this node does. if (!m_pending_obs_load_failed) { m_pending_obs_load_failed = true; - LogPrintf("CCoinJoinClientManager::%s -- ERROR: failed to read the pending observation record, will keep " /* Continued */ + LogPrintf("CCoinJoinClientManager::%s -- ERROR: failed to read the pending observation record, will keep " "retrying silently. The inputs it tracks stay locked until it can be read, `lockunspent` " "releases them manually\n", __func__); @@ -847,7 +847,7 @@ void CCoinJoinClientManager::CheckPendingObservations(const CTxMemPool& mempool) // Still unspent in chain and mempool long after the session completed - // the finalized transaction most likely never propagated, release the // input so the wallet does not lose it forever - LogPrintf("CCoinJoinClientManager::%s -- WARNING: never observed finalized mixing transaction for %s, " /* Continued */ + LogPrintf("CCoinJoinClientManager::%s -- WARNING: never observed finalized mixing transaction for %s, " "releasing lock after %d seconds\n", __func__, outpoint.ToStringShort(), COINJOIN_PENDING_OBSERVATION_TIMEOUT); m_wallet->UnlockCoin(outpoint, &get_batch()); @@ -863,7 +863,7 @@ void CCoinJoinClientManager::CheckPendingObservations(const CTxMemPool& mempool) // selected for another session - so only refresh the timer to keep the check // above from running on every pass. The refresh is deliberately not persisted, // the worst a restart can do is re-run the check once. - WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::%s -- %s is spent in chain/mempool but not according to " /* Continued */ + WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::%s -- %s is spent in chain/mempool but not according to " "the wallet, keeping it locked\n", __func__, outpoint.ToStringShort()); it->second = nNow; @@ -878,7 +878,7 @@ void CCoinJoinClientManager::CheckPendingObservations(const CTxMemPool& mempool) const bool fPersisted{PersistPendingObservations(*m_wallet, get_batch(), m_pending_obs)}; // Retried on every pass until it succeeds, only complain when it starts failing if (!fPersisted && !m_pending_obs_dirty) { - LogPrintf("CCoinJoinClientManager::%s -- ERROR: failed to persist %d pending observation(s), " /* Continued */ + LogPrintf("CCoinJoinClientManager::%s -- ERROR: failed to persist %d pending observation(s), " "will keep retrying\n", __func__, m_pending_obs.size()); } @@ -1437,7 +1437,7 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized, m_mn_metaman.AddUsedMasternode(dmn->proTxHash); if (connman.IsMasternodeOrDisconnectRequested(dmn->pdmnState->netInfo->GetPrimary())) { - WalletCJLogPrint(m_wallet, /* Continued */ + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::JoinExistingQueue -- skipping connection, masternode=%s\n", dmn->proTxHash.ToString()); continue; } @@ -1512,7 +1512,7 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon } if (m_mn_metaman.IsMixingThresholdExceeded(dmn->proTxHash, nMnCount)) { - WalletCJLogPrint(m_wallet, /* Continued */ + WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::StartNewQueue -- too early to mix with node masternode=%s\n", dmn->proTxHash.ToString()); nTries++; @@ -1543,7 +1543,7 @@ bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CCon pendingDsaRequest = CPendingDsaRequest(dmn->proTxHash, CCoinJoinAccept(nSessionDenom, txMyCollateral)); SetState(POOL_STATE_QUEUE); nTimeLastSuccessfulStep = GetTime(); - WalletCJLogPrint(/* Continued */ + WalletCJLogPrint( m_wallet, "CCoinJoinClientSession::StartNewQueue -- pending connection, masternode=%s, nSessionDenom=%d " "(%s)\n", diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 5fe5de73a1ab..04b7f70da612 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -469,7 +469,7 @@ bool CoinJoin::IsCollateralValid(ChainstateManager& chainman, const llmq::CInsta nValueOut += txout.nValue; if (!txout.scriptPubKey.IsPayToPublicKeyHash() && !txout.scriptPubKey.IsUnspendable()) { - LogPrint(BCLog::COINJOIN, "CoinJoin::IsCollateralValid -- Invalid Script, txCollateral=%s", txCollateral.ToString()); /* Continued */ + LogPrint(BCLog::COINJOIN, "CoinJoin::IsCollateralValid -- Invalid Script, txCollateral=%s", txCollateral.ToString()); // NOLINT(bitcoin-unterminated-logprintf) return false; } } @@ -489,11 +489,11 @@ bool CoinJoin::IsCollateralValid(ChainstateManager& chainman, const llmq::CInsta //collateral transactions are required to pay out a small fee to the miners if (nValueIn - nValueOut < GetCollateralAmount()) { - LogPrint(BCLog::COINJOIN, "CoinJoin::IsCollateralValid -- did not include enough fees in transaction: fees: %d, txCollateral=%s", nValueOut - nValueIn, txCollateral.ToString()); /* Continued */ + LogPrint(BCLog::COINJOIN, "CoinJoin::IsCollateralValid -- did not include enough fees in transaction: fees: %d, txCollateral=%s", nValueOut - nValueIn, txCollateral.ToString()); // NOLINT(bitcoin-unterminated-logprintf) return false; } - LogPrint(BCLog::COINJOIN, "CoinJoin::IsCollateralValid -- %s", txCollateral.ToString()); /* Continued */ + LogPrint(BCLog::COINJOIN, "CoinJoin::IsCollateralValid -- %s", txCollateral.ToString()); // NOLINT(bitcoin-unterminated-logprintf) if (!ATMPIfSaneFee(chainman, MakeTransactionRef(txCollateral), /*test_accept=*/true)) { LogPrint(BCLog::COINJOIN, "CoinJoin::IsCollateralValid -- didn't pass ATMPIfSaneFee()\n"); diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index b9cbb197e414..8ce61f1dea16 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -77,7 +77,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv) CCoinJoinAccept dsa; vRecv >> dsa; - LogPrint(BCLog::COINJOIN, "DSACCEPT -- nDenom %d (%s) txCollateral %s", dsa.nDenom, CoinJoin::DenominationToString(dsa.nDenom), dsa.txCollateral.ToString()); /* Continued */ + LogPrint(BCLog::COINJOIN, "DSACCEPT -- nDenom %d (%s) txCollateral %s", dsa.nDenom, CoinJoin::DenominationToString(dsa.nDenom), dsa.txCollateral.ToString()); // NOLINT(bitcoin-unterminated-logprintf) auto mnList = m_dmnman.GetListAtChainTip(); auto dmn = mnList.GetValidMNByCollateral(m_mn_activeman.GetOutPoint()); @@ -209,7 +209,7 @@ void CCoinJoinServer::ProcessDSVIN(CNode& peer, CDataStream& vRecv) CCoinJoinEntry entry; vRecv >> entry; - LogPrint(BCLog::COINJOIN, "DSVIN -- txCollateral %s", entry.txCollateral->ToString()); /* Continued */ + LogPrint(BCLog::COINJOIN, "DSVIN -- txCollateral %s", entry.txCollateral->ToString()); // NOLINT(bitcoin-unterminated-logprintf) // Note: unbalanced (promotion/demotion) entries are only valid post-V24; AddEntry -> // IsValidInOuts rejects them pre-V24 and consumes the collateral to keep spam costly @@ -437,7 +437,7 @@ void CCoinJoinServer::CreateFinalTransaction(int session_id, bool charge_fees) sort(txNew.vout.begin(), txNew.vout.end(), CompareOutputBIP69()); finalMutableTransaction = txNew; - LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateFinalTransaction -- finalMutableTransaction=%s", /* Continued */ + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateFinalTransaction -- finalMutableTransaction=%s", // NOLINT(bitcoin-unterminated-logprintf) txNew.ToString()); // request signatures from clients @@ -472,7 +472,7 @@ void CCoinJoinServer::CommitFinalTransaction(int session_id) } uint256 hashTx = finalTransaction->GetHash(); - LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- finalTransaction=%s", /* Continued */ + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- finalTransaction=%s", // NOLINT(bitcoin-unterminated-logprintf) finalTransaction->ToString()); { @@ -480,7 +480,7 @@ void CCoinJoinServer::CommitFinalTransaction(int session_id) TRY_LOCK(::cs_main, lockMain); mempool.PrioritiseTransaction(hashTx, 0.1 * COIN); if (!lockMain || !ATMPIfSaneFee(m_chainman, finalTransaction)) { - LogPrint(BCLog::COINJOIN, /* Continued */ + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- ATMPIfSaneFee() error: Transaction not valid\n"); // not much we can do in this case, just notify clients RelayCompletedTransaction(session_id, participants, ERR_INVALID_TX); @@ -552,7 +552,7 @@ CTransactionRef CCoinJoinServer::SelectCollateralToCharge() // This queue entry didn't send us the promised transaction if (!fFound) { - LogPrint(BCLog::COINJOIN, /* Continued */ + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::SelectCollateralToCharge -- found uncooperative node (didn't send " "transaction), found offence\n"); vecOffendersCollaterals.push_back(txCollateral); @@ -563,7 +563,7 @@ CTransactionRef CCoinJoinServer::SelectCollateralToCharge() for (const auto& entry : vecEntries) { for (const auto& txdsin : entry.vecTxDSIn) { if (!txdsin.fHasSig) { - LogPrint(BCLog::COINJOIN, /* Continued */ + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::SelectCollateralToCharge -- found uncooperative node (didn't sign), " "found offence\n"); vecOffendersCollaterals.push_back(entry.txCollateral); @@ -584,9 +584,9 @@ CTransactionRef CCoinJoinServer::SelectCollateralToCharge() //charge one of the offenders randomly Shuffle(vecOffendersCollaterals.begin(), vecOffendersCollaterals.end(), FastRandomContext()); - LogPrint(BCLog::COINJOIN, /* Continued */ + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::SelectCollateralToCharge -- found uncooperative node (didn't %s transaction), charging " - "fees: %s", + "fees: %s", // NOLINT(bitcoin-unterminated-logprintf) (state == POOL_STATE_SIGNING) ? "sign" : "send", vecOffendersCollaterals[0]->ToString()); m_collaterals_being_charged.push_back(vecOffendersCollaterals[0]); return vecOffendersCollaterals[0]; @@ -610,8 +610,8 @@ void CCoinJoinServer::ChargeRandomFees(const std::vector& colla for (const auto& txCollateral : collaterals) { if (GetRand(/*nMax=*/100) > 10) return; - LogPrint(BCLog::COINJOIN, /* Continued */ - "CCoinJoinServer::ChargeRandomFees -- charging random fees, txCollateral=%s", txCollateral->ToString()); + LogPrint(BCLog::COINJOIN, + "CCoinJoinServer::ChargeRandomFees -- charging random fees, txCollateral=%s", txCollateral->ToString()); // NOLINT(bitcoin-unterminated-logprintf) ConsumeCollateral(txCollateral); } } @@ -894,7 +894,7 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag // PROMOTION_RATIO outputs; pre-V24 entries are capped at COINJOIN_ENTRY_MAX_SIZE (9) const size_t nMaxEntrySize = fRebalanceSession ? static_cast(CoinJoin::PROMOTION_RATIO) : COINJOIN_ENTRY_MAX_SIZE; if (entry.vecTxDSIn.size() > nMaxEntrySize || entry.vecTxOut.size() > nMaxEntrySize) { - LogPrint(BCLog::COINJOIN, /* Continued */ + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: too many inputs or outputs! inputs=%s/%s, outputs=%s/%s\n", __func__, entry.vecTxDSIn.size(), nMaxEntrySize, entry.vecTxOut.size(), nMaxEntrySize); nMessageIDRet = ERR_MAXIMUM; @@ -1147,7 +1147,7 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, int nPeerVers m_queueman.AddQueue(std::move(dsq)); } - LogPrint(BCLog::COINJOIN, /* Continued */ + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- new session created, nSessionID: %d nSessionDenom: %d (%s) " "participants: %d CoinJoin::GetMaxPoolParticipants(): %d\n", nSessionID, nSessionDenom, CoinJoin::DenominationToString(nSessionDenom), nParticipants, @@ -1246,7 +1246,7 @@ bool CCoinJoinServer::AddUserToExistingSession(const CCoinJoinAccept& dsa, int n // A resent or replayed dsa must not be counted as a new participant; see SessionCollaterals. if (const auto prevout = m_session_collaterals.FindCommittedPrevout(dsa.txCollateral)) { - LogPrint(BCLog::COINJOIN, /* Continued */ + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddUserToExistingSession -- collateral %s spends prevout %s already committed to " "this session\n", dsa.txCollateral.GetHash().ToString(), prevout->ToStringShort()); @@ -1269,7 +1269,7 @@ bool CCoinJoinServer::AddUserToExistingSession(const CCoinJoinAccept& dsa, int n m_mapDeclaredShapes.emplace(dsa.txCollateral.GetHash(), DeclaredShape(dsa)); m_session_collaterals.Add(dsa.txCollateral); - LogPrint(BCLog::COINJOIN, /* Continued */ + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddUserToExistingSession -- new user accepted, nSessionID: %d nSessionDenom: %d (%s) " "participants: %d CoinJoin::GetMaxPoolParticipants(): %d\n", nSessionID, nSessionDenom, CoinJoin::DenominationToString(nSessionDenom), m_session_collaterals.size(), @@ -1397,7 +1397,7 @@ void CCoinJoinServer::ResetSigningSessionIfCurrent(int session_id) AssertLockNotHeld(cs_coinjoin); LOCK(cs_coinjoin); if (nSessionID != session_id || nState != POOL_STATE_SIGNING) { - LogPrint(BCLog::COINJOIN, /* Continued */ + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- signing session %d is no longer current, not resetting\n", __func__, session_id); return; } diff --git a/src/coinjoin/walletman.cpp b/src/coinjoin/walletman.cpp index dc774c18bf87..1717929e6551 100644 --- a/src/coinjoin/walletman.cpp +++ b/src/coinjoin/walletman.cpp @@ -280,7 +280,7 @@ MessageProcessingResult CJWalletManagerImpl::ProcessDSQueue(NodeId from, CConnma if (m_queueman->HasQueueFromMasternode(dsq.masternodeOutpoint, dsq.fReady)) { // no way the same mn can send another dsq with the same readiness this soon - LogPrint(BCLog::COINJOIN, /* Continued */ + LogPrint(BCLog::COINJOIN, "DSQUEUE -- Peer %d is sending WAY too many dsq messages for a masternode with collateral %s\n", from, dsq.masternodeOutpoint.ToStringShort()); return ret; diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index ecc406699b32..8d07afbc56f2 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -102,7 +102,7 @@ class CBitcoinLevelDBLogger : public leveldb::Logger { assert(p <= limit); base[std::min(bufsize - 1, (int)(p - base))] = '\0'; - LogPrintLevel(BCLog::LEVELDB, BCLog::Level::Debug, "%s", base); /* Continued */ + LogPrintLevel(BCLog::LEVELDB, BCLog::Level::Debug, "%s", base); // NOLINT(bitcoin-unterminated-logprintf) if (base != buffer) { delete[] base; } diff --git a/src/dsnotificationinterface.cpp b/src/dsnotificationinterface.cpp index 43c9345e172a..2db2dc7e91e6 100644 --- a/src/dsnotificationinterface.cpp +++ b/src/dsnotificationinterface.cpp @@ -89,7 +89,10 @@ void CDSNotificationInterface::BlockDisconnected(const std::shared_ptr 0 || blockData.credit_pool > 0) { - LogPrint(BCLog::CREDITPOOL, /* Continued */ + LogPrint(BCLog::CREDITPOOL, "CCreditPoolManager: asset unlock limits on height: %d locked: %d.%08d limit: %d.%08d " "unlocked-in-window: %d.%08d locked-at-window-start: %d.%08d\n", block_index->nHeight, blockData.credit_pool / COIN, blockData.credit_pool % COIN, currentLimit / COIN, diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index eb2f68cb730a..c792004828e4 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -430,7 +430,7 @@ bool CSpecialTxProcessor::ApplyTxToList(const CTransaction& tx, gsl::not_nullproTxHash); if (debugLogs) { - LogPrintf("%s -- MN %s removed from list because collateral was used for " /* Continued */ + LogPrintf("%s -- MN %s removed from list because collateral was used for " "a new ProRegTx. collateralOutpoint=%s, nHeight=%d, mapCurMNs.allMNsCount=%d\n", __func__, replacedDmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort(), nHeight, newList.GetCounts().total()); @@ -794,7 +794,7 @@ bool CSpecialTxProcessor::FinishListForBlock(const CBlock& block, gsl::not_null< newList.RemoveMN(dmn->proTxHash); if (debugLogs) { - LogPrintf("%s -- MN %s removed from list because collateral was spent. " /* Continued */ + LogPrintf("%s -- MN %s removed from list because collateral was spent. " "collateralOutpoint=%s, nHeight=%d, mapCurMNs.allMNsCount=%d\n", __func__, dmn->proTxHash.ToString(), dmn->collateralOutpoint.ToStringShort(), nHeight, newList.GetCounts().total()); diff --git a/src/flat-database.h b/src/flat-database.h index f0fb0209ab72..427696543c04 100644 --- a/src/flat-database.h +++ b/src/flat-database.h @@ -164,7 +164,7 @@ class CFlatDB if (readResult == ReadResult::FileError) LogPrintf("Missing file %s, will try to recreate\n", strFilename); else if (readResult != ReadResult::Ok) { - LogPrintf("ERROR: CFlatDB::Read Error reading %s: ", strFilename); + LogPrintf("ERROR: CFlatDB::Read Error reading %s: ", strFilename); // NOLINT(bitcoin-unterminated-logprintf) if (readResult == ReadResult::IncorrectFormat) { LogPrintf("%s: Magic is ok but data has invalid format, will try to recreate\n", __func__); } else { diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index c9559ec039c3..419818fcec6c 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -261,7 +261,7 @@ bool CGovernanceManager::ProcessObject(const std::string& peer_str, const uint25 bool unused_rcb; if (fRateCheckBypassed && fIsValid && !MasternodeRateCheck(govobj, true, true, unused_rcb)) { - LogPrint(BCLog::GOBJECT, /* Continued */ + LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- masternode rate check failed (after signature verification) - %s - (current " "block height %d)\n", strHash, nCachedBlockHeight.load()); diff --git a/src/governance/net_governance.cpp b/src/governance/net_governance.cpp index 4fd4d898468a..520078e1c7e6 100644 --- a/src/governance/net_governance.cpp +++ b/src/governance/net_governance.cpp @@ -233,7 +233,7 @@ void NetGovernance::ProcessMessage(CNode& peer, const std::string& msg_type, CDa ::cs_main, return m_peer_manager->PeerConsumeObjectRequest(peer.GetId(), CInv{MSG_GOVERNANCE_OBJECT_VOTE, nHash})); if (!announced_or_requested) { - LogPrint(BCLog::GOBJECT, /* Continued */ + LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- Received unrequested vote object: %s, hash: %s, peer = %d\n", vote.ToString(tip_mn_list), strHash, peer.GetId()); return; diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 6e7f161d55f2..716326cecdad 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -513,7 +513,7 @@ std::set CGovernanceObject::RemoveInvalidVotes(const CDeterministicMNLi for (const auto& h : removedVotes) { removedStr += strprintf(" %s\n", h.ToString()); } - LogPrintf("CGovernanceObject::%s -- Removed %d invalid votes for %s from MN %s:\n%s", __func__, removedVotes.size(), nParentHash.ToString(), mnOutpoint.ToString(), removedStr); /* Continued */ + LogPrintf("CGovernanceObject::%s -- Removed %d invalid votes for %s from MN %s:\n%s", __func__, removedVotes.size(), nParentHash.ToString(), mnOutpoint.ToString(), removedStr); // NOLINT(bitcoin-unterminated-logprintf) fDirtyCache = true; return removedVotes; diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index edb492c5b23b..b5c43f4a970f 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -202,7 +202,7 @@ void GovernanceSigner::VoteGovernanceTriggers(const std::optionalGetDataAsPlainString()); if (nBlockHeight != entry.sb->GetBlockHeight()) { - LogPrint(BCLog::GOBJECT, /* Continued */ + LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- block height doesn't match nBlockHeight = %d, blockStart = %d, " "continuing\n", nBlockHeight, entry.sb->GetBlockHeight()); diff --git a/src/gsl/assert.cpp b/src/gsl/assert.cpp index df98f8a1aa70..e1a6186cc436 100644 --- a/src/gsl/assert.cpp +++ b/src/gsl/assert.cpp @@ -33,7 +33,7 @@ namespace details << loc.column() << ":" << loc.function_name() << std::endl; std::cerr << s.str() << std::flush; - LogPrintf("%s", s.str()); /* Continued */ + LogPrintf("%s", s.str()); // NOLINT(bitcoin-unterminated-logprintf) std::terminate(); #endif // defined(GSL_MSVC_USE_STL_NOEXCEPTION_WORKAROUND) } diff --git a/src/index/base.cpp b/src/index/base.cpp index 7c9b9056b2ce..f0edc24fb7b8 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -395,7 +395,7 @@ void BaseIndex::BlockConnected(const std::shared_ptr& block, const // in the ValidationInterface queue backlog even after the sync thread has caught up to the // new chain tip. In this unlikely event, log a warning and let the queue clear. if (best_block_index->GetAncestor(pindex->nHeight - 1) != pindex->pprev) { - LogPrintf("%s: WARNING: Block %s does not connect to an ancestor of " /* Continued */ + LogPrintf("%s: WARNING: Block %s does not connect to an ancestor of " "known best chain (tip=%s); not updating index\n", __func__, pindex->GetBlockHash().ToString(), best_block_index->GetBlockHash().ToString()); @@ -434,7 +434,7 @@ void BaseIndex::BlockDisconnected(const std::shared_ptr& block, co // a parent — otherwise same-height siblings would incorrectly trigger a rewind. if (best_block_index && best_block_index->nHeight >= pindex->nHeight && pindex->pprev) { if (best_block_index->GetAncestor(pindex->nHeight) != pindex) { - LogPrintf("%s: WARNING: Block %s is not on the indexed chain " /* Continued */ + LogPrintf("%s: WARNING: Block %s is not on the indexed chain " "(tip=%s); not updating index\n", __func__, pindex->GetBlockHash().ToString(), best_block_index->GetBlockHash().ToString()); return; @@ -471,7 +471,7 @@ void BaseIndex::ChainStateFlushed(const CBlockLocator& locator) // event, log a warning and let the queue clear. const CBlockIndex* best_block_index = m_best_block_index.load(); if (best_block_index->GetAncestor(locator_tip_index->nHeight) != locator_tip_index) { - LogPrintf("%s: WARNING: Locator contains block (hash=%s) not on known best " /* Continued */ + LogPrintf("%s: WARNING: Locator contains block (hash=%s) not on known best " "chain (tip=%s); not writing index locator\n", __func__, locator_tip_hash.ToString(), best_block_index->GetBlockHash().ToString()); diff --git a/src/init.cpp b/src/init.cpp index 4c4892f7054b..1d4e541d2c70 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1530,7 +1530,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // Warn about relative -datadir path. if (args.IsArgSet("-datadir") && !args.GetPathArg("-datadir").is_absolute()) { - LogPrintf("Warning: relative datadir option '%s' specified, which will be interpreted relative to the " /* Continued */ + LogPrintf("Warning: relative datadir option '%s' specified, which will be interpreted relative to the " "current working directory '%s'. This is fragile, because if Dash Core is started in the future " "from a different location, it will be unable to locate the current data files. There could " "also be data loss if Dash Core is started while in a temporary directory.\n", @@ -1648,7 +1648,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) fListen = args.GetBoolArg("-listen", DEFAULT_LISTEN); fDiscover = args.GetBoolArg("-discover", true); - const bool ignores_incoming_txs{args.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY)}; + + PeerManager::Options peerman_opts{}; + ApplyArgsManOptions(args, peerman_opts); { @@ -1713,7 +1715,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.fee_estimator); // Don't initialize fee estimation with old data if we don't relay transactions, // as they would never get updated. - if (!ignores_incoming_txs) { + if (!peerman_opts.ignore_incoming_txs) { bool read_stale_estimates = args.GetBoolArg("-acceptstalefeeestimates", DEFAULT_ACCEPT_STALE_FEE_ESTIMATES); if (read_stale_estimates && (chainparams.GetChainType() != ChainType::REGTEST)) { return InitError(strprintf(_("acceptstalefeeestimates is not supported on %s chain."), chainparams.GetChainTypeString())); @@ -2233,15 +2235,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) if (!node.active_ctx) { // Only constructed in wallet-enabled builds; stays null otherwise, must check before use node.cj_walletman = CJWalletManager::make(chainman, *node.dmnman, *node.mn_metaman, *node.mempool, *node.mn_sync, - *node.isman, !ignores_incoming_txs); + *node.isman, !peerman_opts.ignore_incoming_txs); } #endif - PeerManager::Options peerman_opts{ - .ignore_incoming_txs = ignores_incoming_txs, - }; - ApplyArgsManOptions(args, peerman_opts); - assert(!node.peerman); node.peerman = PeerManager::make(*node.connman, *node.addrman, node.banman.get(), *node.dstxman, chainman, *node.mempool, *node.mn_metaman, *node.mn_sync, @@ -2594,7 +2591,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) if (!result.repair_errors.empty()) { // Critical errors occurred - reindex required - LogPrintf("Failed to repair masternode list diffs. Database corruption detected. " /* Continued */ + LogPrintf("Failed to repair masternode list diffs. Database corruption detected. " "Please restart with -reindex to rebuild the database.\n" "Errors:\n%s\n", Join(result.repair_errors, "\n")); diff --git a/src/instantsend/net_instantsend.cpp b/src/instantsend/net_instantsend.cpp index edbe314aadab..2c0dcef517a0 100644 --- a/src/instantsend/net_instantsend.cpp +++ b/src/instantsend/net_instantsend.cpp @@ -213,7 +213,7 @@ Uint256HashSet NetInstantSend::ApplyVerificationResults( if (it != data.recSigs.end()) { auto recSig = std::make_shared(std::move(it->second)); if (!m_sigman.HasRecoveredSigForId(llmq_params.type, recSig->getId())) { - LogPrint(BCLog::INSTANTSEND, /* Continued */ + LogPrint(BCLog::INSTANTSEND, "NetInstantSend::%s -- txid=%s, islock=%s: " "passing reconstructed recSig to signing mgr, peer=%d\n", __func__, islock->txid.ToString(), hash.ToString(), nodeId); @@ -388,7 +388,7 @@ void NetInstantSend::ProcessInstantSendLock(NodeId from, const uint256& hash, co // Let's see if the TX that was locked by this islock is already mined in a ChainLocked block. If yes, // we can simply ignore the islock, as the ChainLock implies locking of all TXs in that chain if (minedHeight.has_value() && m_chainlocks.HasChainLock(*minedHeight, hashBlock)) { - LogPrint(BCLog::INSTANTSEND, /* Continued */ + LogPrint(BCLog::INSTANTSEND, "NetInstantSend::%s -- txlock=%s, islock=%s: dropping islock as it already got a " "ChainLock in block %s, peer=%d\n", __func__, islock->txid.ToString(), hash.ToString(), hashBlock.ToString(), from); diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index a44d9de9f7b4..9123b8d5dbbb 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -83,6 +83,7 @@ class CTxMemPoolEntry const int32_t nTxSize; //!< ... and avoid recomputing tx size const size_t nUsageSize; //!< ... and total memory usage const int64_t nTime; //!< Local time when entering the mempool + const uint64_t entry_sequence; //!< Sequence number used to determine whether this transaction is too recent for relay const unsigned int entryHeight; //!< Chain height when entering the mempool const bool spendsCoinbase; //!< keep track of transactions that spend a coinbase const int64_t sigOpCount; //!< Legacy sig ops plus P2SH sig op count @@ -106,7 +107,7 @@ class CTxMemPoolEntry public: CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, - int64_t time, unsigned int entry_height, + int64_t time, unsigned int entry_height, uint64_t entry_sequence, bool spends_coinbase, int64_t sigops_count, LockPoints lp) : tx{tx}, @@ -114,6 +115,7 @@ class CTxMemPoolEntry nTxSize(tx->GetTotalSize()), nUsageSize{RecursiveDynamicUsage(tx)}, nTime{time}, + entry_sequence{entry_sequence}, entryHeight{entry_height}, spendsCoinbase{spends_coinbase}, sigOpCount{sigops_count}, @@ -145,6 +147,7 @@ class CTxMemPoolEntry } std::chrono::seconds GetTime() const { return std::chrono::seconds{nTime}; } unsigned int GetHeight() const { return entryHeight; } + uint64_t GetSequence() const { return entry_sequence; } int64_t GetSigOpCount() const { return sigOpCount; } CAmount GetModifiedFee() const { return m_modified_fee; } size_t DynamicMemoryUsage() const { return nUsageSize; } diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index 71038471930d..99edf5dafe3a 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -336,7 +336,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(Chainstate& chainstate, int nHeigh uint256 quorumHash = GetQuorumBlockHash(llmq_params, chainstate.m_chain, nHeight, qc.quorumIndex); - LogPrint(BCLog::LLMQ, /* Continued */ + LogPrint(BCLog::LLMQ, "%s -- processing commitment for block height=%d, type=%d, quorumIndex=%d, quorumHash=%s, signers=%s, " "validMembers=%d, quorumPublicKey=%s " "fJustCheck[%d] processing commitment from block.\n", @@ -349,7 +349,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(Chainstate& chainstate, int nHeigh } if (quorumHash.IsNull()) { - LogPrint(BCLog::LLMQ, /* Continued */ + LogPrint(BCLog::LLMQ, "%s -- height=%d, type=%d, quorumIndex=%d, quorumHash=%s, signers=%s, validMembers=%d, " "quorumPublicKey=%s quorumHash is null.\n", __func__, nHeight, std23::to_underlying(qc.llmqType), qc.quorumIndex, quorumHash.ToString(), qc.CountSigners(), @@ -357,7 +357,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(Chainstate& chainstate, int nHeigh return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-qc-block"); } if (quorumHash != qc.quorumHash) { - LogPrint(BCLog::LLMQ, /* Continued */ + LogPrint(BCLog::LLMQ, "%s -- height=%d, type=%d, quorumIndex=%d, quorumHash=%s, qc.quorumHash=%s signers=%s, " "validMembers=%d, quorumPublicKey=%s non equal quorumHash.\n", __func__, nHeight, std23::to_underlying(qc.llmqType), qc.quorumIndex, quorumHash.ToString(), @@ -367,7 +367,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(Chainstate& chainstate, int nHeigh if (qc.IsNull()) { if (!qc.VerifyNull()) { - LogPrint(BCLog::LLMQ, /* Continued */ + LogPrint(BCLog::LLMQ, "%s -- height=%d, type=%d, quorumIndex=%d, quorumHash=%s, signers=%s, validMembers=%dqc " "verifynull failed.\n", __func__, nHeight, std23::to_underlying(qc.llmqType), qc.quorumIndex, quorumHash.ToString(), @@ -405,7 +405,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(Chainstate& chainstate, int nHeigh // we don't validate signatures here; they already validated on previous step if (!qc.Verify({m_dmnman, m_qsnapman, m_chainman, pQuorumBaseBlockIndex}, /*checksigs=*/false)) { - LogPrint(BCLog::LLMQ, /* Continued */ + LogPrint(BCLog::LLMQ, "%s -- height=%d, type=%d, quorumIndex=%d, quorumHash=%s, signers=%s, validMembers=%d, " "quorumPublicKey=%s qc verify failed.\n", __func__, nHeight, std23::to_underlying(qc.llmqType), qc.quorumIndex, quorumHash.ToString(), qc.CountSigners(), diff --git a/src/llmq/net_dkg.cpp b/src/llmq/net_dkg.cpp index d320c234af90..22158bd9cccd 100644 --- a/src/llmq/net_dkg.cpp +++ b/src/llmq/net_dkg.cpp @@ -836,7 +836,7 @@ static void AddQuorumProbeConnections(const Consensus::LLMQParams& llmqParams, C dmn->pdmnState->netInfo->GetPrimary().ToStringAddrPort()); } } - LogPrint(BCLog::NET_NETCONN, debugMsg.c_str()); /* Continued */ + LogPrint(BCLog::NET_NETCONN, debugMsg.c_str()); } connman.AddPendingProbeConnections(probeConnections); } diff --git a/src/llmq/net_quorum.cpp b/src/llmq/net_quorum.cpp index cde8776470b1..eaf67ff7e833 100644 --- a/src/llmq/net_quorum.cpp +++ b/src/llmq/net_quorum.cpp @@ -824,7 +824,7 @@ bool EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, CConnman& dmn->pdmnState->netInfo->GetPrimary().ToStringAddrPort()); } } - LogPrint(BCLog::NET_NETCONN, debugMsg.c_str()); /* Continued */ + LogPrint(BCLog::NET_NETCONN, debugMsg.c_str()); } connman.SetMasternodeQuorumNodes(llmqParams.type, util_params.m_base_index->GetBlockHash(), connections); } diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 5aa2df60dea1..d1998ffced08 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -474,7 +474,7 @@ bool CSigSharesManager::ProcessMessageSigShare(NodeId fromId, const CSigShare& s LOCK(cs); if (alreadyRecovered) { - LogPrint(BCLog::LLMQ_SIGS, /* Continued */ + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- dropping sigShare for recovered session. signHash=%s, id=%s, " "msgHash=%s, member=%d, node=%d\n", __func__, signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), @@ -699,7 +699,7 @@ std::shared_ptr CSigSharesManager::TryRecoverSig(const CQuorum& q std::shared_ptr singleMemberRecoveredSig; if (quorum.params.is_single_member()) { if (sigSharesForSignHash->empty()) { - LogPrint(BCLog::LLMQ_SIGS, /* Continued */ + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- impossible to recover single-node signature - no shares yet. id=%s, " "msgHash=%s\n", __func__, id.ToString(), msgHash.ToString()); @@ -835,12 +835,12 @@ bool CSigSharesManager::AsyncSignIfMember(Consensus::LLMQType llmqType, const ui db.GetVoteForId(llmqType, id, prevMsgHash); if (msgHash != prevMsgHash) { if (allowDiffMsgHashSigning) { - LogPrintf("%s -- already voted for id=%s and msgHash=%s. Signing for different " /* Continued */ + LogPrintf("%s -- already voted for id=%s and msgHash=%s. Signing for different " "msgHash=%s\n", __func__, id.ToString(), prevMsgHash.ToString(), msgHash.ToString()); hasVoted = false; } else { - LogPrintf("%s -- already voted for id=%s and msgHash=%s. Not voting on " /* Continued */ + LogPrintf("%s -- already voted for id=%s and msgHash=%s. Not voting on " "conflicting msgHash=%s\n", __func__, id.ToString(), prevMsgHash.ToString(), msgHash.ToString()); return false; @@ -1053,7 +1053,7 @@ void CSigSharesManager::CollectSigSharesToSendConcentrated(std::unordered_mapproTxHash; if (quorum.IsValidMember(proTxHash) && (proTxHash == myProTxHash || proTxToNode.contains(proTxHash))) { if (attempt != signedSession.attempt) { - LogPrint(BCLog::LLMQ_SIGS, /* Continued */ + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, skipped %d unreachable recovery members\n", __func__, signedSession.sigShare.GetSignHash().ToString(), attempt - signedSession.attempt); } @@ -1412,13 +1412,13 @@ void CSigSharesManager::Cleanup() } } - LogPrintLevel(BCLog::LLMQ_SIGS, BCLog::Level::Info, /* Continued */ + LogPrintLevel(BCLog::LLMQ_SIGS, BCLog::Level::Info, "CSigSharesManager::%s -- signing session timed out. signHash=%s, id=%s, msgHash=%s, " "sigShareCount=%d, missingMembers=%s\n", __func__, signHash.ToString(), oneSigShare.getId().ToString(), oneSigShare.getMsgHash().ToString(), count, strMissingMembers); } else { - LogPrintLevel(BCLog::LLMQ_SIGS, BCLog::Level::Info, /* Continued */ + LogPrintLevel(BCLog::LLMQ_SIGS, BCLog::Level::Info, "CSigSharesManager::%s -- signing session timed out. signHash=%s, sigShareCount=%d\n", __func__, signHash.ToString(), count); } @@ -1581,7 +1581,7 @@ std::optional CSigSharesManager::CreateSigShareForSingleMember(const sigShare.UpdateKey(); - LogPrint(BCLog::LLMQ_SIGS, /* Continued */ + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- created sigShare. signHash=%s, id=%s, msgHash=%s, llmqType=%d, quorum=%s, " "time=%s\n", __func__, signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a38b79088cec..b1cf73382e5f 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -112,8 +112,6 @@ static constexpr auto GETDATA_TX_INTERVAL{60s}; /** Limit to avoid sending big packets. Not used in processing incoming GETDATA for compatibility */ static const unsigned int MAX_GETDATA_SZ = 1000; -/** How long a transaction has to be in the mempool before it can unconditionally be relayed. */ -static constexpr auto UNCONDITIONAL_RELAY_DELAY = 2min; /** Headers download timeout. * Timeout = base + per_header * (expected number of headers) */ static constexpr auto HEADERS_DOWNLOAD_TIMEOUT_BASE = 15min; @@ -194,13 +192,9 @@ static constexpr auto OUTBOUND_INVENTORY_BROADCAST_INTERVAL{2s}; static constexpr unsigned int INVENTORY_BROADCAST_PER_SECOND = 7; /** Maximum number of inventory items to send per transmission. */ static constexpr unsigned int INVENTORY_BROADCAST_MAX_PER_1MB_BLOCK = 4 * INVENTORY_BROADCAST_PER_SECOND * count_seconds(INBOUND_INVENTORY_BROADCAST_INTERVAL); -/** The number of most recently announced transactions a peer can request. */ -static constexpr unsigned int INVENTORY_MAX_RECENT_RELAY = 3500; -/** Verify that INVENTORY_MAX_RECENT_RELAY is enough to cache everything typically - * relayed before unconditional relay from the mempool kicks in. This is only a - * lower bound, and it should be larger to account for higher inv rate to outbound - * peers, and random variations in the broadcast mechanism. */ -static_assert(INVENTORY_MAX_RECENT_RELAY >= INVENTORY_BROADCAST_PER_SECOND * UNCONDITIONAL_RELAY_DELAY / std::chrono::seconds{1}, "INVENTORY_RELAY_MAX too low"); +static constexpr unsigned int INVENTORY_BROADCAST_MAX = 1000; +static_assert(INVENTORY_BROADCAST_MAX >= INVENTORY_BROADCAST_MAX_PER_1MB_BLOCK, "INVENTORY_BROADCAST_MAX too low"); +static_assert(INVENTORY_BROADCAST_MAX <= MAX_PEER_OBJECT_ANNOUNCEMENTS, "INVENTORY_BROADCAST_MAX too high"); /** Maximum number of compact filters that may be requested with one getcfilters. See BIP 157. */ static constexpr uint32_t MAX_GETCFILTERS_SIZE = 1000; /** Maximum number of cf hashes that may be requested with one getcfheaders. See BIP 157. */ @@ -313,9 +307,6 @@ struct Peer { /** A bloom filter for which transactions to announce to the peer. See BIP37. */ std::unique_ptr m_bloom_filter PT_GUARDED_BY(m_bloom_filter_mutex) GUARDED_BY(m_bloom_filter_mutex){nullptr}; - /** A rolling bloom filter of all announced tx CInvs to this peer */ - CRollingBloomFilter m_recently_announced_invs GUARDED_BY(NetEventsInterface::g_msgproc_mutex){INVENTORY_MAX_RECENT_RELAY, 0.000001}; - mutable RecursiveMutex m_tx_inventory_mutex; /** A filter of all the txids that the peer has announced to * us or we have announced to the peer. We use this to avoid announcing @@ -331,11 +322,12 @@ struct Peer { * permitted if the peer has NetPermissionFlags::Mempool or we advertise * NODE_BLOOM. See BIP35. */ bool m_send_mempool GUARDED_BY(m_tx_inventory_mutex){false}; - /** The last time a BIP35 `mempool` request was serviced. */ - std::atomic m_last_mempool_req{0s}; /** The next time after which we will send an `inv` message containing * transaction announcements to this peer. */ std::chrono::microseconds m_next_inv_send_time GUARDED_BY(m_tx_inventory_mutex){0}; + /** The mempool sequence num at which we sent the last `inv` message to this peer. + * Can relay txs with lower sequence numbers than this (see CTxMempool::info_for_relay). */ + uint64_t m_last_inv_sequence GUARDED_BY(NetEventsInterface::g_msgproc_mutex){1}; }; /** @@ -1114,7 +1106,7 @@ class PeerManagerImpl final : public PeerManager std::atomic m_last_tip_update{0s}; /** Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). */ - CTransactionRef FindTxForGetData(const Peer::TxRelay& tx_relay, const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) + CTransactionRef FindTxForGetData(const Peer::TxRelay& tx_relay, const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex, NetEventsInterface::g_msgproc_mutex); void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic& interruptMsgProc) @@ -2971,28 +2963,20 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } //! Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). -CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer::TxRelay& tx_relay, const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) +CTransactionRef PeerManagerImpl::FindTxForGetData(const Peer::TxRelay& tx_relay, const uint256& txid) { - auto txinfo = m_mempool.info(txid); + // If a tx was in the mempool prior to the last INV for this peer, permit the request. + auto txinfo = m_mempool.info_for_relay(txid, tx_relay.m_last_inv_sequence); if (!txinfo.tx) { // A MSG_ASSET_UNLOCK getdata identifies a version 2 asset unlock by its instance hash if (const auto unlock_tx = m_mempool.GetAssetUnlockByInstanceHash(txid)) { - txinfo = m_mempool.info(unlock_tx->GetHash()); + txinfo = m_mempool.info_for_relay(unlock_tx->GetHash(), tx_relay.m_last_inv_sequence); } } if (txinfo.tx) { - // If a TX could have been INVed in reply to a MEMPOOL request, - // or is older than UNCONDITIONAL_RELAY_DELAY, permit the request - // unconditionally. - if ((mempool_req.count() && txinfo.m_time <= mempool_req) || txinfo.m_time <= now - UNCONDITIONAL_RELAY_DELAY) { - return std::move(txinfo.tx); - } + return std::move(txinfo.tx); } - // Otherwise, the transaction might have been announced recently. - bool recent = tx_relay.m_recently_announced_invs.contains(txid); - if (recent && txinfo.tx) return std::move(txinfo.tx); - // Or it might be from the most recent block { LOCK(m_most_recent_block_mutex); @@ -3015,11 +2999,6 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic std::vector vNotFound; const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); - const auto now{GetTime()}; - // Get last mempool request time - const auto mempool_req = tx_relay != nullptr ? tx_relay->m_last_mempool_req.load() - : std::chrono::seconds::min(); - // Process as many TX items from the front of the getdata queue as // possible, since they're common and it's efficient to batch process // them. @@ -3047,7 +3026,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic bool push = false; if (inv.IsGenTxMsg()) { - CTransactionRef tx = FindTxForGetData(*tx_relay, inv.hash, mempool_req, now); + CTransactionRef tx = FindTxForGetData(*tx_relay, inv.hash); if (tx) { CCoinJoinBroadcastTx dstx; if (inv.IsMsgDstx()) { @@ -3060,29 +3039,6 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic } m_mempool.RemoveUnbroadcastTx(tx->GetHash()); push = true; - - // As we're going to send tx, make sure its unconfirmed parents are made requestable. - std::vector parent_ids_to_add; - { - LOCK(m_mempool.cs); - auto tx_iter = m_mempool.GetIter(tx->GetHash()); - if (tx_iter) { - const CTxMemPoolEntry::Parents& parents = (*tx_iter)->GetMemPoolParentsConst(); - parent_ids_to_add.reserve(parents.size()); - for (const CTxMemPoolEntry& parent : parents) { - if (parent.GetTime() > now - UNCONDITIONAL_RELAY_DELAY) { - parent_ids_to_add.push_back(parent.GetTx().GetHash()); - } - } - } - } - - for (const uint256& parent_txid : parent_ids_to_add) { - // Relaying a transaction with a recent but unconfirmed parent. - if (WITH_LOCK(tx_relay->m_tx_inventory_mutex, return !tx_relay->m_tx_inventory_known_filter.contains(parent_txid))) { - tx_relay->m_recently_announced_invs.insert(parent_txid); - } - } } } @@ -3538,7 +3494,7 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, uint32_t stop_height = stop_index->nHeight; if (start_height > stop_height) { - LogPrint(BCLog::NET, "peer %d sent invalid getcfilters/getcfheaders with " /* Continued */ + LogPrint(BCLog::NET, "peer %d sent invalid getcfilters/getcfheaders with " "start height %d and stop height %d\n", node.GetId(), start_height, stop_height); node.fDisconnect = true; @@ -6713,7 +6669,6 @@ bool PeerManagerImpl::SendMessages(CNode* pto) auto queueAndMaybePushInv = [this, pto, peer, &vInv, &msgMaker](const CInv& invIn) { LogPrint(BCLog::NET, "SendMessages -- queued inv: %s index=%d peer=%d\n", invIn.ToString(), vInv.size(), pto->GetId()); - // Responses to MEMPOOL requests bypass the m_recently_announced_invs filter. vInv.push_back(invIn); if (vInv.size() == MAX_INV_SZ) { LogPrint(BCLog::NET, "SendMessages -- pushing invs: count=%d peer=%d\n", vInv.size(), pto->GetId()); @@ -6787,7 +6742,6 @@ bool PeerManagerImpl::SendMessages(CNode* pto) tx_relay->m_tx_inventory_known_filter.insert(chainlockHash); queueAndMaybePushInv(CInv(MSG_CLSIG, chainlockHash)); } - tx_relay->m_last_mempool_req = std::chrono::duration_cast(current_time); } // Determine transactions to relay @@ -6808,7 +6762,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // especially since we have many peers and some will draw much shorter delays. unsigned int nRelayedTransactions = 0; size_t broadcast_max{INVENTORY_BROADCAST_MAX_PER_1MB_BLOCK * MaxBlockSize() / 1000000 + (tx_relay->m_tx_inventory_to_send.size()/1000)*5}; - broadcast_max = std::min(1000, broadcast_max); + broadcast_max = std::min(INVENTORY_BROADCAST_MAX, broadcast_max); while (!vInvTx.empty() && nRelayedTransactions < broadcast_max) { // Fetch the top element from the heap @@ -6836,11 +6790,14 @@ bool PeerManagerImpl::SendMessages(CNode* pto) nInvType = MSG_DSTX; } // Send - tx_relay->m_recently_announced_invs.insert(announce_hash); nRelayedTransactions++; tx_relay->m_tx_inventory_known_filter.insert(announce_hash); queueAndMaybePushInv(CInv(nInvType, announce_hash)); } + + // Ensure we'll respond to GETDATA requests for anything we've just announced + LOCK(m_mempool.cs); + tx_relay->m_last_inv_sequence = m_mempool.GetSequence(); } } { diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 6c27552bcd89..dda05b7f5f40 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -727,7 +727,7 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne // a reindex. A flush error might also leave some of the data files // untrimmed. if (!FlushBlockFile(!fKnown, finalize_undo)) { - LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, /* Continued */ + LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, "Failed to flush previous block file %05i (finalize=%i, finalize_undo=%i) before opening new block file %05i\n", m_last_blockfile, !fKnown, finalize_undo, nFile); } diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 5e7f32d36184..c0878655a70a 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -1597,7 +1597,7 @@ class ChainImpl : public Chain { if (!m_node.mempool) return {}; LockPoints lp; - CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp); + CTxMemPoolEntry entry(tx, 0, 0, 0, 0, false, 0, lp); const CTxMemPool::Limits& limits{m_node.mempool->m_limits}; LOCK(m_node.mempool->cs); auto ancestors{m_node.mempool->CalculateMemPoolAncestors(entry, limits)}; diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 1def21c59a01..a7ccaf6a1f65 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -214,7 +214,7 @@ void BlockAssembler::FillBlockPayments(CMutableTransaction& txNew, const CBlockI voutMasternodeStr += txout.ToString(); } - LogPrint(BCLog::MNPAYMENTS, "%s -- nBlockHeight %d blockReward %lld voutMasternodePaymentsRet \"%s\" txNew %s", __func__, /* Continued */ + LogPrint(BCLog::MNPAYMENTS, "%s -- nBlockHeight %d blockReward %lld voutMasternodePaymentsRet \"%s\" txNew %s", __func__, // NOLINT(bitcoin-unterminated-logprintf) nBlockHeight, blockSubsidy + feeReward, voutMasternodeStr, txNew.ToString()); } diff --git a/src/node/peerman_args.cpp b/src/node/peerman_args.cpp index c3cdbcc44b06..55570cdfc7ec 100644 --- a/src/node/peerman_args.cpp +++ b/src/node/peerman_args.cpp @@ -23,6 +23,8 @@ void ApplyArgsManOptions(const ArgsManager& argsman, PeerManager::Options& optio if (auto value{argsman.GetBoolArg("-capturemessages")}) options.capture_messages = *value; if (auto value{argsman.GetIntArg("-maxtimeadjustment")}) options.max_time_adjustment = *value; + + if (auto value{argsman.GetBoolArg("-blocksonly")}) options.ignore_incoming_txs = *value; } } // namespace node diff --git a/src/node/sync_manager.cpp b/src/node/sync_manager.cpp index 1573e25c21d0..64d8b5598080 100644 --- a/src/node/sync_manager.cpp +++ b/src/node/sync_manager.cpp @@ -226,7 +226,7 @@ void SyncManager::ProcessTick() if (!pNodeTmp->IsInboundConn() && !fRequestedEarlier && !pNodeTmp->IsBlockRelayOnly()) { m_netfulfilledman.AddFulfilledRequest(pNodeTmp->addr, "mempool-sync"); m_connman.PushMessage(pNodeTmp, msgMaker.Make(NetMsgType::MEMPOOL)); - LogPrint(BCLog::MNSYNC, /* Continued */ + LogPrint(BCLog::MNSYNC, "Sync Tick -- nTick %d asset_id %d -- syncing mempool from peer=%d\n", nTick, asset_id, pNodeTmp->GetId()); } diff --git a/src/node/utxo_snapshot.cpp b/src/node/utxo_snapshot.cpp index 3498e86203e6..479dd6598b57 100644 --- a/src/node/utxo_snapshot.cpp +++ b/src/node/utxo_snapshot.cpp @@ -58,7 +58,7 @@ bool WriteSnapshotBaseBlockhash(Chainstate& snapshot_chainstate) std::optional ReadSnapshotBaseBlockhash(fs::path chaindir) { if (!fs::exists(chaindir)) { - LogPrintf("[snapshot] cannot read base blockhash: no chainstate dir " /* Continued */ + LogPrintf("[snapshot] cannot read base blockhash: no chainstate dir " "exists at path %s\n", fs::PathToString(chaindir)); return std::nullopt; } @@ -66,7 +66,7 @@ std::optional ReadSnapshotBaseBlockhash(fs::path chaindir) const std::string read_from_str = fs::PathToString(read_from); if (!fs::exists(read_from)) { - LogPrintf("[snapshot] snapshot chainstate dir is malformed! no base blockhash file " /* Continued */ + LogPrintf("[snapshot] snapshot chainstate dir is malformed! no base blockhash file " "exists at path %s. Try deleting %s and calling loadtxoutset again?\n", fs::PathToString(chaindir), read_from_str); return std::nullopt; diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 1dc0ecdba30f..861c0ceea0d1 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -384,7 +384,7 @@ static RPCHelpMan gobject_submit() if (node.active_ctx) { const auto& mn_activeman{*node.active_ctx->nodeman}; const bool fMnFound = mnList.HasValidMNByCollateral(mn_activeman.GetOutPoint()); - LogPrint(BCLog::GOBJECT, /* Continued */ + LogPrint(BCLog::GOBJECT, "gobject_submit -- pubKeyOperator = %s, outpoint = %s, params.size() = %lld, fMnFound = %d\n", mn_activeman.GetPubKey().ToString(false), mn_activeman.GetOutPoint().ToStringShort(), request.params.size(), fMnFound); diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index d76f94cab6c1..7ac6af392fb9 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -133,7 +133,7 @@ bool ParseBoolV(const UniValue& v, const std::string &strName) } if (!v.isBool()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be a JSON boolean. " /* Continued */ + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be a JSON boolean. " "Pass -deprecatedrpc=permissive_bool to allow legacy boolean parsing.", strName)); } return v.get_bool(); diff --git a/src/stacktraces.cpp b/src/stacktraces.cpp index a1d59b966941..b0e35c97dcb3 100644 --- a/src/stacktraces.cpp +++ b/src/stacktraces.cpp @@ -532,7 +532,7 @@ static std::string GetCrashInfoStr(const crash_info& ci, size_t spaces) static void PrintCrashInfo(const crash_info& ci) { auto str = GetCrashInfoStr(ci); - LogPrintf("%s", str); /* Continued */ + LogPrintf("%s", str); // NOLINT(bitcoin-unterminated-logprintf) tfm::format(std::cerr, "%s", str); fflush(stderr); } diff --git a/src/sync.cpp b/src/sync.cpp index 79453ed009bf..48799b35fdbb 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -134,7 +134,7 @@ static void double_lock_detected(const void* mutex, const LockStack& lock_stack) LogPrintf("Lock order:\n"); for (const LockStackItem& i : lock_stack) { if (i.first == mutex) { - LogPrintf(" (*)"); /* Continued */ + LogPrintf(" (*)"); // NOLINT(bitcoin-unterminated-logprintf) } LogPrintf(" %s\n", i.second.ToString()); } diff --git a/src/test/fuzz/util/mempool.cpp b/src/test/fuzz/util/mempool.cpp index 4d94196a7f64..6fbc98673204 100644 --- a/src/test/fuzz/util/mempool.cpp +++ b/src/test/fuzz/util/mempool.cpp @@ -23,9 +23,10 @@ CTxMemPoolEntry ConsumeTxMemPoolEntry(FuzzedDataProvider& fuzzed_data_provider, const CAmount fee{ConsumeMoney(fuzzed_data_provider, /*max=*/std::numeric_limits::max() / CAmount{100'000})}; assert(MoneyRange(fee)); const int64_t time = fuzzed_data_provider.ConsumeIntegral(); + const uint64_t entry_sequence{fuzzed_data_provider.ConsumeIntegral()}; const unsigned int entry_height = fuzzed_data_provider.ConsumeIntegral(); const bool spends_coinbase = fuzzed_data_provider.ConsumeBool(); const bool dip1_status = fuzzed_data_provider.ConsumeBool(); const unsigned int sig_op_cost = fuzzed_data_provider.ConsumeIntegralInRange(0, MaxBlockSigOps(dip1_status)); - return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, entry_height, spends_coinbase, sig_op_cost, {}}; + return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, entry_height, entry_sequence, spends_coinbase, sig_op_cost, {}}; } diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp index f65c69e2ad37..cd6e6d1c57a1 100644 --- a/src/test/logging_tests.cpp +++ b/src/test/logging_tests.cpp @@ -83,12 +83,12 @@ BOOST_AUTO_TEST_CASE(logging_timer) BOOST_FIXTURE_TEST_CASE(logging_LogPrintf_, LogSetup) { LogInstance().m_log_sourcelocations = true; - LogPrintf_("fn1", "src1", 1, BCLog::LogFlags::NET, BCLog::Level::Debug, "foo1: %s", "bar1\n"); - LogPrintf_("fn2", "src2", 2, BCLog::LogFlags::NET, BCLog::Level::Info, "foo2: %s", "bar2\n"); - LogPrintf_("fn3", "src3", 3, BCLog::LogFlags::ALL, BCLog::Level::Debug, "foo3: %s", "bar3\n"); - LogPrintf_("fn4", "src4", 4, BCLog::LogFlags::ALL, BCLog::Level::Info, "foo4: %s", "bar4\n"); - LogPrintf_("fn5", "src5", 5, BCLog::LogFlags::NONE, BCLog::Level::Debug, "foo5: %s", "bar5\n"); - LogPrintf_("fn6", "src6", 6, BCLog::LogFlags::NONE, BCLog::Level::Info, "foo6: %s", "bar6\n"); + LogPrintf_("fn1", "src1", 1, BCLog::LogFlags::NET, BCLog::Level::Debug, "foo1: %s\n", "bar1"); + LogPrintf_("fn2", "src2", 2, BCLog::LogFlags::NET, BCLog::Level::Info, "foo2: %s\n", "bar2"); + LogPrintf_("fn3", "src3", 3, BCLog::LogFlags::ALL, BCLog::Level::Debug, "foo3: %s\n", "bar3"); + LogPrintf_("fn4", "src4", 4, BCLog::LogFlags::ALL, BCLog::Level::Info, "foo4: %s\n", "bar4"); + LogPrintf_("fn5", "src5", 5, BCLog::LogFlags::NONE, BCLog::Level::Debug, "foo5: %s\n", "bar5"); + LogPrintf_("fn6", "src6", 6, BCLog::LogFlags::NONE, BCLog::Level::Info, "foo6: %s\n", "bar6"); std::ifstream file{tmp_log_path}; std::vector log_lines; for (std::string log; std::getline(file, log);) { diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h index 4bd17e0efb63..2ac5cffca389 100644 --- a/src/test/util/chainstate.h +++ b/src/test/util/chainstate.h @@ -50,7 +50,7 @@ CreateAndActivateUTXOSnapshot( UniValue result = CreateUTXOSnapshot( node, node.chainman->ActiveChainstate(), auto_outfile, snapshot_path, snapshot_path); LogPrintf( - "Wrote UTXO snapshot to %s: %s", fs::PathToString(snapshot_path.make_preferred()), result.write()); + "Wrote UTXO snapshot to %s: %s\n", fs::PathToString(snapshot_path.make_preferred()), result.write()); // Read the written snapshot in and then activate it. // diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index dccfbefccd41..35407c665f43 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -777,7 +777,7 @@ std::vector TestChainSetup::PopulateMempool(FastRandomContext& LOCK2(cs_main, m_node.mempool->cs); LockPoints lp; m_node.mempool->addUnchecked(CTxMemPoolEntry(ptx, /*fee=*/(total_in - num_outputs * amount_per_output), - /*time=*/0, /*entry_height=*/1, + /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_count=*/4, lp)); } --num_transactions; @@ -807,7 +807,7 @@ void TestChainSetup::MockMempoolMinFee(const CFeeRate& target_feerate) const auto tx_fee = target_feerate.GetFee(GetVirtualTransactionSize(*tx)) - m_node.mempool->m_incremental_relay_feerate.GetFee(GetVirtualTransactionSize(*tx)); m_node.mempool->addUnchecked(CTxMemPoolEntry(tx, /*fee=*/tx_fee, - /*time=*/0, /*entry_height=*/1, + /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/true, /*sigops_count=*/1, lp)); m_node.mempool->TrimToSize(0); assert(m_node.mempool->GetMinFee() == target_feerate); diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index 23aff3f2e2d4..2733fa21c234 100644 --- a/src/test/util/txmempool.cpp +++ b/src/test/util/txmempool.cpp @@ -36,6 +36,6 @@ CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) co CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransactionRef& tx) const { - return CTxMemPoolEntry(tx, nFee, TicksSinceEpoch(time), nHeight, + return CTxMemPoolEntry(tx, nFee, TicksSinceEpoch(time), nHeight, m_sequence, spendsCoinbase, sigOpCount, lp); } diff --git a/src/test/util/txmempool.h b/src/test/util/txmempool.h index c99ee4933ebd..0304a2111505 100644 --- a/src/test/util/txmempool.h +++ b/src/test/util/txmempool.h @@ -19,6 +19,7 @@ struct TestMemPoolEntryHelper { CAmount nFee{0}; NodeSeconds time{}; unsigned int nHeight{1}; + uint64_t m_sequence{0}; bool spendsCoinbase{false}; unsigned int sigOpCount{1}; LockPoints lp; @@ -30,6 +31,7 @@ struct TestMemPoolEntryHelper { TestMemPoolEntryHelper &Fee(CAmount _fee) { nFee = _fee; return *this; } TestMemPoolEntryHelper& Time(NodeSeconds tp) { time = tp; return *this; } TestMemPoolEntryHelper &Height(unsigned int _height) { nHeight = _height; return *this; } + TestMemPoolEntryHelper& Sequence(uint64_t _seq) { m_sequence = _seq; return *this; } TestMemPoolEntryHelper &SpendsCoinbase(bool _flag) { spendsCoinbase = _flag; return *this; } TestMemPoolEntryHelper& SigOps(unsigned int _sigops) { sigOpCount = _sigops; return *this; } }; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 4b1199e69114..7511e73d029f 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1592,6 +1592,17 @@ TxMempoolInfo CTxMemPool::info(const uint256& hash) const return GetInfo(i); } +TxMempoolInfo CTxMemPool::info_for_relay(const uint256& hash, uint64_t last_sequence) const +{ + LOCK(cs); + indexed_transaction_set::const_iterator i = mapTx.find(hash); + if (i != mapTx.end() && i->GetSequence() < last_sequence) { + return GetInfo(i); + } else { + return TxMempoolInfo(); + } +} + bool CTxMemPool::existsProviderTxCrossSchemeConflict(const CTransaction& tx) const { LOCK(cs); diff --git a/src/txmempool.h b/src/txmempool.h index 3fd0fb4e170b..328a0b13b0f5 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -720,6 +720,10 @@ class CTxMemPool CTransactionRef get(const uint256& hash) const; TxMempoolInfo info(const uint256& hash) const; + + /** Returns info for a transaction if its entry_sequence < last_sequence */ + TxMempoolInfo info_for_relay(const uint256& hash, uint64_t last_sequence) const; + std::vector infoAll() const; /** Get the transaction holding this version 2 asset unlock instance hash, if any. */ diff --git a/src/validation.cpp b/src/validation.cpp index bd394b24d84a..f901c654adeb 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1045,7 +1045,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } } - entry.reset(new CTxMemPoolEntry(ptx, ws.m_base_fees, nAcceptTime, m_active_chainstate.m_chain.Height(), + // Set entry_sequence to 0 when bypass_limits is used; this allows txs from a block + // reorg to be marked earlier than any child txs that were already in the mempool. + const uint64_t entry_sequence = bypass_limits ? 0 : m_pool.GetSequence(); + entry.reset(new CTxMemPoolEntry(ptx, ws.m_base_fees, nAcceptTime, m_active_chainstate.m_chain.Height(), entry_sequence, fSpendsCoinbase, nSigOps, lock_points.value())); ws.m_vsize = entry->GetTxSize(); @@ -2616,7 +2619,7 @@ bool Chainstate::IsBlockValueValid(const CBlock& block, const CBlockIndex* pinde const bool is_v24{DeploymentActiveAfter(pindex->pprev, m_chainman, Consensus::DEPLOYMENT_V24)}; if (!IsSuperblockValid(superblock, *block.vtx[0], blockReward, is_v24, nSuperblockPaymentsLimit)) { // triggered but invalid? that's weird - LogPrintf("%s -- ERROR! Invalid superblock detected at height %d: %s", __func__, nBlockHeight, block.vtx[0]->ToString()); /* Continued */ + LogPrintf("%s -- ERROR! Invalid superblock detected at height %d: %s", __func__, nBlockHeight, block.vtx[0]->ToString()); // NOLINT(bitcoin-unterminated-logprintf) // should NOT allow invalid superblocks, when superblocks are enabled strErrorRet = strprintf("invalid superblock detected at height %d", nBlockHeight); return false; @@ -2659,10 +2662,10 @@ bool Chainstate::IsBlockPayeeValid(const CBlock& block, const CBlockIndex* pinde } LogPrintf("%s -- ERROR! Failed to find expected payee %s amount=%lld height=%d\n", __func__, str_payout, txout.nValue, nBlockHeight); - LogPrintf("%s -- ERROR! Invalid masternode payment detected at height %d: %s", __func__, nBlockHeight, txNew.ToString()); /* Continued */ + LogPrintf("%s -- ERROR! Invalid masternode payment detected at height %d: %s", __func__, nBlockHeight, txNew.ToString()); // NOLINT(bitcoin-unterminated-logprintf) return false; } - LogPrint(BCLog::MNPAYMENTS, "%s -- Valid masternode payment at height %d: %s", __func__, nBlockHeight, txNew.ToString()); /* Continued */ + LogPrint(BCLog::MNPAYMENTS, "%s -- Valid masternode payment at height %d: %s", __func__, nBlockHeight, txNew.ToString()); // NOLINT(bitcoin-unterminated-logprintf) return true; } @@ -3930,7 +3933,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< // Belt-and-suspenders check that we aren't attempting to advance the background // chainstate past the snapshot base block. if (WITH_LOCK(::cs_main, return m_disabled)) { - LogPrintf("m_disabled is set - this chainstate should not be in operation. " /* Continued */ + LogPrintf("m_disabled is set - this chainstate should not be in operation. " "Please report this as a bug. %s\n", PACKAGE_BUGREPORT); return false; } @@ -6419,7 +6422,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( auto maybe_au_data = ExpectedAssumeutxo(base_height, GetParams()); if (!maybe_au_data) { - LogPrintf("[snapshot] assumeutxo height in snapshot metadata not recognized " /* Continued */ + LogPrintf("[snapshot] assumeutxo height in snapshot metadata not recognized " "(%d) - refusing to load snapshot\n", base_height); return false; } @@ -6744,8 +6747,8 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation( }; if (index_new.GetBlockHash() != snapshot_blockhash) { - LogPrintf("[snapshot] supposed base block %s does not match the " /* Continued */ - "snapshot base block %s (height %d). Snapshot is not valid.", + LogPrintf("[snapshot] supposed base block %s does not match the " + "snapshot base block %s (height %d). Snapshot is not valid.\n", index_new.ToString(), snapshot_blockhash.ToString(), snapshot_base_height); handle_invalid_snapshot(); return SnapshotCompletionResult::BASE_BLOCKHASH_MISMATCH; @@ -6763,7 +6766,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation( auto maybe_au_data = ExpectedAssumeutxo(curr_height, m_options.chainparams); if (!maybe_au_data) { - LogPrintf("[snapshot] assumeutxo data not found for height " /* Continued */ + LogPrintf("[snapshot] assumeutxo data not found for height " "(%d) - refusing to validate snapshot\n", curr_height); handle_invalid_snapshot(); return SnapshotCompletionResult::MISSING_CHAINPARAMS; @@ -6771,7 +6774,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation( const AssumeutxoData& au_data = *maybe_au_data; std::optional maybe_ibd_stats; - LogPrintf("[snapshot] computing UTXO stats for background chainstate to validate " /* Continued */ + LogPrintf("[snapshot] computing UTXO stats for background chainstate to validate " "snapshot - this could take a few minutes\n"); try { maybe_ibd_stats = ComputeUTXOStats( @@ -7149,7 +7152,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() // is in-memory, in which case we can't do on-disk cleanup. You'd better be // in a unittest! if (!ibd_chainstate_path_maybe || !snapshot_chainstate_path_maybe) { - LogPrintf("[snapshot] snapshot chainstate cleanup cannot happen with " /* Continued */ + LogPrintf("[snapshot] snapshot chainstate cleanup cannot happen with " "in-memory chainstates. You are testing, right?\n"); return false; } @@ -7196,7 +7199,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() throw; } - LogPrintf("[snapshot] moving snapshot chainstate (%s) to " /* Continued */ + LogPrintf("[snapshot] moving snapshot chainstate (%s) to " "default chainstate directory (%s)\n", fs::PathToString(snapshot_chainstate_path), fs::PathToString(ibd_chainstate_path)); @@ -7217,7 +7220,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() if (!DeleteCoinsDBFromDisk(tmp_old, /*is_snapshot=*/false)) { // No need to AbortNode because once the unneeded bg chainstate data is // moved, it will not interfere with subsequent initialization. - LogPrintf("Deletion of %s failed. Please remove it manually, as the " /* Continued */ + LogPrintf("Deletion of %s failed. Please remove it manually, as the " "directory is now unnecessary.\n", fs::PathToString(tmp_old)); } else { diff --git a/src/validation.h b/src/validation.h index 80a4149e9166..995d034aec7d 100644 --- a/src/validation.h +++ b/src/validation.h @@ -255,7 +255,8 @@ struct PackageMempoolAcceptResult * @param[in] tx The transaction to submit for mempool acceptance. * @param[in] accept_time The timestamp for adding the transaction to the mempool. * It is also used to determine when the entry expires. - * @param[in] bypass_limits When true, don't enforce mempool fee and capacity limits. + * @param[in] bypass_limits When true, don't enforce mempool fee and capacity limits, + * and set entry_sequence to zero. * @param[in] test_accept When true, run validation checks but don't submit to mempool. * * @returns a MempoolAcceptResult indicating whether the transaction was accepted/rejected with reason. diff --git a/src/wallet/coinjoin.cpp b/src/wallet/coinjoin.cpp index 8effeb405daf..3fa51d021684 100644 --- a/src/wallet/coinjoin.cpp +++ b/src/wallet/coinjoin.cpp @@ -223,7 +223,7 @@ std::vector CWallet::SelectCoinsGroupedByAddresses(bool fSkipD for (const auto& item : vecTallyRet) { strMessage += strprintf(" %s %f\n", EncodeDestination(item.txdest), float(item.nAmount) / COIN); } - LogPrint(BCLog::SELECTCOINS, "%s", strMessage); /* Continued */ + LogPrint(BCLog::SELECTCOINS, "%s", strMessage); // NOLINT(bitcoin-unterminated-logprintf) } return vecTallyRet; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index f50eb4733360..bee31c7e1788 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -216,7 +216,7 @@ void WalletInit::InitCoinJoinSettings(CCoinJoinClientManager& mgr) const if (fAutoStart) { mgr.startMixing(); } - LogPrintf("CoinJoin: autostart=%d, multisession=%d," /* Continued */ + LogPrintf("CoinJoin: autostart=%d, multisession=%d," "sessions=%d, rounds=%d, amount=%d, denoms_goal=%d, denoms_hardcap=%d\n", fAutoStart, CCoinJoinClientOptions::IsMultiSessionEnabled(), CCoinJoinClientOptions::GetSessions(), CCoinJoinClientOptions::GetRounds(), diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 4fae8272ebbc..08225ce2d65d 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -260,9 +260,10 @@ class ScriptPubKeyMan virtual std::unordered_set GetScriptPubKeys() const { return {}; }; /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */ - template - void WalletLogPrintf(std::string fmt, Params... parameters) const { - LogPrintf(("%s " + fmt).c_str(), m_storage.GetDisplayName(), parameters...); + template + void WalletLogPrintf(const char* fmt, Params... parameters) const + { + LogPrintf(("%s " + std::string{fmt}).c_str(), m_storage.GetDisplayName(), parameters...); }; /** Watch-only address added */ diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d3598fd130e1..358855ad592d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2581,7 +2581,7 @@ bool CWallet::SignGovernanceVote(const CKeyID& keyID, CGovernanceVote& vote) con util::Result CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm) { LOCK(cs_wallet); - WalletLogPrintf("CommitTransaction:\n%s", tx->ToString()); /* Continued */ + WalletLogPrintf("CommitTransaction:\n%s", tx->ToString()); // NOLINT(bitcoin-unterminated-logprintf) // What this commit records about the transaction. const auto record_commit{[&](CWalletTx& wtx, bool new_tx) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3130774a8c49..bba26b669640 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1183,9 +1183,10 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati }; /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */ - template - void WalletLogPrintf(std::string fmt, Params... parameters) const { - LogPrintf(("%s " + fmt).c_str(), GetDisplayName(), parameters...); + template + void WalletLogPrintf(const char* fmt, Params... parameters) const + { + LogPrintf(("%s " + std::string{fmt}).c_str(), GetDisplayName(), parameters...); }; /** Upgrade the wallet */ diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py index cef1a86e19ca..8f440dea5dad 100755 --- a/test/functional/mempool_limit.py +++ b/test/functional/mempool_limit.py @@ -125,7 +125,7 @@ def run_test(self): self.log.info("Check a package that passes mempoolminfee but is evicted immediately after submission") mempoolmin_feerate = node.getmempoolinfo()["mempoolminfee"] target_weight_each = 50000 - assert_greater_than(target_weight_each * 2, node.getmempoolinfo()["maxmempool"] - node.getmempoolinfo()["bytes"]) + assert_greater_than(target_weight_each * 2, node.getmempoolinfo()["maxmempool"] - node.getmempoolinfo()["usage"]) parent_fee = (mempoolmin_feerate / 1000) * (target_weight_each // 4) - Decimal("0.00001") miniwallet.rescan_utxos() tx_parent_just_below = miniwallet.create_self_transfer(fee=parent_fee, target_weight=target_weight_each) diff --git a/test/functional/mempool_reorg.py b/test/functional/mempool_reorg.py index 3a5bc1ebcd02..425da6e75228 100755 --- a/test/functional/mempool_reorg.py +++ b/test/functional/mempool_reorg.py @@ -8,6 +8,17 @@ that spend (directly or indirectly) coinbase transactions. """ +import time + +from test_framework.messages import ( + CInv, + MSG_TX, + msg_getdata, +) +from test_framework.p2p import ( + P2PTxInvStore, + p2p_lock, +) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_raises_rpc_error from test_framework.wallet import MiniWallet @@ -22,8 +33,84 @@ def set_test_params(self): [] ] + def test_reorg_relay(self): + self.log.info("Test that transactions from disconnected blocks are available for relay immediately") + # Prevent time from moving forward + self.nodes[1].setmocktime(int(time.time())) + self.connect_nodes(0, 1) + self.generate(self.wallet, 3) + + # Disconnect node0 and node1 to create different chains. + self.disconnect_nodes(0, 1) + # Connect a peer to node1, which doesn't have immediate tx relay + peer1 = self.nodes[1].add_p2p_connection(P2PTxInvStore()) + + # Create a transaction that is included in a block. + tx_disconnected = self.wallet.send_self_transfer(from_node=self.nodes[1]) + self.generate(self.nodes[1], 1, sync_fun=self.no_op) + + # Create a transaction and submit it to node1's mempool. + tx_before_reorg = self.wallet.send_self_transfer(from_node=self.nodes[1]) + + # Create a child of that transaction and submit it to node1's mempool. + tx_child = self.wallet.send_self_transfer(utxo_to_spend=tx_disconnected["new_utxo"], from_node=self.nodes[1]) + assert_equal(self.nodes[1].getmempoolentry(tx_child["txid"])["ancestorcount"], 1) + assert_equal(len(peer1.get_invs()), 0) + + # node0 has a longer chain in which tx_disconnected was not confirmed. + self.generate(self.nodes[0], 3, sync_fun=self.no_op) + + # Reconnect the nodes and sync chains. node0's chain should win. + self.connect_nodes(0, 1) + self.sync_blocks() + + # Child now has an ancestor from the disconnected block + assert_equal(self.nodes[1].getmempoolentry(tx_child["txid"])["ancestorcount"], 2) + assert_equal(self.nodes[1].getmempoolentry(tx_before_reorg["txid"])["ancestorcount"], 1) + + # peer1 should not have received an inv for any of the transactions during this time, as no + # mocktime has elapsed for those transactions to be announced. Likewise, it cannot + # request very recent, unanounced transactions. + assert_equal(len(peer1.get_invs()), 0) + # It's too early to request these two transactions + requests_too_recent = msg_getdata([CInv(t=MSG_TX, h=int(tx["txid"], 16)) for tx in [tx_before_reorg, tx_child]]) + peer1.send_and_ping(requests_too_recent) + for _ in range(len(requests_too_recent.inv)): + peer1.sync_with_ping() + with p2p_lock: + assert "tx" not in peer1.last_message + assert "notfound" in peer1.last_message + + # Request the tx from the disconnected block + request_disconnected_tx = msg_getdata([CInv(t=MSG_TX, h=int(tx_disconnected["txid"], 16))]) + peer1.send_and_ping(request_disconnected_tx) + + # The tx from the disconnected block was never announced, and it entered the mempool later + # than the transactions that are too recent. + assert_equal(len(peer1.get_invs()), 0) + with p2p_lock: + # However, the node will answer requests for the tx from the recently-disconnected block. + assert_equal(peer1.last_message["tx"].tx.rehash(), tx_disconnected["txid"]) + + self.nodes[1].setmocktime(int(time.time()) + 300) + peer1.sync_with_ping() + # the transactions are now announced + assert_equal(len(peer1.get_invs()), 3) + for _ in range(3): + # make sure all tx requests have been responded to + peer1.sync_with_ping() + last_tx_received = peer1.last_message["tx"] + + tx_after_reorg = self.wallet.send_self_transfer(from_node=self.nodes[1]) + request_after_reorg = msg_getdata([CInv(t=MSG_TX, h=int(tx_after_reorg["txid"], 16))]) + assert tx_after_reorg["txid"] in self.nodes[1].getrawmempool() + peer1.send_and_ping(request_after_reorg) + with p2p_lock: + assert_equal(peer1.last_message["tx"], last_tx_received) + def run_test(self): - wallet = MiniWallet(self.nodes[0]) + self.wallet = MiniWallet(self.nodes[0]) + wallet = self.wallet # Start with a 200 block chain assert_equal(self.nodes[0].getblockcount(), 200) @@ -103,6 +190,8 @@ def run_test(self): assert_equal(set(self.nodes[0].getrawmempool()), set()) self.sync_all() + self.test_reorg_relay() + if __name__ == '__main__': MempoolCoinbaseTest().main() diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index 6992343d6cf6..f42cbd278aa1 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -82,7 +82,7 @@ def main(): matching_files_filtered = [] for matching_file in matching_files: - if not re.search('^src/(dashbls|leveldb|secp256k1|minisketch|tinyformat|test/fuzz/strprintf.cpp|clientversion.cpp)', matching_file): + if not re.search('^src/(dashbls|leveldb|secp256k1|minisketch|tinyformat|test/fuzz/strprintf.cpp|clientversion.cpp)|contrib/devtools/bitcoin-tidy/example_logprintf.cpp', matching_file): matching_files_filtered.append(matching_file) matching_files_filtered.sort() diff --git a/test/lint/lint-include-guards.py b/test/lint/lint-include-guards.py index c5de3b4394a5..e5dfb0ca6b80 100755 --- a/test/lint/lint-include-guards.py +++ b/test/lint/lint-include-guards.py @@ -19,7 +19,8 @@ HEADER_ID_PREFIX = 'BITCOIN_' HEADER_ID_SUFFIX = '_H' -EXCLUDE_FILES_WITH_PREFIX = ['src/crypto/ctaes', +EXCLUDE_FILES_WITH_PREFIX = ['contrib/devtools/bitcoin-tidy', + 'src/crypto/ctaes', 'src/tinyformat.h', 'src/bench/nanobench.h', 'src/test/fuzz/FuzzedDataProvider.h', diff --git a/test/lint/lint-includes.py b/test/lint/lint-includes.py index b09d8993e99f..ab685933affc 100755 --- a/test/lint/lint-includes.py +++ b/test/lint/lint-includes.py @@ -16,6 +16,9 @@ from lint_ignore_dirs import SHARED_EXCLUDED_SUBTREES +EXCLUDED_DIRS = ["contrib/devtools/bitcoin-tidy/", + ] + SHARED_EXCLUDED_SUBTREES + EXPECTED_BOOST_INCLUDES = ["boost/date_time/posix_time/posix_time.hpp", "boost/hana/for_each.hpp", "boost/hana/tuple.hpp", @@ -100,7 +103,7 @@ def find_extra_boosts(): def find_quote_syntax_inclusions(): - exclude_args = [":(exclude)" + dir for dir in SHARED_EXCLUDED_SUBTREES] + exclude_args = [":(exclude)" + dir for dir in EXCLUDED_DIRS] quote_syntax_inclusions = list() try: diff --git a/test/lint/lint-logs.py b/test/lint/lint-logs.py deleted file mode 100755 index de04a1aecaf5..000000000000 --- a/test/lint/lint-logs.py +++ /dev/null @@ -1,34 +0,0 @@ -#!/usr/bin/env python3 -# -# Copyright (c) 2018-2022 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. -# -# Check that all logs are terminated with '\n' -# -# Some logs are continued over multiple lines. They should be explicitly -# commented with /* Continued */ - -import re -import sys - -from subprocess import check_output - - -def main(): - logs_list = check_output(["git", "grep", "--extended-regexp", r"(LogPrintLevel|LogPrintfCategory|LogPrintf?)\(", "--", "*.cpp"], text=True, encoding="utf8").splitlines() - - unterminated_logs = [line for line in logs_list if not re.search(r'(\\n"|/\* Continued \*/)', line)] - - if unterminated_logs != []: - print("All calls to LogPrintf(), LogPrintfCategory(), LogPrint(), LogPrintLevel(), and WalletLogPrintf() should be terminated with \"\\n\".") - print("") - - for line in unterminated_logs: - print(line) - - sys.exit(1) - - -if __name__ == "__main__": - main() diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py index d6adb50c38ab..ed9f5f98e5a1 100755 --- a/test/lint/run-lint-format-strings.py +++ b/test/lint/run-lint-format-strings.py @@ -24,10 +24,10 @@ ("src/util/system.cpp", "strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION)"), ("src/test/translation_tests.cpp", "strprintf(format, arg)"), ("src/validationinterface.cpp", "LogPrint(BCLog::VALIDATION, fmt \"\\n\", __VA_ARGS__)"), - ("src/wallet/wallet.h", "WalletLogPrintf(std::string fmt, Params... parameters)"), - ("src/wallet/wallet.h", "LogPrintf((\"%s \" + fmt).c_str(), GetDisplayName(), parameters...)"), - ("src/wallet/scriptpubkeyman.h", "WalletLogPrintf(std::string fmt, Params... parameters)"), - ("src/wallet/scriptpubkeyman.h", "LogPrintf((\"%s \" + fmt).c_str(), m_storage.GetDisplayName(), parameters...)"), + ("src/wallet/wallet.h", "WalletLogPrintf(const char* fmt, Params... parameters)"), + ("src/wallet/wallet.h", "LogPrintf((\"%s \" + std::string{fmt}).c_str(), GetDisplayName(), parameters...)"), + ("src/wallet/scriptpubkeyman.h", "WalletLogPrintf(const char* fmt, Params... parameters)"), + ("src/wallet/scriptpubkeyman.h", "LogPrintf((\"%s \" + std::string{fmt}).c_str(), m_storage.GetDisplayName(), parameters...)"), ] def parse_function_calls(function_name, source_code):