Merge bitcoin/bitcoin#22270: test: Add bitcoin-util tests (+refactors)

fa4017e refactor: Pass grind args vector as const reference (MarcoFalke)
fa08bc2 Remove gArgs from AppInitUtil (MarcoFalke)
fa751a4 Remove unused OptionsCategory arg from AddCommand (MarcoFalke)
fa3c1ee Remove unused includes from bitcoin-util (MarcoFalke)
fa30492 test: Add bitcoin-util tests (MarcoFalke)
fa831e7 build_msvc: Add bitcoin-util.exe (MarcoFalke)

Pull request description:

  bitcoin-util has no tests

  See https://marcofalke.github.io/btc_cov/total.coverage/src/bitcoin-util.cpp.gcov.html (Coverage report showing 0%)

ACKs for top commit:
  klementtan:
    Code review and tested ACK fa4017e
  theStack:
    Tested ACK fa4017e
  jamesob:
    reACK fa4017e ([`jamesob/ackr/22270.1.MarcoFalke.test_add_bitcoin_util_te`](https://github.com/jamesob/bitcoin/tree/ackr/22270.1.MarcoFalke.test_add_bitcoin_util_te))

Tree-SHA512: 68e2791239bc48d28fbb6394155c39ea0357a96ec7e4896ca579feeef1a803657165a0ef9fa3cf6e2a381e5b0ca0dafa1b594158303a04997db784201d8dd66d

Author: MarcoFalke

build_msvc/bitcoin-util/bitcoin-util.vcxproj

@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="..\common.init.vcxproj" />
+  <PropertyGroup Label="Globals">
+    <ProjectGuid>{D3022AF6-AD33-4CE3-B358-87CB6A1B29CF}</ProjectGuid>
+  </PropertyGroup>
+  <PropertyGroup Label="Configuration">
+    <ConfigurationType>Application</ConfigurationType>
+    <OutDir>$(SolutionDir)$(Platform)\$(Configuration)\</OutDir>
+  </PropertyGroup>
+  <ItemGroup>
+    <ClCompile Include="..\..\src\bitcoin-util.cpp" />
+  </ItemGroup>
+  <ItemGroup>
+    <ProjectReference Include="..\libbitcoinconsensus\libbitcoinconsensus.vcxproj">
+      <Project>{2b384fa8-9ee1-4544-93cb-0d733c25e8ce}</Project>
+    </ProjectReference>
+    <ProjectReference Include="..\libbitcoin_common\libbitcoin_common.vcxproj">
+      <Project>{7c87e378-df58-482e-aa2f-1bc129bc19ce}</Project>
+    </ProjectReference>
+    <ProjectReference Include="..\libbitcoin_crypto\libbitcoin_crypto.vcxproj">
+      <Project>{6190199c-6cf4-4dad-bfbd-93fa72a760c1}</Project>
+    </ProjectReference>
+    <ProjectReference Include="..\libbitcoin_util\libbitcoin_util.vcxproj">
+      <Project>{b53a5535-ee9d-4c6f-9a26-f79ee3bc3754}</Project>
+    </ProjectReference>
+    <ProjectReference Include="..\libunivalue\libunivalue.vcxproj">
+      <Project>{5724ba7d-a09a-4ba8-800b-c4c1561b3d69}</Project>
+    </ProjectReference>
+    <ProjectReference Include="..\libsecp256k1\libsecp256k1.vcxproj">
+      <Project>{bb493552-3b8c-4a8c-bf69-a6e7a51d2ea6}</Project>
+    </ProjectReference>
+  </ItemGroup>
+  <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" />
+  <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
+  <Import Project="..\common.vcxproj" />
+</Project>

build_msvc/bitcoin.sln

@@ -32,6 +32,8 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "bench_bitcoin", "bench_bitc
 EndProject
 Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "bitcoin-tx", "bitcoin-tx\bitcoin-tx.vcxproj", "{D3022AF6-AD33-4CE3-B358-87CB6A1B29CF}"
 EndProject
+Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "bitcoin-util", "bitcoin-util\bitcoin-util.vcxproj", "{D3022AF6-AD33-4CE3-B358-87CB6A1B29CF}"
+EndProject
 Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "bitcoin-wallet", "bitcoin-wallet\bitcoin-wallet.vcxproj", "{84DE8790-EDE3-4483-81AC-C32F15E861F4}"
 EndProject
 Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "libbitcoin_wallet_tool", "libbitcoin_wallet_tool\libbitcoin_wallet_tool.vcxproj", "{F91AC55E-6F5E-4C58-9AC5-B40DB7DEEF93}"

src/bitcoin-util.cpp

@@ -7,28 +7,19 @@
 #endif
 
 #include <arith_uint256.h>
+#include <chain.h>
+#include <chainparams.h>
+#include <chainparamsbase.h>
 #include <clientversion.h>
-#include <coins.h>
-#include <consensus/consensus.h>
 #include <core_io.h>
-#include <key_io.h>
-#include <policy/rbf.h>
-#include <primitives/transaction.h>
-#include <script/script.h>
-#include <script/sign.h>
-#include <script/signingprovider.h>
-#include <univalue.h>
-#include <util/moneystr.h>
-#include <util/rbf.h>
-#include <util/strencodings.h>
-#include <util/string.h>
+#include <streams.h>
 #include <util/system.h>
 #include <util/translation.h>
 
 #include <atomic>
+#include <cstdio>
 #include <functional>
 #include <memory>
-#include <stdio.h>
 #include <thread>
 
 #include <boost/algorithm/string.hpp>
@@ -43,29 +34,29 @@ static void SetupBitcoinUtilArgs(ArgsManager &argsman)
 
     argsman.AddArg("-version", "Print version and exit", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 
-    argsman.AddCommand("grind", "Perform proof of work on hex header string", OptionsCategory::COMMANDS);
+    argsman.AddCommand("grind", "Perform proof of work on hex header string");
 
     SetupChainParamsBaseOptions(argsman);
 }
 
 // This function returns either one of EXIT_ codes when it's expected to stop the process or
 // CONTINUE_EXECUTION when it's expected to continue further.
-static int AppInitUtil(int argc, char* argv[])
+static int AppInitUtil(ArgsManager& args, int argc, char* argv[])
 {
-    SetupBitcoinUtilArgs(gArgs);
+    SetupBitcoinUtilArgs(args);
     std::string error;
-    if (!gArgs.ParseParameters(argc, argv, error)) {
+    if (!args.ParseParameters(argc, argv, error)) {
         tfm::format(std::cerr, "Error parsing command line arguments: %s\n", error);
         return EXIT_FAILURE;
     }
 
-    if (HelpRequested(gArgs) || gArgs.IsArgSet("-version")) {
+    if (HelpRequested(args) || args.IsArgSet("-version")) {
         // First part of help message is specific to this utility
         std::string strUsage = PACKAGE_NAME " bitcoin-util utility version " + FormatFullVersion() + "\n";
-        if (!gArgs.IsArgSet("-version")) {
+        if (!args.IsArgSet("-version")) {
             strUsage += "\n"
                 "Usage:  bitcoin-util [options] [commands]  Do stuff\n";
-            strUsage += "\n" + gArgs.GetHelpMessage();
+            strUsage += "\n" + args.GetHelpMessage();
         }
 
         tfm::format(std::cout, "%s", strUsage);
@@ -79,7 +70,7 @@ static int AppInitUtil(int argc, char* argv[])
 
     // Check for chain settings (Params() calls are only valid after this clause)
     try {
-        SelectParams(gArgs.GetChainName());
+        SelectParams(args.GetChainName());
     } catch (const std::exception& e) {
         tfm::format(std::cerr, "Error: %s\n", e.what());
         return EXIT_FAILURE;
@@ -114,7 +105,7 @@ static void grind_task(uint32_t nBits, CBlockHeader& header_orig, uint32_t offse
     }
 }
 
-static int Grind(std::vector<std::string> args, std::string& strPrint)
+static int Grind(const std::vector<std::string>& args, std::string& strPrint)
 {
     if (args.size() != 1) {
         strPrint = "Must specify block header to grind";
@@ -160,12 +151,14 @@ __declspec(dllexport) int main(int argc, char* argv[])
 int main(int argc, char* argv[])
 #endif
 {
+    ArgsManager& args = gArgs;
     SetupEnvironment();
 
     try {
-        int ret = AppInitUtil(argc, argv);
-        if (ret != CONTINUE_EXECUTION)
+        int ret = AppInitUtil(args, argc, argv);
+        if (ret != CONTINUE_EXECUTION) {
             return ret;
+        }
     } catch (const std::exception& e) {
         PrintExceptionContinue(&e, "AppInitUtil()");
         return EXIT_FAILURE;
@@ -174,7 +167,7 @@ int main(int argc, char* argv[])
         return EXIT_FAILURE;
     }
 
-    const auto cmd = gArgs.GetCommand();
+    const auto cmd = args.GetCommand();
     if (!cmd) {
         tfm::format(std::cerr, "Error: must specify a command\n");
         return EXIT_FAILURE;

src/bitcoin-wallet.cpp

@@ -33,11 +33,11 @@ static void SetupWalletToolArgs(ArgsManager& argsman)
     argsman.AddArg("-format=<format>", "The format of the wallet file to create. Either \"bdb\" or \"sqlite\". Only used with 'createfromdump'", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -debug is true, 0 otherwise).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
 
-    argsman.AddCommand("info", "Get wallet info", OptionsCategory::COMMANDS);
-    argsman.AddCommand("create", "Create new wallet file", OptionsCategory::COMMANDS);
-    argsman.AddCommand("salvage", "Attempt to recover private keys from a corrupt wallet. Warning: 'salvage' is experimental.", OptionsCategory::COMMANDS);
-    argsman.AddCommand("dump", "Print out all of the wallet key-value records", OptionsCategory::COMMANDS);
-    argsman.AddCommand("createfromdump", "Create new wallet file from dumped records", OptionsCategory::COMMANDS);
+    argsman.AddCommand("info", "Get wallet info");
+    argsman.AddCommand("create", "Create new wallet file");
+    argsman.AddCommand("salvage", "Attempt to recover private keys from a corrupt wallet. Warning: 'salvage' is experimental.");
+    argsman.AddCommand("dump", "Print out all of the wallet key-value records");
+    argsman.AddCommand("createfromdump", "Create new wallet file from dumped records");
 }
 
 static bool WalletAppInit(ArgsManager& args, int argc, char* argv[])

src/util/system.cpp

@@ -620,14 +620,14 @@ void ArgsManager::ForceSetArg(const std::string& strArg, const std::string& strV
     m_settings.forced_settings[SettingName(strArg)] = strValue;
 }
 
-void ArgsManager::AddCommand(const std::string& cmd, const std::string& help, const OptionsCategory& cat)
+void ArgsManager::AddCommand(const std::string& cmd, const std::string& help)
 {
     Assert(cmd.find('=') == std::string::npos);
     Assert(cmd.at(0) != '-');
 
     LOCK(cs_args);
     m_accept_any_command = false; // latch to false
-    std::map<std::string, Arg>& arg_map = m_available_args[cat];
+    std::map<std::string, Arg>& arg_map = m_available_args[OptionsCategory::COMMANDS];
     auto ret = arg_map.emplace(cmd, Arg{"", help, ArgsManager::COMMAND});
     Assert(ret.second); // Fail on duplicate commands
 }

src/util/system.h

@@ -374,7 +374,7 @@ class ArgsManager
     /**
      * Add subcommand
      */
-    void AddCommand(const std::string& cmd, const std::string& help, const OptionsCategory& cat);
+    void AddCommand(const std::string& cmd, const std::string& help);
 
     /**
      * Add many hidden arguments

test/util/data/bitcoin-util-test.json

@@ -1,4 +1,34 @@
 [
+  { "exec": "./bitcoin-util",
+    "args": ["foo"],
+    "return_code": 1,
+    "error_txt": "Error parsing command line arguments: Invalid command 'foo'",
+    "description": ""
+  },
+  { "exec": "./bitcoin-util",
+    "args": ["help"],
+    "return_code": 1,
+    "error_txt": "Error parsing command line arguments: Invalid command 'help'",
+    "description": "`help` raises an error. Use `-help`"
+  },
+  { "exec": "./bitcoin-util",
+    "args": ["grind"],
+    "return_code": 1,
+    "error_txt": "Must specify block header to grind",
+    "description": ""
+  },
+  { "exec": "./bitcoin-util",
+    "args": ["grind", "1", "2"],
+    "return_code": 1,
+    "error_txt": "Must specify block header to grind",
+    "description": ""
+  },
+  { "exec": "./bitcoin-util",
+    "args": ["grind", "aa"],
+    "return_code": 1,
+    "error_txt": "Could not decode block header",
+    "description": ""
+  },
   { "exec": "./bitcoin-tx",
     "args": ["-create", "nversion=1"],
     "output_cmp": "blanktxv1.hex",