dead code: Remove dead option in HexStr conversion

[ldm5180]

Problem:

• Nothing uses the fspaces argument to HexStr() besides unit
  tests. This argument results in extra complexity and a small
  performance decrease within the function.

Solution:

• Remove unused fspaces option.
• Remove associated unit tests.

[Empact]

I wouldn’t call the performance difference worthwhile, but I’m all for removing dead code.

[promag]

Concept ACK, if it's not used then it can be removed.

I don't think the performance gain justifies this change. Anyway, I think the following is enough:

--- a/src/util/strencodings.h
+++ b/src/util/strencodings.h
@@ -121,17 +121,15 @@ 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, bool fSpaces=false)
+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((itend-itbegin)*3);
+    rv.reserve((itend-itbegin) * 2);
     for(T it = itbegin; it < itend; ++it)
     {
         unsigned char val = (unsigned char)(*it);
-        if(fSpaces && it != itbegin)
-            rv.push_back(' ');
         rv.push_back(hexmap[val>>4]);
         rv.push_back(hexmap[val&15]);
     }
@@ -140,9 +138,9 @@ std::string HexStr(const T itbegin, const T itend, bool fSpaces=false)
 }

 template<typename T>
-inline std::string HexStr(const T& vch, bool fSpaces=false)
+inline std::string HexStr(const T& vch)
 {
-    return HexStr(vch.begin(), vch.end(), fSpaces);
+    return HexStr(vch.begin(), vch.end());
 }

 /**

[sipa]

HexStr does not in any way affect the Base58 benchmarks, I think?

[ldm5180]

Limited change to only removing dead code and updated PR details.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #15576 (constexpr: Increase constexpr usage in strencodings by ldm5180)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[Empact]

utACK 56f1d28

[practicalswift]

utACK 56f1d28

-42 lines: very nice! Thanks for removing this cruft.

[maflcko]

utACK 56f1d28

[promag]

utACK 56f1d28.

HexStr does not in any way affect the Base58 benchmarks, I think?

@ldm5180 why did you use those benchs in the first place?