From 0aed17ef2892478c28cd660e53223c6dd1dc0187 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 8 Nov 2019 13:22:36 -0800 Subject: [PATCH] Refactor FormatStateMessage into ValidationState --- src/Makefile.am | 2 -- src/consensus/validation.h | 12 +++++++++ src/init.cpp | 3 +-- src/miner.cpp | 3 +-- src/net_processing.cpp | 7 +++--- src/node/transaction.cpp | 3 +-- src/rpc/blockchain.cpp | 7 +++--- src/rpc/mining.cpp | 5 ++-- src/test/util/setup_common.cpp | 3 +-- src/util/validation.cpp | 23 ----------------- src/util/validation.h | 16 ------------ src/validation.cpp | 45 +++++++++++++++++----------------- src/validationinterface.cpp | 3 +-- src/wallet/feebumper.cpp | 1 - src/wallet/wallet.cpp | 1 - 15 files changed, 47 insertions(+), 87 deletions(-) delete mode 100644 src/util/validation.cpp delete mode 100644 src/util/validation.h diff --git a/src/Makefile.am b/src/Makefile.am index eac7b38e03e..0a1c45cf8ce 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -229,7 +229,6 @@ BITCOIN_CORE_H = \ util/time.h \ util/translation.h \ util/url.h \ - util/validation.h \ util/vector.h \ validation.h \ validationinterface.h \ @@ -528,7 +527,6 @@ libbitcoin_util_a_SOURCES = \ util/string.cpp \ util/time.cpp \ util/url.cpp \ - util/validation.cpp \ $(BITCOIN_CORE_H) if GLIBC_BACK_COMPAT diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 8a3abb31f45..4ead26a416d 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -108,6 +108,18 @@ public: bool IsError() const { return m_mode == MODE_ERROR; } std::string GetRejectReason() const { return m_reject_reason; } std::string GetDebugMessage() const { return m_debug_message; } + std::string ToString() const + { + if (IsValid()) { + return "Valid"; + } + + if (!m_debug_message.empty()) { + return m_reject_reason + ", " + m_debug_message; + } + + return m_reject_reason; + } }; inline ValidationState::~ValidationState() {}; diff --git a/src/init.cpp b/src/init.cpp index ece6214aa8f..1c9faec8037 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -51,7 +51,6 @@ #include #include #include -#include #include #include #include @@ -710,7 +709,7 @@ static void ThreadImport(std::vector vImportFiles) // scan for better chains in the block chain database, that are not yet connected in the active best chain BlockValidationState state; if (!ActivateBestChain(state, chainparams)) { - LogPrintf("Failed to connect best block (%s)\n", FormatStateMessage(state)); + LogPrintf("Failed to connect best block (%s)\n", state.ToString()); StartShutdown(); return; } diff --git a/src/miner.cpp b/src/miner.cpp index 6f4e10b6ed6..61d27d17c16 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -20,7 +20,6 @@ #include #include #include -#include #include #include @@ -167,7 +166,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc BlockValidationState state; if (!TestBlockValidity(state, chainparams, *pblock, pindexPrev, false, false)) { - throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, FormatStateMessage(state))); + throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, state.ToString())); } int64_t nTime2 = GetTimeMicros(); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1c1046b6ff8..fa09b60630a 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -26,7 +26,6 @@ #include #include #include -#include #include #include @@ -1432,7 +1431,7 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c if (need_activate_chain) { BlockValidationState state; if (!ActivateBestChain(state, Params(), a_recent_block)) { - LogPrint(BCLog::NET, "failed to activate chain (%s)\n", FormatStateMessage(state)); + LogPrint(BCLog::NET, "failed to activate chain (%s)\n", state.ToString()); } } @@ -2342,7 +2341,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } BlockValidationState state; if (!ActivateBestChain(state, Params(), a_recent_block)) { - LogPrint(BCLog::NET, "failed to activate chain (%s)\n", FormatStateMessage(state)); + LogPrint(BCLog::NET, "failed to activate chain (%s)\n", state.ToString()); } } @@ -2636,7 +2635,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr { LogPrint(BCLog::MEMPOOLREJ, "%s from peer=%d was not accepted: %s\n", tx.GetHash().ToString(), pfrom->GetId(), - FormatStateMessage(state)); + state.ToString()); MaybePunishNodeForTx(pfrom->GetId(), state); } return true; diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 1bb9b88d007..201406ce3b6 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include @@ -41,7 +40,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t TxValidationState state; if (!AcceptToMemoryPool(*node.mempool, state, std::move(tx), nullptr /* plTxnReplaced */, false /* bypass_limits */, max_tx_fee)) { - err_string = FormatStateMessage(state); + err_string = state.ToString(); if (state.IsInvalid()) { if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { return TransactionError::MISSING_INPUTS; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 9b06aba22ba..40637a552e2 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -31,7 +31,6 @@ #include #include #include -#include #include #include #include @@ -1486,7 +1485,7 @@ static UniValue preciousblock(const JSONRPCRequest& request) PreciousBlock(state, Params(), pblockindex); if (!state.IsValid()) { - throw JSONRPCError(RPC_DATABASE_ERROR, FormatStateMessage(state)); + throw JSONRPCError(RPC_DATABASE_ERROR, state.ToString()); } return NullUniValue; @@ -1524,7 +1523,7 @@ static UniValue invalidateblock(const JSONRPCRequest& request) } if (!state.IsValid()) { - throw JSONRPCError(RPC_DATABASE_ERROR, FormatStateMessage(state)); + throw JSONRPCError(RPC_DATABASE_ERROR, state.ToString()); } return NullUniValue; @@ -1561,7 +1560,7 @@ static UniValue reconsiderblock(const JSONRPCRequest& request) ActivateBestChain(state, Params()); if (!state.IsValid()) { - throw JSONRPCError(RPC_DATABASE_ERROR, FormatStateMessage(state)); + throw JSONRPCError(RPC_DATABASE_ERROR, state.ToString()); } return NullUniValue; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index ab5d830b2a0..5bfdcd6555f 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -28,7 +28,6 @@ #include #include #include -#include #include #include #include @@ -307,7 +306,7 @@ static UniValue BIP22ValidationResult(const BlockValidationState& state) return NullUniValue; if (state.IsError()) - throw JSONRPCError(RPC_VERIFY_ERROR, FormatStateMessage(state)); + throw JSONRPCError(RPC_VERIFY_ERROR, state.ToString()); if (state.IsInvalid()) { std::string strRejectReason = state.GetRejectReason(); @@ -823,7 +822,7 @@ static UniValue submitheader(const JSONRPCRequest& request) ProcessNewBlockHeaders({h}, state, Params()); if (state.IsValid()) return NullUniValue; if (state.IsError()) { - throw JSONRPCError(RPC_VERIFY_ERROR, FormatStateMessage(state)); + throw JSONRPCError(RPC_VERIFY_ERROR, state.ToString()); } throw JSONRPCError(RPC_VERIFY_ERROR, state.GetRejectReason()); } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 360377e58ac..fc736bc3a1b 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include @@ -123,7 +122,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha BlockValidationState state; if (!ActivateBestChain(state, chainparams)) { - throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", FormatStateMessage(state))); + throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", state.ToString())); } // Start script-checking threads. Set g_parallel_script_checks to true so they are used. diff --git a/src/util/validation.cpp b/src/util/validation.cpp deleted file mode 100644 index ffbee21aebf..00000000000 --- a/src/util/validation.cpp +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2020 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include - -#include -#include - -std::string FormatStateMessage(const ValidationState &state) -{ - if (state.IsValid()) { - return "Valid"; - } - - const std::string debug_message = state.GetDebugMessage(); - if (!debug_message.empty()) { - return strprintf("%s, %s", state.GetRejectReason(), debug_message); - } - - return state.GetRejectReason(); -} diff --git a/src/util/validation.h b/src/util/validation.h deleted file mode 100644 index 5ee260a055e..00000000000 --- a/src/util/validation.h +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2019 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#ifndef BITCOIN_UTIL_VALIDATION_H -#define BITCOIN_UTIL_VALIDATION_H - -#include - -class ValidationState; - -/** Convert ValidationState to a human-readable message for logging */ -std::string FormatStateMessage(const ValidationState &state); - -#endif // BITCOIN_UTIL_VALIDATION_H diff --git a/src/validation.cpp b/src/validation.cpp index bab04b8e343..c0327c39bc6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -43,7 +43,6 @@ #include #include #include -#include #include #include @@ -662,7 +661,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) CAmount nFees = 0; if (!Consensus::CheckTxInputs(tx, state, m_view, GetSpendHeight(m_view), nFees)) { - return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); + return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), state.ToString()); } // Check for non-standard pay-to-script-hash in inputs @@ -951,7 +950,7 @@ bool MemPoolAccept::ConsensusScriptChecks(ATMPArgs& args, Workspace& ws, Precomp unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(::ChainActive().Tip(), chainparams.GetConsensus()); if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags, txdata)) { return error("%s: BUG! PLEASE REPORT THIS! CheckInputScripts failed against latest-block but not STANDARD flags %s, %s", - __func__, hash.ToString(), FormatStateMessage(state)); + __func__, hash.ToString(), state.ToString()); } return true; @@ -1921,7 +1920,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, // problems. return AbortNode(state, "Corrupt block found indicating potential hardware failure; shutting down"); } - return error("%s: Consensus::CheckBlock: %s", __func__, FormatStateMessage(state)); + return error("%s: Consensus::CheckBlock: %s", __func__, state.ToString()); } // verify that the view's current state corresponds to the previous block @@ -2099,7 +2098,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, // Any transaction validation failure in ConnectBlock is a block consensus failure state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(), tx_state.GetDebugMessage()); - return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); + return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), state.ToString()); } nFees += txfee; if (!MoneyRange(nFees)) { @@ -2142,7 +2141,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(), tx_state.GetDebugMessage()); return error("ConnectBlock(): CheckInputScripts on %s failed with %s", - tx.GetHash().ToString(), FormatStateMessage(state)); + tx.GetHash().ToString(), state.ToString()); } control.Add(vChecks); } @@ -2359,7 +2358,7 @@ void CChainState::ForceFlushStateToDisk() { BlockValidationState state; const CChainParams& chainparams = Params(); if (!this->FlushStateToDisk(chainparams, state, FlushStateMode::ALWAYS)) { - LogPrintf("%s: failed to flush state (%s)\n", __func__, FormatStateMessage(state)); + LogPrintf("%s: failed to flush state (%s)\n", __func__, state.ToString()); } } @@ -2369,7 +2368,7 @@ void CChainState::PruneAndFlush() { const CChainParams& chainparams = Params(); if (!this->FlushStateToDisk(chainparams, state, FlushStateMode::NONE)) { - LogPrintf("%s: failed to flush state (%s)\n", __func__, FormatStateMessage(state)); + LogPrintf("%s: failed to flush state (%s)\n", __func__, state.ToString()); } } @@ -2596,7 +2595,7 @@ bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& ch if (!rv) { if (state.IsInvalid()) InvalidBlockFound(pindexNew, state); - return error("%s: ConnectBlock %s failed, %s", __func__, pindexNew->GetBlockHash().ToString(), FormatStateMessage(state)); + return error("%s: ConnectBlock %s failed, %s", __func__, pindexNew->GetBlockHash().ToString(), state.ToString()); } nTime3 = GetTimeMicros(); nTimeConnectTotal += nTime3 - nTime2; LogPrint(BCLog::BENCH, " - Connect total: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime3 - nTime2) * MILLI, nTimeConnectTotal * MICRO, nTimeConnectTotal * MILLI / nBlocksTotal); @@ -3601,7 +3600,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS } if (!CheckBlockHeader(block, state, chainparams.GetConsensus())) - return error("%s: Consensus::CheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); + return error("%s: Consensus::CheckBlockHeader: %s, %s", __func__, hash.ToString(), state.ToString()); // Get prev block index CBlockIndex* pindexPrev = nullptr; @@ -3616,7 +3615,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk"); } if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime())) - return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); + return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), state.ToString()); /* Determine if this block descends from any block which has been found * invalid (m_failed_blocks), then mark pindexPrev and any blocks between @@ -3766,7 +3765,7 @@ bool CChainState::AcceptBlock(const std::shared_ptr& pblock, Block pindex->nStatus |= BLOCK_FAILED_VALID; setDirtyBlockIndex.insert(pindex); } - return error("%s: %s", __func__, FormatStateMessage(state)); + return error("%s: %s", __func__, state.ToString()); } // Header is valid/has work, merkle tree and segwit merkle tree are good...RELAY NOW @@ -3816,7 +3815,7 @@ bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr= 1 && !CheckBlock(block, state, chainparams.GetConsensus())) return error("%s: *** found bad block at %d, hash=%s (%s)\n", __func__, - pindex->nHeight, pindex->GetBlockHash().ToString(), FormatStateMessage(state)); + pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString()); // check level 2: verify undo validity if (nCheckLevel >= 2 && pindex) { CBlockUndo undo; @@ -4311,7 +4310,7 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, if (!ReadBlockFromDisk(block, pindex, chainparams.GetConsensus())) return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); if (!::ChainstateActive().ConnectBlock(block, state, pindex, coins, chainparams)) - return error("VerifyDB(): *** found unconnectable block at %d, hash=%s (%s)", pindex->nHeight, pindex->GetBlockHash().ToString(), FormatStateMessage(state)); + return error("VerifyDB(): *** found unconnectable block at %d, hash=%s (%s)", pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString()); } } @@ -4499,7 +4498,7 @@ bool CChainState::RewindBlockIndex(const CChainParams& params) // Disconnect block if (!DisconnectTip(state, params, nullptr)) { - return error("RewindBlockIndex: unable to disconnect block at height %i (%s)", tip->nHeight, FormatStateMessage(state)); + return error("RewindBlockIndex: unable to disconnect block at height %i (%s)", tip->nHeight, state.ToString()); } // Reduce validity flag and have-data flags. @@ -4519,7 +4518,7 @@ bool CChainState::RewindBlockIndex(const CChainParams& params) // Occasionally flush state to disk. if (!FlushStateToDisk(params, state, FlushStateMode::PERIODIC)) { - LogPrintf("RewindBlockIndex: unable to flush state to disk (%s)\n", FormatStateMessage(state)); + LogPrintf("RewindBlockIndex: unable to flush state to disk (%s)\n", state.ToString()); return false; } } @@ -4550,7 +4549,7 @@ bool RewindBlockIndex(const CChainParams& params) { // it'll get called a bunch real soon. BlockValidationState state; if (!::ChainstateActive().FlushStateToDisk(params, state, FlushStateMode::ALWAYS)) { - LogPrintf("RewindBlockIndex: unable to flush state to disk (%s)\n", FormatStateMessage(state)); + LogPrintf("RewindBlockIndex: unable to flush state to disk (%s)\n", state.ToString()); return false; } } diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 0f513c065f9..1deb93c9727 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include @@ -193,7 +192,7 @@ void CMainSignals::ChainStateFlushed(const CBlockLocator &locator) { void CMainSignals::BlockChecked(const CBlock& block, const BlockValidationState& state) { LOG_EVENT("%s: block hash=%s state=%s", __func__, - block.GetHash().ToString(), FormatStateMessage(state)); + block.GetHash().ToString(), state.ToString()); m_internals->BlockChecked(block, state); } diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index b93b9ef1bca..486a6b8bf11 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -12,7 +12,6 @@ #include #include #include -#include //! Check whether transaction has descendant in wallet or mempool, or has been //! mined, or conflicts with a mined transaction. Return a feebumper::Result. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 02fe59b601d..5a97f62b71b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -27,7 +27,6 @@ #include #include #include -#include #include #include