Skip to content

Commit

Permalink
Merge #5940: refactor: consolidate activeMasternodeInfo{Cs} into CAct…
Browse files Browse the repository at this point in the history 
…iveMasternodeManager, create NodeContext alias, reduce globals usage

815e4f8 masternode: protect m_{error,state} with cs (pasta)
136e445 refactor: pass CActiveMasternodeManager as pointer arg to LLMQContext (Kittywhiskers Van Gogh)
5e0f777 refactor: pass CActiveMasternodeManager as pointer arg to CJContext (Kittywhiskers Van Gogh)
f171c24 refactor: add CActiveMasternodeManager NodeContext alias, use in RPC (Kittywhiskers Van Gogh)
44beb94 refactor: prefix member variable names with m_ (Kittywhiskers Van Gogh)
73cef4f refactor: make bls{Pub}KeyOperator member variables instead of pointers (Kittywhiskers Van Gogh)
fbc7836 refactor: make m_info private, get const refs (or copies) from Get*() functions (Kittywhiskers Van Gogh)
1b516ce refactor: use signing helper function instead of passing blsKeyOperator (Kittywhiskers Van Gogh)
33702ac refactor: add helper function to decrypt messages with blsKeyOperator (Kittywhiskers Van Gogh)
3eb931b refactor: add helper function to sign messages with blsKeyOperator (Kittywhiskers Van Gogh)
3827355 refactor: move key initialization to InitKeys, define destructor (Kittywhiskers Van Gogh)
e5295de refactor: move activeMasternodeInfo{Cs} into CActiveMasternodeManager (Kittywhiskers Van Gogh)
b8c1f01 refactor: avoid accessing active masternode info if not in masternode mode (Kittywhiskers Van Gogh)
9a3c5a3 trivial: access activeMasternodeInfo when lock is in scope (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * `CActiveMasternodeManager`, unlike other managers, is _conditionally_ initialized (specifically, when the node is hosting a masternode). This means that checks need to be made to ensure that the conditions needed to initialize the manager are true or that the pointer leads to a valid manager instance.

    As the codebase currently checks (and fast-fails) based on the node being in "masternode mode" (`fMasternodeMode`) or not, we will continue with this approach, but with additional assertions _after_ the masternode mode check if the manager exists.

  * Though, since `activeMasternodeInfo`(`Cs`) are global variables, they can be accessed _regardless_ of whether the corresponding manager exists. This means some parts of the codebase attempt to fetch information about the (nonexistent) active masternode _before_ determining if it should use the masternode mode path or not (looking at you, `CMNAuth::ProcessMessage`)

    Moving them into `CActiveMasternodeManager` meant adding checks _before_ attempting to access information about the masternode, as they would no longer be accessible with dummy values ([here](https://github.com/dashpay/dash/blob/2110c0c30983e8f3da02f11907bfe4e1b5dcd89f/src/init.cpp#L1633-L1635)) on account of being part of the conditionally initialized manager.
    * In an attempt to opportunistically dereference the manager, `CDKGSessionManager` (accepting a pointer) was dereferencing the manager before passing it to `CDKGSessionHandler`. This was done under the assumption that  `CDKGSessionManager` would only ever be initialized in masternode mode.

      This is not true. I can confirm that because I spent a few days trying to debug test failures. `CDKGSessionHandler` is initialized in two scenarios:

      * In masternode mode
      * If the `-watchquorums` flag is enabled

      The latter scenario doesn't initialize `CActiveMasternodeManager`.

      Furthermore, the DKG round thread is started unconditionally ([here](https://github.com/dashpay/dash/blob/2110c0c30983e8f3da02f11907bfe4e1b5dcd89f/src/llmq/context.cpp#L79)) and the `CDKGSessionHandler::StartThreads` > `CDKGSessionHandler::StartThread` > `CDKGSessionHandler::PhaseHandlerThread` > `CDKGSessionHandler::HandleDKGRound` > `CDKGSessionHandler::InitNewQuorum` > `CActiveMasternodeManager::GetProTxHash` call chain reveals an attempt to fetch active masternode information without any masternode mode checks.

      This behaviour has now been changed and the thread will only be spun up if in masternode mode.

    * Dereferencing so far has been limited to objects that primarily hold data (like `CCoinJoinBroadcastTx` or `CGovernanceObject`) as they should not have knowledge of node's state (that responsibility lies with whatever manager manipulates those objects), perform one-off operations and static functions.

  * `activeMasternodeInfo` allowed its members to be read-write accessible to anybody who asked. Additionally, signing and decrypting involved borrowing the operator secret key from the active masternode state to perform those operations.

     This behaviour has now been changed. The internal state is now private and accessible read-only as a const ref (or copy) and `Decrypt`/`Sign` functions have been implemented to allow those operations to happen without having another manager access the operator private key in order to do so.

  * You cannot combine a `WITH_LOCK` and an `Assert` (in either mutex or accessed value), doing so will cause errors if `-Werror=thread-safety` is enabled. This is why `assert`s are added even when it would intuitively seem that `Assert` would've been more appropriate to use.

  ## Future Considerations

  Currently there are no unit tests that test the functionality of `CActiveMasternodeManager` as it's never initialized in test contexts, breakage had to be found using functional tests. Perhaps some (rudimentary) tests for `CActiveMasternodeManager` may prove to be valuable.

  ## Breaking Changes

  Not _really_. Some behaviour has been modified but nothing that should necessitate updates or upgrades.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK 815e4f8

Tree-SHA512: cbe49ea9e1c35df514e1b40869ee271baef1c348c9d09e4b356e5fc8fe5449cbbe66569258f2d664029faa9a46f711df9bf9e41eb8734c3aefc6cd8e94378948
  • Loading branch information
@PastaPastaPasta
PastaPastaPasta committed Mar 26, 2024
2 parents f2a42a0 + 815e4f8 commit f217e0a
Show file tree
Hide file tree
Showing 36 changed files with 391 additions and 271 deletions.
12 changes: 4 additions & 8 deletions src/coinjoin/coinjoin.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,10 @@ uint256 CCoinJoinQueue::GetSignatureHash() const
return SerializeHash(*this, SER_GETHASH, PROTOCOL_VERSION);
}

bool CCoinJoinQueue::Sign()
bool CCoinJoinQueue::Sign(const CActiveMasternodeManager& mn_activeman)
{
if (!fMasternodeMode) return false;

uint256 hash = GetSignatureHash();
CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash, false));
CBLSSignature sig = mn_activeman.Sign(hash, /*is_legacy=*/ false);
if (!sig.IsValid()) {
return false;
}
Expand Down Expand Up @@ -99,12 +97,10 @@ uint256 CCoinJoinBroadcastTx::GetSignatureHash() const
return SerializeHash(*this, SER_GETHASH, PROTOCOL_VERSION);
}

bool CCoinJoinBroadcastTx::Sign()
bool CCoinJoinBroadcastTx::Sign(const CActiveMasternodeManager& mn_activeman)
{
if (!fMasternodeMode) return false;

uint256 hash = GetSignatureHash();
CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash, false));
CBLSSignature sig = mn_activeman.Sign(hash, /*is_legacy=*/ false);
if (!sig.IsValid()) {
return false;
}
Expand Down
5 changes: 3 additions & 2 deletions src/coinjoin/coinjoin.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <optional>
#include <utility>

class CActiveMasternodeManager;
class CChainState;
class CConnman;
class CBLSPublicKey;
Expand Down Expand Up @@ -213,7 +214,7 @@ class CCoinJoinQueue
* 3) we signed the message successfully, and
* 4) we verified the message successfully
*/
bool Sign();
bool Sign(const CActiveMasternodeManager& mn_activeman);
/// Check if we have a valid Masternode address
[[nodiscard]] bool CheckSignature(const CBLSPublicKey& blsPubKey) const;

Expand Down Expand Up @@ -284,7 +285,7 @@ class CCoinJoinBroadcastTx

[[nodiscard]] uint256 GetSignatureHash() const;

bool Sign();
bool Sign(const CActiveMasternodeManager& mn_activeman);
[[nodiscard]] bool CheckSignature(const CBLSPublicKey& blsPubKey) const;

void SetConfirmedHeight(std::optional<int> nConfirmedHeightIn) { assert(nConfirmedHeightIn == std::nullopt || *nConfirmedHeightIn > 0); nConfirmedHeight = nConfirmedHeightIn; }
Expand Down
4 changes: 2 additions & 2 deletions src/coinjoin/context.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
#include <coinjoin/server.h>

CJContext::CJContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CTxMemPool& mempool,
const CMasternodeSync& mn_sync, bool relay_txes) :
const CActiveMasternodeManager* mn_activeman, const CMasternodeSync& mn_sync, bool relay_txes) :
dstxman{std::make_unique<CDSTXManager>()},
#ifdef ENABLE_WALLET
walletman{std::make_unique<CoinJoinWalletManager>(connman, dmnman, mempool, mn_sync, queueman)},
queueman {relay_txes ? std::make_unique<CCoinJoinClientQueueManager>(connman, *walletman, dmnman, mn_sync) : nullptr},
#endif // ENABLE_WALLET
server{std::make_unique<CCoinJoinServer>(chainstate, connman, dmnman, *dstxman, mempool, mn_sync)}
server{std::make_unique<CCoinJoinServer>(chainstate, connman, dmnman, *dstxman, mempool, mn_activeman, mn_sync)}
{}

CJContext::~CJContext() {}
3 changes: 2 additions & 1 deletion src/coinjoin/context.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#include <memory>

class CActiveMasternodeManager;
class CBlockPolicyEstimator;
class CChainState;
class CCoinJoinServer;
Expand All @@ -29,7 +30,7 @@ struct CJContext {
CJContext() = delete;
CJContext(const CJContext&) = delete;
CJContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CTxMemPool& mempool,
const CMasternodeSync& mn_sync, bool relay_txes);
const CActiveMasternodeManager* mn_activeman, const CMasternodeSync& mn_sync, bool relay_txes);
~CJContext();

const std::unique_ptr<CDSTXManager> dstxman;
Expand Down
30 changes: 19 additions & 11 deletions src/coinjoin/server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ PeerMsgRet CCoinJoinServer::ProcessMessage(CNode& peer, std::string_view msg_typ

void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
{
assert(m_mn_activeman);

if (IsSessionReady()) {
// too many users in this session already, reject new ones
LogPrint(BCLog::COINJOIN, "DSACCEPT -- queue is already full!\n");
Expand All @@ -56,7 +58,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
LogPrint(BCLog::COINJOIN, "DSACCEPT -- nDenom %d (%s) txCollateral %s", dsa.nDenom, CoinJoin::DenominationToString(dsa.nDenom), dsa.txCollateral.ToString()); /* Continued */

auto mnList = m_dmnman.GetListAtChainTip();
auto dmn = WITH_LOCK(activeMasternodeInfoCs, return mnList.GetValidMNByCollateral(activeMasternodeInfo.outpoint));
auto dmn = WITH_LOCK(m_mn_activeman->cs, return mnList.GetValidMNByCollateral(m_mn_activeman->GetOutPoint()));
if (!dmn) {
PushStatus(peer, STATUS_REJECTED, ERR_MN_LIST);
return;
Expand All @@ -67,7 +69,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
TRY_LOCK(cs_vecqueue, lockRecv);
if (!lockRecv) return;

auto mnOutpoint = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint);
auto mnOutpoint = WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetOutPoint());

if (ranges::any_of(vecCoinJoinQueue,
[&mnOutpoint](const auto& q){return q.masternodeOutpoint == mnOutpoint;})) {
Expand Down Expand Up @@ -308,6 +310,8 @@ void CCoinJoinServer::CommitFinalTransaction()
AssertLockNotHeld(cs_coinjoin);
if (!fMasternodeMode) return; // check and relay final tx only on masternode

assert(m_mn_activeman);

CTransactionRef finalTransaction = WITH_LOCK(cs_coinjoin, return MakeTransactionRef(finalMutableTransaction));
uint256 hashTx = finalTransaction->GetHash();

Expand All @@ -331,10 +335,10 @@ void CCoinJoinServer::CommitFinalTransaction()
// create and sign masternode dstx transaction
if (!m_dstxman.GetDSTX(hashTx)) {
CCoinJoinBroadcastTx dstxNew(finalTransaction,
WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint),
WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash),
WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetOutPoint()),
WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash()),
GetAdjustedTime());
dstxNew.Sign();
dstxNew.Sign(*m_mn_activeman);
m_dstxman.AddDSTX(dstxNew);
}

Expand Down Expand Up @@ -495,16 +499,18 @@ void CCoinJoinServer::CheckForCompleteQueue()
{
if (!fMasternodeMode) return;

assert(m_mn_activeman);

if (nState == POOL_STATE_QUEUE && IsSessionReady()) {
SetState(POOL_STATE_ACCEPTING_ENTRIES);

CCoinJoinQueue dsq(nSessionDenom,
WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint),
WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash),
WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetOutPoint()),
WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash()),
GetAdjustedTime(), true);
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckForCompleteQueue -- queue is ready, signing and relaying (%s) " /* Continued */
"with %d participants\n", dsq.ToString(), vecSessionCollaterals.size());
dsq.Sign();
dsq.Sign(*m_mn_activeman);
dsq.Relay(connman);
}
}
Expand Down Expand Up @@ -692,6 +698,8 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
{
if (!fMasternodeMode || nSessionID != 0) return false;

assert(m_mn_activeman);

// new session can only be started in idle mode
if (nState != POOL_STATE_IDLE) {
nMessageIDRet = ERR_MODE;
Expand All @@ -713,11 +721,11 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
if (!fUnitTest) {
//broadcast that I'm accepting entries, only if it's the first entry through
CCoinJoinQueue dsq(nSessionDenom,
WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint),
WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash),
WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetOutPoint()),
WITH_LOCK(m_mn_activeman->cs, return m_mn_activeman->GetProTxHash()),
GetAdjustedTime(), false);
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString());
dsq.Sign();
dsq.Sign(*m_mn_activeman);
dsq.Relay(connman);
LOCK(cs_vecqueue);
vecCoinJoinQueue.push_back(dsq);
Expand Down
5 changes: 4 additions & 1 deletion src/coinjoin/server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <net_types.h>

class CActiveMasternodeManager;
class CChainState;
class CCoinJoinServer;
class CDataStream;
Expand All @@ -29,6 +30,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
CDeterministicMNManager& m_dmnman;
CDSTXManager& m_dstxman;
CTxMemPool& mempool;
const CActiveMasternodeManager* m_mn_activeman;
const CMasternodeSync& m_mn_sync;

// Mixing uses collateral transactions to trust parties entering the pool
Expand Down Expand Up @@ -85,12 +87,13 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager

public:
explicit CCoinJoinServer(CChainState& chainstate, CConnman& _connman, CDeterministicMNManager& dmnman, CDSTXManager& dstxman,
CTxMemPool& mempool, const CMasternodeSync& mn_sync) :
CTxMemPool& mempool, const CActiveMasternodeManager* mn_activeman, const CMasternodeSync& mn_sync) :
m_chainstate(chainstate),
connman(_connman),
m_dmnman(dmnman),
m_dstxman(dstxman),
mempool(mempool),
m_mn_activeman(mn_activeman),
m_mn_sync(mn_sync),
vecSessionCollaterals(),
fUnitTest(false)
Expand Down
67 changes: 37 additions & 30 deletions src/evo/mnauth.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,40 +21,45 @@

void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CBlockIndex* tip)
{
LOCK(activeMasternodeInfoCs);
if (!fMasternodeMode || activeMasternodeInfo.proTxHash.IsNull()) {
return;
}
if (!fMasternodeMode) return;

CMNAuth mnauth;
uint256 signHash;
const auto receivedMNAuthChallenge = peer.GetReceivedMNAuthChallenge();
if (receivedMNAuthChallenge.IsNull()) {
return;
}
// We include fInbound in signHash to forbid interchanging of challenges by a man in the middle (MITM). This way
// we protect ourselves against MITM in this form:
// node1 <- Eve -> node2
// It does not protect against:
// node1 -> Eve -> node2
// This is ok as we only use MNAUTH as a DoS protection and not for sensitive stuff
int nOurNodeVersion{PROTOCOL_VERSION};
if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
}
const bool is_basic_scheme_active{DeploymentActiveAfter(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)};
const CBLSPublicKeyVersionWrapper pubKey(*activeMasternodeInfo.blsPubKeyOperator, !is_basic_scheme_active);
if (peer.nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
signHash = ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn()));
} else {
signHash = ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn(), nOurNodeVersion));
}
{
LOCK(::activeMasternodeManager->cs);
if (::activeMasternodeManager->GetProTxHash().IsNull()) {
return;
}

CMNAuth mnauth;
mnauth.proRegTxHash = activeMasternodeInfo.proTxHash;
mnauth.sig = activeMasternodeInfo.blsKeyOperator->Sign(signHash);
const auto receivedMNAuthChallenge = peer.GetReceivedMNAuthChallenge();
if (receivedMNAuthChallenge.IsNull()) {
return;
}
// We include fInbound in signHash to forbid interchanging of challenges by a man in the middle (MITM). This way
// we protect ourselves against MITM in this form:
// node1 <- Eve -> node2
// It does not protect against:
// node1 -> Eve -> node2
// This is ok as we only use MNAUTH as a DoS protection and not for sensitive stuff
int nOurNodeVersion{PROTOCOL_VERSION};
if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
}
const bool is_basic_scheme_active{DeploymentActiveAfter(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)};
auto pk = ::activeMasternodeManager->GetPubKey();
const CBLSPublicKeyVersionWrapper pubKey(pk, !is_basic_scheme_active);
if (peer.nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
signHash = ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn()));
} else {
signHash = ::SerializeHash(std::make_tuple(pubKey, receivedMNAuthChallenge, peer.IsInboundConn(), nOurNodeVersion));
}

LogPrint(BCLog::NET_NETCONN, "CMNAuth::%s -- Sending MNAUTH, peer=%d\n", __func__, peer.GetId());
mnauth.proRegTxHash = ::activeMasternodeManager->GetProTxHash();
} // ::activeMasternodeManager->cs

mnauth.sig = ::activeMasternodeManager->Sign(signHash);

LogPrint(BCLog::NET_NETCONN, "CMNAuth::%s -- Sending MNAUTH, peer=%d\n", __func__, peer.GetId());
connman.PushMessage(&peer, CNetMsgMaker(peer.GetCommonVersion()).Make(NetMsgType::MNAUTH, mnauth));
}

Expand Down Expand Up @@ -127,7 +132,9 @@ PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, CConnman& connman, const CDeterm
}
}

const uint256 myProTxHash = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash);
const uint256 myProTxHash = fMasternodeMode ?
WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash()) :
uint256();

connman.ForEachNode([&](CNode* pnode2) {
if (peer.fDisconnect) {
Expand Down
17 changes: 9 additions & 8 deletions src/governance/governance.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -692,14 +692,14 @@ std::optional<const CGovernanceObject> CGovernanceManager::CreateGovernanceTrigg
}

{
LOCK(activeMasternodeInfoCs);
if (mn_payees.front()->proTxHash != activeMasternodeInfo.proTxHash) {
LOCK(::activeMasternodeManager->cs);
if (mn_payees.front()->proTxHash != ::activeMasternodeManager->GetProTxHash()) {
LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s we are not the payee, skipping\n", __func__);
return std::nullopt;
}
gov_sb.SetMasternodeOutpoint(activeMasternodeInfo.outpoint);
gov_sb.Sign( *activeMasternodeInfo.blsKeyOperator);
} // activeMasternodeInfoCs
gov_sb.SetMasternodeOutpoint(::activeMasternodeManager->GetOutPoint());
} // ::activeMasternodeManager->cs
gov_sb.Sign(*::activeMasternodeManager);

if (std::string strError; !gov_sb.IsValidLocally(m_dmnman->GetListAtChainTip(), strError, true)) {
LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Created trigger is invalid:%s\n", __func__, strError);
Expand All @@ -719,7 +719,8 @@ std::optional<const CGovernanceObject> CGovernanceManager::CreateGovernanceTrigg
void CGovernanceManager::VoteGovernanceTriggers(const std::optional<const CGovernanceObject>& trigger_opt, CConnman& connman)
{
// only active masternodes can vote on triggers
if (!fMasternodeMode || WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.proTxHash.IsNull())) return;
if (!fMasternodeMode) return;
if (WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetProTxHash().IsNull())) return;

LOCK2(cs_main, cs);

Expand Down Expand Up @@ -762,9 +763,9 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optional<const CGover

bool CGovernanceManager::VoteFundingTrigger(const uint256& nHash, const vote_outcome_enum_t outcome, CConnman& connman)
{
CGovernanceVote vote(WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint), nHash, VOTE_SIGNAL_FUNDING, outcome);
CGovernanceVote vote(WITH_LOCK(::activeMasternodeManager->cs, return ::activeMasternodeManager->GetOutPoint()), nHash, VOTE_SIGNAL_FUNDING, outcome);
vote.SetTime(GetAdjustedTime());
vote.Sign(WITH_LOCK(activeMasternodeInfoCs, return *activeMasternodeInfo.blsKeyOperator));
vote.Sign(*::activeMasternodeManager);

CGovernanceException exception;
if (!ProcessVoteAndRelay(vote, exception, connman)) {
Expand Down
5 changes: 3 additions & 2 deletions src/governance/object.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <governance/governance.h>
#include <governance/validators.h>
#include <masternode/meta.h>
#include <masternode/node.h>
#include <masternode/sync.h>
#include <messagesigner.h>
#include <net.h>
Expand Down Expand Up @@ -273,9 +274,9 @@ void CGovernanceObject::SetMasternodeOutpoint(const COutPoint& outpoint)
m_obj.masternodeOutpoint = outpoint;
}

bool CGovernanceObject::Sign(const CBLSSecretKey& key)
bool CGovernanceObject::Sign(const CActiveMasternodeManager& mn_activeman)
{
CBLSSignature sig = key.Sign(GetSignatureHash(), false);
CBLSSignature sig = mn_activeman.Sign(GetSignatureHash(), false);
if (!sig.IsValid()) {
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions src/governance/object.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

#include <univalue.h>

class CBLSSecretKey;
class CActiveMasternodeManager;
class CBLSPublicKey;
class CDeterministicMNList;
class CGovernanceManager;
Expand Down Expand Up @@ -217,7 +217,7 @@ class CGovernanceObject
// Signature related functions

void SetMasternodeOutpoint(const COutPoint& outpoint);
bool Sign(const CBLSSecretKey& key);
bool Sign(const CActiveMasternodeManager& mn_activeman);
bool CheckSignature(const CBLSPublicKey& pubKey) const;

uint256 GetSignatureHash() const;
Expand Down
Loading

0 comments on commit f217e0a

Please sign in to comment.