Side effect: this results in the RPC showing unknown service bits as “UNKNOWN[n]” like the GUI.
Note that there is no common mask-to-vector<string> function because both GUI and RPC would need to iterate through it to convert to their desired target formats.
Side effect: this results in the RPC showing unknown service bits as "UNKNOWN[n]" like the GUI.
Note that there is no common mask-to-vector<string> function because both GUI and RPC would need to iterate through it to convert to their desired target formats.
198@@ -199,3 +199,27 @@ const std::vector<std::string> &getAllNetMessageTypes()
199 {
200 return allNetMessageTypesVec;
201 }
202+
203+std::string serviceFlagToStr(const uint64_t mask, const int bit)
bit, or what am I missing?
mask
1ull << bit;. Given how rarely this function is called that seems like an over-optimization.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
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.
763- uint64_t check = 1LL << i;
764+ uint64_t check = 1ull << i;
765 if (mask & check)
766 {
767- strList.append(serviceFlagToStr(check, i));
768+ strList.append(QString::fromStdString(serviceFlagToStr(mask, i)));
Here we should pass check instead of mask? I did not test it but I think with the current patch formatServicesStr(NODE_NETWORK | NODE_WITNESS) will return a string like UNKNOWN[9] & UNKNOWN[9].
The arguments to serviceFlagToStr() are redundant (one can be derived from the other easily) which I think is confusing and could lead to slippages. I agree with @laanwj that just one of them should be passed.
Now that this PR got merged I tested whether my concern above was legit. It was:
instead of
0NETWORK & WITNESS
now I see
0UNKNOWN[1033] & UNKNOWN[1033] & UNKNOWN[2^10]
