From f0a1d7c14035ad3bbc7433685cc95ca15e0a32af Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 8 Nov 2023 10:53:34 -0500 Subject: [PATCH 1/2] Merge bitcoin/bitcoin#28787: init: completely remove `-zapwallettxes` (remaining hidden option) 5039c346ca87d6112ea1eb124bdc622ba9e9a513 init: completely remove `-zapwallettxes` (remaining hidden option) (Sebastian Falbesoner) Pull request description: The `-zapwallettxes` functionality has been removed in v0.21.0 (see commit 3340dbadd38f5624642cf0e14dddbe6f83a3863b / PR #19671), with the parameter being kept as hidden option, to inform users via an exit error that `abandontransaction` should be used instead. As any guides that still suggest to use `-zapwallettxes` would refer to a Bitcoin Core version that is EOL since many years (i.e. <= v0.20.x), it is highly unlikely that the error caused by the option is still relevant for any user, hence it seems fine to remove it now. ACKs for top commit: achow101: ACK 5039c346ca87d6112ea1eb124bdc622ba9e9a513 BrandonOdiwuor: ACK 5039c346ca87d6112ea1eb124bdc622ba9e9a513 fanquake: ACK 5039c346ca87d6112ea1eb124bdc622ba9e9a513 Tree-SHA512: e3ccc6918e0f8fa68dbd1a7ec4999cc2a44e28038711919fcddaf0727648c73a9ba0fb77674317147592a113fad20755d4e727f48176bc17b048fbdebad2d6c9 --- src/wallet/init.cpp | 6 ------ test/lint/check-doc.py | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 444e066b0021..e1be2e7eefb6 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -125,8 +125,6 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const argsman.AddArg("-walletrejectlongchains", strprintf("Wallet will not create transactions that violate mempool chain limits (default: %u)", DEFAULT_WALLET_REJECT_LONG_CHAINS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); argsman.AddArg("-walletcrosschain", strprintf("Allow reusing wallet files across chains (default: %u)", DEFAULT_WALLETCROSSCHAIN), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); - - argsman.AddHiddenArgs({"-zapwallettxes"}); } bool WalletInit::ParameterInteraction() const @@ -150,10 +148,6 @@ bool WalletInit::ParameterInteraction() const LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__); } - if (gArgs.IsArgSet("-zapwallettxes")) { - return InitError(Untranslated("-zapwallettxes has been removed. If you are attempting to remove a stuck transaction from your wallet, please use abandontransaction instead.")); - } - int rescan_mode = gArgs.GetIntArg("-rescan", 0); if (rescan_mode < 0 || rescan_mode > 2) { LogPrintf("%s: Warning: incorrect -rescan mode, falling back to default value.\n", __func__); diff --git a/test/lint/check-doc.py b/test/lint/check-doc.py index 7955dd4f3e65..dd7cb0306eb0 100755 --- a/test/lint/check-doc.py +++ b/test/lint/check-doc.py @@ -23,7 +23,7 @@ CMD_GREP_WALLET_HIDDEN_ARGS = r"git grep --function-context 'void DummyWalletInit::AddWalletOptions' -- {}".format(CMD_ROOT_DIR) CMD_GREP_DOCS = r"git grep --perl-regexp '{}' {}".format(REGEX_DOC, CMD_ROOT_DIR) # list unsupported, deprecated and duplicate args as they need no documentation -SET_DOC_OPTIONAL = set(['-h', '-help', '-dbcrashratio', '-forcecompactdb', '-zapwallettxes']) +SET_DOC_OPTIONAL = set(['-h', '-help', '-dbcrashratio', '-forcecompactdb']) def lint_missing_argument_documentation(): From f35e4792576b080ab34b85a69426ca385066c2f9 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 28 Mar 2022 11:51:39 +0100 Subject: [PATCH 2/2] Merge bitcoin/bitcoin#23083: rpc: Fail to return undocumented or misdocumented JSON fc892c3a80091fbeaa2b5a6ec5bdaa31359b42de rpc: Fail to return undocumented or misdocumented JSON (MarcoFalke) f4bc4a705addea3e60c3b69437913e6571df275d rpc: Add m_skip_type_check to RPCResult (MarcoFalke) Pull request description: This avoids documentation shortcomings such as the ones fixed in commit e7b6272b305386a264adf2c04b7bebfb8499070f, 138d55e6a0241f126916fde6ac9177c7e2a119c4, 577bd51a4b8de066466a445192c1c653872657e2, f8c84e047c61200fae4cc1d85688e113bf270409, 0ee9a00f90e81a6978b30bdb250a37cbfa6da022, 13f41855c5fedf11d4a2525f2f0dd214533e5e62, or faecb2ee0a0ad3eb8303cfc84a87dc02ec553c43 ACKs for top commit: fanquake: ACK fc892c3a80091fbeaa2b5a6ec5bdaa31359b42de - tested that this catches issue, i.e #24691: Tree-SHA512: 9d0d7e6291bfc6f67541a4ff746d374ad8751fefcff6d103d8621c0298b190ab1d209ce96cfc3a0d4a6a5460a9f9bb790eb96027b16e5ff91f2512e40c92ca84 --- src/rpc/util.cpp | 54 ++++++++++++++++++++++++++++++++++----- src/rpc/util.h | 11 +++++--- src/wallet/rpc/wallet.cpp | 2 +- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 9aae2d7e0c96..9d070409a46e 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -725,7 +725,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const // Elements in a JSON structure (dictionary or array) are separated by a comma const std::string maybe_separator{outer_type != OuterType::NONE ? "," : ""}; - // The key name if recursed into an dictionary + // The key name if recursed into a dictionary const std::string maybe_key{ outer_type == OuterType::OBJ ? "\"" + this->m_key_name + "\" : " : @@ -816,10 +816,11 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const bool RPCResult::MatchesType(const UniValue& result) const { - switch (m_type) { - case Type::ELISION: { - return false; + if (m_skip_type_check) { + return true; } + switch (m_type) { + case Type::ELISION: case Type::ANY: { return true; } @@ -840,11 +841,52 @@ bool RPCResult::MatchesType(const UniValue& result) const } case Type::ARR_FIXED: case Type::ARR: { - return UniValue::VARR == result.getType(); + if (UniValue::VARR != result.getType()) return false; + for (size_t i{0}; i < result.get_array().size(); ++i) { + // If there are more results than documented, re-use the last doc_inner. + const RPCResult& doc_inner{m_inner.at(std::min(m_inner.size() - 1, i))}; + if (!doc_inner.MatchesType(result.get_array()[i])) return false; + } + return true; // empty result array is valid } case Type::OBJ_DYN: case Type::OBJ: { - return UniValue::VOBJ == result.getType(); + if (UniValue::VOBJ != result.getType()) return false; + if (!m_inner.empty() && m_inner.at(0).m_type == Type::ELISION) return true; + if (m_type == Type::OBJ_DYN) { + const RPCResult& doc_inner{m_inner.at(0)}; // Assume all types are the same, randomly pick the first + for (size_t i{0}; i < result.get_obj().size(); ++i) { + if (!doc_inner.MatchesType(result.get_obj()[i])) { + return false; + } + } + return true; // empty result obj is valid + } + std::set doc_keys; + for (const auto& doc_entry : m_inner) { + doc_keys.insert(doc_entry.m_key_name); + } + std::map result_obj; + result.getObjMap(result_obj); + for (const auto& result_entry : result_obj) { + if (doc_keys.find(result_entry.first) == doc_keys.end()) { + return false; // missing documentation + } + } + + for (const auto& doc_entry : m_inner) { + const auto result_it{result_obj.find(doc_entry.m_key_name)}; + if (result_it == result_obj.end()) { + if (!doc_entry.m_optional) { + return false; // result is missing a required key + } + continue; + } + if (!doc_entry.MatchesType(result_it->second)) { + return false; // wrong type + } + } + return true; } } // no default case, so the compiler can warn about missing cases NONFATAL_UNREACHABLE(); diff --git a/src/rpc/util.h b/src/rpc/util.h index fe12b4cbaf4f..cc50c75ddce2 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -266,6 +266,7 @@ struct RPCResult { const std::string m_key_name; //!< Only used for dicts const std::vector m_inner; //!< Only used for arrays or dicts const bool m_optional; + const bool m_skip_type_check; const std::string m_description; const std::string m_cond; @@ -280,6 +281,7 @@ struct RPCResult { m_key_name{std::move(m_key_name)}, m_inner{std::move(inner)}, m_optional{optional}, + m_skip_type_check{false}, m_description{std::move(description)}, m_cond{std::move(cond)} { @@ -300,11 +302,13 @@ struct RPCResult { const std::string m_key_name, const bool optional, const std::string description, - const std::vector inner = {}) + const std::vector inner = {}, + bool skip_type_check = false) : m_type{std::move(type)}, m_key_name{std::move(m_key_name)}, m_inner{std::move(inner)}, m_optional{optional}, + m_skip_type_check{skip_type_check}, m_description{std::move(description)}, m_cond{} { @@ -315,8 +319,9 @@ struct RPCResult { const Type type, const std::string m_key_name, const std::string description, - const std::vector inner = {}) - : RPCResult{type, m_key_name, false, description, inner} {} + const std::vector inner = {}, + bool skip_type_check = false) + : RPCResult{type, m_key_name, false, description, inner, skip_type_check} {} /** Append the sections of the result. */ void ToSections(Sections& sections, OuterType outer_type = OuterType::NONE, const int current_indent = 0) const; diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 7fc43fa5ef2a..7504a616e331 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -189,7 +189,7 @@ static RPCHelpMan getwalletinfo() { {RPCResult::Type::NUM, "duration", "elapsed seconds since scan start"}, {RPCResult::Type::NUM, "progress", "scanning progress percentage [0.0, 1.0]"}, - }}, + }, /*skip_type_check=*/true}, {RPCResult::Type::BOOL, "descriptors", "whether this wallet uses descriptors for scriptPubKey management"}, {RPCResult::Type::BOOL, "external_signer", "whether this wallet is configured to use an external signer such as a hardware wallet"}, RESULT_LAST_PROCESSED_BLOCK,