refactor: Make HexStr take a span

laanwj · laanwj · commit 0a8aa626dd69 · 2020-08-06T19:41:43.000+02:00
Make HexStr take a span of bytes, instead of an awkward pair of
templated iterators.
diff --git a/src/core_write.cpp b/src/core_write.cpp
@@ -48,13 +48,14 @@ std::string FormatScript(const CScript& script)
                 }
             }
             if (vch.size() > 0) {
-                ret += strprintf("0x%x 0x%x ", HexStr(it2, it - vch.size()), HexStr(it - vch.size(), it));
+                ret += strprintf("0x%x 0x%x ", HexStr(std::vector<uint8_t>(it2, it - vch.size())),
+                                               HexStr(std::vector<uint8_t>(it - vch.size(), it)));
             } else {
-                ret += strprintf("0x%x ", HexStr(it2, it));
+                ret += strprintf("0x%x ", HexStr(std::vector<uint8_t>(it2, it)));
             }
             continue;
         }
-        ret += strprintf("0x%x ", HexStr(it2, script.end()));
+        ret += strprintf("0x%x ", HexStr(std::vector<uint8_t>(it2, script.end())));
         break;
     }
     return ret.substr(0, ret.size() - 1);
diff --git a/src/net.cpp b/src/net.cpp
@@ -722,8 +722,8 @@ CNetMessage V1TransportDeserializer::GetMessage(const CMessageHeader::MessageSta
     if (!msg.m_valid_checksum) {
         LogPrint(BCLog::NET, "CHECKSUM ERROR (%s, %u bytes), expected %s was %s\n",
                  SanitizeString(msg.m_command), msg.m_message_size,
-                 HexStr(hash.begin(), hash.begin()+CMessageHeader::CHECKSUM_SIZE),
-                 HexStr(hdr.pchChecksum, hdr.pchChecksum+CMessageHeader::CHECKSUM_SIZE));
+                 HexStr(Span<uint8_t>(hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE)),
+                 HexStr(hdr.pchChecksum));
     }
 
     // store receive time
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
@@ -1348,7 +1348,7 @@ UniValue finalizepsbt(const JSONRPCRequest& request)
 
     if (complete && extract) {
         ssTx << mtx;
-        result_str = HexStr(ssTx.str());
+        result_str = HexStr(ssTx);
         result.pushKV("hex", result_str);
     } else {
         ssTx << psbtx;
diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp
@@ -78,7 +78,7 @@ bool GenerateAuthCookie(std::string *cookie_out)
     const size_t COOKIE_SIZE = 32;
     unsigned char rand_pwd[COOKIE_SIZE];
     GetRandBytes(rand_pwd, COOKIE_SIZE);
-    std::string cookie = COOKIEAUTH_USER + ":" + HexStr(rand_pwd, rand_pwd+COOKIE_SIZE);
+    std::string cookie = COOKIEAUTH_USER + ":" + HexStr(rand_pwd);
 
     /** the umask determines what permissions are used to create this file -
      * these are set to 077 in init.cpp unless overridden with -sysperms.
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
@@ -260,7 +260,7 @@ class DescribeAddressVisitor : public boost::static_visitor<UniValue>
         UniValue obj(UniValue::VOBJ);
         obj.pushKV("iswitness", true);
         obj.pushKV("witness_version", (int)id.version);
-        obj.pushKV("witness_program", HexStr(id.program, id.program + id.length));
+        obj.pushKV("witness_program", HexStr(Span<const unsigned char>(id.program, id.length)));
         return obj;
     }
 };
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
@@ -190,7 +190,7 @@ class OriginPubkeyProvider final : public PubkeyProvider
 
     std::string OriginString() const
     {
-        return HexStr(std::begin(m_origin.fingerprint), std::end(m_origin.fingerprint)) + FormatHDKeypath(m_origin.path);
+        return HexStr(m_origin.fingerprint) + FormatHDKeypath(m_origin.path);
     }
 
 public:
diff --git a/src/test/crypto_tests.cpp b/src/test/crypto_tests.cpp
@@ -183,7 +183,7 @@ static void TestHKDF_SHA256_32(const std::string &ikm_hex, const std::string &sa
     CHKDF_HMAC_SHA256_L32 hkdf32(initial_key_material.data(), initial_key_material.size(), salt_stringified);
     unsigned char out[32];
     hkdf32.Expand32(info_stringified, out);
-    BOOST_CHECK(HexStr(out, out + 32) == okm_check_hex);
+    BOOST_CHECK(HexStr(out) == okm_check_hex);
 }
 
 static std::string LongTestString()
diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp
@@ -241,7 +241,7 @@ BOOST_FIXTURE_TEST_CASE(Merge, MergeTestingSetup)
 
     unsigned char out_sha_bytes[CSHA256::OUTPUT_SIZE];
     out_sha.Finalize(out_sha_bytes);
-    std::string out_sha_hex = HexStr(std::begin(out_sha_bytes), std::end(out_sha_bytes));
+    std::string out_sha_hex = HexStr(out_sha_bytes);
 
     // If check below fails, should manually dump the results with:
     //
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
@@ -105,47 +105,24 @@ BOOST_AUTO_TEST_CASE(util_ParseHex)
 BOOST_AUTO_TEST_CASE(util_HexStr)
 {
     BOOST_CHECK_EQUAL(
-        HexStr(ParseHex_expected, ParseHex_expected + sizeof(ParseHex_expected)),
+        HexStr(ParseHex_expected),
         "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f");
 
     BOOST_CHECK_EQUAL(
-        HexStr(ParseHex_expected + sizeof(ParseHex_expected),
-               ParseHex_expected + sizeof(ParseHex_expected)),
+        HexStr(Span<const unsigned char>(
+               ParseHex_expected + sizeof(ParseHex_expected),
+               ParseHex_expected + sizeof(ParseHex_expected))),
         "");
 
     BOOST_CHECK_EQUAL(
-        HexStr(ParseHex_expected, ParseHex_expected),
+        HexStr(Span<const unsigned char>(ParseHex_expected, ParseHex_expected)),
         "");
 
     std::vector<unsigned char> ParseHex_vec(ParseHex_expected, ParseHex_expected + 5);
 
     BOOST_CHECK_EQUAL(
-        HexStr(ParseHex_vec.rbegin(), ParseHex_vec.rend()),
-        "b0fd8a6704"
-    );
-
-    BOOST_CHECK_EQUAL(
-        HexStr(std::reverse_iterator<const uint8_t *>(ParseHex_expected),
-               std::reverse_iterator<const uint8_t *>(ParseHex_expected)),
-        ""
-    );
-
-    BOOST_CHECK_EQUAL(
-        HexStr(std::reverse_iterator<const uint8_t *>(ParseHex_expected + 1),
-               std::reverse_iterator<const uint8_t *>(ParseHex_expected)),
-        "04"
-    );
-
-    BOOST_CHECK_EQUAL(
-        HexStr(std::reverse_iterator<const uint8_t *>(ParseHex_expected + 5),
-               std::reverse_iterator<const uint8_t *>(ParseHex_expected)),
-        "b0fd8a6704"
-    );
-
-    BOOST_CHECK_EQUAL(
-        HexStr(std::reverse_iterator<const uint8_t *>(ParseHex_expected + 65),
-               std::reverse_iterator<const uint8_t *>(ParseHex_expected)),
-        "5f1df16b2b704c8a578d0bbaf74d385cde12c11ee50455f3c438ef4c3fbcf649b6de611feae06279a60939e028a8d65c10b73071a6f16719274855feb0fd8a6704"
+        HexStr(ParseHex_vec),
+        "04678afdb0"
     );
 }
 
@@ -1022,7 +999,7 @@ BOOST_FIXTURE_TEST_CASE(util_ArgsMerge, ArgsMergeTestingSetup)
 
     unsigned char out_sha_bytes[CSHA256::OUTPUT_SIZE];
     out_sha.Finalize(out_sha_bytes);
-    std::string out_sha_hex = HexStr(std::begin(out_sha_bytes), std::end(out_sha_bytes));
+    std::string out_sha_hex = HexStr(out_sha_bytes);
 
     // If check below fails, should manually dump the results with:
     //
@@ -1125,7 +1102,7 @@ BOOST_FIXTURE_TEST_CASE(util_ChainMerge, ChainMergeTestingSetup)
 
     unsigned char out_sha_bytes[CSHA256::OUTPUT_SIZE];
     out_sha.Finalize(out_sha_bytes);
-    std::string out_sha_hex = HexStr(std::begin(out_sha_bytes), std::end(out_sha_bytes));
+    std::string out_sha_hex = HexStr(out_sha_bytes);
 
     // If check below fails, should manually dump the results with:
     //
diff --git a/src/uint256.cpp b/src/uint256.cpp
@@ -19,7 +19,11 @@ base_blob<BITS>::base_blob(const std::vector<unsigned char>& vch)
 template <unsigned int BITS>
 std::string base_blob<BITS>::GetHex() const
 {
-    return HexStr(std::reverse_iterator<const uint8_t*>(m_data + sizeof(m_data)), std::reverse_iterator<const uint8_t*>(m_data));
+    uint8_t m_data_rev[WIDTH];
+    for (int i = 0; i < WIDTH; ++i) {
+        m_data_rev[i] = m_data[WIDTH - 1 - i];
+    }
+    return HexStr(m_data_rev);
 }
 
 template <unsigned int BITS>
diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp
@@ -569,3 +569,16 @@ std::string Capitalize(std::string str)
     str[0] = ToUpper(str.front());
     return str;
 }
+
+std::string HexStr(const Span<const uint8_t> s)
+{
+    std::string rv;
+    static constexpr char hexmap[16] = { '0', '1', '2', '3', '4', '5', '6', '7',
+                                         '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
+    rv.reserve(s.size() * 2);
+    for (uint8_t v: s) {
+        rv.push_back(hexmap[v >> 4]);
+        rv.push_back(hexmap[v & 15]);
+    }
+    return rv;
+}
diff --git a/src/util/strencodings.h b/src/util/strencodings.h
@@ -10,6 +10,7 @@
 #define BITCOIN_UTIL_STRENCODINGS_H
 
 #include <attributes.h>
+#include <span.h>
 
 #include <cstdint>
 #include <iterator>
@@ -119,27 +120,11 @@ NODISCARD bool ParseUInt64(const std::string& str, uint64_t *out);
  */
 NODISCARD bool ParseDouble(const std::string& str, double *out);
 
-template<typename T>
-std::string HexStr(const T itbegin, const T itend)
-{
-    std::string rv;
-    static const char hexmap[16] = { '0', '1', '2', '3', '4', '5', '6', '7',
-                                     '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
-    rv.reserve(std::distance(itbegin, itend) * 2);
-    for(T it = itbegin; it < itend; ++it)
-    {
-        unsigned char val = (unsigned char)(*it);
-        rv.push_back(hexmap[val>>4]);
-        rv.push_back(hexmap[val&15]);
-    }
-    return rv;
-}
-
-template<typename T>
-inline std::string HexStr(const T& vch)
-{
-    return HexStr(vch.begin(), vch.end());
-}
+/**
+ * Convert a span of bytes to a lower-case hexadecimal string.
+ */
+std::string HexStr(const Span<const uint8_t> s);
+inline std::string HexStr(const Span<const char> s) { return HexStr(MakeUCharSpan(s)); }
 
 /**
  * Format a paragraph of text to a fixed width, adding spaces for
diff --git a/src/validation.cpp b/src/validation.cpp
@@ -1198,8 +1198,8 @@ bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, c
 
         if (memcmp(blk_start, message_start, CMessageHeader::MESSAGE_START_SIZE)) {
             return error("%s: Block magic mismatch for %s: %s versus expected %s", __func__, pos.ToString(),
-                    HexStr(blk_start, blk_start + CMessageHeader::MESSAGE_START_SIZE),
-                    HexStr(message_start, message_start + CMessageHeader::MESSAGE_START_SIZE));
+                    HexStr(blk_start),
+                    HexStr(message_start));
         }
 
         if (blk_size > MAX_SIZE) {
diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
@@ -38,7 +38,7 @@ void CheckUniqueFileid(const BerkeleyEnvironment& env, const std::string& filena
     for (const auto& item : env.m_fileids) {
         if (fileid == item.second && &fileid != &item.second) {
             throw std::runtime_error(strprintf("BerkeleyDatabase: Can't open database %s (duplicates fileid %s from %s)", filename,
-                HexStr(std::begin(item.second.value), std::end(item.second.value)), item.first));
+                HexStr(item.second.value), item.first));
         }
     }
 }
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
@@ -34,7 +34,7 @@ std::string static EncodeDumpString(const std::string &str) {
     std::stringstream ret;
     for (const unsigned char c : str) {
         if (c <= 32 || c >= 128 || c == '%') {
-            ret << '%' << HexStr(&c, &c + 1);
+            ret << '%' << HexStr(Span<const unsigned char>(&c, 1));
         } else {
             ret << c;
         }
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -3688,7 +3688,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request)
             if (meta->has_key_origin) {
                 ret.pushKV("hdkeypath", WriteHDKeypath(meta->key_origin.path));
                 ret.pushKV("hdseedid", meta->hd_seed_id.GetHex());
-                ret.pushKV("hdmasterfingerprint", HexStr(meta->key_origin.fingerprint, meta->key_origin.fingerprint + 4));
+                ret.pushKV("hdmasterfingerprint", HexStr(meta->key_origin.fingerprint));
             }
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -48,13 +48,14 @@ std::string FormatScript(const CScript& script)`
`48`	`48`	`}`
`49`	`49`	`}`
`50`	`50`	`if (vch.size() > 0) {`
`51`		`- ret += strprintf("0x%x 0x%x ", HexStr(it2, it - vch.size()), HexStr(it - vch.size(), it));`
	`51`	`+ ret += strprintf("0x%x 0x%x ", HexStr(std::vector<uint8_t>(it2, it - vch.size())),`
	`52`	`+ HexStr(std::vector<uint8_t>(it - vch.size(), it)));`
`52`	`53`	`} else {`
`53`		`- ret += strprintf("0x%x ", HexStr(it2, it));`
	`54`	`+ ret += strprintf("0x%x ", HexStr(std::vector<uint8_t>(it2, it)));`
`54`	`55`	`}`
`55`	`56`	`continue;`
`56`	`57`	`}`
`57`		`- ret += strprintf("0x%x ", HexStr(it2, script.end()));`
	`58`	`+ ret += strprintf("0x%x ", HexStr(std::vector<uint8_t>(it2, script.end())));`
`58`	`59`	`break;`
`59`	`60`	`}`
`60`	`61`	`return ret.substr(0, ret.size() - 1);`
Original file line number	Diff line number	Diff line change
`@@ -260,7 +260,7 @@ class DescribeAddressVisitor : public boost::static_visitor<UniValue>`
`260`	`260`	`UniValue obj(UniValue::VOBJ);`
`261`	`261`	`obj.pushKV("iswitness", true);`
`262`	`262`	`obj.pushKV("witness_version", (int)id.version);`
`263`		`- obj.pushKV("witness_program", HexStr(id.program, id.program + id.length));`
	`263`	`+ obj.pushKV("witness_program", HexStr(Span<const unsigned char>(id.program, id.length)));`
`264`	`264`	`return obj;`
`265`	`265`	`}`
`266`	`266`	`};`
Original file line number	Diff line number	Diff line change
`@@ -190,7 +190,7 @@ class OriginPubkeyProvider final : public PubkeyProvider`
`190`	`190`
`191`	`191`	`std::string OriginString() const`
`192`	`192`	`{`
`193`		`- return HexStr(std::begin(m_origin.fingerprint), std::end(m_origin.fingerprint)) + FormatHDKeypath(m_origin.path);`
	`193`	`+ return HexStr(m_origin.fingerprint) + FormatHDKeypath(m_origin.path);`
`194`	`194`	`}`
`195`	`195`
`196`	`196`	`public:`
Original file line number	Diff line number	Diff line change
`@@ -183,7 +183,7 @@ static void TestHKDF_SHA256_32(const std::string &ikm_hex, const std::string &sa`
`183`	`183`	`CHKDF_HMAC_SHA256_L32 hkdf32(initial_key_material.data(), initial_key_material.size(), salt_stringified);`
`184`	`184`	`unsigned char out[32];`
`185`	`185`	`hkdf32.Expand32(info_stringified, out);`
`186`		`- BOOST_CHECK(HexStr(out, out + 32) == okm_check_hex);`
	`186`	`+ BOOST_CHECK(HexStr(out) == okm_check_hex);`
`187`	`187`	`}`
`188`	`188`
`189`	`189`	`static std::string LongTestString()`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,11 @@ base_blob<BITS>::base_blob(const std::vector<unsigned char>& vch)`
`19`	`19`	`template <unsigned int BITS>`
`20`	`20`	`std::string base_blob<BITS>::GetHex() const`
`21`	`21`	`{`
`22`		`- return HexStr(std::reverse_iterator<const uint8_t>(m_data + sizeof(m_data)), std::reverse_iterator<const uint8_t>(m_data));`
	`22`	`+ uint8_t m_data_rev[WIDTH];`
	`23`	`+ for (int i = 0; i < WIDTH; ++i) {`
	`24`	`+ m_data_rev[i] = m_data[WIDTH - 1 - i];`
	`25`	`+ }`
	`26`	`+ return HexStr(m_data_rev);`
`23`	`27`	`}`
`24`	`28`
`25`	`29`	`template <unsigned int BITS>`