Skip to content

Commit 461392e

Browse files
knstknst
committed
refactor: move RequestGovernanceObjectVotes to NetGovernance to drop dependency governance on net_processing
1 parent 8425ca1 commit 461392e

File tree

5 files changed

+103
-103
lines changed

5 files changed

+103
-103
lines changed

‎src/governance/governance.cpp‎

Lines changed: 6 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#include <governance/validators.h>
1616
#include <masternode/meta.h>
1717
#include <masternode/sync.h>
18-
#include <net_processing.h>
1918
#include <netfulfilledman.h>
2019
#include <netmessagemaker.h>
2120
#include <protocol.h>
@@ -25,7 +24,7 @@
2524
#include <util/ranges.h>
2625
#include <util/thread.h>
2726
#include <util/time.h>
28-
#include <validation.h>
27+
#include <validationinterface.h>
2928

3029
const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-16";
3130

@@ -1055,6 +1054,11 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH
10551054
connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCESYNC, nHash, filter));
10561055
}
10571056

1057+
CDeterministicMNManager& CGovernanceManager::GetMNManager()
1058+
{
1059+
return *Assert(m_dmnman);
1060+
}
1061+
10581062
std::pair<std::vector<uint256>, std::vector<uint256>> CGovernanceManager::FetchGovernanceObjectVotes(
10591063
size_t nPeersPerHashMax, int64_t nNow, std::map<uint256, std::map<CService, int64_t>>& mapAskedRecently) const
10601064
{
@@ -1086,97 +1090,6 @@ std::pair<std::vector<uint256>, std::vector<uint256>> CGovernanceManager::FetchG
10861090
return {vTriggerObjHashes, vOtherObjHashes};
10871091
}
10881092

1089-
int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector<CNode*>& vNodesCopy, CConnman& connman,
1090-
const PeerManagerInternal* peerman) const
1091-
{
1092-
AssertLockNotHeld(cs_store);
1093-
1094-
// Maximum number of nodes to request votes from for the same object hash on real networks
1095-
// (mainnet, testnet, devnets). Keep this low to avoid unnecessary bandwidth usage.
1096-
static constexpr size_t REALNET_PEERS_PER_HASH{3};
1097-
// Maximum number of nodes to request votes from for the same object hash on regtest.
1098-
// During testing, nodes are isolated to create conflicting triggers. Using the real
1099-
// networks limit of 3 nodes often results in querying only "non-isolated" nodes, missing the
1100-
// isolated ones we need to test. This high limit ensures all available nodes are queried.
1101-
static constexpr size_t REGTEST_PEERS_PER_HASH{std::numeric_limits<size_t>::max()};
1102-
1103-
if (vNodesCopy.empty()) return -1;
1104-
1105-
int64_t nNow = GetTime();
1106-
int nTimeout = 60 * 60;
1107-
size_t nPeersPerHashMax = Params().IsMockableChain() ? REGTEST_PEERS_PER_HASH : REALNET_PEERS_PER_HASH;
1108-
1109-
1110-
// This should help us to get some idea about an impact this can bring once deployed on mainnet.
1111-
// Testnet is ~40 times smaller in masternode count, but only ~1000 masternodes usually vote,
1112-
// so 1 obj on mainnet == ~10 objs or ~1000 votes on testnet. However we want to test a higher
1113-
// number of votes to make sure it's robust enough, so aim at 2000 votes per masternode per request.
1114-
// On mainnet nMaxObjRequestsPerNode is always set to 1.
1115-
int nMaxObjRequestsPerNode = 1;
1116-
size_t nProjectedVotes = 2000;
1117-
if (Params().NetworkIDString() != CBaseChainParams::MAIN) {
1118-
nMaxObjRequestsPerNode = std::max(1, int(nProjectedVotes / std::max(1, (int)Assert(m_dmnman)->GetListAtChainTip().GetValidMNsCount())));
1119-
}
1120-
1121-
static Mutex cs_recently;
1122-
static std::map<uint256, std::map<CService, int64_t>> mapAskedRecently GUARDED_BY(cs_recently);
1123-
LOCK(cs_recently);
1124-
1125-
auto [vTriggerObjHashes, vOtherObjHashes] = FetchGovernanceObjectVotes(nMaxObjRequestsPerNode, nNow, mapAskedRecently);
1126-
1127-
if (vTriggerObjHashes.empty() && vOtherObjHashes.empty()) return -2;
1128-
1129-
LogPrint(BCLog::GOBJECT, "CGovernanceManager::RequestGovernanceObjectVotes -- start: vTriggerObjHashes %d vOtherObjHashes %d mapAskedRecently %d\n",
1130-
vTriggerObjHashes.size(), vOtherObjHashes.size(), mapAskedRecently.size());
1131-
1132-
Shuffle(vTriggerObjHashes.begin(), vTriggerObjHashes.end(), FastRandomContext());
1133-
Shuffle(vOtherObjHashes.begin(), vOtherObjHashes.end(), FastRandomContext());
1134-
1135-
for (int i = 0; i < nMaxObjRequestsPerNode; ++i) {
1136-
uint256 nHashGovobj;
1137-
1138-
// ask for triggers first
1139-
if (!vTriggerObjHashes.empty()) {
1140-
nHashGovobj = vTriggerObjHashes.back();
1141-
} else {
1142-
if (vOtherObjHashes.empty()) break;
1143-
nHashGovobj = vOtherObjHashes.back();
1144-
}
1145-
bool fAsked = false;
1146-
for (const auto& pnode : vNodesCopy) {
1147-
// Don't try to sync any data from outbound non-relay "masternode" connections.
1148-
// Inbound connection this early is most likely a "masternode" connection
1149-
// initiated from another node, so skip it too.
1150-
if (!pnode->CanRelay() || (connman.IsActiveMasternode() && pnode->IsInboundConn())) continue;
1151-
// stop early to prevent setAskFor overflow
1152-
{
1153-
LOCK(::cs_main);
1154-
size_t nProjectedSize = peerman->PeerGetRequestedObjectCount(pnode->GetId()) + nProjectedVotes;
1155-
if (nProjectedSize > MAX_INV_SZ) continue;
1156-
}
1157-
// to early to ask the same node
1158-
if (mapAskedRecently[nHashGovobj].count(pnode->addr)) continue;
1159-
1160-
RequestGovernanceObject(pnode, nHashGovobj, connman, true);
1161-
mapAskedRecently[nHashGovobj][pnode->addr] = nNow + nTimeout;
1162-
fAsked = true;
1163-
// stop loop if max number of peers per obj was asked
1164-
if (mapAskedRecently[nHashGovobj].size() >= nPeersPerHashMax) break;
1165-
}
1166-
// NOTE: this should match `if` above (the one before `while`)
1167-
if (!vTriggerObjHashes.empty()) {
1168-
vTriggerObjHashes.pop_back();
1169-
} else {
1170-
vOtherObjHashes.pop_back();
1171-
}
1172-
if (!fAsked) i--;
1173-
}
1174-
LogPrint(BCLog::GOBJECT, "CGovernanceManager::RequestGovernanceObjectVotes -- end: vTriggerObjHashes %d vOtherObjHashes %d mapAskedRecently %d\n",
1175-
vTriggerObjHashes.size(), vOtherObjHashes.size(), mapAskedRecently.size());
1176-
1177-
return int(vTriggerObjHashes.size() + vOtherObjHashes.size());
1178-
}
1179-
11801093
bool CGovernanceManager::AcceptMessage(const uint256& nHash)
11811094
{
11821095
AssertLockNotHeld(cs_store);

‎src/governance/governance.h‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ template<typename T>
3131
class CFlatDB;
3232
class CInv;
3333
class CNode;
34-
class PeerManagerInternal;
3534

3635
class CDeterministicMNList;
3736
class CDeterministicMNManager;
@@ -312,8 +311,6 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
312311
EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
313312
bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) override
314313
EXCLUSIVE_LOCKS_REQUIRED(!cs_store, !cs_relay);
315-
int RequestGovernanceObjectVotes(const std::vector<CNode*>& vNodesCopy, CConnman& connman,
316-
const PeerManagerInternal* peerman) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
317314
[[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type,
318315
CDataStream& vRecv)
319316
EXCLUSIVE_LOCKS_REQUIRED(!cs_store, !cs_relay);
@@ -377,6 +374,10 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
377374
std::pair<std::vector<uint256>, std::vector<uint256>> FetchGovernanceObjectVotes(
378375
size_t peers_per_hash_max, int64_t now, std::map<uint256, std::map<CService, int64_t>>& map_asked_recently) const
379376
EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
377+
void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const
378+
EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
379+
CDeterministicMNManager& GetMNManager();
380+
380381

381382
private:
382383
// Branches of ProcessMessage
@@ -430,9 +431,6 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
430431
void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight)
431432
EXCLUSIVE_LOCKS_REQUIRED(cs_store);
432433

433-
void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const
434-
EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
435-
436434
bool ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman)
437435
EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
438436

‎src/governance/net_governance.cpp‎

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@
55
#include <governance/net_governance.h>
66

77
#include <chainparams.h>
8+
#include <evo/deterministicmns.h>
89
#include <governance/governance.h>
910
#include <logging.h>
1011
#include <masternode/sync.h>
1112
#include <net.h>
1213
#include <netfulfilledman.h>
1314
#include <netmessagemaker.h>
1415
#include <node/interface_ui.h>
16+
#include <random.h>
1517
#include <scheduler.h>
1618
#include <shutdown.h>
1719

@@ -58,6 +60,94 @@ void NetGovernance::SendGovernanceSyncRequest(CNode* pnode, CConnman& connman) c
5860
connman.PushMessage(pnode, msgMaker.Make(NetMsgType::MNGOVERNANCESYNC, uint256(), filter));
5961
}
6062

63+
int NetGovernance::RequestGovernanceObjectVotes(const std::vector<CNode*>& vNodesCopy, CConnman& connman) const
64+
{
65+
// Maximum number of nodes to request votes from for the same object hash on real networks
66+
// (mainnet, testnet, devnets). Keep this low to avoid unnecessary bandwidth usage.
67+
static constexpr size_t REALNET_PEERS_PER_HASH{3};
68+
// Maximum number of nodes to request votes from for the same object hash on regtest.
69+
// During testing, nodes are isolated to create conflicting triggers. Using the real
70+
// networks limit of 3 nodes often results in querying only "non-isolated" nodes, missing the
71+
// isolated ones we need to test. This high limit ensures all available nodes are queried.
72+
static constexpr size_t REGTEST_PEERS_PER_HASH{std::numeric_limits<size_t>::max()};
73+
74+
if (vNodesCopy.empty()) return -1;
75+
76+
int64_t nNow = GetTime();
77+
int nTimeout = 60 * 60;
78+
size_t nPeersPerHashMax = Params().IsMockableChain() ? REGTEST_PEERS_PER_HASH : REALNET_PEERS_PER_HASH;
79+
80+
81+
// This should help us to get some idea about an impact this can bring once deployed on mainnet.
82+
// Testnet is ~40 times smaller in masternode count, but only ~1000 masternodes usually vote,
83+
// so 1 obj on mainnet == ~10 objs or ~1000 votes on testnet. However we want to test a higher
84+
// number of votes to make sure it's robust enough, so aim at 2000 votes per masternode per request.
85+
// On mainnet nMaxObjRequestsPerNode is always set to 1.
86+
int nMaxObjRequestsPerNode = 1;
87+
size_t nProjectedVotes = 2000;
88+
if (Params().NetworkIDString() != CBaseChainParams::MAIN) {
89+
nMaxObjRequestsPerNode = std::max(1, int(nProjectedVotes / std::max(1, (int)m_gov_manager.GetMNManager().GetListAtChainTip().GetValidMNsCount())));
90+
}
91+
92+
static Mutex cs_recently;
93+
static std::map<uint256, std::map<CService, int64_t> > mapAskedRecently GUARDED_BY(cs_recently);
94+
LOCK(cs_recently);
95+
96+
auto [vTriggerObjHashes, vOtherObjHashes] = m_gov_manager.FetchGovernanceObjectVotes(nMaxObjRequestsPerNode, nNow, mapAskedRecently);
97+
98+
if (vTriggerObjHashes.empty() && vOtherObjHashes.empty()) return -2;
99+
100+
LogPrint(BCLog::GOBJECT, "CGovernanceManager::RequestGovernanceObjectVotes -- start: vTriggerObjHashes %d vOtherObjHashes %d mapAskedRecently %d\n",
101+
vTriggerObjHashes.size(), vOtherObjHashes.size(), mapAskedRecently.size());
102+
103+
Shuffle(vTriggerObjHashes.begin(), vTriggerObjHashes.end(), FastRandomContext());
104+
Shuffle(vOtherObjHashes.begin(), vOtherObjHashes.end(), FastRandomContext());
105+
106+
for (int i = 0; i < nMaxObjRequestsPerNode; ++i) {
107+
uint256 nHashGovobj;
108+
109+
// ask for triggers first
110+
if (!vTriggerObjHashes.empty()) {
111+
nHashGovobj = vTriggerObjHashes.back();
112+
} else {
113+
if (vOtherObjHashes.empty()) break;
114+
nHashGovobj = vOtherObjHashes.back();
115+
}
116+
bool fAsked = false;
117+
for (const auto& pnode : vNodesCopy) {
118+
// Don't try to sync any data from outbound non-relay "masternode" connections.
119+
// Inbound connection this early is most likely a "masternode" connection
120+
// initiated from another node, so skip it too.
121+
if (!pnode->CanRelay() || (connman.IsActiveMasternode() && pnode->IsInboundConn())) continue;
122+
// stop early to prevent setAskFor overflow
123+
{
124+
LOCK(::cs_main);
125+
size_t nProjectedSize = m_peer_manager->PeerGetRequestedObjectCount(pnode->GetId()) + nProjectedVotes;
126+
if (nProjectedSize > MAX_INV_SZ) continue;
127+
}
128+
// to early to ask the same node
129+
if (mapAskedRecently[nHashGovobj].count(pnode->addr)) continue;
130+
131+
m_gov_manager.RequestGovernanceObject(pnode, nHashGovobj, connman, true);
132+
mapAskedRecently[nHashGovobj][pnode->addr] = nNow + nTimeout;
133+
fAsked = true;
134+
// stop loop if max number of peers per obj was asked
135+
if (mapAskedRecently[nHashGovobj].size() >= nPeersPerHashMax) break;
136+
}
137+
// NOTE: this should match `if` above (the one before `while`)
138+
if (!vTriggerObjHashes.empty()) {
139+
vTriggerObjHashes.pop_back();
140+
} else {
141+
vOtherObjHashes.pop_back();
142+
}
143+
if (!fAsked) i--;
144+
}
145+
LogPrint(BCLog::GOBJECT, "CGovernanceManager::RequestGovernanceObjectVotes -- end: vTriggerObjHashes %d vOtherObjHashes %d mapAskedRecently %d\n",
146+
vTriggerObjHashes.size(), vOtherObjHashes.size(), mapAskedRecently.size());
147+
148+
return int(vTriggerObjHashes.size() + vOtherObjHashes.size());
149+
}
150+
61151
void NetGovernance::ProcessTick(CConnman& connman)
62152
{
63153
assert(m_netfulfilledman.IsValid());
@@ -87,7 +177,7 @@ void NetGovernance::ProcessTick(CConnman& connman)
87177

88178
// gradually request the rest of the votes after sync finished
89179
if (m_node_sync.IsSynced()) {
90-
m_gov_manager.RequestGovernanceObjectVotes(snap.Nodes(), connman, m_peer_manager);
180+
RequestGovernanceObjectVotes(snap.Nodes(), connman);
91181
return;
92182
}
93183

@@ -217,7 +307,7 @@ void NetGovernance::ProcessTick(CConnman& connman)
217307
continue; // to early for this node
218308
}
219309
const std::vector<CNode*> vNodeCopy{pnode};
220-
int nObjsLeftToAsk = m_gov_manager.RequestGovernanceObjectVotes(vNodeCopy, connman, m_peer_manager);
310+
int nObjsLeftToAsk = RequestGovernanceObjectVotes(vNodeCopy, connman);
221311
// check for data
222312
if (nObjsLeftToAsk == 0) {
223313
static int64_t nTimeNoObjectsLeft = 0;

‎src/governance/net_governance.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ class NetGovernance final : public NetHandler
2626

2727
private:
2828
void SendGovernanceSyncRequest(CNode* pnode, CConnman& connman) const;
29+
int RequestGovernanceObjectVotes(const std::vector<CNode*>& vNodesCopy, CConnman& connman) const;
2930
void ProcessTick(CConnman& connman);
3031

3132
CGovernanceManager& m_gov_manager;

‎test/lint/lint-circular-dependencies.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@
4949
"governance/classes -> governance/object -> governance/governance -> governance/classes",
5050
"governance/governance -> governance/signing -> governance/object -> governance/governance",
5151
"masternode/sync -> net -> masternode/sync",
52-
"governance/governance -> net_processing -> masternode/active/context -> governance/governance",
53-
"governance/governance -> net_processing -> governance/governance",
5452
"instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> instantsend/instantsend",
5553
"instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> llmq/context -> instantsend/instantsend",
5654
"instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> masternode/active/context -> instantsend/instantsend",

0 commit comments

Comments
 (0)