Merge #19106: util: simplify the interface of serviceFlagToStr()

189ae0c util: dedup code in callers of serviceFlagToStr() (Vasil Dimov)
fbacad1 util: simplify the interface of serviceFlagToStr() (Vasil Dimov)

Pull request description:

  Don't take two redundant arguments in `serviceFlagToStr()`.

  Introduce `serviceFlagsToStr()` which takes a mask (with more than one
  bit set) and returns a vector of strings.

  As a side effect this fixes an issue introduced in
  bitcoin/bitcoin#18165 due to which the GUI could
  print something like `UNKNOWN[1033] & UNKNOWN[1033] & UNKNOWN[2^10]`
  instead of `NETWORK & WITNESS`.

ACKs for top commit:
  MarcoFalke:
    ACK 189ae0c
  jonasschnelli:
    Tested ACK 189ae0c

Tree-SHA512: 000c490f16ebbba04458c62ca4ce743abffd344d375d95f5bbd5008742012032787655db2874b168df0270743266261dccf1693761906567502dcbac902bda50

Author: jonasschnelli

src/protocol.cpp

@@ -195,9 +195,15 @@ const std::vector<std::string> &getAllNetMessageTypes()
     return allNetMessageTypesVec;
 }
 
-std::string serviceFlagToStr(const uint64_t mask, const int bit)
+/**
+ * Convert a service flag (NODE_*) to a human readable string.
+ * It supports unknown service flags which will be returned as "UNKNOWN[...]".
+ * @param[in] bit the service flag is calculated as (1 << bit)
+ */
+static std::string serviceFlagToStr(size_t bit)
 {
-    switch (ServiceFlags(mask)) {
+    const uint64_t service_flag = 1ULL << bit;
+    switch ((ServiceFlags)service_flag) {
     case NODE_NONE: abort();  // impossible
     case NODE_NETWORK:         return "NETWORK";
     case NODE_GETUTXO:         return "GETUTXO";
@@ -211,10 +217,23 @@ std::string serviceFlagToStr(const uint64_t mask, const int bit)
     stream.imbue(std::locale::classic());
     stream << "UNKNOWN[";
     if (bit < 8) {
-        stream << mask;
+        stream << service_flag;
     } else {
         stream << "2^" << bit;
     }
     stream << "]";
     return stream.str();
 }
+
+std::vector<std::string> serviceFlagsToStr(uint64_t flags)
+{
+    std::vector<std::string> str_flags;
+
+    for (size_t i = 0; i < sizeof(flags) * 8; ++i) {
+        if (flags & (1ULL << i)) {
+            str_flags.emplace_back(serviceFlagToStr(i));
+        }
+    }
+
+    return str_flags;
+}

src/protocol.h

@@ -288,7 +288,12 @@ enum ServiceFlags : uint64_t {
     // BIP process.
 };
 
-std::string serviceFlagToStr(uint64_t mask, int bit);
+/**
+ * Convert service flags (a bitmask of NODE_*) to human readable strings.
+ * It supports unknown service flags which will be returned as "UNKNOWN[...]".
+ * @param[in] flags multiple NODE_* bitwise-OR-ed together
+ */
+std::vector<std::string> serviceFlagsToStr(uint64_t flags);
 
 /**
  * Gets the set of service flags which are "desirable" for a given peer.

src/qt/guiutil.cpp

@@ -755,12 +755,8 @@ QString formatServicesStr(quint64 mask)
 {
     QStringList strList;
 
-    for (int i = 0; i < 64; i++) {
-        uint64_t check = 1ull << i;
-        if (mask & check)
-        {
-            strList.append(QString::fromStdString(serviceFlagToStr(mask, i)));
-        }
+    for (const auto& flag : serviceFlagsToStr(mask)) {
+        strList.append(QString::fromStdString(flag));
     }
 
     if (strList.size())

src/rpc/util.cpp

@@ -841,14 +841,10 @@ std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, Fl
 
 UniValue GetServicesNames(ServiceFlags services)
 {
-    const uint64_t services_n = services;
     UniValue servicesNames(UniValue::VARR);
 
-    for (int i = 0; i < 64; ++i) {
-        const uint64_t mask = 1ull << i;
-        if (services_n & mask) {
-            servicesNames.push_back(serviceFlagToStr(mask, i));
-        }
+    for (const auto& flag : serviceFlagsToStr(services)) {
+        servicesNames.push_back(flag);
     }
 
     return servicesNames;