From 05732aceafa831cca485911b63fb63bcedc8ca82 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 11 Dec 2024 15:19:53 +0700 Subject: [PATCH 1/6] feat: implement functional tests for RPC getblockheaders --- test/functional/rpc_blockchain.py | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 287641142d..093be59e6b 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -9,6 +9,7 @@ Test the following RPCs: - getchaintxstats - gettxoutsetinfo - getblockheader + - getblockheaders - getdifficulty - getnetworkhashps - waitforblockheight @@ -76,7 +77,8 @@ class BlockchainTest(BitcoinTestFramework): self._test_getblockchaininfo() self._test_getchaintxstats() self._test_gettxoutsetinfo() - self._test_getblockheader() + self._test_getblockheader(rpc_name='getblockheader') + self._test_getblockheader(rpc_name='getblockheaders') self._test_getdifficulty() self._test_getnetworkhashps() self._test_stopatheight() @@ -319,17 +321,20 @@ class BlockchainTest(BitcoinTestFramework): # Unknown hash_type raises an error assert_raises_rpc_error(-8, "'foo hash' is not a valid hash_type", node.gettxoutsetinfo, "foo hash") - def _test_getblockheader(self): - self.log.info("Test getblockheader") + def _test_getblockheader(self, rpc_name): + self.log.info(f"Test {rpc_name}") node = self.nodes[0] - - assert_raises_rpc_error(-8, "hash must be of length 64 (not 8, for 'nonsense')", node.getblockheader, "nonsense") - assert_raises_rpc_error(-8, "hash must be hexadecimal string (not 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844')", node.getblockheader, "ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844") - assert_raises_rpc_error(-5, "Block not found", node.getblockheader, "0cf7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844") + rpc_call = getattr(node, rpc_name) + assert_raises_rpc_error(-8, "hash must be of length 64 (not 8, for 'nonsense')", rpc_call, "nonsense") + assert_raises_rpc_error(-8, "hash must be hexadecimal string (not 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844')", rpc_call, "ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844") + assert_raises_rpc_error(-5, "Block not found", rpc_call, "0cf7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844") besthash = node.getbestblockhash() secondbesthash = node.getblockhash(HEIGHT - 1) - header = node.getblockheader(blockhash=besthash) + header = rpc_call(blockhash=besthash) + if rpc_name == 'getblockheaders': + assert_equal(len(header), 1) + header = header[0] assert_equal(header['hash'], besthash) assert_equal(header['height'], HEIGHT) @@ -349,15 +354,19 @@ class BlockchainTest(BitcoinTestFramework): assert isinstance(header['difficulty'], Decimal) # Test with verbose=False, which should return the header as hex. - header_hex = node.getblockheader(blockhash=besthash, verbose=False) + header_hex = rpc_call(blockhash=besthash, verbose=False) + if rpc_name == 'getblockheaders': + header_hex = header_hex[0] + assert_is_hex_string(header_hex) header = from_hex(CBlockHeader(), header_hex) header.calc_sha256() assert_equal(header.hash, besthash) - assert 'previousblockhash' not in node.getblockheader(node.getblockhash(0)) - assert 'nextblockhash' not in node.getblockheader(node.getbestblockhash()) + if rpc_name == 'getblockheader': + assert 'previousblockhash' not in node.getblockheader(node.getblockhash(0)) + assert 'nextblockhash' not in node.getblockheader(node.getbestblockhash()) def _test_getdifficulty(self): self.log.info("Test getdifficulty") From 59ddac5656a44b1a5c4c317ac9317b0d47a01951 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 11 Dec 2024 17:02:16 +0700 Subject: [PATCH 2/6] feat: hide deprecated RPC from help and add TODOes to remove them --- src/rpc/coinjoin.cpp | 4 +++- src/rpc/masternode.cpp | 8 +++++--- src/rpc/net.cpp | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp index 8e477ce8ce..81f6c37933 100644 --- a/src/rpc/coinjoin.cpp +++ b/src/rpc/coinjoin.cpp @@ -365,6 +365,7 @@ static RPCHelpMan coinjoinsalt_set() } #endif // ENABLE_WALLET +// TODO: remove it completely static RPCHelpMan getpoolinfo() { return RPCHelpMan{"getpoolinfo", @@ -469,7 +470,6 @@ void RegisterCoinJoinRPCCommands(CRPCTable &t) static const CRPCCommand commands[] = { // category actor (function) // --------------------- ----------------------- - { "dash", &getpoolinfo, }, { "dash", &getcoinjoininfo, }, #ifdef ENABLE_WALLET { "dash", &coinjoin, }, @@ -480,6 +480,8 @@ static const CRPCCommand commands[] = { "dash", &coinjoinsalt_generate, }, { "dash", &coinjoinsalt_get, }, { "dash", &coinjoinsalt_set, }, + + { "hidden", &getpoolinfo, }, #endif // ENABLE_WALLET }; // clang-format on diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index b18f7ef06d..5a1e2cb826 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -133,6 +133,7 @@ static UniValue GetNextMasternodeForPayment(const CChain& active_chain, CDetermi return obj; } +// TODO: drop it static RPCHelpMan masternode_winner() { return RPCHelpMan{"masternode winner", @@ -153,6 +154,7 @@ static RPCHelpMan masternode_winner() }; } +// TODO: drop it static RPCHelpMan masternode_current() { return RPCHelpMan{"masternode current", @@ -226,7 +228,7 @@ static RPCHelpMan masternode_status() } UniValue mnObj(UniValue::VOBJ); - // keep compatibility with legacy status for now (might get deprecated/removed later) + // keep compatibility with legacy status for now (TODO: get deprecated/removed later) mnObj.pushKV("outpoint", node.mn_activeman->GetOutPoint().ToStringShort()); mnObj.pushKV("service", node.mn_activeman->GetService().ToStringAddrPort()); auto dmn = CHECK_NONFATAL(node.dmnman)->GetListAtChainTip().GetMN(node.mn_activeman->GetProTxHash()); @@ -749,8 +751,8 @@ static const CRPCCommand commands[] = { "dash", &masternode_status, }, { "dash", &masternode_payments, }, { "dash", &masternode_winners, }, - { "dash", &masternode_current, }, - { "dash", &masternode_winner, }, + { "hidden", &masternode_current, }, + { "hidden", &masternode_winner, }, }; // clang-format on for (const auto& command : commands) { diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index f61b9147c4..9667a48e98 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -241,7 +241,7 @@ static RPCHelpMan getpeerinfo() obj.pushKV("masternode", stats.m_masternode_connection); if (fStateStats) { if (IsDeprecatedRPCEnabled("banscore")) { - // banscore is deprecated in v21 for removal in v22 + // TODO: banscore is deprecated in v21 for removal in v22, maybe impossible due to usages in p2p_quorum_data.py obj.pushKV("banscore", statestats.m_misbehavior_score); } obj.pushKV("startingheight", statestats.m_starting_height); From 1f5fa7e7cfb1142cfda5f27a9a02552a5ee57f16 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 11 Dec 2024 20:26:46 +0700 Subject: [PATCH 3/6] feat: enable linter coverage for functional tests --- ci/test/00_setup_env_native_qt5.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ci/test/00_setup_env_native_qt5.sh b/ci/test/00_setup_env_native_qt5.sh index 4a40030639..bf369f1538 100755 --- a/ci/test/00_setup_env_native_qt5.sh +++ b/ci/test/00_setup_env_native_qt5.sh @@ -9,8 +9,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_qt5 export PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libdbus-1-dev libharfbuzz-dev" export DEP_OPTS="NO_UPNP=1 DEBUG=1" -# TODO: we have few rpcs that aren't covered by any test, re-enable the line below once it's fixed -# export TEST_RUNNER_EXTRA="--previous-releases --coverage --extended --exclude feature_pruning,feature_dbcrash" # Run extended tests so that coverage does not fail, but exclude the very slow dbcrash +export TEST_RUNNER_EXTRA="--previous-releases --coverage --extended --exclude feature_pruning,feature_dbcrash" # Run extended tests so that coverage does not fail, but exclude the very slow dbcrash export RUN_UNIT_TESTS_SEQUENTIAL="true" export RUN_UNIT_TESTS="false" export GOAL="install" From 865b24ea00a17cc61cf8bb55499cb384761cb94c Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 12 Dec 2024 18:14:51 +0700 Subject: [PATCH 4/6] feat: hide cleardiscouraged RPC so far as it no intent to use by regular users --- src/rpc/net.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 9667a48e98..ffa2e7b0b5 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -1126,10 +1126,10 @@ static const CRPCCommand commands[] = { "network", &setban, }, { "network", &listbanned, }, { "network", &clearbanned, }, - { "network", &cleardiscouraged, }, { "network", &setnetworkactive, }, { "network", &getnodeaddresses, }, + { "hidden", &cleardiscouraged, }, { "hidden", &addconnection, }, { "hidden", &addpeeraddress, }, { "hidden", &sendmsgtopeer }, From f0decc879085e9689c780112322167dea63409e7 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 12 Dec 2024 18:30:15 +0700 Subject: [PATCH 5/6] feat: add unit test for ClearDiscouraged --- src/test/denialofservice_tests.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index def090aa63..3d34953280 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -450,6 +450,8 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) peerLogic->Misbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD); BOOST_CHECK(peerLogic->SendMessages(&dummyNode)); BOOST_CHECK(banman->IsDiscouraged(addr)); + banman->ClearDiscouraged(); + BOOST_CHECK(!banman->IsDiscouraged(addr)); peerLogic->FinalizeNode(dummyNode); } From 2e509b96c4870d2c5717d15d7d3c9614c31ae41e Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 12 Dec 2024 18:36:09 +0700 Subject: [PATCH 6/6] fix: add a workaround for RPC getmerkleblocks, debug, coinjoinsalt, voteraw We indeed doesn't have functional tests for them yet, but this linter will help to avoid adding new RPCs without tests --- test/functional/test_runner.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index da9714ebb0..4247f87142 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -891,6 +891,14 @@ class RPCCoverage(): # Consider RPC generate covered, because it is overloaded in # test_framework/test_node.py and not seen by the coverage check. covered_cmds = set({'generate'}) + # TODO: implement functional tests for coinjoinsalt + covered_cmds.add('coinjoinsalt') + # TODO: implement functional tests for voteraw + covered_cmds.add('voteraw') + # TODO: implement functional tests for getmerkleblocks + covered_cmds.add('getmerkleblocks') + # TODO: drop it with v23+: remove `debug` in favour of `logging` + covered_cmds.add('debug') if not os.path.isfile(coverage_ref_filename): raise RuntimeError("No coverage reference found")