forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
test: migrate governance inv cache coverage to unit tests #7387
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
thepastaclaw
wants to merge
4
commits into
dashpay:develop
Choose a base branch
from
thepastaclaw:test/governance-inv-cache-unit
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
8080df4
test: expose governance inv request cache internals for unit tests
thepastaclaw 89ed506
test: add governance_inv_tests for inv request cache expiration
thepastaclaw 291f40e
test: cover NetGovernance::Schedule periodic CheckAndRemove path
thepastaclaw c82e3a0
test: drop p2p_governance_invs.py functional test
thepastaclaw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 <governance/governance.h> | ||
| #include <governance/net_governance.h> | ||
| #include <masternode/meta.h> | ||
| #include <masternode/sync.h> | ||
| #include <net.h> | ||
| #include <net_processing.h> | ||
| #include <netfulfilledman.h> | ||
| #include <node/connection_types.h> | ||
| #include <protocol.h> | ||
| #include <scheduler.h> | ||
| #include <streams.h> | ||
| #include <uint256.h> | ||
| #include <util/time.h> | ||
| #include <version.h> | ||
|
|
||
| #include <test/util/setup_common.h> | ||
|
|
||
| #include <boost/test/unit_test.hpp> | ||
|
|
||
| #include <atomic> | ||
| #include <chrono> | ||
| #include <thread> | ||
| #include <vector> | ||
|
|
||
| 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<NetGovernance>( | ||
| 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<std::chrono::seconds>() + 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<CInv>{inv}; | ||
| return s; | ||
| }; | ||
|
|
||
| BOOST_CHECK_EQUAL(m_node.govman->RequestedHashCacheSizeForTesting(), 0U); | ||
|
|
||
| const std::atomic<bool> 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<std::chrono::seconds>() + 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() | ||
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.