From 81650eb205012f72a80f84198568b48cc2b0f7c0 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 7 Oct 2026 02:46:45 -0500 Subject: [PATCH 1/3] fix: look up the cache multimap entry after pruning CacheMultiMap::Insert took a reference to the key's inner map before calling PruneLast(). When the cache is full and its oldest item is that key's only value, PruneLast() erases the key from mapIndex, so the following emplace wrote through a dangling reference. The new item was also left out of the index, so it could never be found or erased. The governance orphan vote cache, the only user, does not hit this yet because every vote for a parent compares equal, so a key never holds more than its first value. The comparator fix in the next commit makes this path reachable from peer-relayed orphan votes. Check for duplicates first, prune, and only then look up or create the key's entry. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/cachemultimap.h | 11 +++-------- src/test/cachemultimap_tests.cpp | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/cachemultimap.h b/src/cachemultimap.h index 88f3f12c2bea..768faabfd729 100644 --- a/src/cachemultimap.h +++ b/src/cachemultimap.h @@ -86,22 +86,17 @@ class CacheMultiMap bool Insert(const K& key, const V& value) { - map_it mit = mapIndex.find(key); - if(mit == mapIndex.end()) { - mit = mapIndex.emplace(key, it_map_t()).first; - } - it_map_t& mapIt = mit->second; - - if(mapIt.count(value) > 0) { + if (map_cit mit = mapIndex.find(key); mit != mapIndex.end() && mit->second.count(value) > 0) { // Don't insert duplicates return false; } + // Pruning may erase this key's entry in mapIndex, so look it up only afterwards if(listItems.size() == nMaxSize) { PruneLast(); } listItems.push_front(item_t(key, value)); - mapIt.emplace(value, listItems.begin()); + mapIndex[key].emplace(value, listItems.begin()); return true; } diff --git a/src/test/cachemultimap_tests.cpp b/src/test/cachemultimap_tests.cpp index 46712b831b98..ec85d3bacbe4 100644 --- a/src/test/cachemultimap_tests.cpp +++ b/src/test/cachemultimap_tests.cpp @@ -166,4 +166,26 @@ BOOST_AUTO_TEST_CASE(cachemultimap_test) BOOST_CHECK(Compare(cmmapTest1, mapTest4)); } +BOOST_AUTO_TEST_CASE(cachemultimap_insert_evicts_sole_value_of_same_key) +{ + CacheMultiMap cmmap(2); + BOOST_CHECK(cmmap.Insert(1, 10)); + BOOST_CHECK(cmmap.Insert(2, 20)); + + // The cache is full and its oldest item is key 1's only value + BOOST_CHECK(cmmap.Insert(1, 11)); + + BOOST_CHECK_EQUAL(cmmap.GetSize(), 2U); + std::vector vecVals; + BOOST_CHECK(cmmap.GetAll(1, vecVals)); + BOOST_CHECK(vecVals == std::vector{11}); + BOOST_CHECK(cmmap.HasKey(2)); + + // The index still tracks the new item, so erasing it empties the cache + cmmap.Erase(1); + cmmap.Erase(2); + BOOST_CHECK_EQUAL(cmmap.GetSize(), 0U); + BOOST_CHECK(!cmmap.HasKey(1)); +} + BOOST_AUTO_TEST_SUITE_END() From 94c969f2a2d5b88258f7d090eadbed9e5dbe3989 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 7 Oct 2026 01:46:53 -0500 Subject: [PATCH 2/3] fix: preserve distinct orphan governance votes CGovernanceVote::operator< has returned false for every pair of votes since it was introduced in 2016: each step returns early unless the previous field compared less-than, and then requires that same field to compare equal. OrphanVote ordering delegates to it, so the orphan vote cache treated every vote for a parent as a duplicate of the first one and kept a single vote per parent. When the parent object arrived, only that vote was replayed and the others were lost. Compare the identifying fields lexicographically instead. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/governance/vote.cpp | 31 ++------------- src/test/governance_vote_processing_tests.cpp | 38 +++++++++++++++++++ 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/src/governance/vote.cpp b/src/governance/vote.cpp index c0081d7bb053..f8baaf1ac04b 100644 --- a/src/governance/vote.cpp +++ b/src/governance/vote.cpp @@ -16,6 +16,8 @@ #include #include +#include + static_assert(CGovernanceVote::COMPACT_SIG_SIZE == CPubKey::COMPACT_SIGNATURE_SIZE); static_assert(CGovernanceVote::BLS_SIG_SIZE == CBLSSignature::SerSize); @@ -262,31 +264,6 @@ bool operator==(const CGovernanceVote& vote1, const CGovernanceVote& vote2) bool operator<(const CGovernanceVote& vote1, const CGovernanceVote& vote2) { - bool fResult = (vote1.masternodeOutpoint < vote2.masternodeOutpoint); - if (!fResult) { - return false; - } - fResult = (vote1.masternodeOutpoint == vote2.masternodeOutpoint); - - fResult = fResult && (vote1.nParentHash < vote2.nParentHash); - if (!fResult) { - return false; - } - fResult = (vote1.nParentHash == vote2.nParentHash); - - fResult = fResult && (vote1.nVoteOutcome < vote2.nVoteOutcome); - if (!fResult) { - return false; - } - fResult = (vote1.nVoteOutcome == vote2.nVoteOutcome); - - fResult = fResult && (vote1.nVoteSignal == vote2.nVoteSignal); - if (!fResult) { - return false; - } - fResult = (vote1.nVoteSignal == vote2.nVoteSignal); - - fResult = fResult && (vote1.nTime < vote2.nTime); - - return fResult; + return std::tie(vote1.masternodeOutpoint, vote1.nParentHash, vote1.nVoteOutcome, vote1.nVoteSignal, vote1.nTime) < + std::tie(vote2.masternodeOutpoint, vote2.nParentHash, vote2.nVoteOutcome, vote2.nVoteSignal, vote2.nTime); } diff --git a/src/test/governance_vote_processing_tests.cpp b/src/test/governance_vote_processing_tests.cpp index 57c05c103b8f..5705e272991d 100644 --- a/src/test/governance_vote_processing_tests.cpp +++ b/src/test/governance_vote_processing_tests.cpp @@ -282,6 +282,44 @@ BOOST_AUTO_TEST_CASE(orphan_vote_is_cached_and_applied_when_parent_arrives) BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 1); } +BOOST_AUTO_TEST_CASE(orphan_funding_votes_from_multiple_masternodes_are_applied) +{ + CKey second_voting_key; + CBLSSecretKey second_operator_key; + const auto protx{CreateProRegTx(*m_node.chainman, utxos, /*port=*/2, payout_script(), coinbaseKey, + second_voting_key, second_operator_key)}; + MineBlock({protx}); + const COutPoint second_collateral{protx.GetHash(), 0}; + BOOST_REQUIRE(tip_mn_list().GetMNByCollateral(second_collateral) != nullptr); + + auto& govman = *m_node.govman; + const uint256 parent_hash{MakeProposal(uint256{}).GetHash()}; + CGovernanceVote first_vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES)}; + SignWithVotingKey(first_vote, mn_voting_key); + CGovernanceVote second_vote{second_collateral, parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES}; + SignWithVotingKey(second_vote, second_voting_key); + + for (const auto& vote : {first_vote, second_vote, second_vote}) { + BOOST_REQUIRE(vote.IsValid(tip_mn_list(), /*useVotingKey=*/true)); + CGovernanceException exception; + uint256 hash_to_request; + BOOST_CHECK(!govman.ProcessVote(vote, exception, hash_to_request)); + BOOST_CHECK_EQUAL(exception.GetType(), GOVERNANCE_EXCEPTION_WARNING); + BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 0); + } + + const uint256 collateral_hash{ConfirmProposalCollateral(parent_hash)}; + CGovernanceObject proposal{MakeProposal(collateral_hash)}; + BOOST_REQUIRE_EQUAL(proposal.GetHash(), parent_hash); + govman.AddGovernanceObject(proposal, /*peer_str=*/""); + const auto stored{govman.FindConstGovernanceObject(parent_hash)}; + BOOST_REQUIRE(stored != nullptr); + BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 2); + const auto current_votes{govman.GetCurrentVotes(parent_hash, COutPoint{})}; + BOOST_REQUIRE_EQUAL(current_votes.size(), 2U); + BOOST_CHECK(govman.GetOrphanVoteObjectHashes().empty()); +} + // Votes are only counted if they carry a signature from a registered masternode. BOOST_AUTO_TEST_CASE(unsigned_and_unknown_masternode_votes_are_rejected) { From df7d7bbdb0ef3343f0d128dbcfe36f6062e1dcf4 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 7 Oct 2026 05:07:07 -0500 Subject: [PATCH 3/3] fix: bound authenticated orphan governance backlogs Now that distinct orphan votes are kept, one masternode can sign many timestamp variants of the same vote for a parent the node does not have yet, and every one of them would be replayed when the parent arrives. Bound the cache at intake instead: for a given masternode, parent, signal and outcome, keep only the newest vote per authenticating key (voting or operator). Cached entries that have expired or no longer verify against the current masternode list are dropped and never suppress a new vote. OrphanVote ordering breaks ties on signature size so the same vote signed by both keys is kept twice; the signature encoding tells which key authenticated a vote, since a compact signature only verifies against the voting key and a BLS signature only against the operator key. CacheMultiMap gains a range lookup for the per-parent search. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/cachemultimap.h | 12 ++ src/governance/governance.cpp | 40 ++++- src/governance/governance.h | 2 + src/governance/vote.h | 1 + src/test/cachemultimap_tests.cpp | 17 ++ src/test/governance_vote_processing_tests.cpp | 151 ++++++++++++++++++ 6 files changed, 220 insertions(+), 3 deletions(-) diff --git a/src/cachemultimap.h b/src/cachemultimap.h index 768faabfd729..2901a2df0446 100644 --- a/src/cachemultimap.h +++ b/src/cachemultimap.h @@ -132,6 +132,18 @@ class CacheMultiMap return true; } + bool GetAll(const K& key, const V& lower, const V& upper, std::vector& vecValues) const + { + const map_cit mit = mapIndex.find(key); + if (mit == mapIndex.end() || upper < lower) return false; + const it_map_t& mapIt = mit->second; + const auto range_end = mapIt.upper_bound(upper); + for (auto it = mapIt.lower_bound(lower); it != range_end; ++it) { + vecValues.push_back(it->second->value); + } + return true; + } + void GetKeys(std::vector& vecKeys) { for(map_cit it = mapIndex.begin(); it != mapIndex.end(); ++it) { diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 22dafc4214a1..ceb99f33a42d 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -26,6 +26,7 @@ #include +#include #include const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-17"; @@ -37,6 +38,39 @@ constexpr std::chrono::seconds MAX_TIME_FUTURE_DEVIATION{1h}; // Margin below MAX_TIME_FUTURE_DEVIATION so lagging-clock peers don't reject near-limit objects. constexpr std::chrono::seconds RELIABLE_PROPAGATION_TIME{60}; +// The encoding chooses which key to verify: IsValidForUnknownParent accepts only compact +// signatures by the voting key and BLS signatures by the operator key. +bool UsesVotingKey(const CGovernanceVote& vote) +{ + return vote.GetSignal() == VOTE_SIGNAL_FUNDING && vote.GetSignatureSize() == CGovernanceVote::COMPACT_SIG_SIZE; +} + +bool InsertOrphanVote(CacheMultiMap& cache, const CDeterministicMNList& tip_mn_list, + const governance::OrphanVote& orphan_vote) +{ + auto lower{orphan_vote}; + auto upper{orphan_vote}; + lower.vote.SetTime(std::numeric_limits::min()); + lower.vote.SetSignature({}); + upper.vote.SetTime(std::numeric_limits::max()); + upper.vote.SetSignature(std::vector(CGovernanceVote::BLS_SIG_SIZE)); + std::vector variants; + const auto& parent = orphan_vote.vote.GetParentHash(); + cache.GetAll(parent, lower, upper, variants); + const bool use_voting_key{UsesVotingKey(orphan_vote.vote)}; + for (const auto& cached : variants) { + const bool cached_uses_voting_key{UsesVotingKey(cached.vote)}; + // Only current-key authentication may suppress a new vote. + if (cached.expiration < Now() || !cached.vote.IsValid(tip_mn_list, cached_uses_voting_key)) { + cache.Erase(parent, cached); + } else if (cached_uses_voting_key == use_voting_key) { + if (cached.vote.GetTimestamp() >= orphan_vote.vote.GetTimestamp()) return false; + cache.Erase(parent, cached); + } + } + return cache.Insert(parent, orphan_vote); +} + bool IsSyncableObject(const std::shared_ptr& govobj) { const auto& obj = *Assert(govobj); @@ -820,9 +854,8 @@ bool CGovernanceManager::ProcessVote(const CGovernanceVote& vote, CGovernanceExc AssertLockNotHeld(cs_store); hashToRequest = uint256{}; - const auto tip_mn_list{m_dmnman.GetListAtChainTip()}; - LOCK(cs_store); + const auto tip_mn_list{m_dmnman.GetListAtChainTip()}; uint256 nHashVote = vote.GetHash(); uint256 nHashGovobj = vote.GetParentHash(); @@ -846,7 +879,8 @@ bool CGovernanceManager::ProcessVote(const CGovernanceVote& vote, CGovernanceExc // No penalty: the vote is signed by a masternode, it just arrived before its parent object, // which routinely happens during governance sync. Misbehaviour scores never decay. exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_WARNING); - if (cmmapOrphanVotes.Insert(nHashGovobj, governance::OrphanVote{vote, Now() + GOVERNANCE_ORPHAN_EXPIRATION_TIME})) { + if (InsertOrphanVote(cmmapOrphanVotes, tip_mn_list, + governance::OrphanVote{vote, Now() + GOVERNANCE_ORPHAN_EXPIRATION_TIME})) { hashToRequest = nHashGovobj; // Caller should request this object } LogPrint(BCLog::GOBJECT, "%s\n", msg); diff --git a/src/governance/governance.h b/src/governance/governance.h index 04536cb5ef04..a0e212763bd2 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -56,6 +56,8 @@ struct OrphanVote { inline bool operator<(const OrphanVote& lhs, const OrphanVote& rhs) { + // Equal signed fields may arrive authenticated by different voting/operator keys. + if (lhs.vote == rhs.vote) return lhs.vote.GetSignatureSize() < rhs.vote.GetSignatureSize(); return lhs.vote < rhs.vote; } } // namespace governance diff --git a/src/governance/vote.h b/src/governance/vote.h index 36df8a199fed..4beffef728cc 100644 --- a/src/governance/vote.h +++ b/src/governance/vote.h @@ -145,6 +145,7 @@ class CGovernanceVote } void SetSignature(const std::vector& vchSigIn) { vchSig = vchSigIn; } + size_t GetSignatureSize() const { return vchSig.size(); } bool CheckSignature(const CKeyID& keyID) const; bool CheckSignature(const CBLSPublicKey& pubKey) const; diff --git a/src/test/cachemultimap_tests.cpp b/src/test/cachemultimap_tests.cpp index ec85d3bacbe4..6d375e499e3c 100644 --- a/src/test/cachemultimap_tests.cpp +++ b/src/test/cachemultimap_tests.cpp @@ -188,4 +188,21 @@ BOOST_AUTO_TEST_CASE(cachemultimap_insert_evicts_sole_value_of_same_key) BOOST_CHECK(!cmmap.HasKey(1)); } +BOOST_AUTO_TEST_CASE(cachemultimap_get_all_in_value_range) +{ + CacheMultiMap cmmap(5); + for (const int value : {1, 2, 3, 4}) + BOOST_REQUIRE(cmmap.Insert(1, value)); + BOOST_REQUIRE(cmmap.Insert(2, 2)); + std::vector values; + BOOST_CHECK(cmmap.GetAll(1, 2, 3, values)); + BOOST_CHECK(values == (std::vector{2, 3})); + values.clear(); + BOOST_CHECK(cmmap.GetAll(1, 5, 6, values)); + BOOST_CHECK(values.empty()); + BOOST_CHECK(!cmmap.GetAll(3, 1, 4, values)); + BOOST_CHECK(!cmmap.GetAll(1, 3, 2, values)); + BOOST_CHECK(values.empty()); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/governance_vote_processing_tests.cpp b/src/test/governance_vote_processing_tests.cpp index 5705e272991d..f01e5c425c41 100644 --- a/src/test/governance_vote_processing_tests.cpp +++ b/src/test/governance_vote_processing_tests.cpp @@ -5,6 +5,8 @@ #include #include #include +#include +#include #include #include #include @@ -74,6 +76,24 @@ CGovernanceObject MakeFailedTrigger(const COutPoint& mn_collateral, const CBLSSe return govobj; } +struct GovernanceVoteNotifications final : CValidationInterface { + mutable Mutex cs; + std::vector hashes GUARDED_BY(cs); + + void NotifyGovernanceVote(const std::shared_ptr&, + const std::shared_ptr& vote) override EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + LOCK(cs); + hashes.push_back(vote->GetHash()); + } + + std::vector GetHashes() const EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + LOCK(cs); + return hashes; + } +}; + // A chain with one registered masternode whose voting (ECDSA) and operator (BLS) keys are known // to the test, so votes can be signed for real and CGovernanceVote::CheckSignature is actually // reached. Without a populated masternode list every CGovernanceVote::IsValid() call would @@ -83,6 +103,7 @@ struct GovernanceVoteSetup : public TestChainSetup { CBLSSecretKey mn_operator_key; COutPoint mn_collateral; SimpleUTXOMap utxos; + std::shared_ptr vote_notifications; //! Fixed so a proposal can be rebuilt bit-for-bit (and keep its hash) once its fee tx is known. int64_t proposal_time{0}; std::string proposal_payment_address; @@ -135,6 +156,8 @@ struct GovernanceVoteSetup : public TestChainSetup { *m_node.sbman, *m_node.dmnman, *m_node.mn_sync); BOOST_REQUIRE(m_node.govman->LoadCache(/*load_cache=*/false)); + vote_notifications = std::make_shared(); + RegisterSharedValidationInterface(vote_notifications); proposal_time = TicksSinceEpoch(GetAdjustedTime()); CKey payment_key; @@ -144,6 +167,11 @@ struct GovernanceVoteSetup : public TestChainSetup { void TearDown() { + if (vote_notifications) { + SyncWithValidationInterfaceQueue(); + UnregisterSharedValidationInterface(vote_notifications); + vote_notifications.reset(); + } // govman holds a reference to node.sbman, so it must go first (matches PrepareShutdown // ordering in init.cpp). m_node.govman.reset(); @@ -320,6 +348,129 @@ BOOST_AUTO_TEST_CASE(orphan_funding_votes_from_multiple_masternodes_are_applied) BOOST_CHECK(govman.GetOrphanVoteObjectHashes().empty()); } +BOOST_AUTO_TEST_CASE(orphan_timestamp_variants_replay_only_the_newest_vote) +{ + auto& govman = *m_node.govman; + const uint256 parent_hash{MakeProposal(uint256{}).GetHash()}; + uint256 newest_hash; + for (const int offset : {5, 1, 4, 3, 2, 5}) { + CGovernanceVote vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES, std::chrono::seconds{offset})}; + SignWithVotingKey(vote, mn_voting_key); + if (offset == 5) newest_hash = vote.GetHash(); + CGovernanceException exception; + uint256 hash_to_request; + BOOST_CHECK(!govman.ProcessVote(vote, exception, hash_to_request)); + BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 0); + } + + const uint256 collateral_hash{ConfirmProposalCollateral(parent_hash)}; + CGovernanceObject proposal{MakeProposal(collateral_hash)}; + govman.AddGovernanceObject(proposal, /*peer_str=*/""); + SyncWithValidationInterfaceQueue(); + BOOST_CHECK(vote_notifications->GetHashes() == std::vector{newest_hash}); + const auto stored{govman.FindConstGovernanceObject(parent_hash)}; + BOOST_REQUIRE(stored != nullptr); + BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 1); + const auto votes{govman.GetCurrentVotes(parent_hash, mn_collateral)}; + BOOST_REQUIRE_EQUAL(votes.size(), 1U); + BOOST_CHECK_EQUAL(votes.front().GetHash(), newest_hash); +} + +BOOST_AUTO_TEST_CASE(newer_operator_orphan_does_not_replace_voting_key_orphan) +{ + auto& govman = *m_node.govman; + const uint256 parent_hash{MakeProposal(uint256{}).GetHash()}; + CGovernanceVote operator_vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES, 30s)}; + SignWithOperatorKey(operator_vote, mn_operator_key); + CGovernanceVote voting_vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES, 1s)}; + SignWithVotingKey(voting_vote, mn_voting_key); + for (const auto& vote : {operator_vote, voting_vote, voting_vote}) { + CGovernanceException exception; + uint256 hash_to_request; + BOOST_CHECK(!govman.ProcessVote(vote, exception, hash_to_request)); + BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 0); + } + const uint256 collateral_hash{ConfirmProposalCollateral(parent_hash)}; + CGovernanceObject proposal{MakeProposal(collateral_hash)}; + govman.AddGovernanceObject(proposal, /*peer_str=*/""); + SyncWithValidationInterfaceQueue(); + BOOST_CHECK(vote_notifications->GetHashes() == std::vector{voting_vote.GetHash()}); + const auto votes{govman.GetCurrentVotes(parent_hash, mn_collateral)}; + BOOST_REQUIRE_EQUAL(votes.size(), 1U); + BOOST_CHECK_EQUAL(votes.front().GetHash(), voting_vote.GetHash()); +} + +BOOST_AUTO_TEST_CASE(equal_time_operator_orphan_does_not_hide_voting_key_orphan) +{ + auto& govman = *m_node.govman; + const uint256 parent_hash{MakeProposal(uint256{}).GetHash()}; + CGovernanceVote operator_vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES)}; + SignWithOperatorKey(operator_vote, mn_operator_key); + CGovernanceVote voting_vote{operator_vote}; + SignWithVotingKey(voting_vote, mn_voting_key); + for (const auto& vote : {operator_vote, voting_vote}) { + CGovernanceException exception; + uint256 hash_to_request; + BOOST_CHECK(!govman.ProcessVote(vote, exception, hash_to_request)); + BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 0); + } + const uint256 collateral_hash{ConfirmProposalCollateral(parent_hash)}; + CGovernanceObject proposal{MakeProposal(collateral_hash)}; + govman.AddGovernanceObject(proposal, /*peer_str=*/""); + SyncWithValidationInterfaceQueue(); + BOOST_CHECK(vote_notifications->GetHashes() == std::vector{voting_vote.GetHash()}); + const auto votes{govman.GetCurrentVotes(parent_hash, mn_collateral)}; + BOOST_REQUIRE_EQUAL(votes.size(), 1U); + BOOST_CHECK_EQUAL(votes.front().GetHash(), voting_vote.GetHash()); +} + +BOOST_AUTO_TEST_CASE(old_key_orphan_does_not_suppress_a_current_key_vote) +{ + // Before DIP3 enforcement, registrar updates must keep the owner and voting keys equal. + MineBlocks(500 - tip_mn_list().GetHeight()); + auto& govman = *m_node.govman; + const uint256 parent_hash{MakeProposal(uint256{}).GetHash()}; + CGovernanceVote old_vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES, 30s)}; + SignWithVotingKey(old_vote, mn_voting_key); + CGovernanceException exception; + uint256 hash_to_request; + BOOST_CHECK(!govman.ProcessVote(old_vote, exception, hash_to_request)); + BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 0); + + CKey new_voting_key; + new_voting_key.MakeNewKey(true); + CProUpRegTx update; + update.nVersion = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false); + update.proTxHash = mn_collateral.hash; + update.pubKeyOperator.Set(mn_operator_key.GetPublicKey(), update.nVersion == ProTxVersion::LegacyBLS); + update.keyIDVoting = new_voting_key.GetPubKey().GetID(); + update.scriptPayout = payout_script(); + CMutableTransaction tx; + tx.nVersion = 3; + tx.nType = TRANSACTION_PROVIDER_UPDATE_REGISTRAR; + const auto spent{FundTransaction(*m_node.chainman, tx, utxos, payout_script(), 1 * COIN)}; + update.inputsHash = CalcTxInputsHash(CTransaction{tx}); + BOOST_REQUIRE(CHashSigner::SignHash(SerializeHash(update), mn_voting_key, update.vchSig)); + SetTxPayload(tx, update); + SignTransaction(tx, spent, coinbaseKey); + MineBlock({tx}); + BOOST_REQUIRE(!old_vote.IsValid(tip_mn_list(), /*useVotingKey=*/true)); + + CGovernanceVote current_vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES, 1s)}; + SignWithVotingKey(current_vote, new_voting_key); + BOOST_REQUIRE(current_vote.IsValid(tip_mn_list(), /*useVotingKey=*/true)); + BOOST_CHECK(!govman.ProcessVote(current_vote, exception, hash_to_request)); + BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 0); + const uint256 collateral_hash{ConfirmProposalCollateral(parent_hash)}; + CGovernanceObject proposal{MakeProposal(collateral_hash)}; + govman.AddGovernanceObject(proposal, /*peer_str=*/""); + SyncWithValidationInterfaceQueue(); + BOOST_CHECK(vote_notifications->GetHashes() == std::vector{current_vote.GetHash()}); + const auto votes{govman.GetCurrentVotes(parent_hash, mn_collateral)}; + BOOST_REQUIRE_EQUAL(votes.size(), 1U); + BOOST_CHECK_EQUAL(votes.front().GetHash(), current_vote.GetHash()); +} + // Votes are only counted if they carry a signature from a registered masternode. BOOST_AUTO_TEST_CASE(unsigned_and_unknown_masternode_votes_are_rejected) {