From 8845d4f7d8323fa80ae4625b408885ceea05740a Mon Sep 17 00:00:00 2001 From: Nathan Marley Date: Thu, 24 Oct 2019 11:28:31 -0300 Subject: [PATCH 1/6] Update activemn if protx info changed --- src/masternode/activemasternode.cpp | 38 +++++++++++++++++++++++++++-- src/masternode/activemasternode.h | 1 + 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/masternode/activemasternode.cpp b/src/masternode/activemasternode.cpp index a5eb145cefe6..3ca8beef2f7b 100644 --- a/src/masternode/activemasternode.cpp +++ b/src/masternode/activemasternode.cpp @@ -15,6 +15,9 @@ CActiveMasternodeInfo activeMasternodeInfo; CActiveMasternodeManager* activeMasternodeManager; +// Would prefer a lambda func instead +bool detMNInfoChanged(const CDeterministicMNStateCPtr& a, const CDeterministicMNStateCPtr& b); + std::string CActiveMasternodeManager::GetStateString() const { switch (state) { @@ -26,6 +29,8 @@ std::string CActiveMasternodeManager::GetStateString() const return "REMOVED"; case MASTERNODE_OPERATOR_KEY_CHANGED: return "OPERATOR_KEY_CHANGED"; + case MASTERNODE_PROTX_INFO_CHANGED: + return "PROTX_INFO_CHANGED"; case MASTERNODE_READY: return "READY"; case MASTERNODE_ERROR: @@ -46,6 +51,8 @@ std::string CActiveMasternodeManager::GetStatus() const return "Masternode removed from list"; case MASTERNODE_OPERATOR_KEY_CHANGED: return "Operator key changed or revoked"; + case MASTERNODE_PROTX_INFO_CHANGED: + return "ProTx information changed"; case MASTERNODE_READY: return "Ready"; case MASTERNODE_ERROR: @@ -142,20 +149,47 @@ void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, con activeMasternodeInfo.outpoint.SetNull(); // MN might have reappeared in same block with a new ProTx Init(); - } else if (mnList.GetMN(mnListEntry->proTxHash)->pdmnState->pubKeyOperator != mnListEntry->pdmnState->pubKeyOperator) { + return; + } + + const auto pDMNState = (mnList.GetMN(mnListEntry->proTxHash)->pdmnState); + if (pDMNState->pubKeyOperator != mnListEntry->pdmnState->pubKeyOperator) { // MN operator key changed or revoked state = MASTERNODE_OPERATOR_KEY_CHANGED; activeMasternodeInfo.proTxHash = uint256(); activeMasternodeInfo.outpoint.SetNull(); // MN might have reappeared in same block with a new ProTx Init(); + return; + } + + // reset w/status MN Protx info changed + if (detMNInfoChanged(pDMNState, mnListEntry->pdmnState)) { + // MN protx info changed + state = MASTERNODE_PROTX_INFO_CHANGED; + activeMasternodeInfo.proTxHash = uint256(); + activeMasternodeInfo.outpoint.SetNull(); + Init(); + return; } } else { - // MN might have (re)appeared with a new ProTx or we've found some peers and figured out our local address + // MN might have (re)appeared with a new ProTx or we've found some peers + // and figured out our local address Init(); } } +bool detMNInfoChanged(const CDeterministicMNStateCPtr& a, const CDeterministicMNStateCPtr& b) +{ + return a->pubKeyOperator != b->pubKeyOperator || + a->keyIDOwner != b->keyIDOwner || + a->keyIDVoting != b->keyIDVoting || + a->addr != b->addr || + a->scriptPayout != b->scriptPayout || + a->scriptOperatorPayout != b->scriptOperatorPayout + ; +} + bool CActiveMasternodeManager::GetLocalAddress(CService& addrRet) { // First try to find whatever local address is specified by externalip option diff --git a/src/masternode/activemasternode.h b/src/masternode/activemasternode.h index c5da79512ea5..d431d9db4956 100644 --- a/src/masternode/activemasternode.h +++ b/src/masternode/activemasternode.h @@ -42,6 +42,7 @@ class CActiveMasternodeManager : public CValidationInterface MASTERNODE_OPERATOR_KEY_CHANGED, MASTERNODE_READY, MASTERNODE_ERROR, + MASTERNODE_PROTX_INFO_CHANGED, }; private: From fba468758113084785c0bc0fffd6d704911be8a3 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 24 Oct 2019 20:05:03 +0300 Subject: [PATCH 2/6] Add `==` and `!=` operators to CDeterministicMNState --- src/evo/deterministicmns.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index 18c0289c96c3..aae1453c3898 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -112,6 +112,28 @@ class CDeterministicMNState public: std::string ToString() const; void ToJson(UniValue& obj) const; + + friend bool operator==(const CDeterministicMNState& a, const CDeterministicMNState& b) + { + return a.nRegisteredHeight == b.nRegisteredHeight + && a.nLastPaidHeight == b.nLastPaidHeight + && a.nPoSePenalty == b.nPoSePenalty + && a.nPoSeRevivedHeight == b.nPoSeRevivedHeight + && a.nPoSeBanHeight == b.nPoSeBanHeight + && a.nRevocationReason == b.nRevocationReason + && a.confirmedHash == b.confirmedHash + && a.confirmedHashWithProRegTxHash == b.confirmedHashWithProRegTxHash + && a.keyIDOwner == b.keyIDOwner + && a.pubKeyOperator == b.pubKeyOperator + && a.keyIDVoting == b.keyIDVoting + && a.addr == b.addr + && a.scriptPayout == b.scriptPayout + && a.scriptOperatorPayout == b.scriptOperatorPayout; + } + friend bool operator!=(const CDeterministicMNState& a, const CDeterministicMNState& b) + { + return !(a == b); + } }; typedef std::shared_ptr CDeterministicMNStatePtr; typedef std::shared_ptr CDeterministicMNStateCPtr; From fc195b3aa4bc2ec80a33095fa48ffdd7831f6738 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 24 Oct 2019 20:05:54 +0300 Subject: [PATCH 3/6] Only re-init active MN if its IP changed, changes to payout, voting etc. can be done without it --- src/masternode/activemasternode.cpp | 39 +++++++++++------------------ src/masternode/activemasternode.h | 2 +- 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/src/masternode/activemasternode.cpp b/src/masternode/activemasternode.cpp index 3ca8beef2f7b..8a2a9f68ce64 100644 --- a/src/masternode/activemasternode.cpp +++ b/src/masternode/activemasternode.cpp @@ -15,9 +15,6 @@ CActiveMasternodeInfo activeMasternodeInfo; CActiveMasternodeManager* activeMasternodeManager; -// Would prefer a lambda func instead -bool detMNInfoChanged(const CDeterministicMNStateCPtr& a, const CDeterministicMNStateCPtr& b); - std::string CActiveMasternodeManager::GetStateString() const { switch (state) { @@ -29,8 +26,8 @@ std::string CActiveMasternodeManager::GetStateString() const return "REMOVED"; case MASTERNODE_OPERATOR_KEY_CHANGED: return "OPERATOR_KEY_CHANGED"; - case MASTERNODE_PROTX_INFO_CHANGED: - return "PROTX_INFO_CHANGED"; + case MASTERNODE_PROTX_IP_CHANGED: + return "PROTX_IP_CHANGED"; case MASTERNODE_READY: return "READY"; case MASTERNODE_ERROR: @@ -51,8 +48,8 @@ std::string CActiveMasternodeManager::GetStatus() const return "Masternode removed from list"; case MASTERNODE_OPERATOR_KEY_CHANGED: return "Operator key changed or revoked"; - case MASTERNODE_PROTX_INFO_CHANGED: - return "ProTx information changed"; + case MASTERNODE_PROTX_IP_CHANGED: + return "IP address specified in ProTx changed"; case MASTERNODE_READY: return "Ready"; case MASTERNODE_ERROR: @@ -152,8 +149,8 @@ void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, con return; } - const auto pDMNState = (mnList.GetMN(mnListEntry->proTxHash)->pdmnState); - if (pDMNState->pubKeyOperator != mnListEntry->pdmnState->pubKeyOperator) { + auto mnPtr = mnList.GetMN(mnListEntry->proTxHash); + if (mnPtr->pdmnState->pubKeyOperator != mnListEntry->pdmnState->pubKeyOperator) { // MN operator key changed or revoked state = MASTERNODE_OPERATOR_KEY_CHANGED; activeMasternodeInfo.proTxHash = uint256(); @@ -163,15 +160,20 @@ void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, con return; } - // reset w/status MN Protx info changed - if (detMNInfoChanged(pDMNState, mnListEntry->pdmnState)) { - // MN protx info changed - state = MASTERNODE_PROTX_INFO_CHANGED; + if (mnPtr->pdmnState->addr != mnListEntry->pdmnState->addr) { + // MN IP changed + state = MASTERNODE_PROTX_IP_CHANGED; activeMasternodeInfo.proTxHash = uint256(); activeMasternodeInfo.outpoint.SetNull(); Init(); return; } + + if (*mnPtr->pdmnState != *mnListEntry->pdmnState) { + // some non-critical changes in MN state + mnListEntry = mnPtr; + return; + } } else { // MN might have (re)appeared with a new ProTx or we've found some peers // and figured out our local address @@ -179,17 +181,6 @@ void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, con } } -bool detMNInfoChanged(const CDeterministicMNStateCPtr& a, const CDeterministicMNStateCPtr& b) -{ - return a->pubKeyOperator != b->pubKeyOperator || - a->keyIDOwner != b->keyIDOwner || - a->keyIDVoting != b->keyIDVoting || - a->addr != b->addr || - a->scriptPayout != b->scriptPayout || - a->scriptOperatorPayout != b->scriptOperatorPayout - ; -} - bool CActiveMasternodeManager::GetLocalAddress(CService& addrRet) { // First try to find whatever local address is specified by externalip option diff --git a/src/masternode/activemasternode.h b/src/masternode/activemasternode.h index d431d9db4956..f33ffc68c2aa 100644 --- a/src/masternode/activemasternode.h +++ b/src/masternode/activemasternode.h @@ -40,9 +40,9 @@ class CActiveMasternodeManager : public CValidationInterface MASTERNODE_POSE_BANNED, MASTERNODE_REMOVED, MASTERNODE_OPERATOR_KEY_CHANGED, + MASTERNODE_PROTX_IP_CHANGED, MASTERNODE_READY, MASTERNODE_ERROR, - MASTERNODE_PROTX_INFO_CHANGED, }; private: From 4120ca430ff67c6b39a4d2766bff55a37b829fbd Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 24 Oct 2019 20:08:58 +0300 Subject: [PATCH 4/6] Test `masternode status` updates --- test/functional/dip3-deterministicmns.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/functional/dip3-deterministicmns.py b/test/functional/dip3-deterministicmns.py index 94137ea7627b..bdef2d8f6da9 100755 --- a/test/functional/dip3-deterministicmns.py +++ b/test/functional/dip3-deterministicmns.py @@ -184,6 +184,23 @@ def run_test(self): self.start_mn(new_mn) self.sync_all() + self.log.info("testing masternode status updates") + # change voting address and see if changes are reflected in `masternode status` rpc output + mn = mns[0] + node = self.nodes[0] + old_dmnState = mn.node.masternode("status")["dmnState"] + old_voting_address = old_dmnState["votingAddress"] + new_voting_address = node.getnewaddress() + assert(old_voting_address != new_voting_address) + # also check if funds from payout address are used when no fee source address is specified + node.sendtoaddress(mn.rewards_address, 0.001) + node.protx('update_registrar', mn.protx_hash, "", new_voting_address, mn.rewards_address) + node.generate(1) + self.sync_all() + new_dmnState = mn.node.masternode("status")["dmnState"] + new_voting_address_from_rpc = new_dmnState["votingAddress"] + assert(new_voting_address_from_rpc == new_voting_address) + def prepare_mn(self, node, idx, alias): mn = Masternode() mn.idx = idx From 3e0ff6e35d19eff84a1e02c6a89b1f4dc7e6be8c Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 29 Oct 2019 10:07:42 +0100 Subject: [PATCH 5/6] Don't track mnListEntry anymore and instead get the DMN on demand --- src/masternode/activemasternode.cpp | 28 +++++++++++----------------- src/masternode/activemasternode.h | 3 --- src/rpc/masternode.cpp | 2 +- 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/masternode/activemasternode.cpp b/src/masternode/activemasternode.cpp index 8a2a9f68ce64..fdfc790436c9 100644 --- a/src/masternode/activemasternode.cpp +++ b/src/masternode/activemasternode.cpp @@ -98,11 +98,9 @@ void CActiveMasternodeManager::Init() return; } - mnListEntry = dmn; + LogPrintf("CActiveMasternodeManager::Init -- proTxHash=%s, proTx=%s\n", dmn->proTxHash.ToString(), dmn->ToString()); - LogPrintf("CActiveMasternodeManager::Init -- proTxHash=%s, proTx=%s\n", mnListEntry->proTxHash.ToString(), mnListEntry->ToString()); - - if (activeMasternodeInfo.service != mnListEntry->pdmnState->addr) { + if (activeMasternodeInfo.service != dmn->pdmnState->addr) { state = MASTERNODE_ERROR; strError = "Local address does not match the address from ProTx"; LogPrintf("CActiveMasternodeManager::Init -- ERROR: %s", strError); @@ -124,8 +122,8 @@ void CActiveMasternodeManager::Init() } } - activeMasternodeInfo.proTxHash = mnListEntry->proTxHash; - activeMasternodeInfo.outpoint = mnListEntry->collateralOutpoint; + activeMasternodeInfo.proTxHash = dmn->proTxHash; + activeMasternodeInfo.outpoint = dmn->collateralOutpoint; state = MASTERNODE_READY; } @@ -138,8 +136,9 @@ void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, con if (!deterministicMNManager->IsDIP3Enforced(pindexNew->nHeight)) return; if (state == MASTERNODE_READY) { - auto mnList = deterministicMNManager->GetListForBlock(pindexNew); - if (!mnList.IsMNValid(mnListEntry->proTxHash)) { + auto oldMNList = deterministicMNManager->GetListForBlock(pindexNew->pprev); + auto newMNList = deterministicMNManager->GetListForBlock(pindexNew); + if (!newMNList.IsMNValid(activeMasternodeInfo.proTxHash)) { // MN disappeared from MN list state = MASTERNODE_REMOVED; activeMasternodeInfo.proTxHash = uint256(); @@ -149,8 +148,9 @@ void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, con return; } - auto mnPtr = mnList.GetMN(mnListEntry->proTxHash); - if (mnPtr->pdmnState->pubKeyOperator != mnListEntry->pdmnState->pubKeyOperator) { + auto oldDmn = oldMNList.GetMN(activeMasternodeInfo.proTxHash); + auto newDmn = newMNList.GetMN(activeMasternodeInfo.proTxHash); + if (newDmn->pdmnState->pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) { // MN operator key changed or revoked state = MASTERNODE_OPERATOR_KEY_CHANGED; activeMasternodeInfo.proTxHash = uint256(); @@ -160,7 +160,7 @@ void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, con return; } - if (mnPtr->pdmnState->addr != mnListEntry->pdmnState->addr) { + if (newDmn->pdmnState->addr != oldDmn->pdmnState->addr) { // MN IP changed state = MASTERNODE_PROTX_IP_CHANGED; activeMasternodeInfo.proTxHash = uint256(); @@ -168,12 +168,6 @@ void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, con Init(); return; } - - if (*mnPtr->pdmnState != *mnListEntry->pdmnState) { - // some non-critical changes in MN state - mnListEntry = mnPtr; - return; - } } else { // MN might have (re)appeared with a new ProTx or we've found some peers // and figured out our local address diff --git a/src/masternode/activemasternode.h b/src/masternode/activemasternode.h index f33ffc68c2aa..f205cbedfae6 100644 --- a/src/masternode/activemasternode.h +++ b/src/masternode/activemasternode.h @@ -46,7 +46,6 @@ class CActiveMasternodeManager : public CValidationInterface }; private: - CDeterministicMNCPtr mnListEntry; masternode_state_t state{MASTERNODE_WAITING_FOR_PROTX}; std::string strError; @@ -55,8 +54,6 @@ class CActiveMasternodeManager : public CValidationInterface void Init(); - CDeterministicMNCPtr GetDMN() const { return mnListEntry; } - std::string GetStateString() const; std::string GetStatus() const; diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index d9f879f9e560..259567432cda 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -402,7 +402,7 @@ UniValue masternode_status(const JSONRPCRequest& request) mnObj.push_back(Pair("outpoint", activeMasternodeInfo.outpoint.ToStringShort())); mnObj.push_back(Pair("service", activeMasternodeInfo.service.ToString())); - auto dmn = activeMasternodeManager->GetDMN(); + auto dmn = deterministicMNManager->GetListAtChainTip().GetMN(activeMasternodeInfo.proTxHash); if (dmn) { mnObj.push_back(Pair("proTxHash", dmn->proTxHash.ToString())); mnObj.push_back(Pair("collateralHash", dmn->collateralOutpoint.hash.ToString())); From 69e10ad58ed8544d5e64fb8ea70184e9a22929e9 Mon Sep 17 00:00:00 2001 From: Nathan Marley Date: Tue, 29 Oct 2019 13:19:55 -0300 Subject: [PATCH 6/6] Revert "Add `==` and `!=` operators to CDeterministicMNState" This reverts commit fba468758113084785c0bc0fffd6d704911be8a3. --- src/evo/deterministicmns.h | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index aae1453c3898..18c0289c96c3 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -112,28 +112,6 @@ class CDeterministicMNState public: std::string ToString() const; void ToJson(UniValue& obj) const; - - friend bool operator==(const CDeterministicMNState& a, const CDeterministicMNState& b) - { - return a.nRegisteredHeight == b.nRegisteredHeight - && a.nLastPaidHeight == b.nLastPaidHeight - && a.nPoSePenalty == b.nPoSePenalty - && a.nPoSeRevivedHeight == b.nPoSeRevivedHeight - && a.nPoSeBanHeight == b.nPoSeBanHeight - && a.nRevocationReason == b.nRevocationReason - && a.confirmedHash == b.confirmedHash - && a.confirmedHashWithProRegTxHash == b.confirmedHashWithProRegTxHash - && a.keyIDOwner == b.keyIDOwner - && a.pubKeyOperator == b.pubKeyOperator - && a.keyIDVoting == b.keyIDVoting - && a.addr == b.addr - && a.scriptPayout == b.scriptPayout - && a.scriptOperatorPayout == b.scriptOperatorPayout; - } - friend bool operator!=(const CDeterministicMNState& a, const CDeterministicMNState& b) - { - return !(a == b); - } }; typedef std::shared_ptr CDeterministicMNStatePtr; typedef std::shared_ptr CDeterministicMNStateCPtr;