From e7333b420e35054d9302f9c58fd47b6152cbd35f Mon Sep 17 00:00:00 2001
From: Ben Woosley <ben.woosley@gmail.com>
Date: Mon, 6 Mar 2023 17:22:33 +0100
Subject: [PATCH 1/2] refactor: Extract util/exception from util/system

This is a minimal extraction of a single function, but also the only use
of std::exception in util/system.

The background of this commit is an ongoing effort to decouple the
libbitcoinkernel library from the ArgsManager defined in system.h.
Moving the function out of system.h allows including it from a separate
source file without including the ArgsManager definitions from system.h.
---
 ci/test/06_script_b.sh  |  1 +
 src/Makefile.am         |  3 +++
 src/bitcoin-cli.cpp     |  1 +
 src/bitcoin-tx.cpp      |  3 ++-
 src/bitcoin-util.cpp    |  1 +
 src/bitcoin-wallet.cpp  |  1 +
 src/bitcoind.cpp        |  1 +
 src/qt/bitcoin.cpp      |  1 +
 src/qt/guiutil.cpp      |  1 +
 src/qt/initexecutor.cpp |  2 +-
 src/util/exception.cpp  | 41 +++++++++++++++++++++++++++++++++++++++++
 src/util/exception.h    | 14 ++++++++++++++
 src/util/system.cpp     | 23 -----------------------
 src/util/system.h       |  4 ----
 src/util/thread.cpp     |  2 +-
 15 files changed, 69 insertions(+), 30 deletions(-)
 create mode 100644 src/util/exception.cpp
 create mode 100644 src/util/exception.h

diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh
index 7d47f6651c..7a01672d18 100755
--- a/ci/test/06_script_b.sh
+++ b/ci/test/06_script_b.sh
@@ -69,6 +69,7 @@ if [ "${RUN_TIDY}" = "true" ]; then
           " src/util/bytevectorhash.cpp"\
           " src/util/check.cpp"\
           " src/util/error.cpp"\
+          " src/util/exception.cpp"\
           " src/util/getuniquepath.cpp"\
           " src/util/hasher.cpp"\
           " src/util/message.cpp"\
diff --git a/src/Makefile.am b/src/Makefile.am
index 7dc5594cf2..a0c1670287 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -279,6 +279,7 @@ BITCOIN_CORE_H = \
   util/check.h \
   util/epochguard.h \
   util/error.h \
+  util/exception.h \
   util/fastrange.h \
   util/fees.h \
   util/getuniquepath.h \
@@ -699,6 +700,7 @@ libbitcoin_util_a_SOURCES = \
   util/bytevectorhash.cpp \
   util/check.cpp \
   util/error.cpp \
+  util/exception.cpp \
   util/fees.cpp \
   util/getuniquepath.cpp \
   util/hasher.cpp \
@@ -942,6 +944,7 @@ libbitcoinkernel_la_SOURCES = \
   txmempool.cpp \
   uint256.cpp \
   util/check.cpp \
+  util/exception.cpp \
   util/getuniquepath.cpp \
   util/hasher.cpp \
   util/moneystr.cpp \
diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
index 77613f389c..eb52b30ae4 100644
--- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@@ -19,6 +19,7 @@
 #include <rpc/request.h>
 #include <tinyformat.h>
 #include <univalue.h>
+#include <util/exception.h>
 #include <util/strencodings.h>
 #include <util/system.h>
 #include <util/translation.h>
diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp
index 57ca2bbe8a..f36b6054bb 100644
--- a/src/bitcoin-tx.cpp
+++ b/src/bitcoin-tx.cpp
@@ -12,14 +12,15 @@
 #include <consensus/amount.h>
 #include <consensus/consensus.h>
 #include <core_io.h>
-#include <key_io.h>
 #include <fs.h>
+#include <key_io.h>
 #include <policy/policy.h>
 #include <primitives/transaction.h>
 #include <script/script.h>
 #include <script/sign.h>
 #include <script/signingprovider.h>
 #include <univalue.h>
+#include <util/exception.h>
 #include <util/moneystr.h>
 #include <util/rbf.h>
 #include <util/strencodings.h>
diff --git a/src/bitcoin-util.cpp b/src/bitcoin-util.cpp
index 61d4b9c6f1..cf59ca9bff 100644
--- a/src/bitcoin-util.cpp
+++ b/src/bitcoin-util.cpp
@@ -14,6 +14,7 @@
 #include <compat/compat.h>
 #include <core_io.h>
 #include <streams.h>
+#include <util/exception.h>
 #include <util/system.h>
 #include <util/translation.h>
 #include <version.h>
diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp
index fcb845fe64..1ebe98b920 100644
--- a/src/bitcoin-wallet.cpp
+++ b/src/bitcoin-wallet.cpp
@@ -16,6 +16,7 @@
 #include <logging.h>
 #include <pubkey.h>
 #include <tinyformat.h>
+#include <util/exception.h>
 #include <util/system.h>
 #include <util/translation.h>
 #include <wallet/wallettool.h>
diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp
index b69913dddb..2ff34125e1 100644
--- a/src/bitcoind.cpp
+++ b/src/bitcoind.cpp
@@ -20,6 +20,7 @@
 #include <noui.h>
 #include <shutdown.h>
 #include <util/check.h>
+#include <util/exception.h>
 #include <util/strencodings.h>
 #include <util/syscall_sandbox.h>
 #include <util/syserror.h>
diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp
index 5244b72689..2c413e8b43 100644
--- a/src/qt/bitcoin.cpp
+++ b/src/qt/bitcoin.cpp
@@ -29,6 +29,7 @@
 #include <qt/utilitydialog.h>
 #include <qt/winshutdownmonitor.h>
 #include <uint256.h>
+#include <util/exception.h>
 #include <util/string.h>
 #include <util/system.h>
 #include <util/threadnames.h>
diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp
index 67c5295ae3..83c78d5c18 100644
--- a/src/qt/guiutil.cpp
+++ b/src/qt/guiutil.cpp
@@ -20,6 +20,7 @@
 #include <protocol.h>
 #include <script/script.h>
 #include <script/standard.h>
+#include <util/exception.h>
 #include <util/system.h>
 #include <util/time.h>
 
diff --git a/src/qt/initexecutor.cpp b/src/qt/initexecutor.cpp
index b63e7ec01c..bb3f970b55 100644
--- a/src/qt/initexecutor.cpp
+++ b/src/qt/initexecutor.cpp
@@ -5,7 +5,7 @@
 #include <qt/initexecutor.h>
 
 #include <interfaces/node.h>
-#include <util/system.h>
+#include <util/exception.h>
 #include <util/threadnames.h>
 
 #include <exception>
diff --git a/src/util/exception.cpp b/src/util/exception.cpp
new file mode 100644
index 0000000000..d961f0540f
--- /dev/null
+++ b/src/util/exception.cpp
@@ -0,0 +1,41 @@
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-2023 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/exception.h>
+
+#include <logging.h>
+#include <tinyformat.h>
+
+#include <exception>
+#include <iostream>
+#include <string>
+#include <typeinfo>
+
+#ifdef WIN32
+#include <windows.h>
+#endif // WIN32
+
+static std::string FormatException(const std::exception* pex, std::string_view thread_name)
+{
+#ifdef WIN32
+    char pszModule[MAX_PATH] = "";
+    GetModuleFileNameA(nullptr, pszModule, sizeof(pszModule));
+#else
+    const char* pszModule = "bitcoin";
+#endif
+    if (pex)
+        return strprintf(
+            "EXCEPTION: %s       \n%s       \n%s in %s       \n", typeid(*pex).name(), pex->what(), pszModule, thread_name);
+    else
+        return strprintf(
+            "UNKNOWN EXCEPTION       \n%s in %s       \n", pszModule, thread_name);
+}
+
+void PrintExceptionContinue(const std::exception* pex, std::string_view thread_name)
+{
+    std::string message = FormatException(pex, thread_name);
+    LogPrintf("\n\n************************\n%s\n", message);
+    tfm::format(std::cerr, "\n\n************************\n%s\n", message);
+}
diff --git a/src/util/exception.h b/src/util/exception.h
new file mode 100644
index 0000000000..946eb2644c
--- /dev/null
+++ b/src/util/exception.h
@@ -0,0 +1,14 @@
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-2023 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_EXCEPTION_H
+#define BITCOIN_UTIL_EXCEPTION_H
+
+#include <exception>
+#include <string_view>
+
+void PrintExceptionContinue(const std::exception* pex, std::string_view thread_name);
+
+#endif // BITCOIN_UTIL_EXCEPTION_H
diff --git a/src/util/system.cpp b/src/util/system.cpp
index 5b1a1659bf..98e89f82e7 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -792,29 +792,6 @@ std::string HelpMessageOpt(const std::string &option, const std::string &message
            std::string("\n\n");
 }
 
-static std::string FormatException(const std::exception* pex, std::string_view thread_name)
-{
-#ifdef WIN32
-    char pszModule[MAX_PATH] = "";
-    GetModuleFileNameA(nullptr, pszModule, sizeof(pszModule));
-#else
-    const char* pszModule = "bitcoin";
-#endif
-    if (pex)
-        return strprintf(
-            "EXCEPTION: %s       \n%s       \n%s in %s       \n", typeid(*pex).name(), pex->what(), pszModule, thread_name);
-    else
-        return strprintf(
-            "UNKNOWN EXCEPTION       \n%s in %s       \n", pszModule, thread_name);
-}
-
-void PrintExceptionContinue(const std::exception* pex, std::string_view thread_name)
-{
-    std::string message = FormatException(pex, thread_name);
-    LogPrintf("\n\n************************\n%s\n", message);
-    tfm::format(std::cerr, "\n\n************************\n%s\n", message);
-}
-
 fs::path GetDefaultDataDir()
 {
     // Windows: C:\Users\Username\AppData\Roaming\Bitcoin
diff --git a/src/util/system.h b/src/util/system.h
index f7bebe1f2a..2d07e64345 100644
--- a/src/util/system.h
+++ b/src/util/system.h
@@ -19,12 +19,10 @@
 #include <fs.h>
 #include <logging.h>
 #include <sync.h>
-#include <tinyformat.h>
 #include <util/settings.h>
 #include <util/time.h>
 
 #include <any>
-#include <exception>
 #include <map>
 #include <optional>
 #include <set>
@@ -52,8 +50,6 @@ bool error(const char* fmt, const Args&... args)
     return false;
 }
 
-void PrintExceptionContinue(const std::exception* pex, std::string_view thread_name);
-
 /**
  * Ensure file contents are fully committed to disk, using a platform-specific
  * feature analogous to fsync().
diff --git a/src/util/thread.cpp b/src/util/thread.cpp
index e9f611bc50..f380d29f7a 100644
--- a/src/util/thread.cpp
+++ b/src/util/thread.cpp
@@ -5,7 +5,7 @@
 #include <util/thread.h>
 
 #include <logging.h>
-#include <util/system.h>
+#include <util/exception.h>
 #include <util/threadnames.h>
 
 #include <exception>

From aaced5633b81b2f08b993106a527e2a0e1d663c1 Mon Sep 17 00:00:00 2001
From: Ben Woosley <ben.woosley@gmail.com>
Date: Mon, 6 Mar 2023 22:01:13 +0100
Subject: [PATCH 2/2] refactor: Move error() from util/system.h to logging.h

error is a low-level function with a sole dependency on LogPrintf, which
is defined in logging.h

The background of this commit is an ongoing effort to decouple the
libbitcoinkernel library from the ArgsManager defined in system.h.
Moving the function out of system.h allows including it from a separate
source file without including the ArgsManager definitions from system.h.
---
 src/addrdb.cpp                 | 1 +
 src/index/base.cpp             | 1 +
 src/index/coinstatsindex.cpp   | 1 +
 src/index/txindex.cpp          | 1 +
 src/kernel/coinstats.cpp       | 2 +-
 src/logging.h                  | 7 +++++++
 src/net.cpp                    | 3 ++-
 src/netbase.cpp                | 2 +-
 src/node/blockstorage.cpp      | 1 +
 src/script/signingprovider.cpp | 2 +-
 src/txdb.cpp                   | 2 +-
 src/util/system.h              | 7 -------
 12 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/addrdb.cpp b/src/addrdb.cpp
index 7be13c8f1e..9ae8244d1c 100644
--- a/src/addrdb.cpp
+++ b/src/addrdb.cpp
@@ -11,6 +11,7 @@
 #include <cstdint>
 #include <fs.h>
 #include <hash.h>
+#include <logging.h>
 #include <logging/timer.h>
 #include <netbase.h>
 #include <netgroup.h>
diff --git a/src/index/base.cpp b/src/index/base.cpp
index 7c570d4534..8a311296c2 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -6,6 +6,7 @@
 #include <index/base.h>
 #include <interfaces/chain.h>
 #include <kernel/chain.h>
+#include <logging.h>
 #include <node/blockstorage.h>
 #include <node/context.h>
 #include <node/database_args.h>
diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp
index 8cece7d78d..4d637e217a 100644
--- a/src/index/coinstatsindex.cpp
+++ b/src/index/coinstatsindex.cpp
@@ -7,6 +7,7 @@
 #include <crypto/muhash.h>
 #include <index/coinstatsindex.h>
 #include <kernel/coinstats.h>
+#include <logging.h>
 #include <node/blockstorage.h>
 #include <serialize.h>
 #include <txdb.h>
diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp
index 25ccc3e636..9095e7afeb 100644
--- a/src/index/txindex.cpp
+++ b/src/index/txindex.cpp
@@ -5,6 +5,7 @@
 #include <index/txindex.h>
 
 #include <index/disktxpos.h>
+#include <logging.h>
 #include <node/blockstorage.h>
 #include <util/system.h>
 #include <validation.h>
diff --git a/src/kernel/coinstats.cpp b/src/kernel/coinstats.cpp
index 82d7d8c46b..4b75c387a6 100644
--- a/src/kernel/coinstats.cpp
+++ b/src/kernel/coinstats.cpp
@@ -8,6 +8,7 @@
 #include <coins.h>
 #include <crypto/muhash.h>
 #include <hash.h>
+#include <logging.h>
 #include <node/blockstorage.h>
 #include <primitives/transaction.h>
 #include <script/script.h>
@@ -19,7 +20,6 @@
 #include <uint256.h>
 #include <util/check.h>
 #include <util/overflow.h>
-#include <util/system.h>
 #include <validation.h>
 #include <version.h>
 
diff --git a/src/logging.h b/src/logging.h
index 35fb598cef..954731d214 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -257,4 +257,11 @@ static inline void LogPrintf_(const std::string& logging_function, const std::st
         }                                                 \
     } while (0)
 
+template <typename... Args>
+bool error(const char* fmt, const Args&... args)
+{
+    LogPrintf("ERROR: %s\n", tfm::format(fmt, args...));
+    return false;
+}
+
 #endif // BITCOIN_LOGGING_H
diff --git a/src/net.cpp b/src/net.cpp
index 4e4f2f78be..d1895b7172 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -16,12 +16,13 @@
 #include <compat/compat.h>
 #include <consensus/consensus.h>
 #include <crypto/sha256.h>
-#include <node/eviction.h>
 #include <fs.h>
 #include <i2p.h>
+#include <logging.h>
 #include <net_permissions.h>
 #include <netaddress.h>
 #include <netbase.h>
+#include <node/eviction.h>
 #include <node/interface_ui.h>
 #include <protocol.h>
 #include <random.h>
diff --git a/src/netbase.cpp b/src/netbase.cpp
index 797f1e17f2..f39a3635f4 100644
--- a/src/netbase.cpp
+++ b/src/netbase.cpp
@@ -6,12 +6,12 @@
 #include <netbase.h>
 
 #include <compat/compat.h>
+#include <logging.h>
 #include <sync.h>
 #include <tinyformat.h>
 #include <util/sock.h>
 #include <util/strencodings.h>
 #include <util/string.h>
-#include <util/system.h>
 #include <util/time.h>
 
 #include <atomic>
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index c0a25572fc..255b73b347 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -11,6 +11,7 @@
 #include <flatfile.h>
 #include <fs.h>
 #include <hash.h>
+#include <logging.h>
 #include <pow.h>
 #include <reverse_iterator.h>
 #include <shutdown.h>
diff --git a/src/script/signingprovider.cpp b/src/script/signingprovider.cpp
index 5123dd81ac..ef055573b9 100644
--- a/src/script/signingprovider.cpp
+++ b/src/script/signingprovider.cpp
@@ -7,7 +7,7 @@
 #include <script/signingprovider.h>
 #include <script/standard.h>
 
-#include <util/system.h>
+#include <logging.h>
 
 const SigningProvider& DUMMY_SIGNING_PROVIDER = SigningProvider();
 
diff --git a/src/txdb.cpp b/src/txdb.cpp
index 7257fb4959..15351a4355 100644
--- a/src/txdb.cpp
+++ b/src/txdb.cpp
@@ -6,11 +6,11 @@
 #include <txdb.h>
 
 #include <chain.h>
+#include <logging.h>
 #include <pow.h>
 #include <random.h>
 #include <shutdown.h>
 #include <uint256.h>
-#include <util/system.h>
 #include <util/translation.h>
 #include <util/vector.h>
 
diff --git a/src/util/system.h b/src/util/system.h
index 2d07e64345..7292262bea 100644
--- a/src/util/system.h
+++ b/src/util/system.h
@@ -43,13 +43,6 @@ extern const char * const BITCOIN_SETTINGS_FILENAME;
 void SetupEnvironment();
 bool SetupNetworking();
 
-template<typename... Args>
-bool error(const char* fmt, const Args&... args)
-{
-    LogPrintf("ERROR: %s\n", tfm::format(fmt, args...));
-    return false;
-}
-
 /**
  * Ensure file contents are fully committed to disk, using a platform-specific
  * feature analogous to fsync().