Skip to content

Commit df7d7bb

Browse files
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) <noreply@anthropic.com>
1 parent 94c969f commit df7d7bb

6 files changed

Lines changed: 220 additions & 3 deletions

File tree

‎src/cachemultimap.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,18 @@ class CacheMultiMap
132132
return true;
133133
}
134134

135+
bool GetAll(const K& key, const V& lower, const V& upper, std::vector<V>& vecValues) const
136+
{
137+
const map_cit mit = mapIndex.find(key);
138+
if (mit == mapIndex.end() || upper < lower) return false;
139+
const it_map_t& mapIt = mit->second;
140+
const auto range_end = mapIt.upper_bound(upper);
141+
for (auto it = mapIt.lower_bound(lower); it != range_end; ++it) {
142+
vecValues.push_back(it->second->value);
143+
}
144+
return true;
145+
}
146+
135147
void GetKeys(std::vector<K>& vecKeys)
136148
{
137149
for(map_cit it = mapIndex.begin(); it != mapIndex.end(); ++it) {

‎src/governance/governance.cpp‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
#include <univalue.h>
2828

29+
#include <limits>
2930
#include <ranges>
3031

3132
const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-17";
@@ -37,6 +38,39 @@ constexpr std::chrono::seconds MAX_TIME_FUTURE_DEVIATION{1h};
3738
// Margin below MAX_TIME_FUTURE_DEVIATION so lagging-clock peers don't reject near-limit objects.
3839
constexpr std::chrono::seconds RELIABLE_PROPAGATION_TIME{60};
3940

41+
// The encoding chooses which key to verify: IsValidForUnknownParent accepts only compact
42+
// signatures by the voting key and BLS signatures by the operator key.
43+
bool UsesVotingKey(const CGovernanceVote& vote)
44+
{
45+
return vote.GetSignal() == VOTE_SIGNAL_FUNDING && vote.GetSignatureSize() == CGovernanceVote::COMPACT_SIG_SIZE;
46+
}
47+
48+
bool InsertOrphanVote(CacheMultiMap<uint256, governance::OrphanVote>& cache, const CDeterministicMNList& tip_mn_list,
49+
const governance::OrphanVote& orphan_vote)
50+
{
51+
auto lower{orphan_vote};
52+
auto upper{orphan_vote};
53+
lower.vote.SetTime(std::numeric_limits<int64_t>::min());
54+
lower.vote.SetSignature({});
55+
upper.vote.SetTime(std::numeric_limits<int64_t>::max());
56+
upper.vote.SetSignature(std::vector<unsigned char>(CGovernanceVote::BLS_SIG_SIZE));
57+
std::vector<governance::OrphanVote> variants;
58+
const auto& parent = orphan_vote.vote.GetParentHash();
59+
cache.GetAll(parent, lower, upper, variants);
60+
const bool use_voting_key{UsesVotingKey(orphan_vote.vote)};
61+
for (const auto& cached : variants) {
62+
const bool cached_uses_voting_key{UsesVotingKey(cached.vote)};
63+
// Only current-key authentication may suppress a new vote.
64+
if (cached.expiration < Now<NodeSeconds>() || !cached.vote.IsValid(tip_mn_list, cached_uses_voting_key)) {
65+
cache.Erase(parent, cached);
66+
} else if (cached_uses_voting_key == use_voting_key) {
67+
if (cached.vote.GetTimestamp() >= orphan_vote.vote.GetTimestamp()) return false;
68+
cache.Erase(parent, cached);
69+
}
70+
}
71+
return cache.Insert(parent, orphan_vote);
72+
}
73+
4074
bool IsSyncableObject(const std::shared_ptr<CGovernanceObject>& govobj)
4175
{
4276
const auto& obj = *Assert(govobj);
@@ -820,9 +854,8 @@ bool CGovernanceManager::ProcessVote(const CGovernanceVote& vote, CGovernanceExc
820854
AssertLockNotHeld(cs_store);
821855
hashToRequest = uint256{};
822856

823-
const auto tip_mn_list{m_dmnman.GetListAtChainTip()};
824-
825857
LOCK(cs_store);
858+
const auto tip_mn_list{m_dmnman.GetListAtChainTip()};
826859
uint256 nHashVote = vote.GetHash();
827860
uint256 nHashGovobj = vote.GetParentHash();
828861

@@ -846,7 +879,8 @@ bool CGovernanceManager::ProcessVote(const CGovernanceVote& vote, CGovernanceExc
846879
// No penalty: the vote is signed by a masternode, it just arrived before its parent object,
847880
// which routinely happens during governance sync. Misbehaviour scores never decay.
848881
exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_WARNING);
849-
if (cmmapOrphanVotes.Insert(nHashGovobj, governance::OrphanVote{vote, Now<NodeSeconds>() + GOVERNANCE_ORPHAN_EXPIRATION_TIME})) {
882+
if (InsertOrphanVote(cmmapOrphanVotes, tip_mn_list,
883+
governance::OrphanVote{vote, Now<NodeSeconds>() + GOVERNANCE_ORPHAN_EXPIRATION_TIME})) {
850884
hashToRequest = nHashGovobj; // Caller should request this object
851885
}
852886
LogPrint(BCLog::GOBJECT, "%s\n", msg);

‎src/governance/governance.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ struct OrphanVote {
5656

5757
inline bool operator<(const OrphanVote& lhs, const OrphanVote& rhs)
5858
{
59+
// Equal signed fields may arrive authenticated by different voting/operator keys.
60+
if (lhs.vote == rhs.vote) return lhs.vote.GetSignatureSize() < rhs.vote.GetSignatureSize();
5961
return lhs.vote < rhs.vote;
6062
}
6163
} // namespace governance

‎src/governance/vote.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ class CGovernanceVote
145145
}
146146

147147
void SetSignature(const std::vector<unsigned char>& vchSigIn) { vchSig = vchSigIn; }
148+
size_t GetSignatureSize() const { return vchSig.size(); }
148149

149150
bool CheckSignature(const CKeyID& keyID) const;
150151
bool CheckSignature(const CBLSPublicKey& pubKey) const;

‎src/test/cachemultimap_tests.cpp‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,4 +188,21 @@ BOOST_AUTO_TEST_CASE(cachemultimap_insert_evicts_sole_value_of_same_key)
188188
BOOST_CHECK(!cmmap.HasKey(1));
189189
}
190190

191+
BOOST_AUTO_TEST_CASE(cachemultimap_get_all_in_value_range)
192+
{
193+
CacheMultiMap<int, int> cmmap(5);
194+
for (const int value : {1, 2, 3, 4})
195+
BOOST_REQUIRE(cmmap.Insert(1, value));
196+
BOOST_REQUIRE(cmmap.Insert(2, 2));
197+
std::vector<int> values;
198+
BOOST_CHECK(cmmap.GetAll(1, 2, 3, values));
199+
BOOST_CHECK(values == (std::vector<int>{2, 3}));
200+
values.clear();
201+
BOOST_CHECK(cmmap.GetAll(1, 5, 6, values));
202+
BOOST_CHECK(values.empty());
203+
BOOST_CHECK(!cmmap.GetAll(3, 1, 4, values));
204+
BOOST_CHECK(!cmmap.GetAll(1, 3, 2, values));
205+
BOOST_CHECK(values.empty());
206+
}
207+
191208
BOOST_AUTO_TEST_SUITE_END()

‎src/test/governance_vote_processing_tests.cpp‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
#include <bls/bls.h>
66
#include <consensus/amount.h>
77
#include <evo/deterministicmns.h>
8+
#include <evo/providertx.h>
9+
#include <evo/specialtx.h>
810
#include <governance/governance.h>
911
#include <governance/object.h>
1012
#include <governance/vote.h>
@@ -74,6 +76,24 @@ CGovernanceObject MakeFailedTrigger(const COutPoint& mn_collateral, const CBLSSe
7476
return govobj;
7577
}
7678

79+
struct GovernanceVoteNotifications final : CValidationInterface {
80+
mutable Mutex cs;
81+
std::vector<uint256> hashes GUARDED_BY(cs);
82+
83+
void NotifyGovernanceVote(const std::shared_ptr<CDeterministicMNList>&,
84+
const std::shared_ptr<const CGovernanceVote>& vote) override EXCLUSIVE_LOCKS_REQUIRED(!cs)
85+
{
86+
LOCK(cs);
87+
hashes.push_back(vote->GetHash());
88+
}
89+
90+
std::vector<uint256> GetHashes() const EXCLUSIVE_LOCKS_REQUIRED(!cs)
91+
{
92+
LOCK(cs);
93+
return hashes;
94+
}
95+
};
96+
7797
// A chain with one registered masternode whose voting (ECDSA) and operator (BLS) keys are known
7898
// to the test, so votes can be signed for real and CGovernanceVote::CheckSignature is actually
7999
// reached. Without a populated masternode list every CGovernanceVote::IsValid() call would
@@ -83,6 +103,7 @@ struct GovernanceVoteSetup : public TestChainSetup {
83103
CBLSSecretKey mn_operator_key;
84104
COutPoint mn_collateral;
85105
SimpleUTXOMap utxos;
106+
std::shared_ptr<GovernanceVoteNotifications> vote_notifications;
86107
//! Fixed so a proposal can be rebuilt bit-for-bit (and keep its hash) once its fee tx is known.
87108
int64_t proposal_time{0};
88109
std::string proposal_payment_address;
@@ -135,6 +156,8 @@ struct GovernanceVoteSetup : public TestChainSetup {
135156
*m_node.sbman, *m_node.dmnman,
136157
*m_node.mn_sync);
137158
BOOST_REQUIRE(m_node.govman->LoadCache(/*load_cache=*/false));
159+
vote_notifications = std::make_shared<GovernanceVoteNotifications>();
160+
RegisterSharedValidationInterface(vote_notifications);
138161

139162
proposal_time = TicksSinceEpoch<std::chrono::seconds>(GetAdjustedTime());
140163
CKey payment_key;
@@ -144,6 +167,11 @@ struct GovernanceVoteSetup : public TestChainSetup {
144167

145168
void TearDown()
146169
{
170+
if (vote_notifications) {
171+
SyncWithValidationInterfaceQueue();
172+
UnregisterSharedValidationInterface(vote_notifications);
173+
vote_notifications.reset();
174+
}
147175
// govman holds a reference to node.sbman, so it must go first (matches PrepareShutdown
148176
// ordering in init.cpp).
149177
m_node.govman.reset();
@@ -320,6 +348,129 @@ BOOST_AUTO_TEST_CASE(orphan_funding_votes_from_multiple_masternodes_are_applied)
320348
BOOST_CHECK(govman.GetOrphanVoteObjectHashes().empty());
321349
}
322350

351+
BOOST_AUTO_TEST_CASE(orphan_timestamp_variants_replay_only_the_newest_vote)
352+
{
353+
auto& govman = *m_node.govman;
354+
const uint256 parent_hash{MakeProposal(uint256{}).GetHash()};
355+
uint256 newest_hash;
356+
for (const int offset : {5, 1, 4, 3, 2, 5}) {
357+
CGovernanceVote vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES, std::chrono::seconds{offset})};
358+
SignWithVotingKey(vote, mn_voting_key);
359+
if (offset == 5) newest_hash = vote.GetHash();
360+
CGovernanceException exception;
361+
uint256 hash_to_request;
362+
BOOST_CHECK(!govman.ProcessVote(vote, exception, hash_to_request));
363+
BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 0);
364+
}
365+
366+
const uint256 collateral_hash{ConfirmProposalCollateral(parent_hash)};
367+
CGovernanceObject proposal{MakeProposal(collateral_hash)};
368+
govman.AddGovernanceObject(proposal, /*peer_str=*/"");
369+
SyncWithValidationInterfaceQueue();
370+
BOOST_CHECK(vote_notifications->GetHashes() == std::vector<uint256>{newest_hash});
371+
const auto stored{govman.FindConstGovernanceObject(parent_hash)};
372+
BOOST_REQUIRE(stored != nullptr);
373+
BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 1);
374+
const auto votes{govman.GetCurrentVotes(parent_hash, mn_collateral)};
375+
BOOST_REQUIRE_EQUAL(votes.size(), 1U);
376+
BOOST_CHECK_EQUAL(votes.front().GetHash(), newest_hash);
377+
}
378+
379+
BOOST_AUTO_TEST_CASE(newer_operator_orphan_does_not_replace_voting_key_orphan)
380+
{
381+
auto& govman = *m_node.govman;
382+
const uint256 parent_hash{MakeProposal(uint256{}).GetHash()};
383+
CGovernanceVote operator_vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES, 30s)};
384+
SignWithOperatorKey(operator_vote, mn_operator_key);
385+
CGovernanceVote voting_vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES, 1s)};
386+
SignWithVotingKey(voting_vote, mn_voting_key);
387+
for (const auto& vote : {operator_vote, voting_vote, voting_vote}) {
388+
CGovernanceException exception;
389+
uint256 hash_to_request;
390+
BOOST_CHECK(!govman.ProcessVote(vote, exception, hash_to_request));
391+
BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 0);
392+
}
393+
const uint256 collateral_hash{ConfirmProposalCollateral(parent_hash)};
394+
CGovernanceObject proposal{MakeProposal(collateral_hash)};
395+
govman.AddGovernanceObject(proposal, /*peer_str=*/"");
396+
SyncWithValidationInterfaceQueue();
397+
BOOST_CHECK(vote_notifications->GetHashes() == std::vector<uint256>{voting_vote.GetHash()});
398+
const auto votes{govman.GetCurrentVotes(parent_hash, mn_collateral)};
399+
BOOST_REQUIRE_EQUAL(votes.size(), 1U);
400+
BOOST_CHECK_EQUAL(votes.front().GetHash(), voting_vote.GetHash());
401+
}
402+
403+
BOOST_AUTO_TEST_CASE(equal_time_operator_orphan_does_not_hide_voting_key_orphan)
404+
{
405+
auto& govman = *m_node.govman;
406+
const uint256 parent_hash{MakeProposal(uint256{}).GetHash()};
407+
CGovernanceVote operator_vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES)};
408+
SignWithOperatorKey(operator_vote, mn_operator_key);
409+
CGovernanceVote voting_vote{operator_vote};
410+
SignWithVotingKey(voting_vote, mn_voting_key);
411+
for (const auto& vote : {operator_vote, voting_vote}) {
412+
CGovernanceException exception;
413+
uint256 hash_to_request;
414+
BOOST_CHECK(!govman.ProcessVote(vote, exception, hash_to_request));
415+
BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 0);
416+
}
417+
const uint256 collateral_hash{ConfirmProposalCollateral(parent_hash)};
418+
CGovernanceObject proposal{MakeProposal(collateral_hash)};
419+
govman.AddGovernanceObject(proposal, /*peer_str=*/"");
420+
SyncWithValidationInterfaceQueue();
421+
BOOST_CHECK(vote_notifications->GetHashes() == std::vector<uint256>{voting_vote.GetHash()});
422+
const auto votes{govman.GetCurrentVotes(parent_hash, mn_collateral)};
423+
BOOST_REQUIRE_EQUAL(votes.size(), 1U);
424+
BOOST_CHECK_EQUAL(votes.front().GetHash(), voting_vote.GetHash());
425+
}
426+
427+
BOOST_AUTO_TEST_CASE(old_key_orphan_does_not_suppress_a_current_key_vote)
428+
{
429+
// Before DIP3 enforcement, registrar updates must keep the owner and voting keys equal.
430+
MineBlocks(500 - tip_mn_list().GetHeight());
431+
auto& govman = *m_node.govman;
432+
const uint256 parent_hash{MakeProposal(uint256{}).GetHash()};
433+
CGovernanceVote old_vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES, 30s)};
434+
SignWithVotingKey(old_vote, mn_voting_key);
435+
CGovernanceException exception;
436+
uint256 hash_to_request;
437+
BOOST_CHECK(!govman.ProcessVote(old_vote, exception, hash_to_request));
438+
BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 0);
439+
440+
CKey new_voting_key;
441+
new_voting_key.MakeNewKey(true);
442+
CProUpRegTx update;
443+
update.nVersion = ProTxVersion::GetMax(!bls::bls_legacy_scheme, /*is_extended_addr=*/false);
444+
update.proTxHash = mn_collateral.hash;
445+
update.pubKeyOperator.Set(mn_operator_key.GetPublicKey(), update.nVersion == ProTxVersion::LegacyBLS);
446+
update.keyIDVoting = new_voting_key.GetPubKey().GetID();
447+
update.scriptPayout = payout_script();
448+
CMutableTransaction tx;
449+
tx.nVersion = 3;
450+
tx.nType = TRANSACTION_PROVIDER_UPDATE_REGISTRAR;
451+
const auto spent{FundTransaction(*m_node.chainman, tx, utxos, payout_script(), 1 * COIN)};
452+
update.inputsHash = CalcTxInputsHash(CTransaction{tx});
453+
BOOST_REQUIRE(CHashSigner::SignHash(SerializeHash(update), mn_voting_key, update.vchSig));
454+
SetTxPayload(tx, update);
455+
SignTransaction(tx, spent, coinbaseKey);
456+
MineBlock({tx});
457+
BOOST_REQUIRE(!old_vote.IsValid(tip_mn_list(), /*useVotingKey=*/true));
458+
459+
CGovernanceVote current_vote{MakeVote(parent_hash, VOTE_SIGNAL_FUNDING, VOTE_OUTCOME_YES, 1s)};
460+
SignWithVotingKey(current_vote, new_voting_key);
461+
BOOST_REQUIRE(current_vote.IsValid(tip_mn_list(), /*useVotingKey=*/true));
462+
BOOST_CHECK(!govman.ProcessVote(current_vote, exception, hash_to_request));
463+
BOOST_CHECK_EQUAL(exception.GetNodePenalty(), 0);
464+
const uint256 collateral_hash{ConfirmProposalCollateral(parent_hash)};
465+
CGovernanceObject proposal{MakeProposal(collateral_hash)};
466+
govman.AddGovernanceObject(proposal, /*peer_str=*/"");
467+
SyncWithValidationInterfaceQueue();
468+
BOOST_CHECK(vote_notifications->GetHashes() == std::vector<uint256>{current_vote.GetHash()});
469+
const auto votes{govman.GetCurrentVotes(parent_hash, mn_collateral)};
470+
BOOST_REQUIRE_EQUAL(votes.size(), 1U);
471+
BOOST_CHECK_EQUAL(votes.front().GetHash(), current_vote.GetHash());
472+
}
473+
323474
// Votes are only counted if they carry a signature from a registered masternode.
324475
BOOST_AUTO_TEST_CASE(unsigned_and_unknown_masternode_votes_are_rejected)
325476
{

0 commit comments

Comments
 (0)