Skip to content

Commit

Permalink
rpc: return warnings as an array instead of just a single one
Browse files Browse the repository at this point in the history
The RPC documentation for `getblockchaininfo`, `getmininginfo` and
`getnetworkinfo` states that "warnings" returns "any network and
blockchain warnings". In practice, only a single warning is returned.

Fix that by returning all warnings as an array.

As a side benefit, cleans up the GetWarnings() logic.
  • Loading branch information
stickies-v committed Apr 10, 2024
1 parent bf031a5 commit 4b82191
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 38 deletions.
6 changes: 6 additions & 0 deletions doc/release-notes-29845.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
RPC
---

- the `warnings` field in `getblockchaininfo`, `getmininginfo` and
`getnetworkinfo` now returns all the active node warnings as an array
of strings, instead of just a single warning.
3 changes: 2 additions & 1 deletion src/node/interfaces.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include <util/check.h>
#include <util/result.h>
#include <util/signalinterrupt.h>
#include <util/string.h>
#include <util/translation.h>
#include <validation.h>
#include <validationinterface.h>
Expand Down Expand Up @@ -91,7 +92,7 @@ class NodeImpl : public Node
explicit NodeImpl(NodeContext& context) { setContext(&context); }
void initLogging() override { InitLogging(args()); }
void initParameterInteraction() override { InitParameterInteraction(args()); }
bilingual_str getWarnings() override { return GetWarnings(true); }
bilingual_str getWarnings() override { return Join(GetWarnings(), Untranslated("<hr />")); }
int getExitStatus() override { return Assert(m_context)->exit_status.load(); }
uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); }
bool baseInitialize() override
Expand Down
9 changes: 6 additions & 3 deletions src/rpc/blockchain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
#include <validation.h>
#include <validationinterface.h>
#include <versionbits.h>
#include <warnings.h>

#include <stdint.h>

Expand Down Expand Up @@ -1260,7 +1259,11 @@ RPCHelpMan getblockchaininfo()
{RPCResult::Type::NUM, "pruneheight", /*optional=*/true, "height of the last block pruned, plus one (only present if pruning is enabled)"},
{RPCResult::Type::BOOL, "automatic_pruning", /*optional=*/true, "whether automatic pruning is enabled (only present if pruning is enabled)"},
{RPCResult::Type::NUM, "prune_target_size", /*optional=*/true, "the target size used by pruning (only present if automatic pruning is enabled)"},
{RPCResult::Type::STR, "warnings", "any network and blockchain warnings"},
{RPCResult::Type::ARR, "warnings", "any network and blockchain warnings",
{
{RPCResult::Type::STR, "", "warning"},
}
},
}},
RPCExamples{
HelpExampleCli("getblockchaininfo", "")
Expand Down Expand Up @@ -1298,7 +1301,7 @@ RPCHelpMan getblockchaininfo()
}
}

obj.pushKV("warnings", GetWarnings(false).original);
obj.pushKV("warnings", GetNodeWarnings());
return obj;
},
};
Expand Down
9 changes: 6 additions & 3 deletions src/rpc/mining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
#include <util/translation.h>
#include <validation.h>
#include <validationinterface.h>
#include <warnings.h>

#include <memory>
#include <stdint.h>
Expand Down Expand Up @@ -426,7 +425,11 @@ static RPCHelpMan getmininginfo()
{RPCResult::Type::NUM, "networkhashps", "The network hashes per second"},
{RPCResult::Type::NUM, "pooledtx", "The size of the mempool"},
{RPCResult::Type::STR, "chain", "current network name (main, test, signet, regtest)"},
{RPCResult::Type::STR, "warnings", "any network and blockchain warnings"},
{RPCResult::Type::ARR, "warnings", "any network and blockchain warnings",
{
{RPCResult::Type::STR, "", "warning"},
}
},
}},
RPCExamples{
HelpExampleCli("getmininginfo", "")
Expand All @@ -448,7 +451,7 @@ static RPCHelpMan getmininginfo()
obj.pushKV("networkhashps", getnetworkhashps().HandleRequest(request));
obj.pushKV("pooledtx", (uint64_t)mempool.size());
obj.pushKV("chain", chainman.GetParams().GetChainTypeString());
obj.pushKV("warnings", GetWarnings(false).original);
obj.pushKV("warnings", GetNodeWarnings());
return obj;
},
};
Expand Down
9 changes: 6 additions & 3 deletions src/rpc/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#include <util/time.h>
#include <util/translation.h>
#include <validation.h>
#include <warnings.h>

#include <optional>

Expand Down Expand Up @@ -658,7 +657,11 @@ static RPCHelpMan getnetworkinfo()
{RPCResult::Type::NUM, "score", "relative score"},
}},
}},
{RPCResult::Type::STR, "warnings", "any network and blockchain warnings"},
{RPCResult::Type::ARR, "warnings", "any network and blockchain warnings",
{
{RPCResult::Type::STR, "", "warning"},
}
},
}
},
RPCExamples{
Expand Down Expand Up @@ -707,7 +710,7 @@ static RPCHelpMan getnetworkinfo()
}
}
obj.pushKV("localaddresses", localAddresses);
obj.pushKV("warnings", GetWarnings(false).original);
obj.pushKV("warnings", GetNodeWarnings());
return obj;
},
};
Expand Down
12 changes: 12 additions & 0 deletions src/rpc/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@
#include <script/signingprovider.h>
#include <script/solver.h>
#include <tinyformat.h>
#include <univalue.h>
#include <util/check.h>
#include <util/result.h>
#include <util/strencodings.h>
#include <util/string.h>
#include <util/translation.h>
#include <warnings.h>

#include <string_view>
#include <tuple>
Expand Down Expand Up @@ -1345,3 +1347,13 @@ void PushWarnings(const std::vector<bilingual_str>& warnings, UniValue& obj)
if (warnings.empty()) return;
obj.pushKV("warnings", BilingualStringsToUniValue(warnings));
}

UniValue GetNodeWarnings()
{
UniValue warnings{UniValue::VARR};
for (const auto& warning : GetWarnings()) {
warnings.push_back(warning.original);
}

return warnings;
}
2 changes: 2 additions & 0 deletions src/rpc/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -471,4 +471,6 @@ class RPCHelpMan
void PushWarnings(const UniValue& warnings, UniValue& obj);
void PushWarnings(const std::vector<bilingual_str>& warnings, UniValue& obj);

UniValue GetNodeWarnings();

#endif // BITCOIN_RPC_UTIL_H
2 changes: 1 addition & 1 deletion src/test/timedata_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ BOOST_AUTO_TEST_CASE(addtimedata)
MultiAddTimeData(1, DEFAULT_MAX_TIME_ADJUSTMENT + 1); //filter size 5
}

BOOST_CHECK(GetWarnings(true).original.find("clock is wrong") != std::string::npos);
BOOST_CHECK(GetWarnings().back().original.find("clock is wrong") != std::string::npos);

// nTimeOffset is not changed if the median of offsets exceeds DEFAULT_MAX_TIME_ADJUSTMENT
BOOST_CHECK_EQUAL(GetTimeOffset(), 0);
Expand Down
21 changes: 6 additions & 15 deletions src/warnings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

#include <common/system.h>
#include <sync.h>
#include <util/string.h>
#include <util/translation.h>

#include <vector>
Expand All @@ -32,33 +31,25 @@ void SetfLargeWorkInvalidChainFound(bool flag)
fLargeWorkInvalidChainFound = flag;
}

bilingual_str GetWarnings(bool verbose)
std::vector<bilingual_str> GetWarnings()
{
bilingual_str warnings_concise;
std::vector<bilingual_str> warnings_verbose;
std::vector<bilingual_str> warnings;

LOCK(g_warnings_mutex);

// Pre-release build warning
if (!CLIENT_VERSION_IS_RELEASE) {
warnings_concise = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications");
warnings_verbose.emplace_back(warnings_concise);
warnings.emplace_back(_("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications"));
}

// Misc warnings like out of disk space and clock is wrong
if (!g_misc_warnings.empty()) {
warnings_concise = g_misc_warnings;
warnings_verbose.emplace_back(warnings_concise);
warnings.emplace_back(g_misc_warnings);
}

if (fLargeWorkInvalidChainFound) {
warnings_concise = _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.");
warnings_verbose.emplace_back(warnings_concise);
warnings.emplace_back(_("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade."));
}

if (verbose) {
return Join(warnings_verbose, Untranslated("<hr />"));
}

return warnings_concise;
return warnings;
}
11 changes: 3 additions & 8 deletions src/warnings.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,13 @@
#ifndef BITCOIN_WARNINGS_H
#define BITCOIN_WARNINGS_H

#include <string>
#include <vector>

struct bilingual_str;

void SetMiscWarning(const bilingual_str& warning);
void SetfLargeWorkInvalidChainFound(bool flag);
/** Format a string that describes several potential problems detected by the core.
* @param[in] verbose bool
* - if true, get all warnings separated by <hr />
* - if false, get the most important warning
* @returns the warning string
*/
bilingual_str GetWarnings(bool verbose);
/** Return potential problems detected by the node. */
std::vector<bilingual_str> GetWarnings();

#endif // BITCOIN_WARNINGS_H
8 changes: 4 additions & 4 deletions test/functional/feature_versionbits_warning.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ def run_test(self):
self.generatetoaddress(node, VB_PERIOD - VB_THRESHOLD + 1, node_deterministic_address)

# Check that we're not getting any versionbit-related errors in get*info()
assert not VB_PATTERN.match(node.getmininginfo()["warnings"])
assert not VB_PATTERN.match(node.getnetworkinfo()["warnings"])
assert not VB_PATTERN.match(",".join(node.getmininginfo()["warnings"]))
assert not VB_PATTERN.match(",".join(node.getnetworkinfo()["warnings"]))

# Build one period of blocks with VB_THRESHOLD blocks signaling some unknown bit
self.send_blocks_with_version(peer, VB_THRESHOLD, VB_UNKNOWN_VERSION)
Expand All @@ -94,8 +94,8 @@ def run_test(self):
# Generating one more block will be enough to generate an error.
self.generatetoaddress(node, 1, node_deterministic_address)
# Check that get*info() shows the versionbits unknown rules warning
assert WARN_UNKNOWN_RULES_ACTIVE in node.getmininginfo()["warnings"]
assert WARN_UNKNOWN_RULES_ACTIVE in node.getnetworkinfo()["warnings"]
assert WARN_UNKNOWN_RULES_ACTIVE in ",".join(node.getmininginfo()["warnings"])
assert WARN_UNKNOWN_RULES_ACTIVE in ",".join(node.getnetworkinfo()["warnings"])
# Check that the alert file shows the versionbits unknown rules warning
self.wait_until(lambda: self.versionbits_in_alert_file())

Expand Down

0 comments on commit 4b82191

Please sign in to comment.