Skip to content

Commit 513c3f9

Browse files
kwvgkwvg
committed
refactor: avoid accessing active masternode info if not in masternode mode
A later commit will be moving activeMasternodeInfo into activeMasternodeManager and that is only conditionally initialized if the node is in masternode mode, which will render access attempts outside of masternode mode invalid. We need to adjust behaviour to account for that.
1 parent 336f118 commit 513c3f9

File tree

11 files changed

+74
-58
lines changed

11 files changed

+74
-58
lines changed

‎src/evo/mnauth.cpp‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@
2121

2222
void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CBlockIndex* tip)
2323
{
24+
if (!fMasternodeMode) return;
25+
2426
LOCK(activeMasternodeInfoCs);
25-
if (!fMasternodeMode || activeMasternodeInfo.proTxHash.IsNull()) {
26-
return;
27-
}
27+
if (activeMasternodeInfo.proTxHash.IsNull()) return;
2828

2929
uint256 signHash;
3030
const auto receivedMNAuthChallenge = peer.GetReceivedMNAuthChallenge();
@@ -128,7 +128,10 @@ PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, CConnman& connman, std::string_v
128128
}
129129
}
130130

131-
const uint256 myProTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash);
131+
uint256 myProTxHash;
132+
if (fMasternodeMode) {
133+
myProTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash);
134+
}
132135

133136
connman.ForEachNode([&](CNode* pnode2) {
134137
if (peer.fDisconnect) {

‎src/governance/governance.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,8 @@ std::optional<const CGovernanceObject> CGovernanceManager::CreateGovernanceTrigg
712712
void CGovernanceManager::VoteGovernanceTriggers(const std::optional<const CGovernanceObject>& trigger_opt, CConnman& connman)
713713
{
714714
// only active masternodes can vote on triggers
715-
if (!fMasternodeMode || WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash.IsNull())) return;
715+
if (!fMasternodeMode) return;
716+
if (WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash.IsNull())) return;
716717

717718
LOCK2(cs_main, cs);
718719

‎src/init.cpp‎

Lines changed: 26 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -370,14 +370,13 @@ void PrepareShutdown(NodeContext& node)
370370
}
371371
if (fMasternodeMode) {
372372
UnregisterValidationInterface(activeMasternodeManager.get());
373-
activeMasternodeManager.reset();
374-
}
375373

376-
{
377374
LOCK(activeMasternodeInfoCs);
378375
// make sure to clean up BLS keys before global destructors are called (they have allocated from the secure memory pool)
379376
activeMasternodeInfo.blsKeyOperator.reset();
380377
activeMasternodeInfo.blsPubKeyOperator.reset();
378+
379+
activeMasternodeManager.reset();
381380
}
382381

383382
node.chain_clients.clear();
@@ -1606,34 +1605,6 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
16061605
StartScriptCheckWorkerThreads(script_threads);
16071606
}
16081607

1609-
{
1610-
LOCK(activeMasternodeInfoCs);
1611-
assert(activeMasternodeInfo.blsKeyOperator == nullptr);
1612-
assert(activeMasternodeInfo.blsPubKeyOperator == nullptr);
1613-
}
1614-
fMasternodeMode = false;
1615-
std::string strMasterNodeBLSPrivKey = args.GetArg("-masternodeblsprivkey", "");
1616-
if (!strMasterNodeBLSPrivKey.empty()) {
1617-
CBLSSecretKey keyOperator(ParseHex(strMasterNodeBLSPrivKey));
1618-
if (!keyOperator.IsValid()) {
1619-
return InitError(_("Invalid masternodeblsprivkey. Please see documentation."));
1620-
}
1621-
fMasternodeMode = true;
1622-
{
1623-
LOCK(activeMasternodeInfoCs);
1624-
activeMasternodeInfo.blsKeyOperator = std::make_unique<CBLSSecretKey>(keyOperator);
1625-
activeMasternodeInfo.blsPubKeyOperator = std::make_unique<CBLSPublicKey>(keyOperator.GetPublicKey());
1626-
// We don't know the actual scheme at this point, print both
1627-
LogPrintf("MASTERNODE:\n blsPubKeyOperator legacy: %s\n blsPubKeyOperator basic: %s\n",
1628-
activeMasternodeInfo.blsPubKeyOperator->ToString(true),
1629-
activeMasternodeInfo.blsPubKeyOperator->ToString(false));
1630-
}
1631-
} else {
1632-
LOCK(activeMasternodeInfoCs);
1633-
activeMasternodeInfo.blsKeyOperator = std::make_unique<CBLSSecretKey>();
1634-
activeMasternodeInfo.blsPubKeyOperator = std::make_unique<CBLSPublicKey>();
1635-
}
1636-
16371608
assert(!node.scheduler);
16381609
node.scheduler = std::make_unique<CScheduler>();
16391610

@@ -1878,10 +1849,30 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
18781849
);
18791850
RegisterValidationInterface(pdsNotificationInterface);
18801851

1881-
if (fMasternodeMode) {
1882-
// Create and register activeMasternodeManager, will init later in ThreadImport
1883-
activeMasternodeManager = std::make_unique<CActiveMasternodeManager>(*node.connman);
1884-
RegisterValidationInterface(activeMasternodeManager.get());
1852+
fMasternodeMode = false;
1853+
std::string strMasterNodeBLSPrivKey = args.GetArg("-masternodeblsprivkey", "");
1854+
if (!strMasterNodeBLSPrivKey.empty()) {
1855+
CBLSSecretKey keyOperator(ParseHex(strMasterNodeBLSPrivKey));
1856+
if (!keyOperator.IsValid()) {
1857+
return InitError(_("Invalid masternodeblsprivkey. Please see documentation."));
1858+
}
1859+
fMasternodeMode = true;
1860+
{
1861+
// Create and register activeMasternodeManager, will init later in ThreadImport
1862+
activeMasternodeManager = std::make_unique<CActiveMasternodeManager>(*node.connman);
1863+
1864+
LOCK(activeMasternodeInfoCs);
1865+
assert(activeMasternodeInfo.blsKeyOperator == nullptr);
1866+
assert(activeMasternodeInfo.blsPubKeyOperator == nullptr);
1867+
activeMasternodeInfo.blsKeyOperator = std::make_unique<CBLSSecretKey>(keyOperator);
1868+
activeMasternodeInfo.blsPubKeyOperator = std::make_unique<CBLSPublicKey>(keyOperator.GetPublicKey());
1869+
// We don't know the actual scheme at this point, print both
1870+
LogPrintf("MASTERNODE:\n blsPubKeyOperator legacy: %s\n blsPubKeyOperator basic: %s\n",
1871+
activeMasternodeInfo.blsPubKeyOperator->ToString(true),
1872+
activeMasternodeInfo.blsPubKeyOperator->ToString(false));
1873+
1874+
RegisterValidationInterface(activeMasternodeManager.get());
1875+
}
18851876
}
18861877

18871878
// ********************************************************* Step 7a: Load sporks

‎src/llmq/context.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@ void LLMQContext::Start() {
7676
assert(isman == llmq::quorumInstantSendManager.get());
7777

7878
bls_worker->Start();
79-
qdkgsman->StartThreads();
79+
if (fMasternodeMode) {
80+
qdkgsman->StartThreads();
81+
}
8082
qman->Start();
8183
shareman->RegisterAsRecoveredSigsListener();
8284
shareman->StartWorkerThread();
@@ -99,6 +101,8 @@ void LLMQContext::Stop() {
99101
shareman->UnregisterAsRecoveredSigsListener();
100102
sigman->StopWorkerThread();
101103
qman->Stop();
102-
qdkgsman->StopThreads();
104+
if (fMasternodeMode) {
105+
qdkgsman->StopThreads();
106+
}
103107
bls_worker->Stop();
104108
}

‎src/llmq/quorums.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,10 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(const CBlockIndex* pIndex)
249249
const auto vecQuorums = ScanQuorums(params.type, pIndex, params.keepOldConnections);
250250

251251
// First check if we are member of any quorum of this type
252-
auto proTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash);
252+
uint256 proTxHash;
253+
if (fMasternodeMode) {
254+
proTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash);
255+
}
253256
bool fWeAreQuorumTypeMember = ranges::any_of(vecQuorums, [&proTxHash](const auto& pQuorum) {
254257
return pQuorum->IsValidMember(proTxHash);
255258
});
@@ -340,7 +343,10 @@ void CQuorumManager::CheckQuorumConnections(const Consensus::LLMQParams& llmqPar
340343
LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- llmqType[%d] h[%d] keeping mn quorum connections for quorum: [%d:%s]\n", __func__, ToUnderlying(llmqParams.type), pindexNew->nHeight, curDkgHeight, curDkgBlock.ToString());
341344
}
342345

343-
const auto myProTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash);
346+
uint256 myProTxHash;
347+
if (fMasternodeMode) {
348+
myProTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash);
349+
}
344350
bool isISType = llmqParams.type == Params().GetConsensus().llmqTypeDIP0024InstantSend;
345351

346352
bool watchOtherISQuorums = isISType && !myProTxHash.IsNull() &&

‎src/llmq/signing.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -895,9 +895,8 @@ void CSigningManager::UnregisterRecoveredSigsListener(CRecoveredSigsListener* l)
895895

896896
bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigSharesManager& shareman, const uint256& id, const uint256& msgHash, const uint256& quorumHash, bool allowReSign)
897897
{
898-
if (!fMasternodeMode || WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash.IsNull())) {
899-
return false;
900-
}
898+
if (!fMasternodeMode) return false;
899+
if (WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash.IsNull())) return false;
901900

902901
const CQuorumCPtr quorum = [&]() {
903902
if (quorumHash.IsNull()) {

‎src/llmq/signing_shares.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,9 +218,8 @@ void CSigSharesManager::InterruptWorkerThread()
218218
void CSigSharesManager::ProcessMessage(const CNode& pfrom, const CSporkManager& sporkman, const std::string& msg_type, CDataStream& vRecv)
219219
{
220220
// non-masternodes are not interested in sigshares
221-
if (!fMasternodeMode || WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash.IsNull())) {
222-
return;
223-
}
221+
if (!fMasternodeMode) return;
222+
if (WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash.IsNull())) return;
224223

225224
if (sporkman.IsSporkActive(SPORK_21_QUORUM_ALL_CONNECTED) && msg_type == NetMsgType::QSIGSHARE) {
226225
std::vector<CSigShare> receivedSigShares;

‎src/rpc/coinjoin.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ static RPCHelpMan coinjoin()
3838
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
3939
if (!wallet) return NullUniValue;
4040

41-
if (fMasternodeMode)
41+
if (fMasternodeMode) {
4242
throw JSONRPCError(RPC_INTERNAL_ERROR, "Client-side mixing is not supported on masternodes");
43+
}
4344

4445
if (!CCoinJoinClientOptions::IsEnabled()) {
4546
if (!gArgs.GetBoolArg("-enablecoinjoin", true)) {

‎src/rpc/governance.cpp‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -312,11 +312,19 @@ static UniValue gobject_submit(const JSONRPCRequest& request)
312312
}
313313

314314
auto mnList = node.dmnman->GetListAtChainTip();
315-
bool fMnFound = WITH_LOCK(activeMasternodeInfoCs, return mnList.HasValidMNByCollateral(activeMasternodeInfo.outpoint));
316315

317-
LogPrint(BCLog::GOBJECT, "gobject_submit -- pubKeyOperator = %s, outpoint = %s, params.size() = %lld, fMnFound = %d\n",
318-
(WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsPubKeyOperator ? activeMasternodeInfo.blsPubKeyOperator->ToString(activeMasternodeInfo.legacy) : "N/A")),
319-
WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint.ToStringShort()), request.params.size(), fMnFound);
316+
bool fMnFound{false};
317+
if (fMasternodeMode) {
318+
LOCK(activeMasternodeInfoCs);
319+
fMnFound = mnList.HasValidMNByCollateral(activeMasternodeInfo.outpoint);
320+
321+
LogPrint(BCLog::GOBJECT, "gobject_submit -- pubKeyOperator = %s, outpoint = %s, params.size() = %lld, fMnFound = %d\n",
322+
(activeMasternodeInfo.blsPubKeyOperator ? activeMasternodeInfo.blsPubKeyOperator->ToString(activeMasternodeInfo.legacy) : "N/A"),
323+
activeMasternodeInfo.outpoint.ToStringShort(), request.params.size(), fMnFound);
324+
} else {
325+
LogPrint(BCLog::GOBJECT, "gobject_submit -- pubKeyOperator = N/A, outpoint = N/A, params.size() = %lld, fMnFound = %d\n",
326+
request.params.size(), fMnFound);
327+
}
320328

321329
// ASSEMBLE NEW GOVERNANCE OBJECT FROM USER PARAMETERS
322330

‎src/rpc/masternode.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,8 +257,9 @@ static UniValue masternode_status(const JSONRPCRequest& request)
257257
{
258258
masternode_status_help(request);
259259

260-
if (!fMasternodeMode)
261-
throw JSONRPCError(RPC_INTERNAL_ERROR, "This is not a masternode");
260+
if (!fMasternodeMode) {
261+
throw JSONRPCError(RPC_INTERNAL_ERROR, "This node does not run an active masternode.");
262+
}
262263

263264
const NodeContext& node = EnsureAnyNodeContext(request.context);
264265

0 commit comments

Comments
 (0)