Merge #17399: validation: Templatize ValidationState instead of subclassing

10efc0487c442bccb0e4a9ac29452af1592a3cf2 Templatize ValidationState instead of subclassing (Jeffrey Czyz)
10e85d4adc9b7dbbda63e00195e0a962f51e4d2c Remove ValidationState's constructor (Jeffrey Czyz)
0aed17ef2892478c28cd660e53223c6dd1dc0187 Refactor FormatStateMessage into ValidationState (Jeffrey Czyz)

Pull request description:

  This removes boilerplate code in the subclasses which otherwise only
  differ by the result type.

  The subclassing was introduced in a27a295.

ACKs for top commit:
  MarcoFalke:
    ACK 10efc0487c442bccb0e4a9ac29452af1592a3cf2 🐱
  ajtowns:
    ACK 10efc0487c442bccb0e4a9ac29452af1592a3cf2 -- looks good to me
  jonatack:
    ACK 10efc048 code review, build/tests green, nice cleanup

Tree-SHA512: 765dd52dde7d49b9a5c6d99d97c96f4492673e2aed0b0604faa88db0308fa4500a26bf755cca0b896be283874096c215932e1110a2d01dc012cd36a5fce58a42

15 files changed, 58 insertions, 125 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index eac7b38e03..0a1c45cf8c 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 8a3abb31f4..a79e7b9d12 100644
--- a/src/consensus/validation.h
+++ b/src/consensus/validation.h
@@ -16,7 +16,7 @@
   * provider of the transaction should be banned/ignored/disconnected/etc.
   */
 enum class TxValidationResult {
-    TX_RESULT_UNSET,         //!< initial value. Tx has not yet been rejected
+    TX_RESULT_UNSET = 0,     //!< initial value. Tx has not yet been rejected
     TX_CONSENSUS,            //!< invalid by consensus rules
     /**
      * Invalid by a change to consensus rules more recent than SegWit.
@@ -50,7 +50,7 @@ enum class TxValidationResult {
   * useful for some other use-cases.
   */
 enum class BlockValidationResult {
-    BLOCK_RESULT_UNSET,      //!< initial value. Block has not yet been rejected
+    BLOCK_RESULT_UNSET = 0,  //!< initial value. Block has not yet been rejected
     BLOCK_CONSENSUS,         //!< invalid by consensus rules (excluding any below reasons)
     /**
      * Invalid by a change to consensus rules more recent than SegWit.
@@ -71,31 +71,31 @@ enum class BlockValidationResult {
 
 
 
-/** Base class for capturing information about block/transaction validation. This is subclassed
+/** Template for capturing information about block/transaction validation. This is instantiated
  *  by TxValidationState and BlockValidationState for validation information on transactions
  *  and blocks respectively. */
+template <typename Result>
 class ValidationState {
 private:
     enum mode_state {
         MODE_VALID,   //!< everything ok
         MODE_INVALID, //!< network rule violation (DoS value may be set)
         MODE_ERROR,   //!< run-time error
-    } m_mode;
+    } m_mode{MODE_VALID};
+    Result m_result{};
     std::string m_reject_reason;
     std::string m_debug_message;
-protected:
-    void Invalid(const std::string &reject_reason="",
+public:
+    bool Invalid(Result result,
+                 const std::string &reject_reason="",
                  const std::string &debug_message="")
     {
+        m_result = result;
         m_reject_reason = reject_reason;
         m_debug_message = debug_message;
         if (m_mode != MODE_ERROR) m_mode = MODE_INVALID;
+        return false;
     }
-public:
-    // ValidationState is abstract. Have a pure virtual destructor.
-    virtual ~ValidationState() = 0;
-
-    ValidationState() : m_mode(MODE_VALID) {}
     bool Error(const std::string& reject_reason)
     {
         if (m_mode == MODE_VALID)
@@ -106,40 +106,25 @@ public:
     bool IsValid() const { return m_mode == MODE_VALID; }
     bool IsInvalid() const { return m_mode == MODE_INVALID; }
     bool IsError() const { return m_mode == MODE_ERROR; }
+    Result GetResult() const { return m_result; }
     std::string GetRejectReason() const { return m_reject_reason; }
     std::string GetDebugMessage() const { return m_debug_message; }
-};
+    std::string ToString() const
+    {
+        if (IsValid()) {
+            return "Valid";
+        }
 
-inline ValidationState::~ValidationState() {};
+        if (!m_debug_message.empty()) {
+            return m_reject_reason + ", " + m_debug_message;
+        }
 
-class TxValidationState : public ValidationState {
-private:
-    TxValidationResult m_result = TxValidationResult::TX_RESULT_UNSET;
-public:
-    bool Invalid(TxValidationResult result,
-                 const std::string &reject_reason="",
-                 const std::string &debug_message="")
-    {
-        m_result = result;
-        ValidationState::Invalid(reject_reason, debug_message);
-        return false;
+        return m_reject_reason;
     }
-    TxValidationResult GetResult() const { return m_result; }
 };
 
-class BlockValidationState : public ValidationState {
-private:
-    BlockValidationResult m_result = BlockValidationResult::BLOCK_RESULT_UNSET;
-public:
-    bool Invalid(BlockValidationResult result,
-                 const std::string &reject_reason="",
-                 const std::string &debug_message="") {
-        m_result = result;
-        ValidationState::Invalid(reject_reason, debug_message);
-        return false;
-    }
-    BlockValidationResult GetResult() const { return m_result; }
-};
+class TxValidationState : public ValidationState<TxValidationResult> {};
+class BlockValidationState : public ValidationState<BlockValidationResult> {};
 
 // These implement the weight = (stripped_size * 4) + witness_size formula,
 // using only serialization with and without witness data. As witness_size
diff --git a/src/init.cpp b/src/init.cpp
index ece6214aa8..1c9faec803 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -51,7 +51,6 @@
 #include <util/system.h>
 #include <util/threadnames.h>
 #include <util/translation.h>
-#include <util/validation.h>
 #include <util/asmap.h>
 #include <validation.h>
 #include <hash.h>
@@ -710,7 +709,7 @@ static void ThreadImport(std::vector<fs::path> 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 6f4e10b6ed..61d27d17c1 100644
--- a/src/miner.cpp
+++ b/src/miner.cpp
@@ -20,7 +20,6 @@
 #include <timedata.h>
 #include <util/moneystr.h>
 #include <util/system.h>
-#include <util/validation.h>
 
 #include <algorithm>
 #include <utility>
@@ -167,7 +166,7 @@ std::unique_ptr<CBlockTemplate> 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 1c1046b6ff..fa09b60630 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -26,7 +26,6 @@
 #include <txmempool.h>
 #include <util/system.h>
 #include <util/strencodings.h>
-#include <util/validation.h>
 
 #include <memory>
 #include <typeinfo>
@@ -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 1bb9b88d00..201406ce3b 100644
--- a/src/node/transaction.cpp
+++ b/src/node/transaction.cpp
@@ -7,7 +7,6 @@
 #include <net.h>
 #include <net_processing.h>
 #include <node/context.h>
-#include <util/validation.h>
 #include <validation.h>
 #include <validationinterface.h>
 #include <node/transaction.h>
@@ -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 9b06aba22b..40637a552e 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -31,7 +31,6 @@
 #include <undo.h>
 #include <util/strencodings.h>
 #include <util/system.h>
-#include <util/validation.h>
 #include <validation.h>
 #include <validationinterface.h>
 #include <warnings.h>
@@ -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 ab5d830b2a..5bfdcd6555 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -28,7 +28,6 @@
 #include <util/fees.h>
 #include <util/strencodings.h>
 #include <util/system.h>
-#include <util/validation.h>
 #include <validation.h>
 #include <validationinterface.h>
 #include <versionbitsinfo.h>
@@ -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 360377e58a..fc736bc3a1 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -25,7 +25,6 @@
 #include <util/strencodings.h>
 #include <util/time.h>
 #include <util/translation.h>
-#include <util/validation.h>
 #include <validation.h>
 #include <validationinterface.h>
 
@@ -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 ffbee21aeb..0000000000
--- 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 <util/validation.h>
-
-#include <consensus/validation.h>
-#include <tinyformat.h>
-
-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 5ee260a055..0000000000
--- 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 <string>
-
-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 bab04b8e34..c0327c39bc 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -43,7 +43,6 @@
 #include <util/strencodings.h>
 #include <util/system.h>
 #include <util/translation.h>
-#include <util/validation.h>
 #include <validationinterface.h>
 #include <warnings.h>
 
@@ -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<const CBlock>& 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<cons
         }
         if (!ret) {
             GetMainSignals().BlockChecked(*pblock, state);
-            return error("%s: AcceptBlock FAILED (%s)", __func__, FormatStateMessage(state));
+            return error("%s: AcceptBlock FAILED (%s)", __func__, state.ToString());
         }
     }
 
@@ -3824,7 +3823,7 @@ bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<cons
 
     BlockValidationState state; // Only used to report errors, not invalidity - ignore it
     if (!::ChainstateActive().ActivateBestChain(state, chainparams, pblock))
-        return error("%s: ActivateBestChain failed (%s)", __func__, FormatStateMessage(state));
+        return error("%s: ActivateBestChain failed (%s)", __func__, state.ToString());
 
     return true;
 }
@@ -3842,11 +3841,11 @@ bool TestBlockValidity(BlockValidationState& state, const CChainParams& chainpar
 
     // NOTE: CheckBlockHeader is called by CheckBlock
     if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime()))
-        return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, FormatStateMessage(state));
+        return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, state.ToString());
     if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot))
-        return error("%s: Consensus::CheckBlock: %s", __func__, FormatStateMessage(state));
+        return error("%s: Consensus::CheckBlock: %s", __func__, state.ToString());
     if (!ContextualCheckBlock(block, state, chainparams.GetConsensus(), pindexPrev))
-        return error("%s: Consensus::ContextualCheckBlock: %s", __func__, FormatStateMessage(state));
+        return error("%s: Consensus::ContextualCheckBlock: %s", __func__, state.ToString());
     if (!::ChainstateActive().ConnectBlock(block, state, &indexDummy, viewNew, chainparams, true))
         return false;
     assert(state.IsValid());
@@ -3944,7 +3943,7 @@ void PruneBlockFilesManual(int nManualPruneHeight)
     const CChainParams& chainparams = Params();
     if (!::ChainstateActive().FlushStateToDisk(
             chainparams, state, FlushStateMode::NONE, nManualPruneHeight)) {
-        LogPrintf("%s: failed to flush state (%s)\n", __func__, FormatStateMessage(state));
+        LogPrintf("%s: failed to flush state (%s)\n", __func__, state.ToString());
     }
 }
 
@@ -4262,7 +4261,7 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview,
         // check level 1: verify block validity
         if (nCheckLevel >= 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 0f513c065f..1deb93c972 100644
--- a/src/validationinterface.cpp
+++ b/src/validationinterface.cpp
@@ -11,7 +11,6 @@
 #include <primitives/block.h>
 #include <primitives/transaction.h>
 #include <scheduler.h>
-#include <util/validation.h>
 
 #include <future>
 #include <unordered_map>
@@ -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 b93b9ef1bc..486a6b8bf1 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -12,7 +12,6 @@
 #include <util/moneystr.h>
 #include <util/rbf.h>
 #include <util/system.h>
-#include <util/validation.h>
 
 //! 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 02fe59b601..5a97f62b71 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -27,7 +27,6 @@
 #include <util/moneystr.h>
 #include <util/rbf.h>
 #include <util/translation.h>
-#include <util/validation.h>
 #include <wallet/coincontrol.h>
 #include <wallet/fees.h>