Merge bitcoin/bitcoin#27765: test: Throw error when -signetchallenge is non-hex

fa6b11a556 test: Throw error when -signetchallenge is non-hex (MarcoFalke)

Pull request description:

  Instead of silently parsing non-hex to an empty challenge, throw an error.

  Also, add missing includes while touching the file.

ACKs for top commit:
  kevkevinpal:
    ACK [fa6b11a](fa6b11a556)
  kallewoof:
    ACK fa6b11a
  TheCharlatan:
    Nice, ACK fa6b11a556

Tree-SHA512: 018ebbbf819ba7cdf0c6dd294fdfaa5ddb81b87058a8b9c57b96066d5b07e1656fd78f18e3cef375aebefa191fa515c2c70bc764880fa05f98f526334431a616

src/chainparams.cpp

@@ -6,17 +6,20 @@
 #include <chainparams.h>
 
 #include <chainparamsbase.h>
-#include <chainparamsseeds.h>
 #include <common/args.h>
-#include <consensus/merkle.h>
+#include <consensus/params.h>
 #include <deploymentinfo.h>
-#include <hash.h> // for signet block challenge hash
 #include <logging.h>
-#include <script/interpreter.h>
+#include <tinyformat.h>
 #include <util/chaintype.h>
+#include <util/strencodings.h>
 #include <util/string.h>
 
-#include <assert.h>
+#include <cassert>
+#include <cstdint>
+#include <limits>
+#include <stdexcept>
+#include <vector>
 
 void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& options)
 {
@@ -26,9 +29,13 @@ void ReadSigNetArgs(const ArgsManager& args, CChainParams::SigNetOptions& option
     if (args.IsArgSet("-signetchallenge")) {
         const auto signet_challenge = args.GetArgs("-signetchallenge");
         if (signet_challenge.size() != 1) {
-            throw std::runtime_error(strprintf("%s: -signetchallenge cannot be multiple values.", __func__));
+            throw std::runtime_error("-signetchallenge cannot be multiple values.");
         }
-        options.challenge.emplace(ParseHex(signet_challenge[0]));
+        const auto val{TryParseHex<uint8_t>(signet_challenge[0])};
+        if (!val) {
+            throw std::runtime_error(strprintf("-signetchallenge must be hex, not '%s'.", signet_challenge[0]));
+        }
+        options.challenge.emplace(*val);
     }
 }
 

src/chainparams.h

@@ -6,20 +6,9 @@
 #ifndef BITCOIN_CHAINPARAMS_H
 #define BITCOIN_CHAINPARAMS_H
 
-#include <kernel/chainparams.h>
+#include <kernel/chainparams.h> // IWYU pragma: export
 
-#include <consensus/params.h>
-#include <netaddress.h>
-#include <primitives/block.h>
-#include <protocol.h>
-#include <util/chaintype.h>
-#include <util/hash_type.h>
-
-#include <cstdint>
 #include <memory>
-#include <string>
-#include <unordered_map>
-#include <vector>
 
 class ArgsManager;
 

src/util/strencodings.h

@@ -17,8 +17,8 @@
 #include <cstdint>
 #include <limits>
 #include <optional>
-#include <string>
-#include <string_view>
+#include <string>      // IWYU pragma: export
+#include <string_view> // IWYU pragma: export
 #include <system_error>
 #include <type_traits>
 #include <vector>

src/util/string.h

@@ -12,8 +12,8 @@
 #include <cstring>
 #include <locale>
 #include <sstream>
-#include <string>
-#include <string_view>
+#include <string>      // IWYU pragma: export
+#include <string_view> // IWYU pragma: export
 #include <vector>
 
 void ReplaceAll(std::string& in_out, const std::string& search, const std::string& substitute);

test/functional/feature_signet.py

@@ -76,6 +76,9 @@ class SignetBasicTest(BitcoinTestFramework):
         self.log.info("test that signet logs the network magic on node start")
         with self.nodes[0].assert_debug_log(["Signet derived magic (message start)"]):
             self.restart_node(0)
+        self.stop_node(0)
+        self.nodes[0].assert_start_raises_init_error(extra_args=["-signetchallenge=abc"], expected_msg="Error: -signetchallenge must be hex, not 'abc'.")
+        self.nodes[0].assert_start_raises_init_error(extra_args=["-signetchallenge=abc"] * 2, expected_msg="Error: -signetchallenge cannot be multiple values.")
 
 
 if __name__ == '__main__':