diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 2885c5fccb25..8dc73896ae96 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -120,6 +120,7 @@ BITCOIN_TESTS =\ test/flatfile_tests.cpp \ test/fs_tests.cpp \ test/getarg_tests.cpp \ + test/governance_inv_tests.cpp \ test/governance_superblock_tests.cpp \ test/governance_validators_tests.cpp \ test/coinjoin_inouts_tests.cpp \ diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 05d0d103fd7e..ec4419bce7b5 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -32,7 +32,7 @@ namespace { constexpr std::chrono::seconds GOVERNANCE_DELETION_DELAY{10min}; constexpr std::chrono::seconds GOVERNANCE_ORPHAN_EXPIRATION_TIME{10min}; constexpr std::chrono::seconds MAX_TIME_FUTURE_DEVIATION{1h}; -constexpr std::chrono::seconds RELIABLE_PROPAGATION_TIME{1min}; +using governance::RELIABLE_PROPAGATION_TIME; class ScopedLockBool { @@ -608,6 +608,13 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) return true; } +size_t CGovernanceManager::RequestedHashCacheSizeForTesting() const +{ + AssertLockNotHeld(cs_store); + LOCK(cs_store); + return m_requested_hash_time.size(); +} + std::vector CGovernanceManager::GetSyncableVoteInvs(const uint256& nProp, const CBloomFilter& filter) const { LOCK(cs_store); diff --git a/src/governance/governance.h b/src/governance/governance.h index 07a3bc4f606e..dff378c20d47 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -39,6 +39,8 @@ class UniValue; namespace governance { class SuperblockManager; +// How long a requested governance inv hash remains in the request cache. +inline constexpr std::chrono::seconds RELIABLE_PROPAGATION_TIME{60}; } // namespace governance using vote_time_pair_t = std::pair; @@ -297,6 +299,10 @@ class CGovernanceManager : public GovernanceStore */ bool ConfirmInventoryRequest(const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(!cs_store); + /** Test-only accessor: number of inv hashes currently tracked by + * ConfirmInventoryRequest pending expiration in CheckAndRemove. */ + size_t RequestedHashCacheSizeForTesting() const + EXCLUSIVE_LOCKS_REQUIRED(!cs_store); bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_store, !cs_relay); void RelayObject(const CGovernanceObject& obj) diff --git a/src/test/governance_inv_tests.cpp b/src/test/governance_inv_tests.cpp new file mode 100644 index 000000000000..9a5c06561199 --- /dev/null +++ b/src/test/governance_inv_tests.cpp @@ -0,0 +1,219 @@ +// Copyright (c) 2026 The Dash Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include + +#include +#include +#include +#include + +using namespace std::chrono_literals; + +namespace { +struct GovernanceInvSetup : public TestingSetup { + GovernanceInvSetup() : TestingSetup{CBaseChainParams::MAIN} + { + // ConfirmInventoryRequest and CheckAndRemove short-circuit on + // !IsBlockchainSynced(); CheckAndRemove also asserts metaman.IsValid(). + // NetGovernance::AlreadyHave gates on m_gov_manager.IsValid(). + BOOST_REQUIRE(m_node.mn_sync); + m_node.mn_sync->SwitchToNextAsset(); + BOOST_REQUIRE(m_node.mn_sync->IsBlockchainSynced()); + + BOOST_REQUIRE(m_node.mn_metaman); + BOOST_REQUIRE(m_node.mn_metaman->LoadCache(/*load_cache=*/false)); + + BOOST_REQUIRE(m_node.govman); + // Match runtime preconditions: NetGovernance::AlreadyHave claims we + // already have the inv when governance isn't loaded (e.g. + // -disablegovernance), so ConfirmInventoryRequest would never run. + BOOST_REQUIRE(m_node.govman->LoadCache(/*load_cache=*/false)); + + BOOST_REQUIRE(m_node.netfulfilledman); + // Loaded here for the later test that advances GOVERNANCE -> FINISHED; + // the sync notifier asserts netfulfilledman.IsValid(). + BOOST_REQUIRE(m_node.netfulfilledman->LoadCache(/*load_cache=*/false)); + BOOST_REQUIRE(m_node.connman); + BOOST_REQUIRE(m_node.peerman); + + // Intentional unit-test boundary: TestingSetup does not run the + // init.cpp/AppInit startup path that registers the Dash-specific + // handlers, so the INV branch in PeerManagerImpl::AlreadyHave would not + // route MSG_GOVERNANCE_OBJECT[_VOTE] anywhere. Install the same + // NetGovernance handler init.cpp registers so a real INV reaches + // CGovernanceManager::ConfirmInventoryRequest; the startup registration + // itself stays outside this unit test. + m_node.peerman->AddExtraHandler(std::make_unique( + m_node.peerman.get(), *m_node.govman, *m_node.mn_sync, + *m_node.netfulfilledman, *m_node.connman)); + + // Anchor the mocked clock so SetMockTime advances are deterministic. + SetMockTime(1'700'000'000s); + } +}; + +// Replaces the per-type loop in test/functional/p2p_governance_invs.py: an +// inv hash is recorded by ConfirmInventoryRequest, deduplicated while valid, +// and purged by CheckAndRemove only after the reliable propagation timeout. +void CheckInvExpirationCycle(CGovernanceManager& govman, const CInv& inv) +{ + BOOST_CHECK_EQUAL(govman.RequestedHashCacheSizeForTesting(), 0U); + + // First inv is recorded. + BOOST_CHECK(govman.ConfirmInventoryRequest(inv)); + BOOST_CHECK_EQUAL(govman.RequestedHashCacheSizeForTesting(), 1U); + + // Duplicate inv before expiry does not re-insert. + BOOST_CHECK(govman.ConfirmInventoryRequest(inv)); + BOOST_CHECK_EQUAL(govman.RequestedHashCacheSizeForTesting(), 1U); + + // Cleanup before the reliable propagation timeout must not expire the entry. + govman.CheckAndRemove(); + BOOST_CHECK_EQUAL(govman.RequestedHashCacheSizeForTesting(), 1U); + + // Still recorded -> another inv for the same hash is treated as a duplicate. + BOOST_CHECK(govman.ConfirmInventoryRequest(inv)); + BOOST_CHECK_EQUAL(govman.RequestedHashCacheSizeForTesting(), 1U); + + // Advance past the reliable propagation timeout and clean: the entry is evicted. + SetMockTime(GetTime() + governance::RELIABLE_PROPAGATION_TIME + 1s); + govman.CheckAndRemove(); + BOOST_CHECK_EQUAL(govman.RequestedHashCacheSizeForTesting(), 0U); + + // After eviction the same inv must be accepted and recorded again. + BOOST_CHECK(govman.ConfirmInventoryRequest(inv)); + BOOST_CHECK_EQUAL(govman.RequestedHashCacheSizeForTesting(), 1U); +} +} // namespace + +BOOST_FIXTURE_TEST_SUITE(governance_inv_tests, GovernanceInvSetup) + +BOOST_AUTO_TEST_CASE(object_inv_request_expiration) +{ + CheckInvExpirationCycle(*m_node.govman, CInv{MSG_GOVERNANCE_OBJECT, uint256S("01")}); +} + +BOOST_AUTO_TEST_CASE(vote_inv_request_expiration) +{ + CheckInvExpirationCycle(*m_node.govman, CInv{MSG_GOVERNANCE_OBJECT_VOTE, uint256S("02")}); +} + +// Replaces the end-to-end check the old functional test performed via real P2P: +// a governance INV delivered to PeerManager::ProcessMessage must reach +// CGovernanceManager::ConfirmInventoryRequest through PeerManagerImpl::AlreadyHave +// and the registered NetGovernance handler. Exercising the full inbound INV path +// keeps the wiring from regressing if PeerManager's dispatch ever changes. +BOOST_AUTO_TEST_CASE(peerman_inv_routes_to_governance_request_cache) +{ + LOCK(NetEventsInterface::g_msgproc_mutex); + + in_addr peer_in_addr{}; + peer_in_addr.s_addr = htonl(0x01020304); + CNode peer{/*id=*/0, + /*sock=*/nullptr, + /*addrIn=*/CAddress{CService{peer_in_addr, 8333}, NODE_NETWORK}, + /*nKeyedNetGroupIn=*/0, + /*nLocalHostNonceIn=*/0, + /*addrBindIn=*/CAddress{}, + /*addrNameIn=*/std::string{}, + /*conn_type_in=*/ConnectionType::INBOUND, + /*inbound_onion=*/false}; + peer.nVersion = PROTOCOL_VERSION; + peer.SetCommonVersion(PROTOCOL_VERSION); + m_node.peerman->InitializeNode(peer, NODE_NETWORK); + peer.fSuccessfullyConnected = true; + + auto make_inv_stream = [](const CInv& inv) { + CDataStream s{SER_NETWORK, PROTOCOL_VERSION}; + s << std::vector{inv}; + return s; + }; + + BOOST_CHECK_EQUAL(m_node.govman->RequestedHashCacheSizeForTesting(), 0U); + + const std::atomic interrupt_dummy{false}; + + // Object INV: PeerManager -> AlreadyHave -> NetGovernance -> ConfirmInventoryRequest. + { + const CInv inv{MSG_GOVERNANCE_OBJECT, uint256S("06")}; + auto stream = make_inv_stream(inv); + m_node.peerman->ProcessMessage(peer, NetMsgType::INV, stream, + /*time_received=*/std::chrono::microseconds{0}, + interrupt_dummy); + BOOST_CHECK_EQUAL(m_node.govman->RequestedHashCacheSizeForTesting(), 1U); + + // Duplicate INV with the same hash must not grow the cache. + auto dup_stream = make_inv_stream(inv); + m_node.peerman->ProcessMessage(peer, NetMsgType::INV, dup_stream, + std::chrono::microseconds{0}, interrupt_dummy); + BOOST_CHECK_EQUAL(m_node.govman->RequestedHashCacheSizeForTesting(), 1U); + } + + // Vote INV travels the same path and adds a separate entry. + { + const CInv vote_inv{MSG_GOVERNANCE_OBJECT_VOTE, uint256S("07")}; + auto stream = make_inv_stream(vote_inv); + m_node.peerman->ProcessMessage(peer, NetMsgType::INV, stream, + std::chrono::microseconds{0}, interrupt_dummy); + BOOST_CHECK_EQUAL(m_node.govman->RequestedHashCacheSizeForTesting(), 2U); + } + + m_node.peerman->FinalizeNode(peer); +} + +// Pins the periodic-cleanup wiring the deleted functional test exercised via +// node.mockscheduler: NetGovernance::Schedule queues a task that calls +// CGovernanceManager::CheckAndRemove, so an expired inv request is purged +// without any manual CheckAndRemove call. +BOOST_AUTO_TEST_CASE(net_governance_schedule_drives_check_and_remove) +{ + // NetGovernance::Schedule's periodic callback short-circuits on + // !m_node_sync.IsSynced(); advance from GOVERNANCE to FINISHED. + m_node.mn_sync->SwitchToNextAsset(); + BOOST_REQUIRE(m_node.mn_sync->IsSynced()); + + // Pre-load an entry that has already passed the reliable propagation timeout so + // the very next CheckAndRemove evicts it. + const CInv inv{MSG_GOVERNANCE_OBJECT, uint256S("05")}; + BOOST_REQUIRE(m_node.govman->ConfirmInventoryRequest(inv)); + BOOST_CHECK_EQUAL(m_node.govman->RequestedHashCacheSizeForTesting(), 1U); + SetMockTime(GetTime() + governance::RELIABLE_PROPAGATION_TIME + 1s); + + // Drive a dedicated scheduler so the assertion is independent of + // m_node.scheduler's existing workload. + CScheduler scheduler; + NetGovernance net_gov(m_node.peerman.get(), *m_node.govman, *m_node.mn_sync, + *m_node.netfulfilledman, *m_node.connman); + net_gov.Schedule(scheduler); + std::thread worker([&] { scheduler.serviceQueue(); }); + + // First periodic fire is at +5min; bump the clock so the queue is ready. + scheduler.MockForward(std::chrono::minutes{5}); + + // Queue a stop marker after the mocked-forward tasks; due cleanup runs first. + scheduler.scheduleFromNow([&scheduler] { scheduler.stop(); }, 1ms); + worker.join(); + + BOOST_CHECK_EQUAL(m_node.govman->RequestedHashCacheSizeForTesting(), 0U); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/test/functional/p2p_governance_invs.py b/test/functional/p2p_governance_invs.py deleted file mode 100755 index 4679d18cc25d..000000000000 --- a/test/functional/p2p_governance_invs.py +++ /dev/null @@ -1,62 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) 2024 The Dash Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. - -""" -Test inv expiration for governance objects/votes -""" - -from test_framework.messages import ( - CInv, - msg_inv, - MSG_GOVERNANCE_OBJECT, - MSG_GOVERNANCE_OBJECT_VOTE, -) -from test_framework.p2p import P2PInterface -from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import force_finish_mnsync - -RELIABLE_PROPAGATION_TIME = 60 # src/governance/governance.cpp -DATA_CLEANUP_TIME = 5 * 60 # src/init.cpp -MSG_INV_ADDED = 'CGovernanceManager::ConfirmInventoryRequest added {} inv hash to m_requested_hash_time' - -class GovernanceInvsTest(BitcoinTestFramework): - def set_test_params(self): - self.num_nodes = 1 - - def run_test(self): - node = self.nodes[0] - force_finish_mnsync(node) - inv = msg_inv([CInv(MSG_GOVERNANCE_OBJECT, 1)]) - self.test_request_expiration(inv, "object") - inv = msg_inv([CInv(MSG_GOVERNANCE_OBJECT_VOTE, 2)]) - self.test_request_expiration(inv, "vote") - - def test_request_expiration(self, inv, name): - msg = MSG_INV_ADDED.format(name) - node = self.nodes[0] - peer = node.add_p2p_connection(P2PInterface()) - self.log.info(f"Send dummy governance {name} inv and make sure it's added to requested map") - with node.assert_debug_log([msg]): - peer.send_message(inv) - self.log.info(f"Send dummy governance {name} inv again and make sure it's not added because we know about it already") - with node.assert_debug_log([], [msg]): - peer.send_message(inv) - self.log.info("Force internal cleanup") - with node.assert_debug_log(['UpdateCachesAndClean']): - node.mockscheduler(DATA_CLEANUP_TIME + 1) - self.log.info(f"Send dummy governance {name} inv again and make sure it's not added because we still know about it") - with node.assert_debug_log([], [msg]): - peer.send_message(inv) - self.log.info(f"Bump mocktime, force internal cleanup, send dummy governance {name} inv again and make sure it's accepted again") - self.bump_mocktime(RELIABLE_PROPAGATION_TIME + 1, nodes=[node]) - with node.assert_debug_log(['UpdateCachesAndClean']): - node.mockscheduler(DATA_CLEANUP_TIME + 1) - with node.assert_debug_log([msg]): - peer.send_message(inv) - node.disconnect_p2ps() - - -if __name__ == '__main__': - GovernanceInvsTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index b398e6e55e47..a3c9c9a15119 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -344,7 +344,6 @@ 'feature_cltv.py', 'feature_new_quorum_type_activation.py', 'feature_governance_objects.py', - 'p2p_governance_invs.py', 'p2p_govsync_bloom.py', 'rpc_uptime.py', 'feature_discover.py',